[dpdk-dev] [PATCH v2] net/ixgbe: fix parsing fdir nvgre issue

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, December 22, 2017 2:32 PM
> To: Zhao1, Wei <wei.zhao1 at intel.com>; dev at dpdk.org
> Subject: RE: [PATCH v2] net/ixgbe: fix parsing fdir nvgre issue
> 
> 
> 
> > -----Original Message-----
> > From: Zhao1, Wei
> > Sent: Friday, December 22, 2017 1:46 PM
> > To: dev at dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang at intel.com>; Zhao1, Wei
> > <wei.zhao1 at intel.com>
> > Subject: [PATCH v2] net/ixgbe: fix parsing fdir nvgre issue
> >
> > There is some wrong of mask check in nvgre parser for flow API.
> >
> > Fixes: 11777435c727 ("net/ixgbe: parse flow director filter")
> >
> > Signed-off-by: Wei Zhao <wei.zhao1 at intel.com>
> >
> > ---
> >
> > V2:
> > -change c_k_s_rsvd0_ver  mask check to 0xFFFF.
> > ---
> >  drivers/net/ixgbe/ixgbe_flow.c | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_flow.c
> > b/drivers/net/ixgbe/ixgbe_flow.c index 19c2d47..887d933 100644
> > --- a/drivers/net/ixgbe/ixgbe_flow.c
> > +++ b/drivers/net/ixgbe/ixgbe_flow.c
> > @@ -2466,8 +2466,7 @@ ixgbe_parse_fdir_filter_tunnel(const struct
> > rte_flow_attr *attr,
> >  				item, "Not supported by fdir filter");
> >  			return -rte_errno;
> >  		}
> > -		if (nvgre_mask->c_k_s_rsvd0_ver !=
> > -			rte_cpu_to_be_16(0x3000) ||
> > +		if (nvgre_mask->protocol &&
> >  		    nvgre_mask->protocol != 0xFFFF) {
> >  			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
> >  			rte_flow_error_set(error, EINVAL,
> > @@ -2475,6 +2474,15 @@ ixgbe_parse_fdir_filter_tunnel(const struct
> > rte_flow_attr *attr,
> >  				item, "Not supported by fdir filter");
> >  			return -rte_errno;
> >  		}
> > +		if (nvgre_mask->c_k_s_rsvd0_ver &&
> > +		    nvgre_mask->c_k_s_rsvd0_ver !=
> > +			rte_cpu_to_be_16(0xFFFF)) {
> > +			memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
> > +			rte_flow_error_set(error, EINVAL,
> > +				RTE_FLOW_ERROR_TYPE_ITEM,
> > +				item, "Not supported by fdir filter");
> > +			return -rte_errno;
> > +		}
> >  		/* TNI must be totally masked or not. */
> >  		if (nvgre_mask->tni[0] &&
> >  		    ((nvgre_mask->tni[0] != 0xFF) || @@ -2496,7 +2504,15 @@
> > ixgbe_parse_fdir_filter_tunnel(const struct rte_flow_attr *attr,
> >  			nvgre_spec =
> >  				(const struct rte_flow_item_nvgre *)item->spec;
> >  			if (nvgre_spec->c_k_s_rsvd0_ver !=
> > -			    rte_cpu_to_be_16(0x2000) ||
> > +			    rte_cpu_to_be_16(0x2000) &&
> > +				nvgre_mask->c_k_s_rsvd0_ver) {
> 
> It's not necessary to && with a 0xffff, why not just keep consistent with how
> you handle the protocol field.
> BTW, I missed the issue on the i40e patch, its better if you can also capture it.

OK, it's necessary to consider a 0 mask, so please ignore above comment.
> 
> 
> > +				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
> > +				rte_flow_error_set(error, EINVAL,
> > +					RTE_FLOW_ERROR_TYPE_ITEM,
> > +					item, "Not supported by fdir filter");
> > +				return -rte_errno;
> > +			}
> > +			if (nvgre_mask->protocol &&
> >  			    nvgre_spec->protocol !=
> >  			    rte_cpu_to_be_16(NVGRE_PROTOCOL)) {
> >  				memset(rule, 0, sizeof(struct ixgbe_fdir_rule));
> > --
> > 2.9.3