[dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Olivier Matz]

Add support the set_mc_addr_list device operation in the i40evf PMD.

The configured addresses are stored in the device private area, so
they can be flushed before adding new ones.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---

Target is v18.05.

To reproduce the issue:

guest (testpmd)
+----------+
|          |
|    port1 |
|    VF    |
|      X   |
+------X---+
       X
+------X---+        +----------+
|      X   |        |          |
|    ens3f2+--------+ntfp2     |
|    PF    |        |          |
|          |        |          |
+----------+        +----------+
host                     tester
(linux)                  node


Start testpmd in guest in rx only mode:

 set fwd rxonly
 set verbose 1
 start

Send packets from the tester node:

 # broadcast packet is received
 arp = Ether(dst='ff:ff:ff:ff:ff:ff')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
 sendp(arp, iface="ntfp2")

 # unicast packet to the correct mac (VF) is received
 arp = Ether(dst='00:09:C0:38:6D:C2')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
 sendp(arp, iface="ntfp2")

 # multicast packet is not received
 arp = Ether(dst='33:33:00:01:00:02')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
 sendp(arp, iface="ntfp2")

Try to add the multicast address in testpmd:

 mcast_addr add 1 33:33:00:01:00:02

Without the patch, it fails (ENOTSUP).

With the patch, it is possible to add/remove several multicast addresses,
and the multicast packets sent by the tester node are properly received.


 drivers/net/i40e/i40e_ethdev.h    |   3 ++
 drivers/net/i40e/i40e_ethdev_vf.c | 100 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 69ea6c189..83c9d3b19 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -995,6 +995,9 @@ struct i40e_vf {
 	uint16_t promisc_flags; /* Promiscuous setting */
 	uint32_t vlan[I40E_VFTA_SIZE]; /* VLAN bit map */
 
+	struct ether_addr mc_addrs[I40E_NUM_MACADDR_MAX]; /* Multicast addrs */
+	uint16_t mc_addrs_num;   /* Multicast mac addresses number */
+
 	/* Event from pf */
 	bool dev_closed;
 	bool link_up;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 6ac3f8c04..0d73c1b7f 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -130,6 +130,14 @@ static void i40evf_handle_pf_event(struct rte_eth_dev *dev,
 				   uint8_t *msg,
 				   uint16_t msglen);
 
+static int
+i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
+			struct ether_addr *mc_addr_set,
+			uint32_t nb_mc_addr, bool add);
+static int
+i40evf_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr *mc_addr_set,
+			uint32_t nb_mc_addr);
+
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
 
@@ -195,6 +203,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops = {
 	.txq_info_get         = i40e_txq_info_get,
 	.mac_addr_add	      = i40evf_add_mac_addr,
 	.mac_addr_remove      = i40evf_del_mac_addr,
+	.set_mc_addr_list     = i40evf_set_mc_addr_list,
 	.reta_update          = i40evf_dev_rss_reta_update,
 	.reta_query           = i40evf_dev_rss_reta_query,
 	.rss_hash_update      = i40evf_dev_rss_hash_update,
@@ -2011,6 +2020,9 @@ i40evf_dev_start(struct rte_eth_dev *dev)
 
 	/* Set all mac addrs */
 	i40evf_add_del_all_mac_addr(dev, TRUE);
+	/* Set all multicast addresses */
+	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
+				TRUE);
 
 	if (i40evf_start_queues(dev) != 0) {
 		PMD_DRV_LOG(ERR, "enable queues failed");
@@ -2035,6 +2047,8 @@ i40evf_dev_start(struct rte_eth_dev *dev)
 
 err_mac:
 	i40evf_add_del_all_mac_addr(dev, FALSE);
+	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
+				FALSE);
 err_queue:
 	return -1;
 }
@@ -2045,6 +2059,7 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -2062,6 +2077,9 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
 	}
 	/* remove all mac addrs */
 	i40evf_add_del_all_mac_addr(dev, FALSE);
+	/* remove all multicast addresses */
+	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
+				FALSE);
 	hw->adapter_stopped = 1;
 
 }
@@ -2676,3 +2694,85 @@ i40evf_set_default_mac_addr(struct rte_eth_dev *dev,
 
 	ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr);
 }
+
+static int
+i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
+			struct ether_addr *mc_addrs,
+			uint32_t mc_addrs_num, bool add)
+{
+	struct virtchnl_ether_addr_list *list;
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	uint8_t cmd_buffer[sizeof(struct virtchnl_ether_addr_list) +
+		(I40E_NUM_MACADDR_MAX * sizeof(struct virtchnl_ether_addr))];
+	uint32_t i;
+	int err;
+	struct vf_cmd_info args;
+
+	if (mc_addrs == NULL || mc_addrs_num == 0)
+		return 0;
+
+	if (mc_addrs_num > I40E_NUM_MACADDR_MAX)
+		return -EINVAL;
+
+	list = (struct virtchnl_ether_addr_list *)cmd_buffer;
+	list->vsi_id = vf->vsi_res->vsi_id;
+	list->num_elements = mc_addrs_num;
+
+	for (i = 0; i < mc_addrs_num; i++) {
+		if (!I40E_IS_MULTICAST(mc_addrs[i].addr_bytes)) {
+			PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
+				    mc_addrs[i].addr_bytes[0],
+				    mc_addrs[i].addr_bytes[1],
+				    mc_addrs[i].addr_bytes[2],
+				    mc_addrs[i].addr_bytes[3],
+				    mc_addrs[i].addr_bytes[4],
+				    mc_addrs[i].addr_bytes[5]);
+			return -EINVAL;
+		}
+
+		memcpy(list->list[i].addr, mc_addrs[i].addr_bytes,
+			sizeof(list->list[i].addr));
+	}
+
+	args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR : VIRTCHNL_OP_DEL_ETH_ADDR;
+	args.in_args = cmd_buffer;
+	args.in_args_size = sizeof(struct virtchnl_ether_addr_list) +
+		i * sizeof(struct virtchnl_ether_addr);
+	args.out_buffer = vf->aq_resp;
+	args.out_size = I40E_AQ_BUF_SZ;
+	err = i40evf_execute_vf_cmd(dev, &args);
+	if (err) {
+		PMD_DRV_LOG(ERR, "fail to execute command %s",
+			add ? "OP_ADD_ETH_ADDR" : "OP_DEL_ETH_ADDR");
+		return err;
+	}
+
+	return 0;
+}
+
+static int
+i40evf_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr *mc_addrs,
+			uint32_t mc_addrs_num)
+{
+	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
+	int err;
+
+	/* flush previous addresses */
+	err = i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
+				FALSE);
+	if (err)
+		return err;
+
+	vf->mc_addrs_num = 0;
+
+	/* add new ones */
+	err = i40evf_add_del_mc_addr_list(dev, mc_addrs, mc_addrs_num,
+					TRUE);
+	if (err)
+		return err;
+
+	vf->mc_addrs_num = mc_addrs_num;
+	memcpy(vf->mc_addrs, mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+
+	return 0;
+}
-- 
2.11.0

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Zhang, Qi Z]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Thursday, January 25, 2018 10:36 PM
> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering
> 
> Add support the set_mc_addr_list device operation in the i40evf PMD.
> 
> The configured addresses are stored in the device private area, so they can be
> flushed before adding new ones.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Thanks for enable this!

> ---
> 
> Target is v18.05.
> 
> To reproduce the issue:
> 
> guest (testpmd)
> +----------+
> |          |
> |    port1 |
> |    VF    |
> |      X   |
> +------X---+
>        X
> +------X---+        +----------+
> |      X   |        |          |
> |    ens3f2+--------+ntfp2     |
> |    PF    |        |          |
> |          |        |          |
> +----------+        +----------+
> host                     tester
> (linux)                  node
> 
> 
> Start testpmd in guest in rx only mode:
> 
>  set fwd rxonly
>  set verbose 1
>  start
> 
> Send packets from the tester node:
> 
>  # broadcast packet is received
>  arp = Ether(dst='ff:ff:ff:ff:ff:ff')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
> sendp(arp, iface="ntfp2")
> 
>  # unicast packet to the correct mac (VF) is received  arp =
> Ether(dst='00:09:C0:38:6D:C2')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
> sendp(arp, iface="ntfp2")
> 
>  # multicast packet is not received
>  arp = Ether(dst='33:33:00:01:00:02')/ARP(psrc='1.1.1.2', pdst='1.1.1.3')
> sendp(arp, iface="ntfp2")
> 
> Try to add the multicast address in testpmd:
> 
>  mcast_addr add 1 33:33:00:01:00:02
> 
> Without the patch, it fails (ENOTSUP).
> 
> With the patch, it is possible to add/remove several multicast addresses, and
> the multicast packets sent by the tester node are properly received.
> 
> 
>  drivers/net/i40e/i40e_ethdev.h    |   3 ++
>  drivers/net/i40e/i40e_ethdev_vf.c | 100
> ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 69ea6c189..83c9d3b19 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -995,6 +995,9 @@ struct i40e_vf {
>  	uint16_t promisc_flags; /* Promiscuous setting */
>  	uint32_t vlan[I40E_VFTA_SIZE]; /* VLAN bit map */
> 
> +	struct ether_addr mc_addrs[I40E_NUM_MACADDR_MAX]; /* Multicast
> addrs */
> +	uint16_t mc_addrs_num;   /* Multicast mac addresses number */
> +
>  	/* Event from pf */
>  	bool dev_closed;
>  	bool link_up;
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 6ac3f8c04..0d73c1b7f 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -130,6 +130,14 @@ static void i40evf_handle_pf_event(struct
> rte_eth_dev *dev,
>  				   uint8_t *msg,
>  				   uint16_t msglen);
> 
> +static int
> +i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
> +			struct ether_addr *mc_addr_set,
> +			uint32_t nb_mc_addr, bool add);
> +static int
> +i40evf_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr
> *mc_addr_set,
> +			uint32_t nb_mc_addr);
> +
>  /* Default hash key buffer for RSS */
>  static uint32_t rss_key_default[I40E_VFQF_HKEY_MAX_INDEX + 1];
> 
> @@ -195,6 +203,7 @@ static const struct eth_dev_ops i40evf_eth_dev_ops
> = {
>  	.txq_info_get         = i40e_txq_info_get,
>  	.mac_addr_add	      = i40evf_add_mac_addr,
>  	.mac_addr_remove      = i40evf_del_mac_addr,
> +	.set_mc_addr_list     = i40evf_set_mc_addr_list,
>  	.reta_update          = i40evf_dev_rss_reta_update,
>  	.reta_query           = i40evf_dev_rss_reta_query,
>  	.rss_hash_update      = i40evf_dev_rss_hash_update,
> @@ -2011,6 +2020,9 @@ i40evf_dev_start(struct rte_eth_dev *dev)
> 
>  	/* Set all mac addrs */
>  	i40evf_add_del_all_mac_addr(dev, TRUE);
> +	/* Set all multicast addresses */
> +	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
> +				TRUE);
> 
>  	if (i40evf_start_queues(dev) != 0) {
>  		PMD_DRV_LOG(ERR, "enable queues failed"); @@ -2035,6 +2047,8
> @@ i40evf_dev_start(struct rte_eth_dev *dev)
> 
>  err_mac:
>  	i40evf_add_del_all_mac_addr(dev, FALSE);
> +	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
> +				FALSE);
>  err_queue:
>  	return -1;
>  }
> @@ -2045,6 +2059,7 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
>  	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct i40e_vf *vf =
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> @@ -2062,6 +2077,9 @@ i40evf_dev_stop(struct rte_eth_dev *dev)
>  	}
>  	/* remove all mac addrs */
>  	i40evf_add_del_all_mac_addr(dev, FALSE);
> +	/* remove all multicast addresses */
> +	i40evf_add_del_mc_addr_list(dev, vf->mc_addrs, vf->mc_addrs_num,
> +				FALSE);
>  	hw->adapter_stopped = 1;
> 
>  }
> @@ -2676,3 +2694,85 @@ i40evf_set_default_mac_addr(struct
> rte_eth_dev *dev,
> 
>  	ether_addr_copy(mac_addr, (struct ether_addr *)hw->mac.addr);  }
> +
> +static int
> +i40evf_add_del_mc_addr_list(struct rte_eth_dev *dev,
> +			struct ether_addr *mc_addrs,
> +			uint32_t mc_addrs_num, bool add)
> +{
> +	struct virtchnl_ether_addr_list *list;
> +	struct i40e_vf *vf =
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> +	uint8_t cmd_buffer[sizeof(struct virtchnl_ether_addr_list) +
> +		(I40E_NUM_MACADDR_MAX * sizeof(struct
> virtchnl_ether_addr))];
> +	uint32_t i;
> +	int err;
> +	struct vf_cmd_info args;
> +
> +	if (mc_addrs == NULL || mc_addrs_num == 0)
> +		return 0;
> +
> +	if (mc_addrs_num > I40E_NUM_MACADDR_MAX)
> +		return -EINVAL;
> +
> +	list = (struct virtchnl_ether_addr_list *)cmd_buffer;
> +	list->vsi_id = vf->vsi_res->vsi_id;
> +	list->num_elements = mc_addrs_num;
> +
> +	for (i = 0; i < mc_addrs_num; i++) {
> +		if (!I40E_IS_MULTICAST(mc_addrs[i].addr_bytes)) {
> +			PMD_DRV_LOG(ERR, "Invalid mac:%x:%x:%x:%x:%x:%x",
> +				    mc_addrs[i].addr_bytes[0],
> +				    mc_addrs[i].addr_bytes[1],
> +				    mc_addrs[i].addr_bytes[2],
> +				    mc_addrs[i].addr_bytes[3],
> +				    mc_addrs[i].addr_bytes[4],
> +				    mc_addrs[i].addr_bytes[5]);
> +			return -EINVAL;
> +		}
> +
> +		memcpy(list->list[i].addr, mc_addrs[i].addr_bytes,
> +			sizeof(list->list[i].addr));
> +	}
> +
> +	args.ops = add ? VIRTCHNL_OP_ADD_ETH_ADDR :
> VIRTCHNL_OP_DEL_ETH_ADDR;
> +	args.in_args = cmd_buffer;
> +	args.in_args_size = sizeof(struct virtchnl_ether_addr_list) +
> +		i * sizeof(struct virtchnl_ether_addr);
> +	args.out_buffer = vf->aq_resp;
> +	args.out_size = I40E_AQ_BUF_SZ;
> +	err = i40evf_execute_vf_cmd(dev, &args);
> +	if (err) {
> +		PMD_DRV_LOG(ERR, "fail to execute command %s",
> +			add ? "OP_ADD_ETH_ADDR" : "OP_DEL_ETH_ADDR");
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +i40evf_set_mc_addr_list(struct rte_eth_dev *dev, struct ether_addr
> *mc_addrs,
> +			uint32_t mc_addrs_num)
> +{
> +	struct i40e_vf *vf =
> I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
> +	int err;
> +
> +	/* flush previous addresses */
> +	err = i40evf_add_del_mc_addr_list(dev, vf->mc_addrs,
> vf->mc_addrs_num,
> +				FALSE);
> +	if (err)
> +		return err;
> +
> +	vf->mc_addrs_num = 0;
> +
> +	/* add new ones */
> +	err = i40evf_add_del_mc_addr_list(dev, mc_addrs, mc_addrs_num,
> +					TRUE);
> +	if (err)
> +		return err;
> +
> +	vf->mc_addrs_num = mc_addrs_num;
> +	memcpy(vf->mc_addrs, mc_addrs, mc_addrs_num *
> sizeof(*mc_addrs));
> +
> +	return 0;
> +}
> --
> 2.11.0

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Zhang, Helin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhang, Qi Z
> Sent: Tuesday, March 27, 2018 9:52 PM
> To: Olivier Matz; dev@dpdk.org; Xing, Beilei; Lu, Wenzhuo
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> filtering
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> > Sent: Thursday, January 25, 2018 10:36 PM
> > To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> > Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> > filtering
> >
> > Add support the set_mc_addr_list device operation in the i40evf PMD.
> >
> > The configured addresses are stored in the device private area, so
> > they can be flushed before adding new ones.
> >
> > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Applied to dpdk-next-net-intel, thanks!

/Helin

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Ferruh Yigit]

On 3/27/2018 2:51 PM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
>> Sent: Thursday, January 25, 2018 10:36 PM
>> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering
>>
>> Add support the set_mc_addr_list device operation in the i40evf PMD.
>>
>> The configured addresses are stored in the device private area, so they can be
>> flushed before adding new ones.
>>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Thanks for enable this!

Hi Qi,

This feature was already documented as supported in i40e_vf.ini which seems
wrong but now it is supported.

i40e also documents this feature as supported, can you please check if the
feature is supported there and update i40e.ini if required?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Zhang, Qi Z]

Hi Ferruh:

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, March 28, 2018 6:01 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> filtering
> 
> On 3/27/2018 2:51 PM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> >> Sent: Thursday, January 25, 2018 10:36 PM
> >> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> >> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> >> filtering
> >>
> >> Add support the set_mc_addr_list device operation in the i40evf PMD.
> >>
> >> The configured addresses are stored in the device private area, so
> >> they can be flushed before adding new ones.
> >>
> >> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > Thanks for enable this!
> 
> Hi Qi,
> 
> This feature was already documented as supported in i40e_vf.ini which
> seems wrong but now it is supported.
> 
> i40e also documents this feature as supported, can you please check if the
> feature is supported there and update i40e.ini if required?

Do you mean

" Multicast MAC filter = Y" ?

My understanding is, though i40e PF/VF doesn't support set_mc_addr_list, but we still can use mac_addr_add to add a multi cast address, so we still have that flag on in i40e.ini.

Regards
Qi

> 
> Thanks,
> ferruh

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Ferruh Yigit]

On 3/28/2018 11:15 AM, Zhang, Qi Z wrote:
> Hi Ferruh:
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, March 28, 2018 6:01 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
>> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
>> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
>> filtering
>>
>> On 3/27/2018 2:51 PM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
>>>> Sent: Thursday, January 25, 2018 10:36 PM
>>>> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>>>> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
>>>> filtering
>>>>
>>>> Add support the set_mc_addr_list device operation in the i40evf PMD.
>>>>
>>>> The configured addresses are stored in the device private area, so
>>>> they can be flushed before adding new ones.
>>>>
>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>
>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>
>>> Thanks for enable this!
>>
>> Hi Qi,
>>
>> This feature was already documented as supported in i40e_vf.ini which
>> seems wrong but now it is supported.
>>
>> i40e also documents this feature as supported, can you please check if the
>> feature is supported there and update i40e.ini if required?
> 
> Do you mean
> 
> " Multicast MAC filter = Y" ?
> 
> My understanding is, though i40e PF/VF doesn't support set_mc_addr_list, but we still can use mac_addr_add to add a multi cast address, so we still have that flag on in i40e.ini.

Right, current "mac_addr_add" dev_ops lets setting multicast addresses. In this
logic if HW supports setting multicast address PMD may claim this support.

Or this may explicitly mean implementing set_mc_addr_list dev_ops in driver.


These feature list is to help user / application developer about what to expect
and what to not from drivers.

A user may expect set_mc_addr_list dev_ops is implemented when this feature
documented and it may be confusing when it fails.

Overall which information, hw supports setting mc mad address or
set_mc_addr_list implemented, adds more value from user point of view? What do
you think?

> 
> Regards
> Qi
> 
>>
>> Thanks,
>> ferruh

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Zhang, Qi Z]

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, March 28, 2018 9:53 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas
> Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> filtering
> 
> On 3/28/2018 11:15 AM, Zhang, Qi Z wrote:
> > Hi Ferruh:
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Wednesday, March 28, 2018 6:01 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
> >> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
> >> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> >> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> >> filtering
> >>
> >> On 3/27/2018 2:51 PM, Zhang, Qi Z wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> >>>> Sent: Thursday, January 25, 2018 10:36 PM
> >>>> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> >>>> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
> >>>> filtering
> >>>>
> >>>> Add support the set_mc_addr_list device operation in the i40evf PMD.
> >>>>
> >>>> The configured addresses are stored in the device private area, so
> >>>> they can be flushed before adding new ones.
> >>>>
> >>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >>>
> >>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >>>
> >>> Thanks for enable this!
> >>
> >> Hi Qi,
> >>
> >> This feature was already documented as supported in i40e_vf.ini which
> >> seems wrong but now it is supported.
> >>
> >> i40e also documents this feature as supported, can you please check
> >> if the feature is supported there and update i40e.ini if required?
> >
> > Do you mean
> >
> > " Multicast MAC filter = Y" ?
> >
> > My understanding is, though i40e PF/VF doesn't support set_mc_addr_list,
> but we still can use mac_addr_add to add a multi cast address, so we still
> have that flag on in i40e.ini.
> 
> Right, current "mac_addr_add" dev_ops lets setting multicast addresses. In
> this logic if HW supports setting multicast address PMD may claim this
> support.
> 
> Or this may explicitly mean implementing set_mc_addr_list dev_ops in
> driver.
> 
> 
> These feature list is to help user / application developer about what to expect
> and what to not from drivers.
> 
> A user may expect set_mc_addr_list dev_ops is implemented when this
> feature documented and it may be confusing when it fails.
> 
> Overall which information, hw supports setting mc mad address or
> set_mc_addr_list implemented, adds more value from user point of view?
> What do you think?

I'm not sure if it is feasible to bind "Multicast MAC filter = Y" to set_mc_addr_list's implementation only in document.
For me, I think it's reasonable to support two ways to enable mc filters
set_mc_addr_list support batch, but it also overwrite, in the case user don't want to overwrite, mac_addr_add could be taken.
so for i40e, it just not support batch overwrite and if just set "Multicast MAC filter = N" seems a little bit unfair to i40e, 
properly we should update the document?

> 
> >
> > Regards
> > Qi
> >
> >>
> >> Thanks,
> >> ferruh

Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address filtering

[Ferruh Yigit]

On 3/28/2018 3:23 PM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Wednesday, March 28, 2018 9:53 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
>> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
>> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Thomas
>> Monjalon <thomas@monjalon.net>
>> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
>> filtering
>>
>> On 3/28/2018 11:15 AM, Zhang, Qi Z wrote:
>>> Hi Ferruh:
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh
>>>> Sent: Wednesday, March 28, 2018 6:01 PM
>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Olivier Matz
>>>> <olivier.matz@6wind.com>; dev@dpdk.org; Xing, Beilei
>>>> <beilei.xing@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
>>>> filtering
>>>>
>>>> On 3/27/2018 2:51 PM, Zhang, Qi Z wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
>>>>>> Sent: Thursday, January 25, 2018 10:36 PM
>>>>>> To: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
>>>>>> <qi.z.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>>>>>> Subject: [dpdk-dev] [PATCH] net/i40evf: add multicast MAC address
>>>>>> filtering
>>>>>>
>>>>>> Add support the set_mc_addr_list device operation in the i40evf PMD.
>>>>>>
>>>>>> The configured addresses are stored in the device private area, so
>>>>>> they can be flushed before adding new ones.
>>>>>>
>>>>>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>>>>>
>>>>> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>>>>>
>>>>> Thanks for enable this!
>>>>
>>>> Hi Qi,
>>>>
>>>> This feature was already documented as supported in i40e_vf.ini which
>>>> seems wrong but now it is supported.
>>>>
>>>> i40e also documents this feature as supported, can you please check
>>>> if the feature is supported there and update i40e.ini if required?
>>>
>>> Do you mean
>>>
>>> " Multicast MAC filter = Y" ?
>>>
>>> My understanding is, though i40e PF/VF doesn't support set_mc_addr_list,
>> but we still can use mac_addr_add to add a multi cast address, so we still
>> have that flag on in i40e.ini.
>>
>> Right, current "mac_addr_add" dev_ops lets setting multicast addresses. In
>> this logic if HW supports setting multicast address PMD may claim this
>> support.
>>
>> Or this may explicitly mean implementing set_mc_addr_list dev_ops in
>> driver.
>>
>>
>> These feature list is to help user / application developer about what to expect
>> and what to not from drivers.
>>
>> A user may expect set_mc_addr_list dev_ops is implemented when this
>> feature documented and it may be confusing when it fails.
>>
>> Overall which information, hw supports setting mc mad address or
>> set_mc_addr_list implemented, adds more value from user point of view?
>> What do you think?
> 
> I'm not sure if it is feasible to bind "Multicast MAC filter = Y" to set_mc_addr_list's implementation only in document.
> For me, I think it's reasonable to support two ways to enable mc filters
> set_mc_addr_list support batch, but it also overwrite, in the case user don't want to overwrite, mac_addr_add could be taken.
> so for i40e, it just not support batch overwrite and if just set "Multicast MAC filter = N" seems a little bit unfair to i40e, 
> properly we should update the document?

As long as agreed on it, no problem on updating the document [1].

[1]
https://dpdk.org/doc/guides/nics/features.html

> 
>>
>>>
>>> Regards
>>> Qi
>>>
>>>>
>>>> Thanks,
>>>> ferruh
>