> -----Original Message----- > From: Wu, Jingjing > Sent: Thursday, September 7, 2017 3:51 PM > To: Dai, Wei <wei....@intel.com>; Xing, Beilei <beilei.x...@intel.com> > Cc: dev@dpdk.org; sta...@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...@intel.com>; Xing, Beilei > > <beilei.x...@intel.com> > > Cc: dev@dpdk.org; Dai, Wei <wei....@intel.com>; sta...@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: sta...@dpdk.org > > > > Signed-off-by: Wei Dai <wei....@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