[v7,1/2] ethdev: introduce generic modify rte flow action

Message ID 20210116045054.14539-2-akozyrev@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series generic modify rte flow action support |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Alexander Kozyrev Jan. 16, 2021, 4:50 a.m. UTC
  Implement the generic modify flow API to allow manipulations on
an arbitrary header field (as well as mark, metadata or tag) using
data from another field or a user-specified value.
This generic modify mechanism removes the necessity to implement
a separate RTE Flow action every time we need to modify a new packet
field in the future.

Supported operation are:
- set: copy data from source to destination.
- add: integer addition, stores the result in destination.
- sub: integer subtraction, stores the result in destination.

The field ID is used to specify the desired source/destination packet
field in order to simplify the API for various encapsulation models.
Specifying the packet field ID with the needed encapsulation level
is able to quickly get a packet field for any inner packet header.

Alternatively, the special ID (ITEM_START) can be used to point to
the very beginning of a packet. This ID in conjunction with the
offset parameter provides great flexibility to copy/modify any part of
a packet as needed.

The number of bits to use from a source as well as the offset can be
be specified to allow a partial copy or dividing a big packet field
into multiple small fields (e.g. copying 128 bits of IPv6 to 4 tags).

An immediate value (or pointer to it) can be specified instead of the
level and the offset for the special FIELD_VALUE ID (or FIELD_POINTER).
Can be used as a source only.

RFC: http://patches.dpdk.org/patch/85384/

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>

---
v1: Initialy RTE_FLOW_ACTION_TYPE_COPY_ITEM
v2: Renamed to RTE_FLOW_ACTION_TYPE_COPY_FIELD
v3: Redesigned as RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
v4: Fixed typos in documentation, renamed mov operation to set.
---
 doc/guides/prog_guide/rte_flow.rst     | 65 +++++++++++++++++++++
 doc/guides/rel_notes/release_21_02.rst |  8 +++
 lib/librte_ethdev/rte_flow.c           |  2 +
 lib/librte_ethdev/rte_flow.h           | 79 +++++++++++++++++++++++++-
 4 files changed, 153 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Jan. 17, 2021, 11:15 p.m. UTC | #1
16/01/2021 05:50, Alexander Kozyrev:
> Implement the generic modify flow API to allow manipulations on
> an arbitrary header field (as well as mark, metadata or tag) using
> data from another field or a user-specified value.
> This generic modify mechanism removes the necessity to implement
> a separate RTE Flow action every time we need to modify a new packet
> field in the future.
> 
> Supported operation are:
> - set: copy data from source to destination.
> - add: integer addition, stores the result in destination.
> - sub: integer subtraction, stores the result in destination.
> 
> The field ID is used to specify the desired source/destination packet
> field in order to simplify the API for various encapsulation models.
> Specifying the packet field ID with the needed encapsulation level
> is able to quickly get a packet field for any inner packet header.
> 
> Alternatively, the special ID (ITEM_START) can be used to point to
> the very beginning of a packet. This ID in conjunction with the
> offset parameter provides great flexibility to copy/modify any part of
> a packet as needed.
> 
> The number of bits to use from a source as well as the offset can be
> be specified to allow a partial copy or dividing a big packet field
> into multiple small fields (e.g. copying 128 bits of IPv6 to 4 tags).
> 
> An immediate value (or pointer to it) can be specified instead of the
> level and the offset for the special FIELD_VALUE ID (or FIELD_POINTER).
> Can be used as a source only.
> 
> RFC: http://patches.dpdk.org/patch/85384/

You don't need to refer to the RFC in the commit message.

> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> 
> ---
> +Action: ``MODIFY_FIELD``
> +^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Modify ``dst`` field according to ``op`` selected (move, addition,
> +subtraction) with ``width`` bits of data from ``src`` field.

"move" is changed to "set", right?

> +Any arbitrary header field (as well as mark, metadata or tag values)
> +can be used as both source and destination fields as set by ``field``.
> +The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
> +``RTE_FLOW_FIELD_POINTER``) is allowed as a source only.
> +``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
> +
> +``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``
> +
> +``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.

I feel the interpretation of the level 0 is driver-dependent?

> +- ``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).
> +For the tag array ``level`` translates directly into the array index.

You should define what is a tag array.

> +
> +``offset`` specifies the number of bits to skip from a field's start.
> +That allows performing a partial copy of the needed part or to divide a big
> +packet field into multiple smaller fields. Alternatively, ``offset`` allows
> +going past the specified packet field boundary to copy a field to an
> +arbitrary place in a packet, essentially providing a way to copy any part of
> +a packet to any other part of it if supported by an underlying PMD driver.

All features of this specification require PMD support,
so I think it is useless to mention here.

> +
> +``value`` sets an immediate value to be used as a source or points to a
> +location of the value in memory. It is used instead of ``level`` and ``offset``
> +for ``RTE_FLOW_FIELD_VALUE`` and ``RTE_FLOW_FIELD_POINTER`` respectively.
> +
> +.. _table_rte_flow_action_modify_field:
> +
> +.. table:: MODIFY_FIELD
> +
> +   +-----------------------------------------+
> +   | Field         | Value                   |
> +   +===============+=========================+
> +   | ``op``        | operation to perform    |
> +   | ``dst``       | destination field       |
> +   | ``src``       | source field            |
> +   | ``width``     | number of bits to use   |
> +   +---------------+-------------------------+
> +
> +.. _table_rte_flow_action_modify_data:
> +
> +.. table:: destination/source field definition
> +
> +   +--------------------------------------------------------------------------+
> +   | Field         | Value                                                    |
> +   +===============+==========================================================+
> +   | ``field``     | ID: packet field, mark, meta, tag, immediate, pointer    |
> +   | ``level``     | encapsulation level of a packet field or tag array index |
> +   | ``offset``    | number of bits to skip at the beginning                  |
> +   | ``value``     | immediate value or a pointer to this value               |
> +   +---------------+----------------------------------------------------------+

I know the whole rte_flow guide has this kind of table,
but really it looks like doxygen and can be skipped here.

If really this redundant info is required, it should be RST definition lists,
not tables.

[...]

In my opinion, the most important info is in the doxygen comments.

Please add a doxygen comment for this enum.

> +enum rte_flow_field_id {
> +	RTE_FLOW_FIELD_START = 0,

Please add a doxygen comment for the value RTE_FLOW_FIELD_START.

> +	RTE_FLOW_FIELD_MAC_DST,
> +	RTE_FLOW_FIELD_MAC_SRC,
> +	RTE_FLOW_FIELD_VLAN_TYPE,
> +	RTE_FLOW_FIELD_VLAN_ID,
> +	RTE_FLOW_FIELD_MAC_TYPE,
> +	RTE_FLOW_FIELD_IPV4_DSCP,
> +	RTE_FLOW_FIELD_IPV4_TTL,
> +	RTE_FLOW_FIELD_IPV4_SRC,
> +	RTE_FLOW_FIELD_IPV4_DST,
> +	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
> +	RTE_FLOW_FIELD_IPV6_SRC,
> +	RTE_FLOW_FIELD_IPV6_DST,
> +	RTE_FLOW_FIELD_TCP_PORT_SRC,
> +	RTE_FLOW_FIELD_TCP_PORT_DST,
> +	RTE_FLOW_FIELD_TCP_SEQ_NUM,
> +	RTE_FLOW_FIELD_TCP_ACK_NUM,
> +	RTE_FLOW_FIELD_TCP_FLAGS,
> +	RTE_FLOW_FIELD_UDP_PORT_SRC,
> +	RTE_FLOW_FIELD_UDP_PORT_DST,
> +	RTE_FLOW_FIELD_VXLAN_VNI,
> +	RTE_FLOW_FIELD_GENEVE_VNI,
> +	RTE_FLOW_FIELD_GTP_TEID,
> +	RTE_FLOW_FIELD_TAG,
> +	RTE_FLOW_FIELD_MARK,
> +	RTE_FLOW_FIELD_META,
> +	RTE_FLOW_FIELD_POINTER,

Please add a doxygen comment for the value RTE_FLOW_FIELD_POINTER.

> +	RTE_FLOW_FIELD_VALUE,

Please add a doxygen comment for the value RTE_FLOW_FIELD_VALUE.

> +struct rte_flow_action_modify_data {
> +	enum rte_flow_field_id field;
> +	RTE_STD_C11
> +	union {
> +		struct {
> +			uint32_t level;
> +			uint32_t offset;
> +		};
> +		uint64_t value;
> +	};
> +};

Please add doxygen for this struct and fields.

> +
> +enum rte_flow_modify_op {
> +	RTE_FLOW_MODIFY_SET = 0,
> +	RTE_FLOW_MODIFY_ADD,
> +	RTE_FLOW_MODIFY_SUB,
> +};

Please add doxygen for this enum.

> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this structure may change without prior notice
> + *
> + * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
> + * Modifies a destination header field according to the specified
> + * operation. Another packet field can be used as a source as well
> + * as tag, mark, metadata or an immediate value or a pointer to it.
> + * Width is the number of bits used from the source item.

width should be documented below as other fields.

> + */
> +struct rte_flow_action_modify_field {
> +	enum rte_flow_modify_op operation;
> +	struct rte_flow_action_modify_data dst;
> +	struct rte_flow_action_modify_data src;
> +	uint32_t width;
> +};
  
Alexander Kozyrev Jan. 18, 2021, 4:03 p.m. UTC | #2
> From: Thomas Monjalon <thomas@monjalon.net> on Sunday, January 17, 2021 18:16
> 16/01/2021 05:50, Alexander Kozyrev:
> > Implement the generic modify flow API to allow manipulations on
> > an arbitrary header field (as well as mark, metadata or tag) using
> > data from another field or a user-specified value.
> > This generic modify mechanism removes the necessity to implement
> > a separate RTE Flow action every time we need to modify a new packet
> > field in the future.
> >
> > Supported operation are:
> > - set: copy data from source to destination.
> > - add: integer addition, stores the result in destination.
> > - sub: integer subtraction, stores the result in destination.
> >
> > The field ID is used to specify the desired source/destination packet
> > field in order to simplify the API for various encapsulation models.
> > Specifying the packet field ID with the needed encapsulation level
> > is able to quickly get a packet field for any inner packet header.
> >
> > Alternatively, the special ID (ITEM_START) can be used to point to
> > the very beginning of a packet. This ID in conjunction with the
> > offset parameter provides great flexibility to copy/modify any part of
> > a packet as needed.
> >
> > The number of bits to use from a source as well as the offset can be
> > be specified to allow a partial copy or dividing a big packet field
> > into multiple small fields (e.g. copying 128 bits of IPv6 to 4 tags).
> >
> > An immediate value (or pointer to it) can be specified instead of the
> > level and the offset for the special FIELD_VALUE ID (or FIELD_POINTER).
> > Can be used as a source only.
> >
> > RFC:
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
> es.dpdk.org%2Fpatch%2F85384%2F&amp;data=04%7C01%7Cakozyrev%40nv
> idia.com%7Ce20d2c38372b4062420d08d8bb3dd549%7C43083d15727340c1b7d
> b39efd9ccc17a%7C0%7C0%7C637465221537183554%7CUnknown%7CTWFpb
> GZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI
> 6Mn0%3D%7C1000&amp;sdata=gWEdmyAabnCCmeFyi%2FvHs0aheMmWNx
> IyR%2BTC96O3Yck%3D&amp;reserved=0
> 
> You don't need to refer to the RFC in the commit message.
Removing link to it.

> 
> > Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
> > Acked-by: Ori Kam <orika@nvidia.com>
> >
> > ---
> > +Action: ``MODIFY_FIELD``
> > +^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +Modify ``dst`` field according to ``op`` selected (move, addition,
> > +subtraction) with ``width`` bits of data from ``src`` field.
> 
> "move" is changed to "set", right?
Right, thanks for catching this.

> 
> > +Any arbitrary header field (as well as mark, metadata or tag values)
> > +can be used as both source and destination fields as set by ``field``.
> > +The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
> > +``RTE_FLOW_FIELD_POINTER``) is allowed as a source only.
> > +``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
> > +
> > +``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``
> > +
> > +``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.
> 
> I feel the interpretation of the level 0 is driver-dependent?
It is driver-dependent, the behavior is the same as in case of RSS.

> > +- ``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).
> > +For the tag array ``level`` translates directly into the array index.
> 
> You should define what is a tag array.
Ok, but it is defined already in RTE_FLOW_ACTION_TYPE_SET_TAG action.

> > +
> > +``offset`` specifies the number of bits to skip from a field's start.
> > +That allows performing a partial copy of the needed part or to divide a big
> > +packet field into multiple smaller fields. Alternatively, ``offset`` allows
> > +going past the specified packet field boundary to copy a field to an
> > +arbitrary place in a packet, essentially providing a way to copy any part of
> > +a packet to any other part of it if supported by an underlying PMD driver.
> 
> All features of this specification require PMD support,
> so I think it is useless to mention here.
Removing.
 
> > +
> > +``value`` sets an immediate value to be used as a source or points to a
> > +location of the value in memory. It is used instead of ``level`` and ``offset``
> > +for ``RTE_FLOW_FIELD_VALUE`` and ``RTE_FLOW_FIELD_POINTER``
> respectively.
> > +
> > +.. _table_rte_flow_action_modify_field:
> > +
> > +.. table:: MODIFY_FIELD
> > +
> > +   +-----------------------------------------+
> > +   | Field         | Value                   |
> > +   +===============+=========================+
> > +   | ``op``        | operation to perform    |
> > +   | ``dst``       | destination field       |
> > +   | ``src``       | source field            |
> > +   | ``width``     | number of bits to use   |
> > +   +---------------+-------------------------+
> > +
> > +.. _table_rte_flow_action_modify_data:
> > +
> > +.. table:: destination/source field definition
> > +
> > +   +--------------------------------------------------------------------------+
> > +   | Field         | Value                                                    |
> > +
> +===============+=========================================
> =================+
> > +   | ``field``     | ID: packet field, mark, meta, tag, immediate, pointer    |
> > +   | ``level``     | encapsulation level of a packet field or tag array index |
> > +   | ``offset``    | number of bits to skip at the beginning                  |
> > +   | ``value``     | immediate value or a pointer to this value               |
> > +   +---------------+----------------------------------------------------------+
> 
> I know the whole rte_flow guide has this kind of table,
> but really it looks like doxygen and can be skipped here.
> 
> If really this redundant info is required, it should be RST definition lists,
> not tables.
I would keep these tables for the sake of consistency of the rte_flow guide.

> [...]
> 
> In my opinion, the most important info is in the doxygen comments.
Agree, adding them for all the structures/fields mentioned below.

> Please add a doxygen comment for this enum.
> 
> > +enum rte_flow_field_id {
> > +	RTE_FLOW_FIELD_START = 0,
> 
> Please add a doxygen comment for the value RTE_FLOW_FIELD_START.
> 
> > +	RTE_FLOW_FIELD_MAC_DST,
> > +	RTE_FLOW_FIELD_MAC_SRC,
> > +	RTE_FLOW_FIELD_VLAN_TYPE,
> > +	RTE_FLOW_FIELD_VLAN_ID,
> > +	RTE_FLOW_FIELD_MAC_TYPE,
> > +	RTE_FLOW_FIELD_IPV4_DSCP,
> > +	RTE_FLOW_FIELD_IPV4_TTL,
> > +	RTE_FLOW_FIELD_IPV4_SRC,
> > +	RTE_FLOW_FIELD_IPV4_DST,
> > +	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
> > +	RTE_FLOW_FIELD_IPV6_SRC,
> > +	RTE_FLOW_FIELD_IPV6_DST,
> > +	RTE_FLOW_FIELD_TCP_PORT_SRC,
> > +	RTE_FLOW_FIELD_TCP_PORT_DST,
> > +	RTE_FLOW_FIELD_TCP_SEQ_NUM,
> > +	RTE_FLOW_FIELD_TCP_ACK_NUM,
> > +	RTE_FLOW_FIELD_TCP_FLAGS,
> > +	RTE_FLOW_FIELD_UDP_PORT_SRC,
> > +	RTE_FLOW_FIELD_UDP_PORT_DST,
> > +	RTE_FLOW_FIELD_VXLAN_VNI,
> > +	RTE_FLOW_FIELD_GENEVE_VNI,
> > +	RTE_FLOW_FIELD_GTP_TEID,
> > +	RTE_FLOW_FIELD_TAG,
> > +	RTE_FLOW_FIELD_MARK,
> > +	RTE_FLOW_FIELD_META,
> > +	RTE_FLOW_FIELD_POINTER,
> 
> Please add a doxygen comment for the value RTE_FLOW_FIELD_POINTER.
> 
> > +	RTE_FLOW_FIELD_VALUE,
> 
> Please add a doxygen comment for the value RTE_FLOW_FIELD_VALUE.
> 
> > +struct rte_flow_action_modify_data {
> > +	enum rte_flow_field_id field;
> > +	RTE_STD_C11
> > +	union {
> > +		struct {
> > +			uint32_t level;
> > +			uint32_t offset;
> > +		};
> > +		uint64_t value;
> > +	};
> > +};
> 
> Please add doxygen for this struct and fields.
> 
> > +
> > +enum rte_flow_modify_op {
> > +	RTE_FLOW_MODIFY_SET = 0,
> > +	RTE_FLOW_MODIFY_ADD,
> > +	RTE_FLOW_MODIFY_SUB,
> > +};
> 
> Please add doxygen for this enum.
> 
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this structure may change without prior notice
> > + *
> > + * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > + *
> > + * Modifies a destination header field according to the specified
> > + * operation. Another packet field can be used as a source as well
> > + * as tag, mark, metadata or an immediate value or a pointer to it.
> > + * Width is the number of bits used from the source item.
> 
> width should be documented below as other fields.
> 
> > + */
> > +struct rte_flow_action_modify_field {
> > +	enum rte_flow_modify_op operation;
> > +	struct rte_flow_action_modify_data dst;
> > +	struct rte_flow_action_modify_data src;
> > +	uint32_t width;
> > +};
> 
>
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 86b3444803..d959ca4a8c 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2766,6 +2766,71 @@  The behaviour of the shared action defined by ``action`` argument of type
    | no properties |
    +---------------+
 
+Action: ``MODIFY_FIELD``
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Modify ``dst`` field according to ``op`` selected (move, addition,
+subtraction) with ``width`` bits of data from ``src`` field.
+
+Any arbitrary header field (as well as mark, metadata or tag values)
+can be used as both source and destination fields as set by ``field``.
+The immediate value ``RTE_FLOW_FIELD_VALUE`` (or a pointer to it
+``RTE_FLOW_FIELD_POINTER``) is allowed as a source only.
+``RTE_FLOW_FIELD_START`` is used to point to the beginning of a packet.
+
+``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``
+
+``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.
+- ``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).
+For the tag array ``level`` translates directly into the array index.
+
+``offset`` specifies the number of bits to skip from a field's start.
+That allows performing a partial copy of the needed part or to divide a big
+packet field into multiple smaller fields. Alternatively, ``offset`` allows
+going past the specified packet field boundary to copy a field to an
+arbitrary place in a packet, essentially providing a way to copy any part of
+a packet to any other part of it if supported by an underlying PMD driver.
+
+``value`` sets an immediate value to be used as a source or points to a
+location of the value in memory. It is used instead of ``level`` and ``offset``
+for ``RTE_FLOW_FIELD_VALUE`` and ``RTE_FLOW_FIELD_POINTER`` respectively.
+
+.. _table_rte_flow_action_modify_field:
+
+.. table:: MODIFY_FIELD
+
+   +-----------------------------------------+
+   | Field         | Value                   |
+   +===============+=========================+
+   | ``op``        | operation to perform    |
+   | ``dst``       | destination field       |
+   | ``src``       | source field            |
+   | ``width``     | number of bits to use   |
+   +---------------+-------------------------+
+
+.. _table_rte_flow_action_modify_data:
+
+.. table:: destination/source field definition
+
+   +--------------------------------------------------------------------------+
+   | Field         | Value                                                    |
+   +===============+==========================================================+
+   | ``field``     | ID: packet field, mark, meta, tag, immediate, pointer    |
+   | ``level``     | encapsulation level of a packet field or tag array index |
+   | ``offset``    | number of bits to skip at the beginning                  |
+   | ``value``     | immediate value or a pointer to this value               |
+   +---------------+----------------------------------------------------------+
+
 Negative types
 ~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_21_02.rst b/doc/guides/rel_notes/release_21_02.rst
index c64294e7a6..5962c9f80b 100644
--- a/doc/guides/rel_notes/release_21_02.rst
+++ b/doc/guides/rel_notes/release_21_02.rst
@@ -55,6 +55,14 @@  New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added support of modify field action in the flow API.**
+
+  Added modify action support to perform various operations on
+  any arbitrary header field (as well as mark, metadata or tag values):
+  ``RTE_FLOW_ACTION_TYPE_MODIFY_FIELD``.
+  supported operations are: overwriting a field with the content from
+  another field, addition and subtraction using an immediate value.
+
 * **Updated Broadcom bnxt driver.**
 
   Updated the Broadcom bnxt driver with fixes and improvements, including:
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index a06f64c271..9dd051f3c2 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -176,6 +176,8 @@  static const struct rte_flow_desc_data rte_flow_desc_action[] = {
 	MK_FLOW_ACTION(SET_IPV6_DSCP, sizeof(struct rte_flow_action_set_dscp)),
 	MK_FLOW_ACTION(AGE, sizeof(struct rte_flow_action_age)),
 	MK_FLOW_ACTION(SAMPLE, sizeof(struct rte_flow_action_sample)),
+	MK_FLOW_ACTION(MODIFY_FIELD,
+		       sizeof(struct rte_flow_action_modify_field)),
 	/**
 	 * Shared action represented as handle of type
 	 * (struct rte_flow_shared action *) stored in conf field (see
diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index 0977a78270..07c4d859c2 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -2198,6 +2198,17 @@  enum rte_flow_action_type {
 	 * struct rte_flow_shared_action).
 	 */
 	RTE_FLOW_ACTION_TYPE_SHARED,
+
+	/**
+	 * Modify a packet header field, tag, mark or metadata.
+	 *
+	 * Allow the modification of an arbitrary header field via
+	 * set, add and sub operations or copying its content into
+	 * tag, meta or mark for future processing.
+	 *
+	 * See struct rte_flow_action_modify_field.
+	 */
+	RTE_FLOW_ACTION_TYPE_MODIFY_FIELD,
 };
 
 /**
@@ -2777,7 +2788,6 @@  struct rte_flow_action_set_dscp {
 	uint8_t dscp;
 };
 
-
 /**
  * RTE_FLOW_ACTION_TYPE_SHARED
  *
@@ -2791,6 +2801,73 @@  struct rte_flow_action_set_dscp {
  */
 struct rte_flow_shared_action;
 
+enum rte_flow_field_id {
+	RTE_FLOW_FIELD_START = 0,
+	RTE_FLOW_FIELD_MAC_DST,
+	RTE_FLOW_FIELD_MAC_SRC,
+	RTE_FLOW_FIELD_VLAN_TYPE,
+	RTE_FLOW_FIELD_VLAN_ID,
+	RTE_FLOW_FIELD_MAC_TYPE,
+	RTE_FLOW_FIELD_IPV4_DSCP,
+	RTE_FLOW_FIELD_IPV4_TTL,
+	RTE_FLOW_FIELD_IPV4_SRC,
+	RTE_FLOW_FIELD_IPV4_DST,
+	RTE_FLOW_FIELD_IPV6_HOPLIMIT,
+	RTE_FLOW_FIELD_IPV6_SRC,
+	RTE_FLOW_FIELD_IPV6_DST,
+	RTE_FLOW_FIELD_TCP_PORT_SRC,
+	RTE_FLOW_FIELD_TCP_PORT_DST,
+	RTE_FLOW_FIELD_TCP_SEQ_NUM,
+	RTE_FLOW_FIELD_TCP_ACK_NUM,
+	RTE_FLOW_FIELD_TCP_FLAGS,
+	RTE_FLOW_FIELD_UDP_PORT_SRC,
+	RTE_FLOW_FIELD_UDP_PORT_DST,
+	RTE_FLOW_FIELD_VXLAN_VNI,
+	RTE_FLOW_FIELD_GENEVE_VNI,
+	RTE_FLOW_FIELD_GTP_TEID,
+	RTE_FLOW_FIELD_TAG,
+	RTE_FLOW_FIELD_MARK,
+	RTE_FLOW_FIELD_META,
+	RTE_FLOW_FIELD_POINTER,
+	RTE_FLOW_FIELD_VALUE,
+};
+
+struct rte_flow_action_modify_data {
+	enum rte_flow_field_id field;
+	RTE_STD_C11
+	union {
+		struct {
+			uint32_t level;
+			uint32_t offset;
+		};
+		uint64_t value;
+	};
+};
+
+enum rte_flow_modify_op {
+	RTE_FLOW_MODIFY_SET = 0,
+	RTE_FLOW_MODIFY_ADD,
+	RTE_FLOW_MODIFY_SUB,
+};
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this structure may change without prior notice
+ *
+ * RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
+ * Modifies a destination header field according to the specified
+ * operation. Another packet field can be used as a source as well
+ * as tag, mark, metadata or an immediate value or a pointer to it.
+ * Width is the number of bits used from the source item.
+ */
+struct rte_flow_action_modify_field {
+	enum rte_flow_modify_op operation;
+	struct rte_flow_action_modify_data dst;
+	struct rte_flow_action_modify_data src;
+	uint32_t width;
+};
+
 /* Mbuf dynamic field offset for metadata. */
 extern int32_t rte_flow_dynf_metadata_offs;