[v3] ethdev: deprecate header fields and metadata flow actions

Message ID 20211124153756.12198-1-viacheslavo@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v3] ethdev: deprecate header fields and metadata flow actions |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Slava Ovsiienko Nov. 24, 2021, 3:37 p.m. UTC
  The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
introduced by [1]. This action provides an unified way
to perform various arithmetic and transfer operations over
packet network header fields and packet metadata.

[1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")

On other side there are a bunch of multiple legacy actions,
that can be superseded by the generic modify field action:

RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
                                        mlx5, octeontx2, sfc
RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
                                        mlx5, octeontx2, sfc
RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
RTE_FLOW_ACTION_TYPE_SET_META           mlx5

This note deprecates the following RTE Flow actions:
1. As not supported by any of PMDs:

RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN

2. As supposed to be replaced by generig field modify action:
RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
RTE_FLOW_ACTION_TYPE_SET_TP_SRC
RTE_FLOW_ACTION_TYPE_SET_TP_DST
RTE_FLOW_ACTION_TYPE_DEC_TTL
RTE_FLOW_ACTION_TYPE_SET_TTL
RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
RTE_FLOW_ACTION_TYPE_SET_MAC_DST
RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
RTE_FLOW_ACTION_TYPE_SET_TAG
RTE_FLOW_ACTION_TYPE_SET_META

The VLAN set actions are interrelated to VLAN header insertion/removal
and supported by multiple PMDs and supposed to be just deprecated but
not be removed in 22.11.

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

--
v2  - deprecation.rst is updated
v3  - doc comments addressed
    - commit message comments addressed
    - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
      be removed in 22.11
---
 doc/guides/prog_guide/rte_flow.rst   |  26 +++++++
 doc/guides/rel_notes/deprecation.rst |  14 ++++
 lib/ethdev/rte_flow.h                | 105 +++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
  

Comments

Thomas Monjalon Nov. 24, 2021, 3:59 p.m. UTC | #1
24/11/2021 16:37, Viacheslav Ovsiienko:
> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> introduced by [1]. This action provides an unified way
> to perform various arithmetic and transfer operations over
> packet network header fields and packet metadata.
> 
> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> 
> On other side there are a bunch of multiple legacy actions,
> that can be superseded by the generic modify field action:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
>                                         mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
>                                         mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> 
> This note deprecates the following RTE Flow actions:
> 1. As not supported by any of PMDs:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> 
> 2. As supposed to be replaced by generig field modify action:
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> RTE_FLOW_ACTION_TYPE_SET_TP_DST
> RTE_FLOW_ACTION_TYPE_DEC_TTL
> RTE_FLOW_ACTION_TYPE_SET_TTL
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> RTE_FLOW_ACTION_TYPE_SET_TAG
> RTE_FLOW_ACTION_TYPE_SET_META
> 
> The VLAN set actions are interrelated to VLAN header insertion/removal
> and supported by multiple PMDs and supposed to be just deprecated but
> not be removed in 22.11.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

Few more infos:
The deprecation is only in comments to suggest a replacement.
It has no impact on applications using old actions.
The drivers should adopt the new API for a better user experience,
and to prepare future removal.

> +* ethdev: Actions ``OF_SET_MPLS_TTL``, ``OF_DEC_MPLS_TTL``, ``OF_SET_NW_TTL``,
> +  ``OF_COPY_TTL_OUT``, ``OF_COPY_TTL_IN`` are deprecated as not supported by
> +  PMDs, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_DEC_NW_TTL``, ``SET_IPV4_SRC``, ``SET_IPV4_DST``,
> +  ``SET_IPV6_SRC``, ``SET_IPV6_DST``, ``SET_TP_SRC``, ``SET_TP_DST``,
> +  ``DEC_TTL``, ``SET_TTL``, ``SET_MAC_SRC``, ``SET_MAC_DST``, ``INC_TCP_SEQ``,
> +  ``DEC_TCP_SEQ``, ``INC_TCP_ACK``, ``DEC_TCP_ACK``, ``SET_IPV4_DSCP``,
> +  ``SET_IPV6_DSCP``, ``SET_TAG``, ``SET_META`` are deprecated as superseded
> +  by generic MODIFY_FIELD action, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_SET_VLAN_VID``, ``OF_SET_VLAN_PCP`` are deprecated
> +  as superseded by generic MODIFY_FIELD action.

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  
Ori Kam Nov. 24, 2021, 4:21 p.m. UTC | #2
Hi Slava,

> -----Original Message-----
> From: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> Sent: Wednesday, November 24, 2021 5:38 PM
> To: dev@dpdk.org
> Cc: ferruh.yigit@intel.com; NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>
> Subject: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
> 
> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> introduced by [1]. This action provides an unified way
> to perform various arithmetic and transfer operations over
> packet network header fields and packet metadata.
> 
> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> 
> On other side there are a bunch of multiple legacy actions,
> that can be superseded by the generic modify field action:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
>                                         mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
>                                         mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> 
> This note deprecates the following RTE Flow actions:
> 1. As not supported by any of PMDs:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> 
> 2. As supposed to be replaced by generig field modify action:
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> RTE_FLOW_ACTION_TYPE_SET_TP_DST
> RTE_FLOW_ACTION_TYPE_DEC_TTL
> RTE_FLOW_ACTION_TYPE_SET_TTL
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> RTE_FLOW_ACTION_TYPE_SET_TAG
> RTE_FLOW_ACTION_TYPE_SET_META
> 
> The VLAN set actions are interrelated to VLAN header insertion/removal
> and supported by multiple PMDs and supposed to be just deprecated but
> not be removed in 22.11.
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> 
> --
> v2  - deprecation.rst is updated
> v3  - doc comments addressed
>     - commit message comments addressed
>     - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
>       be removed in 22.11
> ---
>  doc/guides/prog_guide/rte_flow.rst   |  26 +++++++
>  doc/guides/rel_notes/deprecation.rst |  14 ++++
>  lib/ethdev/rte_flow.h                | 105 +++++++++++++++++++++++++++
>  3 files changed, 145 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 77de8da973..dbed183b6c 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -2238,6 +2238,7 @@ fields in the pattern items.
> 
>  Action: ``OF_SET_MPLS_TTL``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 

Since no PMD support this action why set reference to MODIFY_FIELD_?
Same for all unsupported actions.

>  Implements ``OFPAT_SET_MPLS_TTL`` ("MPLS TTL") as defined by the `OpenFlow
>  Switch Specification`_.
> @@ -2254,6 +2255,7 @@ Switch Specification`_.
> 
>  Action: ``OF_DEC_MPLS_TTL``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_DEC_MPLS_TTL`` ("decrement MPLS TTL") as defined by the
>  `OpenFlow Switch Specification`_.
> @@ -2270,6 +2272,7 @@ Implements ``OFPAT_DEC_MPLS_TTL`` ("decrement MPLS TTL") as defined
> by the
> 
>  Action: ``OF_SET_NW_TTL``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_SET_NW_TTL`` ("IP TTL") as defined by the `OpenFlow
>  Switch Specification`_.
> @@ -2286,6 +2289,7 @@ Switch Specification`_.
> 
>  Action: ``OF_DEC_NW_TTL``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_DEC_NW_TTL`` ("decrement IP TTL") as defined by the
>  `OpenFlow Switch Specification`_.
> @@ -2302,6 +2306,7 @@ Implements ``OFPAT_DEC_NW_TTL`` ("decrement IP TTL") as defined by the
> 
>  Action: ``OF_COPY_TTL_OUT``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_COPY_TTL_OUT`` ("copy TTL "outwards" -- from
>  next-to-outermost to outermost") as defined by the `OpenFlow Switch
> @@ -2319,6 +2324,7 @@ Specification`_.
> 
>  Action: ``OF_COPY_TTL_IN``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_COPY_TTL_IN`` ("copy TTL "inwards" -- from outermost to
>  next-to-outermost") as defined by the `OpenFlow Switch Specification`_.
> @@ -2367,6 +2373,7 @@ Implements ``OFPAT_PUSH_VLAN`` ("push a new VLAN tag") as defined by
> the
> 
>  Action: ``OF_SET_VLAN_VID``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_SET_VLAN_VID`` ("set the 802.1q VLAN id") as defined by
>  the `OpenFlow Switch Specification`_.
> @@ -2383,6 +2390,7 @@ the `OpenFlow Switch Specification`_.
> 
>  Action: ``OF_SET_VLAN_PCP``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Implements ``OFPAT_SET_LAN_PCP`` ("set the 802.1q priority") as defined by
>  the `OpenFlow Switch Specification`_.
> @@ -2589,6 +2597,7 @@ valid packet.
> 
>  Action: ``SET_IPV4_SRC``
>  ^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new IPv4 source address in the outermost IPv4 header.
> 
> @@ -2607,6 +2616,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_IPV4_DST``
>  ^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new IPv4 destination address in the outermost IPv4 header.
> 
> @@ -2625,6 +2635,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_IPV6_SRC``
>  ^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new IPv6 source address in the outermost IPv6 header.
> 
> @@ -2643,6 +2654,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_IPV6_DST``
>  ^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new IPv6 destination address in the outermost IPv6 header.
> 
> @@ -2661,6 +2673,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_TP_SRC``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new source port number in the outermost TCP/UDP header.
> 
> @@ -2679,6 +2692,7 @@ flow pattern item. Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will
> be returned.
> 
>  Action: ``SET_TP_DST``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set a new destination port number in the outermost TCP/UDP header.
> 
> @@ -2716,6 +2730,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``DEC_TTL``
>  ^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Decrease TTL value.
> 
> @@ -2734,6 +2749,7 @@ in pattern, Some PMDs will reject rule because behavior will be undefined.
> 
>  Action: ``SET_TTL``
>  ^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Assigns a new TTL value.
> 
> @@ -2752,6 +2768,7 @@ in pattern, Some PMDs will reject rule because behavior will be undefined.
> 
>  Action: ``SET_MAC_SRC``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set source MAC address.
> 
> @@ -2770,6 +2787,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_MAC_DST``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set destination MAC address.
> 
> @@ -2788,6 +2806,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``INC_TCP_SEQ``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Increase sequence number in the outermost TCP header.
>  Value to increase TCP sequence number by is a big-endian 32 bit integer.
> @@ -2796,6 +2815,7 @@ Using this action on non-matching traffic will result in undefined behavior.
> 
>  Action: ``DEC_TCP_SEQ``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Decrease sequence number in the outermost TCP header.
>  Value to decrease TCP sequence number by is a big-endian 32 bit integer.
> @@ -2804,6 +2824,7 @@ Using this action on non-matching traffic will result in undefined behavior.
> 
>  Action: ``INC_TCP_ACK``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Increase acknowledgment number in the outermost TCP header.
>  Value to increase TCP acknowledgment number by is a big-endian 32 bit integer.
> @@ -2812,6 +2833,7 @@ Using this action on non-matching traffic will result in undefined behavior.
> 
>  Action: ``DEC_TCP_ACK``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Decrease acknowledgment number in the outermost TCP header.
>  Value to decrease TCP acknowledgment number by is a big-endian 32 bit integer.
> @@ -2820,6 +2842,7 @@ Using this action on non-matching traffic will result in undefined behavior.
> 
>  Action: ``SET_TAG``
>  ^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set Tag.
> 
> @@ -2842,6 +2865,7 @@ application. Multiple tags are supported by specifying index.
> 
>  Action: ``SET_META``
>  ^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set metadata. Item ``META`` matches metadata.
> 
> @@ -2876,6 +2900,7 @@ used to connect the Rx and Tx flows if it can be propagated from Rx to Tx
> path.
> 
>  Action: ``SET_IPV4_DSCP``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set IPv4 DSCP.
> 
> @@ -2896,6 +2921,7 @@ Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
> 
>  Action: ``SET_IPV6_DSCP``
>  ^^^^^^^^^^^^^^^^^^^^^^^^^
> +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> 
>  Set IPv6 DSCP.
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 6d087c64ef..d04a606b7d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -101,6 +101,20 @@ Deprecation Notices
>    is deprecated as ambiguous with respect to the embedded switch. The use of
>    these attributes will become invalid starting from DPDK 22.11.
> 
> +* ethdev: Actions ``OF_SET_MPLS_TTL``, ``OF_DEC_MPLS_TTL``, ``OF_SET_NW_TTL``,
> +  ``OF_COPY_TTL_OUT``, ``OF_COPY_TTL_IN`` are deprecated as not supported by
> +  PMDs, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_DEC_NW_TTL``, ``SET_IPV4_SRC``, ``SET_IPV4_DST``,
> +  ``SET_IPV6_SRC``, ``SET_IPV6_DST``, ``SET_TP_SRC``, ``SET_TP_DST``,
> +  ``DEC_TTL``, ``SET_TTL``, ``SET_MAC_SRC``, ``SET_MAC_DST``, ``INC_TCP_SEQ``,
> +  ``DEC_TCP_SEQ``, ``INC_TCP_ACK``, ``DEC_TCP_ACK``, ``SET_IPV4_DSCP``,
> +  ``SET_IPV6_DSCP``, ``SET_TAG``, ``SET_META`` are deprecated as superseded
> +  by generic MODIFY_FIELD action, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_SET_VLAN_VID``, ``OF_SET_VLAN_PCP`` are deprecated
> +  as superseded by generic MODIFY_FIELD action.
> +
>  * cryptodev: Hide structures ``rte_cryptodev_sym_session`` and
>    ``rte_cryptodev_asym_session`` to remove unnecessary indirection between
>    session and the private data of session. An opaque pointer can be exposed
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 650376c16d..42699b5b03 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -2363,6 +2363,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SECURITY,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_SET_MPLS_TTL ("MPLS TTL") as defined by the
>  	 * OpenFlow Switch Specification.
>  	 *
> @@ -2371,6 +2374,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_DEC_MPLS_TTL ("decrement MPLS TTL") as defined
>  	 * by the OpenFlow Switch Specification.
>  	 *
> @@ -2379,6 +2385,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_SET_NW_TTL ("IP TTL") as defined by the OpenFlow
>  	 * Switch Specification.
>  	 *
> @@ -2395,6 +2404,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_COPY_TTL_OUT ("copy TTL "outwards" -- from
>  	 * next-to-outermost to outermost") as defined by the OpenFlow
>  	 * Switch Specification.
> @@ -2404,6 +2416,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_COPY_TTL_IN ("copy TTL "inwards" -- from
>  	 * outermost to next-to-outermost") as defined by the OpenFlow
>  	 * Switch Specification.
> @@ -2429,6 +2444,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_SET_VLAN_VID ("set the 802.1q VLAN ID") as
>  	 * defined by the OpenFlow Switch Specification.
>  	 *
> @@ -2437,6 +2455,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Implements OFPAT_SET_LAN_PCP ("set the 802.1q priority") as
>  	 * defined by the OpenFlow Switch Specification.
>  	 *
> @@ -2509,6 +2530,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_RAW_DECAP,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv4 source address in the outermost IPv4 header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
> @@ -2519,6 +2543,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv4 destination address in the outermost IPv4 header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
> @@ -2529,6 +2556,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_IPV4_DST,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv6 source address in the outermost IPv6 header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
> @@ -2539,6 +2569,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv6 destination address in the outermost IPv6 header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
> @@ -2549,6 +2582,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_IPV6_DST,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify source port number in the outermost TCP/UDP header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_TCP
> @@ -2560,6 +2596,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_TP_SRC,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify destination port number in the outermost TCP/UDP header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_TCP
> @@ -2582,6 +2621,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_MAC_SWAP,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Decrease TTL value directly
>  	 *
>  	 * No associated configuration structure.
> @@ -2589,6 +2631,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_DEC_TTL,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Set TTL value
>  	 *
>  	 * See struct rte_flow_action_set_ttl
> @@ -2596,6 +2641,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_TTL,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Set source MAC address from matched flow.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
> @@ -2606,6 +2654,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Set destination MAC address from matched flow.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
> @@ -2616,6 +2667,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_MAC_DST,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Increase sequence number in the outermost TCP header.
>  	 *
>  	 * Action configuration specifies the value to increase
> @@ -2630,6 +2684,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Decrease sequence number in the outermost TCP header.
>  	 *
>  	 * Action configuration specifies the value to decrease
> @@ -2644,6 +2701,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Increase acknowledgment number in the outermost TCP header.
>  	 *
>  	 * Action configuration specifies the value to increase
> @@ -2658,6 +2718,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_INC_TCP_ACK,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Decrease acknowledgment number in the outermost TCP header.
>  	 *
>  	 * Action configuration specifies the value to decrease
> @@ -2672,6 +2735,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Set Tag.
>  	 *
>  	 * Tag is for internal flow usage only and
> @@ -2682,6 +2748,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_TAG,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Set metadata on ingress or egress path.
>  	 *
>  	 * See struct rte_flow_action_set_meta.
> @@ -2689,6 +2758,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_META,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv4 DSCP in the outermost IP header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
> @@ -2699,6 +2771,9 @@ enum rte_flow_action_type {
>  	RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP,
> 
>  	/**
> +	 * @deprecated
> +	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> +	 *
>  	 * Modify IPv6 DSCP in the outermost IP header.
>  	 *
>  	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
> @@ -3069,6 +3144,9 @@ struct rte_flow_action_security {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *

If no PMD support this why do we want to add it to the modify_field?
Same for all of the unsupported actions.

>   * RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
>   *
>   * Implements OFPAT_SET_MPLS_TTL ("MPLS TTL") as defined by the OpenFlow
> @@ -3079,6 +3157,9 @@ struct rte_flow_action_of_set_mpls_ttl {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
>   *
>   * Implements OFPAT_SET_NW_TTL ("IP TTL") as defined by the OpenFlow Switch
> @@ -3253,6 +3334,9 @@ struct rte_flow_action_raw_decap {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
>   *
> @@ -3268,6 +3352,9 @@ struct rte_flow_action_set_ipv4 {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
>   *
> @@ -3283,6 +3370,9 @@ struct rte_flow_action_set_ipv6 {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
>   *
> @@ -3298,6 +3388,9 @@ struct rte_flow_action_set_tp {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * RTE_FLOW_ACTION_TYPE_SET_TTL
>   *
>   * Set the TTL value directly for IPv4 or IPv6
> @@ -3307,6 +3400,9 @@ struct rte_flow_action_set_ttl {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * RTE_FLOW_ACTION_TYPE_SET_MAC
>   *
>   * Set MAC address from the matched flow
> @@ -3316,6 +3412,9 @@ struct rte_flow_action_set_mac {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
>   *
> @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * @warning
>   * @b EXPERIMENTAL: this structure may change without prior notice
>   *
> @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
>  };
> 
>  /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>   * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>   * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>   *
> --
> 2.18.1

Best,
Ori
  
Slava Ovsiienko Nov. 24, 2021, 4:37 p.m. UTC | #3
Hi, Ori

> -----Original Message-----
> From: Ori Kam <orika@nvidia.com>
> Sent: Wednesday, November 24, 2021 18:21
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: ferruh.yigit@intel.com; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>
> Subject: RE: [PATCH v3] ethdev: deprecate header fields and metadata flow
> actions
> 
> Hi Slava,
> 
> > -----Original Message-----
> > From: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > Sent: Wednesday, November 24, 2021 5:38 PM
> > To: dev@dpdk.org
> > Cc: ferruh.yigit@intel.com; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > <thomas@monjalon.net>
> > Subject: [PATCH v3] ethdev: deprecate header fields and metadata flow
> > actions
> >
[..snip..]

> >
> >  Action: ``OF_SET_MPLS_TTL``
> >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> >
> 
> Since no PMD support this action why set reference to MODIFY_FIELD_?
> Same for all unsupported actions.

There are some references to this action in PMDs (for example bnxt),
but the ENOTSUP returned. I.e., action is in source code, but actually
not supported. Hence (due to this and in general), we can't just occasionally
remove existing actions, we should deprecate these ones first.
Other point - if someone will decide to implement support for this
action in his/her PMD - the deprecation notice will point out the right way -
"consider MODIFY_FIELD."

> 
> >  Implements ``OFPAT_SET_MPLS_TTL`` ("MPLS TTL") as defined by the
> > `OpenFlow  Switch Specification`_.
> > @@ -2254,6 +2255,7 @@ Switch Specification`_.
> >
[..snip..]
> 
> If no PMD support this why do we want to add it to the modify_field?
> Same for all of the unsupported actions.
As I said above - we just propose to consider MODIFY_FIELD actions
if someone has intention to implement deprecated OFPAT_SET_MPLS_TTL.

[..snip..]

> >   *
> > --
> > 2.18.1
> 
> Best,
> Ori

With best regards,
Slava
  
Thomas Monjalon Nov. 24, 2021, 5:32 p.m. UTC | #4
24/11/2021 17:37, Slava Ovsiienko:
> From: Ori Kam <orika@nvidia.com>
> > From: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > >  Action: ``OF_SET_MPLS_TTL``
> > >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> > >
> > 
> > Since no PMD support this action why set reference to MODIFY_FIELD_?
> > Same for all unsupported actions.
> 
> There are some references to this action in PMDs (for example bnxt),
> but the ENOTSUP returned. I.e., action is in source code, but actually
> not supported. Hence (due to this and in general), we can't just occasionally
> remove existing actions, we should deprecate these ones first.
> Other point - if someone will decide to implement support for this
> action in his/her PMD - the deprecation notice will point out the right way -
> "consider MODIFY_FIELD."
> 
> > 
> > >  Implements ``OFPAT_SET_MPLS_TTL`` ("MPLS TTL") as defined by the
> > > `OpenFlow  Switch Specification`_.
> > > @@ -2254,6 +2255,7 @@ Switch Specification`_.
> > >
> [..snip..]
> > 
> > If no PMD support this why do we want to add it to the modify_field?
> > Same for all of the unsupported actions.
> As I said above - we just propose to consider MODIFY_FIELD actions
> if someone has intention to implement deprecated OFPAT_SET_MPLS_TTL.

It was an API, we must guide the API reader in the right direction.
BTW, it can be implemented in a driver which is not yet upstream.
  
Ori Kam Nov. 24, 2021, 6:09 p.m. UTC | #5
Hi

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 24, 2021 7:33 PM
> 
> 24/11/2021 17:37, Slava Ovsiienko:
> > From: Ori Kam <orika@nvidia.com>
> > > From: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > > >  Action: ``OF_SET_MPLS_TTL``
> > > >  ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > +This action is deprecated. Consider `Action: MODIFY_FIELD`_.
> > > >
> > >
> > > Since no PMD support this action why set reference to MODIFY_FIELD_?
> > > Same for all unsupported actions.
> >
> > There are some references to this action in PMDs (for example bnxt),
> > but the ENOTSUP returned. I.e., action is in source code, but actually
> > not supported. Hence (due to this and in general), we can't just occasionally
> > remove existing actions, we should deprecate these first.
> > Other point - if someone will decide to implement support for this
> > action in his/her PMD - the deprecation notice will point out the right way -
> > "consider MODIFY_FIELD."
> >
If someone will want to create such action he can see how all the rest of the actions are done.
I don't think we need to point a user incorrect path.

> > >
> > > >  Implements ``OFPAT_SET_MPLS_TTL`` ("MPLS TTL") as defined by the
> > > > `OpenFlow  Switch Specification`_.
> > > > @@ -2254,6 +2255,7 @@ Switch Specification`_.
> > > >
> > [..snip..]
> > >
> > > If no PMD support this why do we want to add it to the modify_field?
> > > Same for all of the unsupported actions.
> > As I said above - we just propose to consider MODIFY_FIELD actions
> > if someone has intention to implement deprecated OFPAT_SET_MPLS_TTL.
> 
> It was an API, we must guide the API reader in the right direction.
> BTW, it can be implemented in a driver which is not yet upstream.
> 

So you think we should keep it?
The guideline is that if there is no implementation it should not go in.
I see that this API was added in 2018
if someone is using this API let him rise his voice.
  
Ferruh Yigit Nov. 25, 2021, 11:53 a.m. UTC | #6
On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> introduced by [1]. This action provides an unified way
> to perform various arithmetic and transfer operations over
> packet network header fields and packet metadata.
> 
> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> 
> On other side there are a bunch of multiple legacy actions,
> that can be superseded by the generic modify field action:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
>                                          mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
>                                          mlx5, octeontx2, sfc
> RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> 
> This note deprecates the following RTE Flow actions:
> 1. As not supported by any of PMDs:
> 
> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> 
> 2. As supposed to be replaced by generig field modify action:
> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> RTE_FLOW_ACTION_TYPE_SET_TP_DST
> RTE_FLOW_ACTION_TYPE_DEC_TTL
> RTE_FLOW_ACTION_TYPE_SET_TTL
> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> RTE_FLOW_ACTION_TYPE_SET_TAG
> RTE_FLOW_ACTION_TYPE_SET_META
> 
> The VLAN set actions are interrelated to VLAN header insertion/removal
> and supported by multiple PMDs and supposed to be just deprecated but
> not be removed in 22.11.
> 

Why not remove them for v22.11? Do you think PMDs can't change the
existing implementation until 22.11?

> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> 
> --
> v2  - deprecation.rst is updated
> v3  - doc comments addressed
>      - commit message comments addressed
>      - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
>        be removed in 22.11

Deprecated symbols are to prevent new code using them, but for this case
there is no alternative, since PMDs still don't support
'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
This patch is forcing users to use deprecated actions (except from mlx).

What about a slight change:
1- In this release, update header/document as 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
    is preferred way if supported. Instead of deprecating old ones.

2- Have an agreement with PMD maintainers to switch to new action before v22.11,
    and don't accept old action implementation in PMDs anymore.
    Based on agreement update 'deprecation.rst' in this release to note that
    old actions will be removed on v22.11.
    (It would be good to have a check to prevent old actions merged during that time.)

3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
    action will lose the feature.

What do you think?


Andrew, Ajit, Somnath, Rahul,

We need your confirmation for item (2) above, do you have any objection?
  
Somnath Kotur Nov. 25, 2021, 12:03 p.m. UTC | #7
On Thu, Nov 25, 2021 at 5:23 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> > introduced by [1]. This action provides an unified way
> > to perform various arithmetic and transfer operations over
> > packet network header fields and packet metadata.
> >
> > [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> >
> > On other side there are a bunch of multiple legacy actions,
> > that can be superseded by the generic modify field action:
> >
> > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> > RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
> >                                          mlx5, octeontx2, sfc
> > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
> >                                          mlx5, octeontx2, sfc
> > RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> > RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> >
> > This note deprecates the following RTE Flow actions:
> > 1. As not supported by any of PMDs:
> >
> > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> >
> > 2. As supposed to be replaced by generig field modify action:
> > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> > RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> > RTE_FLOW_ACTION_TYPE_SET_TP_DST
> > RTE_FLOW_ACTION_TYPE_DEC_TTL
> > RTE_FLOW_ACTION_TYPE_SET_TTL
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> > RTE_FLOW_ACTION_TYPE_SET_TAG
> > RTE_FLOW_ACTION_TYPE_SET_META
> >
> > The VLAN set actions are interrelated to VLAN header insertion/removal
> > and supported by multiple PMDs and supposed to be just deprecated but
> > not be removed in 22.11.
> >
>
> Why not remove them for v22.11? Do you think PMDs can't change the
> existing implementation until 22.11?
>
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> >
> > --
> > v2  - deprecation.rst is updated
> > v3  - doc comments addressed
> >      - commit message comments addressed
> >      - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
> >        be removed in 22.11
>
> Deprecated symbols are to prevent new code using them, but for this case
> there is no alternative, since PMDs still don't support
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
> This patch is forcing users to use deprecated actions (except from mlx).
>
> What about a slight change:
> 1- In this release, update header/document as 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>     is preferred way if supported. Instead of deprecating old ones.
>
> 2- Have an agreement with PMD maintainers to switch to new action before v22.11,
>     and don't accept old action implementation in PMDs anymore.
>     Based on agreement update 'deprecation.rst' in this release to note that
>     old actions will be removed on v22.11.
>     (It would be good to have a check to prevent old actions merged during that time.)
>
> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
>     action will lose the feature.
>
> What do you think?
>
>
> Andrew, Ajit, Somnath, Rahul,
>
> We need your confirmation for item (2) above, do you have any objection?
I'm okay with item (2) i.e have an agreement to switch to the new
action by 22.11
  
Ferruh Yigit Nov. 25, 2021, 12:18 p.m. UTC | #8
On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
>   };
>   
>   /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>    * @warning
>    * @b EXPERIMENTAL: this structure may change without prior notice
>    *
> @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
>   };
>   
>   /**
> + * @deprecated
> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> + *
>    * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>    * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>    *

Hi Viacheslav, Ori,

Just recognized that some of the "struct rte_flow_*" added as experimental
(experimental note in the struct comment), some without experimental note.

Is it because some forgotten the add the experimental note?

What is the rule to add new "struct rte_flow_*"?


Thanks,
ferruh
  
Ori Kam Nov. 25, 2021, 12:29 p.m. UTC | #9
Hi

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 25, 2021 2:19 PM
> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
> 
> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
> >   };
> >
> >   /**
> > + * @deprecated
> > + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > + *
> >    * @warning
> >    * @b EXPERIMENTAL: this structure may change without prior notice
> >    *
> > @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
> >   };
> >
> >   /**
> > + * @deprecated
> > + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> > + *
> >    * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> >    * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> >    *
> 
> Hi Viacheslav, Ori,
> 
> Just recognized that some of the "struct rte_flow_*" added as experimental
> (experimental note in the struct comment), some without experimental note.
> 
> Is it because some forgotten the add the experimental note?
> 
> What is the rule to add new "struct rte_flow_*"?
> 

In the beginning struct were never experimental,
and in the last few releases we started to put experimental also on structs
which I think is much better since at the end this is a new API and just like any other
new API it is not stable.

Best,
Ori
> 
> Thanks,
> ferruh
  
Ferruh Yigit Nov. 25, 2021, 12:31 p.m. UTC | #10
On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 6d087c64ef..d04a606b7d 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -101,6 +101,20 @@ Deprecation Notices
>     is deprecated as ambiguous with respect to the embedded switch. The use of
>     these attributes will become invalid starting from DPDK 22.11.
>   
> +* ethdev: Actions ``OF_SET_MPLS_TTL``, ``OF_DEC_MPLS_TTL``, ``OF_SET_NW_TTL``,
> +  ``OF_COPY_TTL_OUT``, ``OF_COPY_TTL_IN`` are deprecated as not supported by
> +  PMDs, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_DEC_NW_TTL``, ``SET_IPV4_SRC``, ``SET_IPV4_DST``,
> +  ``SET_IPV6_SRC``, ``SET_IPV6_DST``, ``SET_TP_SRC``, ``SET_TP_DST``,
> +  ``DEC_TTL``, ``SET_TTL``, ``SET_MAC_SRC``, ``SET_MAC_DST``, ``INC_TCP_SEQ``,
> +  ``DEC_TCP_SEQ``, ``INC_TCP_ACK``, ``DEC_TCP_ACK``, ``SET_IPV4_DSCP``,
> +  ``SET_IPV6_DSCP``, ``SET_TAG``, ``SET_META`` are deprecated as superseded
> +  by generic MODIFY_FIELD action, will be removed in DPDK 22.11.
> +
> +* ethdev: Actions ``OF_SET_VLAN_VID``, ``OF_SET_VLAN_PCP`` are deprecated
> +  as superseded by generic MODIFY_FIELD action.
> +


I have a question about ABI/API related issue for rte_flow support,

If a driver removes an flow API item/action support, it directly impacts
the user application. The application previously working may stop working
and require code update, this is something we want to prevent with
ABI policy. And this kind of changes are not caught by our tools.

Do we have a process to deprecate/remove a flow API item/action support?
Like they can be only removed on ABI break release...


Thanks,
ferruh
  
Ferruh Yigit Nov. 25, 2021, 12:46 p.m. UTC | #11
On 11/25/2021 12:29 PM, Ori Kam wrote:
> Hi
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, November 25, 2021 2:19 PM
>> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
>>
>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
>>> @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
>>>    };
>>>
>>>    /**
>>> + * @deprecated
>>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
>>> + *
>>>     * @warning
>>>     * @b EXPERIMENTAL: this structure may change without prior notice
>>>     *
>>> @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
>>>    };
>>>
>>>    /**
>>> + * @deprecated
>>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
>>> + *
>>>     * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>>>     * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>>>     *
>>
>> Hi Viacheslav, Ori,
>>
>> Just recognized that some of the "struct rte_flow_*" added as experimental
>> (experimental note in the struct comment), some without experimental note.
>>
>> Is it because some forgotten the add the experimental note?
>>
>> What is the rule to add new "struct rte_flow_*"?
>>
> 
> In the beginning struct were never experimental,
> and in the last few releases we started to put experimental also on structs
> which I think is much better since at the end this is a new API and just like any other
> new API it is not stable.
> 

Got it, thanks.

So to record for archives, new "struct rte_flow_*" structs should be
experimental by default.

Does it applies to enums too?
  
Thomas Monjalon Nov. 25, 2021, 12:50 p.m. UTC | #12
25/11/2021 13:31, Ferruh Yigit:
> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index 6d087c64ef..d04a606b7d 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -101,6 +101,20 @@ Deprecation Notices
> >     is deprecated as ambiguous with respect to the embedded switch. The use of
> >     these attributes will become invalid starting from DPDK 22.11.
> >   
> > +* ethdev: Actions ``OF_SET_MPLS_TTL``, ``OF_DEC_MPLS_TTL``, ``OF_SET_NW_TTL``,
> > +  ``OF_COPY_TTL_OUT``, ``OF_COPY_TTL_IN`` are deprecated as not supported by
> > +  PMDs, will be removed in DPDK 22.11.
> > +
> > +* ethdev: Actions ``OF_DEC_NW_TTL``, ``SET_IPV4_SRC``, ``SET_IPV4_DST``,
> > +  ``SET_IPV6_SRC``, ``SET_IPV6_DST``, ``SET_TP_SRC``, ``SET_TP_DST``,
> > +  ``DEC_TTL``, ``SET_TTL``, ``SET_MAC_SRC``, ``SET_MAC_DST``, ``INC_TCP_SEQ``,
> > +  ``DEC_TCP_SEQ``, ``INC_TCP_ACK``, ``DEC_TCP_ACK``, ``SET_IPV4_DSCP``,
> > +  ``SET_IPV6_DSCP``, ``SET_TAG``, ``SET_META`` are deprecated as superseded
> > +  by generic MODIFY_FIELD action, will be removed in DPDK 22.11.
> > +
> > +* ethdev: Actions ``OF_SET_VLAN_VID``, ``OF_SET_VLAN_PCP`` are deprecated
> > +  as superseded by generic MODIFY_FIELD action.
> > +
> 
> 
> I have a question about ABI/API related issue for rte_flow support,
> 
> If a driver removes an flow API item/action support, it directly impacts
> the user application. The application previously working may stop working
> and require code update, this is something we want to prevent with
> ABI policy. And this kind of changes are not caught by our tools.
> 
> Do we have a process to deprecate/remove a flow API item/action support?
> Like they can be only removed on ABI break release...

If possible, we should avoid removing them, or dropping support in a driver.
I think removing a feature could be considered only if not too many drivers
use it, or if it becomes a real burden to maintain.
  
Thomas Monjalon Nov. 25, 2021, 1:06 p.m. UTC | #13
25/11/2021 12:53, Ferruh Yigit:
> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> > introduced by [1]. This action provides an unified way
> > to perform various arithmetic and transfer operations over
> > packet network header fields and packet metadata.
> > 
> > [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> > 
> > On other side there are a bunch of multiple legacy actions,
> > that can be superseded by the generic modify field action:
> > 
> > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> > RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
> >                                          mlx5, octeontx2, sfc
> > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
> >                                          mlx5, octeontx2, sfc
> > RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> > RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> > 
> > This note deprecates the following RTE Flow actions:
> > 1. As not supported by any of PMDs:
> > 
> > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> > 
> > 2. As supposed to be replaced by generig field modify action:
> > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> > RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> > RTE_FLOW_ACTION_TYPE_SET_TP_DST
> > RTE_FLOW_ACTION_TYPE_DEC_TTL
> > RTE_FLOW_ACTION_TYPE_SET_TTL
> > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> > RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> > RTE_FLOW_ACTION_TYPE_SET_TAG
> > RTE_FLOW_ACTION_TYPE_SET_META
> > 
> > The VLAN set actions are interrelated to VLAN header insertion/removal
> > and supported by multiple PMDs and supposed to be just deprecated but
> > not be removed in 22.11.
> > 
> 
> Why not remove them for v22.11? Do you think PMDs can't change the
> existing implementation until 22.11?
> 
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > 
> > --
> > v2  - deprecation.rst is updated
> > v3  - doc comments addressed
> >      - commit message comments addressed
> >      - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
> >        be removed in 22.11
> 
> Deprecated symbols are to prevent new code using them, but for this case
> there is no alternative, since PMDs still don't support
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.

This patch is not preventing new code using old actions,
there are just comments to point to the new direction.

> This patch is forcing users to use deprecated actions (except from mlx).

I don't get it.
It is encouraging to use the new generic action,
which is supported only by mlx5 for now.


> What about a slight change:
> 1- In this release, update header/document as 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>     is preferred way if supported. Instead of deprecating old ones.

Deprecation is just a comment, clearly showing that it may be removed in future.
In my opinion, it makes the message simple and clear.


> 2- Have an agreement with PMD maintainers to switch to new action before v22.11,
>     and don't accept old action implementation in PMDs anymore.
>     Based on agreement update 'deprecation.rst' in this release to note that
>     old actions will be removed on v22.11.
>     (It would be good to have a check to prevent old actions merged during that time.)

Not sure I get it.
You want to remove VLAN actions? I think it is premature.

> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
>     action will lose the feature.

The VLAN actions are probably already used a lot in the field.
I would consider removing them only if it becomes a burden to maintain.
  
Slava Ovsiienko Nov. 25, 2021, 1:56 p.m. UTC | #14
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 25, 2021 13:54
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
> <rahul.lakkireddy@chelsio.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> dev@dpdk.org
> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow
> actions
> 
> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> introduced by
> > [1]. This action provides an unified way to perform various arithmetic
> > and transfer operations over packet network header fields and packet
> > metadata.
> >
> > [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> >
> > On other side there are a bunch of multiple legacy actions, that can
> > be superseded by the generic modify field action:
[.. snip ..]
> >
> > The VLAN set actions are interrelated to VLAN header insertion/removal
> > and supported by multiple PMDs and supposed to be just deprecated but
> > not be removed in 22.11.
> >
> 
> Why not remove them for v22.11? Do you think PMDs can't change the
> existing implementation until 22.11?

Yes,  this is a main concern - these actions are supported by multiple PMDs,
and its handling might be a little bit complicated. For example, in mlx5
SET_VLAN_ID can be translated into different HW/FW primitives
depending on presence of PUSH_VLAN action. I think any PMD should do
the check of VLAN actions order for the case of PUSH, replacing with
MODIFY_FIELD would sophisticate the check

In mlx5 we are going to support MODIFY_FIELD and kept SET_VLAN_XX
in any combinations, but, actually, there is no objection about dropping
SET_VLAN_XXX  regarding mlx5. I just would not like to force
the commitments for that from other PMDs.

> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> >
> > --
> > v2  - deprecation.rst is updated
> > v3  - doc comments addressed
> >      - commit message comments addressed
> >      - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
> >        be removed in 22.11
> 
> Deprecated symbols are to prevent new code using them, but for this case
> there is no alternative, since PMDs still don't support
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
> This patch is forcing users to use deprecated actions (except from mlx).
> 
> What about a slight change:
> 1- In this release, update header/document as
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>     is preferred way if supported. Instead of deprecating old ones.
> 
> 2- Have an agreement with PMD maintainers to switch to new action before
> v22.11,
>     and don't accept old action implementation in PMDs anymore.
>     Based on agreement update 'deprecation.rst' in this release to note that
>     old actions will be removed on v22.11.
>     (It would be good to have a check to prevent old actions merged during that
> time.)
> 
> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
>     action will lose the feature.
>
> What do you think?

In my opinion, deprecation warns about coming changes - entity is going to be
removed or changed. It does not force to take some immediate action now,
just to be aware about.

There are two questions:
- do we want to replace a bunch of legacy actions with new one? I suppose - yes, there
  is a lot of advantages
- are we going to do this with MODIFY_FIELD? Yes, this is an intention.

So, we have answers for these questions now, and should handle legacy actions accordingly -
mark as deprecated and provide the clue about MODIFY_FIELD.

The question remaining - how we are going to proceed. Actions can't be removed till
alternative is provided. So, in my understanding the process should be:

A. mark actions as deprecated, to advertise an intention
B. implement MODIFY_FIELD support (maintainers agreement/commitment), assign the due date for this
C. advertise the date of legacy actions removal (if it was not claimed in B)
D. remove legacy actions entirely

We are doing step A now.
Also step B is imposed by advertising the removal date. It is ultimate approach, and is not exactly what we need.
Do we have alternative softened way to encourage PMDs to be updated? No objections to follow 😊.

Summary:
- If we are OK about intention - step A should be taken, we should emit deprecation
- we need to gain "B". Now it is proposed to do with advertising the removal date. Alternatives?

With best regards,
Slava
  
Ori Kam Nov. 25, 2021, 1:56 p.m. UTC | #15
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 25, 2021 2:47 PM
> To: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; David Marchand
> <david.marchand@redhat.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>
> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
> 
> On 11/25/2021 12:29 PM, Ori Kam wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Thursday, November 25, 2021 2:19 PM
> >> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
> >>
> >> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> >>> @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
> >>>    };
> >>>
> >>>    /**
> >>> + * @deprecated
> >>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> >>> + *
> >>>     * @warning
> >>>     * @b EXPERIMENTAL: this structure may change without prior notice
> >>>     *
> >>> @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
> >>>    };
> >>>
> >>>    /**
> >>> + * @deprecated
> >>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
> >>> + *
> >>>     * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> >>>     * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> >>>     *
> >>
> >> Hi Viacheslav, Ori,
> >>
> >> Just recognized that some of the "struct rte_flow_*" added as experimental
> >> (experimental note in the struct comment), some without experimental note.
> >>
> >> Is it because some forgotten the add the experimental note?
> >>
> >> What is the rule to add new "struct rte_flow_*"?
> >>
> >
> > In the beginning struct were never experimental,
> > and in the last few releases we started to put experimental also on structs
> > which I think is much better since at the end this is a new API and just like any other
> > new API it is not stable.
> >
> 
> Got it, thanks.
> 
> So to record for archives, new "struct rte_flow_*" structs should be
> experimental by default.
> 
> Does it applies to enums too?

Yes, I think that anything new is experimental don't you?
  
Slava Ovsiienko Nov. 25, 2021, 2:13 p.m. UTC | #16
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, November 25, 2021 15:07
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
> <rahul.lakkireddy@chelsio.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>
> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow
> actions
> 
> 25/11/2021 12:53, Ferruh Yigit:
> > On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> > > The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> introduced
> > > by [1]. This action provides an unified way to perform various
> > > arithmetic and transfer operations over packet network header fields
> > > and packet metadata.
> > >
> > > [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow
> > > action")
> > >
> > > On other side there are a bunch of multiple legacy actions, that can
> > > be superseded by the generic modify field action:
> > >
> > > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
> > > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
> > > RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
> > > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
> > > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
> > > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
> > > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
> > > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
> > >                                          mlx5, octeontx2, sfc
> > > RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
> > >                                          mlx5, octeontx2, sfc
> > > RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
> > > RTE_FLOW_ACTION_TYPE_SET_META           mlx5
> > >
> > > This note deprecates the following RTE Flow actions:
> > > 1. As not supported by any of PMDs:
> > >
> > > RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
> > > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
> > > RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
> > >
> > > 2. As supposed to be replaced by generig field modify action:
> > > RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
> > > RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> > > RTE_FLOW_ACTION_TYPE_SET_TP_DST
> > > RTE_FLOW_ACTION_TYPE_DEC_TTL
> > > RTE_FLOW_ACTION_TYPE_SET_TTL
> > > RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
> > > RTE_FLOW_ACTION_TYPE_SET_MAC_DST
> > > RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
> > > RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
> > > RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
> > > RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
> > > RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
> > > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
> > > RTE_FLOW_ACTION_TYPE_SET_TAG
> > > RTE_FLOW_ACTION_TYPE_SET_META
> > >
> > > The VLAN set actions are interrelated to VLAN header
> > > insertion/removal and supported by multiple PMDs and supposed to be
> > > just deprecated but not be removed in 22.11.
> > >
> >
> > Why not remove them for v22.11? Do you think PMDs can't change the
> > existing implementation until 22.11?
> >
> > > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > >
> > > --
> > > v2  - deprecation.rst is updated
> > > v3  - doc comments addressed
> > >      - commit message comments addressed
> > >      - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
> > >        be removed in 22.11
> >
> > Deprecated symbols are to prevent new code using them, but for this
> > case there is no alternative, since PMDs still don't support
> > 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
> 
> This patch is not preventing new code using old actions, there are just
> comments to point to the new direction.
> 
> > This patch is forcing users to use deprecated actions (except from mlx).
> 
> I don't get it.
> It is encouraging to use the new generic action, which is supported only by
> mlx5 for now.
> 
> 
> > What about a slight change:
> > 1- In this release, update header/document as
> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
> >     is preferred way if supported. Instead of deprecating old ones.
> 
> Deprecation is just a comment, clearly showing that it may be removed in
> future.
> In my opinion, it makes the message simple and clear.
> 
> 
> > 2- Have an agreement with PMD maintainers to switch to new action before
> v22.11,
> >     and don't accept old action implementation in PMDs anymore.
> >     Based on agreement update 'deprecation.rst' in this release to note that
> >     old actions will be removed on v22.11.
> >     (It would be good to have a check to prevent old actions merged
> > during that time.)
> 
> Not sure I get it.
> You want to remove VLAN actions? I think it is premature.
> 
> > 3- In v22.11, remove old actions, the PMDs that don't support
> MODIFY_FIELD
> >     action will lose the feature.
> 
> The VLAN actions are probably already used a lot in the field.
> I would consider removing them only if it becomes a burden to maintain.

+1
Dropping VLAN might trigger an avalanche of changes in applications - it is supported by multiple PMDs and should be widely engaged.
Other legacy actions are supported by very limited set of drivers and usage area should be smaller, I would say risk is moderate.

With best regards,
Slava
  
Ferruh Yigit Nov. 25, 2021, 2:15 p.m. UTC | #17
On 11/25/2021 1:06 PM, Thomas Monjalon wrote:
> 25/11/2021 12:53, Ferruh Yigit:
>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
>>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
>>> introduced by [1]. This action provides an unified way
>>> to perform various arithmetic and transfer operations over
>>> packet network header fields and packet metadata.
>>>
>>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
>>>
>>> On other side there are a bunch of multiple legacy actions,
>>> that can be superseded by the generic modify field action:
>>>
>>> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
>>> RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
>>> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
>>> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
>>> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
>>>                                           mlx5, octeontx2, sfc
>>> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
>>>                                           mlx5, octeontx2, sfc
>>> RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
>>> RTE_FLOW_ACTION_TYPE_SET_META           mlx5
>>>
>>> This note deprecates the following RTE Flow actions:
>>> 1. As not supported by any of PMDs:
>>>
>>> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
>>>
>>> 2. As supposed to be replaced by generig field modify action:
>>> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
>>> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
>>> RTE_FLOW_ACTION_TYPE_SET_TP_DST
>>> RTE_FLOW_ACTION_TYPE_DEC_TTL
>>> RTE_FLOW_ACTION_TYPE_SET_TTL
>>> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
>>> RTE_FLOW_ACTION_TYPE_SET_MAC_DST
>>> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
>>> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>>> RTE_FLOW_ACTION_TYPE_SET_TAG
>>> RTE_FLOW_ACTION_TYPE_SET_META
>>>
>>> The VLAN set actions are interrelated to VLAN header insertion/removal
>>> and supported by multiple PMDs and supposed to be just deprecated but
>>> not be removed in 22.11.
>>>
>>
>> Why not remove them for v22.11? Do you think PMDs can't change the
>> existing implementation until 22.11?
>>
>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>>>
>>> --
>>> v2  - deprecation.rst is updated
>>> v3  - doc comments addressed
>>>       - commit message comments addressed
>>>       - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
>>>         be removed in 22.11
>>
>> Deprecated symbols are to prevent new code using them, but for this case
>> there is no alternative, since PMDs still don't support
>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
> 
> This patch is not preventing new code using old actions,
> there are just comments to point to the new direction.
> 

Since the 'deprecated' implementation is done as documentation, it doesn't
practically prevent anything, that is true but
if user applications wants to use 'SET_IPV4_SRC' action, she will be have
to use a *deprecated* action for any driver other than mlx.

I just suggest updating the wording so that it still points the direction,
but also clarifies that 'SET_IPV4_SRC' action is still can be used.

Btw, isn't the 'deprecation.rst' to document the long term direction.

>> This patch is forcing users to use deprecated actions (except from mlx).
> 
> I don't get it.
> It is encouraging to use the new generic action,
> which is supported only by mlx5 for now.
> 

That is the problem, we are deprecating old method and new generic
action is only supported by single PMD. And we didn't give enough
time to other PMDs for the implementation.

And it is a headache for application that a feature is supported in two
different ways in different PMDs.
For some PMDs applications should use 'SET_IPV4_SRC' action, for other
it should use 'MODIFY_FIELD' action, to have same rule.

Wouldn't be nice to move slowly:
- add 'MODIFY_FIELD' as experimental
- give time to PMDs to implement it while old ones are still default
- deprecate old actions and make 'MODIFY_FIELD' default
- remove old actions

In this case single PMD is implementing an action and we are making it
default immediately.


> 
>> What about a slight change:
>> 1- In this release, update header/document as 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>>      is preferred way if supported. Instead of deprecating old ones.
> 
> Deprecation is just a comment, clearly showing that it may be removed in future.
> In my opinion, it makes the message simple and clear.
> 

What does '@deprecated' exactly mean?
What is the expectation from application for a '@deprecated' symbol, keep using
it with new code?

> 
>> 2- Have an agreement with PMD maintainers to switch to new action before v22.11,
>>      and don't accept old action implementation in PMDs anymore.
>>      Based on agreement update 'deprecation.rst' in this release to note that
>>      old actions will be removed on v22.11.
>>      (It would be good to have a check to prevent old actions merged during that time.)
> 
> Not sure I get it.
> You want to remove VLAN actions? I think it is premature.
> 

Nothing specific to VLAN here.
Main point is having an agreement with other PMD maintainers on the direction,
since they will need to do some update in their code.

>> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
>>      action will lose the feature.
> 
> The VLAN actions are probably already used a lot in the field.
> I would consider removing them only if it becomes a burden to maintain.
> 

I am not saying anything specific to VLAN.
But for all actions, are you suggesting to keep both 'MODIFY_FIELD' and old
actions support in PMDs?
Since this patch has a deprecation note to remove old actions on v22.11.
  
Ferruh Yigit Nov. 25, 2021, 2:28 p.m. UTC | #18
On 11/25/2021 1:56 PM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, November 25, 2021 2:47 PM
>> To: Ori Kam <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
>> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; David Marchand
>> <david.marchand@redhat.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
>> dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>
>> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
>>
>> On 11/25/2021 12:29 PM, Ori Kam wrote:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>>> Sent: Thursday, November 25, 2021 2:19 PM
>>>> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions
>>>>
>>>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
>>>>> @@ -3331,6 +3430,9 @@ struct rte_flow_action_set_tag {
>>>>>     };
>>>>>
>>>>>     /**
>>>>> + * @deprecated
>>>>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
>>>>> + *
>>>>>      * @warning
>>>>>      * @b EXPERIMENTAL: this structure may change without prior notice
>>>>>      *
>>>>> @@ -3355,6 +3457,9 @@ struct rte_flow_action_set_meta {
>>>>>     };
>>>>>
>>>>>     /**
>>>>> + * @deprecated
>>>>> + * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
>>>>> + *
>>>>>      * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>>>>>      * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>>>>>      *
>>>>
>>>> Hi Viacheslav, Ori,
>>>>
>>>> Just recognized that some of the "struct rte_flow_*" added as experimental
>>>> (experimental note in the struct comment), some without experimental note.
>>>>
>>>> Is it because some forgotten the add the experimental note?
>>>>
>>>> What is the rule to add new "struct rte_flow_*"?
>>>>
>>>
>>> In the beginning struct were never experimental,
>>> and in the last few releases we started to put experimental also on structs
>>> which I think is much better since at the end this is a new API and just like any other
>>> new API it is not stable.
>>>
>>
>> Got it, thanks.
>>
>> So to record for archives, new "struct rte_flow_*" structs should be
>> experimental by default.
>>
>> Does it applies to enums too?
> 
> Yes, I think that anything new is experimental don't you?
> 

I just want to record some guidelines that we can reference later.

So we all can check that new struct & enums are marked as experimental
in rte_flow.h after this point.
  
Ferruh Yigit Nov. 25, 2021, 2:40 p.m. UTC | #19
On 11/25/2021 2:13 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Thursday, November 25, 2021 15:07
>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
>> <rahul.lakkireddy@chelsio.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
>> Ferruh Yigit <ferruh.yigit@intel.com>
>> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>
>> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow
>> actions
>>
>> 25/11/2021 12:53, Ferruh Yigit:
>>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
>>>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
>> introduced
>>>> by [1]. This action provides an unified way to perform various
>>>> arithmetic and transfer operations over packet network header fields
>>>> and packet metadata.
>>>>
>>>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow
>>>> action")
>>>>
>>>> On other side there are a bunch of multiple legacy actions, that can
>>>> be superseded by the generic modify field action:
>>>>
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL      sfc
>>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
>>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC       bnxt, cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST       bnxt, cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC       cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST       cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_TP_SRC         cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_TP_DST         cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_DEC_TTL            mlx5, sfc
>>>> RTE_FLOW_ACTION_TYPE_SET_TTL            mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC        cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_MAC_DST        cxgbe, mlx5
>>>> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ        mlx5
>>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ        mlx5
>>>> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK        mlx5
>>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK        mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP      mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP      mlx5
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID    bnxt, cnxk, cxgbe, enic,
>>>>                                           mlx5, octeontx2, sfc
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP    bnxt, cnxk, cxgbe, enic,
>>>>                                           mlx5, octeontx2, sfc
>>>> RTE_FLOW_ACTION_TYPE_SET_TAG            mlx5
>>>> RTE_FLOW_ACTION_TYPE_SET_META           mlx5
>>>>
>>>> This note deprecates the following RTE Flow actions:
>>>> 1. As not supported by any of PMDs:
>>>>
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
>>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT
>>>> RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_IN
>>>>
>>>> 2. As supposed to be replaced by generig field modify action:
>>>> RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DST
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DST
>>>> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
>>>> RTE_FLOW_ACTION_TYPE_SET_TP_DST
>>>> RTE_FLOW_ACTION_TYPE_DEC_TTL
>>>> RTE_FLOW_ACTION_TYPE_SET_TTL
>>>> RTE_FLOW_ACTION_TYPE_SET_MAC_SRC
>>>> RTE_FLOW_ACTION_TYPE_SET_MAC_DST
>>>> RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ
>>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ
>>>> RTE_FLOW_ACTION_TYPE_INC_TCP_ACK
>>>> RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
>>>> RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
>>>> RTE_FLOW_ACTION_TYPE_SET_TAG
>>>> RTE_FLOW_ACTION_TYPE_SET_META
>>>>
>>>> The VLAN set actions are interrelated to VLAN header
>>>> insertion/removal and supported by multiple PMDs and supposed to be
>>>> just deprecated but not be removed in 22.11.
>>>>
>>>
>>> Why not remove them for v22.11? Do you think PMDs can't change the
>>> existing implementation until 22.11?
>>>
>>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>>>>
>>>> --
>>>> v2  - deprecation.rst is updated
>>>> v3  - doc comments addressed
>>>>       - commit message comments addressed
>>>>       - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
>>>>         be removed in 22.11
>>>
>>> Deprecated symbols are to prevent new code using them, but for this
>>> case there is no alternative, since PMDs still don't support
>>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
>>
>> This patch is not preventing new code using old actions, there are just
>> comments to point to the new direction.
>>
>>> This patch is forcing users to use deprecated actions (except from mlx).
>>
>> I don't get it.
>> It is encouraging to use the new generic action, which is supported only by
>> mlx5 for now.
>>
>>
>>> What about a slight change:
>>> 1- In this release, update header/document as
>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>>>      is preferred way if supported. Instead of deprecating old ones.
>>
>> Deprecation is just a comment, clearly showing that it may be removed in
>> future.
>> In my opinion, it makes the message simple and clear.
>>
>>
>>> 2- Have an agreement with PMD maintainers to switch to new action before
>> v22.11,
>>>      and don't accept old action implementation in PMDs anymore.
>>>      Based on agreement update 'deprecation.rst' in this release to note that
>>>      old actions will be removed on v22.11.
>>>      (It would be good to have a check to prevent old actions merged
>>> during that time.)
>>
>> Not sure I get it.
>> You want to remove VLAN actions? I think it is premature.
>>
>>> 3- In v22.11, remove old actions, the PMDs that don't support
>> MODIFY_FIELD
>>>      action will lose the feature.
>>
>> The VLAN actions are probably already used a lot in the field.
>> I would consider removing them only if it becomes a burden to maintain.
> 
> +1
> Dropping VLAN might trigger an avalanche of changes in applications - it is supported by multiple PMDs and should be widely engaged.
> Other legacy actions are supported by very limited set of drivers and usage area should be smaller, I would say risk is moderate.
> 

Got it, 'SET_VLAN*' is treat differently because its impact can be more.

Can we do the same for other implemented actions, support them longer and
give more time for deprecation.
How big will be the maintenance cost in the PMD?
  
Slava Ovsiienko Nov. 25, 2021, 2:52 p.m. UTC | #20
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, November 25, 2021 16:41
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; NBU-Contact-Thomas
> Monjalon (EXTERNAL) <thomas@monjalon.net>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
> <rahul.lakkireddy@chelsio.com>
> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>
> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow
> actions
> 
> On 11/25/2021 2:13 PM, Slava Ovsiienko wrote:
> >> -----Original Message-----
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >> Sent: Thursday, November 25, 2021 15:07
> >> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
> >> <ajit.khaparde@broadcom.com>; Somnath Kotur
> >> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
> >> <rahul.lakkireddy@chelsio.com>; Slava Ovsiienko
> >> <viacheslavo@nvidia.com>; Ferruh Yigit <ferruh.yigit@intel.com>
> >> Cc: dev@dpdk.org; Ori Kam <orika@nvidia.com>
> >> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata
> >> flow actions
> >>
> >> 25/11/2021 12:53, Ferruh Yigit:
> >>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> >>>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> >> introduced
> >>>> by [1]. This action provides an unified way to perform various
> >>>> arithmetic and transfer operations over packet network header
> >>>> fields and packet metadata.
> >>>>
> >>>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow
> >>>> action")
> >>>>

[..snip..]

> > +1
> > Dropping VLAN might trigger an avalanche of changes in applications - it is
> supported by multiple PMDs and should be widely engaged.
> > Other legacy actions are supported by very limited set of drivers and usage
> area should be smaller, I would say risk is moderate.
> >
> 
> Got it, 'SET_VLAN*' is treat differently because its impact can be more.
> 
> Can we do the same for other implemented actions, support them longer and
> give more time for deprecation.
> How big will be the maintenance cost in the PMD?

Yes, I share your concern. Sure, there should be the time period of handling both
actions. And there is our intention for mlx5 at least. I think development/maintenance 
costs should not raise significantly - the main efforts is supposed for implementing
MODIFY_FIELD action. 

Please, see my previous mail about milestones.

With best regards,
Slava
  
Ferruh Yigit Nov. 25, 2021, 3:14 p.m. UTC | #21
On 11/25/2021 1:56 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, November 25, 2021 13:54
>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>; Somnath Kotur
>> <somnath.kotur@broadcom.com>; Rahul Lakkireddy
>> <rahul.lakkireddy@chelsio.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
>> Cc: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
>> dev@dpdk.org
>> Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow
>> actions
>>
>> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
>>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
>> introduced by
>>> [1]. This action provides an unified way to perform various arithmetic
>>> and transfer operations over packet network header fields and packet
>>> metadata.
>>>
>>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
>>>
>>> On other side there are a bunch of multiple legacy actions, that can
>>> be superseded by the generic modify field action:
> [.. snip ..]
>>>
>>> The VLAN set actions are interrelated to VLAN header insertion/removal
>>> and supported by multiple PMDs and supposed to be just deprecated but
>>> not be removed in 22.11.
>>>
>>
>> Why not remove them for v22.11? Do you think PMDs can't change the
>> existing implementation until 22.11?
> 
> Yes,  this is a main concern - these actions are supported by multiple PMDs,
> and its handling might be a little bit complicated. For example, in mlx5
> SET_VLAN_ID can be translated into different HW/FW primitives
> depending on presence of PUSH_VLAN action. I think any PMD should do
> the check of VLAN actions order for the case of PUSH, replacing with
> MODIFY_FIELD would sophisticate the check
> 
> In mlx5 we are going to support MODIFY_FIELD and kept SET_VLAN_XX
> in any combinations, but, actually, there is no objection about dropping
> SET_VLAN_XXX  regarding mlx5. I just would not like to force
> the commitments for that from other PMDs.
> 
>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>>>
>>> --
>>> v2  - deprecation.rst is updated
>>> v3  - doc comments addressed
>>>       - commit message comments addressed
>>>       - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
>>>         be removed in 22.11
>>
>> Deprecated symbols are to prevent new code using them, but for this case
>> there is no alternative, since PMDs still don't support
>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
>> This patch is forcing users to use deprecated actions (except from mlx).
>>
>> What about a slight change:
>> 1- In this release, update header/document as
>> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
>>      is preferred way if supported. Instead of deprecating old ones.
>>
>> 2- Have an agreement with PMD maintainers to switch to new action before
>> v22.11,
>>      and don't accept old action implementation in PMDs anymore.
>>      Based on agreement update 'deprecation.rst' in this release to note that
>>      old actions will be removed on v22.11.
>>      (It would be good to have a check to prevent old actions merged during that
>> time.)
>>
>> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
>>      action will lose the feature.
>>
>> What do you think?
> 
> In my opinion, deprecation warns about coming changes - entity is going to be
> removed or changed. It does not force to take some immediate action now,
> just to be aware about.
> 
> There are two questions:
> - do we want to replace a bunch of legacy actions with new one? I suppose - yes, there
>    is a lot of advantages
> - are we going to do this with MODIFY_FIELD? Yes, this is an intention.
> 
> So, we have answers for these questions now, and should handle legacy actions accordingly -
> mark as deprecated and provide the clue about MODIFY_FIELD.
> 
> The question remaining - how we are going to proceed. Actions can't be removed till
> alternative is provided. So, in my understanding the process should be:
> 
> A. mark actions as deprecated, to advertise an intention
> B. implement MODIFY_FIELD support (maintainers agreement/commitment), assign the due date for this
> C. advertise the date of legacy actions removal (if it was not claimed in B)
> D. remove legacy actions entirely
> 
> We are doing step A now.
> Also step B is imposed by advertising the removal date. It is ultimate approach, and is not exactly what we need.
> Do we have alternative softened way to encourage PMDs to be updated? No objections to follow 😊.
> 

We are aligned on the intention.

For (A), those symbols interface with both drivers and applications,
for drivers no concern on deprecating the symbols but I think it is wrong
to deprecate from applications perspective.

Slightly updated version:
A. document legacy actions as not preferred way, but applications still can use them,
    and no change is required by applications (unless they prefer to use new action).
    For PMDs we don't allow any more implementation of these actions.
B. PMDs implement 'MODIFY_FIELD', have a deadline for it (same with your B.)
    Assuming deadline is v22.11, announce that old actions will be deprecated
    on v22.11 in this release.
C. on v22.11 deprecate old actions, exiting applications still can use them
    but new code should use 'MODIFY_FIELD' action.
    Announce removal date for the legacy actions
D. Remove legacy actions entirely

Down side is, from this release to until legacy actions are removed, PMD
needs to maintain both legacy and 'MODIFY_FIELD' actions.
But hopefully can provide smooth transitions for the applications.

What do you think?

btw, this for the legacy actions that is implemented at least by one PMD,
we can remove the ones that are not implemented.

> Summary:
> - If we are OK about intention - step A should be taken, we should emit deprecation
> - we need to gain "B". Now it is proposed to do with advertising the removal date. Alternatives?
> 
> With best regards,
> Slava
>
  
Thomas Monjalon Nov. 25, 2021, 3:53 p.m. UTC | #22
25/11/2021 16:14, Ferruh Yigit:
> On 11/25/2021 1:56 PM, Slava Ovsiienko wrote:
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote:
> >>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was
> >> introduced by
> >>> [1]. This action provides an unified way to perform various arithmetic
> >>> and transfer operations over packet network header fields and packet
> >>> metadata.
> >>>
> >>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action")
> >>>
> >>> On other side there are a bunch of multiple legacy actions, that can
> >>> be superseded by the generic modify field action:
> > [.. snip ..]
> >>>
> >>> The VLAN set actions are interrelated to VLAN header insertion/removal
> >>> and supported by multiple PMDs and supposed to be just deprecated but
> >>> not be removed in 22.11.
> >>>
> >>
> >> Why not remove them for v22.11? Do you think PMDs can't change the
> >> existing implementation until 22.11?
> > 
> > Yes,  this is a main concern - these actions are supported by multiple PMDs,
> > and its handling might be a little bit complicated. For example, in mlx5
> > SET_VLAN_ID can be translated into different HW/FW primitives
> > depending on presence of PUSH_VLAN action. I think any PMD should do
> > the check of VLAN actions order for the case of PUSH, replacing with
> > MODIFY_FIELD would sophisticate the check
> > 
> > In mlx5 we are going to support MODIFY_FIELD and kept SET_VLAN_XX
> > in any combinations, but, actually, there is no objection about dropping
> > SET_VLAN_XXX  regarding mlx5. I just would not like to force
> > the commitments for that from other PMDs.
> > 
> >>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> >>>
> >>> --
> >>> v2  - deprecation.rst is updated
> >>> v3  - doc comments addressed
> >>>       - commit message comments addressed
> >>>       - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not
> >>>         be removed in 22.11
> >>
> >> Deprecated symbols are to prevent new code using them, but for this case
> >> there is no alternative, since PMDs still don't support
> >> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet.
> >> This patch is forcing users to use deprecated actions (except from mlx).
> >>
> >> What about a slight change:
> >> 1- In this release, update header/document as
> >> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD'
> >>      is preferred way if supported. Instead of deprecating old ones.
> >>
> >> 2- Have an agreement with PMD maintainers to switch to new action before
> >> v22.11,
> >>      and don't accept old action implementation in PMDs anymore.
> >>      Based on agreement update 'deprecation.rst' in this release to note that
> >>      old actions will be removed on v22.11.
> >>      (It would be good to have a check to prevent old actions merged during that
> >> time.)
> >>
> >> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_FIELD
> >>      action will lose the feature.
> >>
> >> What do you think?
> > 
> > In my opinion, deprecation warns about coming changes - entity is going to be
> > removed or changed. It does not force to take some immediate action now,
> > just to be aware about.
> > 
> > There are two questions:
> > - do we want to replace a bunch of legacy actions with new one? I suppose - yes, there
> >    is a lot of advantages
> > - are we going to do this with MODIFY_FIELD? Yes, this is an intention.
> > 
> > So, we have answers for these questions now, and should handle legacy actions accordingly -
> > mark as deprecated and provide the clue about MODIFY_FIELD.
> > 
> > The question remaining - how we are going to proceed. Actions can't be removed till
> > alternative is provided. So, in my understanding the process should be:
> > 
> > A. mark actions as deprecated, to advertise an intention
> > B. implement MODIFY_FIELD support (maintainers agreement/commitment), assign the due date for this
> > C. advertise the date of legacy actions removal (if it was not claimed in B)
> > D. remove legacy actions entirely
> > 
> > We are doing step A now.
> > Also step B is imposed by advertising the removal date. It is ultimate approach, and is not exactly what we need.
> > Do we have alternative softened way to encourage PMDs to be updated? No objections to follow 😊.
> > 
> 
> We are aligned on the intention.

Yes we are mostly aligned,
but we do not put the same meaning to word "deprecate".
First of all, "deprecate" does not mean "removed" or "forbidden".
The intention is to warn that it is not the latest or future-proof API.

> For (A), those symbols interface with both drivers and applications,
> for drivers no concern on deprecating the symbols but I think it is wrong
> to deprecate from applications perspective.
> 
> Slightly updated version:
> A. document legacy actions as not preferred way, but applications still can use them,

Of course applications can use them, because they are not removed.
If we don't agree on the word "deprecated", we can reword to something like that:
"This is a legacy API, please consider using MODIFY_FIELD action".

>     and no change is required by applications (unless they prefer to use new action).
>     For PMDs we don't allow any more implementation of these actions.

Yes we should not allow new implementation of the legacy API.

> B. PMDs implement 'MODIFY_FIELD', have a deadline for it (same with your B.)
>     Assuming deadline is v22.11, announce that old actions will be deprecated
>     on v22.11 in this release.
> C. on v22.11 deprecate old actions, exiting applications still can use them
>     but new code should use 'MODIFY_FIELD' action.
>     Announce removal date for the legacy actions

I think we should remove most of the actions in 22.11, except VLAN ones.

> D. Remove legacy actions entirely

Maybe we will keep VLAN actions forever.
  

Patch

diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 77de8da973..dbed183b6c 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2238,6 +2238,7 @@  fields in the pattern items.
 
 Action: ``OF_SET_MPLS_TTL``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_SET_MPLS_TTL`` ("MPLS TTL") as defined by the `OpenFlow
 Switch Specification`_.
@@ -2254,6 +2255,7 @@  Switch Specification`_.
 
 Action: ``OF_DEC_MPLS_TTL``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_DEC_MPLS_TTL`` ("decrement MPLS TTL") as defined by the
 `OpenFlow Switch Specification`_.
@@ -2270,6 +2272,7 @@  Implements ``OFPAT_DEC_MPLS_TTL`` ("decrement MPLS TTL") as defined by the
 
 Action: ``OF_SET_NW_TTL``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_SET_NW_TTL`` ("IP TTL") as defined by the `OpenFlow
 Switch Specification`_.
@@ -2286,6 +2289,7 @@  Switch Specification`_.
 
 Action: ``OF_DEC_NW_TTL``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_DEC_NW_TTL`` ("decrement IP TTL") as defined by the
 `OpenFlow Switch Specification`_.
@@ -2302,6 +2306,7 @@  Implements ``OFPAT_DEC_NW_TTL`` ("decrement IP TTL") as defined by the
 
 Action: ``OF_COPY_TTL_OUT``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_COPY_TTL_OUT`` ("copy TTL "outwards" -- from
 next-to-outermost to outermost") as defined by the `OpenFlow Switch
@@ -2319,6 +2324,7 @@  Specification`_.
 
 Action: ``OF_COPY_TTL_IN``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_COPY_TTL_IN`` ("copy TTL "inwards" -- from outermost to
 next-to-outermost") as defined by the `OpenFlow Switch Specification`_.
@@ -2367,6 +2373,7 @@  Implements ``OFPAT_PUSH_VLAN`` ("push a new VLAN tag") as defined by the
 
 Action: ``OF_SET_VLAN_VID``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_SET_VLAN_VID`` ("set the 802.1q VLAN id") as defined by
 the `OpenFlow Switch Specification`_.
@@ -2383,6 +2390,7 @@  the `OpenFlow Switch Specification`_.
 
 Action: ``OF_SET_VLAN_PCP``
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Implements ``OFPAT_SET_LAN_PCP`` ("set the 802.1q priority") as defined by
 the `OpenFlow Switch Specification`_.
@@ -2589,6 +2597,7 @@  valid packet.
 
 Action: ``SET_IPV4_SRC``
 ^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new IPv4 source address in the outermost IPv4 header.
 
@@ -2607,6 +2616,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_IPV4_DST``
 ^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new IPv4 destination address in the outermost IPv4 header.
 
@@ -2625,6 +2635,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_IPV6_SRC``
 ^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new IPv6 source address in the outermost IPv6 header.
 
@@ -2643,6 +2654,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_IPV6_DST``
 ^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new IPv6 destination address in the outermost IPv6 header.
 
@@ -2661,6 +2673,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_TP_SRC``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new source port number in the outermost TCP/UDP header.
 
@@ -2679,6 +2692,7 @@  flow pattern item. Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_TP_DST``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set a new destination port number in the outermost TCP/UDP header.
 
@@ -2716,6 +2730,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``DEC_TTL``
 ^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Decrease TTL value.
 
@@ -2734,6 +2749,7 @@  in pattern, Some PMDs will reject rule because behavior will be undefined.
 
 Action: ``SET_TTL``
 ^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Assigns a new TTL value.
 
@@ -2752,6 +2768,7 @@  in pattern, Some PMDs will reject rule because behavior will be undefined.
 
 Action: ``SET_MAC_SRC``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set source MAC address.
 
@@ -2770,6 +2787,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_MAC_DST``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set destination MAC address.
 
@@ -2788,6 +2806,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``INC_TCP_SEQ``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Increase sequence number in the outermost TCP header.
 Value to increase TCP sequence number by is a big-endian 32 bit integer.
@@ -2796,6 +2815,7 @@  Using this action on non-matching traffic will result in undefined behavior.
 
 Action: ``DEC_TCP_SEQ``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Decrease sequence number in the outermost TCP header.
 Value to decrease TCP sequence number by is a big-endian 32 bit integer.
@@ -2804,6 +2824,7 @@  Using this action on non-matching traffic will result in undefined behavior.
 
 Action: ``INC_TCP_ACK``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Increase acknowledgment number in the outermost TCP header.
 Value to increase TCP acknowledgment number by is a big-endian 32 bit integer.
@@ -2812,6 +2833,7 @@  Using this action on non-matching traffic will result in undefined behavior.
 
 Action: ``DEC_TCP_ACK``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Decrease acknowledgment number in the outermost TCP header.
 Value to decrease TCP acknowledgment number by is a big-endian 32 bit integer.
@@ -2820,6 +2842,7 @@  Using this action on non-matching traffic will result in undefined behavior.
 
 Action: ``SET_TAG``
 ^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set Tag.
 
@@ -2842,6 +2865,7 @@  application. Multiple tags are supported by specifying index.
 
 Action: ``SET_META``
 ^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set metadata. Item ``META`` matches metadata.
 
@@ -2876,6 +2900,7 @@  used to connect the Rx and Tx flows if it can be propagated from Rx to Tx path.
 
 Action: ``SET_IPV4_DSCP``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set IPv4 DSCP.
 
@@ -2896,6 +2921,7 @@  Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
 
 Action: ``SET_IPV6_DSCP``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
+This action is deprecated. Consider `Action: MODIFY_FIELD`_.
 
 Set IPv6 DSCP.
 
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 6d087c64ef..d04a606b7d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -101,6 +101,20 @@  Deprecation Notices
   is deprecated as ambiguous with respect to the embedded switch. The use of
   these attributes will become invalid starting from DPDK 22.11.
 
+* ethdev: Actions ``OF_SET_MPLS_TTL``, ``OF_DEC_MPLS_TTL``, ``OF_SET_NW_TTL``,
+  ``OF_COPY_TTL_OUT``, ``OF_COPY_TTL_IN`` are deprecated as not supported by
+  PMDs, will be removed in DPDK 22.11.
+
+* ethdev: Actions ``OF_DEC_NW_TTL``, ``SET_IPV4_SRC``, ``SET_IPV4_DST``,
+  ``SET_IPV6_SRC``, ``SET_IPV6_DST``, ``SET_TP_SRC``, ``SET_TP_DST``,
+  ``DEC_TTL``, ``SET_TTL``, ``SET_MAC_SRC``, ``SET_MAC_DST``, ``INC_TCP_SEQ``,
+  ``DEC_TCP_SEQ``, ``INC_TCP_ACK``, ``DEC_TCP_ACK``, ``SET_IPV4_DSCP``,
+  ``SET_IPV6_DSCP``, ``SET_TAG``, ``SET_META`` are deprecated as superseded
+  by generic MODIFY_FIELD action, will be removed in DPDK 22.11.
+
+* ethdev: Actions ``OF_SET_VLAN_VID``, ``OF_SET_VLAN_PCP`` are deprecated
+  as superseded by generic MODIFY_FIELD action.
+
 * cryptodev: Hide structures ``rte_cryptodev_sym_session`` and
   ``rte_cryptodev_asym_session`` to remove unnecessary indirection between
   session and the private data of session. An opaque pointer can be exposed
diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
index 650376c16d..42699b5b03 100644
--- a/lib/ethdev/rte_flow.h
+++ b/lib/ethdev/rte_flow.h
@@ -2363,6 +2363,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SECURITY,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_SET_MPLS_TTL ("MPLS TTL") as defined by the
 	 * OpenFlow Switch Specification.
 	 *
@@ -2371,6 +2374,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_DEC_MPLS_TTL ("decrement MPLS TTL") as defined
 	 * by the OpenFlow Switch Specification.
 	 *
@@ -2379,6 +2385,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_DEC_MPLS_TTL,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_SET_NW_TTL ("IP TTL") as defined by the OpenFlow
 	 * Switch Specification.
 	 *
@@ -2395,6 +2404,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_DEC_NW_TTL,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_COPY_TTL_OUT ("copy TTL "outwards" -- from
 	 * next-to-outermost to outermost") as defined by the OpenFlow
 	 * Switch Specification.
@@ -2404,6 +2416,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_COPY_TTL_OUT,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_COPY_TTL_IN ("copy TTL "inwards" -- from
 	 * outermost to next-to-outermost") as defined by the OpenFlow
 	 * Switch Specification.
@@ -2429,6 +2444,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_SET_VLAN_VID ("set the 802.1q VLAN ID") as
 	 * defined by the OpenFlow Switch Specification.
 	 *
@@ -2437,6 +2455,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Implements OFPAT_SET_LAN_PCP ("set the 802.1q priority") as
 	 * defined by the OpenFlow Switch Specification.
 	 *
@@ -2509,6 +2530,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_RAW_DECAP,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv4 source address in the outermost IPv4 header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
@@ -2519,6 +2543,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv4 destination address in the outermost IPv4 header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
@@ -2529,6 +2556,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_IPV4_DST,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv6 source address in the outermost IPv6 header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
@@ -2539,6 +2569,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv6 destination address in the outermost IPv6 header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
@@ -2549,6 +2582,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_IPV6_DST,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify source port number in the outermost TCP/UDP header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_TCP
@@ -2560,6 +2596,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_TP_SRC,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify destination port number in the outermost TCP/UDP header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_TCP
@@ -2582,6 +2621,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_MAC_SWAP,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Decrease TTL value directly
 	 *
 	 * No associated configuration structure.
@@ -2589,6 +2631,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_DEC_TTL,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Set TTL value
 	 *
 	 * See struct rte_flow_action_set_ttl
@@ -2596,6 +2641,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_TTL,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Set source MAC address from matched flow.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
@@ -2606,6 +2654,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_MAC_SRC,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Set destination MAC address from matched flow.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
@@ -2616,6 +2667,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_MAC_DST,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Increase sequence number in the outermost TCP header.
 	 *
 	 * Action configuration specifies the value to increase
@@ -2630,6 +2684,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Decrease sequence number in the outermost TCP header.
 	 *
 	 * Action configuration specifies the value to decrease
@@ -2644,6 +2701,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_DEC_TCP_SEQ,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Increase acknowledgment number in the outermost TCP header.
 	 *
 	 * Action configuration specifies the value to increase
@@ -2658,6 +2718,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_INC_TCP_ACK,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Decrease acknowledgment number in the outermost TCP header.
 	 *
 	 * Action configuration specifies the value to decrease
@@ -2672,6 +2735,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_DEC_TCP_ACK,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Set Tag.
 	 *
 	 * Tag is for internal flow usage only and
@@ -2682,6 +2748,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_TAG,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Set metadata on ingress or egress path.
 	 *
 	 * See struct rte_flow_action_set_meta.
@@ -2689,6 +2758,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_META,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv4 DSCP in the outermost IP header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV4,
@@ -2699,6 +2771,9 @@  enum rte_flow_action_type {
 	RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP,
 
 	/**
+	 * @deprecated
+	 * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+	 *
 	 * Modify IPv6 DSCP in the outermost IP header.
 	 *
 	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_IPV6,
@@ -3069,6 +3144,9 @@  struct rte_flow_action_security {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * RTE_FLOW_ACTION_TYPE_OF_SET_MPLS_TTL
  *
  * Implements OFPAT_SET_MPLS_TTL ("MPLS TTL") as defined by the OpenFlow
@@ -3079,6 +3157,9 @@  struct rte_flow_action_of_set_mpls_ttl {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * RTE_FLOW_ACTION_TYPE_OF_SET_NW_TTL
  *
  * Implements OFPAT_SET_NW_TTL ("IP TTL") as defined by the OpenFlow Switch
@@ -3253,6 +3334,9 @@  struct rte_flow_action_raw_decap {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
  *
@@ -3268,6 +3352,9 @@  struct rte_flow_action_set_ipv4 {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
  *
@@ -3283,6 +3370,9 @@  struct rte_flow_action_set_ipv6 {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
  *
@@ -3298,6 +3388,9 @@  struct rte_flow_action_set_tp {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * RTE_FLOW_ACTION_TYPE_SET_TTL
  *
  * Set the TTL value directly for IPv4 or IPv6
@@ -3307,6 +3400,9 @@  struct rte_flow_action_set_ttl {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * RTE_FLOW_ACTION_TYPE_SET_MAC
  *
  * Set MAC address from the matched flow
@@ -3316,6 +3412,9 @@  struct rte_flow_action_set_mac {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
  *
@@ -3331,6 +3430,9 @@  struct rte_flow_action_set_tag {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * @warning
  * @b EXPERIMENTAL: this structure may change without prior notice
  *
@@ -3355,6 +3457,9 @@  struct rte_flow_action_set_meta {
 };
 
 /**
+ * @deprecated
+ * @see RTE_FLOW_ACTION_TYPE_MODIFY_FIELD
+ *
  * RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP
  * RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP
  *