DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
@ 2020-10-15  2:02 Guinan Sun
  2020-10-15  5:13 ` Peng, Yuan
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Guinan Sun @ 2020-10-15  2:02 UTC (permalink / raw)
  To: dev; +Cc: Beilei Xing, Jingjing Wu, Qi Zhang, Guinan Sun

When the multicast address is added, it will flush
previous addresses first, and then add new ones.
So when adding an address that exceeds the upper limit
causes a failure, you need to add the previous address
list back. This patch fixes the issue.

Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")

Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
 drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
 drivers/net/iavf/iavf_vchnl.c  |  3 ---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index e68e3bc71..042edadd9 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	int err;
+	int err, temp_err;
+
+	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
+		PMD_DRV_LOG(ERR,
+			    "can't add more than a limited number (%u) of addresses.",
+			    (uint32_t)IAVF_NUM_MACADDR_MAX);
+		return -EINVAL;
+	}
 
 	/* flush previous addresses */
 	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf->mc_addrs_num,
@@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	if (err)
 		return err;
 
-	vf->mc_addrs_num = 0;
-
 	/* add new ones */
 	err = iavf_add_del_mc_addr_list(adapter, 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));
+	if (err) {
+		/* When adding new addresses fail, need to add the
+		 * previous addresses back.
+		 */
+		temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
+						     vf->mc_addrs_num, true);
+		if (temp_err)
+			return temp_err;
+	} else {
+		vf->mc_addrs_num = mc_addrs_num;
+		memcpy(vf->mc_addrs,
+		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+	}
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index db0b76876..a2295f879 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 	if (mc_addrs == NULL || mc_addrs_num == 0)
 		return 0;
 
-	if (mc_addrs_num > IAVF_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;
-- 
2.13.6


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
  2020-10-15  2:02 [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address Guinan Sun
@ 2020-10-15  5:13 ` Peng, Yuan
  2020-10-15  5:37   ` Xing, Beilei
  2020-10-15  6:55 ` [dpdk-dev] [PATCH v2] " Guinan Sun
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Peng, Yuan @ 2020-10-15  5:13 UTC (permalink / raw)
  To: Sun, GuinanX, dev; +Cc: Xing, Beilei, Wu, Jingjing, Zhang, Qi Z, Sun, GuinanX

Test-by Peng, Yuan <yuan.peng@intel.com>


-----Original Message-----
From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun
Sent: Thursday, October 15, 2020 10:02 AM
To: dev@dpdk.org
Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address

When the multicast address is added, it will flush previous addresses first, and then add new ones.
So when adding an address that exceeds the upper limit causes a failure, you need to add the previous address list back. This patch fixes the issue.

Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")

Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
---
 drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------  drivers/net/iavf/iavf_vchnl.c  |  3 ---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	int err;
+	int err, temp_err;
+
+	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
+		PMD_DRV_LOG(ERR,
+			    "can't add more than a limited number (%u) of addresses.",
+			    (uint32_t)IAVF_NUM_MACADDR_MAX);
+		return -EINVAL;
+	}
 
 	/* flush previous addresses */
 	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf->mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	if (err)
 		return err;
 
-	vf->mc_addrs_num = 0;
-
 	/* add new ones */
 	err = iavf_add_del_mc_addr_list(adapter, 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));
+	if (err) {
+		/* When adding new addresses fail, need to add the
+		 * previous addresses back.
+		 */
+		temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
+						     vf->mc_addrs_num, true);
+		if (temp_err)
+			return temp_err;
+	} else {
+		vf->mc_addrs_num = mc_addrs_num;
+		memcpy(vf->mc_addrs,
+		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+	}
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index db0b76876..a2295f879 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 	if (mc_addrs == NULL || mc_addrs_num == 0)
 		return 0;
 
-	if (mc_addrs_num > IAVF_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;
--
2.13.6


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
  2020-10-15  5:13 ` Peng, Yuan
@ 2020-10-15  5:37   ` Xing, Beilei
  2020-10-15  5:57     ` Sun, GuinanX
  0 siblings, 1 reply; 11+ messages in thread
From: Xing, Beilei @ 2020-10-15  5:37 UTC (permalink / raw)
  To: Peng, Yuan, Sun, GuinanX, dev; +Cc: Wu, Jingjing, Zhang, Qi Z, Sun, GuinanX



> -----Original Message-----
> From: Peng, Yuan <yuan.peng@intel.com>
> Sent: Thursday, October 15, 2020 1:14 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> Test-by Peng, Yuan <yuan.peng@intel.com>
> 
> 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun
> Sent: Thursday, October 15, 2020 10:02 AM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> When the multicast address is added, it will flush previous addresses first, and
> then add new ones.
> So when adding an address that exceeds the upper limit causes a failure, you
> need to add the previous address list back. This patch fixes the issue.
> 
> Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> 
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> ---
>  drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
> drivers/net/iavf/iavf_vchnl.c  |  3 ---
>  2 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index e68e3bc71..042edadd9 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
>  	struct iavf_adapter *adapter =
>  		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -	int err;
> +	int err, temp_err;
> +
> +	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
> +		PMD_DRV_LOG(ERR,
> +			    "can't add more than a limited number (%u) of
> addresses.",
> +			    (uint32_t)IAVF_NUM_MACADDR_MAX);
> +		return -EINVAL;
> +	}
> 
>  	/* flush previous addresses */
>  	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> rte_eth_dev *dev,
>  	if (err)
>  		return err;
> 
> -	vf->mc_addrs_num = 0;
> -
>  	/* add new ones */
>  	err = iavf_add_del_mc_addr_list(adapter, 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));
> +	if (err) {
> +		/* When adding new addresses fail, need to add the
> +		 * previous addresses back.
> +		 */
> +		temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> +						     vf->mc_addrs_num, true);

Can we reuse err here?

> +		if (temp_err)
> +			return temp_err;
> +	} else {
> +		vf->mc_addrs_num = mc_addrs_num;
> +		memcpy(vf->mc_addrs,
> +		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
> +	}
> 
> -	return 0;
> +	return err;
>  }
> 
>  static int
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> db0b76876..a2295f879 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter
> *adapter,
>  	if (mc_addrs == NULL || mc_addrs_num == 0)
>  		return 0;
> 
> -	if (mc_addrs_num > IAVF_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;
> --
> 2.13.6


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
  2020-10-15  5:37   ` Xing, Beilei
@ 2020-10-15  5:57     ` Sun, GuinanX
  2020-10-15  6:43       ` Xing, Beilei
  0 siblings, 1 reply; 11+ messages in thread
From: Sun, GuinanX @ 2020-10-15  5:57 UTC (permalink / raw)
  To: Xing, Beilei, Peng, Yuan, dev; +Cc: Wu, Jingjing, Zhang, Qi Z

Hi beilei

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Thursday, October 15, 2020 1:37 PM
> To: Peng, Yuan <yuan.peng@intel.com>; Sun, GuinanX
> <guinanx.sun@intel.com>; dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Sun, GuinanX <guinanx.sun@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> 
> 
> > -----Original Message-----
> > From: Peng, Yuan <yuan.peng@intel.com>
> > Sent: Thursday, October 15, 2020 1:14 PM
> > To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > GuinanX <guinanx.sun@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > address
> >
> > Test-by Peng, Yuan <yuan.peng@intel.com>
> >
> >
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun
> > Sent: Thursday, October 15, 2020 10:02 AM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > GuinanX <guinanx.sun@intel.com>
> > Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> >
> > When the multicast address is added, it will flush previous addresses
> > first, and then add new ones.
> > So when adding an address that exceeds the upper limit causes a
> > failure, you need to add the previous address list back. This patch fixes the
> issue.
> >
> > Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> >
> > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > ---
> >  drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
> > drivers/net/iavf/iavf_vchnl.c  |  3 ---
> >  2 files changed, 22 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
> > --- a/drivers/net/iavf/iavf_ethdev.c
> > +++ b/drivers/net/iavf/iavf_ethdev.c
> > @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
> > struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> > >dev_private);
> >  struct iavf_adapter *adapter =
> >  IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > -int err;
> > +int err, temp_err;
> > +
> > +if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) { PMD_DRV_LOG(ERR,
> > +    "can't add more than a limited number (%u) of
> > addresses.",
> > +    (uint32_t)IAVF_NUM_MACADDR_MAX);
> > +return -EINVAL;
> > +}
> >
> >  /* flush previous addresses */
> >  err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> > rte_eth_dev *dev,
> >  if (err)
> >  return err;
> >
> > -vf->mc_addrs_num = 0;
> > -
> >  /* add new ones */
> >  err = iavf_add_del_mc_addr_list(adapter, 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));
> > +if (err) {
> > +/* When adding new addresses fail, need to add the
> > + * previous addresses back.
> > + */
> > +temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> > +     vf->mc_addrs_num, true);
> 
> Can we reuse err here?

This err cannot be reused.

When performing mac address addition, the testpmd side will first add the mac address to mac_pool.
When the driver layer returns that it failed to add an address, the tespmd layer will delete the failed address from mac_pool.

If err is reused, the successful result of executing the add back address will overwrite the previous err, causing problems with the address stored in mac_pool on the testpmd side.

> 
> > +if (temp_err)
> > +return temp_err;
> > +} else {
> > +vf->mc_addrs_num = mc_addrs_num;
> > +memcpy(vf->mc_addrs,
> > +       mc_addrs, mc_addrs_num * sizeof(*mc_addrs)); }
> >
> > -return 0;
> > +return err;
> >  }
> >
> >  static int
> > diff --git a/drivers/net/iavf/iavf_vchnl.c
> > b/drivers/net/iavf/iavf_vchnl.c index
> > db0b76876..a2295f879 100644
> > --- a/drivers/net/iavf/iavf_vchnl.c
> > +++ b/drivers/net/iavf/iavf_vchnl.c
> > @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter
> > *adapter,  if (mc_addrs == NULL || mc_addrs_num == 0)  return 0;
> >
> > -if (mc_addrs_num > IAVF_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;
> > --
> > 2.13.6
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
  2020-10-15  5:57     ` Sun, GuinanX
@ 2020-10-15  6:43       ` Xing, Beilei
  2020-10-15  6:46         ` Sun, GuinanX
  0 siblings, 1 reply; 11+ messages in thread
From: Xing, Beilei @ 2020-10-15  6:43 UTC (permalink / raw)
  To: Sun, GuinanX, Peng, Yuan, dev; +Cc: Wu, Jingjing, Zhang, Qi Z



> -----Original Message-----
> From: Sun, GuinanX <guinanx.sun@intel.com>
> Sent: Thursday, October 15, 2020 1:57 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Peng, Yuan <yuan.peng@intel.com>;
> dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> Hi beilei
> 
> > -----Original Message-----
> > From: Xing, Beilei <beilei.xing@intel.com>
> > Sent: Thursday, October 15, 2020 1:37 PM
> > To: Peng, Yuan <yuan.peng@intel.com>; Sun, GuinanX
> > <guinanx.sun@intel.com>; dev@dpdk.org
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > address
> >
> >
> >
> > > -----Original Message-----
> > > From: Peng, Yuan <yuan.peng@intel.com>
> > > Sent: Thursday, October 15, 2020 1:14 PM
> > > To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > > GuinanX <guinanx.sun@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > address
> > >
> > > Test-by Peng, Yuan <yuan.peng@intel.com>
> > >
> > >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun
> > > Sent: Thursday, October 15, 2020 10:02 AM
> > > To: dev@dpdk.org
> > > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > > GuinanX <guinanx.sun@intel.com>
> > > Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > address
> > >
> > > When the multicast address is added, it will flush previous
> > > addresses first, and then add new ones.
> > > So when adding an address that exceeds the upper limit causes a
> > > failure, you need to add the previous address list back. This patch
> > > fixes the
> > issue.
> > >
> > > Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> > >
> > > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > > ---
> > >  drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
> > > drivers/net/iavf/iavf_vchnl.c  |  3 ---
> > >  2 files changed, 22 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > > b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
> > > --- a/drivers/net/iavf/iavf_ethdev.c
> > > +++ b/drivers/net/iavf/iavf_ethdev.c
> > > @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
> > > struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> > > >dev_private);
> > >  struct iavf_adapter *adapter =
> > >  IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > > -int err;
> > > +int err, temp_err;
> > > +
> > > +if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) { PMD_DRV_LOG(ERR,
> > > +    "can't add more than a limited number (%u) of
> > > addresses.",
> > > +    (uint32_t)IAVF_NUM_MACADDR_MAX); return -EINVAL; }
> > >
> > >  /* flush previous addresses */
> > >  err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > > >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> > > rte_eth_dev *dev,
> > >  if (err)
> > >  return err;
> > >
> > > -vf->mc_addrs_num = 0;
> > > -
> > >  /* add new ones */
> > >  err = iavf_add_del_mc_addr_list(adapter, 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));
> > > +if (err) {
> > > +/* When adding new addresses fail, need to add the
> > > + * previous addresses back.
> > > + */
> > > +temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> > > +     vf->mc_addrs_num, true);
> >
> > Can we reuse err here?
> 
> This err cannot be reused.
> 
> When performing mac address addition, the testpmd side will first add the
> mac address to mac_pool.
> When the driver layer returns that it failed to add an address, the tespmd layer
> will delete the failed address from mac_pool.
> 
> If err is reused, the successful result of executing the add back address will
> overwrite the previous err, causing problems with the address stored in
> mac_pool on the testpmd side.
> 

Yes, I missed return err in the end of the function.
Little comments, can we use 'ret' which is more common to indicate return value?

> >
> > > +if (temp_err)
> > > +return temp_err;
> > > +} else {
> > > +vf->mc_addrs_num = mc_addrs_num;
> > > +memcpy(vf->mc_addrs,
> > > +       mc_addrs, mc_addrs_num * sizeof(*mc_addrs)); }
> > >
> > > -return 0;
> > > +return err;
> > >  }
> > >
> > >  static int
> > > diff --git a/drivers/net/iavf/iavf_vchnl.c
> > > b/drivers/net/iavf/iavf_vchnl.c index
> > > db0b76876..a2295f879 100644
> > > --- a/drivers/net/iavf/iavf_vchnl.c
> > > +++ b/drivers/net/iavf/iavf_vchnl.c
> > > @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter
> > > *adapter,  if (mc_addrs == NULL || mc_addrs_num == 0)  return 0;
> > >
> > > -if (mc_addrs_num > IAVF_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;
> > > --
> > > 2.13.6
> >
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
  2020-10-15  6:43       ` Xing, Beilei
@ 2020-10-15  6:46         ` Sun, GuinanX
  0 siblings, 0 replies; 11+ messages in thread
From: Sun, GuinanX @ 2020-10-15  6:46 UTC (permalink / raw)
  To: Xing, Beilei, Peng, Yuan, dev; +Cc: Wu, Jingjing, Zhang, Qi Z

Hi Beilei

> -----Original Message-----
> From: Xing, Beilei <beilei.xing@intel.com>
> Sent: Thursday, October 15, 2020 2:44 PM
> To: Sun, GuinanX <guinanx.sun@intel.com>; Peng, Yuan
> <yuan.peng@intel.com>; dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address
> 
> 
> 
> > -----Original Message-----
> > From: Sun, GuinanX <guinanx.sun@intel.com>
> > Sent: Thursday, October 15, 2020 1:57 PM
> > To: Xing, Beilei <beilei.xing@intel.com>; Peng, Yuan
> > <yuan.peng@intel.com>; dev@dpdk.org
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > address
> >
> > Hi beilei
> >
> > > -----Original Message-----
> > > From: Xing, Beilei <beilei.xing@intel.com>
> > > Sent: Thursday, October 15, 2020 1:37 PM
> > > To: Peng, Yuan <yuan.peng@intel.com>; Sun, GuinanX
> > > <guinanx.sun@intel.com>; dev@dpdk.org
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > address
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Peng, Yuan <yuan.peng@intel.com>
> > > > Sent: Thursday, October 15, 2020 1:14 PM
> > > > To: Sun, GuinanX <guinanx.sun@intel.com>; dev@dpdk.org
> > > > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > > > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > > > GuinanX <guinanx.sun@intel.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > > address
> > > >
> > > > Test-by Peng, Yuan <yuan.peng@intel.com>
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Guinan Sun
> > > > Sent: Thursday, October 15, 2020 10:02 AM
> > > > To: dev@dpdk.org
> > > > Cc: Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing
> > > > <jingjing.wu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Sun,
> > > > GuinanX <guinanx.sun@intel.com>
> > > > Subject: [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC
> > > > address
> > > >
> > > > When the multicast address is added, it will flush previous
> > > > addresses first, and then add new ones.
> > > > So when adding an address that exceeds the upper limit causes a
> > > > failure, you need to add the previous address list back. This
> > > > patch fixes the
> > > issue.
> > > >
> > > > Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> > > >
> > > > Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> > > > ---
> > > >  drivers/net/iavf/iavf_ethdev.c | 30
> > > > ++++++++++++++++++++++-------- drivers/net/iavf/iavf_vchnl.c  |  3
> > > > ---
> > > >  2 files changed, 22 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > > > b/drivers/net/iavf/iavf_ethdev.c index e68e3bc71..042edadd9 100644
> > > > --- a/drivers/net/iavf/iavf_ethdev.c
> > > > +++ b/drivers/net/iavf/iavf_ethdev.c
> > > > @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev
> > > > *dev, struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> > > > >dev_private);
> > > >  struct iavf_adapter *adapter =
> > > >  IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > > > -int err;
> > > > +int err, temp_err;
> > > > +
> > > > +if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) { PMD_DRV_LOG(ERR,
> > > > +    "can't add more than a limited number (%u) of
> > > > addresses.",
> > > > +    (uint32_t)IAVF_NUM_MACADDR_MAX); return -EINVAL; }
> > > >
> > > >  /* flush previous addresses */
> > > >  err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> > > > >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> > > > rte_eth_dev *dev,
> > > >  if (err)
> > > >  return err;
> > > >
> > > > -vf->mc_addrs_num = 0;
> > > > -
> > > >  /* add new ones */
> > > >  err = iavf_add_del_mc_addr_list(adapter, 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));
> > > > +if (err) {
> > > > +/* When adding new addresses fail, need to add the
> > > > + * previous addresses back.
> > > > + */
> > > > +temp_err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> > > > +     vf->mc_addrs_num, true);
> > >
> > > Can we reuse err here?
> >
> > This err cannot be reused.
> >
> > When performing mac address addition, the testpmd side will first add
> > the mac address to mac_pool.
> > When the driver layer returns that it failed to add an address, the
> > tespmd layer will delete the failed address from mac_pool.
> >
> > If err is reused, the successful result of executing the add back
> > address will overwrite the previous err, causing problems with the
> > address stored in mac_pool on the testpmd side.
> >
> 
> Yes, I missed return err in the end of the function.
> Little comments, can we use 'ret' which is more common to indicate return
> value?

OK, patch V2 will fix it.

> 
> > >
> > > > +if (temp_err)
> > > > +return temp_err;
> > > > +} else {
> > > > +vf->mc_addrs_num = mc_addrs_num;
> > > > +memcpy(vf->mc_addrs,
> > > > +       mc_addrs, mc_addrs_num * sizeof(*mc_addrs)); }
> > > >
> > > > -return 0;
> > > > +return err;
> > > >  }
> > > >
> > > >  static int
> > > > diff --git a/drivers/net/iavf/iavf_vchnl.c
> > > > b/drivers/net/iavf/iavf_vchnl.c index
> > > > db0b76876..a2295f879 100644
> > > > --- a/drivers/net/iavf/iavf_vchnl.c
> > > > +++ b/drivers/net/iavf/iavf_vchnl.c
> > > > @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct
> > > > iavf_adapter *adapter,  if (mc_addrs == NULL || mc_addrs_num == 0)
> > > > return 0;
> > > >
> > > > -if (mc_addrs_num > IAVF_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;
> > > > --
> > > > 2.13.6
> > >
> >
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v2] net/iavf: fix adding multicast MAC address
  2020-10-15  2:02 [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address Guinan Sun
  2020-10-15  5:13 ` Peng, Yuan
@ 2020-10-15  6:55 ` Guinan Sun
  2020-10-15  8:13 ` [dpdk-dev] [PATCH v3] " Guinan Sun
  2020-10-15  8:43 ` [dpdk-dev] [PATCH v4] " Guinan Sun
  3 siblings, 0 replies; 11+ messages in thread
From: Guinan Sun @ 2020-10-15  6:55 UTC (permalink / raw)
  To: dev; +Cc: Beilei Xing, Qi Zhang, Jingjing Wu, Guinan Sun

When the multicast address is added, it will flush
previous addresses first, and then add new ones.
So when adding an address that exceeds the upper limit
causes a failure, you need to add the previous address
list back. This patch fixes the issue.

Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")

Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
Tested-by: Peng Yuan <yuan.peng@intel.com>
---
v2:
* modify the variable name
---
 drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
 drivers/net/iavf/iavf_vchnl.c  |  3 ---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index e68e3bc71..596cf24cf 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	int err;
+	int err, ret;
+
+	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
+		PMD_DRV_LOG(ERR,
+			    "can't add more than a limited number (%u) of addresses.",
+			    (uint32_t)IAVF_NUM_MACADDR_MAX);
+		return -EINVAL;
+	}
 
 	/* flush previous addresses */
 	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf->mc_addrs_num,
@@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	if (err)
 		return err;
 
-	vf->mc_addrs_num = 0;
-
 	/* add new ones */
 	err = iavf_add_del_mc_addr_list(adapter, 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));
+	if (err) {
+		/* When adding new addresses fail, need to add the
+		 * previous addresses back.
+		 */
+		ret = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
+						vf->mc_addrs_num, true);
+		if (ret)
+			return ret;
+	} else {
+		vf->mc_addrs_num = mc_addrs_num;
+		memcpy(vf->mc_addrs,
+		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+	}
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index db0b76876..a2295f879 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 	if (mc_addrs == NULL || mc_addrs_num == 0)
 		return 0;
 
-	if (mc_addrs_num > IAVF_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;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v3] net/iavf: fix adding multicast MAC address
  2020-10-15  2:02 [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address Guinan Sun
  2020-10-15  5:13 ` Peng, Yuan
  2020-10-15  6:55 ` [dpdk-dev] [PATCH v2] " Guinan Sun
@ 2020-10-15  8:13 ` Guinan Sun
  2020-10-15  8:38   ` Xing, Beilei
  2020-10-15  8:43 ` [dpdk-dev] [PATCH v4] " Guinan Sun
  3 siblings, 1 reply; 11+ messages in thread
From: Guinan Sun @ 2020-10-15  8:13 UTC (permalink / raw)
  To: dev; +Cc: Beilei Xing, Qi Zhang, Jingjing Wu, Guinan Sun

When the multicast address list is added, it will flush
previous addresses first, and then add new ones.
So when the number of multicast addresses added to the list
exceeds the upper limit causes a failure, should add the
previous addresses back. This patch fixes the issue.

Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")

Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
Tested-by: Peng Yuan <yuan.peng@intel.com>
---
v3:
* modify commit message
v2:
* modify the variable name
---
 drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
 drivers/net/iavf/iavf_vchnl.c  |  3 ---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index e68e3bc71..8a7577230 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	int err;
+	int err, ret;
+
+	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
+		PMD_DRV_LOG(ERR,
+			    "can't add more than a limited number (%u) of addresses.",
+			    (uint32_t)IAVF_NUM_MACADDR_MAX);
+		return -EINVAL;
+	}
 
 	/* flush previous addresses */
 	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf->mc_addrs_num,
@@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	if (err)
 		return err;
 
-	vf->mc_addrs_num = 0;
-
 	/* add new ones */
 	err = iavf_add_del_mc_addr_list(adapter, 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));
+	if (err) {
+		/* if adding mac address list fails, should add the previous
+		 * addresses back.
+		 */
+		ret = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
+						vf->mc_addrs_num, true);
+		if (ret)
+			return ret;
+	} else {
+		vf->mc_addrs_num = mc_addrs_num;
+		memcpy(vf->mc_addrs,
+		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+	}
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index db0b76876..a2295f879 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 	if (mc_addrs == NULL || mc_addrs_num == 0)
 		return 0;
 
-	if (mc_addrs_num > IAVF_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;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/iavf: fix adding multicast MAC address
  2020-10-15  8:13 ` [dpdk-dev] [PATCH v3] " Guinan Sun
@ 2020-10-15  8:38   ` Xing, Beilei
  0 siblings, 0 replies; 11+ messages in thread
From: Xing, Beilei @ 2020-10-15  8:38 UTC (permalink / raw)
  To: Sun, GuinanX, dev; +Cc: Zhang, Qi Z, Wu, Jingjing, Sun, GuinanX



> -----Original Message-----
> From: Guinan Sun <guinanx.sun@intel.com>
> Sent: Thursday, October 15, 2020 4:14 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> Subject: [PATCH v3] net/iavf: fix adding multicast MAC address
> 
> When the multicast address list is added, it will flush previous addresses first,
> and then add new ones.
> So when the number of multicast addresses added to the list exceeds the
> upper limit causes a failure, should add the previous addresses back. This
> patch fixes the issue.

If the number of multicast address list exceeds the upper limit, it will
cause failure, then need to roll back previous addresses.

Except this,
Acked-by: Beilei Xing <beilei.xing@intel.com>

> 
> Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> 
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> Tested-by: Peng Yuan <yuan.peng@intel.com>
> ---
> v3:
> * modify commit message
> v2:
> * modify the variable name
> ---
>  drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
> drivers/net/iavf/iavf_vchnl.c  |  3 ---
>  2 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index e68e3bc71..8a7577230 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
>  	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data-
> >dev_private);
>  	struct iavf_adapter *adapter =
>  		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> -	int err;
> +	int err, ret;
> +
> +	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
> +		PMD_DRV_LOG(ERR,
> +			    "can't add more than a limited number (%u) of
> addresses.",
> +			    (uint32_t)IAVF_NUM_MACADDR_MAX);
> +		return -EINVAL;
> +	}
> 
>  	/* flush previous addresses */
>  	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf-
> >mc_addrs_num, @@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct
> rte_eth_dev *dev,
>  	if (err)
>  		return err;
> 
> -	vf->mc_addrs_num = 0;
> -
>  	/* add new ones */
>  	err = iavf_add_del_mc_addr_list(adapter, 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));
> +	if (err) {
> +		/* if adding mac address list fails, should add the previous
> +		 * addresses back.
> +		 */
> +		ret = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
> +						vf->mc_addrs_num, true);
> +		if (ret)
> +			return ret;
> +	} else {
> +		vf->mc_addrs_num = mc_addrs_num;
> +		memcpy(vf->mc_addrs,
> +		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
> +	}
> 
> -	return 0;
> +	return err;
>  }
> 
>  static int
> diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c index
> db0b76876..a2295f879 100644
> --- a/drivers/net/iavf/iavf_vchnl.c
> +++ b/drivers/net/iavf/iavf_vchnl.c
> @@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter
> *adapter,
>  	if (mc_addrs == NULL || mc_addrs_num == 0)
>  		return 0;
> 
> -	if (mc_addrs_num > IAVF_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;
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [dpdk-dev] [PATCH v4] net/iavf: fix adding multicast MAC address
  2020-10-15  2:02 [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address Guinan Sun
                   ` (2 preceding siblings ...)
  2020-10-15  8:13 ` [dpdk-dev] [PATCH v3] " Guinan Sun
@ 2020-10-15  8:43 ` Guinan Sun
  2020-10-20  9:06   ` Zhang, Qi Z
  3 siblings, 1 reply; 11+ messages in thread
From: Guinan Sun @ 2020-10-15  8:43 UTC (permalink / raw)
  To: dev; +Cc: Beilei Xing, Qi Zhang, Jingjing Wu, Guinan Sun

When the multicast address list is added, it will flush
previous addresses first, and then add new ones.
If the number of multicast address in the list exceeds
the upper limit, it will cause failure, then need to
roll back previous addresses. This patch fixes the issue.

Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")

Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
Tested-by: Peng Yuan <yuan.peng@intel.com>
Acked-by: Beilei Xing <beilei.xing@intel.com>
---
v4:
* modify commit message
* add Acked-by
v3:
* modify commit message
v2:
* modify the variable name
---
 drivers/net/iavf/iavf_ethdev.c | 30 ++++++++++++++++++++++--------
 drivers/net/iavf/iavf_vchnl.c  |  3 ---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index e68e3bc71..8a7577230 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -164,7 +164,14 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
 	struct iavf_adapter *adapter =
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
-	int err;
+	int err, ret;
+
+	if (mc_addrs_num > IAVF_NUM_MACADDR_MAX) {
+		PMD_DRV_LOG(ERR,
+			    "can't add more than a limited number (%u) of addresses.",
+			    (uint32_t)IAVF_NUM_MACADDR_MAX);
+		return -EINVAL;
+	}
 
 	/* flush previous addresses */
 	err = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, vf->mc_addrs_num,
@@ -172,17 +179,24 @@ iavf_set_mc_addr_list(struct rte_eth_dev *dev,
 	if (err)
 		return err;
 
-	vf->mc_addrs_num = 0;
-
 	/* add new ones */
 	err = iavf_add_del_mc_addr_list(adapter, 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));
+	if (err) {
+		/* if adding mac address list fails, should add the previous
+		 * addresses back.
+		 */
+		ret = iavf_add_del_mc_addr_list(adapter, vf->mc_addrs,
+						vf->mc_addrs_num, true);
+		if (ret)
+			return ret;
+	} else {
+		vf->mc_addrs_num = mc_addrs_num;
+		memcpy(vf->mc_addrs,
+		       mc_addrs, mc_addrs_num * sizeof(*mc_addrs));
+	}
 
-	return 0;
+	return err;
 }
 
 static int
diff --git a/drivers/net/iavf/iavf_vchnl.c b/drivers/net/iavf/iavf_vchnl.c
index db0b76876..a2295f879 100644
--- a/drivers/net/iavf/iavf_vchnl.c
+++ b/drivers/net/iavf/iavf_vchnl.c
@@ -1107,9 +1107,6 @@ iavf_add_del_mc_addr_list(struct iavf_adapter *adapter,
 	if (mc_addrs == NULL || mc_addrs_num == 0)
 		return 0;
 
-	if (mc_addrs_num > IAVF_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;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH v4] net/iavf: fix adding multicast MAC address
  2020-10-15  8:43 ` [dpdk-dev] [PATCH v4] " Guinan Sun
@ 2020-10-20  9:06   ` Zhang, Qi Z
  0 siblings, 0 replies; 11+ messages in thread
From: Zhang, Qi Z @ 2020-10-20  9:06 UTC (permalink / raw)
  To: Sun, GuinanX, dev; +Cc: Xing, Beilei, Wu, Jingjing, Sun, GuinanX



> -----Original Message-----
> From: Guinan Sun <guinanx.sun@intel.com>
> Sent: Thursday, October 15, 2020 4:43 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Sun, GuinanX <guinanx.sun@intel.com>
> Subject: [PATCH v4] net/iavf: fix adding multicast MAC address
> 
> When the multicast address list is added, it will flush previous addresses first,
> and then add new ones.
> If the number of multicast address in the list exceeds the upper limit, it will
> cause failure, then need to roll back previous addresses. This patch fixes the
> issue.
> 
> Fixes: 05e4c3aff35f ("net/iavf: support multicast configuration")
> 
> Signed-off-by: Guinan Sun <guinanx.sun@intel.com>
> Tested-by: Peng Yuan <yuan.peng@intel.com>
> Acked-by: Beilei Xing <beilei.xing@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-10-20  9:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15  2:02 [dpdk-dev] [PATCH] net/iavf: fix adding multicast MAC address Guinan Sun
2020-10-15  5:13 ` Peng, Yuan
2020-10-15  5:37   ` Xing, Beilei
2020-10-15  5:57     ` Sun, GuinanX
2020-10-15  6:43       ` Xing, Beilei
2020-10-15  6:46         ` Sun, GuinanX
2020-10-15  6:55 ` [dpdk-dev] [PATCH v2] " Guinan Sun
2020-10-15  8:13 ` [dpdk-dev] [PATCH v3] " Guinan Sun
2020-10-15  8:38   ` Xing, Beilei
2020-10-15  8:43 ` [dpdk-dev] [PATCH v4] " Guinan Sun
2020-10-20  9:06   ` Zhang, Qi Z

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git