diff mbox series

[1/3] net/ice: fix flow get inputset check

Message ID 1563413923-404004-2-git-send-email-ying.a.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [1/3] net/ice: fix flow get inputset check |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Ying Wang July 18, 2019, 1:38 a.m. UTC 
  ice_get_flow_field should not set error if item->type is
RTE_FLOW_ITEM_TYPE_VOID.
This patch fixes this issue.

Fixes: d76116a4678f ("net/ice: add generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
---
 drivers/net/ice/ice_generic_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Qiming Yang July 18, 2019, 2:33 a.m. UTC | #1 
It's no need to add RTE_FLOW_ITEM_TYPE_VOID because we have skipped all the VOID items.

-----Original Message-----
From: Wang, Ying A 
Sent: Thursday, July 18, 2019 9:39 AM
To: Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org; Wang, Ying A <ying.a.wang@intel.com>; stable@dpdk.org
Subject: [PATCH 1/3] net/ice: fix flow get inputset check

ice_get_flow_field should not set error if item->type is RTE_FLOW_ITEM_TYPE_VOID.
This patch fixes this issue.

Fixes: d76116a4678f ("net/ice: add generic flow API")
Cc: stable@dpdk.org

Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
---
 drivers/net/ice/ice_generic_flow.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index c2931a1..464f6ec 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -465,7 +465,7 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
 			nvgre_spec = item->spec;
 			nvgre_mask = item->mask;
-			/* Check if VXLAN item is used to describe protocol.
+			/* Check if NVGRE item is used to describe protocol.
 			 * If yes, both spec and mask should be NULL.
 			 * If no, both spec and mask shouldn't be NULL.
 			 */
@@ -480,6 +480,8 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			is_tunnel = 1;
 
 			break;
+		case RTE_FLOW_ITEM_TYPE_VOID:
+			break;
 		default:
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,
--
1.8.3.1
Ying Wang July 18, 2019, 2:40 a.m. UTC | #2 
> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, July 18, 2019 10:33 AM
> To: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> It's no need to add RTE_FLOW_ITEM_TYPE_VOID because we have skipped all
> the VOID items.

But actually, we passed the original pattern to this function, not the one skipped all the VOID items.


> 
> -----Original Message-----
> From: Wang, Ying A
> Sent: Thursday, July 18, 2019 9:39 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org; Wang, Ying A
> <ying.a.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> ice_get_flow_field should not set error if item->type is
> RTE_FLOW_ITEM_TYPE_VOID.
> This patch fixes this issue.
> 
> Fixes: d76116a4678f ("net/ice: add generic flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
> ---
>  drivers/net/ice/ice_generic_flow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> index c2931a1..464f6ec 100644
> --- a/drivers/net/ice/ice_generic_flow.c
> +++ b/drivers/net/ice/ice_generic_flow.c
> @@ -465,7 +465,7 @@ static uint64_t ice_get_flow_field(const struct
> rte_flow_item pattern[],
>  		case RTE_FLOW_ITEM_TYPE_NVGRE:
>  			nvgre_spec = item->spec;
>  			nvgre_mask = item->mask;
> -			/* Check if VXLAN item is used to describe protocol.
> +			/* Check if NVGRE item is used to describe protocol.
>  			 * If yes, both spec and mask should be NULL.
>  			 * If no, both spec and mask shouldn't be NULL.
>  			 */
> @@ -480,6 +480,8 @@ static uint64_t ice_get_flow_field(const struct
> rte_flow_item pattern[],
>  			is_tunnel = 1;
> 
>  			break;
> +		case RTE_FLOW_ITEM_TYPE_VOID:
> +			break;
>  		default:
>  			rte_flow_error_set(error, EINVAL,
>  					   RTE_FLOW_ERROR_TYPE_ITEM,
> --
> 1.8.3.1
Qiming Yang July 18, 2019, 2:46 a.m. UTC | #3 
So I think the bug is the skipped pattern don't pass to the next function.

-----Original Message-----
From: Wang, Ying A 
Sent: Thursday, July 18, 2019 10:40 AM
To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Cc: dev@dpdk.org; stable@dpdk.org
Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check



> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, July 18, 2019 10:33 AM
> To: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> It's no need to add RTE_FLOW_ITEM_TYPE_VOID because we have skipped 
> all the VOID items.

But actually, we passed the original pattern to this function, not the one skipped all the VOID items.


> 
> -----Original Message-----
> From: Wang, Ying A
> Sent: Thursday, July 18, 2019 9:39 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org; Wang, Ying A 
> <ying.a.wang@intel.com>; stable@dpdk.org
> Subject: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> ice_get_flow_field should not set error if item->type is 
> RTE_FLOW_ITEM_TYPE_VOID.
> This patch fixes this issue.
> 
> Fixes: d76116a4678f ("net/ice: add generic flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
> ---
>  drivers/net/ice/ice_generic_flow.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ice/ice_generic_flow.c
> b/drivers/net/ice/ice_generic_flow.c
> index c2931a1..464f6ec 100644
> --- a/drivers/net/ice/ice_generic_flow.c
> +++ b/drivers/net/ice/ice_generic_flow.c
> @@ -465,7 +465,7 @@ static uint64_t ice_get_flow_field(const struct 
> rte_flow_item pattern[],
>  		case RTE_FLOW_ITEM_TYPE_NVGRE:
>  			nvgre_spec = item->spec;
>  			nvgre_mask = item->mask;
> -			/* Check if VXLAN item is used to describe protocol.
> +			/* Check if NVGRE item is used to describe protocol.
>  			 * If yes, both spec and mask should be NULL.
>  			 * If no, both spec and mask shouldn't be NULL.
>  			 */
> @@ -480,6 +480,8 @@ static uint64_t ice_get_flow_field(const struct 
> rte_flow_item pattern[],
>  			is_tunnel = 1;
> 
>  			break;
> +		case RTE_FLOW_ITEM_TYPE_VOID:
> +			break;
>  		default:
>  			rte_flow_error_set(error, EINVAL,
>  					   RTE_FLOW_ERROR_TYPE_ITEM,
> --
> 1.8.3.1
Ying Wang July 18, 2019, 2:52 a.m. UTC | #4 
> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, July 18, 2019 10:47 AM
> To: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> So I think the bug is the skipped pattern don't pass to the next function.

Yes, this is another solution. But maybe, this solution will change more codes. 
Anyway, if you think this is the better solution, I will modify it.
> 
> -----Original Message-----
> From: Wang, Ying A
> Sent: Thursday, July 18, 2019 10:40 AM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> 
> 
> > -----Original Message-----
> > From: Yang, Qiming
> > Sent: Thursday, July 18, 2019 10:33 AM
> > To: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> >
> > It's no need to add RTE_FLOW_ITEM_TYPE_VOID because we have skipped
> > all the VOID items.
> 
> But actually, we passed the original pattern to this function, not the one skipped
> all the VOID items.
> 
> 
> >
> > -----Original Message-----
> > From: Wang, Ying A
> > Sent: Thursday, July 18, 2019 9:39 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org; Wang, Ying A
> > <ying.a.wang@intel.com>; stable@dpdk.org
> > Subject: [PATCH 1/3] net/ice: fix flow get inputset check
> >
> > ice_get_flow_field should not set error if item->type is
> > RTE_FLOW_ITEM_TYPE_VOID.
> > This patch fixes this issue.
> >
> > Fixes: d76116a4678f ("net/ice: add generic flow API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wang Ying A <ying.a.wang@intel.com>
> > ---
> >  drivers/net/ice/ice_generic_flow.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ice/ice_generic_flow.c
> > b/drivers/net/ice/ice_generic_flow.c
> > index c2931a1..464f6ec 100644
> > --- a/drivers/net/ice/ice_generic_flow.c
> > +++ b/drivers/net/ice/ice_generic_flow.c
> > @@ -465,7 +465,7 @@ static uint64_t ice_get_flow_field(const struct
> > rte_flow_item pattern[],
> >  		case RTE_FLOW_ITEM_TYPE_NVGRE:
> >  			nvgre_spec = item->spec;
> >  			nvgre_mask = item->mask;
> > -			/* Check if VXLAN item is used to describe protocol.
> > +			/* Check if NVGRE item is used to describe protocol.
> >  			 * If yes, both spec and mask should be NULL.
> >  			 * If no, both spec and mask shouldn't be NULL.
> >  			 */
> > @@ -480,6 +480,8 @@ static uint64_t ice_get_flow_field(const struct
> > rte_flow_item pattern[],
> >  			is_tunnel = 1;
> >
> >  			break;
> > +		case RTE_FLOW_ITEM_TYPE_VOID:
> > +			break;
> >  		default:
> >  			rte_flow_error_set(error, EINVAL,
> >  					   RTE_FLOW_ERROR_TYPE_ITEM,
> > --
> > 1.8.3.1
Qi Zhang July 19, 2019, 12:31 a.m. UTC | #5 
> -----Original Message-----
> From: Wang, Ying A
> Sent: Thursday, July 18, 2019 10:52 AM
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> 
> 
> 
> > -----Original Message-----
> > From: Yang, Qiming
> > Sent: Thursday, July 18, 2019 10:47 AM
> > To: Wang, Ying A <ying.a.wang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [PATCH 1/3] net/ice: fix flow get inputset check
> >
> > So I think the bug is the skipped pattern don't pass to the next function.
> 
> Yes, this is another solution. But maybe, this solution will change more codes.
> Anyway, if you think this is the better solution, I will modify it.

Let's keep current fix so far.
code refactoring to avoid duplicate void item handle can be done on a separate patch later.

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
diff mbox series

Patch

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index c2931a1..464f6ec 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -465,7 +465,7 @@  static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
 			nvgre_spec = item->spec;
 			nvgre_mask = item->mask;
-			/* Check if VXLAN item is used to describe protocol.
+			/* Check if NVGRE item is used to describe protocol.
 			 * If yes, both spec and mask should be NULL.
 			 * If no, both spec and mask shouldn't be NULL.
 			 */
@@ -480,6 +480,8 @@  static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			is_tunnel = 1;
 
 			break;
+		case RTE_FLOW_ITEM_TYPE_VOID:
+			break;
 		default:
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,