[PATCH v1 0/3] support VXLAN rsvd1 modification

[PATCH v1 0/3] support VXLAN rsvd1 modification

[Rongwei Liu]

Support to modify VXLAN the last byte reserved field.

Rongwei Liu (3):
  ethdev: add VXLAN last reserved field
  app/testpmd: add VXLAN last reserved modification command
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  4 ++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 5 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v1 1/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_flow.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..b807713cec 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN reserved1. */
 };
 
 /**
-- 
2.27.0

[PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification command

[Rongwei Liu]

Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..9e88034709 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random", "vxlan_last_rsvd",
 	NULL
 };
 
-- 
2.27.0

[PATCH v1 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index dcc1beb812..afa35bf1e6 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -685,6 +685,11 @@ Limitations
     - Second tunnel fields are not supported.
 
   - Encapsulation levels greater than ``2`` are not supported.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only in HW steering (``dv_flow_en=2``).
+    - Support VNI and rsvd1 modifications for traffic with default VXLAN/VXLAN-GPE/VXLAN-GBP
+      UDP destination port.
 
 - Age action:
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f9c56af86c..7645369533 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 80efcf44fa..e2d41707c8 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1363,6 +1363,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_RSVD1) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5494,6 +5498,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

RE: [PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification command

[Dariusz Sosnowski]

> -----Original Message-----
> From: rongwei liu <rongweil@nvidia.com>
> Sent: Thursday, May 23, 2024 09:29
> To: 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: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> Subject: [PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification
> command
> 
> Add 'vxlan_last_rsvd' as the modification command string.
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

Could you please split this commit into 2?

1. Fix commit for reorganizing flow field IDs in testpmd (so that this fix gets backported to next LTS releases).
2. Commit adding "vxlan_last_rsvd".

Best regards,
Dariusz Sosnowski

[PATCH v2 0/4] support VXLAN rsvd1 modification

[Rongwei Liu]

Support to modify VXLAN the last byte reserved field.

v2: patch split.

Rongwei Liu (4):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  app/testpmd: add VXLAN last reserved modification command
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  4 ++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 5 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v2 1/4] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v2 2/4] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_flow.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..b807713cec 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN reserved1. */
 };
 
 /**
-- 
2.27.0

[PATCH v2 3/4] app/testpmd: add VXLAN last reserved modification command

[Rongwei Liu]

Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
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 775ce20fc4..9e88034709 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1005,7 +1005,7 @@ static const char *const flow_field_ids[] = {
 	"ipv4_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
-	"random",
+	"random", "vxlan_last_rsvd",
 	NULL
 };
 
-- 
2.27.0

[PATCH v2 4/4] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index dcc1beb812..afa35bf1e6 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -685,6 +685,11 @@ Limitations
     - Second tunnel fields are not supported.
 
   - Encapsulation levels greater than ``2`` are not supported.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only in HW steering (``dv_flow_en=2``).
+    - Support VNI and rsvd1 modifications for traffic with default VXLAN/VXLAN-GPE/VXLAN-GBP
+      UDP destination port.
 
 - Age action:
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f9c56af86c..7645369533 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 80efcf44fa..e2d41707c8 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1363,6 +1363,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_RSVD1) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5494,6 +5498,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification command

[rongwei liu]

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

Hi


BR

Rongwei



________________________________
From: Dariusz Sosnowski <dsosnowski@nvidia.com>
Sent: Friday, May 24, 2024 23:53
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: Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
Subject: RE: [PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification command

> -----Original Message-----
> From: rongwei liu <rongweil@nvidia.com>
> Sent: Thursday, May 23, 2024 09:29
> To: 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: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>
> Subject: [PATCH v1 2/3] app/testpmd: add VXLAN last reserved modification
> command
>
> Add 'vxlan_last_rsvd' as the modification command string.
>
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

Could you please split this commit into 2?

1. Fix commit for reorganizing flow field IDs in testpmd (so that this fix gets backported to next LTS releases).
split is done. 22.11 and 23.11 don't have so many fields, IMO, Backport to LTS is not needed.
2. Commit adding "vxlan_last_rsvd".
Already have.

Best regards,
Dariusz Sosnowski

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

Re: [PATCH v2 1/4] app/testpmd: fix the testpmd field string sequence

[rongwei liu]

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

Hi
@Dariusz Sosnowski<mailto:dsosnowski@nvidia.com> Can you comment on this patch? Thanks.


BR

Rongwei



________________________________
From: Rongwei Liu <rongweil@nvidia.com>
Sent: Monday, May 27, 2024 10:47
To: 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>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>
Subject: [PATCH v2 1/4] app/testpmd: fix the testpmd field string sequence

External email: Use caution opening links or attachments


The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
        "hash_result",
        "geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
        "tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-       "random",
        "ipv4_proto",
-       "esp_spi", "esp_seq_num", "esp_proto",
        "ipv6_flow_label", "ipv6_traffic_class",
+       "esp_spi", "esp_seq_num", "esp_proto",
+       "random",
        NULL
 };

--
2.27.0


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

RE: [PATCH v2 1/4] app/testpmd: fix the testpmd field string sequence

[Dariusz Sosnowski]

> -----Original Message-----
> From: rongwei liu <rongweil@nvidia.com>
> Sent: Monday, May 27, 2024 04:47
> To: 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; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Ferruh Yigit <ferruh.yigit@amd.com>; Dariusz Sosnowski
> <dsosnowski@nvidia.com>
> Subject: [PATCH v2 1/4] app/testpmd: fix the testpmd field string sequence
> 
> The field string should be in the same order as the rte_flow_field_id
> enumration definitions
> 
> Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
> Cc: michaelba@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>

Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

Best regards,
Dariusz Sosnowski

Re: [PATCH v2 2/4] ethdev: add VXLAN last reserved field

[rongwei liu]

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

Hi @Ori Kam<mailto:orika@nvidia.com>@Andrew Rybchenko<mailto:andrew.rybchenko@oktetlabs.ru>@NBU-Contact-Thomas Monjalon (EXTERNAL)<mailto:thomas@monjalon.net>@Ferruh Yigit<mailto:ferruh.yigit@amd.com>
      Can you help ACK this patch?
      Thanks


BR

Rongwei



________________________________
From: Rongwei Liu <rongweil@nvidia.com>
Sent: Monday, May 27, 2024 10:47
To: 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: Dariusz Sosnowski <dsosnowski@nvidia.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: [PATCH v2 2/4] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


Add VXLAN last reserved byte in rte flow packet field.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 lib/ethdev/rte_flow.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..b807713cec 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
        RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
        RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
        RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+       RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN reserved1. */
 };

 /**
--
2.27.0


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

Re: [PATCH v2 2/4] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

27/05/2024 04:47, Rongwei Liu:
> Add VXLAN last reserved byte in rte flow packet field.

You should squash addition in testpmd in the same patch.
Adding a field in rte_flow.h should be atomic with testpmd update.

> @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
>  	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
>  	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
>  	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> +	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN reserved1. */

It isn't clear what reserved1 is.
You should say it is the last byte of the VXLAN header.

Re: [PATCH v2 3/4] app/testpmd: add VXLAN last reserved modification command

[Thomas Monjalon]

27/05/2024 04:47, Rongwei Liu:
> @@ -1005,7 +1005,7 @@ static const char *const flow_field_ids[] = {
>  	"ipv4_proto",
>  	"ipv6_flow_label", "ipv6_traffic_class",
>  	"esp_spi", "esp_seq_num", "esp_proto",
> -	"random",
> +	"random", "vxlan_last_rsvd",

it is not related to "random", so a new line looks better

Re: [PATCH v2 2/4] ethdev: add VXLAN last reserved field

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Tuesday, June 4, 2024 19:59
To: rongwei liu <rongweil@nvidia.com>
Cc: 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>; Dariusz Sosnowski <dsosnowski@nvidia.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v2 2/4] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


27/05/2024 04:47, Rongwei Liu:
> Add VXLAN last reserved byte in rte flow packet field.

You should squash addition in testpmd in the same patch.
Adding a field in rte_flow.h should be atomic with testpmd update.

Sure. Squash into one commit.

> @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
>       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
>       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
>       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN reserved1. */

It isn't clear what reserved1 is.
You should say it is the last byte of the VXLAN header.

ACK.



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

[PATCH v3 0/3] support VXLAN rsvd1 modification

[Rongwei Liu]

Support to modify VXLAN the last byte reserved field.

v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (3):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  5 +++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 5 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v3 1/3] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v3 2/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.
Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..f3d6a2cfd7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..5a1dbfc084 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
 };
 
 /**
-- 
2.27.0

[PATCH v3 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index b5928d40b2..2cedb3696b 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -685,6 +685,11 @@ Limitations
     - Second tunnel fields are not supported.
 
   - Encapsulation levels greater than ``2`` are not supported.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only in HW steering (``dv_flow_en=2``).
+    - Support VNI and rsvd1 modifications for traffic with default VXLAN/VXLAN-GPE/VXLAN-GBP
+      UDP destination port.
 
 - Age action:
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6f72185916..48554ab8d1 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 27e3ec550e..2feff05c24 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1371,6 +1371,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_RSVD1) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5515,6 +5519,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v2 4/4] net/mlx5: implement VXLAN last reserved modification

[Thomas Monjalon]

27/05/2024 04:47, Rongwei Liu:
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -685,6 +685,11 @@ Limitations
>      - Second tunnel fields are not supported.
>  
>    - Encapsulation levels greater than ``2`` are not supported.
> +  - Modification of the VXLAN header is supported with below limitations:
> +
> +    - Only in HW steering (``dv_flow_en=2``).
> +    - Support VNI and rsvd1 modifications for traffic with default VXLAN/VXLAN-GPE/VXLAN-GBP
> +      UDP destination port.

Don't add at the end of bullet lists without trying to order.
It is limitation for a specific protocol, it should go above with other protocols,
between "Modification of VXLAN network" and GENEVE limitations.

"rsvd1" should be replaced with "last reserved byte".

What do you mean with "default VXLAN/VXLAN-GPE/VXLAN-GBP UDP destination port"?
Is it port 4789 for VXLAN and VXLAN-GBP, and port 4790 for VXLAN-GPE?
We need to be more specific probably.
  

Re: [PATCH v2 4/4] net/mlx5: implement VXLAN last reserved modification

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Tuesday, June 4, 2024 20:19
To: rongwei liu <rongweil@nvidia.com>
Cc: 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>; Dariusz Sosnowski <dsosnowski@nvidia.com>
Subject: Re: [PATCH v2 4/4] net/mlx5: implement VXLAN last reserved modification

External email: Use caution opening links or attachments


27/05/2024 04:47, Rongwei Liu:
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -685,6 +685,11 @@ Limitations
>      - Second tunnel fields are not supported.
>
>    - Encapsulation levels greater than ``2`` are not supported.
> +  - Modification of the VXLAN header is supported with below limitations:
> +
> +    - Only in HW steering (``dv_flow_en=2``).
> +    - Support VNI and rsvd1 modifications for traffic with default VXLAN/VXLAN-GPE/VXLAN-GBP
> +      UDP destination port.

Don't add at the end of bullet lists without trying to order.
It is limitation for a specific protocol, it should go above with other protocols,
between "Modification of VXLAN network" and GENEVE limitations.

"rsvd1" should be replaced with "last reserved byte".
Ok.

What do you mean with "default VXLAN/VXLAN-GPE/VXLAN-GBP UDP destination port"?
Is it port 4789 for VXLAN and VXLAN-GBP, and port 4790 for VXLAN-GPE?
We need to be more specific probably.

Will change to the exact number.




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

[PATCH v4 0/3] support VXLAN rsvd1 modification

[Rongwei Liu]

Support to modify VXLAN the last byte reserved field.

v4: specify the limitation with exact port number.
v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (3):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  5 +++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 5 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v4 1/3] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.
Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..f3d6a2cfd7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..5a1dbfc084 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
 };
 
 /**
-- 
2.27.0

[PATCH v4 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index b5928d40b2..8b3aaf5aac 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -637,6 +637,11 @@ Limitations
   - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported.
   - Modification of the 802.1Q tag is not supported.
   - Modification of VXLAN network or GENEVE network ID is supported only for HW steering.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only for HW steering (``dv_flow_en=2``).
+    - Support VNI and the last reserved byte modifications for traffic with default
+      UDP destination port. 4789 for VXLAN and VXLAN-GBP, 4790 for VXLAN-GPE.
   - Modification of GENEVE network ID is not supported when configured
     ``FLEX_PARSER_PROFILE_ENABLE`` supports Geneve TLV options.
     See :ref:`mlx5_firmware_config` for more flex parser information.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6f72185916..48554ab8d1 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 27e3ec550e..2feff05c24 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1371,6 +1371,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_RSVD1) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5515,6 +5519,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_RSVD1:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

04/06/2024 14:38, Rongwei Liu:
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
>  	"ipv6_flow_label", "ipv6_traffic_class",
>  	"esp_spi", "esp_seq_num", "esp_proto",
>  	"random",
> +	"vxlan_last_rsvd",
>  	NULL
>  };

How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in testpmd?
Just because it is the same order?


> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
>  	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
>  	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
>  	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> +	RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
>  };

I think we should use the same naming as in testpmd.
What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Tuesday, June 4, 2024 23:03
To: rongwei liu <rongweil@nvidia.com>
Cc: 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>; Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


04/06/2024 14:38, Rongwei Liu:
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
>       "ipv6_flow_label", "ipv6_traffic_class",
>       "esp_spi", "esp_seq_num", "esp_proto",
>       "random",
> +     "vxlan_last_rsvd",
>       NULL
>  };

How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in testpmd?
Just because it is the same order?

Consider the on-the-way VxLAN extension changes, the last reserved byte was naming as "uint8_t rsvd1".

> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
>       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
>       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
>       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
>  };

I think we should use the same naming as in testpmd.
What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?

To be honest, no strong objection per my personal thought.  Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1", maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1 mapping?



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

RE: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Dariusz Sosnowski]

> > 04/06/2024 14:38, Rongwei Liu:
> > > --- a/app/test-pmd/cmdline_flow.c
> > > +++ b/app/test-pmd/cmdline_flow.c
> > > @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
> > >       "ipv6_flow_label", "ipv6_traffic_class",
> > >       "esp_spi", "esp_seq_num", "esp_proto",
> > >       "random",
> > > +     "vxlan_last_rsvd",
> > >       NULL
> > >  };
> >
> > How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in testpmd?
> > Just because it is the same order?

Yes, it's because of the order.
We should refactor this to use array designators.

> > > --- a/lib/ethdev/rte_flow.h
> > > +++ b/lib/ethdev/rte_flow.h
> > > @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
> > >       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
> > >       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
> > >       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> > > +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
> > >  };
> >
> > I think we should use the same naming as in testpmd.
> > What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?
> To be honest, no strong objection per my personal thought.  Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1", maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1 mapping?

+1 on using RSVD1 so it matches rte_vxlan_hdr definition.

In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches existing "last_rsvd" field in VXLAN item.
If we choose to use "rsvd1", we should probably rename all other instances of "last_rsvd" to match.

Best regards,
Dariusz Sosnowski

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

04/06/2024 18:40, Dariusz Sosnowski:
> > > 04/06/2024 14:38, Rongwei Liu:
> > > > --- a/app/test-pmd/cmdline_flow.c
> > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
> > > >       "ipv6_flow_label", "ipv6_traffic_class",
> > > >       "esp_spi", "esp_seq_num", "esp_proto",
> > > >       "random",
> > > > +     "vxlan_last_rsvd",
> > > >       NULL
> > > >  };
> > >
> > > How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in testpmd?
> > > Just because it is the same order?
> 
> Yes, it's because of the order.
> We should refactor this to use array designators.
> 
> > > > --- a/lib/ethdev/rte_flow.h
> > > > +++ b/lib/ethdev/rte_flow.h
> > > > @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
> > > >       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
> > > >       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
> > > >       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> > > > +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
> > > >  };
> > >
> > > I think we should use the same naming as in testpmd.
> > > What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?
> > To be honest, no strong objection per my personal thought.  Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1", maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1 mapping?
> 
> +1 on using RSVD1 so it matches rte_vxlan_hdr definition.
> 
> In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches existing "last_rsvd" field in VXLAN item.
> If we choose to use "rsvd1", we should probably rename all other instances of "last_rsvd" to match.

I prefer "vxlan_last_rsvd" for 2 reasons:
	- it is more meaningful
	- we are adding first, second and third reserved fields to match the 3 bytes of rsvd0 (patch to come)

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Wednesday, June 5, 2024 2:37
To: rongwei liu <rongweil@nvidia.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>
Cc: 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>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


04/06/2024 18:40, Dariusz Sosnowski:
> > > 04/06/2024 14:38, Rongwei Liu:
> > > > --- a/app/test-pmd/cmdline_flow.c
> > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
> > > >       "ipv6_flow_label", "ipv6_traffic_class",
> > > >       "esp_spi", "esp_seq_num", "esp_proto",
> > > >       "random",
> > > > +     "vxlan_last_rsvd",
> > > >       NULL
> > > >  };
> > >
> > > How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in testpmd?
> > > Just because it is the same order?
>
> Yes, it's because of the order.
> We should refactor this to use array designators.
>
> > > > --- a/lib/ethdev/rte_flow.h
> > > > +++ b/lib/ethdev/rte_flow.h
> > > > @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
> > > >       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
> > > >       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
> > > >       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> > > > +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved byte. */
> > > >  };
> > >
> > > I think we should use the same naming as in testpmd.
> > > What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?
> > To be honest, no strong objection per my personal thought.  Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1", maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1 mapping?
>
> +1 on using RSVD1 so it matches rte_vxlan_hdr definition.
>
> In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches existing "last_rsvd" field in VXLAN item.
> If we choose to use "rsvd1", we should probably rename all other instances of "last_rsvd" to match.

I prefer "vxlan_last_rsvd" for 2 reasons:
        - it is more meaningful
        - we are adding first, second and third reserved fields to match the 3 bytes of rsvd0 (patch to come)

Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:

  1.
RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
  2.
"uint8_t rsvd1"  ---->  "uint8_t last_rsvd"




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

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

05/06/2024 03:16, rongwei liu:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 04/06/2024 18:40, Dariusz Sosnowski:
> > > > > 04/06/2024 14:38, Rongwei Liu:
> > > > > > --- a/app/test-pmd/cmdline_flow.c
> > > > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > > > @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] =
> > > > > > {
> > > > > > 
> > > > > >       "ipv6_flow_label", "ipv6_traffic_class",
> > > > > >       "esp_spi", "esp_seq_num", "esp_proto",
> > > > > >       "random",
> > > > > > 
> > > > > > +     "vxlan_last_rsvd",
> > > > > > 
> > > > > >       NULL
> > > > > >  
> > > > > >  };
> > > > > 
> > > > > How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in
> > > > > testpmd?
> > > > > Just because it is the same order?
> > > 
> > > Yes, it's because of the order.
> > > We should refactor this to use array designators.
> > > 
> > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
> > > > > > 
> > > > > >       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
> > > > > >       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol
> > > > > >       value. */
> > > > > >       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> > > > > > 
> > > > > > +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved
> > > > > > byte. */
> > > > > > 
> > > > > >  };
> > > > > 
> > > > > I think we should use the same naming as in testpmd.
> > > > > What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?
> > > > 
> > > > To be honest, no strong objection per my personal thought. 
> > > > Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1",
> > > > maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1
> > > > mapping?> > > 
> > > +1 on using RSVD1 so it matches rte_vxlan_hdr definition.
> > > 
> > > In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches
> > > existing "last_rsvd" field in VXLAN item. If we choose to use "rsvd1",
> > > we should probably rename all other instances of "last_rsvd" to match.> > 
> > I prefer "vxlan_last_rsvd" for 2 reasons:
> >         - it is more meaningful
> >         - we are adding first, second and third reserved fields to match
> >         the 3 bytes of rsvd0 (patch to come)
> 
> Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:
> 
>   1.
> RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
>   2.
> "uint8_t rsvd1"  ---->  "uint8_t last_rsvd"

We don't change rte_vxlan_hdr, because we avoid breaking compatibility.

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Wednesday, June 5, 2024 15:13
To: Dariusz Sosnowski <dsosnowski@nvidia.com>; rongwei liu <rongweil@nvidia.com>
Cc: 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>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


05/06/2024 03:16, rongwei liu:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 04/06/2024 18:40, Dariusz Sosnowski:
> > > > > 04/06/2024 14:38, Rongwei Liu:
> > > > > > --- a/app/test-pmd/cmdline_flow.c
> > > > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > > > @@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] =
> > > > > > {
> > > > > >
> > > > > >       "ipv6_flow_label", "ipv6_traffic_class",
> > > > > >       "esp_spi", "esp_seq_num", "esp_proto",
> > > > > >       "random",
> > > > > >
> > > > > > +     "vxlan_last_rsvd",
> > > > > >
> > > > > >       NULL
> > > > > >
> > > > > >  };
> > > > >
> > > > > How vxlan_last_rsvd is linked to RTE_FLOW_FIELD_VXLAN_RSVD1 in
> > > > > testpmd?
> > > > > Just because it is the same order?
> > >
> > > Yes, it's because of the order.
> > > We should refactor this to use array designators.
> > >
> > > > > > --- a/lib/ethdev/rte_flow.h
> > > > > > +++ b/lib/ethdev/rte_flow.h
> > > > > > @@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
> > > > > >
> > > > > >       RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
> > > > > >       RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol
> > > > > >       value. */
> > > > > >       RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
> > > > > >
> > > > > > +     RTE_FLOW_FIELD_VXLAN_RSVD1,     /**< VXLAN last reserved
> > > > > > byte. */
> > > > > >
> > > > > >  };
> > > > >
> > > > > I think we should use the same naming as in testpmd.
> > > > > What about RTE_FLOW_FIELD_VXLAN_LAST_RSVD?
> > > >
> > > > To be honest, no strong objection per my personal thought.
> > > > Considering the API "vxlan_hdr" names this field as "uint8_t rsvd1",
> > > > maybe RTE_FLOW_FIELD_VXLAN_RSVD1 will be clearer for user as 1 vs 1
> > > > mapping?> > >
> > > +1 on using RSVD1 so it matches rte_vxlan_hdr definition.
> > >
> > > In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches
> > > existing "last_rsvd" field in VXLAN item. If we choose to use "rsvd1",
> > > we should probably rename all other instances of "last_rsvd" to match.> >
> > I prefer "vxlan_last_rsvd" for 2 reasons:
> >         - it is more meaningful
> >         - we are adding first, second and third reserved fields to match
> >         the 3 bytes of rsvd0 (patch to come)
>
> Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:
>
>   1.
> RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
>   2.
> "uint8_t rsvd1"  ---->  "uint8_t last_rsvd"

We don't change rte_vxlan_hdr, because we avoid breaking compatibility.

How about to add a new union:

union {
    uint8_t rsvd1;
    uint8_t last_rsvd;
}
 RTE_FLOW_FIELD_VXLAN_LAST_RSVD will perfectly match the rte_vxlan_hdr definition.

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

[PATCH v5 0/3] support VXLAN last reserved byte modification

[Rongwei Liu]

Support to modify VXLAN last reserved byte.

v5: change the field enumeration naming.
v4: specify the limitation with exact port number.
v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (3):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  5 +++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 5 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.27.0

[PATCH v5 1/3] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v5 2/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.
Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..f3d6a2cfd7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..74cf2e0f59 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_LAST_RSVD, /**< VXLAN last reserved byte. */
 };
 
 /**
-- 
2.27.0

[PATCH v5 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index b5928d40b2..8b3aaf5aac 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -637,6 +637,11 @@ Limitations
   - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported.
   - Modification of the 802.1Q tag is not supported.
   - Modification of VXLAN network or GENEVE network ID is supported only for HW steering.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only for HW steering (``dv_flow_en=2``).
+    - Support VNI and the last reserved byte modifications for traffic with default
+      UDP destination port. 4789 for VXLAN and VXLAN-GBP, 4790 for VXLAN-GPE.
   - Modification of GENEVE network ID is not supported when configured
     ``FLEX_PARSER_PROFILE_ENABLE`` supports Geneve TLV options.
     See :ref:`mlx5_firmware_config` for more flex parser information.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 06f5427abf..99abb2f2fb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 63194935a3..01362ae121 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1385,6 +1385,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_LAST_RSVD) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5580,6 +5584,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

05/06/2024 10:14, rongwei liu:
> > > > > In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches
> > > > > existing "last_rsvd" field in VXLAN item. If we choose to use
> > > > > "rsvd1",
> > > > > we should probably rename all other instances of "last_rsvd" to
> > > > > match.> >
> > > > 
> > > > I prefer "vxlan_last_rsvd" for 2 reasons:
> > > >         - it is more meaningful
> > > >         - we are adding first, second and third reserved fields to
> > > >         match
> > > >         the 3 bytes of rsvd0 (patch to come)
> > > 
> > > Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:
> > >   1.
> > > 
> > > RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
> > > 
> > >   2.
> > > 
> > > "uint8_t rsvd1"  ---->  "uint8_t last_rsvd"
> > 
> > We don't change rte_vxlan_hdr, because we avoid breaking compatibility.
> 
> How about to add a new union:
> 
> union {
>     uint8_t rsvd1;
>     uint8_t last_rsvd;
> }
>  RTE_FLOW_FIELD_VXLAN_LAST_RSVD will perfectly match the rte_vxlan_hdr definition.

It could be a solution, yes,
but I don't see it in your v5.

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

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



BR

Rongwei



________________________________
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Wednesday, June 5, 2024 17:35
To: rongwei liu <rongweil@nvidia.com>
Cc: Dariusz Sosnowski <dsosnowski@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>; Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Subject: Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

External email: Use caution opening links or attachments


05/06/2024 10:14, rongwei liu:
> > > > > In this patch, "vxlan_last_rsvd" is used in testpmd, so it matches
> > > > > existing "last_rsvd" field in VXLAN item. If we choose to use
> > > > > "rsvd1",
> > > > > we should probably rename all other instances of "last_rsvd" to
> > > > > match.> >
> > > >
> > > > I prefer "vxlan_last_rsvd" for 2 reasons:
> > > >         - it is more meaningful
> > > >         - we are adding first, second and third reserved fields to
> > > >         match
> > > >         the 3 bytes of rsvd0 (patch to come)
> > >
> > > Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:
> > >   1.
> > >
> > > RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
> > >
> > >   2.
> > >
> > > "uint8_t rsvd1"  ---->  "uint8_t last_rsvd"
> >
> > We don't change rte_vxlan_hdr, because we avoid breaking compatibility.
>
> How about to add a new union:
>
> union {
>     uint8_t rsvd1;
>     uint8_t last_rsvd;
> }
>  RTE_FLOW_FIELD_VXLAN_LAST_RSVD will perfectly match the rte_vxlan_hdr definition.

It could be a solution, yes,
but I don't see it in your v5.

Should I add this change in my serial or expect it in the upcoming new VXLAN header definitions?





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

Re: [PATCH v4 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

05/06/2024 11:41, rongwei liu:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 05/06/2024 10:14, rongwei liu:
> > > > > > > In this patch, "vxlan_last_rsvd" is used in testpmd, so it
> > > > > > > matches
> > > > > > > existing "last_rsvd" field in VXLAN item. If we choose to use
> > > > > > > "rsvd1",
> > > > > > > we should probably rename all other instances of "last_rsvd" to
> > > > > > > match.> >
> > > > > > 
> > > > > > I prefer "vxlan_last_rsvd" for 2 reasons:
> > > > > >         - it is more meaningful
> > > > > >         - we are adding first, second and third reserved fields to
> > > > > >         match
> > > > > >         the 3 bytes of rsvd0 (patch to come)
> > > > > 
> > > > > Sound clear and reasonable. I would like to propose the alignment between rte_flow_field_id and rte_vxlan_hdr:
> > > > >   1.
> > > > > 
> > > > > RTE_FLOW_FIELD_VXLAN_RSVD1  ---> RTE_FLOW_FIELD_VXLAN_LAST_RSVD
> > > > > 
> > > > >   2.
> > > > > 
> > > > > "uint8_t rsvd1"  ---->  "uint8_t last_rsvd"
> > > > 
> > > > We don't change rte_vxlan_hdr, because we avoid breaking
> > > > compatibility.
> > > 
> > > How about to add a new union:
> > > 
> > > union {
> > > 
> > >     uint8_t rsvd1;
> > >     uint8_t last_rsvd;
> > > 
> > > }
> > > 
> > >  RTE_FLOW_FIELD_VXLAN_LAST_RSVD will perfectly match the rte_vxlan_hdr
> > >  definition.> > 
> > It could be a solution, yes,
> > but I don't see it in your v5.
> 
> Should I add this change in my serial or expect it in the upcoming new VXLAN header definitions?

It fits better in your series.

[PATCH v6 0/4] support VXLAN last reserved byte modification

[Rongwei Liu]

Support to modify VXLAN last reserved byte.

v6: add union into vxlan header.
v5: change the field enumeration naming.
v4: specify the limitation with exact port number.
v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (4):
  app/testpmd: fix the testpmd field string sequence
  net: add new union into VXLAN header
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  5 +++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 lib/net/rte_vxlan.h             |  5 ++++-
 6 files changed, 30 insertions(+), 3 deletions(-)

-- 
2.27.0

[PATCH v6 1/4] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v6 2/4] net: add new union into VXLAN header

[Rongwei Liu]

Add "uint8_t last_rsvd" as union with origin rsvd1.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
---
 lib/net/rte_vxlan.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
index 997fc784fc..57300fb442 100644
--- a/lib/net/rte_vxlan.h
+++ b/lib/net/rte_vxlan.h
@@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
 			uint8_t    flags;    /**< Should be 8 (I flag). */
 			uint8_t    rsvd0[3]; /**< Reserved. */
 			uint8_t    vni[3];   /**< VXLAN identifier. */
-			uint8_t    rsvd1;    /**< Reserved. */
+			union {
+				uint8_t    rsvd1;        /**< Reserved. */
+				uint8_t    last_rsvd;    /**< Reserved. */
+			};
 		};
 	};
 } __rte_packed;
-- 
2.27.0

[PATCH v6 3/4] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add VXLAN last reserved byte in rte flow packet field.
Add 'vxlan_last_rsvd' as the modification command string.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..f3d6a2cfd7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..74cf2e0f59 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_LAST_RSVD, /**< VXLAN last reserved byte. */
 };
 
 /**
-- 
2.27.0

[PATCH v6 4/4] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index b5928d40b2..8b3aaf5aac 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -637,6 +637,11 @@ Limitations
   - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported.
   - Modification of the 802.1Q tag is not supported.
   - Modification of VXLAN network or GENEVE network ID is supported only for HW steering.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only for HW steering (``dv_flow_en=2``).
+    - Support VNI and the last reserved byte modifications for traffic with default
+      UDP destination port. 4789 for VXLAN and VXLAN-GBP, 4790 for VXLAN-GPE.
   - Modification of GENEVE network ID is not supported when configured
     ``FLEX_PARSER_PROFILE_ENABLE`` supports Geneve TLV options.
     See :ref:`mlx5_firmware_config` for more flex parser information.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 06f5427abf..99abb2f2fb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 63194935a3..01362ae121 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1385,6 +1385,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_LAST_RSVD) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5580,6 +5584,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v6 2/4] net: add new union into VXLAN header

[Thomas Monjalon]

05/06/2024 12:21, Rongwei Liu:
> Add "uint8_t last_rsvd" as union with origin rsvd1.

You should explain why: for matching what we have in testpmd.
I think it should be squashed with the next patch.

> -			uint8_t    rsvd1;    /**< Reserved. */
> +			union {
> +				uint8_t    rsvd1;        /**< Reserved. */
> +				uint8_t    last_rsvd;    /**< Reserved. */
> +			};

RE: [PATCH v6 2/4] net: add new union into VXLAN header

[rongwei liu]

BR
Rongwei

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, June 5, 2024 18:42
> To: rongwei liu <rongweil@nvidia.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>
> Subject: Re: [PATCH v6 2/4] net: add new union into VXLAN header
> 
> External email: Use caution opening links or attachments
> 
> 
> 05/06/2024 12:21, Rongwei Liu:
> > Add "uint8_t last_rsvd" as union with origin rsvd1.
> 
> You should explain why: for matching what we have in testpmd.
> I think it should be squashed with the next patch.
Sure. Let' squash them, still with "ethdev" prefix?
The explanation should in the commit log not in the header file. What do you think?
> 
> > -                     uint8_t    rsvd1;    /**< Reserved. */
> > +                     union {
> > +                             uint8_t    rsvd1;        /**< Reserved. */
> > +                             uint8_t    last_rsvd;    /**< Reserved. */
> > +                     };
> 
> 

Re: [PATCH v6 2/4] net: add new union into VXLAN header

[Thomas Monjalon]

05/06/2024 13:28, rongwei liu:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 05/06/2024 12:21, Rongwei Liu:
> > > Add "uint8_t last_rsvd" as union with origin rsvd1.
> > 
> > You should explain why: for matching what we have in testpmd.
> > I think it should be squashed with the next patch.
> 
> Sure. Let' squash them, still with "ethdev" prefix?

Yes, the main change is adding new field item in ethdev.

> The explanation should in the commit log not in the header file. What do you think?

Yes, explanation in the commit log.

[PATCH v7 0/3] support VXLAN last reserved byte modification

[Rongwei Liu]

v7: squash, add more commit logs.
v6: add union into vxlan header.
v5: change the field enumeration naming.
v4: specify the limitation with exact port number.
v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (3):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  5 +++--
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 lib/net/rte_vxlan.h             |  5 ++++-
 6 files changed, 30 insertions(+), 3 deletions(-)

-- 
2.27.0

[PATCH v7 1/3] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v7 2/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add "uint8_t last_rsvd" as union with origin rsvd1.
Add RTE_FLOW_FIELD_VXLAN_LAST_RSVD into rte flow packet
field.

The new union is used by testpmd matching item VXLAN
"last_rsvd" and modify target "vxlan_last_rsvd".

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 1 +
 lib/ethdev/rte_flow.h       | 1 +
 lib/net/rte_vxlan.h         | 5 ++++-
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..f3d6a2cfd7 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..74cf2e0f59 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_LAST_RSVD, /**< VXLAN last reserved byte. */
 };
 
 /**
diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
index 997fc784fc..57300fb442 100644
--- a/lib/net/rte_vxlan.h
+++ b/lib/net/rte_vxlan.h
@@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
 			uint8_t    flags;    /**< Should be 8 (I flag). */
 			uint8_t    rsvd0[3]; /**< Reserved. */
 			uint8_t    vni[3];   /**< VXLAN identifier. */
-			uint8_t    rsvd1;    /**< Reserved. */
+			union {
+				uint8_t    rsvd1;        /**< Reserved. */
+				uint8_t    last_rsvd;    /**< Reserved. */
+			};
 		};
 	};
 } __rte_packed;
-- 
2.27.0

[PATCH v7 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index b5928d40b2..8b3aaf5aac 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -637,6 +637,11 @@ Limitations
   - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported.
   - Modification of the 802.1Q tag is not supported.
   - Modification of VXLAN network or GENEVE network ID is supported only for HW steering.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only for HW steering (``dv_flow_en=2``).
+    - Support VNI and the last reserved byte modifications for traffic with default
+      UDP destination port. 4789 for VXLAN and VXLAN-GBP, 4790 for VXLAN-GPE.
   - Modification of GENEVE network ID is not supported when configured
     ``FLEX_PARSER_PROFILE_ENABLE`` supports Geneve TLV options.
     See :ref:`mlx5_firmware_config` for more flex parser information.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 06f5427abf..99abb2f2fb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 63194935a3..01362ae121 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1385,6 +1385,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_LAST_RSVD) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5580,6 +5584,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v7 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

05/06/2024 15:04, Rongwei Liu:
> Add "uint8_t last_rsvd" as union with origin rsvd1.
> Add RTE_FLOW_FIELD_VXLAN_LAST_RSVD into rte flow packet
> field.
> 
> The new union is used by testpmd matching item VXLAN
> "last_rsvd"

no it is still using the name "rsvd1" in testpmd

> and modify target "vxlan_last_rsvd".

You should give the real name: RTE_FLOW_FIELD_VXLAN_LAST_RSVD

With above changes,
Acked-by: Thomas Monjalon <thomas@monjalon.net>

> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>

RE: [PATCH v7 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

BR
Rongwei

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, June 7, 2024 21:26
> To: rongwei liu <rongweil@nvidia.com>
> Cc: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Ori Kam <orika@nvidia.com>; Suanming Mou
> <suanmingm@nvidia.com>; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> Aman Singh <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>; Ferruh Yigit <ferruh.yigit@amd.com>; Andrew
> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [PATCH v7 2/3] ethdev: add VXLAN last reserved field
> 
> External email: Use caution opening links or attachments
> 
> 
> 05/06/2024 15:04, Rongwei Liu:
> > Add "uint8_t last_rsvd" as union with origin rsvd1.
> > Add RTE_FLOW_FIELD_VXLAN_LAST_RSVD into rte flow packet field.
> >
> > The new union is used by testpmd matching item VXLAN "last_rsvd"
> 
> no it is still using the name "rsvd1" in testpmd
> 
Map the testpmd cli "last_rsvd" to the new entry "last_rsvd"
Keep the vxlan-gpe as the original since it refers to rte_vxlan_gpe_hdr not rte_vxlan_hdr.
"rsvd1" string: I though you are talking about the vxlan-gpe not vxlan.
If we plan to change this too, another commit is desired.
> > and modify target "vxlan_last_rsvd".
> 
> You should give the real name: RTE_FLOW_FIELD_VXLAN_LAST_RSVD
ACK
> 
> With above changes,
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> > Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> > Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> 
> 

[PATCH v8 0/3] support VXLAN last reserved byte modification

[Rongwei Liu]

v8: enhance commit log and remap testpmd "last_rsvd" to new union.
v7: squash, add more commit logs.
v6: add union into vxlan header.
v5: change the field enumeration naming.
v4: specify the limitation with exact port number.
v3: squash testpmd and rte_flow into one commit.
v2: patch split.

Rongwei Liu (3):
  app/testpmd: fix the testpmd field string sequence
  ethdev: add VXLAN last reserved field
  net/mlx5: implement VXLAN last reserved modification

 app/test-pmd/cmdline_flow.c     |  9 +++++----
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 lib/ethdev/rte_flow.h           |  1 +
 lib/net/rte_vxlan.h             |  5 ++++-
 6 files changed, 32 insertions(+), 5 deletions(-)

-- 
2.27.0

[PATCH v8 1/3] app/testpmd: fix the testpmd field string sequence

[Rongwei Liu]

The field string should be in the same order as the rte_flow_field_id
enumration definitions

Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
Cc: michaelba@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/cmdline_flow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 60ee9337cf..775ce20fc4 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
 	"hash_result",
 	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
-	"random",
 	"ipv4_proto",
-	"esp_spi", "esp_seq_num", "esp_proto",
 	"ipv6_flow_label", "ipv6_traffic_class",
+	"esp_spi", "esp_seq_num", "esp_proto",
+	"random",
 	NULL
 };
 
-- 
2.27.0

[PATCH v8 2/3] ethdev: add VXLAN last reserved field

[Rongwei Liu]

Add "uint8_t last_rsvd" as union with origin rsvd1.
Add RTE_FLOW_FIELD_VXLAN_LAST_RSVD into rte flow packet
field.

The new union is used by testpmd matching item VXLAN
"last_rsvd" and modify target RTE_FLOW_FIELD_VXLAN_LAST_RSVD.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 app/test-pmd/cmdline_flow.c | 5 +++--
 lib/ethdev/rte_flow.h       | 1 +
 lib/net/rte_vxlan.h         | 5 ++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 775ce20fc4..99e2db04df 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -1006,6 +1006,7 @@ static const char *const flow_field_ids[] = {
 	"ipv6_flow_label", "ipv6_traffic_class",
 	"esp_spi", "esp_seq_num", "esp_proto",
 	"random",
+	"vxlan_last_rsvd",
 	NULL
 };
 
@@ -4912,11 +4913,11 @@ static const struct token token_list[] = {
 	},
 	[ITEM_VXLAN_LAST_RSVD] = {
 		.name = "last_rsvd",
-		.help = "VXLAN last reserved bits",
+		.help = "VXLAN last reserved byte",
 		.next = NEXT(item_vxlan, NEXT_ENTRY(COMMON_UNSIGNED),
 			     item_param),
 		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_item_vxlan,
-					     hdr.rsvd1)),
+					     hdr.last_rsvd)),
 	},
 	[ITEM_E_TAG] = {
 		.name = "e_tag",
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 6e8ab1d4c7..74cf2e0f59 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2428,6 +2428,7 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_ESP_SEQ_NUM,     /**< ESP Sequence Number. */
 	RTE_FLOW_FIELD_ESP_PROTO,       /**< ESP next protocol value. */
 	RTE_FLOW_FIELD_RANDOM,          /**< Random value. */
+	RTE_FLOW_FIELD_VXLAN_LAST_RSVD, /**< VXLAN last reserved byte. */
 };
 
 /**
diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
index 997fc784fc..57300fb442 100644
--- a/lib/net/rte_vxlan.h
+++ b/lib/net/rte_vxlan.h
@@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
 			uint8_t    flags;    /**< Should be 8 (I flag). */
 			uint8_t    rsvd0[3]; /**< Reserved. */
 			uint8_t    vni[3];   /**< VXLAN identifier. */
-			uint8_t    rsvd1;    /**< Reserved. */
+			union {
+				uint8_t    rsvd1;        /**< Reserved. */
+				uint8_t    last_rsvd;    /**< Reserved. */
+			};
 		};
 	};
 } __rte_packed;
-- 
2.27.0

[PATCH v8 3/3] net/mlx5: implement VXLAN last reserved modification

[Rongwei Liu]

Implementing the VxLAN last reserved byte modification.
Following the RFC, the field is only 1 byte and needs to
use the field_length as 8 instead of the real dst_field->size.

Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 12 ++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c |  5 +++++
 3 files changed, 22 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 8afb32ed35..4d0b8bf309 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -639,6 +639,11 @@ Limitations
   - Modify field action using ``RTE_FLOW_FIELD_RANDOM`` is not supported.
   - Modification of the 802.1Q tag is not supported.
   - Modification of VXLAN network or GENEVE network ID is supported only for HW steering.
+  - Modification of the VXLAN header is supported with below limitations:
+
+    - Only for HW steering (``dv_flow_en=2``).
+    - Support VNI and the last reserved byte modifications for traffic with default
+      UDP destination port. 4789 for VXLAN and VXLAN-GBP, 4790 for VXLAN-GPE.
   - Modification of GENEVE network ID is not supported when configured
     ``FLEX_PARSER_PROFILE_ENABLE`` supports Geneve TLV options.
     See :ref:`mlx5_firmware_config` for more flex parser information.
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 714613b15a..789e28b0ff 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1439,6 +1439,8 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_VXLAN_VNI:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		return 8;
 	case RTE_FLOW_FIELD_GTP_TEID:
 	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
@@ -2038,6 +2040,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
+		MLX5_ASSERT(data->offset + width <= 8);
+		/* Last_rsvd is on bits 7-0 of TUNNEL_HDR_DW_1. */
+		off_be = 8 - (data->offset + width);
+		info[idx] = (struct field_modify_info){1, 0, MLX5_MODI_TUNNEL_HDR_DW_1};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_8(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 		MLX5_ASSERT(data->offset + width <= 8);
 		modi_id = flow_geneve_opt_modi_field_get(priv, data);
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index e8562660dd..41a83768fb 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1313,6 +1313,10 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 			value = *(const uint8_t *)item.spec;
 			value = rte_cpu_to_be_32(value << 8);
 			item.spec = &value;
+		} else if (conf->dst.field == RTE_FLOW_FIELD_VXLAN_LAST_RSVD) {
+			value = *(const uint8_t *)item.spec << 24;
+			value = rte_cpu_to_be_32(value);
+			item.spec = &value;
 		}
 	} else {
 		type = conf->operation == RTE_FLOW_MODIFY_SET ?
@@ -5508,6 +5512,7 @@ flow_hw_validate_modify_field_level(const struct rte_flow_field_data *data,
 	case RTE_FLOW_FIELD_ESP_SPI:
 	case RTE_FLOW_FIELD_ESP_SEQ_NUM:
 	case RTE_FLOW_FIELD_VXLAN_VNI:
+	case RTE_FLOW_FIELD_VXLAN_LAST_RSVD:
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 	case RTE_FLOW_FIELD_GENEVE_OPT_TYPE:
 	case RTE_FLOW_FIELD_GENEVE_OPT_CLASS:
-- 
2.27.0

Re: [PATCH v8 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

07/06/2024 16:02, Rongwei Liu:
> Add "uint8_t last_rsvd" as union with origin rsvd1.
> Add RTE_FLOW_FIELD_VXLAN_LAST_RSVD into rte flow packet
> field.
> 
> The new union is used by testpmd matching item VXLAN
> "last_rsvd" and modify target RTE_FLOW_FIELD_VXLAN_LAST_RSVD.
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

OK this version looks good, thank you.

RE: [PATCH v6 1/4] app/testpmd: fix the testpmd field string sequence

[Ori Kam]

> -----Original Message-----
> From: rongwei liu <rongweil@nvidia.com>
> Sent: Wednesday, June 5, 2024 1:22 PM
> Subject: [PATCH v6 1/4] app/testpmd: fix the testpmd field string sequence
> 
> The field string should be in the same order as the rte_flow_field_id
> enumration definitions
> 
> Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
> Cc: michaelba@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---
Acked-by: Ori Kam <orika@nvidia.com>

RE: [PATCH v8 1/3] app/testpmd: fix the testpmd field string sequence

[Ori Kam]

> -----Original Message-----
> From: rongwei liu <rongweil@nvidia.com>
> Sent: Friday, June 7, 2024 5:02 PM
> 
> The field string should be in the same order as the rte_flow_field_id
> enumration definitions
> 
> Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
> Cc: michaelba@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---

Acked-by: Ori Kam <orika@nvidia.com>
Best,
Ori

Re: [PATCH v8 1/3] app/testpmd: fix the testpmd field string sequence

[Ferruh Yigit]

On 6/7/2024 3:02 PM, Rongwei Liu wrote:
> The field string should be in the same order as the rte_flow_field_id
> enumration definitions
> 
> Fixes: bfc007802 ("ethdev: allow modifying IPv6 FL and TC fields")
> Cc: michaelba@nvidia.com
> Cc: stable@dpdk.org
> 

 Fixes: d66aa38f431d ("ethdev: allow modifying IPsec fields")
 Fixes: b160da13b398 ("ethdev: allow modifying IPv4 next protocol field")

Above can be because of merge conflict, I didn't investigate in depth.

> Signed-off-by: Rongwei Liu <rongweil@nvidia.com>
> Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---
>  app/test-pmd/cmdline_flow.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 60ee9337cf..775ce20fc4 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -1002,10 +1002,10 @@ static const char *const flow_field_ids[] = {
>  	"hash_result",
>  	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
>  	"tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
> -	"random",
>  	"ipv4_proto",
> -	"esp_spi", "esp_seq_num", "esp_proto",
>  	"ipv6_flow_label", "ipv6_traffic_class",
> +	"esp_spi", "esp_seq_num", "esp_proto",
> +	"random",
>  	NULL
>  };
>  

Re: [PATCH v8 2/3] ethdev: add VXLAN last reserved field

[Ferruh Yigit]

On 6/7/2024 3:02 PM, Rongwei Liu wrote:
> diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h
> index 997fc784fc..57300fb442 100644
> --- a/lib/net/rte_vxlan.h
> +++ b/lib/net/rte_vxlan.h
> @@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
>  			uint8_t    flags;    /**< Should be 8 (I flag). */
>  			uint8_t    rsvd0[3]; /**< Reserved. */
>  			uint8_t    vni[3];   /**< VXLAN identifier. */
> -			uint8_t    rsvd1;    /**< Reserved. */
> +			union {
> +				uint8_t    rsvd1;        /**< Reserved. */
> +				uint8_t    last_rsvd;    /**< Reserved. */
> +			};
>

Is there a plan to remove 'rsvd1' in next ABI break release?
We can keep both, but I guess it is not logically necessary to keep it,
to prevent bloat by time, we can remove the old one.
If decided to remove, sending a 'deprecation.rst' update helps us to
remember doing it.

Re: [PATCH v8 0/3] support VXLAN last reserved byte modification

[Ferruh Yigit]

On 6/7/2024 3:02 PM, Rongwei Liu wrote:
> v8: enhance commit log and remap testpmd "last_rsvd" to new union.
> v7: squash, add more commit logs.
> v6: add union into vxlan header.
> v5: change the field enumeration naming.
> v4: specify the limitation with exact port number.
> v3: squash testpmd and rte_flow into one commit.
> v2: patch split.
> 
> Rongwei Liu (3):
>   app/testpmd: fix the testpmd field string sequence
>   ethdev: add VXLAN last reserved field
>   net/mlx5: implement VXLAN last reserved modification
>

Series applied to dpdk-next-net/main, thanks.

RE: [PATCH v8 2/3] ethdev: add VXLAN last reserved field

[rongwei liu]

BR
Rongwei

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Tuesday, June 11, 2024 22:53
> To: rongwei liu <rongweil@nvidia.com>; 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: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [PATCH v8 2/3] ethdev: add VXLAN last reserved field
> 
> External email: Use caution opening links or attachments
> 
> 
> On 6/7/2024 3:02 PM, Rongwei Liu wrote:
> > diff --git a/lib/net/rte_vxlan.h b/lib/net/rte_vxlan.h index
> > 997fc784fc..57300fb442 100644
> > --- a/lib/net/rte_vxlan.h
> > +++ b/lib/net/rte_vxlan.h
> > @@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
> >                       uint8_t    flags;    /**< Should be 8 (I flag). */
> >                       uint8_t    rsvd0[3]; /**< Reserved. */
> >                       uint8_t    vni[3];   /**< VXLAN identifier. */
> > -                     uint8_t    rsvd1;    /**< Reserved. */
> > +                     union {
> > +                             uint8_t    rsvd1;        /**< Reserved. */
> > +                             uint8_t    last_rsvd;    /**< Reserved. */
> > +                     };
> >
> 
> Is there a plan to remove 'rsvd1' in next ABI break release?
> We can keep both, but I guess it is not logically necessary to keep it, to prevent
> bloat by time, we can remove the old one.
> If decided to remove, sending a 'deprecation.rst' update helps us to remember
> doing it.
> 
I think it should. @NBU-Contact-Thomas Monjalon (EXTERNAL) @Andrew Rybchenko@Ori Kam what do you think?

Re: [PATCH v8 2/3] ethdev: add VXLAN last reserved field

[Thomas Monjalon]

12/06/2024 03:25, rongwei liu:
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> > On 6/7/2024 3:02 PM, Rongwei Liu wrote:
> > > @@ -41,7 +41,10 @@ struct rte_vxlan_hdr {
> > >                       uint8_t    flags;    /**< Should be 8 (I flag). */
> > >                       uint8_t    rsvd0[3]; /**< Reserved. */
> > >                       uint8_t    vni[3];   /**< VXLAN identifier. */
> > > -                     uint8_t    rsvd1;    /**< Reserved. */
> > > +                     union {
> > > +                             uint8_t    rsvd1;        /**< Reserved. */
> > > +                             uint8_t    last_rsvd;    /**< Reserved. */
> > > +                     };
> > >
> > 
> > Is there a plan to remove 'rsvd1' in next ABI break release?
> > We can keep both, but I guess it is not logically necessary to keep it, to prevent
> > bloat by time, we can remove the old one.
> > If decided to remove, sending a 'deprecation.rst' update helps us to remember
> > doing it.
> > 
> I think it should. @NBU-Contact-Thomas Monjalon (EXTERNAL) @Andrew Rybchenko@Ori Kam what do you think?

From user perspective, there is no benefit in removing an aliased field,
except for simplicity.
The drawback is a potential API compatibility breakage.

We may mark it as deprecated in the comment and plan for removal in a long time, let's say 25.11?
Is there anyone against removing "rsvd1" in VXLAN header for compatibility purpose?