[PATCH] net/i40e: fix mirror rule reset when port is stopped

[PATCH] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related SW and HW resources should also be removed. All of
them are should be removed by calling i40e_mirror_rule_reset( ).

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5f26e24..93fb6cd 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
+		i40e_mirror_rule_reset(dev, p_mirror->index);
 	}
 	pf->nb_mirror_rule = 0;
 
-- 
2.7.5

Re: [PATCH] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, September 6, 2017 10:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related SW and HW resources should also be removed. All of them
> are should be removed by calling i40e_mirror_rule_reset( ).
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..93fb6cd 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> -		rte_free(p_mirror);
> +		i40e_mirror_rule_reset(dev, p_mirror->index);
>  	}
>  	pf->nb_mirror_rule = 0;
> 
> --
> 2.7.5

Re: [PATCH] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

Hi, Lijuan
How about your test result ?

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, September 6, 2017 10:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related SW and HW resources should also be removed. All of them
> are should be removed by calling i40e_mirror_rule_reset( ).
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..93fb6cd 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> -		rte_free(p_mirror);
> +		i40e_mirror_rule_reset(dev, p_mirror->index);
>  	}
>  	pf->nb_mirror_rule = 0;
> 
> --
> 2.7.5

Re: [PATCH] net/i40e: fix mirror rule reset when port is stopped

[Tu, LijuanX A]

I have tested the patch ,it passed.

-----Original Message-----
From: Dai, Wei 
Sent: Thursday, September 7, 2017 3:36 PM
To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Tu, LijuanX A <lijuanx.a.tu@intel.com>
Cc: dev@dpdk.org; stable@dpdk.org
Subject: RE: [PATCH] net/i40e: fix mirror rule reset when port is stopped

Hi, Lijuan
How about your test result ?

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, September 6, 2017 10:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei 
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related SW and HW resources should also be removed. All of 
> them are should be removed by calling i40e_mirror_rule_reset( ).
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu<lijuanx.a.tu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c 
> b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..93fb6cd 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> -		rte_free(p_mirror);
> +		i40e_mirror_rule_reset(dev, p_mirror->index);
>  	}
>  	pf->nb_mirror_rule = 0;
> 
> --
> 2.7.5

Re: [PATCH] net/i40e: fix mirror rule reset when port is stopped

[Wu, Jingjing]

> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, September 6, 2017 10:52 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related SW and HW resources should also be removed. All of
> them are should be removed by calling i40e_mirror_rule_reset( ).
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..93fb6cd 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> -		rte_free(p_mirror);
> +		i40e_mirror_rule_reset(dev, p_mirror->index);
>  	}
>  	pf->nb_mirror_rule = 0;
> 
It is correct to remove mirror rule in HW. But looking into the function i40e_mirror_rul_reset, it's waste to call the function here.
It is much economic to do like
  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
		i40e_aq_del_mirror_rule(hw, seid,
				p_mirror->rule_type,
				p_mirror->entries,
				p_mirror->num_entries, p_mirror->id);
		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
		rte_free(p_mirror);
		i40e_mirror_rule_reset(dev, p_mirror->index);
  	}

Thanks
Jingjing

Re: [PATCH] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, September 7, 2017 3:51 PM
> To: Dai, Wei <wei.dai@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> 
> 
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Wednesday, September 6, 2017 10:52 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: [PATCH] net/i40e: fix mirror rule reset when port is stopped
> >
> > When an i40e PF port is stopped, all mirror rules should be removed.
> > All rule related SW and HW resources should also be removed. All of
> > them are should be removed by calling i40e_mirror_rule_reset( ).
> >
> > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..93fb6cd 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2094,8 +2094,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> >
> >  	/* Remove all mirror rules */
> >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > -		rte_free(p_mirror);
> > +		i40e_mirror_rule_reset(dev, p_mirror->index);
> >  	}
> >  	pf->nb_mirror_rule = 0;
> >
> It is correct to remove mirror rule in HW. But looking into the function
> i40e_mirror_rul_reset, it's waste to call the function here.
> It is much economic to do like
>   	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> 		i40e_aq_del_mirror_rule(hw, seid,
> 				p_mirror->rule_type,
> 				p_mirror->entries,
> 				p_mirror->num_entries, p_mirror->id);
> 		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> 		rte_free(p_mirror);
> 		i40e_mirror_rule_reset(dev, p_mirror->index);

Indeed, the function i40e_mirror_rule_reset( ) includes above 3 code lines:
I40e_aq_del_mirror_rule( ),  TAILQ_REMOVE( ) and rte_free( ).
So did you suggest that use these 3 lines instead of calling i40e_mirror_rule_reset( ) ?
I mean i40e_mirror_rule_reset( ) is not necessary in your suggestion.

What's more, all these are in i40e_dev_stop( ), so it doesn't matter which method
to get same result :-)


>   	}
> 
> Thanks
> Jingjing

[PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related software and hardware resources should also be
removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5f26e24..a278748 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2094,10 +2094,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
+		int ret;
+		ret = i40e_aq_del_mirror_rule(hw, pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+		} else {
+			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+			rte_free(p_mirror);
+			pf->nb_mirror_rule--;
+		}
 	}
-	pf->nb_mirror_rule = 0;
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
-- 
2.7.5

[PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related software and hardware resources should also be
removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 5f26e24..5902618 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -360,6 +360,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
+static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+						     uint16_t seid,
+						     uint16_t rule_type,
+						     uint16_t *entries,
+						     uint16_t count,
+						     uint16_t rule_id);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t sw_id, uint8_t on);
@@ -2094,10 +2100,23 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
+		int ret;
+		ret = i40e_aq_del_mirror_rule(hw,
+					      pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+		} else {
+			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+			rte_free(p_mirror);
+			pf->nb_mirror_rule--;
+		}
 	}
-	pf->nb_mirror_rule = 0;
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
-- 
2.7.5

Re: [PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Xing, Beilei]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> Sent: Monday, September 11, 2017 10:12 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when port is
> stopped
> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related software and hardware resources should also be removed.
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 5f26e24..a278748 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -2094,10 +2094,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> -		rte_free(p_mirror);
> +		int ret;
> +		ret = i40e_aq_del_mirror_rule(hw, pf->main_vsi->veb->seid,
> +					      p_mirror->rule_type,
> +					      p_mirror->entries,
> +					      p_mirror->num_entries,
> +					      p_mirror->id);
> +		if (ret < 0) {
> +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> +				    "status = %d, aq_err = %d.", ret,
> +				    hw->aq.asq_last_status);
> +		} else {
> +			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> +			rte_free(p_mirror);
> +			pf->nb_mirror_rule--;
> +		}

little comment: else statement can be omitted here.

>  	}
> -	pf->nb_mirror_rule = 0;
> 
>  	if (!rte_intr_allow_others(intr_handle))
>  		/* resume to the default handler */
> --
> 2.7.5

Re: [PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Wu, Jingjing]

> -----Original Message-----
> From: Xing, Beilei
> Sent: Monday, September 11, 2017 10:46 AM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when port is
> stopped
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > Sent: Monday, September 11, 2017 10:12 AM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when
> > port is stopped
> >
> > When an i40e PF port is stopped, all mirror rules should be removed.
> > All rule related software and hardware resources should also be removed.
> >
> > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..a278748 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2094,10 +2094,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> >
> >  	/* Remove all mirror rules */
> >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > -		rte_free(p_mirror);
> > +		int ret;
> > +		ret = i40e_aq_del_mirror_rule(hw, pf->main_vsi->veb->seid,
> > +					      p_mirror->rule_type,
> > +					      p_mirror->entries,
> > +					      p_mirror->num_entries,
> > +					      p_mirror->id);
> > +		if (ret < 0) {
> > +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> > +				    "status = %d, aq_err = %d.", ret,
> > +				    hw->aq.asq_last_status);
> > +		} else {
> > +			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > +			rte_free(p_mirror);
> > +			pf->nb_mirror_rule--;
> > +		}
> 
> little comment: else statement can be omitted here.
> 
Another minor coding style comment:

Do not use braces (``{`` and ``}``) for control statements with zero or just a single statement, unless that statement is more than a single line in which case the braces are permitted.
> >  	}
> > -	pf->nb_mirror_rule = 0;
> >
> >  	if (!rte_intr_allow_others(intr_handle))
> >  		/* resume to the default handler */
> > --
> > 2.7.5

Re: [PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Wu, Jingjing]

> -----Original Message-----
> From: Xing, Beilei
> Sent: Monday, September 11, 2017 10:46 AM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when port is
> stopped
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > Sent: Monday, September 11, 2017 10:12 AM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when
> > port is stopped
> >
> > When an i40e PF port is stopped, all mirror rules should be removed.
> > All rule related software and hardware resources should also be removed.
> >
> > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..a278748 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -2094,10 +2094,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> >
> >  	/* Remove all mirror rules */
> >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > -		rte_free(p_mirror);
> > +		int ret;

You can move the declaration of ret to the beginning of the func.

> > +		ret = i40e_aq_del_mirror_rule(hw, pf->main_vsi->veb->seid,
> > +					      p_mirror->rule_type,
> > +					      p_mirror->entries,
> > +					      p_mirror->num_entries,
> > +					      p_mirror->id);
> > +		if (ret < 0) {
> > +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> > +				    "status = %d, aq_err = %d.", ret,
> > +				    hw->aq.asq_last_status);
> > +		} else {
> > +			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > +			rte_free(p_mirror);
> > +			pf->nb_mirror_rule--;
> > +		}
> 
> little comment: else statement can be omitted here.

Same comment, software entries need to be released.
 
Thanks
Jingjing

Re: [PATCH v2] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

Many thanks to Jingjing and Beilei.
Sorry, this patch is a wrong v2 due to a building error.
I have submitted another v2 .
Anyway, I will submit v3 patch per your feedback. 

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Tuesday, September 19, 2017 10:21 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei <wei.dai@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when port
> is stopped
> 
> 
> 
> > -----Original Message-----
> > From: Xing, Beilei
> > Sent: Monday, September 11, 2017 10:46 AM
> > To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset
> > when port is stopped
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > > Sent: Monday, September 11, 2017 10:12 AM
> > > To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > > <beilei.xing@intel.com>
> > > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix mirror rule reset when
> > > port is stopped
> > >
> > > When an i40e PF port is stopped, all mirror rules should be removed.
> > > All rule related software and hardware resources should also be
> removed.
> > >
> > > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > > Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c | 18 +++++++++++++++---
> > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index 5f26e24..a278748 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -2094,10 +2094,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> > >
> > >  	/* Remove all mirror rules */
> > >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > > -		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > > -		rte_free(p_mirror);
> > > +		int ret;
> 
> You can move the declaration of ret to the beginning of the func.
> 
> > > +		ret = i40e_aq_del_mirror_rule(hw, pf->main_vsi->veb->seid,
> > > +					      p_mirror->rule_type,
> > > +					      p_mirror->entries,
> > > +					      p_mirror->num_entries,
> > > +					      p_mirror->id);
> > > +		if (ret < 0) {
> > > +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> > > +				    "status = %d, aq_err = %d.", ret,
> > > +				    hw->aq.asq_last_status);
> > > +		} else {
> > > +			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > > +			rte_free(p_mirror);
> > > +			pf->nb_mirror_rule--;
> > > +		}
> >
> > little comment: else statement can be omitted here.
> 
> Same comment, software entries need to be released.
> 
> Thanks
> Jingjing

[PATCH v3] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related software and hardware resources should also be
removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f12aefa..23b9b38 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
+static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+						     uint16_t seid,
+						     uint16_t rule_type,
+						     uint16_t *entries,
+						     uint16_t count,
+						     uint16_t rule_id);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t sw_id, uint8_t on);
@@ -2096,10 +2102,23 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
+		int ret;
+		ret = i40e_aq_del_mirror_rule(hw,
+					      pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+		} else {
+			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+			rte_free(p_mirror);
+			pf->nb_mirror_rule--;
+		}
 	}
-	pf->nb_mirror_rule = 0;
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
-- 
2.7.5

[PATCH v4] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related software and hardware resources should also be
removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f12aefa..99f2486 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
+static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+						     uint16_t seid,
+						     uint16_t rule_type,
+						     uint16_t *entries,
+						     uint16_t count,
+						     uint16_t rule_id);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t sw_id, uint8_t on);
@@ -2069,6 +2075,7 @@ i40e_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;
 	int i;
+	int ret;
 
 	if (hw->adapter_stopped == 1)
 		return;
@@ -2096,10 +2103,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
+		ret = i40e_aq_del_mirror_rule(hw,
+					      pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0)
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+		else {
+			TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+			rte_free(p_mirror);
+			pf->nb_mirror_rule--;
+		}
 	}
-	pf->nb_mirror_rule = 0;
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
-- 
2.7.5

[PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be removed.
All rule related software and hardware resources should also be
removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f12aefa..14cf6c0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
+static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+						     uint16_t seid,
+						     uint16_t rule_type,
+						     uint16_t *entries,
+						     uint16_t count,
+						     uint16_t rule_id);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t sw_id, uint8_t on);
@@ -2069,6 +2075,7 @@ i40e_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;
 	int i;
+	int ret;
 
 	if (hw->adapter_stopped == 1)
 		return;
@@ -2096,10 +2103,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 
 	/* Remove all mirror rules */
 	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		ret = i40e_aq_del_mirror_rule(hw,
+					      pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0)
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+
+		/* remove mirror software resource any way */
 		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
 		rte_free(p_mirror);
+		pf->nb_mirror_rule--;
 	}
-	pf->nb_mirror_rule = 0;
 
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
-- 
2.7.5

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Ananyev, Konstantin]

Hi Wei,

> 
> When an i40e PF port is stopped, all mirror rules should be removed.
> All rule related software and hardware resources should also be
> removed.

Could you clarify why we have to remove all mirror rules when PF is stopped?
As I remember mirror rule can direct to VF, which still can be running, no?
Konstantin 

> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index f12aefa..14cf6c0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
>  static void i40e_configure_registers(struct i40e_hw *hw);
>  static void i40e_hw_init(struct rte_eth_dev *dev);
>  static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
> +static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
> +						     uint16_t seid,
> +						     uint16_t rule_type,
> +						     uint16_t *entries,
> +						     uint16_t count,
> +						     uint16_t rule_id);
>  static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
>  			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t sw_id, uint8_t on);
> @@ -2069,6 +2075,7 @@ i40e_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;
>  	int i;
> +	int ret;
> 
>  	if (hw->adapter_stopped == 1)
>  		return;
> @@ -2096,10 +2103,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> 
>  	/* Remove all mirror rules */
>  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> +		ret = i40e_aq_del_mirror_rule(hw,
> +					      pf->main_vsi->veb->seid,
> +					      p_mirror->rule_type,
> +					      p_mirror->entries,
> +					      p_mirror->num_entries,
> +					      p_mirror->id);
> +		if (ret < 0)
> +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> +				    "status = %d, aq_err = %d.", ret,
> +				    hw->aq.asq_last_status);
> +
> +		/* remove mirror software resource any way */
>  		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
>  		rte_free(p_mirror);
> +		pf->nb_mirror_rule--;
>  	}
> -	pf->nb_mirror_rule = 0;
> 
>  	if (!rte_intr_allow_others(intr_handle))
>  		/* resume to the default handler */
> --
> 2.7.5

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

Hi, Konstantin

Without this patch, when a port is stopped, all mirror SW resource are cleared but HW settings are still there.
And once the port is started again, creating mirror rule may fail due to remain HW settings.

There is a testpmd use case which can show why this patch is needed.
1. bind PF to igb_uio
#/root/dpdk-devbind.py -b igb_uio 0000:08:00.0
2. create 2 VFs
#echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/max_vfs
3. launch testpmd with PF, and set port mirror configuration
#./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
 Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
 Testpmd > quit
4. relaunch testpmd with PF, and set port mirror as the same configuration
#./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
 Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
 I40e_mirror_rule_set( ): failed to add mirror rule: ret = -53, aq_err = 13.
 Mirror rule add error: (Function not implemented)

When testpmd is quitted, rte_eth_dev_stop( ) and then rte_eth_dev_close( ) are called.
In these function, i40e_dev_stop( ) and i40e_dev_close( ) is called.
And i40e_dev_stop is also called by i40e_dev_close( ).

Without this patch, the error in second running of testpmd always happen.
This patch can address this error.

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, September 20, 2017 5:23 PM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port
> is stopped
> 
> Hi Wei,
> 
> >
> > When an i40e PF port is stopped, all mirror rules should be removed.
> > All rule related software and hardware resources should also be
> > removed.
> 
> Could you clarify why we have to remove all mirror rules when PF is
> stopped?
> As I remember mirror rule can direct to VF, which still can be running, no?
> Konstantin
> 
> >
> > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 21 ++++++++++++++++++++-
> >  1 file changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index f12aefa..14cf6c0 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct
> i40e_hw
> > *hw);  static void i40e_configure_registers(struct i40e_hw *hw);
> > static void i40e_hw_init(struct rte_eth_dev *dev);  static int
> > i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
> > +static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw
> *hw,
> > +						     uint16_t seid,
> > +						     uint16_t rule_type,
> > +						     uint16_t *entries,
> > +						     uint16_t count,
> > +						     uint16_t rule_id);
> >  static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
> >  			struct rte_eth_mirror_conf *mirror_conf,
> >  			uint8_t sw_id, uint8_t on);
> > @@ -2069,6 +2075,7 @@ i40e_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;
> >  	int i;
> > +	int ret;
> >
> >  	if (hw->adapter_stopped == 1)
> >  		return;
> > @@ -2096,10 +2103,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> >
> >  	/* Remove all mirror rules */
> >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > +		ret = i40e_aq_del_mirror_rule(hw,
> > +					      pf->main_vsi->veb->seid,
> > +					      p_mirror->rule_type,
> > +					      p_mirror->entries,
> > +					      p_mirror->num_entries,
> > +					      p_mirror->id);
> > +		if (ret < 0)
> > +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> > +				    "status = %d, aq_err = %d.", ret,
> > +				    hw->aq.asq_last_status);
> > +
> > +		/* remove mirror software resource any way */
> >  		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> >  		rte_free(p_mirror);
> > +		pf->nb_mirror_rule--;
> >  	}
> > -	pf->nb_mirror_rule = 0;
> >
> >  	if (!rte_intr_allow_others(intr_handle))
> >  		/* resume to the default handler */
> > --
> > 2.7.5

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Ananyev, Konstantin]

Hi Wei,

> 
> Hi, Konstantin
> 
> Without this patch, when a port is stopped, all mirror SW resource are cleared but HW settings are still there.
> And once the port is started again, creating mirror rule may fail due to remain HW settings.
> 
> There is a testpmd use case which can show why this patch is needed.
> 1. bind PF to igb_uio
> #/root/dpdk-devbind.py -b igb_uio 0000:08:00.0
> 2. create 2 VFs
> #echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/max_vfs
> 3. launch testpmd with PF, and set port mirror configuration
> #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
>  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
>  Testpmd > quit
> 4. relaunch testpmd with PF, and set port mirror as the same configuration
> #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
>  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
>  I40e_mirror_rule_set( ): failed to add mirror rule: ret = -53, aq_err = 13.
>  Mirror rule add error: (Function not implemented)
> 
> When testpmd is quitted, rte_eth_dev_stop( ) and then rte_eth_dev_close( ) are called.
> In these function, i40e_dev_stop( ) and i40e_dev_close( ) is called.
> And i40e_dev_stop is also called by i40e_dev_close( ).
> 
> Without this patch, the error in second running of testpmd always happen.
> This patch can address this error.

Thanks for explanation.
Though it seems that the patch while fixing the issue might introduce some inconsistences:
1. right now for i40e: dev_stop(port); dev_start(port) would keep existing HW mirror rule working.
   With your patch is not any more.
2. What about ixgbe? Do we also need to change its behavior?
    As I can see right now ixgbe doesn't reset any mirror rules at dev_stop().
   Would it be reset automatically, or do we need to update ixgbe PMD too?

About #1 - if we'll decide that this is a desired behavior that dev_stop() voids 
all mirror rules, then at least we probably need to update the documentation.
As alternative - at dev_stop() we can reset only actual HW rule, but keep SW configuration intact,
and restore them at dev_start().
I personally think second option is a bit better - as it preserves existing behavior,
and probably more convenient for the user.


About the patch itself, why just not:
while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) 
    i40e_mirror_rule_reset(dev, p_mirror->index);

?

Konstantin    

  

> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Wednesday, September 20, 2017 5:23 PM
> > To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port
> > is stopped
> >
> > Hi Wei,
> >
> > >
> > > When an i40e PF port is stopped, all mirror rules should be removed.
> > > All rule related software and hardware resources should also be
> > > removed.
> >
> > Could you clarify why we have to remove all mirror rules when PF is
> > stopped?
> > As I remember mirror rule can direct to VF, which still can be running, no?
> > Konstantin
> >
> > >
> > > Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > > Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> > > ---
> > >  drivers/net/i40e/i40e_ethdev.c | 21 ++++++++++++++++++++-
> > >  1 file changed, 20 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > > b/drivers/net/i40e/i40e_ethdev.c index f12aefa..14cf6c0 100644
> > > --- a/drivers/net/i40e/i40e_ethdev.c
> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> > > @@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct
> > i40e_hw
> > > *hw);  static void i40e_configure_registers(struct i40e_hw *hw);
> > > static void i40e_hw_init(struct rte_eth_dev *dev);  static int
> > > i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
> > > +static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw
> > *hw,
> > > +						     uint16_t seid,
> > > +						     uint16_t rule_type,
> > > +						     uint16_t *entries,
> > > +						     uint16_t count,
> > > +						     uint16_t rule_id);
> > >  static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
> > >  			struct rte_eth_mirror_conf *mirror_conf,
> > >  			uint8_t sw_id, uint8_t on);
> > > @@ -2069,6 +2075,7 @@ i40e_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;
> > >  	int i;
> > > +	int ret;
> > >
> > >  	if (hw->adapter_stopped == 1)
> > >  		return;
> > > @@ -2096,10 +2103,22 @@ i40e_dev_stop(struct rte_eth_dev *dev)
> > >
> > >  	/* Remove all mirror rules */
> > >  	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
> > > +		ret = i40e_aq_del_mirror_rule(hw,
> > > +					      pf->main_vsi->veb->seid,
> > > +					      p_mirror->rule_type,
> > > +					      p_mirror->entries,
> > > +					      p_mirror->num_entries,
> > > +					      p_mirror->id);
> > > +		if (ret < 0)
> > > +			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
> > > +				    "status = %d, aq_err = %d.", ret,
> > > +				    hw->aq.asq_last_status);
> > > +
> > > +		/* remove mirror software resource any way */
> > >  		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
> > >  		rte_free(p_mirror);
> > > +		pf->nb_mirror_rule--;
> > >  	}
> > > -	pf->nb_mirror_rule = 0;
> > >
> > >  	if (!rte_intr_allow_others(intr_handle))
> > >  		/* resume to the default handler */
> > > --
> > > 2.7.5

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Wu, Jingjing]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, September 21, 2017 6:46 AM
> To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port is stopped
> 
> Hi Wei,
> 
> >
> > Hi, Konstantin
> >
> > Without this patch, when a port is stopped, all mirror SW resource are cleared but HW
> settings are still there.
> > And once the port is started again, creating mirror rule may fail due to remain HW
> settings.
> >
> > There is a testpmd use case which can show why this patch is needed.
> > 1. bind PF to igb_uio
> > #/root/dpdk-devbind.py -b igb_uio 0000:08:00.0
> > 2. create 2 VFs
> > #echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/max_vfs
> > 3. launch testpmd with PF, and set port mirror configuration
> > #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
> >  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
> >  Testpmd > quit
> > 4. relaunch testpmd with PF, and set port mirror as the same configuration
> > #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
> >  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
> >  I40e_mirror_rule_set( ): failed to add mirror rule: ret = -53, aq_err = 13.
> >  Mirror rule add error: (Function not implemented)
> >
> > When testpmd is quitted, rte_eth_dev_stop( ) and then rte_eth_dev_close( ) are called.
> > In these function, i40e_dev_stop( ) and i40e_dev_close( ) is called.
> > And i40e_dev_stop is also called by i40e_dev_close( ).
> >
> > Without this patch, the error in second running of testpmd always happen.
> > This patch can address this error.
> 
> Thanks for explanation.
> Though it seems that the patch while fixing the issue might introduce some
> inconsistences:
> 1. right now for i40e: dev_stop(port); dev_start(port) would keep existing HW mirror rule
> working.
>    With your patch is not any more.
> 2. What about ixgbe? Do we also need to change its behavior?
>     As I can see right now ixgbe doesn't reset any mirror rules at dev_stop().
>    Would it be reset automatically, or do we need to update ixgbe PMD too?
> 
> About #1 - if we'll decide that this is a desired behavior that dev_stop() voids
> all mirror rules, then at least we probably need to update the documentation.
> As alternative - at dev_stop() we can reset only actual HW rule, but keep SW
> configuration intact,
> and restore them at dev_start().
> I personally think second option is a bit better - as it preserves existing behavior,
> and probably more convenient for the user.

Yes, you reminded me the mirror is to VF. The mirror rule should be kept or at least
be restored when device start again..
Because the dev_stop should only stop the pf interface, but not affect VF. It looks
like we don't delete the VEB we dev_stop.
The reset behavior may need to be done in dev_close, but not in dev_stop.

> 
> About the patch itself, why just not:
> while ((p_mirror = TAILQ_FIRST(&pf->mirror_list)))
>     i40e_mirror_rule_reset(dev, p_mirror->index);
> 
Wei's v1 patch is doing like that. But I commented it to change it. Because
i40e_mirror_rule_reset is doing a search in the list which is not necessary.

Thanks
Jingjing

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Ananyev, Konstantin]

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Saturday, September 23, 2017 3:27 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Dai, Wei <wei.dai@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port is stopped
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Thursday, September 21, 2017 6:46 AM
> > To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port is stopped
> >
> > Hi Wei,
> >
> > >
> > > Hi, Konstantin
> > >
> > > Without this patch, when a port is stopped, all mirror SW resource are cleared but HW
> > settings are still there.
> > > And once the port is started again, creating mirror rule may fail due to remain HW
> > settings.
> > >
> > > There is a testpmd use case which can show why this patch is needed.
> > > 1. bind PF to igb_uio
> > > #/root/dpdk-devbind.py -b igb_uio 0000:08:00.0
> > > 2. create 2 VFs
> > > #echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/max_vfs
> > > 3. launch testpmd with PF, and set port mirror configuration
> > > #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
> > >  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
> > >  Testpmd > quit
> > > 4. relaunch testpmd with PF, and set port mirror as the same configuration
> > > #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i
> > >  Testpmd > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
> > >  I40e_mirror_rule_set( ): failed to add mirror rule: ret = -53, aq_err = 13.
> > >  Mirror rule add error: (Function not implemented)
> > >
> > > When testpmd is quitted, rte_eth_dev_stop( ) and then rte_eth_dev_close( ) are called.
> > > In these function, i40e_dev_stop( ) and i40e_dev_close( ) is called.
> > > And i40e_dev_stop is also called by i40e_dev_close( ).
> > >
> > > Without this patch, the error in second running of testpmd always happen.
> > > This patch can address this error.
> >
> > Thanks for explanation.
> > Though it seems that the patch while fixing the issue might introduce some
> > inconsistences:
> > 1. right now for i40e: dev_stop(port); dev_start(port) would keep existing HW mirror rule
> > working.
> >    With your patch is not any more.
> > 2. What about ixgbe? Do we also need to change its behavior?
> >     As I can see right now ixgbe doesn't reset any mirror rules at dev_stop().
> >    Would it be reset automatically, or do we need to update ixgbe PMD too?
> >
> > About #1 - if we'll decide that this is a desired behavior that dev_stop() voids
> > all mirror rules, then at least we probably need to update the documentation.
> > As alternative - at dev_stop() we can reset only actual HW rule, but keep SW
> > configuration intact,
> > and restore them at dev_start().
> > I personally think second option is a bit better - as it preserves existing behavior,
> > and probably more convenient for the user.
> 
> Yes, you reminded me the mirror is to VF. The mirror rule should be kept or at least
> be restored when device start again..
> Because the dev_stop should only stop the pf interface, but not affect VF. It looks
> like we don't delete the VEB we dev_stop.
> The reset behavior may need to be done in dev_close, but not in dev_stop.

if we can move that code into dev_close() , then indeed might be a better approach.

> 
> >
> > About the patch itself, why just not:
> > while ((p_mirror = TAILQ_FIRST(&pf->mirror_list)))
> >     i40e_mirror_rule_reset(dev, p_mirror->index);
> >
> Wei's v1 patch is doing like that. But I commented it to change it. Because
> i40e_mirror_rule_reset is doing a search in the list which is not necessary.

Yes, but it is a control path, so need to be extra quick  here.
As a plus - avoid code duplication and easier to control/maintain it.
Konstantin

Re: [PATCH v5] net/i40e: fix mirror rule reset when port is stopped

[Dai, Wei]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Saturday, September 23, 2017 6:37 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Dai, Wei <wei.dai@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset when port
> is stopped
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Saturday, September 23, 2017 3:27 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Dai, Wei
> > <wei.dai@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset
> > when port is stopped
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Thursday, September 21, 2017 6:46 AM
> > > To: Dai, Wei <wei.dai@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v5] net/i40e: fix mirror rule reset
> > > when port is stopped
> > >
> > > Hi Wei,
> > >
> > > >
> > > > Hi, Konstantin
> > > >
> > > > Without this patch, when a port is stopped, all mirror SW resource
> > > > are cleared but HW
> > > settings are still there.
> > > > And once the port is started again, creating mirror rule may fail
> > > > due to remain HW
> > > settings.
> > > >
> > > > There is a testpmd use case which can show why this patch is needed.
> > > > 1. bind PF to igb_uio
> > > > #/root/dpdk-devbind.py -b igb_uio 0000:08:00.0 2. create 2 VFs
> > > > #echo 2 > /sys/bus/pci/devices/0000\:08\:00.0/max_vfs
> > > > 3. launch testpmd with PF, and set port mirror configuration
> > > > #./x86_64-native-linuxapp-gcc/app/testpmd -c 3 -n 4 -- -i  Testpmd
> > > > > set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
> > > > Testpmd > quit 4. relaunch testpmd with PF, and set port mirror as
> > > > the same configuration #./x86_64-native-linuxapp-gcc/app/testpmd
> > > > -c 3 -n 4 -- -i  Testpmd > set port 0 mirror-rule 0 pool-mirror-up
> > > > 0x1 dst-pool 1 on  I40e_mirror_rule_set( ): failed to add mirror
> > > > rule: ret = -53, aq_err = 13.
> > > >  Mirror rule add error: (Function not implemented)
> > > >
> > > > When testpmd is quitted, rte_eth_dev_stop( ) and then
> rte_eth_dev_close( ) are called.
> > > > In these function, i40e_dev_stop( ) and i40e_dev_close( ) is called.
> > > > And i40e_dev_stop is also called by i40e_dev_close( ).
> > > >
> > > > Without this patch, the error in second running of testpmd always
> happen.
> > > > This patch can address this error.
> > >
> > > Thanks for explanation.
> > > Though it seems that the patch while fixing the issue might
> > > introduce some
> > > inconsistences:
> > > 1. right now for i40e: dev_stop(port); dev_start(port) would keep
> > > existing HW mirror rule working.
> > >    With your patch is not any more.
> > > 2. What about ixgbe? Do we also need to change its behavior?
> > >     As I can see right now ixgbe doesn't reset any mirror rules at
> dev_stop().
> > >    Would it be reset automatically, or do we need to update ixgbe PMD
> too?
> > >
> > > About #1 - if we'll decide that this is a desired behavior that
> > > dev_stop() voids all mirror rules, then at least we probably need to
> update the documentation.
> > > As alternative - at dev_stop() we can reset only actual HW rule, but
> > > keep SW configuration intact, and restore them at dev_start().
> > > I personally think second option is a bit better - as it preserves
> > > existing behavior, and probably more convenient for the user.
> >
> > Yes, you reminded me the mirror is to VF. The mirror rule should be
> > kept or at least be restored when device start again..
> > Because the dev_stop should only stop the pf interface, but not affect
> > VF. It looks like we don't delete the VEB we dev_stop.
> > The reset behavior may need to be done in dev_close, but not in dev_stop.
> 
> if we can move that code into dev_close() , then indeed might be a better
> approach.
> 
> >
> > >
> > > About the patch itself, why just not:
> > > while ((p_mirror = TAILQ_FIRST(&pf->mirror_list)))
> > >     i40e_mirror_rule_reset(dev, p_mirror->index);
> > >
> > Wei's v1 patch is doing like that. But I commented it to change it.
> > Because i40e_mirror_rule_reset is doing a search in the list which is not
> necessary.
> 
> Yes, but it is a control path, so need to be extra quick  here.
> As a plus - avoid code duplication and easier to control/maintain it.
> Konstantin
> 
Thanks to Konstantin and Jingjing for all of your suggestion.
I will work out v6 patch to put all these mirror reset operations in dev_close( ).
As to ixgbe, same scheme will be tried and tested.

[PATCH v6] net/i40e: fix mirror rule reset when port is closed

[Wei Dai]

When an i40e PF port is stopped, all mirror rules should be reserved.
But when an i40e PF port is closed, all mirror rules should be removed.
When a mirror rule is removed, its associated hardware and software
resource should also be removed.

Fixes: a4def5edf0fc ("i40e: enable port mirroring")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index f12aefa..85c160a 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -361,6 +361,12 @@ static int i40e_dev_sync_phy_type(struct i40e_hw *hw);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct rte_eth_dev *dev);
 static int i40e_config_qinq(struct i40e_hw *hw, struct i40e_vsi *vsi);
+static enum i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+						     uint16_t seid,
+						     uint16_t rule_type,
+						     uint16_t *entries,
+						     uint16_t count,
+						     uint16_t rule_id);
 static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t sw_id, uint8_t on);
@@ -2065,7 +2071,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *main_vsi = pf->main_vsi;
-	struct i40e_mirror_rule *p_mirror;
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	int i;
@@ -2094,13 +2099,6 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Set link down */
 	i40e_dev_set_link_down(dev);
 
-	/* Remove all mirror rules */
-	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
-		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
-		rte_free(p_mirror);
-	}
-	pf->nb_mirror_rule = 0;
-
 	if (!rte_intr_allow_others(intr_handle))
 		/* resume to the default handler */
 		rte_intr_callback_register(intr_handle,
@@ -2127,12 +2125,34 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
+	struct i40e_mirror_rule *p_mirror;
 	uint32_t reg;
 	int i;
+	int ret;
 
 	PMD_INIT_FUNC_TRACE();
 
 	i40e_dev_stop(dev);
+
+	/* Remove all mirror rules */
+	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		ret = i40e_aq_del_mirror_rule(hw,
+					      pf->main_vsi->veb->seid,
+					      p_mirror->rule_type,
+					      p_mirror->entries,
+					      p_mirror->num_entries,
+					      p_mirror->id);
+		if (ret < 0)
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule: "
+				    "status = %d, aq_err = %d.", ret,
+				    hw->aq.asq_last_status);
+
+		/* remove mirror software resource anyway */
+		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+		rte_free(p_mirror);
+		pf->nb_mirror_rule--;
+	}
+
 	i40e_dev_free_queues(dev);
 
 	/* Disable interrupt */
-- 
2.9.4

Re: [PATCH v6] net/i40e: fix mirror rule reset when port is closed

[Wu, Jingjing]

> -----Original Message-----
> From: Dai, Wei
> Sent: Monday, September 25, 2017 2:37 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> Subject: [PATCH v6] net/i40e: fix mirror rule reset when port is closed
> 
> When an i40e PF port is stopped, all mirror rules should be reserved.
> But when an i40e PF port is closed, all mirror rules should be removed.
> When a mirror rule is removed, its associated hardware and software
> resource should also be removed.
> 
> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Re: [dpdk-stable] [PATCH v6] net/i40e: fix mirror rule reset when port is closed

[Ferruh Yigit]

On 9/26/2017 2:49 PM, Wu, Jingjing wrote:
> 
> 
>> -----Original Message-----
>> From: Dai, Wei
>> Sent: Monday, September 25, 2017 2:37 PM
>> To: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Ananyev,
>> Konstantin <konstantin.ananyev@intel.com>
>> Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
>> Subject: [PATCH v6] net/i40e: fix mirror rule reset when port is closed
>>
>> When an i40e PF port is stopped, all mirror rules should be reserved.
>> But when an i40e PF port is closed, all mirror rules should be removed.
>> When a mirror rule is removed, its associated hardware and software
>> resource should also be removed.
>>
>> Fixes: a4def5edf0fc ("i40e: enable port mirroring")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Wei Dai <wei.dai@intel.com>
>> Tested-by: Lijuan Tu <lijuanx.a.tu@intel.com>
> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Applied to dpdk-next-net/master, thanks.