[dpdk-dev,1/5] ethdev: add multicast address filtering

Message ID 1432825523-19006-2-git-send-email-ivan.boule@6wind.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Ivan Boule May 28, 2015, 3:05 p.m. UTC
  With the current PMD API, the receipt of multicast packets on a given
port can only be enabled by invoking the "rte_eth_allmulticast_enable"
function.
This method may not work on Virtual Functions in SR-IOV architectures
when the host PF driver does not allow such operation on VFs.
In such cases, joined multicast addresses must be individually added
in the set of multicast addresses that are filtered by the [VF] port.

For this purpose, a new function "set_mc_addr_list" is introduced
into the set of functions that are exported by a Poll Mode Driver.

Signed-off-by: Ivan Boule <ivan.boule@6wind.com>
---
 lib/librte_ether/rte_ethdev.c |   17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h |   26 ++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)
  

Comments

Stephen Hemminger May 28, 2015, 4:22 p.m. UTC | #1
On Thu, 28 May 2015 17:05:19 +0200
Ivan Boule <ivan.boule@6wind.com> wrote:

> +	if (port_id >= nb_ports) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -ENODEV;
> +	}
> +

Use rte_eth_dev_is_valid_port() function instead.
  
Ivan Boule May 29, 2015, 7:52 a.m. UTC | #2
On 05/28/2015 06:22 PM, Stephen Hemminger wrote:
> On Thu, 28 May 2015 17:05:19 +0200
> Ivan Boule <ivan.boule@6wind.com> wrote:
>
>> +	if (port_id >= nb_ports) {
>> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
>> +		return -ENODEV;
>> +	}
>> +
>
> Use rte_eth_dev_is_valid_port() function instead.
>
I missed that.
Thanks.
  
Thomas Monjalon May 29, 2015, 1:12 p.m. UTC | #3
Hi Stephen,

Looking at mac address management, you and Changchun added an entry in
driver ops to be able to change the default mac address with virtio:
	http://dpdk.org/browse/dpdk/commit/?id=5186fb1f37fe986
Other ops functions (mac_addr_add/remove) manage the secondary unicast
mac addresses and have a wrapper function in the API:
	http://dpdk.org/doc/api/rte__ethdev_8h.html#aa2b81750086f5f9e55cf65e5cf9f2c58

It seems now that the review of the above patch was too weak and I'd like
these issues to be fixed:
	- mac_addr_set must be wrapped by rte_eth_dev_mac_addr_set()
	- eth_mac_addr_set_t must be fixed to explicitly state that it
replaces the default address

I'm wondering what was the first intent since virtio_mac_addr_set() is
never called?
  
Stephen Hemminger May 29, 2015, 3:18 p.m. UTC | #4
On Fri, 29 May 2015 15:12:28 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> Hi Stephen,
> 
> Looking at mac address management, you and Changchun added an entry in
> driver ops to be able to change the default mac address with virtio:
> 	http://dpdk.org/browse/dpdk/commit/?id=5186fb1f37fe986
> Other ops functions (mac_addr_add/remove) manage the secondary unicast
> mac addresses and have a wrapper function in the API:
> 	http://dpdk.org/doc/api/rte__ethdev_8h.html#aa2b81750086f5f9e55cf65e5cf9f2c58
> 
> It seems now that the review of the above patch was too weak and I'd like
> these issues to be fixed:
> 	- mac_addr_set must be wrapped by rte_eth_dev_mac_addr_set()
> 	- eth_mac_addr_set_t must be fixed to explicitly state that it
> replaces the default address

Sure, also some other drivers need mac_addr_set hook.
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 024fe8b..f5784de 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3628,3 +3628,20 @@  rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
 	/* Callback wasn't found. */
 	return -EINVAL;
 }
+
+int
+rte_eth_dev_set_mc_addr_list(uint8_t port_id,
+			     struct ether_addr *mc_addr_set,
+			     uint32_t nb_mc_addr)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_mc_addr_list, -ENOTSUP);
+	return dev->dev_ops->set_mc_addr_list(dev, mc_addr_set, nb_mc_addr);
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..04c192d 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1228,6 +1228,10 @@  typedef int (*eth_udp_tunnel_del_t)(struct rte_eth_dev *dev,
 				    struct rte_eth_udp_tunnel *tunnel_udp);
 /**< @internal Delete tunneling UDP info */
 
+typedef int (*eth_set_mc_addr_list_t)(struct rte_eth_dev *dev,
+				      struct ether_addr *mc_addr_set,
+				      uint32_t nb_mc_addr);
+/**< @internal set the list of multicast addresses on an Ethernet device */
 
 #ifdef RTE_NIC_BYPASS
 
@@ -1386,6 +1390,7 @@  struct eth_dev_ops {
 	/** Get current RSS hash configuration. */
 	rss_hash_conf_get_t rss_hash_conf_get;
 	eth_filter_ctrl_t              filter_ctrl;          /**< common filter control*/
+	eth_set_mc_addr_list_t set_mc_addr_list; /**< set list of mcast addrs */
 };
 
 /**
@@ -3615,4 +3620,25 @@  int rte_eth_remove_tx_callback(uint8_t port_id, uint16_t queue_id,
 }
 #endif
 
+/**
+ * Set the list of multicast addresses to filter on an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param mc_addr_set
+ *   The array of multicast addresses to set. Equal to NULL when the function
+ *   is invoked to flush the set of filtered addresses.
+ * @param nb_mc_addr
+ *   The number of multicast addresses in the *mc_addr_set* array. Equal to 0
+ *   when the function is invoked to flush the set of filtered addresses.
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if *port_id* invalid.
+ *   - (-ENOTSUP) if PMD of *port_id* doesn't support multicast filtering.
+ *   - (-ENOSPC) if *port_id* has not enough multicast filtering resources.
+ */
+int rte_eth_dev_set_mc_addr_list(uint8_t port_id,
+				 struct ether_addr *mc_addr_set,
+				 uint32_t nb_mc_addr);
+
 #endif /* _RTE_ETHDEV_H_ */