[dpdk-dev,v2] net/i40e: fix mirror rule reset when port is stopped
Checks
Commit Message
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(-)
Comments
> -----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
> -----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
> -----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
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
@@ -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 */