diff mbox

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

Message ID 20171222054559.54207-1-wei.zhao1@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Helin Zhang
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Zhao1, Wei Dec. 22, 2017, 5:45 a.m. UTC 
  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@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(-)

Comments

Qi Zhang Dec. 22, 2017, 6:31 a.m. UTC | #1 
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, December 22, 2017 1:46 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@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@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.

Regards
Qi

> +				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
Qi Zhang Dec. 22, 2017, 7:02 a.m. UTC | #2 
> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Friday, December 22, 2017 2:32 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@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@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei
> > <wei.zhao1@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@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
Qi Zhang Dec. 22, 2017, 7:05 a.m. UTC | #3 
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Friday, December 22, 2017 1:46 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@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@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) {
> +				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

Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Wenzhuo Lu Jan. 4, 2018, 8:40 a.m. UTC | #4 
Hi,


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 22, 2017 1:46 PM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [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@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Zhang, Helin Jan. 9, 2018, 6:54 a.m. UTC | #5 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Lu, Wenzhuo
> Sent: Thursday, January 4, 2018 4:41 PM
> To: Zhao1, Wei; dev@dpdk.org
> Cc: Zhang, Qi Z; Zhao1, Wei
> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix parsing fdir nvgre issue
> 
> Hi,
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> > Sent: Friday, December 22, 2017 1:46 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> > Subject: [dpdk-dev] [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@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Applied to dpdk-next-net-intel, with minior typo corrections. thanks!

/Helin
diff mbox

Patch

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) {
+				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));