On 11/25/2021 2:13 PM, Slava Ovsiienko wrote:
-----Original Message-----
From: Thomas Monjalon <tho...@monjalon.net>
Sent: Thursday, November 25, 2021 15:07
To: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Ajit Khaparde
<ajit.khapa...@broadcom.com>; Somnath Kotur
<somnath.ko...@broadcom.com>; Rahul Lakkireddy
<rahul.lakkire...@chelsio.com>; Slava Ovsiienko <viachesl...@nvidia.com>;
Ferruh Yigit <ferruh.yi...@intel.com>
Cc: dev@dpdk.org; Ori Kam <or...@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 <viachesl...@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?

Reply via email to