DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/3] ice: flow inputset/action validation fix and add ipv6 tc support
@ 2019-07-18  1:38 Wang Ying A
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check Wang Ying A
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Wang Ying A @ 2019-07-18  1:38 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: qiming.yang, dev, ying.a.wang

Patch 1/3 avoids set error for RTE_FLOW_ITEM_TYPE_VOID in inputset check.
Patch 2/3 adds check for each element of action list.
Patch 3/3 adds ipv6 tc support for flow rule.

Wang Ying A (3):
  net/ice: fix flow get inputset check
  net/ice: fix flow action validation
  net/ice: add flow ipv6 tc support

 drivers/net/ice/ice_generic_flow.c | 53 ++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 22 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  1:38 [dpdk-dev] [PATCH 0/3] ice: flow inputset/action validation fix and add ipv6 tc support Wang Ying A
@ 2019-07-18  1:38 ` Wang Ying A
  2019-07-18  2:33   ` Yang, Qiming
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support Wang Ying A
  2 siblings, 1 reply; 15+ messages in thread
From: Wang Ying A @ 2019-07-18  1:38 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: qiming.yang, dev, ying.a.wang, stable

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation
  2019-07-18  1:38 [dpdk-dev] [PATCH 0/3] ice: flow inputset/action validation fix and add ipv6 tc support Wang Ying A
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check Wang Ying A
@ 2019-07-18  1:38 ` Wang Ying A
  2019-07-18  2:40   ` Yang, Qiming
                     ` (2 more replies)
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support Wang Ying A
  2 siblings, 3 replies; 15+ messages in thread
From: Wang Ying A @ 2019-07-18  1:38 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: qiming.yang, dev, ying.a.wang, stable

Action is a list. We should check each element of the action
rather than the first one.
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 | 39 ++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 464f6ec..2c57276 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -517,28 +517,31 @@ static int ice_flow_valid_action(struct rte_eth_dev *dev,
 {
 	const struct rte_flow_action_queue *act_q;
 	uint16_t queue;
-
-	switch (actions->type) {
-	case RTE_FLOW_ACTION_TYPE_QUEUE:
-		act_q = actions->conf;
-		queue = act_q->index;
-		if (queue >= dev->data->nb_rx_queues) {
+	const struct rte_flow_action *action;
+	for (action = actions; action->type !=
+			RTE_FLOW_ACTION_TYPE_END; action++) {
+		switch (action->type) {
+		case RTE_FLOW_ACTION_TYPE_QUEUE:
+			act_q = action->conf;
+			queue = act_q->index;
+			if (queue >= dev->data->nb_rx_queues) {
+				rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						actions, "Invalid queue ID for"
+						" switch filter.");
+				return -rte_errno;
+			}
+			break;
+		case RTE_FLOW_ACTION_TYPE_DROP:
+		case RTE_FLOW_ACTION_TYPE_VOID:
+			break;
+		default:
 			rte_flow_error_set(error, EINVAL,
-					   RTE_FLOW_ERROR_TYPE_ACTION,
-					   actions, "Invalid queue ID for"
-					   " switch filter.");
+					   RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					   "Invalid action.");
 			return -rte_errno;
 		}
-		break;
-	case RTE_FLOW_ACTION_TYPE_DROP:
-		break;
-	default:
-		rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ACTION, actions,
-				   "Invalid action.");
-		return -rte_errno;
 	}
-
 	return 0;
 }
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support
  2019-07-18  1:38 [dpdk-dev] [PATCH 0/3] ice: flow inputset/action validation fix and add ipv6 tc support Wang Ying A
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check Wang Ying A
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
@ 2019-07-18  1:38 ` Wang Ying A
  2019-07-18  3:06   ` Yang, Qiming
  2 siblings, 1 reply; 15+ messages in thread
From: Wang Ying A @ 2019-07-18  1:38 UTC (permalink / raw)
  To: qi.z.zhang; +Cc: qiming.yang, dev, ying.a.wang, stable

When set flow ipv6 tc rule, ice_get_flow_field will set error.
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 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 2c57276..5725bff 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -282,8 +282,7 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			if (!(ipv6_spec && ipv6_mask))
 				break;
 
-			if (ipv6_mask->hdr.payload_len ||
-			    ipv6_mask->hdr.vtc_flow) {
+			if (ipv6_mask->hdr.payload_len) {
 				rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,
 					   item,
@@ -317,6 +316,11 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 					input_set |= ICE_INSET_IPV6_PROTO;
 				if (ipv6_mask->hdr.hop_limits == UINT8_MAX)
 					input_set |= ICE_INSET_IPV6_HOP_LIMIT;
+				if ((ipv6_mask->hdr.vtc_flow &
+					rte_cpu_to_be_32(RTE_IPV6_HDR_TC_MASK))
+						== rte_cpu_to_be_32
+						(RTE_IPV6_HDR_TC_MASK))
+					input_set |= ICE_INSET_IPV6_TOS;
 			}
 
 			break;
@@ -486,7 +490,7 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,
 					   item,
-					   "Invalid mask no exist");
+					   "Invalid pattern");
 			break;
 		}
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check Wang Ying A
@ 2019-07-18  2:33   ` Yang, Qiming
  2019-07-18  2:40     ` Wang, Ying A
  0 siblings, 1 reply; 15+ messages in thread
From: Yang, Qiming @ 2019-07-18  2:33 UTC (permalink / raw)
  To: Wang, Ying A, Zhang, Qi Z; +Cc: dev, stable

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  2:33   ` Yang, Qiming
@ 2019-07-18  2:40     ` Wang, Ying A
  2019-07-18  2:46       ` Yang, Qiming
  0 siblings, 1 reply; 15+ messages in thread
From: Wang, Ying A @ 2019-07-18  2:40 UTC (permalink / raw)
  To: Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable



> -----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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
@ 2019-07-18  2:40   ` Yang, Qiming
  2019-07-18  3:06   ` Yang, Qiming
  2019-07-18  9:08   ` Ye Xiaolong
  2 siblings, 0 replies; 15+ messages in thread
From: Yang, Qiming @ 2019-07-18  2:40 UTC (permalink / raw)
  To: Wang, Ying A, Zhang, Qi Z, Xing, Beilei; +Cc: dev

Hi, Ying
For the action check reorganization, please according to Beilei's comments, use array  to stand the action element list.
And as we discussed before, it's better to change in the next release.

Qiming
-----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 2/3] net/ice: fix flow action validation

Action is a list. We should check each element of the action rather than the first one.
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 | 39 ++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 464f6ec..2c57276 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -517,28 +517,31 @@ static int ice_flow_valid_action(struct rte_eth_dev *dev,  {
 	const struct rte_flow_action_queue *act_q;
 	uint16_t queue;
-
-	switch (actions->type) {
-	case RTE_FLOW_ACTION_TYPE_QUEUE:
-		act_q = actions->conf;
-		queue = act_q->index;
-		if (queue >= dev->data->nb_rx_queues) {
+	const struct rte_flow_action *action;
+	for (action = actions; action->type !=
+			RTE_FLOW_ACTION_TYPE_END; action++) {
+		switch (action->type) {
+		case RTE_FLOW_ACTION_TYPE_QUEUE:
+			act_q = action->conf;
+			queue = act_q->index;
+			if (queue >= dev->data->nb_rx_queues) {
+				rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						actions, "Invalid queue ID for"
+						" switch filter.");
+				return -rte_errno;
+			}
+			break;
+		case RTE_FLOW_ACTION_TYPE_DROP:
+		case RTE_FLOW_ACTION_TYPE_VOID:
+			break;
+		default:
 			rte_flow_error_set(error, EINVAL,
-					   RTE_FLOW_ERROR_TYPE_ACTION,
-					   actions, "Invalid queue ID for"
-					   " switch filter.");
+					   RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					   "Invalid action.");
 			return -rte_errno;
 		}
-		break;
-	case RTE_FLOW_ACTION_TYPE_DROP:
-		break;
-	default:
-		rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ACTION, actions,
-				   "Invalid action.");
-		return -rte_errno;
 	}
-
 	return 0;
 }
 
--
1.8.3.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  2:40     ` Wang, Ying A
@ 2019-07-18  2:46       ` Yang, Qiming
  2019-07-18  2:52         ` Wang, Ying A
  0 siblings, 1 reply; 15+ messages in thread
From: Yang, Qiming @ 2019-07-18  2:46 UTC (permalink / raw)
  To: Wang, Ying A, Zhang, Qi Z; +Cc: dev, stable

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  2:46       ` Yang, Qiming
@ 2019-07-18  2:52         ` Wang, Ying A
  2019-07-19  0:31           ` Zhang, Qi Z
  0 siblings, 1 reply; 15+ messages in thread
From: Wang, Ying A @ 2019-07-18  2:52 UTC (permalink / raw)
  To: Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable



> -----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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support Wang Ying A
@ 2019-07-18  3:06   ` Yang, Qiming
  2019-07-19  0:48     ` Zhang, Qi Z
  0 siblings, 1 reply; 15+ messages in thread
From: Yang, Qiming @ 2019-07-18  3:06 UTC (permalink / raw)
  To: Wang, Ying A, Zhang, Qi Z; +Cc: dev, stable



-----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 3/3] net/ice: add flow ipv6 tc support

When set flow ipv6 tc rule, ice_get_flow_field will set error.
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 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 2c57276..5725bff 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -282,8 +282,7 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			if (!(ipv6_spec && ipv6_mask))
 				break;
 
-			if (ipv6_mask->hdr.payload_len ||
-			    ipv6_mask->hdr.vtc_flow) {
+			if (ipv6_mask->hdr.payload_len) {
 				rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,
 					   item,
@@ -317,6 +316,11 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 					input_set |= ICE_INSET_IPV6_PROTO;
 				if (ipv6_mask->hdr.hop_limits == UINT8_MAX)
 					input_set |= ICE_INSET_IPV6_HOP_LIMIT;
+				if ((ipv6_mask->hdr.vtc_flow &
+					rte_cpu_to_be_32(RTE_IPV6_HDR_TC_MASK))
+						== rte_cpu_to_be_32
+						(RTE_IPV6_HDR_TC_MASK))
+					input_set |= ICE_INSET_IPV6_TOS;
 			}
 
 			break;
@@ -486,7 +490,7 @@ static uint64_t ice_get_flow_field(const struct rte_flow_item pattern[],
 			rte_flow_error_set(error, EINVAL,
 					   RTE_FLOW_ERROR_TYPE_ITEM,
 					   item,
-					   "Invalid mask no exist");
+					   "Invalid pattern");
 			break;
 		}
 	}
-- 
1.8.3.1

Acked-by: Qiming Yang <qiming.yang@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
  2019-07-18  2:40   ` Yang, Qiming
@ 2019-07-18  3:06   ` Yang, Qiming
  2019-07-19  0:34     ` Zhang, Qi Z
  2019-07-18  9:08   ` Ye Xiaolong
  2 siblings, 1 reply; 15+ messages in thread
From: Yang, Qiming @ 2019-07-18  3:06 UTC (permalink / raw)
  To: Wang, Ying A, Zhang, Qi Z; +Cc: dev, stable



-----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 2/3] net/ice: fix flow action validation

Action is a list. We should check each element of the action rather than the first one.
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 | 39 ++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 464f6ec..2c57276 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -517,28 +517,31 @@ static int ice_flow_valid_action(struct rte_eth_dev *dev,  {
 	const struct rte_flow_action_queue *act_q;
 	uint16_t queue;
-
-	switch (actions->type) {
-	case RTE_FLOW_ACTION_TYPE_QUEUE:
-		act_q = actions->conf;
-		queue = act_q->index;
-		if (queue >= dev->data->nb_rx_queues) {
+	const struct rte_flow_action *action;
+	for (action = actions; action->type !=
+			RTE_FLOW_ACTION_TYPE_END; action++) {
+		switch (action->type) {
+		case RTE_FLOW_ACTION_TYPE_QUEUE:
+			act_q = action->conf;
+			queue = act_q->index;
+			if (queue >= dev->data->nb_rx_queues) {
+				rte_flow_error_set(error, EINVAL,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						actions, "Invalid queue ID for"
+						" switch filter.");
+				return -rte_errno;
+			}
+			break;
+		case RTE_FLOW_ACTION_TYPE_DROP:
+		case RTE_FLOW_ACTION_TYPE_VOID:
+			break;
+		default:
 			rte_flow_error_set(error, EINVAL,
-					   RTE_FLOW_ERROR_TYPE_ACTION,
-					   actions, "Invalid queue ID for"
-					   " switch filter.");
+					   RTE_FLOW_ERROR_TYPE_ACTION, actions,
+					   "Invalid action.");
 			return -rte_errno;
 		}
-		break;
-	case RTE_FLOW_ACTION_TYPE_DROP:
-		break;
-	default:
-		rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_ACTION, actions,
-				   "Invalid action.");
-		return -rte_errno;
 	}
-
 	return 0;
 }
 
--
1.8.3.1

Acked-by: Qiming Yang <qiming.yang@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation
  2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
  2019-07-18  2:40   ` Yang, Qiming
  2019-07-18  3:06   ` Yang, Qiming
@ 2019-07-18  9:08   ` Ye Xiaolong
  2 siblings, 0 replies; 15+ messages in thread
From: Ye Xiaolong @ 2019-07-18  9:08 UTC (permalink / raw)
  To: Wang Ying A; +Cc: qi.z.zhang, qiming.yang, dev, stable

On 07/18, Wang Ying A wrote:
>Action is a list. We should check each element of the action
>rather than the first one.
>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 | 39 ++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
>diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
>index 464f6ec..2c57276 100644
>--- a/drivers/net/ice/ice_generic_flow.c
>+++ b/drivers/net/ice/ice_generic_flow.c
>@@ -517,28 +517,31 @@ static int ice_flow_valid_action(struct rte_eth_dev *dev,
> {
> 	const struct rte_flow_action_queue *act_q;
> 	uint16_t queue;
>-
>-	switch (actions->type) {
>-	case RTE_FLOW_ACTION_TYPE_QUEUE:
>-		act_q = actions->conf;
>-		queue = act_q->index;
>-		if (queue >= dev->data->nb_rx_queues) {
>+	const struct rte_flow_action *action;
>+	for (action = actions; action->type !=
>+			RTE_FLOW_ACTION_TYPE_END; action++) {
>+		switch (action->type) {
>+		case RTE_FLOW_ACTION_TYPE_QUEUE:
>+			act_q = action->conf;
>+			queue = act_q->index;
>+			if (queue >= dev->data->nb_rx_queues) {
>+				rte_flow_error_set(error, EINVAL,
>+						RTE_FLOW_ERROR_TYPE_ACTION,
>+						actions, "Invalid queue ID for"
>+						" switch filter.");
>+				return -rte_errno;
>+			}
>+			break;
>+		case RTE_FLOW_ACTION_TYPE_DROP:
>+		case RTE_FLOW_ACTION_TYPE_VOID:
>+			break;
>+		default:
> 			rte_flow_error_set(error, EINVAL,
>-					   RTE_FLOW_ERROR_TYPE_ACTION,
>-					   actions, "Invalid queue ID for"
>-					   " switch filter.");
>+					   RTE_FLOW_ERROR_TYPE_ACTION, actions,
>+					   "Invalid action.");
> 			return -rte_errno;
> 		}
>-		break;
>-	case RTE_FLOW_ACTION_TYPE_DROP:
>-		break;
>-	default:
>-		rte_flow_error_set(error, EINVAL,
>-				   RTE_FLOW_ERROR_TYPE_ACTION, actions,
>-				   "Invalid action.");
>-		return -rte_errno;
> 	}
>-
> 	return 0;
> }
> 
>-- 
>1.8.3.1
>

Reviewed-by: Xiaolong Ye <xiaolong.ye@intel.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check
  2019-07-18  2:52         ` Wang, Ying A
@ 2019-07-19  0:31           ` Zhang, Qi Z
  0 siblings, 0 replies; 15+ messages in thread
From: Zhang, Qi Z @ 2019-07-19  0:31 UTC (permalink / raw)
  To: Wang, Ying A, Yang, Qiming; +Cc: dev, stable



> -----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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation
  2019-07-18  3:06   ` Yang, Qiming
@ 2019-07-19  0:34     ` Zhang, Qi Z
  0 siblings, 0 replies; 15+ messages in thread
From: Zhang, Qi Z @ 2019-07-19  0:34 UTC (permalink / raw)
  To: Yang, Qiming, Wang, Ying A; +Cc: dev, stable



> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, July 18, 2019 11:07 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 2/3] net/ice: fix flow action validation
> 
> 
> 
> -----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 2/3] net/ice: fix flow action validation
> 
> Action is a list. We should check each element of the action rather than the
> first one.
> 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>
> ---
> Acked-by: Qiming Yang <qiming.yang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support
  2019-07-18  3:06   ` Yang, Qiming
@ 2019-07-19  0:48     ` Zhang, Qi Z
  0 siblings, 0 replies; 15+ messages in thread
From: Zhang, Qi Z @ 2019-07-19  0:48 UTC (permalink / raw)
  To: Yang, Qiming, Wang, Ying A; +Cc: dev, stable



> -----Original Message-----
> From: Yang, Qiming
> Sent: Thursday, July 18, 2019 11:06 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 3/3] net/ice: add flow ipv6 tc support
> 
> 
> 
> -----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 3/3] net/ice: add flow ipv6 tc support
> 
> When set flow ipv6 tc rule, ice_get_flow_field will set error.
> 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>
> ---
> 
> Acked-by: Qiming Yang <qiming.yang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-18  1:38 [dpdk-dev] [PATCH 0/3] ice: flow inputset/action validation fix and add ipv6 tc support Wang Ying A
2019-07-18  1:38 ` [dpdk-dev] [PATCH 1/3] net/ice: fix flow get inputset check Wang Ying A
2019-07-18  2:33   ` Yang, Qiming
2019-07-18  2:40     ` Wang, Ying A
2019-07-18  2:46       ` Yang, Qiming
2019-07-18  2:52         ` Wang, Ying A
2019-07-19  0:31           ` Zhang, Qi Z
2019-07-18  1:38 ` [dpdk-dev] [PATCH 2/3] net/ice: fix flow action validation Wang Ying A
2019-07-18  2:40   ` Yang, Qiming
2019-07-18  3:06   ` Yang, Qiming
2019-07-19  0:34     ` Zhang, Qi Z
2019-07-18  9:08   ` Ye Xiaolong
2019-07-18  1:38 ` [dpdk-dev] [PATCH 3/3] net/ice: add flow ipv6 tc support Wang Ying A
2019-07-18  3:06   ` Yang, Qiming
2019-07-19  0:48     ` Zhang, Qi Z

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox