[dpdk-dev] [PATCH 0/3] update ice switch base code

[dpdk-dev] [PATCH 0/3] update ice switch base code

[Wei Zhao]

This patch set fix some ice base code bug of switch.

Wei Zhao (3):
  net/ice/base: check the number of recipe when in chain
  net/ice/base: add mask check when find switch recipe
  net/ice/base: force switch to use different recipe for

 drivers/net/ice/base/ice_switch.c | 23 +++++++++++++++++------
 drivers/net/ice/base/ice_switch.h |  1 +
 2 files changed, 18 insertions(+), 6 deletions(-)

-- 
2.19.1

[dpdk-dev] [PATCH 1/3] net/ice/base: check the number of recipe when in chain

[Wei Zhao]

when we add some long switch rule, we need check the
number of final recipe number, if it is large than
ICE_MAX_CHAIN_RECIPE, we should refuse this rule.
For example:

"flow create 0 ingress pattern eth / ipv6
src is CDCD:910A:2222:5498:8475:1111:3900:1536
dst is CDCD:910A:2222:5498:8475:1111:3900:2022
tc is 3 / udp dst is 45 / end actions queue index 2 / end"

This rule will consum 6 recipe, if it is not refused, it
will cause the following code over write of lkup_indx and mask.

LIST_FOR_EACH_ENTRY(entry, &rm->rg_list, ice_recp_grp_entry,
		l_entry) {
	last_chain_entry->fv_idx[i] = entry->chain_idx;
	buf[recps].content.lkup_indx[i] = entry->chain_idx;
	buf[recps].content.mask[i++] = CPU_TO_LE16(0xFFFF);
	..........
}

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

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index b5aa5abd9..c17219274 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -5352,6 +5352,9 @@ ice_add_sw_recipe(struct ice_hw *hw, struct ice_sw_recipe *rm,
 		rm->n_grp_count++;
 	}
 
+	if (rm->n_grp_count > ICE_MAX_CHAIN_RECIPE)
+		return ICE_ERR_MAX_LIMIT;
+
 	tmp = (struct ice_aqc_recipe_data_elem *)ice_calloc(hw,
 							    ICE_MAX_NUM_RECIPES,
 							    sizeof(*tmp));
-- 
2.19.1

[dpdk-dev] [PATCH 2/3] net/ice/base: add mask check when find switch recipe

[Wei Zhao]

In order to find accurate recipe for switch filter, we
need to add mask as an element when searching for recipe.
If we create different rules with the same input set, but
using different masks, then proper recipes should use
those different mask.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/ice/base/ice_switch.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index c17219274..bc0c368d7 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -5012,6 +5012,8 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 		if (lkup_exts->n_val_words == recp[i].lkup_exts.n_val_words) {
 			struct ice_fv_word *a = lkup_exts->fv_words;
 			struct ice_fv_word *b = recp[i].lkup_exts.fv_words;
+			u16 *c = recp[i].lkup_exts.field_mask;
+			u16 *d = lkup_exts->field_mask;
 			bool found = true;
 			u8 p, q;
 
@@ -5019,7 +5021,8 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 				for (q = 0; q < recp[i].lkup_exts.n_val_words;
 				     q++) {
 					if (a[p].off == b[q].off &&
-					    a[p].prot_id == b[q].prot_id)
+					    a[p].prot_id == b[q].prot_id &&
+					    d[p] == c[q])
 						/* Found the "p"th word in the
 						 * given recipe
 						 */
-- 
2.19.1

[dpdk-dev] [PATCH 3/3] net/ice/base: force switch to use different recipe for

[Wei Zhao]

When we use profile rule as swicth rule to download, if
we download 2 different rules one by one, there will be
rejection from function ice_aq_sw_rules(), for example:

"flow create 0 priority 0 ingress pattern eth / ipv6 / ah
/ end actions queue index 3 / end"
"flow create 0 priority 0 ingress pattern eth / ipv6 / esp
/ end actions queue index 2 / end"

That is because the 2 rules has the same s_rule input set
except action queue index, so it will be rejected by
hardware. So we have to use different recipes for them.

Also, we need to add recipe_id to keep record of recipe
index, which will be used in rule remove, if not, there
will be error when search recipe in function
ice_rem_adv_rule() if we create 2 or more profile rule.
For example:

"flow create 0 priority 0 ingress pattern eth / ipv4 / udp
/ pfcp s_field is 1 / end actions queue index 4 / end"
"flow create 0 priority 0 ingress pattern eth / ipv4 / udp
/ pfcp s_field is 0 / end actions queue index 5 / end"

then,

"flow flush 0"

you will find only the first rule will be delete,
because ice_find_recp() will always return recipe
id of the first rule.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/ice/base/ice_switch.c | 15 ++++++++++-----
 drivers/net/ice/base/ice_switch.h |  1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index bc0c368d7..d8eec7f51 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -4983,7 +4983,8 @@ static const struct ice_protocol_entry ice_prot_id_tbl[ICE_PROTOCOL_LAST] = {
  *
  * Returns index of matching recipe, or ICE_MAX_NUM_RECIPES if not found.
  */
-static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
+static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts,
+			 enum ice_sw_tunnel_type tun_type)
 {
 	bool refresh_required = true;
 	struct ice_sw_recipe *recp;
@@ -5042,7 +5043,10 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 			/* If for "i"th recipe the found was never set to false
 			 * then it means we found our match
 			 */
-			if (found)
+			if (ice_is_prof_rule(tun_type) &&
+			    tun_type == recp[i].tun_type && found)
+				return i; /* Return the recipe ID */
+			else if (!ice_is_prof_rule(tun_type) && found)
 				return i; /* Return the recipe ID */
 		}
 	}
@@ -5798,7 +5802,7 @@ ice_get_compat_fv_bitmap(struct ice_hw *hw, struct ice_adv_rule_info *rinfo,
  * if the rule type is a profile rule, that means that there no field value
  * match required, in this case just a profile hit is required.
  */
-static bool ice_is_prof_rule(enum ice_sw_tunnel_type type)
+bool ice_is_prof_rule(enum ice_sw_tunnel_type type)
 {
 	switch (type) {
 	case ICE_SW_TUN_PROFID_IPV6_ESP:
@@ -5952,11 +5956,12 @@ ice_add_adv_recipe(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 		goto err_free_lkup_exts;
 
 	/* Look for a recipe which matches our requested fv / mask list */
-	*rid = ice_find_recp(hw, lkup_exts);
+	*rid = ice_find_recp(hw, lkup_exts, rinfo->tun_type);
 	if (*rid < ICE_MAX_NUM_RECIPES)
 		/* Success if found a recipe that match the existing criteria */
 		goto err_unroll;
 
+	rm->tun_type = rinfo->tun_type;
 	/* Recipe we need does not exist, add a recipe */
 	status = ice_add_sw_recipe(hw, rm, match_tun, profiles);
 	if (status)
@@ -6873,7 +6878,7 @@ ice_rem_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 	if (status)
 		return status;
 
-	rid = ice_find_recp(hw, &lkup_exts);
+	rid = ice_find_recp(hw, &lkup_exts, rinfo->tun_type);
 	/* If did not find a recipe that match the existing criteria */
 	if (rid == ICE_MAX_NUM_RECIPES)
 		return ICE_ERR_PARAM;
diff --git a/drivers/net/ice/base/ice_switch.h b/drivers/net/ice/base/ice_switch.h
index f7ae5c741..9666422f7 100644
--- a/drivers/net/ice/base/ice_switch.h
+++ b/drivers/net/ice/base/ice_switch.h
@@ -483,5 +483,6 @@ bool ice_is_vsi_valid(struct ice_hw *hw, u16 vsi_handle);
 
 enum ice_status ice_replay_vsi_all_fltr(struct ice_hw *hw, u16 vsi_handle);
 void ice_rm_all_sw_replay_rule_info(struct ice_hw *hw);
+bool ice_is_prof_rule(enum ice_sw_tunnel_type type);
 
 #endif /* _ICE_SWITCH_H_ */
-- 
2.19.1

Re: [dpdk-dev] [PATCH 0/3] update ice switch base code

[Zhang, Qi Z]

> -----Original Message-----
> From: Zhao1, Wei <wei.zhao1@intel.com>
> Sent: Friday, April 10, 2020 8:42 AM
> To: dev@dpdk.org
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Nannan <nannan.lu@intel.com>;
> Peng, Yuan <yuan.peng@intel.com>
> Subject: [PATCH 0/3] update ice switch base code
> 
> This patch set fix some ice base code bug of switch.
> 
> Wei Zhao (3):
>   net/ice/base: check the number of recipe when in chain
>   net/ice/base: add mask check when find switch recipe
>   net/ice/base: force switch to use different recipe for
> 
>  drivers/net/ice/base/ice_switch.c | 23 +++++++++++++++++------
> drivers/net/ice/base/ice_switch.h |  1 +
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> --
> 2.19.1

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

Re: [dpdk-dev] [PATCH 1/3] net/ice/base: check the number of recipe when in chain

[Lu, Nannan]

Tested-by: Lu, Nannan <nannan.lu@intel.com>

-----Original Message-----
From: Zhao1, Wei 
Sent: Friday, April 10, 2020 8:42 AM
To: dev@dpdk.org
Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Nannan <nannan.lu@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
Subject: [PATCH 1/3] net/ice/base: check the number of recipe when in chain

when we add some long switch rule, we need check the number of final recipe number, if it is large than ICE_MAX_CHAIN_RECIPE, we should refuse this rule.
For example:

"flow create 0 ingress pattern eth / ipv6 src is CDCD:910A:2222:5498:8475:1111:3900:1536
dst is CDCD:910A:2222:5498:8475:1111:3900:2022
tc is 3 / udp dst is 45 / end actions queue index 2 / end"

This rule will consum 6 recipe, if it is not refused, it will cause the following code over write of lkup_indx and mask.

LIST_FOR_EACH_ENTRY(entry, &rm->rg_list, ice_recp_grp_entry,
		l_entry) {
	last_chain_entry->fv_idx[i] = entry->chain_idx;
	buf[recps].content.lkup_indx[i] = entry->chain_idx;
	buf[recps].content.mask[i++] = CPU_TO_LE16(0xFFFF);
	..........
}

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

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index b5aa5abd9..c17219274 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -5352,6 +5352,9 @@ ice_add_sw_recipe(struct ice_hw *hw, struct ice_sw_recipe *rm,
 		rm->n_grp_count++;
 	}
 
+	if (rm->n_grp_count > ICE_MAX_CHAIN_RECIPE)
+		return ICE_ERR_MAX_LIMIT;
+
 	tmp = (struct ice_aqc_recipe_data_elem *)ice_calloc(hw,
 							    ICE_MAX_NUM_RECIPES,
 							    sizeof(*tmp));
--
2.19.1

Re: [dpdk-dev] [PATCH 3/3] net/ice/base: force switch to use different recipe for

[Peng, Yuan]

Test-by Peng, Yuan <yuan.peng@intel.com>

-----Original Message-----
From: Zhao1, Wei <wei.zhao1@intel.com> 
Sent: Friday, April 10, 2020 8:42 AM
To: dev@dpdk.org
Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Nannan <nannan.lu@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
Subject: [PATCH 3/3] net/ice/base: force switch to use different recipe for

When we use profile rule as swicth rule to download, if we download 2 different rules one by one, there will be rejection from function ice_aq_sw_rules(), for example:

"flow create 0 priority 0 ingress pattern eth / ipv6 / ah / end actions queue index 3 / end"
"flow create 0 priority 0 ingress pattern eth / ipv6 / esp / end actions queue index 2 / end"

That is because the 2 rules has the same s_rule input set except action queue index, so it will be rejected by hardware. So we have to use different recipes for them.

Also, we need to add recipe_id to keep record of recipe index, which will be used in rule remove, if not, there will be error when search recipe in function
ice_rem_adv_rule() if we create 2 or more profile rule.
For example:

"flow create 0 priority 0 ingress pattern eth / ipv4 / udp / pfcp s_field is 1 / end actions queue index 4 / end"
"flow create 0 priority 0 ingress pattern eth / ipv4 / udp / pfcp s_field is 0 / end actions queue index 5 / end"

then,

"flow flush 0"

you will find only the first rule will be delete, because ice_find_recp() will always return recipe id of the first rule.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/ice/base/ice_switch.c | 15 ++++++++++-----  drivers/net/ice/base/ice_switch.h |  1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index bc0c368d7..d8eec7f51 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -4983,7 +4983,8 @@ static const struct ice_protocol_entry ice_prot_id_tbl[ICE_PROTOCOL_LAST] = {
  *
  * Returns index of matching recipe, or ICE_MAX_NUM_RECIPES if not found.
  */
-static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
+static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts,
+			 enum ice_sw_tunnel_type tun_type)
 {
 	bool refresh_required = true;
 	struct ice_sw_recipe *recp;
@@ -5042,7 +5043,10 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 			/* If for "i"th recipe the found was never set to false
 			 * then it means we found our match
 			 */
-			if (found)
+			if (ice_is_prof_rule(tun_type) &&
+			    tun_type == recp[i].tun_type && found)
+				return i; /* Return the recipe ID */
+			else if (!ice_is_prof_rule(tun_type) && found)
 				return i; /* Return the recipe ID */
 		}
 	}
@@ -5798,7 +5802,7 @@ ice_get_compat_fv_bitmap(struct ice_hw *hw, struct ice_adv_rule_info *rinfo,
  * if the rule type is a profile rule, that means that there no field value
  * match required, in this case just a profile hit is required.
  */
-static bool ice_is_prof_rule(enum ice_sw_tunnel_type type)
+bool ice_is_prof_rule(enum ice_sw_tunnel_type type)
 {
 	switch (type) {
 	case ICE_SW_TUN_PROFID_IPV6_ESP:
@@ -5952,11 +5956,12 @@ ice_add_adv_recipe(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 		goto err_free_lkup_exts;
 
 	/* Look for a recipe which matches our requested fv / mask list */
-	*rid = ice_find_recp(hw, lkup_exts);
+	*rid = ice_find_recp(hw, lkup_exts, rinfo->tun_type);
 	if (*rid < ICE_MAX_NUM_RECIPES)
 		/* Success if found a recipe that match the existing criteria */
 		goto err_unroll;
 
+	rm->tun_type = rinfo->tun_type;
 	/* Recipe we need does not exist, add a recipe */
 	status = ice_add_sw_recipe(hw, rm, match_tun, profiles);
 	if (status)
@@ -6873,7 +6878,7 @@ ice_rem_adv_rule(struct ice_hw *hw, struct ice_adv_lkup_elem *lkups,
 	if (status)
 		return status;
 
-	rid = ice_find_recp(hw, &lkup_exts);
+	rid = ice_find_recp(hw, &lkup_exts, rinfo->tun_type);
 	/* If did not find a recipe that match the existing criteria */
 	if (rid == ICE_MAX_NUM_RECIPES)
 		return ICE_ERR_PARAM;
diff --git a/drivers/net/ice/base/ice_switch.h b/drivers/net/ice/base/ice_switch.h
index f7ae5c741..9666422f7 100644
--- a/drivers/net/ice/base/ice_switch.h
+++ b/drivers/net/ice/base/ice_switch.h
@@ -483,5 +483,6 @@ bool ice_is_vsi_valid(struct ice_hw *hw, u16 vsi_handle);
 
 enum ice_status ice_replay_vsi_all_fltr(struct ice_hw *hw, u16 vsi_handle);  void ice_rm_all_sw_replay_rule_info(struct ice_hw *hw);
+bool ice_is_prof_rule(enum ice_sw_tunnel_type type);
 
 #endif /* _ICE_SWITCH_H_ */
--
2.19.1

Re: [dpdk-dev] [PATCH 2/3] net/ice/base: add mask check when find switch recipe

[Lu, Nannan]

Tested-by: Lu, Nannan <nannan.lu@intel.com>

-----Original Message-----
From: Zhao1, Wei 
Sent: Friday, April 10, 2020 8:42 AM
To: dev@dpdk.org
Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Nannan <nannan.lu@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
Subject: [PATCH 2/3] net/ice/base: add mask check when find switch recipe

In order to find accurate recipe for switch filter, we need to add mask as an element when searching for recipe.
If we create different rules with the same input set, but using different masks, then proper recipes should use those different mask.

Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
 drivers/net/ice/base/ice_switch.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ice/base/ice_switch.c b/drivers/net/ice/base/ice_switch.c
index c17219274..bc0c368d7 100644
--- a/drivers/net/ice/base/ice_switch.c
+++ b/drivers/net/ice/base/ice_switch.c
@@ -5012,6 +5012,8 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 		if (lkup_exts->n_val_words == recp[i].lkup_exts.n_val_words) {
 			struct ice_fv_word *a = lkup_exts->fv_words;
 			struct ice_fv_word *b = recp[i].lkup_exts.fv_words;
+			u16 *c = recp[i].lkup_exts.field_mask;
+			u16 *d = lkup_exts->field_mask;
 			bool found = true;
 			u8 p, q;
 
@@ -5019,7 +5021,8 @@ static u16 ice_find_recp(struct ice_hw *hw, struct ice_prot_lkup_ext *lkup_exts)
 				for (q = 0; q < recp[i].lkup_exts.n_val_words;
 				     q++) {
 					if (a[p].off == b[q].off &&
-					    a[p].prot_id == b[q].prot_id)
+					    a[p].prot_id == b[q].prot_id &&
+					    d[p] == c[q])
 						/* Found the "p"th word in the
 						 * given recipe
 						 */
--
2.19.1

Re: [dpdk-dev] [PATCH 0/3] update ice switch base code

[Ye Xiaolong]

On 04/10, Zhang, Qi Z wrote:
>
>
>> -----Original Message-----
>> From: Zhao1, Wei <wei.zhao1@intel.com>
>> Sent: Friday, April 10, 2020 8:42 AM
>> To: dev@dpdk.org
>> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Nannan <nannan.lu@intel.com>;
>> Peng, Yuan <yuan.peng@intel.com>
>> Subject: [PATCH 0/3] update ice switch base code
>> 
>> This patch set fix some ice base code bug of switch.
>> 
>> Wei Zhao (3):
>>   net/ice/base: check the number of recipe when in chain
>>   net/ice/base: add mask check when find switch recipe
>>   net/ice/base: force switch to use different recipe for
>> 
>>  drivers/net/ice/base/ice_switch.c | 23 +++++++++++++++++------
>> drivers/net/ice/base/ice_switch.h |  1 +
>>  2 files changed, 18 insertions(+), 6 deletions(-)
>> 
>> --
>> 2.19.1
>
>Acked-by: Qi Zhang <qi.z.zhang@intel.com>
>

Applied to dpdk-next-net-intel, Thanks.