[dpdk-dev] [PATCH v2 02/11] net/e1000: restore n-tuple filter

[Zhao1, Wei]

Hi, wenzhuo

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Friday, June 2, 2017 3:57 PM
> To: Zhao1, Wei <wei.zhao1 at intel.com>; dev at dpdk.org
> Subject: RE: [PATCH v2 02/11] net/e1000: restore n-tuple filter
> 
> Hi Wei,
> 
> > -----Original Message-----
> > From: Zhao1, Wei
> > Sent: Friday, June 2, 2017 2:36 PM
> > To: dev at dpdk.org
> > Cc: Lu, Wenzhuo; Zhao1, Wei
> > Subject: [PATCH v2 02/11] net/e1000: restore n-tuple filter
> >
> > Add support for restoring n-tuple
> > filter in SW.
> >
> > Signed-off-by: Wei Zhao <wei.zhao1 at intel.com>
> > ---
> >  drivers/net/e1000/igb_ethdev.c | 262
> > +++++++++++++++++++++++++------------
> > ----
> >  1 file changed, 159 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/net/e1000/igb_ethdev.c
> > b/drivers/net/e1000/igb_ethdev.c index 1077870..1e321d6 100644
> > --- a/drivers/net/e1000/igb_ethdev.c
> > +++ b/drivers/net/e1000/igb_ethdev.c
> > @@ -757,6 +757,35 @@ igb_reset_swfw_lock(struct e1000_hw *hw)
> >  	return E1000_SUCCESS;
> >  }
> >
> > +/* Remove all ntuple filters of the device */ static int
> > +igb_ntuple_filter_uninit(struct rte_eth_dev *eth_dev) {
> > +	struct e1000_filter_info *filter_info =
> > +		E1000_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data-
> > >dev_private);
> > +
> > +	struct e1000_5tuple_filter *p_5tuple, *p_5tuple_next;
> > +	struct e1000_2tuple_filter *p_2tuple, *p_2tuple_next;
> > +
> > +	for (p_5tuple = TAILQ_FIRST(&filter_info->fivetuple_list);
> > +	     p_5tuple != NULL; p_5tuple = p_5tuple_next) {
> > +		p_5tuple_next = TAILQ_NEXT(p_5tuple, entries);
> > +		TAILQ_REMOVE(&filter_info->fivetuple_list,
> > +			     p_5tuple, entries);
> > +		rte_free(p_5tuple);
> > +	}
> I know you don't change this code. It's moved here. But this implementation
> is complex and not friendly. Would you like to change it to,
> 	while ((p_5tuple = TAILQ_FIRST(&filter_info->fivetuple_list))) {
> 		TAILQ_REMOVE(&filter_info->fivetuple_list,
> 			     p_5tuple, entries);
> 		rte_free(p_5tuple);
> 	}
> The same below.

Ok, I will change as your suggestion in v3 later.

> 
> > +	filter_info->fivetuple_mask = 0;
> > +	for (p_2tuple = TAILQ_FIRST(&filter_info->twotuple_list);
> > +	     p_2tuple != NULL; p_2tuple = p_2tuple_next) {
> > +		p_2tuple_next = TAILQ_NEXT(p_2tuple, entries);
> > +		TAILQ_REMOVE(&filter_info->twotuple_list,
> > +			     p_2tuple, entries);
> > +		rte_free(p_2tuple);
> > +	}
> > +	filter_info->twotuple_mask = 0;
> > +
> > +	return 0;
> > +}