DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v1 0/7] ethdev: modify field API for multiple headers
@ 2023-05-16  6:37 Michael Baum
  2023-05-16  6:37 ` [PATCH v1 1/7] doc: fix blank lines in modify field action description Michael Baum
                   ` (7 more replies)
  0 siblings, 8 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

This patch-set extend the modify field action API to support both
multiple MPLS and GENEVE option headers.

In current API, the header type is provided by rte_flow_field_id
enumeration and the encapsulation level (inner/outer/tunnel) is
specified by data.level field.
However, there is no way to specify header inside encapsulation level.

For example, for this packet:

eth / mpls / mpls / mpls / ipv4 / udp
the both second and third MPLS headers cannot be modified using this
API.

RFC:
https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/

Michael Baum (7):
  doc: fix blank lines in modify field action description
  doc: fix blank line in asynchronous operations description
  doc: fix wrong indentation in RSS action description
  net/mlx5: reduce modify field encapsulation level size
  ethdev: add GENEVE TLV option modification support
  ethdev: add MPLS header modification support
  net/mlx5: add MPLS modify field support

 app/test-pmd/cmdline_flow.c            | 70 ++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 38 +++++++++++---
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/common/mlx5/mlx5_prm.h         |  5 ++
 drivers/net/mlx5/mlx5_flow_dv.c        | 23 ++++++++
 drivers/net/mlx5/mlx5_flow_hw.c        | 38 +++++++-------
 lib/ethdev/rte_flow.h                  | 72 ++++++++++++++++++++++++--
 7 files changed, 219 insertions(+), 30 deletions(-)

-- 
2.25.1


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

* [PATCH v1 1/7] doc: fix blank lines in modify field action description
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-16  6:37 ` [PATCH v1 2/7] doc: fix blank line in asynchronous operations description Michael Baum
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The modify field action description inside "Generic flow API (rte_flow)"
documentation, lists all operations supported for a destination field.
In addition, it lists the values supported for a encapsulation level
field.

Before the lists, in both cases, miss a blank line causing them to look
regular text lines.

This patch adds the blank lines.

Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..e7faa368a1 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2917,20 +2917,23 @@ The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
 ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
 See ``enum rte_flow_field_id`` for the list of supported fields.
 
-``op`` selects the operation to perform on a destination field.
+``op`` selects the operation to perform on a destination field:
+
 - ``set`` copies the data from ``src`` field to ``dst`` field.
 - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
-- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
+- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
 ``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array.
-- ``0`` means the default behaviour. Depending on the packet type, it can
-mean outermost, innermost or anything in between.
+as well as any tag element in the tag array:
+
+- ``0`` means the default behaviour. Depending on the packet type,
+  it can mean outermost, innermost or anything in between.
 - ``1`` requests access to the outermost packet encapsulation level.
 - ``2`` and subsequent values requests access to the specified packet
-encapsulation level, from outermost to innermost (lower to higher values).
+  encapsulation level, from outermost to innermost (lower to higher values).
+
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
-- 
2.25.1


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

* [PATCH v1 2/7] doc: fix blank line in asynchronous operations description
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
  2023-05-16  6:37 ` [PATCH v1 1/7] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-17 17:13   ` Ori Kam
  2023-05-16  6:37 ` [PATCH v1 3/7] doc: fix wrong indentation in RSS action description Michael Baum
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The asynchronous operations description inside "Generic flow API
(rte_flow)" documentation, adds some bullets to describe asynchronous
operations behavior.

Before the first bullet, miss a blank line causing it to look a regular
text line.

This patch adds the blank line.

Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules operations")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e7faa368a1..76e69190fc 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3702,6 +3702,7 @@ Asynchronous operations
 -----------------------
 
 Flow rules management can be done via special lockless flow management queues.
+
 - Queue operations are asynchronous and not thread-safe.
 
 - Operations can thus be invoked by the app's datapath,
-- 
2.25.1


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

* [PATCH v1 3/7] doc: fix wrong indentation in RSS action description
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
  2023-05-16  6:37 ` [PATCH v1 1/7] doc: fix blank lines in modify field action description Michael Baum
  2023-05-16  6:37 ` [PATCH v1 2/7] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-18  6:18   ` Ori Kam
  2023-05-16  6:37 ` [PATCH v1 4/7] net/mlx5: reduce modify field encapsulation level size Michael Baum
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	adrien.mazarguil, stable

The RSS action description inside "Generic flow API (rte_flow)"
documentation, lists the values supported for a encapsulation level
field.

For "2" value, it uses 3 spaces as an indentation instead of 2 after
line breaking, causing the first line to be bold.

This patch updates the number of spaces in the indentation.

Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 76e69190fc..25b57bf86d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
   level.
 
 - ``2`` and subsequent values request RSS to be performed on the specified
-   inner packet encapsulation level, from outermost to innermost (lower to
-   higher values).
+  inner packet encapsulation level, from outermost to innermost (lower to
+  higher values).
 
 Values other than ``0`` are not necessarily supported.
 
-- 
2.25.1


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

* [PATCH v1 4/7] net/mlx5: reduce modify field encapsulation level size
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
                   ` (2 preceding siblings ...)
  2023-05-16  6:37 ` [PATCH v1 3/7] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-16  6:37 ` [PATCH v1 5/7] ethdev: add GENEVE TLV option modification support Michael Baum
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

The type of "level" field in "rte_flow_action_modify_data" structure is
uint32_t for now, but it is going to be changed to uint8_t in the next
patch.

For representing encapsulation level, 8 bits are more than enough and
this change shouldn't affect the current implementation.
However, when action template is created, the PMD requests to provide
this field "fully masked" in action mask. The "fully masked" value is
different between uint32_t and uint8_t types.

This patch reduces all modify field encapsulation level "fully masked"
initializations to use UINT8_MAX instead of UINT32_MAX. This change will
avoid compilation warning after it will be changed to uint8_t by API.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 7e0ee8d883..1b68a19900 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3565,7 +3565,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
-	if (mask_conf->dst.level != UINT32_MAX)
+	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
 			"destination encapsulation level must be fully masked");
@@ -3579,7 +3579,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (mask_conf->src.level != UINT32_MAX)
+		if (mask_conf->src.level != UINT8_MAX)
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source encapsulation level must be fully masked");
@@ -4450,7 +4450,7 @@ flow_hw_set_vlan_vid(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = RTE_FLOW_FIELD_VLAN_ID,
-			.level = 0xffffffff, .offset = 0xffffffff,
+			.level = 0xff, .offset = 0xffffffff,
 		},
 		.src = {
 			.field = RTE_FLOW_FIELD_VALUE,
@@ -4583,12 +4583,12 @@ flow_hw_actions_template_create(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -5653,7 +5653,7 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -5677,12 +5677,12 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -6009,7 +6009,7 @@ flow_hw_create_ctrl_regc_jump_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -6182,12 +6182,12 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
-- 
2.25.1


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

* [PATCH v1 5/7] ethdev: add GENEVE TLV option modification support
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
                   ` (3 preceding siblings ...)
  2023-05-16  6:37 ` [PATCH v1 4/7] net/mlx5: reduce modify field encapsulation level size Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-16  6:37 ` [PATCH v1 6/7] ethdev: add MPLS header " Michael Baum
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add modify field support for GENEVE option fields:
 - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
 - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
 - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"

Each GENEVE TLV option is identified by both its "class" and "type", so
2 new fields were added to "rte_flow_action_modify_data" structure to
help specify which option to modify.

To get room for those 2 new fields, the "level" field move to use
"uint8_t" which is more than enough for encapsulation level.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 48 +++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 12 ++++++
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 lib/ethdev/rte_flow.h                  | 51 +++++++++++++++++++++++++-
 4 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 58939ec321..8c1dea53c0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,11 +636,15 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -854,7 +858,9 @@ static const char *const modify_field_ids[] = {
 	"ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color",
 	"ipv6_proto",
 	"flex_item",
-	"hash_result", NULL
+	"hash_result",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	NULL
 };
 
 static const char *const meter_colors[] = {
@@ -2295,6 +2301,8 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ZERO,
@@ -2302,6 +2310,8 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -6388,6 +6398,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
+		.name = "dst_type_id",
+		.help = "destination field type ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_DST_CLASS_ID] = {
+		.name = "dst_class",
+		.help = "destination field class ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     dst.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_OFFSET] = {
 		.name = "dst_offset",
 		.help = "destination field bit offset",
@@ -6423,6 +6451,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
+		.name = "src_type_id",
+		.help = "source field type ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_SRC_CLASS_ID] = {
+		.name = "src_class",
+		.help = "source field class ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     src.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_OFFSET] = {
 		.name = "src_offset",
 		.help = "source field bit offset",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 25b57bf86d..cd38f0de46 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,14 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+``type`` is used to specify (along with ``class_id``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
+``class_id`` is used to specify (along with ``type``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
 ``flex_handle`` is used to specify the flex item pointer which is being
 modified. ``flex_handle`` and ``level`` are mutually exclusive.
 
@@ -2994,6 +3002,10 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``type``        | geneve option type                                       |
+   +-----------------+----------------------------------------------------------+
+   | ``class_id``    | geneve option class ID                                   |
+   +-----------------+----------------------------------------------------------+
    | ``flex_handle`` | flex item handle of a packet field                       |
    +-----------------+----------------------------------------------------------+
    | ``offset``      | number of bits to skip at the beginning                  |
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..ce1755096f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,6 +84,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The ``level`` field in experimental structure
+  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+
 
 ABI Changes
 -----------
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..b82eb0c0a8 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3773,6 +3773,9 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_PROTO,	/**< IPv6 next header. */
 	RTE_FLOW_FIELD_FLEX_ITEM,	/**< Flex item. */
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
+	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
+	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
 };
 
 /**
@@ -3788,7 +3791,53 @@ struct rte_flow_action_modify_data {
 		struct {
 			/** Encapsulation level or tag index or flex item handle. */
 			union {
-				uint32_t level;
+				struct {
+					/**
+					 * Packet encapsulation level containing
+					 * the field modify to.
+					 *
+					 * - @p 0 requests the default behavior.
+					 *   Depending on the packet type, it
+					 *   can mean outermost, innermost or
+					 *   anything in between.
+					 *
+					 *   It basically stands for the
+					 *   innermost encapsulation level
+					 *   modification can be performed on
+					 *   according to PMD and device
+					 *   capabilities.
+					 *
+					 * - @p 1 requests modification to be
+					 *   performed on the outermost packet
+					 *   encapsulation level.
+					 *
+					 * - @p 2 and subsequent values request
+					 *   modification to be performed on
+					 *   the specified inner packet
+					 *   encapsulation level, from
+					 *   outermost to innermost (lower to
+					 *   higher values).
+					 *
+					 * Values other than @p 0 are not
+					 * necessarily supported.
+					 *
+					 * For RTE_FLOW_FIELD_TAG it represents
+					 * the tag element in the tag array.
+					 */
+					uint8_t level;
+					/**
+					 * Geneve option type. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					uint8_t type;
+					/**
+					 * Geneve option class. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					rte_be16_t class_id;
+				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
 			/** Number of bits to skip from a field. */
-- 
2.25.1


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

* [PATCH v1 6/7] ethdev: add MPLS header modification support
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
                   ` (4 preceding siblings ...)
  2023-05-16  6:37 ` [PATCH v1 5/7] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-16  6:37 ` [PATCH v1 7/7] net/mlx5: add MPLS modify field support Michael Baum
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
  7 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.

Since MPLS heaser might appear more the one time in inner/outer/tunnel,
a new field was added to "rte_flow_action_modify_data" structure in
addition to "level" field.
The "sub_level" field is the index of the header inside encapsulation
level. It is used for modify multiple MPLS headers in same encapsulation
level.

This addition enables to modify multiple VLAN headers too, so the
description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c        | 24 ++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst |  6 ++++
 lib/ethdev/rte_flow.h              | 47 ++++++++++++++++++++----------
 3 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8c1dea53c0..5370be1e3c 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,6 +636,7 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_SUB_LEVEL,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -643,6 +644,7 @@ enum index {
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_SUB_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -859,7 +861,7 @@ static const char *const modify_field_ids[] = {
 	"ipv6_proto",
 	"flex_item",
 	"hash_result",
-	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	NULL
 };
 
@@ -2301,6 +2303,7 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_SUB_LEVEL,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -2310,6 +2313,7 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_SUB_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -6398,6 +6402,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_SUB_LEVEL] = {
+		.name = "dst_sub_level",
+		.help = "destination field sub level",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.sub_level)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
 		.name = "dst_type_id",
 		.help = "destination field type ID",
@@ -6451,6 +6464,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_SUB_LEVEL] = {
+		.name = "stc_sub_level",
+		.help = "source field sub level",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.sub_level)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
 		.name = "src_type_id",
 		.help = "source field type ID",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index cd38f0de46..1f681a38e4 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,10 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+- ``sub_level`` is the index of the header inside encapsulation level.
+  It is used for modify either ``VLAN`` or ``MPLS`` headers which multiple of
+  them might be supported in same encapsulation level.
+
 ``type`` is used to specify (along with ``class_id``) the Geneve option which
 is being modified.
 This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
@@ -3002,6 +3006,8 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``sub_level``   | header level inside encapsulation level                  |
+   +-----------------+----------------------------------------------------------+
    | ``type``        | geneve option type                                       |
    +-----------------+----------------------------------------------------------+
    | ``class_id``    | geneve option class ID                                   |
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index b82eb0c0a8..4b2e17e266 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
 	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC Address. */
 	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
-	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
-	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier. */
+	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
+	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
 	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
 	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
 	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
@@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
 	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
-	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option data */
+	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
 };
 
 /**
@@ -3821,22 +3822,38 @@ struct rte_flow_action_modify_data {
 					 * Values other than @p 0 are not
 					 * necessarily supported.
 					 *
+					 * @note that for MPLS field,
+					 * encapsulation level also include
+					 * tunnel since MPLS may appear in
+					 * outer, inner or tunnel.
+					 *
 					 * For RTE_FLOW_FIELD_TAG it represents
 					 * the tag element in the tag array.
 					 */
 					uint8_t level;
-					/**
-					 * Geneve option type. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					uint8_t type;
-					/**
-					 * Geneve option class. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					rte_be16_t class_id;
+					union {
+						/**
+						 * Header level inside
+						 * encapsulation level.
+						 */
+						uint8_t sub_level;
+						/**
+						 * Geneve option identifier.
+						 * relevant only for
+						 * RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+						 * modification type.
+						 */
+						struct {
+							/**
+							 * Geneve option type.
+							 */
+							uint8_t type;
+							/**
+							 * Geneve option class.
+							 */
+							rte_be16_t class_id;
+						};
+					};
 				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
-- 
2.25.1


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

* [PATCH v1 7/7] net/mlx5: add MPLS modify field support
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
                   ` (5 preceding siblings ...)
  2023-05-16  6:37 ` [PATCH v1 6/7] ethdev: add MPLS header " Michael Baum
@ 2023-05-16  6:37 ` Michael Baum
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
  7 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-16  6:37 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for modify field in tunnel MPLS header.
For now it is supported only to copy from.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/common/mlx5/mlx5_prm.h  |  5 +++++
 drivers/net/mlx5/mlx5_flow_dv.c | 23 +++++++++++++++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c | 16 +++++++++-------
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index ed3d5efbb7..04c1400a1e 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -787,6 +787,11 @@ enum mlx5_modification_field {
 	MLX5_MODI_TUNNEL_HDR_DW_1 = 0x75,
 	MLX5_MODI_GTPU_FIRST_EXT_DW_0 = 0x76,
 	MLX5_MODI_HASH_RESULT = 0x81,
+	MLX5_MODI_IN_MPLS_LABEL_0 = 0x8a,
+	MLX5_MODI_IN_MPLS_LABEL_1,
+	MLX5_MODI_IN_MPLS_LABEL_2,
+	MLX5_MODI_IN_MPLS_LABEL_3,
+	MLX5_MODI_IN_MPLS_LABEL_4,
 	MLX5_MODI_OUT_IPV6_NEXT_HDR = 0x4A,
 	MLX5_MODI_INVALID = INT_MAX,
 };
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f136f43b0a..93cce16a1e 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1388,6 +1388,7 @@ mlx5_flow_item_field_width(struct rte_eth_dev *dev,
 	case RTE_FLOW_FIELD_GENEVE_VNI:
 		return 24;
 	case RTE_FLOW_FIELD_GTP_TEID:
+	case RTE_FLOW_FIELD_MPLS:
 	case RTE_FLOW_FIELD_TAG:
 		return 32;
 	case RTE_FLOW_FIELD_MARK:
@@ -1435,6 +1436,12 @@ flow_modify_info_mask_32_masked(uint32_t length, uint32_t off, uint32_t post_mas
 	return rte_cpu_to_be_32(mask & post_mask);
 }
 
+static __rte_always_inline enum mlx5_modification_field
+mlx5_mpls_modi_field_get(const struct rte_flow_action_modify_data *data)
+{
+	return MLX5_MODI_IN_MPLS_LABEL_0 + data->sub_level;
+}
+
 static void
 mlx5_modify_flex_item(const struct rte_eth_dev *dev,
 		      const struct mlx5_flex_item *flex,
@@ -1893,6 +1900,16 @@ mlx5_flow_field_id_to_modify_info
 		else
 			info[idx].offset = off_be;
 		break;
+	case RTE_FLOW_FIELD_MPLS:
+		MLX5_ASSERT(data->offset + width <= 32);
+		off_be = 32 - (data->offset + width);
+		info[idx] = (struct field_modify_info){4, 0,
+					mlx5_mpls_modi_field_get(data)};
+		if (mask)
+			mask[idx] = flow_modify_info_mask_32(width, off_be);
+		else
+			info[idx].offset = off_be;
+		break;
 	case RTE_FLOW_FIELD_TAG:
 		{
 			MLX5_ASSERT(data->offset + width <= 32);
@@ -5344,6 +5361,12 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of the GENEVE Network"
 				" Identifier is not supported");
+	if (action_modify_field->dst.field == RTE_FLOW_FIELD_MPLS ||
+	    action_modify_field->src.field == RTE_FLOW_FIELD_MPLS)
+		return rte_flow_error_set(error, ENOTSUP,
+				RTE_FLOW_ERROR_TYPE_ACTION, action,
+				"modifications of the MPLS header "
+				"is not supported");
 	if (action_modify_field->dst.field == RTE_FLOW_FIELD_MARK ||
 	    action_modify_field->src.field == RTE_FLOW_FIELD_MARK)
 		if (config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 1b68a19900..80e6398992 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3546,10 +3546,8 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				     const struct rte_flow_action *mask,
 				     struct rte_flow_error *error)
 {
-	const struct rte_flow_action_modify_field *action_conf =
-		action->conf;
-	const struct rte_flow_action_modify_field *mask_conf =
-		mask->conf;
+	const struct rte_flow_action_modify_field *action_conf = action->conf;
+	const struct rte_flow_action_modify_field *mask_conf = mask->conf;
 
 	if (action_conf->operation != mask_conf->operation)
 		return rte_flow_error_set(error, EINVAL,
@@ -3604,6 +3602,11 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifying Geneve VNI is not supported");
+	/* Due to HW bug, tunnel MPLS header is read only. */
+	if (action_conf->dst.field == RTE_FLOW_FIELD_MPLS)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, action,
+				"MPLS cannot be used as destination");
 	return 0;
 }
 
@@ -4134,9 +4137,8 @@ mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_METER;
 			break;
 		case RTE_FLOW_ACTION_TYPE_MODIFY_FIELD:
-			ret = flow_hw_validate_action_modify_field(action,
-									mask,
-									error);
+			ret = flow_hw_validate_action_modify_field(action, mask,
+								   error);
 			if (ret < 0)
 				return ret;
 			action_flags |= MLX5_FLOW_ACTION_MODIFY_FIELD;
-- 
2.25.1


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

* RE: [PATCH v1 2/7] doc: fix blank line in asynchronous operations description
  2023-05-16  6:37 ` [PATCH v1 2/7] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-17 17:13   ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-17 17:13 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	Alexander Kozyrev, stable

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Tuesday, May 16, 2023 9:38 AM
> 
> The asynchronous operations description inside "Generic flow API
> (rte_flow)" documentation, adds some bullets to describe asynchronous
> operations behavior.
> 
> Before the first bullet, miss a blank line causing it to look a regular
> text line.
> 
> This patch adds the blank line.
> 
> Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules
> operations")
> Cc: akozyrev@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index e7faa368a1..76e69190fc 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3702,6 +3702,7 @@ Asynchronous operations
>  -----------------------
> 
>  Flow rules management can be done via special lockless flow management
> queues.
> +
>  - Queue operations are asynchronous and not thread-safe.
> 
>  - Operations can thus be invoked by the app's datapath,
> --
> 2.25.1

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

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

* RE: [PATCH v1 3/7] doc: fix wrong indentation in RSS action description
  2023-05-16  6:37 ` [PATCH v1 3/7] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-18  6:18   ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-18  6:18 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	NBU-Contact-Adrien Mazarguil (EXTERNAL),
	stable

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Tuesday, May 16, 2023 9:38 AM
> 
> The RSS action description inside "Generic flow API (rte_flow)"
> documentation, lists the values supported for a encapsulation level
> field.
> 
> For "2" value, it uses 3 spaces as an indentation instead of 2 after
> line breaking, causing the first line to be bold.
> 
> This patch updates the number of spaces in the indentation.
> 
> Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
> Cc: adrien.mazarguil@6wind.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 76e69190fc..25b57bf86d 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
>    level.
> 
>  - ``2`` and subsequent values request RSS to be performed on the specified
> -   inner packet encapsulation level, from outermost to innermost (lower to
> -   higher values).
> +  inner packet encapsulation level, from outermost to innermost (lower to
> +  higher values).
> 
>  Values other than ``0`` are not necessarily supported.
> 
> --
> 2.25.1

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

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

* [PATCH v2 0/5] ethdev: modify field API for multiple headers
  2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
                   ` (6 preceding siblings ...)
  2023-05-16  6:37 ` [PATCH v1 7/7] net/mlx5: add MPLS modify field support Michael Baum
@ 2023-05-18 17:40 ` Michael Baum
  2023-05-18 17:40   ` [PATCH v2 1/5] doc: fix blank lines in modify field action description Michael Baum
                     ` (5 more replies)
  7 siblings, 6 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

This patch-set extend the modify field action API to support both
multiple MPLS and GENEVE option headers.

In current API, the header type is provided by rte_flow_field_id
enumeration and the encapsulation level (inner/outer/tunnel) is
specified by data.level field.
However, there is no way to specify header inside encapsulation level.

For example, for this packet:

eth / mpls / mpls / mpls / ipv4 / udp
the both second and third MPLS headers cannot be modified using this
API.

RFC:
https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/

v2:
 - Change "sub_level" name to "tag_index".
 - Squash PMD changes into API changes patch.
 - Remove PMD private patch from the patch-set.

Michael Baum (5):
  doc: fix blank lines in modify field action description
  doc: fix blank line in asynchronous operations description
  doc: fix wrong indentation in RSS action description
  ethdev: add GENEVE TLV option modification support
  ethdev: add MPLS header modification support

 app/test-pmd/cmdline_flow.c            | 70 ++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 53 ++++++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++----
 lib/ethdev/rte_flow.h                  | 71 ++++++++++++++++++++++++--
 5 files changed, 191 insertions(+), 28 deletions(-)

-- 
2.25.1


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

* [PATCH v2 1/5] doc: fix blank lines in modify field action description
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
@ 2023-05-18 17:40   ` Michael Baum
  2023-05-21 10:07     ` Ori Kam
  2023-05-18 17:40   ` [PATCH v2 2/5] doc: fix blank line in asynchronous operations description Michael Baum
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The modify field action description inside "Generic flow API (rte_flow)"
documentation, lists all operations supported for a destination field.
In addition, it lists the values supported for a encapsulation level
field.

Before the lists, in both cases, miss a blank line causing them to look
regular text lines.

This patch adds the blank lines.

Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..e7faa368a1 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2917,20 +2917,23 @@ The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
 ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
 See ``enum rte_flow_field_id`` for the list of supported fields.
 
-``op`` selects the operation to perform on a destination field.
+``op`` selects the operation to perform on a destination field:
+
 - ``set`` copies the data from ``src`` field to ``dst`` field.
 - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
-- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
+- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
 ``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array.
-- ``0`` means the default behaviour. Depending on the packet type, it can
-mean outermost, innermost or anything in between.
+as well as any tag element in the tag array:
+
+- ``0`` means the default behaviour. Depending on the packet type,
+  it can mean outermost, innermost or anything in between.
 - ``1`` requests access to the outermost packet encapsulation level.
 - ``2`` and subsequent values requests access to the specified packet
-encapsulation level, from outermost to innermost (lower to higher values).
+  encapsulation level, from outermost to innermost (lower to higher values).
+
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
-- 
2.25.1


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

* [PATCH v2 2/5] doc: fix blank line in asynchronous operations description
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-18 17:40   ` [PATCH v2 1/5] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-18 17:40   ` Michael Baum
  2023-05-21 10:07     ` Ori Kam
  2023-05-18 17:40   ` [PATCH v2 3/5] doc: fix wrong indentation in RSS action description Michael Baum
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The asynchronous operations description inside "Generic flow API
(rte_flow)" documentation, adds some bullets to describe asynchronous
operations behavior.

Before the first bullet, miss a blank line causing it to look a regular
text line.

This patch adds the blank line.

Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules operations")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e7faa368a1..76e69190fc 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3702,6 +3702,7 @@ Asynchronous operations
 -----------------------
 
 Flow rules management can be done via special lockless flow management queues.
+
 - Queue operations are asynchronous and not thread-safe.
 
 - Operations can thus be invoked by the app's datapath,
-- 
2.25.1


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

* [PATCH v2 3/5] doc: fix wrong indentation in RSS action description
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-18 17:40   ` [PATCH v2 1/5] doc: fix blank lines in modify field action description Michael Baum
  2023-05-18 17:40   ` [PATCH v2 2/5] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-18 17:40   ` Michael Baum
  2023-05-21 10:08     ` Ori Kam
  2023-05-18 17:40   ` [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	adrien.mazarguil, stable

The RSS action description inside "Generic flow API (rte_flow)"
documentation, lists the values supported for a encapsulation level
field.

For "2" value, it uses 3 spaces as an indentation instead of 2 after
line breaking, causing the first line to be bold.

This patch updates the number of spaces in the indentation.

Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 76e69190fc..25b57bf86d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
   level.
 
 - ``2`` and subsequent values request RSS to be performed on the specified
-   inner packet encapsulation level, from outermost to innermost (lower to
-   higher values).
+  inner packet encapsulation level, from outermost to innermost (lower to
+  higher values).
 
 Values other than ``0`` are not necessarily supported.
 
-- 
2.25.1


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

* [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
                     ` (2 preceding siblings ...)
  2023-05-18 17:40   ` [PATCH v2 3/5] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-18 17:40   ` Michael Baum
  2023-05-21 18:52     ` Ori Kam
  2023-05-18 17:40   ` [PATCH v2 5/5] ethdev: add MPLS header " Michael Baum
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add modify field support for GENEVE option fields:
 - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
 - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
 - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"

Each GENEVE TLV option is identified by both its "class" and "type", so
2 new fields were added to "rte_flow_action_modify_data" structure to
help specify which option to modify.

To get room for those 2 new fields, the "level" field move to use
"uint8_t" which is more than enough for encapsulation level.
This patch also reduces all modify field encapsulation level "fully
masked" initializations to use UINT8_MAX instead of UINT32_MAX.
This change avoids compilation warning caused by this API changing.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 48 +++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 23 ++++++++++++
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++++------
 lib/ethdev/rte_flow.h                  | 48 +++++++++++++++++++++++++-
 5 files changed, 131 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 58939ec321..8c1dea53c0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,11 +636,15 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -854,7 +858,9 @@ static const char *const modify_field_ids[] = {
 	"ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color",
 	"ipv6_proto",
 	"flex_item",
-	"hash_result", NULL
+	"hash_result",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	NULL
 };
 
 static const char *const meter_colors[] = {
@@ -2295,6 +2301,8 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ZERO,
@@ -2302,6 +2310,8 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -6388,6 +6398,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
+		.name = "dst_type_id",
+		.help = "destination field type ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_DST_CLASS_ID] = {
+		.name = "dst_class",
+		.help = "destination field class ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     dst.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_OFFSET] = {
 		.name = "dst_offset",
 		.help = "destination field bit offset",
@@ -6423,6 +6451,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
+		.name = "src_type_id",
+		.help = "source field type ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_SRC_CLASS_ID] = {
+		.name = "src_class",
+		.help = "source field class ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     src.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_OFFSET] = {
 		.name = "src_offset",
 		.help = "source field bit offset",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 25b57bf86d..ec812de335 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,14 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+``type`` is used to specify (along with ``class_id``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
+``class_id`` is used to specify (along with ``type``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
 ``flex_handle`` is used to specify the flex item pointer which is being
 modified. ``flex_handle`` and ``level`` are mutually exclusive.
 
@@ -2967,6 +2975,17 @@ to replace the third byte of MAC address with value 0x85, application should
 specify destination width as 8, destination offset as 16, and provide immediate
 value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
 
+The ``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` type supports modifying only one DW in
+single action and align to 32 bits. For example, for modifying 16 bits start
+from offset 24, 2 different actions should be prepared. The first one includs
+``offset=24`` and ``width=8``, and the seconde one includs ``offset=32`` and
+``width=8``.
+Application should provide the data in immediate value memory only for the
+single DW even though the offset is related to start of first DW. For example,
+to replace the third byte of second DW in Geneve option data with value 0x85,
+application should specify destination width as 8, destination offset as 48,
+and provide immediate value 0xXXXX85XX.
+
 .. _table_rte_flow_action_modify_field:
 
 .. table:: MODIFY_FIELD
@@ -2994,6 +3013,10 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``type``        | geneve option type                                       |
+   +-----------------+----------------------------------------------------------+
+   | ``class_id``    | geneve option class ID                                   |
+   +-----------------+----------------------------------------------------------+
    | ``flex_handle`` | flex item handle of a packet field                       |
    +-----------------+----------------------------------------------------------+
    | ``offset``      | number of bits to skip at the beginning                  |
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..ce1755096f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,6 +84,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The ``level`` field in experimental structure
+  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+
 
 ABI Changes
 -----------
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 7e0ee8d883..1b68a19900 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3565,7 +3565,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
-	if (mask_conf->dst.level != UINT32_MAX)
+	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
 			"destination encapsulation level must be fully masked");
@@ -3579,7 +3579,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (mask_conf->src.level != UINT32_MAX)
+		if (mask_conf->src.level != UINT8_MAX)
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source encapsulation level must be fully masked");
@@ -4450,7 +4450,7 @@ flow_hw_set_vlan_vid(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = RTE_FLOW_FIELD_VLAN_ID,
-			.level = 0xffffffff, .offset = 0xffffffff,
+			.level = 0xff, .offset = 0xffffffff,
 		},
 		.src = {
 			.field = RTE_FLOW_FIELD_VALUE,
@@ -4583,12 +4583,12 @@ flow_hw_actions_template_create(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -5653,7 +5653,7 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -5677,12 +5677,12 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -6009,7 +6009,7 @@ flow_hw_create_ctrl_regc_jump_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -6182,12 +6182,12 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..f30d4b033f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3773,6 +3773,9 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_PROTO,	/**< IPv6 next header. */
 	RTE_FLOW_FIELD_FLEX_ITEM,	/**< Flex item. */
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
+	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
+	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
 };
 
 /**
@@ -3788,7 +3791,50 @@ struct rte_flow_action_modify_data {
 		struct {
 			/** Encapsulation level or tag index or flex item handle. */
 			union {
-				uint32_t level;
+				struct {
+					/**
+					 * Packet encapsulation level containing
+					 * the field modify to.
+					 *
+					 * - @p 0 requests the default behavior.
+					 *   Depending on the packet type, it
+					 *   can mean outermost, innermost or
+					 *   anything in between.
+					 *
+					 *   It basically stands for the
+					 *   innermost encapsulation level
+					 *   modification can be performed on
+					 *   according to PMD and device
+					 *   capabilities.
+					 *
+					 * - @p 1 requests modification to be
+					 *   performed on the outermost packet
+					 *   encapsulation level.
+					 *
+					 * - @p 2 and subsequent values request
+					 *   modification to be performed on
+					 *   the specified inner packet
+					 *   encapsulation level, from
+					 *   outermost to innermost (lower to
+					 *   higher values).
+					 *
+					 * Values other than @p 0 are not
+					 * necessarily supported.
+					 */
+					uint8_t level;
+					/**
+					 * Geneve option type. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					uint8_t type;
+					/**
+					 * Geneve option class. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					rte_be16_t class_id;
+				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
 			/** Number of bits to skip from a field. */
-- 
2.25.1


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

* [PATCH v2 5/5] ethdev: add MPLS header modification support
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
                     ` (3 preceding siblings ...)
  2023-05-18 17:40   ` [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-18 17:40   ` Michael Baum
  2023-05-21 19:03     ` Ori Kam
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-18 17:40 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.

Since MPLS heaser might appear more the one time in inner/outer/tunnel,
a new field was added to "rte_flow_action_modify_data" structure in
addition to "level" field.
The "tag_index" field is the index of the header inside encapsulation
level. It is used for modify multiple MPLS headers in same encapsulation
level.

This addition enables to modify multiple VLAN headers too, so the
description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c        | 24 ++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst | 12 +++++---
 lib/ethdev/rte_flow.h              | 49 ++++++++++++++++++++----------
 3 files changed, 63 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8c1dea53c0..a51e37276b 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,6 +636,7 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -643,6 +644,7 @@ enum index {
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -859,7 +861,7 @@ static const char *const modify_field_ids[] = {
 	"ipv6_proto",
 	"flex_item",
 	"hash_result",
-	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	NULL
 };
 
@@ -2301,6 +2303,7 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -2310,6 +2313,7 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -6398,6 +6402,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TAG_INDEX] = {
+		.name = "dst_tag_index",
+		.help = "destination field tag array",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
 		.name = "dst_type_id",
 		.help = "destination field type ID",
@@ -6451,6 +6464,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
+		.name = "stc_tag_index",
+		.help = "source field tag array",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
 		.name = "src_type_id",
 		.help = "source field type ID",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index ec812de335..591e43b5ac 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of supported fields.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
-``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array:
+``level`` is used to access any packet field on any encapsulation level:
 
 - ``0`` means the default behaviour. Depending on the packet type,
   it can mean outermost, innermost or anything in between.
@@ -2934,8 +2933,9 @@ as well as any tag element in the tag array:
 - ``2`` and subsequent values requests access to the specified packet
   encapsulation level, from outermost to innermost (lower to higher values).
 
-For the tag array (in case of multiple tags are supported and present)
-``level`` translates directly into the array index.
+``tag_index`` is the index of the header inside encapsulation level.
+It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers which
+multiple of them might be supported in same encapsulation level.
 
 ``type`` is used to specify (along with ``class_id``) the Geneve option which
 is being modified.
@@ -3011,7 +3011,9 @@ and provide immediate value 0xXXXX85XX.
    +=================+==========================================================+
    | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
    +-----------------+----------------------------------------------------------+
-   | ``level``       | encapsulation level of a packet field or tag array index |
+   | ``level``       | encapsulation level of a packet field                    |
+   +-----------------+----------------------------------------------------------+
+   | ``tag_index``   | tag index inside encapsulation level                     |
    +-----------------+----------------------------------------------------------+
    | ``type``        | geneve option type                                       |
    +-----------------+----------------------------------------------------------+
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index f30d4b033f..34e536d662 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
 	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC Address. */
 	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
-	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
-	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier. */
+	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
+	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
 	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
 	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
 	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
@@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
 	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
-	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option data */
+	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
 };
 
 /**
@@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
 	RTE_STD_C11
 	union {
 		struct {
-			/** Encapsulation level or tag index or flex item handle. */
+			/** Encapsulation level and tag index or flex item handle. */
 			union {
 				struct {
 					/**
@@ -3820,20 +3821,36 @@ struct rte_flow_action_modify_data {
 					 *
 					 * Values other than @p 0 are not
 					 * necessarily supported.
+					 *
+					 * @note that for MPLS field,
+					 * encapsulation level also include
+					 * tunnel since MPLS may appear in
+					 * outer, inner or tunnel.
 					 */
 					uint8_t level;
-					/**
-					 * Geneve option type. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					uint8_t type;
-					/**
-					 * Geneve option class. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					rte_be16_t class_id;
+					union {
+						/**
+						 * Tag index array inside
+						 * encapsulation level.
+						 */
+						uint8_t tag_index;
+						/**
+						 * Geneve option identifier.
+						 * relevant only for
+						 * RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+						 * modification type.
+						 */
+						struct {
+							/**
+							 * Geneve option type.
+							 */
+							uint8_t type;
+							/**
+							 * Geneve option class.
+							 */
+							rte_be16_t class_id;
+						};
+					};
 				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
-- 
2.25.1


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

* RE: [PATCH v2 2/5] doc: fix blank line in asynchronous operations description
  2023-05-18 17:40   ` [PATCH v2 2/5] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-21 10:07     ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-21 10:07 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	Alexander Kozyrev, stable

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Thursday, May 18, 2023 8:40 PM
> 
> The asynchronous operations description inside "Generic flow API
> (rte_flow)" documentation, adds some bullets to describe asynchronous
> operations behavior.
> 
> Before the first bullet, miss a blank line causing it to look a regular
> text line.
> 
> This patch adds the blank line.
> 
> Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules
> operations")
> Cc: akozyrev@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index e7faa368a1..76e69190fc 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -3702,6 +3702,7 @@ Asynchronous operations
>  -----------------------
> 
>  Flow rules management can be done via special lockless flow management
> queues.
> +
>  - Queue operations are asynchronous and not thread-safe.
> 
>  - Operations can thus be invoked by the app's datapath,
> --
> 2.25.1

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

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

* RE: [PATCH v2 1/5] doc: fix blank lines in modify field action description
  2023-05-18 17:40   ` [PATCH v2 1/5] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-21 10:07     ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-21 10:07 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	Alexander Kozyrev, stable

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Thursday, May 18, 2023 8:40 PM
> 
> The modify field action description inside "Generic flow API (rte_flow)"
> documentation, lists all operations supported for a destination field.
> In addition, it lists the values supported for a encapsulation level
> field.
> 
> Before the lists, in both cases, miss a blank line causing them to look
> regular text lines.
> 
> This patch adds the blank lines.
> 
> Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
> Cc: akozyrev@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 32fc45516a..e7faa368a1 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2917,20 +2917,23 @@ The immediate value
> ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
>  ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
>  See ``enum rte_flow_field_id`` for the list of supported fields.
> 
> -``op`` selects the operation to perform on a destination field.
> +``op`` selects the operation to perform on a destination field:
> +
>  - ``set`` copies the data from ``src`` field to ``dst`` field.
>  - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
> -- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
> +- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
> 
>  ``width`` defines a number of bits to use from ``src`` field.
> 
>  ``level`` is used to access any packet field on any encapsulation level
> -as well as any tag element in the tag array.
> -- ``0`` means the default behaviour. Depending on the packet type, it can
> -mean outermost, innermost or anything in between.
> +as well as any tag element in the tag array:
> +
> +- ``0`` means the default behaviour. Depending on the packet type,
> +  it can mean outermost, innermost or anything in between.
>  - ``1`` requests access to the outermost packet encapsulation level.
>  - ``2`` and subsequent values requests access to the specified packet
> -encapsulation level, from outermost to innermost (lower to higher values).
> +  encapsulation level, from outermost to innermost (lower to higher
> values).
> +
>  For the tag array (in case of multiple tags are supported and present)
>  ``level`` translates directly into the array index.
> 
> --
> 2.25.1

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

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

* RE: [PATCH v2 3/5] doc: fix wrong indentation in RSS action description
  2023-05-18 17:40   ` [PATCH v2 3/5] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-21 10:08     ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-21 10:08 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	NBU-Contact-Adrien Mazarguil (EXTERNAL),
	stable

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Thursday, May 18, 2023 8:40 PM
> To: dev@dpdk.org
> 
> The RSS action description inside "Generic flow API (rte_flow)"
> documentation, lists the values supported for a encapsulation level
> field.
> 
> For "2" value, it uses 3 spaces as an indentation instead of 2 after
> line breaking, causing the first line to be bold.
> 
> This patch updates the number of spaces in the indentation.
> 
> Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
> Cc: adrien.mazarguil@6wind.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---
>  doc/guides/prog_guide/rte_flow.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index 76e69190fc..25b57bf86d 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
>    level.
> 
>  - ``2`` and subsequent values request RSS to be performed on the specified
> -   inner packet encapsulation level, from outermost to innermost (lower to
> -   higher values).
> +  inner packet encapsulation level, from outermost to innermost (lower to
> +  higher values).
> 
>  Values other than ``0`` are not necessarily supported.
> 
> --
> 2.25.1

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

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

* RE: [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support
  2023-05-18 17:40   ` [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-21 18:52     ` Ori Kam
  0 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-21 18:52 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL)

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Thursday, May 18, 2023 8:40 PM
> 
> Add modify field support for GENEVE option fields:
>  - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
>  - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
>  - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"
> 
> Each GENEVE TLV option is identified by both its "class" and "type", so
> 2 new fields were added to "rte_flow_action_modify_data" structure to
> help specify which option to modify.
> 
> To get room for those 2 new fields, the "level" field move to use
> "uint8_t" which is more than enough for encapsulation level.
> This patch also reduces all modify field encapsulation level "fully
> masked" initializations to use UINT8_MAX instead of UINT32_MAX.
> This change avoids compilation warning caused by this API changing.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---

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

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

* RE: [PATCH v2 5/5] ethdev: add MPLS header modification support
  2023-05-18 17:40   ` [PATCH v2 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-05-21 19:03     ` Ori Kam
  2023-05-22 12:04       ` Michael Baum
  0 siblings, 1 reply; 43+ messages in thread
From: Ori Kam @ 2023-05-21 19:03 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL)

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Thursday, May 18, 2023 8:40 PM
> 
> Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.
> 
> Since MPLS heaser might appear more the one time in inner/outer/tunnel,
> a new field was added to "rte_flow_action_modify_data" structure in
> addition to "level" field.
> The "tag_index" field is the index of the header inside encapsulation
> level. It is used for modify multiple MPLS headers in same encapsulation
> level.
> 
> This addition enables to modify multiple VLAN headers too, so the
> description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> ---

[Snip]

> diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> index ec812de335..591e43b5ac 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of
> supported fields.
> 
>  ``width`` defines a number of bits to use from ``src`` field.
> 
> -``level`` is used to access any packet field on any encapsulation level
> -as well as any tag element in the tag array:
> +``level`` is used to access any packet field on any encapsulation level:
> 
>  - ``0`` means the default behaviour. Depending on the packet type,
>    it can mean outermost, innermost or anything in between.
> @@ -2934,8 +2933,9 @@ as well as any tag element in the tag array:
>  - ``2`` and subsequent values requests access to the specified packet
>    encapsulation level, from outermost to innermost (lower to higher values).
> 
> -For the tag array (in case of multiple tags are supported and present)
> -``level`` translates directly into the array index.
> +``tag_index`` is the index of the header inside encapsulation level.
> +It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers which
> +multiple of them might be supported in same encapsulation level.
> 
This is an API change and must be documented as such in the release notes.
I suggest to avoid breaking working apps that it will be documented that
for this release when using tag the PMD will look at level and tag_id
If both equal to 0 --> the requested tag is zero
If tag_id == 0 and level != 0 PMD will selected the tag based on level, but will output a warning
If tag_id != 0 and level == 0 PMD will select the tag based on the tag_id

>  ``type`` is used to specify (along with ``class_id``) the Geneve option which
>  is being modified.
> @@ -3011,7 +3011,9 @@ and provide immediate value 0xXXXX85XX.
> 
> +=================+============================================
> ==============+
>     | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
>     +-----------------+----------------------------------------------------------+
> -   | ``level``       | encapsulation level of a packet field or tag array index |
> +   | ``level``       | encapsulation level of a packet field                    |
> +   +-----------------+----------------------------------------------------------+
> +   | ``tag_index``   | tag index inside encapsulation level                     |
>     +-----------------+----------------------------------------------------------+
>     | ``type``        | geneve option type                                       |
>     +-----------------+----------------------------------------------------------+
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index f30d4b033f..34e536d662 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
>  	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
>  	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC
> Address. */
>  	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
> -	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
> -	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier.
> */
> +	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
> +	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
>  	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
>  	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
>  	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
> @@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
>  	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
>  	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
>  	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
> -	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
> +	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option
> data */
> +	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
>  };
> 
>  /**
> @@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
>  	RTE_STD_C11
>  	union {
>  		struct {
> -			/** Encapsulation level or tag index or flex item
> handle. */
> +			/** Encapsulation level and tag index or flex item
> handle. */
>  			union {
>  				struct {
>  					/**
> @@ -3820,20 +3821,36 @@ struct rte_flow_action_modify_data {
>  					 *
>  					 * Values other than @p 0 are not
>  					 * necessarily supported.
> +					 *
> +					 * @note that for MPLS field,
> +					 * encapsulation level also include
> +					 * tunnel since MPLS may appear in
> +					 * outer, inner or tunnel.
>  					 */
>  					uint8_t level;
> -					/**
> -					 * Geneve option type. relevant only
> -					 * for
> RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> -					 * modification type.
> -					 */
> -					uint8_t type;
> -					/**
> -					 * Geneve option class. relevant only
> -					 * for
> RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> -					 * modification type.
> -					 */
> -					rte_be16_t class_id;
> +					union {
> +						/**
> +						 * Tag index array inside
> +						 * encapsulation level.
> +						 */

I think it is also worth noting that this is used also for VLAN and MPLS.

> +						uint8_t tag_index;
> +						/**
> +						 * Geneve option identifier.
> +						 * relevant only for
> +						 *
> RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> +						 * modification type.
> +						 */
> +						struct {
> +							/**
> +							 * Geneve option
> type.
> +							 */
> +							uint8_t type;
> +							/**
> +							 * Geneve option
> class.
> +							 */
> +							rte_be16_t class_id;
> +						};
> +					};
>  				};
>  				struct rte_flow_item_flex_handle
> *flex_handle;
>  			};
> --
> 2.25.1


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

* RE: [PATCH v2 5/5] ethdev: add MPLS header modification support
  2023-05-21 19:03     ` Ori Kam
@ 2023-05-22 12:04       ` Michael Baum
  0 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 12:04 UTC (permalink / raw)
  To: Ori Kam, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL)

Hi Ori,

> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Sunday, 21 May 2023 22:03
> 
> Hi Michael,
> 
> > -----Original Message-----
> > From: Michael Baum <michaelba@nvidia.com>
> > Sent: Thursday, May 18, 2023 8:40 PM
> >
> > Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.
> >
> > Since MPLS heaser might appear more the one time in
> > inner/outer/tunnel, a new field was added to
> > "rte_flow_action_modify_data" structure in addition to "level" field.
> > The "tag_index" field is the index of the header inside encapsulation
> > level. It is used for modify multiple MPLS headers in same
> > encapsulation level.
> >
> > This addition enables to modify multiple VLAN headers too, so the
> > description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> > ---
> 
> [Snip]
> 
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index ec812de335..591e43b5ac 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of
> > supported fields.
> >
> >  ``width`` defines a number of bits to use from ``src`` field.
> >
> > -``level`` is used to access any packet field on any encapsulation
> > level -as well as any tag element in the tag array:
> > +``level`` is used to access any packet field on any encapsulation level:
> >
> >  - ``0`` means the default behaviour. Depending on the packet type,
> >    it can mean outermost, innermost or anything in between.
> > @@ -2934,8 +2933,9 @@ as well as any tag element in the tag array:
> >  - ``2`` and subsequent values requests access to the specified packet
> >    encapsulation level, from outermost to innermost (lower to higher values).
> >
> > -For the tag array (in case of multiple tags are supported and
> > present) -``level`` translates directly into the array index.
> > +``tag_index`` is the index of the header inside encapsulation level.
> > +It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers
> > +which multiple of them might be supported in same encapsulation level.
> >
> This is an API change and must be documented as such in the release notes.

OK, I'll add it to release notes and send v3.

> I suggest to avoid breaking working apps that it will be documented that for this
> release when using tag the PMD will look at level and tag_id If both equal to 0 -->
> the requested tag is zero If tag_id == 0 and level != 0 PMD will selected the tag
> based on level, but will output a warning If tag_id != 0 and level == 0 PMD will
> select the tag based on the tag_id

I'm preparing patch for it in MLX5 PMD (the only PMD which uses "RTE_FLOW_FIELD_TAG").
The behavior for "RTE_FLOW_FIELD_TAG" type is:
1. (level == 0 && tag_index == 0) -> the requested tag is zero.
2. (level == 0 && tag_index != 0) -> the requested tag is "tag_index".
3. (level != 0 && tag_index == 0) -> the requested tag is "level" + warning.
4. (level != 0 && tag_index != 0) -> action creation failed + error.

My question is where to document it? In rte_flow.rst or for PMD doc?

> >  ``type`` is used to specify (along with ``class_id``) the Geneve
> > option which  is being modified.
> > @@ -3011,7 +3011,9 @@ and provide immediate value 0xXXXX85XX.
> >
> > +=================+============================================
> > ==============+
> >     | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
> >     +-----------------+----------------------------------------------------------+
> > -   | ``level``       | encapsulation level of a packet field or tag array index |
> > +   | ``level``       | encapsulation level of a packet field                    |
> > +   +-----------------+----------------------------------------------------------+
> > +   | ``tag_index``   | tag index inside encapsulation level                     |
> >     +-----------------+----------------------------------------------------------+
> >     | ``type``        | geneve option type                                       |
> >
> > +-----------------+---------------------------------------------------
> > -------+ diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> > index f30d4b033f..34e536d662 100644
> > --- a/lib/ethdev/rte_flow.h
> > +++ b/lib/ethdev/rte_flow.h
> > @@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
> >  	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
> >  	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC
> > Address. */
> >  	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
> > -	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
> > -	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier.
> > */
> > +	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
> > +	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
> >  	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
> >  	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
> >  	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
> > @@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
> >  	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
> >  	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
> >  	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
> > -	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
> > +	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option
> > data */
> > +	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
> >  };
> >
> >  /**
> > @@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
> >  	RTE_STD_C11
> >  	union {
> >  		struct {
> > -			/** Encapsulation level or tag index or flex item
> > handle. */
> > +			/** Encapsulation level and tag index or flex item
> > handle. */
> >  			union {
> >  				struct {
> >  					/**
> > @@ -3820,20 +3821,36 @@ struct rte_flow_action_modify_data {
> >  					 *
> >  					 * Values other than @p 0 are not
> >  					 * necessarily supported.
> > +					 *
> > +					 * @note that for MPLS field,
> > +					 * encapsulation level also include
> > +					 * tunnel since MPLS may appear in
> > +					 * outer, inner or tunnel.
> >  					 */
> >  					uint8_t level;
> > -					/**
> > -					 * Geneve option type. relevant only
> > -					 * for
> > RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> > -					 * modification type.
> > -					 */
> > -					uint8_t type;
> > -					/**
> > -					 * Geneve option class. relevant only
> > -					 * for
> > RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> > -					 * modification type.
> > -					 */
> > -					rte_be16_t class_id;
> > +					union {
> > +						/**
> > +						 * Tag index array inside
> > +						 * encapsulation level.
> > +						 */
> 
> I think it is also worth noting that this is used also for VLAN and MPLS.

OK

> 
> > +						uint8_t tag_index;
> > +						/**
> > +						 * Geneve option identifier.
> > +						 * relevant only for
> > +						 *
> > RTE_FLOW_FIELD_GENEVE_OPT_XXXX
> > +						 * modification type.
> > +						 */
> > +						struct {
> > +							/**
> > +							 * Geneve option
> > type.
> > +							 */
> > +							uint8_t type;
> > +							/**
> > +							 * Geneve option
> > class.
> > +							 */
> > +							rte_be16_t class_id;
> > +						};
> > +					};
> >  				};
> >  				struct rte_flow_item_flex_handle
> > *flex_handle;
> >  			};
> > --
> > 2.25.1


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

* [PATCH v3 0/5] ethdev: modify field API for multiple headers
  2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
                     ` (4 preceding siblings ...)
  2023-05-18 17:40   ` [PATCH v2 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-05-22 19:27   ` Michael Baum
  2023-05-22 19:28     ` [PATCH v3 1/5] doc: fix blank lines in modify field action description Michael Baum
                       ` (6 more replies)
  5 siblings, 7 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:27 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

This patch-set extend the modify field action API to support both
multiple MPLS and GENEVE option headers.

In current API, the header type is provided by rte_flow_field_id
enumeration and the encapsulation level (inner/outer/tunnel) is
specified by data.level field.
However, there is no way to specify header inside encapsulation level.

For example, for this packet:

eth / mpls / mpls / mpls / ipv4 / udp
the both second and third MPLS headers cannot be modified using this
API.

RFC:
https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/

v2:
 - Change "sub_level" name to "tag_index".
 - Squash PMD changes into API changes patch.
 - Remove PMD private patch from the patch-set.

v3:
 - Add TAG array API change to release notes.
 - Improve comment and documentation.

Michael Baum (5):
  doc: fix blank lines in modify field action description
  doc: fix blank line in asynchronous operations description
  doc: fix wrong indentation in RSS action description
  ethdev: add GENEVE TLV option modification support
  ethdev: add MPLS header modification support

 app/test-pmd/cmdline_flow.c            | 70 +++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 59 ++++++++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  7 +++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++----
 lib/ethdev/rte_flow.h                  | 73 ++++++++++++++++++++++++--
 5 files changed, 203 insertions(+), 28 deletions(-)

-- 
2.25.1


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

* [PATCH v3 1/5] doc: fix blank lines in modify field action description
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
@ 2023-05-22 19:28     ` Michael Baum
  2023-05-22 19:28     ` [PATCH v3 2/5] doc: fix blank line in asynchronous operations description Michael Baum
                       ` (5 subsequent siblings)
  6 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:28 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The modify field action description inside "Generic flow API (rte_flow)"
documentation, lists all operations supported for a destination field.
In addition, it lists the values supported for a encapsulation level
field.

Before the lists, in both cases, miss a blank line causing them to look
regular text lines.

This patch adds the blank lines.

Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..e7faa368a1 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2917,20 +2917,23 @@ The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
 ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
 See ``enum rte_flow_field_id`` for the list of supported fields.
 
-``op`` selects the operation to perform on a destination field.
+``op`` selects the operation to perform on a destination field:
+
 - ``set`` copies the data from ``src`` field to ``dst`` field.
 - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
-- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
+- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
 ``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array.
-- ``0`` means the default behaviour. Depending on the packet type, it can
-mean outermost, innermost or anything in between.
+as well as any tag element in the tag array:
+
+- ``0`` means the default behaviour. Depending on the packet type,
+  it can mean outermost, innermost or anything in between.
 - ``1`` requests access to the outermost packet encapsulation level.
 - ``2`` and subsequent values requests access to the specified packet
-encapsulation level, from outermost to innermost (lower to higher values).
+  encapsulation level, from outermost to innermost (lower to higher values).
+
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
-- 
2.25.1


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

* [PATCH v3 2/5] doc: fix blank line in asynchronous operations description
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-22 19:28     ` [PATCH v3 1/5] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-22 19:28     ` Michael Baum
  2023-05-22 19:28     ` [PATCH v3 3/5] doc: fix wrong indentation in RSS action description Michael Baum
                       ` (4 subsequent siblings)
  6 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:28 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The asynchronous operations description inside "Generic flow API
(rte_flow)" documentation, adds some bullets to describe asynchronous
operations behavior.

Before the first bullet, miss a blank line causing it to look a regular
text line.

This patch adds the blank line.

Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules operations")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e7faa368a1..76e69190fc 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3702,6 +3702,7 @@ Asynchronous operations
 -----------------------
 
 Flow rules management can be done via special lockless flow management queues.
+
 - Queue operations are asynchronous and not thread-safe.
 
 - Operations can thus be invoked by the app's datapath,
-- 
2.25.1


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

* [PATCH v3 3/5] doc: fix wrong indentation in RSS action description
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-22 19:28     ` [PATCH v3 1/5] doc: fix blank lines in modify field action description Michael Baum
  2023-05-22 19:28     ` [PATCH v3 2/5] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-22 19:28     ` Michael Baum
  2023-05-22 19:28     ` [PATCH v3 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
                       ` (3 subsequent siblings)
  6 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:28 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	adrien.mazarguil, stable

The RSS action description inside "Generic flow API (rte_flow)"
documentation, lists the values supported for a encapsulation level
field.

For "2" value, it uses 3 spaces as an indentation instead of 2 after
line breaking, causing the first line to be bold.

This patch updates the number of spaces in the indentation.

Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 76e69190fc..25b57bf86d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
   level.
 
 - ``2`` and subsequent values request RSS to be performed on the specified
-   inner packet encapsulation level, from outermost to innermost (lower to
-   higher values).
+  inner packet encapsulation level, from outermost to innermost (lower to
+  higher values).
 
 Values other than ``0`` are not necessarily supported.
 
-- 
2.25.1


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

* [PATCH v3 4/5] ethdev: add GENEVE TLV option modification support
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
                       ` (2 preceding siblings ...)
  2023-05-22 19:28     ` [PATCH v3 3/5] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-22 19:28     ` Michael Baum
  2023-05-22 19:28     ` [PATCH v3 5/5] ethdev: add MPLS header " Michael Baum
                       ` (2 subsequent siblings)
  6 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:28 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add modify field support for GENEVE option fields:
 - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
 - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
 - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"

Each GENEVE TLV option is identified by both its "class" and "type", so
2 new fields were added to "rte_flow_action_modify_data" structure to
help specify which option to modify.

To get room for those 2 new fields, the "level" field move to use
"uint8_t" which is more than enough for encapsulation level.
This patch also reduces all modify field encapsulation level "fully
masked" initializations to use UINT8_MAX instead of UINT32_MAX.
This change avoids compilation warning caused by this API changing.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 48 +++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 23 ++++++++++++
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++++------
 lib/ethdev/rte_flow.h                  | 48 +++++++++++++++++++++++++-
 5 files changed, 131 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 58939ec321..8c1dea53c0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,11 +636,15 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -854,7 +858,9 @@ static const char *const modify_field_ids[] = {
 	"ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color",
 	"ipv6_proto",
 	"flex_item",
-	"hash_result", NULL
+	"hash_result",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	NULL
 };
 
 static const char *const meter_colors[] = {
@@ -2295,6 +2301,8 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ZERO,
@@ -2302,6 +2310,8 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -6388,6 +6398,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
+		.name = "dst_type_id",
+		.help = "destination field type ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_DST_CLASS_ID] = {
+		.name = "dst_class",
+		.help = "destination field class ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     dst.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_OFFSET] = {
 		.name = "dst_offset",
 		.help = "destination field bit offset",
@@ -6423,6 +6451,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
+		.name = "src_type_id",
+		.help = "source field type ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_SRC_CLASS_ID] = {
+		.name = "src_class",
+		.help = "source field class ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     src.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_OFFSET] = {
 		.name = "src_offset",
 		.help = "source field bit offset",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 25b57bf86d..ec812de335 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,14 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+``type`` is used to specify (along with ``class_id``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
+``class_id`` is used to specify (along with ``type``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
 ``flex_handle`` is used to specify the flex item pointer which is being
 modified. ``flex_handle`` and ``level`` are mutually exclusive.
 
@@ -2967,6 +2975,17 @@ to replace the third byte of MAC address with value 0x85, application should
 specify destination width as 8, destination offset as 16, and provide immediate
 value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
 
+The ``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` type supports modifying only one DW in
+single action and align to 32 bits. For example, for modifying 16 bits start
+from offset 24, 2 different actions should be prepared. The first one includs
+``offset=24`` and ``width=8``, and the seconde one includs ``offset=32`` and
+``width=8``.
+Application should provide the data in immediate value memory only for the
+single DW even though the offset is related to start of first DW. For example,
+to replace the third byte of second DW in Geneve option data with value 0x85,
+application should specify destination width as 8, destination offset as 48,
+and provide immediate value 0xXXXX85XX.
+
 .. _table_rte_flow_action_modify_field:
 
 .. table:: MODIFY_FIELD
@@ -2994,6 +3013,10 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``type``        | geneve option type                                       |
+   +-----------------+----------------------------------------------------------+
+   | ``class_id``    | geneve option class ID                                   |
+   +-----------------+----------------------------------------------------------+
    | ``flex_handle`` | flex item handle of a packet field                       |
    +-----------------+----------------------------------------------------------+
    | ``offset``      | number of bits to skip at the beginning                  |
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..ce1755096f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,6 +84,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The ``level`` field in experimental structure
+  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+
 
 ABI Changes
 -----------
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 7e0ee8d883..1b68a19900 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3565,7 +3565,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
-	if (mask_conf->dst.level != UINT32_MAX)
+	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
 			"destination encapsulation level must be fully masked");
@@ -3579,7 +3579,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (mask_conf->src.level != UINT32_MAX)
+		if (mask_conf->src.level != UINT8_MAX)
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source encapsulation level must be fully masked");
@@ -4450,7 +4450,7 @@ flow_hw_set_vlan_vid(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = RTE_FLOW_FIELD_VLAN_ID,
-			.level = 0xffffffff, .offset = 0xffffffff,
+			.level = 0xff, .offset = 0xffffffff,
 		},
 		.src = {
 			.field = RTE_FLOW_FIELD_VALUE,
@@ -4583,12 +4583,12 @@ flow_hw_actions_template_create(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -5653,7 +5653,7 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -5677,12 +5677,12 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -6009,7 +6009,7 @@ flow_hw_create_ctrl_regc_jump_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -6182,12 +6182,12 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..f30d4b033f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3773,6 +3773,9 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_PROTO,	/**< IPv6 next header. */
 	RTE_FLOW_FIELD_FLEX_ITEM,	/**< Flex item. */
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
+	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
+	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
 };
 
 /**
@@ -3788,7 +3791,50 @@ struct rte_flow_action_modify_data {
 		struct {
 			/** Encapsulation level or tag index or flex item handle. */
 			union {
-				uint32_t level;
+				struct {
+					/**
+					 * Packet encapsulation level containing
+					 * the field modify to.
+					 *
+					 * - @p 0 requests the default behavior.
+					 *   Depending on the packet type, it
+					 *   can mean outermost, innermost or
+					 *   anything in between.
+					 *
+					 *   It basically stands for the
+					 *   innermost encapsulation level
+					 *   modification can be performed on
+					 *   according to PMD and device
+					 *   capabilities.
+					 *
+					 * - @p 1 requests modification to be
+					 *   performed on the outermost packet
+					 *   encapsulation level.
+					 *
+					 * - @p 2 and subsequent values request
+					 *   modification to be performed on
+					 *   the specified inner packet
+					 *   encapsulation level, from
+					 *   outermost to innermost (lower to
+					 *   higher values).
+					 *
+					 * Values other than @p 0 are not
+					 * necessarily supported.
+					 */
+					uint8_t level;
+					/**
+					 * Geneve option type. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					uint8_t type;
+					/**
+					 * Geneve option class. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					rte_be16_t class_id;
+				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
 			/** Number of bits to skip from a field. */
-- 
2.25.1


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

* [PATCH v3 5/5] ethdev: add MPLS header modification support
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
                       ` (3 preceding siblings ...)
  2023-05-22 19:28     ` [PATCH v3 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-22 19:28     ` Michael Baum
  2023-05-23 10:40     ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Ori Kam
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
  6 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-22 19:28 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.

Since MPLS heaser might appear more the one time in inner/outer/tunnel,
a new field was added to "rte_flow_action_modify_data" structure in
addition to "level" field.
The "tag_index" field is the index of the header inside encapsulation
level. It is used for modify multiple MPLS headers in same encapsulation
level.

This addition enables to modify multiple VLAN headers too, so the
description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 24 +++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 18 ++++++---
 doc/guides/rel_notes/release_23_07.rst |  8 +++-
 lib/ethdev/rte_flow.h                  | 51 ++++++++++++++++++--------
 4 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8c1dea53c0..a51e37276b 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,6 +636,7 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -643,6 +644,7 @@ enum index {
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -859,7 +861,7 @@ static const char *const modify_field_ids[] = {
 	"ipv6_proto",
 	"flex_item",
 	"hash_result",
-	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	NULL
 };
 
@@ -2301,6 +2303,7 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -2310,6 +2313,7 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -6398,6 +6402,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TAG_INDEX] = {
+		.name = "dst_tag_index",
+		.help = "destination field tag array",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
 		.name = "dst_type_id",
 		.help = "destination field type ID",
@@ -6451,6 +6464,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
+		.name = "stc_tag_index",
+		.help = "source field tag array",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
 		.name = "src_type_id",
 		.help = "source field type ID",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index ec812de335..e4328e7ed6 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of supported fields.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
-``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array:
+``level`` is used to access any packet field on any encapsulation level:
 
 - ``0`` means the default behaviour. Depending on the packet type,
   it can mean outermost, innermost or anything in between.
@@ -2934,8 +2933,15 @@ as well as any tag element in the tag array:
 - ``2`` and subsequent values requests access to the specified packet
   encapsulation level, from outermost to innermost (lower to higher values).
 
-For the tag array (in case of multiple tags are supported and present)
-``level`` translates directly into the array index.
+``tag_index`` is the index of the header inside encapsulation level.
+It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers which
+multiple of them might be supported in same encapsulation level.
+
+.. note::
+
+   For ``RTE_FLOW_FIELD_TAG`` type, the tag array was provided in ``level``
+   field and it is still supported for backwards compatibility.
+   When ``tag_index`` is zero, the tag array is taken from ``level`` field.
 
 ``type`` is used to specify (along with ``class_id``) the Geneve option which
 is being modified.
@@ -3011,7 +3017,9 @@ and provide immediate value 0xXXXX85XX.
    +=================+==========================================================+
    | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
    +-----------------+----------------------------------------------------------+
-   | ``level``       | encapsulation level of a packet field or tag array index |
+   | ``level``       | encapsulation level of a packet field                    |
+   +-----------------+----------------------------------------------------------+
+   | ``tag_index``   | tag index inside encapsulation level                     |
    +-----------------+----------------------------------------------------------+
    | ``type``        | geneve option type                                       |
    +-----------------+----------------------------------------------------------+
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index ce1755096f..fd3e35eea3 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,8 +84,12 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-* The ``level`` field in experimental structure
-  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+* ethdev: in experimental structure ``struct rte_flow_action_modify_data``:
+
+  * ``level`` field was reduced to 8 bits.
+
+  * ``tag_index`` field replaced ``level`` field in representing tag array for
+    ``RTE_FLOW_FIELD_TAG`` type.
 
 
 ABI Changes
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index f30d4b033f..1df4b49219 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
 	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC Address. */
 	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
-	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
-	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier. */
+	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
+	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
 	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
 	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
 	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
@@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
 	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
-	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option data */
+	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
 };
 
 /**
@@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
 	RTE_STD_C11
 	union {
 		struct {
-			/** Encapsulation level or tag index or flex item handle. */
+			/** Encapsulation level and tag index or flex item handle. */
 			union {
 				struct {
 					/**
@@ -3820,20 +3821,38 @@ struct rte_flow_action_modify_data {
 					 *
 					 * Values other than @p 0 are not
 					 * necessarily supported.
+					 *
+					 * @note that for MPLS field,
+					 * encapsulation level also include
+					 * tunnel since MPLS may appear in
+					 * outer, inner or tunnel.
 					 */
 					uint8_t level;
-					/**
-					 * Geneve option type. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					uint8_t type;
-					/**
-					 * Geneve option class. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					rte_be16_t class_id;
+					union {
+						/**
+						 * Tag index array inside
+						 * encapsulation level.
+						 * Used for VLAN, MPLS or TAG
+						 * types.
+						 */
+						uint8_t tag_index;
+						/**
+						 * Geneve option identifier.
+						 * relevant only for
+						 * RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+						 * modification type.
+						 */
+						struct {
+							/**
+							 * Geneve option type.
+							 */
+							uint8_t type;
+							/**
+							 * Geneve option class.
+							 */
+							rte_be16_t class_id;
+						};
+					};
 				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
-- 
2.25.1


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

* RE: [PATCH v3 0/5] ethdev: modify field API for multiple headers
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
                       ` (4 preceding siblings ...)
  2023-05-22 19:28     ` [PATCH v3 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-05-23 10:40     ` Ori Kam
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
  6 siblings, 0 replies; 43+ messages in thread
From: Ori Kam @ 2023-05-23 10:40 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Aman Singh, Yuying Zhang, Ferruh Yigit,
	NBU-Contact-Thomas Monjalon (EXTERNAL)

Hi Michael,

> -----Original Message-----
> From: Michael Baum <michaelba@nvidia.com>
> Sent: Monday, May 22, 2023 10:28 PM
> To: dev@dpdk.org
> Cc: Ori Kam <orika@nvidia.com>; Aman Singh <aman.deep.singh@intel.com>;
> Yuying Zhang <yuying.zhang@intel.com>; Ferruh Yigit
> <ferruh.yigit@amd.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>
> Subject: [PATCH v3 0/5] ethdev: modify field API for multiple headers
> 
> This patch-set extend the modify field action API to support both
> multiple MPLS and GENEVE option headers.
> 
> In current API, the header type is provided by rte_flow_field_id
> enumeration and the encapsulation level (inner/outer/tunnel) is
> specified by data.level field.
> However, there is no way to specify header inside encapsulation level.
> 
> For example, for this packet:
> 
> eth / mpls / mpls / mpls / ipv4 / udp
> the both second and third MPLS headers cannot be modified using this
> API.
> 
> RFC:
> https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-
> michaelba@nvidia.com/
> 
> v2:
>  - Change "sub_level" name to "tag_index".
>  - Squash PMD changes into API changes patch.
>  - Remove PMD private patch from the patch-set.
> 
> v3:
>  - Add TAG array API change to release notes.
>  - Improve comment and documentation.
> 
> Michael Baum (5):
>   doc: fix blank lines in modify field action description
>   doc: fix blank line in asynchronous operations description
>   doc: fix wrong indentation in RSS action description
>   ethdev: add GENEVE TLV option modification support
>   ethdev: add MPLS header modification support
> 
>  app/test-pmd/cmdline_flow.c            | 70 +++++++++++++++++++++++-
>  doc/guides/prog_guide/rte_flow.rst     | 59 ++++++++++++++++-----
>  doc/guides/rel_notes/release_23_07.rst |  7 +++
>  drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++----
>  lib/ethdev/rte_flow.h                  | 73 ++++++++++++++++++++++++--
>  5 files changed, 203 insertions(+), 28 deletions(-)
> 
> --
> 2.25.1

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



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

* [PATCH v4 0/5] ethdev: modify field API for multiple headers
  2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
                       ` (5 preceding siblings ...)
  2023-05-23 10:40     ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Ori Kam
@ 2023-05-23 12:48     ` Michael Baum
  2023-05-23 12:48       ` [PATCH v4 1/5] doc: fix blank lines in modify field action description Michael Baum
                         ` (5 more replies)
  6 siblings, 6 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

This patch-set extend the modify field action API to support both
multiple MPLS and GENEVE option headers.

In current API, the header type is provided by rte_flow_field_id
enumeration and the encapsulation level (inner/outer/tunnel) is
specified by data.level field.
However, there is no way to specify header inside encapsulation level.

For example, for this packet:

eth / mpls / mpls / mpls / ipv4 / udp
the both second and third MPLS headers cannot be modified using this
API.

RFC:
https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/

v2:
 - Change "sub_level" name to "tag_index".
 - Squash PMD changes into API changes patch.
 - Remove PMD private patch from the patch-set.

v3:
 - Add TAG array API change to release notes.
 - Improve comment and documentation. 

v4:
 - Add "Acked-by" labels.
 - Add PMD adjustment for TAG array API change.

Michael Baum (5):
  doc: fix blank lines in modify field action description
  doc: fix blank line in asynchronous operations description
  doc: fix wrong indentation in RSS action description
  ethdev: add GENEVE TLV option modification support
  ethdev: add MPLS header modification support

 app/test-pmd/cmdline_flow.c            | 70 +++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 59 ++++++++++++++++-----
 doc/guides/rel_notes/release_23_07.rst |  7 +++
 drivers/net/mlx5/mlx5_flow.c           | 34 ++++++++++++
 drivers/net/mlx5/mlx5_flow.h           | 23 ++++++++
 drivers/net/mlx5/mlx5_flow_dv.c        | 29 +++++-----
 drivers/net/mlx5/mlx5_flow_hw.c        | 43 +++++++++------
 lib/ethdev/rte_flow.h                  | 73 ++++++++++++++++++++++++--
 8 files changed, 288 insertions(+), 50 deletions(-)

-- 
2.25.1


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

* [PATCH v4 1/5] doc: fix blank lines in modify field action description
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
@ 2023-05-23 12:48       ` Michael Baum
  2023-05-23 12:48       ` [PATCH v4 2/5] doc: fix blank line in asynchronous operations description Michael Baum
                         ` (4 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The modify field action description inside "Generic flow API (rte_flow)"
documentation, lists all operations supported for a destination field.
In addition, it lists the values supported for a encapsulation level
field.

Before the lists, in both cases, miss a blank line causing them to look
regular text lines.

This patch adds the blank lines.

Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..e7faa368a1 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2917,20 +2917,23 @@ The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
 ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
 See ``enum rte_flow_field_id`` for the list of supported fields.
 
-``op`` selects the operation to perform on a destination field.
+``op`` selects the operation to perform on a destination field:
+
 - ``set`` copies the data from ``src`` field to ``dst`` field.
 - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
-- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
+- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
 ``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array.
-- ``0`` means the default behaviour. Depending on the packet type, it can
-mean outermost, innermost or anything in between.
+as well as any tag element in the tag array:
+
+- ``0`` means the default behaviour. Depending on the packet type,
+  it can mean outermost, innermost or anything in between.
 - ``1`` requests access to the outermost packet encapsulation level.
 - ``2`` and subsequent values requests access to the specified packet
-encapsulation level, from outermost to innermost (lower to higher values).
+  encapsulation level, from outermost to innermost (lower to higher values).
+
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
-- 
2.25.1


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

* [PATCH v4 2/5] doc: fix blank line in asynchronous operations description
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
  2023-05-23 12:48       ` [PATCH v4 1/5] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-23 12:48       ` Michael Baum
  2023-05-23 12:48       ` [PATCH v4 3/5] doc: fix wrong indentation in RSS action description Michael Baum
                         ` (3 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The asynchronous operations description inside "Generic flow API
(rte_flow)" documentation, adds some bullets to describe asynchronous
operations behavior.

Before the first bullet, miss a blank line causing it to look a regular
text line.

This patch adds the blank line.

Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules operations")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e7faa368a1..76e69190fc 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3702,6 +3702,7 @@ Asynchronous operations
 -----------------------
 
 Flow rules management can be done via special lockless flow management queues.
+
 - Queue operations are asynchronous and not thread-safe.
 
 - Operations can thus be invoked by the app's datapath,
-- 
2.25.1


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

* [PATCH v4 3/5] doc: fix wrong indentation in RSS action description
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
  2023-05-23 12:48       ` [PATCH v4 1/5] doc: fix blank lines in modify field action description Michael Baum
  2023-05-23 12:48       ` [PATCH v4 2/5] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-23 12:48       ` Michael Baum
  2023-05-23 12:48       ` [PATCH v4 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
                         ` (2 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	adrien.mazarguil, stable

The RSS action description inside "Generic flow API (rte_flow)"
documentation, lists the values supported for a encapsulation level
field.

For "2" value, it uses 3 spaces as an indentation instead of 2 after
line breaking, causing the first line to be bold.

This patch updates the number of spaces in the indentation.

Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 76e69190fc..25b57bf86d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
   level.
 
 - ``2`` and subsequent values request RSS to be performed on the specified
-   inner packet encapsulation level, from outermost to innermost (lower to
-   higher values).
+  inner packet encapsulation level, from outermost to innermost (lower to
+  higher values).
 
 Values other than ``0`` are not necessarily supported.
 
-- 
2.25.1


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

* [PATCH v4 4/5] ethdev: add GENEVE TLV option modification support
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
                         ` (2 preceding siblings ...)
  2023-05-23 12:48       ` [PATCH v4 3/5] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-23 12:48       ` Michael Baum
  2023-05-23 12:48       ` [PATCH v4 5/5] ethdev: add MPLS header " Michael Baum
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add modify field support for GENEVE option fields:
 - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
 - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
 - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"

Each GENEVE TLV option is identified by both its "class" and "type", so
2 new fields were added to "rte_flow_action_modify_data" structure to
help specify which option to modify.

To get room for those 2 new fields, the "level" field move to use
"uint8_t" which is more than enough for encapsulation level.
This patch also reduces all modify field encapsulation level "fully
masked" initializations to use UINT8_MAX instead of UINT32_MAX.
This change avoids compilation warning caused by this API changing.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 48 +++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 23 ++++++++++++
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++++------
 lib/ethdev/rte_flow.h                  | 48 +++++++++++++++++++++++++-
 5 files changed, 131 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 58939ec321..8c1dea53c0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,11 +636,15 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -854,7 +858,9 @@ static const char *const modify_field_ids[] = {
 	"ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color",
 	"ipv6_proto",
 	"flex_item",
-	"hash_result", NULL
+	"hash_result",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	NULL
 };
 
 static const char *const meter_colors[] = {
@@ -2295,6 +2301,8 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ZERO,
@@ -2302,6 +2310,8 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -6388,6 +6398,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
+		.name = "dst_type_id",
+		.help = "destination field type ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_DST_CLASS_ID] = {
+		.name = "dst_class",
+		.help = "destination field class ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     dst.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_OFFSET] = {
 		.name = "dst_offset",
 		.help = "destination field bit offset",
@@ -6423,6 +6451,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
+		.name = "src_type_id",
+		.help = "source field type ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_SRC_CLASS_ID] = {
+		.name = "src_class",
+		.help = "source field class ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     src.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_OFFSET] = {
 		.name = "src_offset",
 		.help = "source field bit offset",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 25b57bf86d..ec812de335 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,14 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+``type`` is used to specify (along with ``class_id``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
+``class_id`` is used to specify (along with ``type``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
 ``flex_handle`` is used to specify the flex item pointer which is being
 modified. ``flex_handle`` and ``level`` are mutually exclusive.
 
@@ -2967,6 +2975,17 @@ to replace the third byte of MAC address with value 0x85, application should
 specify destination width as 8, destination offset as 16, and provide immediate
 value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
 
+The ``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` type supports modifying only one DW in
+single action and align to 32 bits. For example, for modifying 16 bits start
+from offset 24, 2 different actions should be prepared. The first one includs
+``offset=24`` and ``width=8``, and the seconde one includs ``offset=32`` and
+``width=8``.
+Application should provide the data in immediate value memory only for the
+single DW even though the offset is related to start of first DW. For example,
+to replace the third byte of second DW in Geneve option data with value 0x85,
+application should specify destination width as 8, destination offset as 48,
+and provide immediate value 0xXXXX85XX.
+
 .. _table_rte_flow_action_modify_field:
 
 .. table:: MODIFY_FIELD
@@ -2994,6 +3013,10 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``type``        | geneve option type                                       |
+   +-----------------+----------------------------------------------------------+
+   | ``class_id``    | geneve option class ID                                   |
+   +-----------------+----------------------------------------------------------+
    | ``flex_handle`` | flex item handle of a packet field                       |
    +-----------------+----------------------------------------------------------+
    | ``offset``      | number of bits to skip at the beginning                  |
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..ce1755096f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,6 +84,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The ``level`` field in experimental structure
+  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+
 
 ABI Changes
 -----------
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 7e0ee8d883..1b68a19900 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3565,7 +3565,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
-	if (mask_conf->dst.level != UINT32_MAX)
+	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
 			"destination encapsulation level must be fully masked");
@@ -3579,7 +3579,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (mask_conf->src.level != UINT32_MAX)
+		if (mask_conf->src.level != UINT8_MAX)
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source encapsulation level must be fully masked");
@@ -4450,7 +4450,7 @@ flow_hw_set_vlan_vid(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = RTE_FLOW_FIELD_VLAN_ID,
-			.level = 0xffffffff, .offset = 0xffffffff,
+			.level = 0xff, .offset = 0xffffffff,
 		},
 		.src = {
 			.field = RTE_FLOW_FIELD_VALUE,
@@ -4583,12 +4583,12 @@ flow_hw_actions_template_create(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -5653,7 +5653,7 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -5677,12 +5677,12 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -6009,7 +6009,7 @@ flow_hw_create_ctrl_regc_jump_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -6182,12 +6182,12 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..f30d4b033f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3773,6 +3773,9 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_PROTO,	/**< IPv6 next header. */
 	RTE_FLOW_FIELD_FLEX_ITEM,	/**< Flex item. */
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
+	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
+	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
 };
 
 /**
@@ -3788,7 +3791,50 @@ struct rte_flow_action_modify_data {
 		struct {
 			/** Encapsulation level or tag index or flex item handle. */
 			union {
-				uint32_t level;
+				struct {
+					/**
+					 * Packet encapsulation level containing
+					 * the field modify to.
+					 *
+					 * - @p 0 requests the default behavior.
+					 *   Depending on the packet type, it
+					 *   can mean outermost, innermost or
+					 *   anything in between.
+					 *
+					 *   It basically stands for the
+					 *   innermost encapsulation level
+					 *   modification can be performed on
+					 *   according to PMD and device
+					 *   capabilities.
+					 *
+					 * - @p 1 requests modification to be
+					 *   performed on the outermost packet
+					 *   encapsulation level.
+					 *
+					 * - @p 2 and subsequent values request
+					 *   modification to be performed on
+					 *   the specified inner packet
+					 *   encapsulation level, from
+					 *   outermost to innermost (lower to
+					 *   higher values).
+					 *
+					 * Values other than @p 0 are not
+					 * necessarily supported.
+					 */
+					uint8_t level;
+					/**
+					 * Geneve option type. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					uint8_t type;
+					/**
+					 * Geneve option class. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					rte_be16_t class_id;
+				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
 			/** Number of bits to skip from a field. */
-- 
2.25.1


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

* [PATCH v4 5/5] ethdev: add MPLS header modification support
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
                         ` (3 preceding siblings ...)
  2023-05-23 12:48       ` [PATCH v4 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-23 12:48       ` Michael Baum
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 12:48 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.

Since MPLS heaser might appear more the one time in inner/outer/tunnel,
a new field was added to "rte_flow_action_modify_data" structure in
addition to "level" field.
The "tag_index" field is the index of the header inside encapsulation
level. It is used for modify multiple MPLS headers in same encapsulation
level.

This addition enables to modify multiple VLAN headers too, so the
description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.

Since the "tag_index" field is added, the "RTE_FLOW_FIELD_TAG" type
moves to use it for tag array instead of using "level" field.
Using "level" is still supported for backwards compatibility when
"tag_index" field is zero.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 24 +++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 18 ++++++---
 doc/guides/rel_notes/release_23_07.rst |  8 +++-
 drivers/net/mlx5/mlx5_flow.c           | 34 +++++++++++++++++
 drivers/net/mlx5/mlx5_flow.h           | 23 ++++++++++++
 drivers/net/mlx5/mlx5_flow_dv.c        | 29 +++++++--------
 drivers/net/mlx5/mlx5_flow_hw.c        | 21 ++++++++---
 lib/ethdev/rte_flow.h                  | 51 ++++++++++++++++++--------
 8 files changed, 162 insertions(+), 46 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8c1dea53c0..a51e37276b 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,6 +636,7 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -643,6 +644,7 @@ enum index {
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -859,7 +861,7 @@ static const char *const modify_field_ids[] = {
 	"ipv6_proto",
 	"flex_item",
 	"hash_result",
-	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	NULL
 };
 
@@ -2301,6 +2303,7 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -2310,6 +2313,7 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -6398,6 +6402,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TAG_INDEX] = {
+		.name = "dst_tag_index",
+		.help = "destination field tag array",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
 		.name = "dst_type_id",
 		.help = "destination field type ID",
@@ -6451,6 +6464,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
+		.name = "stc_tag_index",
+		.help = "source field tag array",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
 		.name = "src_type_id",
 		.help = "source field type ID",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index ec812de335..e4328e7ed6 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of supported fields.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
-``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array:
+``level`` is used to access any packet field on any encapsulation level:
 
 - ``0`` means the default behaviour. Depending on the packet type,
   it can mean outermost, innermost or anything in between.
@@ -2934,8 +2933,15 @@ as well as any tag element in the tag array:
 - ``2`` and subsequent values requests access to the specified packet
   encapsulation level, from outermost to innermost (lower to higher values).
 
-For the tag array (in case of multiple tags are supported and present)
-``level`` translates directly into the array index.
+``tag_index`` is the index of the header inside encapsulation level.
+It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers which
+multiple of them might be supported in same encapsulation level.
+
+.. note::
+
+   For ``RTE_FLOW_FIELD_TAG`` type, the tag array was provided in ``level``
+   field and it is still supported for backwards compatibility.
+   When ``tag_index`` is zero, the tag array is taken from ``level`` field.
 
 ``type`` is used to specify (along with ``class_id``) the Geneve option which
 is being modified.
@@ -3011,7 +3017,9 @@ and provide immediate value 0xXXXX85XX.
    +=================+==========================================================+
    | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
    +-----------------+----------------------------------------------------------+
-   | ``level``       | encapsulation level of a packet field or tag array index |
+   | ``level``       | encapsulation level of a packet field                    |
+   +-----------------+----------------------------------------------------------+
+   | ``tag_index``   | tag index inside encapsulation level                     |
    +-----------------+----------------------------------------------------------+
    | ``type``        | geneve option type                                       |
    +-----------------+----------------------------------------------------------+
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index ce1755096f..fd3e35eea3 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,8 +84,12 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-* The ``level`` field in experimental structure
-  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+* ethdev: in experimental structure ``struct rte_flow_action_modify_data``:
+
+  * ``level`` field was reduced to 8 bits.
+
+  * ``tag_index`` field replaced ``level`` field in representing tag array for
+    ``RTE_FLOW_FIELD_TAG`` type.
 
 
 ABI Changes
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 19f7f92717..867b7b8ea2 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2318,6 +2318,40 @@ mlx5_validate_action_ct(struct rte_eth_dev *dev,
 	return 0;
 }
 
+/**
+ * Validate the level value for modify field action.
+ *
+ * @param[in] data
+ *   Pointer to the rte_flow_action_modify_data structure either src or dst.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+flow_validate_modify_field_level(const struct rte_flow_action_modify_data *data,
+				 struct rte_flow_error *error)
+{
+	if (data->level == 0)
+		return 0;
+	if (data->field != RTE_FLOW_FIELD_TAG)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "inner header fields modification is not supported");
+	if (data->tag_index != 0)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "tag array can be provided using 'level' or 'tag_index' fields, not both");
+	/*
+	 * The tag array for RTE_FLOW_FIELD_TAG type is provided using
+	 * 'tag_index' field. In old API, it was provided using 'level' field
+	 * and it is still supported for backwards compatibility.
+	 */
+	DRV_LOG(WARNING, "tag array provided in 'level' field instead of 'tag_index' field.");
+	return 0;
+}
+
 /**
  * Validate ICMP6 item.
  *
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 1d116ea0f6..cba04b4f45 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1045,6 +1045,26 @@ flow_items_to_tunnel(const struct rte_flow_item items[])
 	return items[0].spec;
 }
 
+/**
+ * Gets the tag array given for RTE_FLOW_FIELD_TAG type.
+ *
+ * In old API the value was provided in "level" field, but in new API
+ * it is provided in "tag_array" field. Since encapsulation level is not
+ * relevant for metadata, the tag array can be still provided in "level"
+ * for backwards compatibility.
+ *
+ * @param[in] data
+ *   Pointer to tag modify data structure.
+ *
+ * @return
+ *   Tag array index.
+ */
+static inline uint8_t
+flow_tag_index_get(const struct rte_flow_action_modify_data *data)
+{
+	return data->tag_index ? data->tag_index : data->level;
+}
+
 /**
  * Fetch 1, 2, 3 or 4 byte field from the byte array
  * and return as unsigned integer in host-endian format.
@@ -2276,6 +2296,9 @@ int mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
 int mlx5_flow_validate_action_default_miss(uint64_t action_flags,
 				const struct rte_flow_attr *attr,
 				struct rte_flow_error *error);
+int flow_validate_modify_field_level
+			(const struct rte_flow_action_modify_data *data,
+			 struct rte_flow_error *error);
 int mlx5_flow_item_acceptable(const struct rte_flow_item *item,
 			      const uint8_t *mask,
 			      const uint8_t *nic_mask,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f136f43b0a..729962a488 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1896,16 +1896,17 @@ mlx5_flow_field_id_to_modify_info
 	case RTE_FLOW_FIELD_TAG:
 		{
 			MLX5_ASSERT(data->offset + width <= 32);
+			uint8_t tag_index = flow_tag_index_get(data);
 			int reg;
 
-			off_be = (data->level == MLX5_LINEAR_HASH_TAG_INDEX) ?
+			off_be = (tag_index == MLX5_LINEAR_HASH_TAG_INDEX) ?
 				 16 - (data->offset + width) + 16 : data->offset;
 			if (priv->sh->config.dv_flow_en == 2)
 				reg = flow_hw_get_reg_id(RTE_FLOW_ITEM_TYPE_TAG,
-							 data->level);
+							 tag_index);
 			else
 				reg = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG,
-							   data->level, error);
+							   tag_index, error);
 			if (reg < 0)
 				return;
 			MLX5_ASSERT(reg != REG_NON);
@@ -1985,7 +1986,7 @@ mlx5_flow_field_id_to_modify_info
 		{
 			uint32_t meta_mask = priv->sh->dv_meta_mask;
 			uint32_t meta_count = __builtin_popcount(meta_mask);
-			uint32_t reg = data->level;
+			uint8_t reg = flow_tag_index_get(data);
 
 			RTE_SET_USED(meta_count);
 			MLX5_ASSERT(data->offset + width <= meta_count);
@@ -5250,6 +5251,14 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 	uint32_t dst_width, src_width;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
+	if (ret)
+		return ret;
+	ret = flow_validate_modify_field_level(&action_modify_field->dst,
+					       error);
+	if (ret)
+		return ret;
+	ret = flow_validate_modify_field_level(&action_modify_field->src,
+					       error);
 	if (ret)
 		return ret;
 	if (action_modify_field->src.field == RTE_FLOW_FIELD_FLEX_ITEM ||
@@ -5279,12 +5288,6 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"destination offset is too big");
-		if (action_modify_field->dst.level &&
-		    action_modify_field->dst.field != RTE_FLOW_FIELD_TAG)
-			return rte_flow_error_set(error, ENOTSUP,
-					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"inner header fields modification"
-					" is not supported");
 	}
 	if (action_modify_field->src.field != RTE_FLOW_FIELD_VALUE &&
 	    action_modify_field->src.field != RTE_FLOW_FIELD_POINTER) {
@@ -5298,12 +5301,6 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"source offset is too big");
-		if (action_modify_field->src.level &&
-		    action_modify_field->src.field != RTE_FLOW_FIELD_TAG)
-			return rte_flow_error_set(error, ENOTSUP,
-					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"inner header fields modification"
-					" is not supported");
 	}
 	if ((action_modify_field->dst.field ==
 	     action_modify_field->src.field) &&
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 1b68a19900..e55e3d6c1a 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1022,9 +1022,11 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 		    conf->dst.field == RTE_FLOW_FIELD_TAG ||
 		    conf->dst.field == RTE_FLOW_FIELD_METER_COLOR ||
 		    conf->dst.field == (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
+			uint8_t tag_index = flow_tag_index_get(&conf->dst);
+
 			value = *(const unaligned_uint32_t *)item.spec;
 			if (conf->dst.field == RTE_FLOW_FIELD_TAG &&
-			    conf->dst.level == MLX5_LINEAR_HASH_TAG_INDEX)
+			    tag_index == MLX5_LINEAR_HASH_TAG_INDEX)
 				value = rte_cpu_to_be_32(value << 16);
 			else
 				value = rte_cpu_to_be_32(value);
@@ -2055,9 +2057,11 @@ flow_hw_modify_field_construct(struct mlx5_hw_q_job *job,
 	    mhdr_action->dst.field == RTE_FLOW_FIELD_TAG ||
 	    mhdr_action->dst.field == RTE_FLOW_FIELD_METER_COLOR ||
 	    mhdr_action->dst.field == (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
+		uint8_t tag_index = flow_tag_index_get(&mhdr_action->dst);
+
 		value_p = (unaligned_uint32_t *)values;
 		if (mhdr_action->dst.field == RTE_FLOW_FIELD_TAG &&
-		    mhdr_action->dst.level == MLX5_LINEAR_HASH_TAG_INDEX)
+		    tag_index == MLX5_LINEAR_HASH_TAG_INDEX)
 			*value_p = rte_cpu_to_be_32(*value_p << 16);
 		else
 			*value_p = rte_cpu_to_be_32(*value_p);
@@ -3546,11 +3550,16 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				     const struct rte_flow_action *mask,
 				     struct rte_flow_error *error)
 {
-	const struct rte_flow_action_modify_field *action_conf =
-		action->conf;
-	const struct rte_flow_action_modify_field *mask_conf =
-		mask->conf;
+	const struct rte_flow_action_modify_field *action_conf = action->conf;
+	const struct rte_flow_action_modify_field *mask_conf = mask->conf;
+	int ret;
 
+	ret = flow_validate_modify_field_level(&action_conf->dst, error);
+	if (ret)
+		return ret;
+	ret = flow_validate_modify_field_level(&action_conf->src, error);
+	if (ret)
+		return ret;
 	if (action_conf->operation != mask_conf->operation)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index f30d4b033f..1df4b49219 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
 	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC Address. */
 	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
-	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
-	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier. */
+	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
+	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
 	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
 	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
 	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
@@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
 	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
-	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option data */
+	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
 };
 
 /**
@@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
 	RTE_STD_C11
 	union {
 		struct {
-			/** Encapsulation level or tag index or flex item handle. */
+			/** Encapsulation level and tag index or flex item handle. */
 			union {
 				struct {
 					/**
@@ -3820,20 +3821,38 @@ struct rte_flow_action_modify_data {
 					 *
 					 * Values other than @p 0 are not
 					 * necessarily supported.
+					 *
+					 * @note that for MPLS field,
+					 * encapsulation level also include
+					 * tunnel since MPLS may appear in
+					 * outer, inner or tunnel.
 					 */
 					uint8_t level;
-					/**
-					 * Geneve option type. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					uint8_t type;
-					/**
-					 * Geneve option class. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					rte_be16_t class_id;
+					union {
+						/**
+						 * Tag index array inside
+						 * encapsulation level.
+						 * Used for VLAN, MPLS or TAG
+						 * types.
+						 */
+						uint8_t tag_index;
+						/**
+						 * Geneve option identifier.
+						 * relevant only for
+						 * RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+						 * modification type.
+						 */
+						struct {
+							/**
+							 * Geneve option type.
+							 */
+							uint8_t type;
+							/**
+							 * Geneve option class.
+							 */
+							rte_be16_t class_id;
+						};
+					};
 				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
-- 
2.25.1


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

* [PATCH v5 0/5] ethdev: modify field API for multiple headers
  2023-05-23 12:48     ` [PATCH v4 " Michael Baum
                         ` (4 preceding siblings ...)
  2023-05-23 12:48       ` [PATCH v4 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-05-23 21:31       ` Michael Baum
  2023-05-23 21:31         ` [PATCH v5 1/5] doc: fix blank lines in modify field action description Michael Baum
                           ` (5 more replies)
  5 siblings, 6 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

This patch-set extend the modify field action API to support both
multiple MPLS and GENEVE option headers.

In current API, the header type is provided by rte_flow_field_id
enumeration and the encapsulation level (inner/outer/tunnel) is
specified by data.level field.
However, there is no way to specify header inside encapsulation level.

For example, for this packet:

eth / mpls / mpls / mpls / ipv4 / udp
the both second and third MPLS headers cannot be modified using this
API.

RFC:
https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/

v2:
 - Change "sub_level" name to "tag_index".
 - Squash PMD changes into API changes patch.
 - Remove PMD private patch from the patch-set.

v3:
 - Add TAG array API change to release notes.
 - Improve comment and documentation.

v4:
 - Add "Acked-by" labels.
 - Add PMD adjustment for TAG array API change. 

v5:
 - Fix location of level validation.

Michael Baum (5):
  doc: fix blank lines in modify field action description
  doc: fix blank line in asynchronous operations description
  doc: fix wrong indentation in RSS action description
  ethdev: add GENEVE TLV option modification support
  ethdev: add MPLS header modification support

 app/test-pmd/cmdline_flow.c            |  70 +++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     |  59 ++++++++++---
 doc/guides/rel_notes/release_23_07.rst |   7 ++
 drivers/net/mlx5/mlx5_flow.c           |  34 ++++++++
 drivers/net/mlx5/mlx5_flow.h           |  23 ++++++
 drivers/net/mlx5/mlx5_flow_dv.c        | 110 +++++++++++--------------
 drivers/net/mlx5/mlx5_flow_hw.c        |  43 ++++++----
 lib/ethdev/rte_flow.h                  |  73 +++++++++++++++-
 8 files changed, 325 insertions(+), 94 deletions(-)

-- 
2.25.1


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

* [PATCH v5 1/5] doc: fix blank lines in modify field action description
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
@ 2023-05-23 21:31         ` Michael Baum
  2023-05-23 21:31         ` [PATCH v5 2/5] doc: fix blank line in asynchronous operations description Michael Baum
                           ` (4 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The modify field action description inside "Generic flow API (rte_flow)"
documentation, lists all operations supported for a destination field.
In addition, it lists the values supported for a encapsulation level
field.

Before the lists, in both cases, miss a blank line causing them to look
regular text lines.

This patch adds the blank lines.

Fixes: 73b68f4c54a0 ("ethdev: introduce generic modify flow action")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 32fc45516a..e7faa368a1 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2917,20 +2917,23 @@ The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
 ``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
 See ``enum rte_flow_field_id`` for the list of supported fields.
 
-``op`` selects the operation to perform on a destination field.
+``op`` selects the operation to perform on a destination field:
+
 - ``set`` copies the data from ``src`` field to ``dst`` field.
 - ``add`` adds together ``dst`` and ``src`` and stores the result into ``dst``.
-- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``
+- ``sub`` subtracts ``src`` from ``dst`` and stores the result into ``dst``.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
 ``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array.
-- ``0`` means the default behaviour. Depending on the packet type, it can
-mean outermost, innermost or anything in between.
+as well as any tag element in the tag array:
+
+- ``0`` means the default behaviour. Depending on the packet type,
+  it can mean outermost, innermost or anything in between.
 - ``1`` requests access to the outermost packet encapsulation level.
 - ``2`` and subsequent values requests access to the specified packet
-encapsulation level, from outermost to innermost (lower to higher values).
+  encapsulation level, from outermost to innermost (lower to higher values).
+
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
-- 
2.25.1


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

* [PATCH v5 2/5] doc: fix blank line in asynchronous operations description
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-23 21:31         ` [PATCH v5 1/5] doc: fix blank lines in modify field action description Michael Baum
@ 2023-05-23 21:31         ` Michael Baum
  2023-05-23 21:31         ` [PATCH v5 3/5] doc: fix wrong indentation in RSS action description Michael Baum
                           ` (3 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	akozyrev, stable

The asynchronous operations description inside "Generic flow API
(rte_flow)" documentation, adds some bullets to describe asynchronous
operations behavior.

Before the first bullet, miss a blank line causing it to look a regular
text line.

This patch adds the blank line.

Fixes: 197e820c6685 ("ethdev: bring in async queue-based flow rules operations")
Cc: akozyrev@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index e7faa368a1..76e69190fc 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -3702,6 +3702,7 @@ Asynchronous operations
 -----------------------
 
 Flow rules management can be done via special lockless flow management queues.
+
 - Queue operations are asynchronous and not thread-safe.
 
 - Operations can thus be invoked by the app's datapath,
-- 
2.25.1


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

* [PATCH v5 3/5] doc: fix wrong indentation in RSS action description
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
  2023-05-23 21:31         ` [PATCH v5 1/5] doc: fix blank lines in modify field action description Michael Baum
  2023-05-23 21:31         ` [PATCH v5 2/5] doc: fix blank line in asynchronous operations description Michael Baum
@ 2023-05-23 21:31         ` Michael Baum
  2023-05-23 21:31         ` [PATCH v5 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
                           ` (2 subsequent siblings)
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev
  Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon,
	adrien.mazarguil, stable

The RSS action description inside "Generic flow API (rte_flow)"
documentation, lists the values supported for a encapsulation level
field.

For "2" value, it uses 3 spaces as an indentation instead of 2 after
line breaking, causing the first line to be bold.

This patch updates the number of spaces in the indentation.

Fixes: 18aee2861a1f ("ethdev: add encap level to RSS flow API action")
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/prog_guide/rte_flow.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 76e69190fc..25b57bf86d 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1954,8 +1954,8 @@ Also, regarding packet encapsulation ``level``:
   level.
 
 - ``2`` and subsequent values request RSS to be performed on the specified
-   inner packet encapsulation level, from outermost to innermost (lower to
-   higher values).
+  inner packet encapsulation level, from outermost to innermost (lower to
+  higher values).
 
 Values other than ``0`` are not necessarily supported.
 
-- 
2.25.1


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

* [PATCH v5 4/5] ethdev: add GENEVE TLV option modification support
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
                           ` (2 preceding siblings ...)
  2023-05-23 21:31         ` [PATCH v5 3/5] doc: fix wrong indentation in RSS action description Michael Baum
@ 2023-05-23 21:31         ` Michael Baum
  2023-05-23 21:31         ` [PATCH v5 5/5] ethdev: add MPLS header " Michael Baum
  2023-06-01 11:54         ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Ferruh Yigit
  5 siblings, 0 replies; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add modify field support for GENEVE option fields:
 - "RTE_FLOW_FIELD_GENEVE_OPT_TYPE"
 - "RTE_FLOW_FIELD_GENEVE_OPT_CLASS"
 - "RTE_FLOW_FIELD_GENEVE_OPT_DATA"

Each GENEVE TLV option is identified by both its "class" and "type", so
2 new fields were added to "rte_flow_action_modify_data" structure to
help specify which option to modify.

To get room for those 2 new fields, the "level" field move to use
"uint8_t" which is more than enough for encapsulation level.
This patch also reduces all modify field encapsulation level "fully
masked" initializations to use UINT8_MAX instead of UINT32_MAX.
This change avoids compilation warning caused by this API changing.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            | 48 +++++++++++++++++++++++++-
 doc/guides/prog_guide/rte_flow.rst     | 23 ++++++++++++
 doc/guides/rel_notes/release_23_07.rst |  3 ++
 drivers/net/mlx5/mlx5_flow_hw.c        | 22 ++++++------
 lib/ethdev/rte_flow.h                  | 48 +++++++++++++++++++++++++-
 5 files changed, 131 insertions(+), 13 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 58939ec321..8c1dea53c0 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,11 +636,15 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -854,7 +858,9 @@ static const char *const modify_field_ids[] = {
 	"ipv4_ecn", "ipv6_ecn", "gtp_psc_qfi", "meter_color",
 	"ipv6_proto",
 	"flex_item",
-	"hash_result", NULL
+	"hash_result",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	NULL
 };
 
 static const char *const meter_colors[] = {
@@ -2295,6 +2301,8 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TYPE_ID,
+	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_TYPE,
 	ZERO,
@@ -2302,6 +2310,8 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
+	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
 	ACTION_MODIFY_FIELD_SRC_VALUE,
 	ACTION_MODIFY_FIELD_SRC_POINTER,
@@ -6388,6 +6398,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
+		.name = "dst_type_id",
+		.help = "destination field type ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_DST_CLASS_ID] = {
+		.name = "dst_class",
+		.help = "destination field class ID",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     dst.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_OFFSET] = {
 		.name = "dst_offset",
 		.help = "destination field bit offset",
@@ -6423,6 +6451,24 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
+		.name = "src_type_id",
+		.help = "source field type ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.type)),
+		.call = parse_vc_conf,
+	},
+	[ACTION_MODIFY_FIELD_SRC_CLASS_ID] = {
+		.name = "src_class",
+		.help = "source field class ID",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY_HTON(struct rte_flow_action_modify_field,
+					     src.class_id)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_OFFSET] = {
 		.name = "src_offset",
 		.help = "source field bit offset",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 25b57bf86d..ec812de335 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2937,6 +2937,14 @@ as well as any tag element in the tag array:
 For the tag array (in case of multiple tags are supported and present)
 ``level`` translates directly into the array index.
 
+``type`` is used to specify (along with ``class_id``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
+``class_id`` is used to specify (along with ``type``) the Geneve option which
+is being modified.
+This field is relevant only for ``RTE_FLOW_FIELD_GENEVE_OPT_XXXX`` type.
+
 ``flex_handle`` is used to specify the flex item pointer which is being
 modified. ``flex_handle`` and ``level`` are mutually exclusive.
 
@@ -2967,6 +2975,17 @@ to replace the third byte of MAC address with value 0x85, application should
 specify destination width as 8, destination offset as 16, and provide immediate
 value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
 
+The ``RTE_FLOW_FIELD_GENEVE_OPT_DATA`` type supports modifying only one DW in
+single action and align to 32 bits. For example, for modifying 16 bits start
+from offset 24, 2 different actions should be prepared. The first one includs
+``offset=24`` and ``width=8``, and the seconde one includs ``offset=32`` and
+``width=8``.
+Application should provide the data in immediate value memory only for the
+single DW even though the offset is related to start of first DW. For example,
+to replace the third byte of second DW in Geneve option data with value 0x85,
+application should specify destination width as 8, destination offset as 48,
+and provide immediate value 0xXXXX85XX.
+
 .. _table_rte_flow_action_modify_field:
 
 .. table:: MODIFY_FIELD
@@ -2994,6 +3013,10 @@ value as sequence of bytes {xxx, xxx, 0x85, xxx, xxx, xxx}.
    +-----------------+----------------------------------------------------------+
    | ``level``       | encapsulation level of a packet field or tag array index |
    +-----------------+----------------------------------------------------------+
+   | ``type``        | geneve option type                                       |
+   +-----------------+----------------------------------------------------------+
+   | ``class_id``    | geneve option class ID                                   |
+   +-----------------+----------------------------------------------------------+
    | ``flex_handle`` | flex item handle of a packet field                       |
    +-----------------+----------------------------------------------------------+
    | ``offset``      | number of bits to skip at the beginning                  |
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index a9b1293689..ce1755096f 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,6 +84,9 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
+* The ``level`` field in experimental structure
+  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+
 
 ABI Changes
 -----------
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 7e0ee8d883..1b68a19900 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -3565,7 +3565,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
-	if (mask_conf->dst.level != UINT32_MAX)
+	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
 			"destination encapsulation level must be fully masked");
@@ -3579,7 +3579,7 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				"destination field mask and template are not equal");
 	if (action_conf->src.field != RTE_FLOW_FIELD_POINTER &&
 	    action_conf->src.field != RTE_FLOW_FIELD_VALUE) {
-		if (mask_conf->src.level != UINT32_MAX)
+		if (mask_conf->src.level != UINT8_MAX)
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source encapsulation level must be fully masked");
@@ -4450,7 +4450,7 @@ flow_hw_set_vlan_vid(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = RTE_FLOW_FIELD_VLAN_ID,
-			.level = 0xffffffff, .offset = 0xffffffff,
+			.level = 0xff, .offset = 0xffffffff,
 		},
 		.src = {
 			.field = RTE_FLOW_FIELD_VALUE,
@@ -4583,12 +4583,12 @@ flow_hw_actions_template_create(struct rte_eth_dev *dev,
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -5653,7 +5653,7 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -5677,12 +5677,12 @@ flow_hw_create_tx_repr_tag_jump_acts_tmpl(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
@@ -6009,7 +6009,7 @@ flow_hw_create_ctrl_regc_jump_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
@@ -6182,12 +6182,12 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev)
 		.operation = RTE_FLOW_MODIFY_SET,
 		.dst = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.src = {
 			.field = (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG,
-			.level = UINT32_MAX,
+			.level = UINT8_MAX,
 			.offset = UINT32_MAX,
 		},
 		.width = UINT32_MAX,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 713ba8b65c..f30d4b033f 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3773,6 +3773,9 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_IPV6_PROTO,	/**< IPv6 next header. */
 	RTE_FLOW_FIELD_FLEX_ITEM,	/**< Flex item. */
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
+	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
+	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
 };
 
 /**
@@ -3788,7 +3791,50 @@ struct rte_flow_action_modify_data {
 		struct {
 			/** Encapsulation level or tag index or flex item handle. */
 			union {
-				uint32_t level;
+				struct {
+					/**
+					 * Packet encapsulation level containing
+					 * the field modify to.
+					 *
+					 * - @p 0 requests the default behavior.
+					 *   Depending on the packet type, it
+					 *   can mean outermost, innermost or
+					 *   anything in between.
+					 *
+					 *   It basically stands for the
+					 *   innermost encapsulation level
+					 *   modification can be performed on
+					 *   according to PMD and device
+					 *   capabilities.
+					 *
+					 * - @p 1 requests modification to be
+					 *   performed on the outermost packet
+					 *   encapsulation level.
+					 *
+					 * - @p 2 and subsequent values request
+					 *   modification to be performed on
+					 *   the specified inner packet
+					 *   encapsulation level, from
+					 *   outermost to innermost (lower to
+					 *   higher values).
+					 *
+					 * Values other than @p 0 are not
+					 * necessarily supported.
+					 */
+					uint8_t level;
+					/**
+					 * Geneve option type. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					uint8_t type;
+					/**
+					 * Geneve option class. relevant only
+					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+					 * modification type.
+					 */
+					rte_be16_t class_id;
+				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
 			/** Number of bits to skip from a field. */
-- 
2.25.1


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

* [PATCH v5 5/5] ethdev: add MPLS header modification support
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
                           ` (3 preceding siblings ...)
  2023-05-23 21:31         ` [PATCH v5 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
@ 2023-05-23 21:31         ` Michael Baum
  2023-06-01 11:54           ` Ferruh Yigit
  2023-06-01 11:54         ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Ferruh Yigit
  5 siblings, 1 reply; 43+ messages in thread
From: Michael Baum @ 2023-05-23 21:31 UTC (permalink / raw)
  To: dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Ferruh Yigit, Thomas Monjalon

Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.

Since MPLS heaser might appear more the one time in inner/outer/tunnel,
a new field was added to "rte_flow_action_modify_data" structure in
addition to "level" field.
The "tag_index" field is the index of the header inside encapsulation
level. It is used for modify multiple MPLS headers in same encapsulation
level.

This addition enables to modify multiple VLAN headers too, so the
description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.

Since the "tag_index" field is added, the "RTE_FLOW_FIELD_TAG" type
moves to use it for tag array instead of using "level" field.
Using "level" is still supported for backwards compatibility when
"tag_index" field is zero.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 app/test-pmd/cmdline_flow.c            |  24 +++++-
 doc/guides/prog_guide/rte_flow.rst     |  18 ++--
 doc/guides/rel_notes/release_23_07.rst |   8 +-
 drivers/net/mlx5/mlx5_flow.c           |  34 ++++++++
 drivers/net/mlx5/mlx5_flow.h           |  23 ++++++
 drivers/net/mlx5/mlx5_flow_dv.c        | 110 +++++++++++--------------
 drivers/net/mlx5/mlx5_flow_hw.c        |  21 +++--
 lib/ethdev/rte_flow.h                  |  51 ++++++++----
 8 files changed, 199 insertions(+), 90 deletions(-)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 8c1dea53c0..a51e37276b 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -636,6 +636,7 @@ enum index {
 	ACTION_MODIFY_FIELD_DST_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_DST_LEVEL,
 	ACTION_MODIFY_FIELD_DST_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -643,6 +644,7 @@ enum index {
 	ACTION_MODIFY_FIELD_SRC_TYPE_VALUE,
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
 	ACTION_MODIFY_FIELD_SRC_LEVEL_VALUE,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -859,7 +861,7 @@ static const char *const modify_field_ids[] = {
 	"ipv6_proto",
 	"flex_item",
 	"hash_result",
-	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data",
+	"geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
 	NULL
 };
 
@@ -2301,6 +2303,7 @@ static const enum index next_action_sample[] = {
 
 static const enum index action_modify_field_dst[] = {
 	ACTION_MODIFY_FIELD_DST_LEVEL,
+	ACTION_MODIFY_FIELD_DST_TAG_INDEX,
 	ACTION_MODIFY_FIELD_DST_TYPE_ID,
 	ACTION_MODIFY_FIELD_DST_CLASS_ID,
 	ACTION_MODIFY_FIELD_DST_OFFSET,
@@ -2310,6 +2313,7 @@ static const enum index action_modify_field_dst[] = {
 
 static const enum index action_modify_field_src[] = {
 	ACTION_MODIFY_FIELD_SRC_LEVEL,
+	ACTION_MODIFY_FIELD_SRC_TAG_INDEX,
 	ACTION_MODIFY_FIELD_SRC_TYPE_ID,
 	ACTION_MODIFY_FIELD_SRC_CLASS_ID,
 	ACTION_MODIFY_FIELD_SRC_OFFSET,
@@ -6398,6 +6402,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_DST_TAG_INDEX] = {
+		.name = "dst_tag_index",
+		.help = "destination field tag array",
+		.next = NEXT(action_modify_field_dst,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					dst.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_DST_TYPE_ID] = {
 		.name = "dst_type_id",
 		.help = "destination field type ID",
@@ -6451,6 +6464,15 @@ static const struct token token_list[] = {
 		.call = parse_vc_modify_field_level,
 		.comp = comp_none,
 	},
+	[ACTION_MODIFY_FIELD_SRC_TAG_INDEX] = {
+		.name = "stc_tag_index",
+		.help = "source field tag array",
+		.next = NEXT(action_modify_field_src,
+			     NEXT_ENTRY(COMMON_UNSIGNED)),
+		.args = ARGS(ARGS_ENTRY(struct rte_flow_action_modify_field,
+					src.tag_index)),
+		.call = parse_vc_conf,
+	},
 	[ACTION_MODIFY_FIELD_SRC_TYPE_ID] = {
 		.name = "src_type_id",
 		.help = "source field type ID",
diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index ec812de335..e4328e7ed6 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2925,8 +2925,7 @@ See ``enum rte_flow_field_id`` for the list of supported fields.
 
 ``width`` defines a number of bits to use from ``src`` field.
 
-``level`` is used to access any packet field on any encapsulation level
-as well as any tag element in the tag array:
+``level`` is used to access any packet field on any encapsulation level:
 
 - ``0`` means the default behaviour. Depending on the packet type,
   it can mean outermost, innermost or anything in between.
@@ -2934,8 +2933,15 @@ as well as any tag element in the tag array:
 - ``2`` and subsequent values requests access to the specified packet
   encapsulation level, from outermost to innermost (lower to higher values).
 
-For the tag array (in case of multiple tags are supported and present)
-``level`` translates directly into the array index.
+``tag_index`` is the index of the header inside encapsulation level.
+It is used for modify either ``VLAN`` or ``MPLS`` or ``TAG`` headers which
+multiple of them might be supported in same encapsulation level.
+
+.. note::
+
+   For ``RTE_FLOW_FIELD_TAG`` type, the tag array was provided in ``level``
+   field and it is still supported for backwards compatibility.
+   When ``tag_index`` is zero, the tag array is taken from ``level`` field.
 
 ``type`` is used to specify (along with ``class_id``) the Geneve option which
 is being modified.
@@ -3011,7 +3017,9 @@ and provide immediate value 0xXXXX85XX.
    +=================+==========================================================+
    | ``field``       | ID: packet field, mark, meta, tag, immediate, pointer    |
    +-----------------+----------------------------------------------------------+
-   | ``level``       | encapsulation level of a packet field or tag array index |
+   | ``level``       | encapsulation level of a packet field                    |
+   +-----------------+----------------------------------------------------------+
+   | ``tag_index``   | tag index inside encapsulation level                     |
    +-----------------+----------------------------------------------------------+
    | ``type``        | geneve option type                                       |
    +-----------------+----------------------------------------------------------+
diff --git a/doc/guides/rel_notes/release_23_07.rst b/doc/guides/rel_notes/release_23_07.rst
index ce1755096f..fd3e35eea3 100644
--- a/doc/guides/rel_notes/release_23_07.rst
+++ b/doc/guides/rel_notes/release_23_07.rst
@@ -84,8 +84,12 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =======================================================
 
-* The ``level`` field in experimental structure
-  ``struct rte_flow_action_modify_data`` was reduced to 8 bits.
+* ethdev: in experimental structure ``struct rte_flow_action_modify_data``:
+
+  * ``level`` field was reduced to 8 bits.
+
+  * ``tag_index`` field replaced ``level`` field in representing tag array for
+    ``RTE_FLOW_FIELD_TAG`` type.
 
 
 ABI Changes
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 19f7f92717..867b7b8ea2 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2318,6 +2318,40 @@ mlx5_validate_action_ct(struct rte_eth_dev *dev,
 	return 0;
 }
 
+/**
+ * Validate the level value for modify field action.
+ *
+ * @param[in] data
+ *   Pointer to the rte_flow_action_modify_data structure either src or dst.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+int
+flow_validate_modify_field_level(const struct rte_flow_action_modify_data *data,
+				 struct rte_flow_error *error)
+{
+	if (data->level == 0)
+		return 0;
+	if (data->field != RTE_FLOW_FIELD_TAG)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "inner header fields modification is not supported");
+	if (data->tag_index != 0)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "tag array can be provided using 'level' or 'tag_index' fields, not both");
+	/*
+	 * The tag array for RTE_FLOW_FIELD_TAG type is provided using
+	 * 'tag_index' field. In old API, it was provided using 'level' field
+	 * and it is still supported for backwards compatibility.
+	 */
+	DRV_LOG(WARNING, "tag array provided in 'level' field instead of 'tag_index' field.");
+	return 0;
+}
+
 /**
  * Validate ICMP6 item.
  *
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 1d116ea0f6..cba04b4f45 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1045,6 +1045,26 @@ flow_items_to_tunnel(const struct rte_flow_item items[])
 	return items[0].spec;
 }
 
+/**
+ * Gets the tag array given for RTE_FLOW_FIELD_TAG type.
+ *
+ * In old API the value was provided in "level" field, but in new API
+ * it is provided in "tag_array" field. Since encapsulation level is not
+ * relevant for metadata, the tag array can be still provided in "level"
+ * for backwards compatibility.
+ *
+ * @param[in] data
+ *   Pointer to tag modify data structure.
+ *
+ * @return
+ *   Tag array index.
+ */
+static inline uint8_t
+flow_tag_index_get(const struct rte_flow_action_modify_data *data)
+{
+	return data->tag_index ? data->tag_index : data->level;
+}
+
 /**
  * Fetch 1, 2, 3 or 4 byte field from the byte array
  * and return as unsigned integer in host-endian format.
@@ -2276,6 +2296,9 @@ int mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
 int mlx5_flow_validate_action_default_miss(uint64_t action_flags,
 				const struct rte_flow_attr *attr,
 				struct rte_flow_error *error);
+int flow_validate_modify_field_level
+			(const struct rte_flow_action_modify_data *data,
+			 struct rte_flow_error *error);
 int mlx5_flow_item_acceptable(const struct rte_flow_item *item,
 			      const uint8_t *mask,
 			      const uint8_t *nic_mask,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index f136f43b0a..3070f75ce8 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -1896,16 +1896,17 @@ mlx5_flow_field_id_to_modify_info
 	case RTE_FLOW_FIELD_TAG:
 		{
 			MLX5_ASSERT(data->offset + width <= 32);
+			uint8_t tag_index = flow_tag_index_get(data);
 			int reg;
 
-			off_be = (data->level == MLX5_LINEAR_HASH_TAG_INDEX) ?
+			off_be = (tag_index == MLX5_LINEAR_HASH_TAG_INDEX) ?
 				 16 - (data->offset + width) + 16 : data->offset;
 			if (priv->sh->config.dv_flow_en == 2)
 				reg = flow_hw_get_reg_id(RTE_FLOW_ITEM_TYPE_TAG,
-							 data->level);
+							 tag_index);
 			else
 				reg = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG,
-							   data->level, error);
+							   tag_index, error);
 			if (reg < 0)
 				return;
 			MLX5_ASSERT(reg != REG_NON);
@@ -1985,7 +1986,7 @@ mlx5_flow_field_id_to_modify_info
 		{
 			uint32_t meta_mask = priv->sh->dv_meta_mask;
 			uint32_t meta_count = __builtin_popcount(meta_mask);
-			uint32_t reg = data->level;
+			uint8_t reg = flow_tag_index_get(data);
 
 			RTE_SET_USED(meta_count);
 			MLX5_ASSERT(data->offset + width <= meta_count);
@@ -5245,115 +5246,105 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_sh_config *config = &priv->sh->config;
 	struct mlx5_hca_attr *hca_attr = &priv->sh->cdev->config.hca_attr;
-	const struct rte_flow_action_modify_field *action_modify_field =
-		action->conf;
-	uint32_t dst_width, src_width;
+	const struct rte_flow_action_modify_field *conf = action->conf;
+	const struct rte_flow_action_modify_data *src_data = &conf->src;
+	const struct rte_flow_action_modify_data *dst_data = &conf->dst;
+	uint32_t dst_width, src_width, width = conf->width;
 
 	ret = flow_dv_validate_action_modify_hdr(action_flags, action, error);
 	if (ret)
 		return ret;
-	if (action_modify_field->src.field == RTE_FLOW_FIELD_FLEX_ITEM ||
-	    action_modify_field->dst.field == RTE_FLOW_FIELD_FLEX_ITEM)
+	if (src_data->field == RTE_FLOW_FIELD_FLEX_ITEM ||
+	    dst_data->field == RTE_FLOW_FIELD_FLEX_ITEM)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"flex item fields modification"
 				" is not supported");
-	dst_width = mlx5_flow_item_field_width(dev, action_modify_field->dst.field,
+	dst_width = mlx5_flow_item_field_width(dev, dst_data->field,
 					       -1, attr, error);
-	src_width = mlx5_flow_item_field_width(dev, action_modify_field->src.field,
+	src_width = mlx5_flow_item_field_width(dev, src_data->field,
 					       dst_width, attr, error);
-	if (action_modify_field->width == 0)
+	if (width == 0)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"no bits are requested to be modified");
-	else if (action_modify_field->width > dst_width ||
-		 action_modify_field->width > src_width)
+	else if (width > dst_width || width > src_width)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"cannot modify more bits than"
 				" the width of a field");
-	if (action_modify_field->dst.field != RTE_FLOW_FIELD_VALUE &&
-	    action_modify_field->dst.field != RTE_FLOW_FIELD_POINTER) {
-		if (action_modify_field->dst.offset +
-		    action_modify_field->width > dst_width)
+	if (dst_data->field != RTE_FLOW_FIELD_VALUE &&
+	    dst_data->field != RTE_FLOW_FIELD_POINTER) {
+		if (dst_data->offset + width > dst_width)
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"destination offset is too big");
-		if (action_modify_field->dst.level &&
-		    action_modify_field->dst.field != RTE_FLOW_FIELD_TAG)
-			return rte_flow_error_set(error, ENOTSUP,
-					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"inner header fields modification"
-					" is not supported");
+		ret = flow_validate_modify_field_level(dst_data, error);
+		if (ret)
+			return ret;
 	}
-	if (action_modify_field->src.field != RTE_FLOW_FIELD_VALUE &&
-	    action_modify_field->src.field != RTE_FLOW_FIELD_POINTER) {
+	if (src_data->field != RTE_FLOW_FIELD_VALUE &&
+	    src_data->field != RTE_FLOW_FIELD_POINTER) {
 		if (root)
 			return rte_flow_error_set(error, ENOTSUP,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"modify field action is not"
 					" supported for group 0");
-		if (action_modify_field->src.offset +
-		    action_modify_field->width > src_width)
+		if (src_data->offset + width > src_width)
 			return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"source offset is too big");
-		if (action_modify_field->src.level &&
-		    action_modify_field->src.field != RTE_FLOW_FIELD_TAG)
-			return rte_flow_error_set(error, ENOTSUP,
-					RTE_FLOW_ERROR_TYPE_ACTION, action,
-					"inner header fields modification"
-					" is not supported");
+		ret = flow_validate_modify_field_level(src_data, error);
+		if (ret)
+			return ret;
 	}
-	if ((action_modify_field->dst.field ==
-	     action_modify_field->src.field) &&
-	    (action_modify_field->dst.level ==
-	     action_modify_field->src.level))
+	if ((dst_data->field == src_data->field) &&
+	    (dst_data->level == src_data->level))
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source and destination fields"
 				" cannot be the same");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VALUE ||
-	    action_modify_field->dst.field == RTE_FLOW_FIELD_POINTER ||
-	    action_modify_field->dst.field == RTE_FLOW_FIELD_MARK)
+	if (dst_data->field == RTE_FLOW_FIELD_VALUE ||
+	    dst_data->field == RTE_FLOW_FIELD_POINTER ||
+	    dst_data->field == RTE_FLOW_FIELD_MARK)
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"mark, immediate value or a pointer to it"
 				" cannot be used as a destination");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_START ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_START)
+	if (dst_data->field == RTE_FLOW_FIELD_START ||
+	    src_data->field == RTE_FLOW_FIELD_START)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of an arbitrary"
 				" place in a packet is not supported");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VLAN_TYPE ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_VLAN_TYPE)
+	if (dst_data->field == RTE_FLOW_FIELD_VLAN_TYPE ||
+	    src_data->field == RTE_FLOW_FIELD_VLAN_TYPE)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of the 802.1Q Tag"
 				" Identifier is not supported");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_VXLAN_VNI ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_VXLAN_VNI)
+	if (dst_data->field == RTE_FLOW_FIELD_VXLAN_VNI ||
+	    src_data->field == RTE_FLOW_FIELD_VXLAN_VNI)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of the VXLAN Network"
 				" Identifier is not supported");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_GENEVE_VNI ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_GENEVE_VNI)
+	if (dst_data->field == RTE_FLOW_FIELD_GENEVE_VNI ||
+	    src_data->field == RTE_FLOW_FIELD_GENEVE_VNI)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of the GENEVE Network"
 				" Identifier is not supported");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_MARK ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_MARK)
+	if (dst_data->field == RTE_FLOW_FIELD_MARK ||
+	    src_data->field == RTE_FLOW_FIELD_MARK)
 		if (config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 		    !mlx5_flow_ext_mreg_supported(dev))
 			return rte_flow_error_set(error, ENOTSUP,
 					RTE_FLOW_ERROR_TYPE_ACTION, action,
 					"cannot modify mark in legacy mode"
 					" or without extensive registers");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_META ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_META) {
+	if (dst_data->field == RTE_FLOW_FIELD_META ||
+	    src_data->field == RTE_FLOW_FIELD_META) {
 		if (config->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY &&
 		    !mlx5_flow_ext_mreg_supported(dev))
 			return rte_flow_error_set(error, ENOTSUP,
@@ -5367,20 +5358,19 @@ flow_dv_validate_action_modify_field(struct rte_eth_dev *dev,
 					"cannot modify meta without"
 					" extensive registers available");
 	}
-	if (action_modify_field->operation == RTE_FLOW_MODIFY_SUB)
+	if (conf->operation == RTE_FLOW_MODIFY_SUB)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"sub operations are not supported");
-	if (action_modify_field->dst.field == RTE_FLOW_FIELD_IPV4_ECN ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_IPV4_ECN ||
-	    action_modify_field->dst.field == RTE_FLOW_FIELD_IPV6_ECN ||
-	    action_modify_field->src.field == RTE_FLOW_FIELD_IPV6_ECN)
+	if (dst_data->field == RTE_FLOW_FIELD_IPV4_ECN ||
+	    src_data->field == RTE_FLOW_FIELD_IPV4_ECN ||
+	    dst_data->field == RTE_FLOW_FIELD_IPV6_ECN ||
+	    src_data->field == RTE_FLOW_FIELD_IPV6_ECN)
 		if (!hca_attr->modify_outer_ip_ecn && root)
 			return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"modifications of the ECN for current firmware is not supported");
-	return (action_modify_field->width / 32) +
-	       !!(action_modify_field->width % 32);
+	return (width / 32) + !!(width % 32);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 1b68a19900..39ea76c0c0 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1022,9 +1022,11 @@ flow_hw_modify_field_compile(struct rte_eth_dev *dev,
 		    conf->dst.field == RTE_FLOW_FIELD_TAG ||
 		    conf->dst.field == RTE_FLOW_FIELD_METER_COLOR ||
 		    conf->dst.field == (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
+			uint8_t tag_index = flow_tag_index_get(&conf->dst);
+
 			value = *(const unaligned_uint32_t *)item.spec;
 			if (conf->dst.field == RTE_FLOW_FIELD_TAG &&
-			    conf->dst.level == MLX5_LINEAR_HASH_TAG_INDEX)
+			    tag_index == MLX5_LINEAR_HASH_TAG_INDEX)
 				value = rte_cpu_to_be_32(value << 16);
 			else
 				value = rte_cpu_to_be_32(value);
@@ -2055,9 +2057,11 @@ flow_hw_modify_field_construct(struct mlx5_hw_q_job *job,
 	    mhdr_action->dst.field == RTE_FLOW_FIELD_TAG ||
 	    mhdr_action->dst.field == RTE_FLOW_FIELD_METER_COLOR ||
 	    mhdr_action->dst.field == (enum rte_flow_field_id)MLX5_RTE_FLOW_FIELD_META_REG) {
+		uint8_t tag_index = flow_tag_index_get(&mhdr_action->dst);
+
 		value_p = (unaligned_uint32_t *)values;
 		if (mhdr_action->dst.field == RTE_FLOW_FIELD_TAG &&
-		    mhdr_action->dst.level == MLX5_LINEAR_HASH_TAG_INDEX)
+		    tag_index == MLX5_LINEAR_HASH_TAG_INDEX)
 			*value_p = rte_cpu_to_be_32(*value_p << 16);
 		else
 			*value_p = rte_cpu_to_be_32(*value_p);
@@ -3546,10 +3550,9 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 				     const struct rte_flow_action *mask,
 				     struct rte_flow_error *error)
 {
-	const struct rte_flow_action_modify_field *action_conf =
-		action->conf;
-	const struct rte_flow_action_modify_field *mask_conf =
-		mask->conf;
+	const struct rte_flow_action_modify_field *action_conf = action->conf;
+	const struct rte_flow_action_modify_field *mask_conf = mask->conf;
+	int ret;
 
 	if (action_conf->operation != mask_conf->operation)
 		return rte_flow_error_set(error, EINVAL,
@@ -3565,6 +3568,9 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"immediate value, pointer and hash result cannot be used as destination");
+	ret = flow_validate_modify_field_level(&action_conf->dst, error);
+	if (ret)
+		return ret;
 	if (mask_conf->dst.level != UINT8_MAX)
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_ACTION, action,
@@ -3587,6 +3593,9 @@ flow_hw_validate_action_modify_field(const struct rte_flow_action *action,
 			return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION, action,
 				"source offset level must be fully masked");
+		ret = flow_validate_modify_field_level(&action_conf->src, error);
+		if (ret)
+			return ret;
 	}
 	if (mask_conf->width != UINT32_MAX)
 		return rte_flow_error_set(error, EINVAL,
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index f30d4b033f..1df4b49219 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -3740,8 +3740,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_START = 0,	/**< Start of a packet. */
 	RTE_FLOW_FIELD_MAC_DST,		/**< Destination MAC Address. */
 	RTE_FLOW_FIELD_MAC_SRC,		/**< Source MAC Address. */
-	RTE_FLOW_FIELD_VLAN_TYPE,	/**< 802.1Q Tag Identifier. */
-	RTE_FLOW_FIELD_VLAN_ID,		/**< 802.1Q VLAN Identifier. */
+	RTE_FLOW_FIELD_VLAN_TYPE,	/**< VLAN Tag Identifier. */
+	RTE_FLOW_FIELD_VLAN_ID,		/**< VLAN Identifier. */
 	RTE_FLOW_FIELD_MAC_TYPE,	/**< EtherType. */
 	RTE_FLOW_FIELD_IPV4_DSCP,	/**< IPv4 DSCP. */
 	RTE_FLOW_FIELD_IPV4_TTL,	/**< IPv4 Time To Live. */
@@ -3775,7 +3775,8 @@ enum rte_flow_field_id {
 	RTE_FLOW_FIELD_HASH_RESULT,	/**< Hash result. */
 	RTE_FLOW_FIELD_GENEVE_OPT_TYPE,	/**< GENEVE option type */
 	RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class */
-	RTE_FLOW_FIELD_GENEVE_OPT_DATA	/**< GENEVE option data */
+	RTE_FLOW_FIELD_GENEVE_OPT_DATA,	/**< GENEVE option data */
+	RTE_FLOW_FIELD_MPLS		/**< MPLS header. */
 };
 
 /**
@@ -3789,7 +3790,7 @@ struct rte_flow_action_modify_data {
 	RTE_STD_C11
 	union {
 		struct {
-			/** Encapsulation level or tag index or flex item handle. */
+			/** Encapsulation level and tag index or flex item handle. */
 			union {
 				struct {
 					/**
@@ -3820,20 +3821,38 @@ struct rte_flow_action_modify_data {
 					 *
 					 * Values other than @p 0 are not
 					 * necessarily supported.
+					 *
+					 * @note that for MPLS field,
+					 * encapsulation level also include
+					 * tunnel since MPLS may appear in
+					 * outer, inner or tunnel.
 					 */
 					uint8_t level;
-					/**
-					 * Geneve option type. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					uint8_t type;
-					/**
-					 * Geneve option class. relevant only
-					 * for RTE_FLOW_FIELD_GENEVE_OPT_XXXX
-					 * modification type.
-					 */
-					rte_be16_t class_id;
+					union {
+						/**
+						 * Tag index array inside
+						 * encapsulation level.
+						 * Used for VLAN, MPLS or TAG
+						 * types.
+						 */
+						uint8_t tag_index;
+						/**
+						 * Geneve option identifier.
+						 * relevant only for
+						 * RTE_FLOW_FIELD_GENEVE_OPT_XXXX
+						 * modification type.
+						 */
+						struct {
+							/**
+							 * Geneve option type.
+							 */
+							uint8_t type;
+							/**
+							 * Geneve option class.
+							 */
+							rte_be16_t class_id;
+						};
+					};
 				};
 				struct rte_flow_item_flex_handle *flex_handle;
 			};
-- 
2.25.1


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

* Re: [PATCH v5 5/5] ethdev: add MPLS header modification support
  2023-05-23 21:31         ` [PATCH v5 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-06-01 11:54           ` Ferruh Yigit
  0 siblings, 0 replies; 43+ messages in thread
From: Ferruh Yigit @ 2023-06-01 11:54 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Thomas Monjalon

On 5/23/2023 10:31 PM, Michael Baum wrote:
> Add support for MPLS modify header using "RTE_FLOW_FIELD_MPLS" id.
> 
> Since MPLS heaser might appear more the one time in inner/outer/tunnel,

header, fixed while merging.

> a new field was added to "rte_flow_action_modify_data" structure in
> addition to "level" field.
> The "tag_index" field is the index of the header inside encapsulation
> level. It is used for modify multiple MPLS headers in same encapsulation
> level.
> 
> This addition enables to modify multiple VLAN headers too, so the
> description of "RTE_FLOW_FIELD_VLAN_XXXX" was updated.
> 
> Since the "tag_index" field is added, the "RTE_FLOW_FIELD_TAG" type
> moves to use it for tag array instead of using "level" field.
> Using "level" is still supported for backwards compatibility when
> "tag_index" field is zero.
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>

<...>

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

* Re: [PATCH v5 0/5] ethdev: modify field API for multiple headers
  2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
                           ` (4 preceding siblings ...)
  2023-05-23 21:31         ` [PATCH v5 5/5] ethdev: add MPLS header " Michael Baum
@ 2023-06-01 11:54         ` Ferruh Yigit
  5 siblings, 0 replies; 43+ messages in thread
From: Ferruh Yigit @ 2023-06-01 11:54 UTC (permalink / raw)
  To: Michael Baum, dev; +Cc: Ori Kam, Aman Singh, Yuying Zhang, Thomas Monjalon

On 5/23/2023 10:31 PM, Michael Baum wrote:
> This patch-set extend the modify field action API to support both
> multiple MPLS and GENEVE option headers.
> 
> In current API, the header type is provided by rte_flow_field_id
> enumeration and the encapsulation level (inner/outer/tunnel) is
> specified by data.level field.
> However, there is no way to specify header inside encapsulation level.
> 
> For example, for this packet:
> 
> eth / mpls / mpls / mpls / ipv4 / udp
> the both second and third MPLS headers cannot be modified using this
> API.
> 
> RFC:
> https://patchwork.dpdk.org/project/dpdk/cover/20230420092145.522389-1-michaelba@nvidia.com/
> 
> v2:
>  - Change "sub_level" name to "tag_index".
>  - Squash PMD changes into API changes patch.
>  - Remove PMD private patch from the patch-set.
> 
> v3:
>  - Add TAG array API change to release notes.
>  - Improve comment and documentation.
> 
> v4:
>  - Add "Acked-by" labels.
>  - Add PMD adjustment for TAG array API change. 
> 
> v5:
>  - Fix location of level validation.
> 
> Michael Baum (5):
>   doc: fix blank lines in modify field action description
>   doc: fix blank line in asynchronous operations description
>   doc: fix wrong indentation in RSS action description
>   ethdev: add GENEVE TLV option modification support
>   ethdev: add MPLS header modification support
> 

Doc patches merged into single one,

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


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

end of thread, other threads:[~2023-06-01 11:55 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-16  6:37 [PATCH v1 0/7] ethdev: modify field API for multiple headers Michael Baum
2023-05-16  6:37 ` [PATCH v1 1/7] doc: fix blank lines in modify field action description Michael Baum
2023-05-16  6:37 ` [PATCH v1 2/7] doc: fix blank line in asynchronous operations description Michael Baum
2023-05-17 17:13   ` Ori Kam
2023-05-16  6:37 ` [PATCH v1 3/7] doc: fix wrong indentation in RSS action description Michael Baum
2023-05-18  6:18   ` Ori Kam
2023-05-16  6:37 ` [PATCH v1 4/7] net/mlx5: reduce modify field encapsulation level size Michael Baum
2023-05-16  6:37 ` [PATCH v1 5/7] ethdev: add GENEVE TLV option modification support Michael Baum
2023-05-16  6:37 ` [PATCH v1 6/7] ethdev: add MPLS header " Michael Baum
2023-05-16  6:37 ` [PATCH v1 7/7] net/mlx5: add MPLS modify field support Michael Baum
2023-05-18 17:40 ` [PATCH v2 0/5] ethdev: modify field API for multiple headers Michael Baum
2023-05-18 17:40   ` [PATCH v2 1/5] doc: fix blank lines in modify field action description Michael Baum
2023-05-21 10:07     ` Ori Kam
2023-05-18 17:40   ` [PATCH v2 2/5] doc: fix blank line in asynchronous operations description Michael Baum
2023-05-21 10:07     ` Ori Kam
2023-05-18 17:40   ` [PATCH v2 3/5] doc: fix wrong indentation in RSS action description Michael Baum
2023-05-21 10:08     ` Ori Kam
2023-05-18 17:40   ` [PATCH v2 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
2023-05-21 18:52     ` Ori Kam
2023-05-18 17:40   ` [PATCH v2 5/5] ethdev: add MPLS header " Michael Baum
2023-05-21 19:03     ` Ori Kam
2023-05-22 12:04       ` Michael Baum
2023-05-22 19:27   ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Michael Baum
2023-05-22 19:28     ` [PATCH v3 1/5] doc: fix blank lines in modify field action description Michael Baum
2023-05-22 19:28     ` [PATCH v3 2/5] doc: fix blank line in asynchronous operations description Michael Baum
2023-05-22 19:28     ` [PATCH v3 3/5] doc: fix wrong indentation in RSS action description Michael Baum
2023-05-22 19:28     ` [PATCH v3 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
2023-05-22 19:28     ` [PATCH v3 5/5] ethdev: add MPLS header " Michael Baum
2023-05-23 10:40     ` [PATCH v3 0/5] ethdev: modify field API for multiple headers Ori Kam
2023-05-23 12:48     ` [PATCH v4 " Michael Baum
2023-05-23 12:48       ` [PATCH v4 1/5] doc: fix blank lines in modify field action description Michael Baum
2023-05-23 12:48       ` [PATCH v4 2/5] doc: fix blank line in asynchronous operations description Michael Baum
2023-05-23 12:48       ` [PATCH v4 3/5] doc: fix wrong indentation in RSS action description Michael Baum
2023-05-23 12:48       ` [PATCH v4 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
2023-05-23 12:48       ` [PATCH v4 5/5] ethdev: add MPLS header " Michael Baum
2023-05-23 21:31       ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Michael Baum
2023-05-23 21:31         ` [PATCH v5 1/5] doc: fix blank lines in modify field action description Michael Baum
2023-05-23 21:31         ` [PATCH v5 2/5] doc: fix blank line in asynchronous operations description Michael Baum
2023-05-23 21:31         ` [PATCH v5 3/5] doc: fix wrong indentation in RSS action description Michael Baum
2023-05-23 21:31         ` [PATCH v5 4/5] ethdev: add GENEVE TLV option modification support Michael Baum
2023-05-23 21:31         ` [PATCH v5 5/5] ethdev: add MPLS header " Michael Baum
2023-06-01 11:54           ` Ferruh Yigit
2023-06-01 11:54         ` [PATCH v5 0/5] ethdev: modify field API for multiple headers Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).