mbox series

[v3,00/12] ethdev: rework transfer flow API

Message ID 20211010143930.4985-1-ivan.malov@oktetlabs.ru (mailing list archive)
Headers
Series ethdev: rework transfer flow API |

Message

Ivan Malov Oct. 10, 2021, 2:39 p.m. UTC
  As per RFC [1], action PORT_ID appears to be ambiguous. Its name suggests
that matching traffic be sent to the ethdev with the specified ID, that
is, to the application. However, in Open vSwitch, the action is used to
send traffic to a remote entity represented by the given port, that is,
in the opposite direction. Its interpretation across PMDs also varies.

RFC [2] attempted to define action PORT_ID semantics in vSwitch sense.
However, this solution would completely abandon the opposite meaning.

One more effort, RFC [3], was meant to declare that the use of direction
attributes in "transfer" flows assumed implicit filtering by the port ID
appearing as the first argument in rte_flow_create(). However, not all
PMDs require such filtering, so the RFC turned out rather disputable.


Since then, all of that has been given more thought:

1. One should not attempt to fix action PORT_ID. Instead, two new actions
   should be introduced. The first one should send traffic to the given
   ethdev. The second one should send it to the represented entity.

2. Similar to (1), two new items should be defined. The first one should
   match traffic going down from the given ethdev. The second one should
   match traffic going up from the entity represented by that ethdev.

3. The application always knows which packets come through which ethdevs.
   So, as per (2), the application can use the new item to match traffic
   arriving from precise entities represented by the relevant ethdev IDs.

4. New items suggested in (2) do not require the use of direction
   attributes. These items define precise directions on their own.

5. As a consequence of (3) and (4), the problem of implicit filtering
   by rte_flow_create() port ID argument and direction attributes is
   no longer a blocker. The new items allow to dispose of it.


The new items appear to be symmetrical to each other. So do the new
actions. This requires that their names reflect the symmetry. Also,
the names should respect the existing concept of port representors.
By the looks of it, terms "PORT_REPRESENTOR" and "REPRESENTED_PORT"
satisfy these requirements. However, currently, ethdevs associated
with network ports are not considered as their representors. Such
understanding is mentioned in the documentation, but it's not
expressed in the code (see enum rte_eth_representor_type).


The short of it, this patch series follows points (1-5) to rework
support for "transfer" flows accordingly. On the way, a string of
ambiguous pattern items (PF, VF, PHY_PORT) is deprecated.

The patch series also updates PMDs which support item and action PORT_ID
to add support for replacements (1-2). However, there're some exceptions:

 - Support for traffic source items in the case of net/mlx5 is really
   complicated. This series does not rework it. The PMD maintainer
   can do the job much better and test the new code accordingly;

 - Support for action REPRESENTED_PORT is not added to net/sfc.
   This will be done when support for VF representors has been
   upstreamed, just for the new code to apply cleanly.


Changes in v2:
* New naming and reworked comments
* New diagrams

Changes in v3:
* Diagram improvements
* Spelling fixes

[1] https://patches.dpdk.org/project/dpdk/patch/20210907125157.3843-1-ivan.malov@oktetlabs.ru/
[2] https://patches.dpdk.org/project/dpdk/patch/20210903074610.313622-1-andrew.rybchenko@oktetlabs.ru/
[3] https://patches.dpdk.org/project/dpdk/patch/20210901151104.3923889-1-andrew.rybchenko@oktetlabs.ru/

Andrew Rybchenko (6):
  net/bnxt: support meta flow items to match on traffic source
  net/bnxt: support meta flow actions to overrule destinations
  net/enic: support meta flow actions to overrule destinations
  net/mlx5: support represented port flow action
  net/octeontx2: support port representor flow action
  net/sfc: support port representor flow item

Ivan Malov (6):
  ethdev: add port representor item to flow API
  ethdev: add represented port item to flow API
  ethdev: add port representor action to flow API
  ethdev: add represented port action to flow API
  ethdev: deprecate hard-to-use or ambiguous items and actions
  ethdev: deprecate direction attributes in transfer flows

 app/test-pmd/cmdline_flow.c                   | 104 +++++++
 doc/guides/nics/mlx5.rst                      |   4 +-
 doc/guides/nics/octeontx2.rst                 |   5 +-
 doc/guides/nics/sfc_efx.rst                   |   2 +
 doc/guides/prog_guide/rte_flow.rst            | 270 +++++++++++++++++-
 doc/guides/rel_notes/deprecation.rst          |  18 +-
 doc/guides/rel_notes/release_21_11.rst        |   8 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst   |  19 ++
 drivers/net/bnxt/tf_ulp/ulp_rte_handler_tbl.c |  22 +-
 drivers/net/bnxt/tf_ulp/ulp_rte_parser.c      | 161 ++++++++---
 drivers/net/bnxt/tf_ulp/ulp_rte_parser.h      |  12 +-
 drivers/net/enic/enic_fm_flow.c               |  93 ++++--
 drivers/net/mlx5/mlx5_flow_dv.c               |  62 +++-
 drivers/net/octeontx2/otx2_flow_parse.c       |  16 +-
 drivers/net/sfc/sfc_mae.c                     |  72 +++++
 lib/ethdev/rte_flow.c                         |   4 +
 lib/ethdev/rte_flow.h                         | 162 ++++++++++-
 17 files changed, 914 insertions(+), 120 deletions(-)
  

Comments

Ferruh Yigit Oct. 12, 2021, 2:45 p.m. UTC | #1
On 10/10/2021 3:39 PM, Ivan Malov wrote:
> As per RFC [1], action PORT_ID appears to be ambiguous. Its name suggests
> that matching traffic be sent to the ethdev with the specified ID, that
> is, to the application. However, in Open vSwitch, the action is used to
> send traffic to a remote entity represented by the given port, that is,
> in the opposite direction. Its interpretation across PMDs also varies.
> 
> RFC [2] attempted to define action PORT_ID semantics in vSwitch sense.
> However, this solution would completely abandon the opposite meaning.
> 
> One more effort, RFC [3], was meant to declare that the use of direction
> attributes in "transfer" flows assumed implicit filtering by the port ID
> appearing as the first argument in rte_flow_create(). However, not all
> PMDs require such filtering, so the RFC turned out rather disputable.
> 
> 
> Since then, all of that has been given more thought:
> 
> 1. One should not attempt to fix action PORT_ID. Instead, two new actions
>     should be introduced. The first one should send traffic to the given
>     ethdev. The second one should send it to the represented entity.
> 
> 2. Similar to (1), two new items should be defined. The first one should
>     match traffic going down from the given ethdev. The second one should
>     match traffic going up from the entity represented by that ethdev.
> 
> 3. The application always knows which packets come through which ethdevs.
>     So, as per (2), the application can use the new item to match traffic
>     arriving from precise entities represented by the relevant ethdev IDs.
> 
> 4. New items suggested in (2) do not require the use of direction
>     attributes. These items define precise directions on their own.
> 
> 5. As a consequence of (3) and (4), the problem of implicit filtering
>     by rte_flow_create() port ID argument and direction attributes is
>     no longer a blocker. The new items allow to dispose of it.
> 
> 
> The new items appear to be symmetrical to each other. So do the new
> actions. This requires that their names reflect the symmetry. Also,
> the names should respect the existing concept of port representors.
> By the looks of it, terms "PORT_REPRESENTOR" and "REPRESENTED_PORT"
> satisfy these requirements. However, currently, ethdevs associated
> with network ports are not considered as their representors. Such
> understanding is mentioned in the documentation, but it's not
> expressed in the code (see enum rte_eth_representor_type).
> 
> 
> The short of it, this patch series follows points (1-5) to rework
> support for "transfer" flows accordingly. On the way, a string of
> ambiguous pattern items (PF, VF, PHY_PORT) is deprecated.
> 
> The patch series also updates PMDs which support item and action PORT_ID
> to add support for replacements (1-2). However, there're some exceptions:
> 
>   - Support for traffic source items in the case of net/mlx5 is really
>     complicated. This series does not rework it. The PMD maintainer
>     can do the job much better and test the new code accordingly;
> 
>   - Support for action REPRESENTED_PORT is not added to net/sfc.
>     This will be done when support for VF representors has been
>     upstreamed, just for the new code to apply cleanly.
> 
> 
> Changes in v2:
> * New naming and reworked comments
> * New diagrams
> 
> Changes in v3:
> * Diagram improvements
> * Spelling fixes
> 
> [1] https://patches.dpdk.org/project/dpdk/patch/20210907125157.3843-1-ivan.malov@oktetlabs.ru/
> [2] https://patches.dpdk.org/project/dpdk/patch/20210903074610.313622-1-andrew.rybchenko@oktetlabs.ru/
> [3] https://patches.dpdk.org/project/dpdk/patch/20210901151104.3923889-1-andrew.rybchenko@oktetlabs.ru/
> 
> Andrew Rybchenko (6):
>    net/bnxt: support meta flow items to match on traffic source
>    net/bnxt: support meta flow actions to overrule destinations
>    net/enic: support meta flow actions to overrule destinations
>    net/mlx5: support represented port flow action
>    net/octeontx2: support port representor flow action
>    net/sfc: support port representor flow item
> 
> Ivan Malov (6):
>    ethdev: add port representor item to flow API
>    ethdev: add represented port item to flow API
>    ethdev: add port representor action to flow API
>    ethdev: add represented port action to flow API
>    ethdev: deprecate hard-to-use or ambiguous items and actions
>    ethdev: deprecate direction attributes in transfer flows
> 

This set also updates the drivers, 'bnxt', 'enic', 'mlx5' & 'octeontx2',
PMD maintainers please review/ack the patches.

If there is no review from PMDs, patch will go in as it is, FYI.
  
Ferruh Yigit Oct. 13, 2021, 11:51 a.m. UTC | #2
On 10/10/2021 3:39 PM, Ivan Malov wrote:
> As per RFC [1], action PORT_ID appears to be ambiguous. Its name suggests
> that matching traffic be sent to the ethdev with the specified ID, that
> is, to the application. However, in Open vSwitch, the action is used to
> send traffic to a remote entity represented by the given port, that is,
> in the opposite direction. Its interpretation across PMDs also varies.
> 
> RFC [2] attempted to define action PORT_ID semantics in vSwitch sense.
> However, this solution would completely abandon the opposite meaning.
> 
> One more effort, RFC [3], was meant to declare that the use of direction
> attributes in "transfer" flows assumed implicit filtering by the port ID
> appearing as the first argument in rte_flow_create(). However, not all
> PMDs require such filtering, so the RFC turned out rather disputable.
> 
> 
> Since then, all of that has been given more thought:
> 
> 1. One should not attempt to fix action PORT_ID. Instead, two new actions
>     should be introduced. The first one should send traffic to the given
>     ethdev. The second one should send it to the represented entity.
> 
> 2. Similar to (1), two new items should be defined. The first one should
>     match traffic going down from the given ethdev. The second one should
>     match traffic going up from the entity represented by that ethdev.
> 
> 3. The application always knows which packets come through which ethdevs.
>     So, as per (2), the application can use the new item to match traffic
>     arriving from precise entities represented by the relevant ethdev IDs.
> 
> 4. New items suggested in (2) do not require the use of direction
>     attributes. These items define precise directions on their own.
> 
> 5. As a consequence of (3) and (4), the problem of implicit filtering
>     by rte_flow_create() port ID argument and direction attributes is
>     no longer a blocker. The new items allow to dispose of it.
> 
> 
> The new items appear to be symmetrical to each other. So do the new
> actions. This requires that their names reflect the symmetry. Also,
> the names should respect the existing concept of port representors.
> By the looks of it, terms "PORT_REPRESENTOR" and "REPRESENTED_PORT"
> satisfy these requirements. However, currently, ethdevs associated
> with network ports are not considered as their representors. Such
> understanding is mentioned in the documentation, but it's not
> expressed in the code (see enum rte_eth_representor_type).
> 
> 
> The short of it, this patch series follows points (1-5) to rework
> support for "transfer" flows accordingly. On the way, a string of
> ambiguous pattern items (PF, VF, PHY_PORT) is deprecated.
> 
> The patch series also updates PMDs which support item and action PORT_ID
> to add support for replacements (1-2). However, there're some exceptions:
> 
>   - Support for traffic source items in the case of net/mlx5 is really
>     complicated. This series does not rework it. The PMD maintainer
>     can do the job much better and test the new code accordingly;
> 
>   - Support for action REPRESENTED_PORT is not added to net/sfc.
>     This will be done when support for VF representors has been
>     upstreamed, just for the new code to apply cleanly.
> 
> 
> Changes in v2:
> * New naming and reworked comments
> * New diagrams
> 
> Changes in v3:
> * Diagram improvements
> * Spelling fixes
> 
> [1] https://patches.dpdk.org/project/dpdk/patch/20210907125157.3843-1-ivan.malov@oktetlabs.ru/
> [2] https://patches.dpdk.org/project/dpdk/patch/20210903074610.313622-1-andrew.rybchenko@oktetlabs.ru/
> [3] https://patches.dpdk.org/project/dpdk/patch/20210901151104.3923889-1-andrew.rybchenko@oktetlabs.ru/
> 
> Andrew Rybchenko (6):
>    net/bnxt: support meta flow items to match on traffic source
>    net/bnxt: support meta flow actions to overrule destinations
>    net/enic: support meta flow actions to overrule destinations
>    net/mlx5: support represented port flow action
>    net/octeontx2: support port representor flow action
>    net/sfc: support port representor flow item
> 
> Ivan Malov (6):
>    ethdev: add port representor item to flow API
>    ethdev: add represented port item to flow API
>    ethdev: add port representor action to flow API
>    ethdev: add represented port action to flow API
>    ethdev: deprecate hard-to-use or ambiguous items and actions
>    ethdev: deprecate direction attributes in transfer flows
> 

The rte flow documentation script is failing [1], should new actions
added to the documentation?

[1]
./devtools/check-doc-vs-code.sh
rte_flow doc out of sync for bnxt
         item port_representor
         item represented_port
         action port_representor
         action represented_port
rte_flow doc out of sync for enic
         action port_representor
         action represented_port
rte_flow doc out of sync for mlx5
         action represented_port
rte_flow doc out of sync for octeontx2
         action port_representor
rte_flow doc out of sync for sfc
         item port_representor