Re: [dpdk-dev] [RFC] ethdev: add action to swap source and destination MAC to flow API

[Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>]

On Tuesday, August 08/28/18, 2018 at 16:27:43 +0530, Andrew Rybchenko wrote:
>    On 08/27/2018 03:54 PM, Rahul Lakkireddy wrote:
> 
>  From: Shagun Agrawal [1]<shaguna@chelsio.com>
> 
>  This action is useful for offloading loopback mode, where the hardware
>  will swap source and destination MAC address before looping back the
>  packet. This action can be used in conjunction with other rewrite
>  actions to achieve MAC layer transparent NAT where the MAC addresses
>  are swapped before either the source or destination MAC address
>  is rewritten and NAT is performed.
> 
>  Signed-off-by: Shagun Agrawal [2]<shaguna@chelsio.com>
>  Signed-off-by: Rahul Lakkireddy [3]<rahul.lakkireddy@chelsio.com>
>  ---
>   app/test-pmd/cmdline_flow.c                 |  9 +++++++++
>   app/test-pmd/config.c                       |  1 +
>   doc/guides/prog_guide/rte_flow.rst          | 15 +++++++++++++++
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst |  2 ++
>   lib/librte_ethdev/rte_flow.c                |  1 +
>   lib/librte_ethdev/rte_flow.h                |  7 +++++++
>   6 files changed, 35 insertions(+)
> 
>  diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>  index f9260600e..4b83b55c4 100644
>  --- a/app/test-pmd/cmdline_flow.c
>  +++ b/app/test-pmd/cmdline_flow.c
>  @@ -243,6 +243,7 @@ enum index {
>          ACTION_VXLAN_DECAP,
>          ACTION_NVGRE_ENCAP,
>          ACTION_NVGRE_DECAP,
>  +   ACTION_MAC_SWAP,
>   };
> 
>   /** Maximum size for pattern in struct rte_flow_item_raw. */
>  @@ -816,6 +817,7 @@ static const enum index next_action[] = {
>          ACTION_VXLAN_DECAP,
>          ACTION_NVGRE_ENCAP,
>          ACTION_NVGRE_DECAP,
>  +   ACTION_MAC_SWAP,
>          ZERO,
>   };
> 
>  @@ -2470,6 +2472,13 @@ static const struct token token_list[] = {
>                  .next = NEXT(NEXT_ENTRY(ACTION_NEXT)),
>                  .call = parse_vc,
>          },
>  +   [ACTION_MAC_SWAP] = {
>  +           .name = "mac_swap",
>  +           .help = "swap source and destination mac address",
>  +           .priv = PRIV_ACTION(MAC_SWAP, 0),
>  +           .next = NEXT(NEXT_ENTRY(ACTION_NEXT)),
>  +           .call = parse_vc,
>  +   },
>   };
> 
>   /** Remove and return last entry from argument stack. */
>  diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>  index 14ccd6864..b7393967a 100644
>  --- a/app/test-pmd/config.c
>  +++ b/app/test-pmd/config.c
>  @@ -1153,6 +1153,7 @@ static const struct {
>                         sizeof(struct rte_flow_action_of_pop_mpls)),
>          MK_FLOW_ACTION(OF_PUSH_MPLS,
>                         sizeof(struct rte_flow_action_of_push_mpls)),
>  +   MK_FLOW_ACTION(MAC_SWAP, 0),
>   };
> 
>   /** Compute storage space needed by action configuration and copy it. */
>  diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
>  index b305a72a5..530dbc504 100644
>  --- a/doc/guides/prog_guide/rte_flow.rst
>  +++ b/doc/guides/prog_guide/rte_flow.rst
>  @@ -2076,6 +2076,21 @@ RTE_FLOW_ERROR_TYPE_ACTION error should be returned.
> 
>   This action modifies the payload of matched flows.
> 
>  +Action: ``MAC_SWAP``
>  +^^^^^^^^^^^^^^^^^^^^^^^^^
>  +
>  +Swap source and destination mac address.
>  +
>  +.. _table_rte_flow_action_mac_swap:
>  +
>  +.. table:: MAC_SWAP
>  +
>  +   +---------------+
>  +   | Field         |
>  +   +===============+
>  +   | no properties |
>  +   +---------------+
>  +
>   Negative types
>   ~~~~~~~~~~~~~~
> 
>  diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>  index dde205a2b..4f0da4fb6 100644
>  --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>  +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>  @@ -3697,6 +3697,8 @@ This section lists supported actions and their attributes, if any.
>   - ``nvgre_decap``: Performs a decapsulation action by stripping all headers of
>     the NVGRE tunnel network overlay from the matched flow.
> 
>  +- ``mac_swap``: Swap source and destination mac address.
>  +
>   Destroying flow rules
>   ~~~~~~~~~~~~~~~~~~~~~
> 
>  diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
>  index cff4b5209..04b0b40ea 100644
>  --- a/lib/librte_ethdev/rte_flow.c
>  +++ b/lib/librte_ethdev/rte_flow.c
>  @@ -109,6 +109,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
>                         sizeof(struct rte_flow_action_of_pop_mpls)),
>          MK_FLOW_ACTION(OF_PUSH_MPLS,
>                         sizeof(struct rte_flow_action_of_push_mpls)),
>  +   MK_FLOW_ACTION(MAC_SWAP, 0),
>   };
> 
>   static int
>  diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>  index f8ba71cdb..e1fa17b7e 100644
>  --- a/lib/librte_ethdev/rte_flow.h
>  +++ b/lib/librte_ethdev/rte_flow.h
>  @@ -1505,6 +1505,13 @@ enum rte_flow_action_type {
>           * error.
>           */
>          RTE_FLOW_ACTION_TYPE_NVGRE_DECAP,
>  +
>  +   /**
>  +    * swap the source and destination mac address in ethernet header
> 
>    Swap the source and destination MAC address in Ethernet header.
> 
>    May be it is useful to highlight that outermost Ethernet header is edited.
>

Makes sense. Will update in v2.

>    MAC address rewrite actions require Ethernet pattern item. Is it required
>    here?
>

Yes, a valid Ethernet pattern item is required here as well. Will update
in v2.

>  +    *
>  +    * No associated configuration structure.
>  +    */
>  +   RTE_FLOW_ACTION_TYPE_MAC_SWAP,
>   };
> 
>   /**
> 
> References
> 
>    Visible links
>    1. mailto:shaguna@chelsio.com
>    2. mailto:shaguna@chelsio.com
>    3. mailto:rahul.lakkireddy@chelsio.com

Thanks,
Rahul