[PATCH v1 1/2] app/testpmd: fix modify tag typo

[PATCH v1 1/2] app/testpmd: fix modify tag typo

[Rongwei Liu]

Update the name to the right one: "src_tag_index"

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index c6ea9d9be0..929bf18119 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -7469,7 +7469,7 @@ static const struct token token_list[] = {
 		.comp = comp_none,
 	},
 	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
-		.name = "stc_tag_index",
+		.name = "src_tag_index",
 		.help = "source field tag array",
 		.next = NEXT(action_modify_field_src,
 			     NEXT_ENTRY(COMMON_UNSIGNED)),
-- 
2.27.0

Re: [PATCH v1 1/2] app/testpmd: fix modify tag typo

[Ferruh Yigit]

On 2/21/2024 7:10 AM, Rongwei Liu wrote:
> Update the name to the right one: "src_tag_index"
> 
> Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v1 1/2] app/testpmd: fix modify tag typo

[Ferruh Yigit]

On 2/21/2024 4:33 PM, Ferruh Yigit wrote:
> On 2/21/2024 7:10 AM, Rongwei Liu wrote:
>> Update the name to the right one: "src_tag_index"
>>
>> Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 

btw, fixes commit is wrong, it should be [1], I guess this is related to
the tree you are using as other patch has similar issue.


[1]
Fixes: c23626f27b09 ("ethdev: add MPLS header modification")

Re: [PATCH v1 1/2] app/testpmd: fix modify tag typo

[rongwei liu]

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

I am using the master-net-mlx branch as base to verify.
And cross-checked the main and master-net-mlx branch diverage now.
Patch 1 was good and patch 2 failed to apply on the main branch.

[rongweil@gen-l-vrt-290 mlnx_dpdk]$ patch -d ./ -p 1 < 1.patch
patching file app/test-pmd/cmdline_flow.c
Hunk #1 succeeded at 7283 (offset -186 lines).
[rongweil@gen-l-vrt-290 mlnx_dpdk]$ patch -d ./ -p 1 < 2.patch
patching file drivers/net/mlx5/mlx5_flow.c
Hunk #1 succeeded at 2384 with fuzz 1 (offset 15 lines).
patching file drivers/net/mlx5/mlx5_flow_hw.c
Hunk #1 FAILED at 6104.
Hunk #2 FAILED at 6129.
2 out of 2 hunks FAILED -- saving rejects to file drivers/net/mlx5/mlx5_flow_hw.c.rej

@Raslan Darawsheh<mailto:rasland@nvidia.com>Do I need to switch to main branch when sending patches to community?


BR

Rongwei



________________________________
From: Ferruh Yigit <ferruh.yigit@amd.com>
Sent: Thursday, February 22, 2024 0:42
To: rongwei liu <rongweil@nvidia.com>; dev@dpdk.org <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou <suanmingm@nvidia.com>; NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>
Cc: Michael Baum <michaelba@nvidia.com>; stable@dpdk.org <stable@dpdk.org>; Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
Subject: Re: [PATCH v1 1/2] app/testpmd: fix modify tag typo

External email: Use caution opening links or attachments


On 2/21/2024 4:33 PM, Ferruh Yigit wrote:
> On 2/21/2024 7:10 AM, Rongwei Liu wrote:
>> Update the name to the right one: "src_tag_index"
>>
>> Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
>> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>>
>
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
>

btw, fixes commit is wrong, it should be [1], I guess this is related to
the tree you are using as other patch has similar issue.


[1]
Fixes: c23626f27b09 ("ethdev: add MPLS header modification")

[-- Attachment #2: Type: text/html, Size: 6576 bytes --]

[PATCH v1 2/2] net/mlx5: fix modify flex item error

[Rongwei Liu]

In the rte_flow_field_data structure, the flex item handle is part
of union with other members like level/tag_index.

If the user wants to modify the flex item as source or destination,
there should not be any checking against zero.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 3c306a5f70..9caecc3db6 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2369,7 +2369,7 @@ int
 flow_validate_modify_field_level(const struct rte_flow_field_data *data,
 				 struct rte_flow_error *error)
 {
-	if (data->level == 0)
+	if (data->level == 0 || data->field == RTE_FLOW_FIELD_FLEX_ITEM)
 		return 0;
 	if (data->field != RTE_FLOW_FIELD_TAG &&
 	    data->field != (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index efbf88f56d..6dde19c1b9 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -6104,7 +6104,8 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev *dev,
 	ret = flow_hw_validate_modify_field_level(&action_conf->dst, false, error);
 	if (ret)
 		return ret;
-	if (!flow_hw_modify_field_is_geneve_opt(action_conf->dst.field)) {
+	if (action_conf->dst.field != RTE_FLOW_FIELD_FLEX_ITEM &&
+	    !flow_hw_modify_field_is_geneve_opt(action_conf->dst.field)) {
 		if (action_conf->dst.tag_index &&
 		    !flow_modify_field_support_tag_array(action_conf->dst.field))
 			return rte_flow_error_set(error, EINVAL,
@@ -6129,7 +6130,8 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev *dev,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (!flow_hw_modify_field_is_geneve_opt(action_conf->src.field)) {
+		if (action_conf->src.field != RTE_FLOW_FIELD_FLEX_ITEM &&
+		    !flow_hw_modify_field_is_geneve_opt(action_conf->src.field)) {
 			if (action_conf->src.tag_index &&
 			    !flow_modify_field_support_tag_array(action_conf->src.field))
 				return rte_flow_error_set(error, EINVAL,
-- 
2.27.0

Re: [PATCH v1 2/2] net/mlx5: fix modify flex item error

[Ferruh Yigit]

On 2/21/2024 7:10 AM, Rongwei Liu wrote:
> In the rte_flow_field_data structure, the flex item handle is part
> of union with other members like level/tag_index.
> 
> If the user wants to modify the flex item as source or destination,
> there should not be any checking against zero.
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Fixes: 7ea38955b ("ethdev: update MPLS header modification API")
> Cc: michaelba@nvidia.com
> Cc: stable@dpdk.org
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
>

Hi Rongwei,

I can't (also CI can't) apply this patch, on which tree it is based on?

[PATCH v2 1/2] app/testpmd: fix modify tag typo

[Rongwei Liu]

Update the name to the right one: "src_tag_index"

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Fixes: c23626f27b09 ("ethdev: add MPLS header modification")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index b4389e5150..f69516faf2 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -7316,7 +7316,7 @@ static const struct token token_list[] = {
 		.comp = comp_none,
 	},
 	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
-		.name = "stc_tag_index",
+		.name = "src_tag_index",
 		.help = "source field tag array",
 		.next = NEXT(action_modify_field_src,
 			     NEXT_ENTRY(COMMON_UNSIGNED)),
-- 
2.27.0

Re: [PATCH v2 1/2] app/testpmd: fix modify tag typo

[Ferruh Yigit]

On 2/23/2024 3:21 AM, Rongwei Liu wrote:
> Update the name to the right one: "src_tag_index"
> 
> Fixes: c23626f27b09 ("ethdev: add MPLS header modification")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

[PATCH v2 2/2] net/mlx5: fix modify flex item error

[Rongwei Liu]

In the rte_flow_field_data structure, the flex item handle is part
of union with other members like level/tag_index.

If the user wants to modify the flex item as source or destination,
there should not be any checking against zero.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Fixes: c23626f27b09 ("ethdev: add MPLS header modification")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 2b2ae62618..d8ed1ed6f6 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2384,7 +2384,7 @@ int
 flow_validate_modify_field_level(const struct rte_flow_field_data *data,
 				 struct rte_flow_error *error)
 {
-	if (data->level == 0)
+	if (data->level == 0 || data->field == RTE_FLOW_FIELD_FLEX_ITEM)
 		return 0;
 	if (data->field != RTE_FLOW_FIELD_TAG &&
 	    data->field != (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG)
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index bcf43f5457..5b269b9c82 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -5036,7 +5036,8 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev *dev,
 	ret = flow_validate_modify_field_level(&action_conf->dst, error);
 	if (ret)
 		return ret;
-	if (!flow_hw_modify_field_is_geneve_opt(action_conf->dst.field)) {
+	if (action_conf->dst.field != RTE_FLOW_FIELD_FLEX_ITEM &&
+	    !flow_hw_modify_field_is_geneve_opt(action_conf->dst.field)) {
 		if (action_conf->dst.tag_index &&
 		    !flow_modify_field_support_tag_array(action_conf->dst.field))
 			return rte_flow_error_set(error, EINVAL,
@@ -5061,7 +5062,8 @@ flow_hw_validate_action_modify_field(struct rte_eth_dev *dev,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (!flow_hw_modify_field_is_geneve_opt(action_conf->src.field)) {
+		if (action_conf->src.field != RTE_FLOW_FIELD_FLEX_ITEM &&
+		    !flow_hw_modify_field_is_geneve_opt(action_conf->src.field)) {
 			if (action_conf->src.tag_index &&
 			    !flow_modify_field_support_tag_array(action_conf->src.field))
 				return rte_flow_error_set(error, EINVAL,
-- 
2.27.0