[dpdk-dev] net/i40e: add fdir nvgre parameters check

Message ID 20171201084706.7741-1-wei.zhao1@intel.com (mailing list archive)
State Superseded, 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. 1, 2017, 8:47 a.m. UTC
  Add mask parameters check in nvgre parser for flow API.

Fixes: 30965ca341278 ("net/i40e: add NVGRE flow parsing")

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/i40e/i40e_flow.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
  

Comments

Qi Zhang Dec. 22, 2017, 2:10 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao
> Sent: Friday, December 1, 2017 4:47 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check
> 
> Add mask parameters check in nvgre parser for flow API.
> 
> Fixes: 30965ca341278 ("net/i40e: add NVGRE flow parsing")
> 
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
>  drivers/net/i40e/i40e_flow.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c index
> 7e4936e..05e54f1 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -3610,6 +3610,41 @@ i40e_flow_parse_nvgre_pattern(__rte_unused
> struct rte_eth_dev *dev,
>  						       "Invalid TNI mask");
>  					return -rte_errno;
>  				}
> +				if (nvgre_mask->protocol &&
> +					nvgre_mask->protocol != 0xFFFF) {
> +					rte_flow_error_set(error, EINVAL,
> +						RTE_FLOW_ERROR_TYPE_ITEM,
> +						item,
> +						"Invalid NVGRE item");
> +					return -rte_errno;
> +				}
> +				if (nvgre_mask->c_k_s_rsvd0_ver &&
> +					nvgre_mask->c_k_s_rsvd0_ver !=
> +					rte_cpu_to_be_16(0x3000)) {

If " c_k_s_rsvd0_ver must have value 0x2000 according to RFC 7637" , I think here we
should have mask 0xffff to make sure each bit of c_k_s_rsvd0_ver be set correctly.

Regards
Qi 
> +					rte_flow_error_set(error, EINVAL,
> +						   RTE_FLOW_ERROR_TYPE_ITEM,
> +						   item,
> +						   "Invalid NVGRE item");
> +					return -rte_errno;
> +				}
> +				if (nvgre_spec->c_k_s_rsvd0_ver !=
> +					rte_cpu_to_be_16(0x2000) &&
> +					nvgre_mask->c_k_s_rsvd0_ver) {
> +					rte_flow_error_set(error, EINVAL,
> +						   RTE_FLOW_ERROR_TYPE_ITEM,
> +						   item,
> +						   "Invalid NVGRE item");
> +					return -rte_errno;
> +				}
> +				if (nvgre_mask->protocol &&
> +					nvgre_spec->protocol !=
> +					rte_cpu_to_be_16(0x6558)) {
> +					rte_flow_error_set(error, EINVAL,
> +						   RTE_FLOW_ERROR_TYPE_ITEM,
> +						   item,
> +						   "Invalid NVGRE item");
> +					return -rte_errno;
> +				}
>  				rte_memcpy(((uint8_t *)&tenant_id_be + 1),
>  					   nvgre_spec->tni, 3);
>  				filter->tenant_id =
> --
> 2.9.3
  
Zhao1, Wei Dec. 22, 2017, 2:26 a.m. UTC | #2
Hi,zhangqi

> -----Original Message-----

> From: Zhang, Qi Z

> Sent: Friday, December 22, 2017 10:11 AM

> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org

> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei

> <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>

> Subject: RE: [dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check

> 

> 

> 

> > -----Original Message-----

> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao

> > Sent: Friday, December 1, 2017 4:47 PM

> > To: dev@dpdk.org

> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei

> > <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>

> > Subject: [dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check

> >

> > Add mask parameters check in nvgre parser for flow API.

> >

> > Fixes: 30965ca341278 ("net/i40e: add NVGRE flow parsing")

> >

> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>

> > ---

> >  drivers/net/i40e/i40e_flow.c | 35

> +++++++++++++++++++++++++++++++++++

> >  1 file changed, 35 insertions(+)

> >

> > diff --git a/drivers/net/i40e/i40e_flow.c

> > b/drivers/net/i40e/i40e_flow.c index

> > 7e4936e..05e54f1 100644

> > --- a/drivers/net/i40e/i40e_flow.c

> > +++ b/drivers/net/i40e/i40e_flow.c

> > @@ -3610,6 +3610,41 @@ i40e_flow_parse_nvgre_pattern(__rte_unused

> > struct rte_eth_dev *dev,

> >  						       "Invalid TNI mask");

> >  					return -rte_errno;

> >  				}

> > +				if (nvgre_mask->protocol &&

> > +					nvgre_mask->protocol != 0xFFFF) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> 	RTE_FLOW_ERROR_TYPE_ITEM,

> > +						item,

> > +						"Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_mask->c_k_s_rsvd0_ver &&

> > +					nvgre_mask->c_k_s_rsvd0_ver !=

> > +					rte_cpu_to_be_16(0x3000)) {

> 

> If " c_k_s_rsvd0_ver must have value 0x2000 according to RFC 7637" , I think

> here we should have mask 0xffff to make sure each bit of c_k_s_rsvd0_ver

> be set correctly.

> 


Yes, our old code is as your suggestion, but testpmd can not support config that parameter
,if that stv can not test this feature.
so, we did a balance.

> Regards

> Qi

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_spec->c_k_s_rsvd0_ver !=

> > +					rte_cpu_to_be_16(0x2000) &&

> > +					nvgre_mask->c_k_s_rsvd0_ver) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_mask->protocol &&

> > +					nvgre_spec->protocol !=

> > +					rte_cpu_to_be_16(0x6558)) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> >  				rte_memcpy(((uint8_t *)&tenant_id_be + 1),

> >  					   nvgre_spec->tni, 3);

> >  				filter->tenant_id =

> > --

> > 2.9.3
  
Zhao1, Wei Dec. 22, 2017, 2:45 a.m. UTC | #3
Hi,zhangqi

> -----Original Message-----

> From: Zhang, Qi Z

> Sent: Friday, December 22, 2017 10:11 AM

> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org

> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei

> <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>

> Subject: RE: [dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check

> 

> 

> 

> > -----Original Message-----

> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao

> > Sent: Friday, December 1, 2017 4:47 PM

> > To: dev@dpdk.org

> > Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei

> > <beilei.xing@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>

> > Subject: [dpdk-dev] [PATCH] net/i40e: add fdir nvgre parameters check

> >

> > Add mask parameters check in nvgre parser for flow API.

> >

> > Fixes: 30965ca341278 ("net/i40e: add NVGRE flow parsing")

> >

> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>

> > ---

> >  drivers/net/i40e/i40e_flow.c | 35

> +++++++++++++++++++++++++++++++++++

> >  1 file changed, 35 insertions(+)

> >

> > diff --git a/drivers/net/i40e/i40e_flow.c

> > b/drivers/net/i40e/i40e_flow.c index

> > 7e4936e..05e54f1 100644

> > --- a/drivers/net/i40e/i40e_flow.c

> > +++ b/drivers/net/i40e/i40e_flow.c

> > @@ -3610,6 +3610,41 @@ i40e_flow_parse_nvgre_pattern(__rte_unused

> > struct rte_eth_dev *dev,

> >  						       "Invalid TNI mask");

> >  					return -rte_errno;

> >  				}

> > +				if (nvgre_mask->protocol &&

> > +					nvgre_mask->protocol != 0xFFFF) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> 	RTE_FLOW_ERROR_TYPE_ITEM,

> > +						item,

> > +						"Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_mask->c_k_s_rsvd0_ver &&

> > +					nvgre_mask->c_k_s_rsvd0_ver !=

> > +					rte_cpu_to_be_16(0x3000)) {

> 

> If " c_k_s_rsvd0_ver must have value 0x2000 according to RFC 7637" , I think

> here we should have mask 0xffff to make sure each bit of c_k_s_rsvd0_ver

> be set correctly.


I will change c_k_s_rsvd0_ver  mask check to 0Xffff, may be that is more reasonable.

> 

> Regards

> Qi

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_spec->c_k_s_rsvd0_ver !=

> > +					rte_cpu_to_be_16(0x2000) &&

> > +					nvgre_mask->c_k_s_rsvd0_ver) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> > +				if (nvgre_mask->protocol &&

> > +					nvgre_spec->protocol !=

> > +					rte_cpu_to_be_16(0x6558)) {

> > +					rte_flow_error_set(error, EINVAL,

> > +

> RTE_FLOW_ERROR_TYPE_ITEM,

> > +						   item,

> > +						   "Invalid NVGRE item");

> > +					return -rte_errno;

> > +				}

> >  				rte_memcpy(((uint8_t *)&tenant_id_be + 1),

> >  					   nvgre_spec->tni, 3);

> >  				filter->tenant_id =

> > --

> > 2.9.3
  

Patch

diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index 7e4936e..05e54f1 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -3610,6 +3610,41 @@  i40e_flow_parse_nvgre_pattern(__rte_unused struct rte_eth_dev *dev,
 						       "Invalid TNI mask");
 					return -rte_errno;
 				}
+				if (nvgre_mask->protocol &&
+					nvgre_mask->protocol != 0xFFFF) {
+					rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ITEM,
+						item,
+						"Invalid NVGRE item");
+					return -rte_errno;
+				}
+				if (nvgre_mask->c_k_s_rsvd0_ver &&
+					nvgre_mask->c_k_s_rsvd0_ver !=
+					rte_cpu_to_be_16(0x3000)) {
+					rte_flow_error_set(error, EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ITEM,
+						   item,
+						   "Invalid NVGRE item");
+					return -rte_errno;
+				}
+				if (nvgre_spec->c_k_s_rsvd0_ver !=
+					rte_cpu_to_be_16(0x2000) &&
+					nvgre_mask->c_k_s_rsvd0_ver) {
+					rte_flow_error_set(error, EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ITEM,
+						   item,
+						   "Invalid NVGRE item");
+					return -rte_errno;
+				}
+				if (nvgre_mask->protocol &&
+					nvgre_spec->protocol !=
+					rte_cpu_to_be_16(0x6558)) {
+					rte_flow_error_set(error, EINVAL,
+						   RTE_FLOW_ERROR_TYPE_ITEM,
+						   item,
+						   "Invalid NVGRE item");
+					return -rte_errno;
+				}
 				rte_memcpy(((uint8_t *)&tenant_id_be + 1),
 					   nvgre_spec->tni, 3);
 				filter->tenant_id =