[dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front to jump over ntuple filter case

Zhang, Qi Z qi.z.zhang at intel.com
Fri Oct 19 19:10:19 CEST 2018



> -----Original Message-----
> From: Zhao1, Wei
> Sent: Tuesday, October 16, 2018 10:57 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; mocan <faicker.mo at ucloud.cn>
> Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in front
> to jump over ntuple filter case
> 
> 
> 
> > -----Original Message-----
> > From: Zhang, Qi Z
> > Sent: Monday, October 15, 2018 11:31 AM
> > To: Zhao1, Wei <wei.zhao1 at intel.com>; mocan <faicker.mo at ucloud.cn>
> > Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check in
> > front to jump over ntuple filter case
> >
> > Hi Wei:
> >
> > > -----Original Message-----
> > > From: Zhao1, Wei
> > > Sent: Thursday, October 11, 2018 1:10 AM
> > > To: Zhang, Qi Z <qi.z.zhang at intel.com>; mocan <faicker.mo at ucloud.cn>
> > > Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check
> > > in front to jump over ntuple filter case
> > >
> > > Hi, qi
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Qi Z
> > > > Sent: Thursday, October 11, 2018 2:36 AM
> > > > To: Zhao1, Wei <wei.zhao1 at intel.com>; mocan
> <faicker.mo at ucloud.cn>
> > > > Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> > > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple check
> > > > in front to jump over ntuple filter case
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Zhao1, Wei
> > > > > Sent: Monday, October 8, 2018 2:46 AM
> > > > > To: Zhang, Qi Z <qi.z.zhang at intel.com>; mocan
> > > > > <faicker.mo at ucloud.cn>
> > > > > Cc: dev at dpdk.org; Lu, Wenzhuo <wenzhuo.lu at intel.com>
> > > > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple
> > > > > check in front to jump over ntuple filter case
> > > > >
> > > > > Hi,
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Zhang, Qi Z
> > > > > > Sent: Wednesday, September 26, 2018 7:14 PM
> > > > > > To: mocan <faicker.mo at ucloud.cn>; Zhao1, Wei
> > > <wei.zhao1 at intel.com>
> > > > > > Cc: dev at dpdk.org
> > > > > > Subject: RE: Re:RE: [dpdk-dev] [PATCH] net/ixgbe: put 5tuple
> > > > > > check in front to jump over ntuple filter case
> > > > > >
> > > > > > OK, got your point. We should not reject a possible valid fdir
> > > > > > flow at n-tuple flow check stage.
> > > > > >
> > > > > > Review-by: Qi Zhang <qi.z.zhang at intel.com>
> > > > >
> > > > >
> > > > > I agree with the point of " We should not reject a possible
> > > > > valid fdir flow at n-tuple flow check stage".
> > > > > But, I think the fix patch should be more generic for all types
> > > > > filter of this problem.
> > > > > Maybe, we should delete all " goto out"  in function
> > > ixgbe_flow_create().
> > > > > Then, it will go to ntuple filter and  ethertype filter, syn
> > > > > filter and fdir filter ,l2_tn_filter one by one.
> > > > > And aslo, we should code as
> > > > >
> > > > > {
> > > > >
> > > > > Ntuple:
> > > > > ..........
> > > > > if(ret)
> > > > >     Goto ethertype
> > > > > ..........
> > > > >
> > > > > Ethertype:
> > > > >
> > > > > ..........
> > > > > if(ret)
> > > > >     Goto fdir filter
> > > > > .........
> > > > >
> > > > > fdir filter:
> > > > >
> > > > > if(ret)
> > > > >   Goto l2_tn_filter
> > > > >
> > > > > l2_tn_filter:
> > > > >
> > > > > .............
> > > > >
> > > > > }
> > > > >
> > > > > This fix patch only solve the problem of  ntuple and fdir.
> > > > > Qi, What do you think of this?
> > > >
> > > > I'm not the author of this part of code, so my understanding of
> > > > current implementation is:
> > > > It assume a flow will not be ambiguous which means if it match to
> > > > some filter parser (ixgbe_parse_xxx_filter), it is not necessary
> > > > to match on a different filter.
> > > > But I'm not sure if the assumption is correct or not, (this
> > > > depends on the knowledge of the device capability), So do you mean
> > > > the assumption is not correct? If you think a generic fix is
> > > > necessary, I have below comments
> > >
> > > Yes, the assumption is may cause bug, this patch is an evidence,
> > > maybe this user has encountered this problem.
> > >
> > > >
> > > > 1. it is better be done by Intel people with enough validation
> > >
> > > I agree with you, I will commit a generic fix patch later.
> > >
> > > >2. two options  for patch submit.
> > > > 	Submit a v2 with the generic fix, and it will be captured in this
> release.
> > > > 	If it is not urgent, we can just accept current one first, then
> > > >have a  separate patch in next release.
> > >
> > > Ok, If someone supply a v2 with the generic fix, I will ack.
> > >
> >
> > Just want to confirm with you , are you agree to merge this patch?
> > Or you think v2 with generic fix is necessary?
> > From my view, the patch can be accepted, since it just add more strict
> > check in cons_parse_ntuple_filter, it does not break anything, and it
> > fix the specific issue.
> >
> > Thanks
> > Qi
> >
> >
> 
> Of course, it can be merge, but a more generic still need.
> Acked-by:  Wei Zhao <wei.zhao1 at intel.com>

Applied to dpdk-next-net-intel with below changes

1) change the title to "fix to reject a valid flow during ntuple check"
2) more detail commit log base on auther's comment in mail list
3) add fix line and Cc stable at dpdk.org

Thanks
Qi



More information about the dev mailing list