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

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

[Rahul Lakkireddy]

This action is useful for offloading loopback mode, where the hardware
will swap source and destination MAC addresses in the outermost Ethernet
header 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.

Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
PMDs.

Original work by Shagun Agrawal

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
---
RFC v1: http://mails.dpdk.org/archives/dev/2018-August/110232.html
RFC v2: http://mails.dpdk.org/archives/dev/2018-August/110355.html

Changes since RFC v2:
- Updated release notes.

 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 app/test-pmd/config.c                       |  1 +
 doc/guides/prog_guide/rte_flow.rst          | 19 +++++++++++++++++++
 doc/guides/rel_notes/release_18_11.rst      |  5 +++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +++
 lib/librte_ethdev/rte_flow.c                |  1 +
 lib/librte_ethdev/rte_flow.h                | 11 +++++++++++
 7 files changed, 50 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index f9260600e..196c76de1 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,14 @@ static const struct token token_list[] = {
 		.next = NEXT(NEXT_ENTRY(ACTION_NEXT)),
 		.call = parse_vc,
 	},
+	[ACTION_MAC_SWAP] = {
+		.name = "mac_swap",
+		.help = "Swap the source and destination MAC addresses"
+			" in the outermost Ethernet header",
+		.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 794aa5268..43d803abb 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1172,6 +1172,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..d09806d38 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2076,6 +2076,25 @@ RTE_FLOW_ERROR_TYPE_ACTION error should be returned.
 
 This action modifies the payload of matched flows.
 
+Action: ``MAC_SWAP``
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Swap the source and destination MAC addresses in the outermost Ethernet
+header.
+
+It must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
+Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
+
+.. _table_rte_flow_action_mac_swap:
+
+.. table:: MAC_SWAP
+
+   +---------------+
+   | Field         |
+   +===============+
+   | no properties |
+   +---------------+
+
 Negative types
 ~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index f39cb15d2..83de90bcf 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -87,6 +87,11 @@ New Features
   the specified port. The port must be stopped before the command call in order
   to reconfigure queues.
 
+* **Added new Flow API action to swap MAC addresses in Ethernet header.**
+
+  Added new Flow API action to swap the source and destination MAC
+  addresses in the outermost Ethernet header.
+
 
 API Changes
 -----------
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 3a73000a6..c58b18e1a 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3704,6 +3704,9 @@ 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 the source and destination MAC addresses in the outermost
+  Ethernet header.
+
 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..c743f818e 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1505,6 +1505,17 @@ enum rte_flow_action_type {
 	 * error.
 	 */
 	RTE_FLOW_ACTION_TYPE_NVGRE_DECAP,
+
+	/**
+	 * Swap the source and destination MAC addresses in the outermost
+	 * Ethernet header.
+	 *
+	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
+	 * then the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
+	 *
+	 * No associated configuration structure.
+	 */
+	RTE_FLOW_ACTION_TYPE_MAC_SWAP,
 };
 
 /**
-- 
2.18.0

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

[Andrew Rybchenko]

On 9/24/18 11:29 AM, wrote:
> This action is useful for offloading loopback mode, where the hardware
> will swap source and destination MAC addresses in the outermost Ethernet
> header 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.
>
> Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
> Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
> PMDs.
>
> Original work by Shagun Agrawal
>
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

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

[Ferruh Yigit]

On 9/24/2018 9:32 AM, Andrew Rybchenko wrote:
> On 9/24/18 11:29 AM, wrote:
>> This action is useful for offloading loopback mode, where the hardware
>> will swap source and destination MAC addresses in the outermost Ethernet
>> header 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.
>>
>> Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
>> Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
>> PMDs.
>>
>> Original work by Shagun Agrawal
>>
>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> 
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Same here, can you please rebase the patch and keep the ack?

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

[Rahul Lakkireddy]

This action is useful for offloading loopback mode, where the hardware
will swap source and destination MAC addresses in the outermost Ethernet
header 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.

Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
PMDs.

Original work by Shagun Agrawal

Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
RFC v1: http://mails.dpdk.org/archives/dev/2018-August/110232.html
RFC v2: http://mails.dpdk.org/archives/dev/2018-August/110355.html

v2:
- Rebased to tip.
- Removed adding action to app/test-pmd/config.c, to sync with
  rte_flow_conv() changes.

v1 changes since RFC v2:
- Updated release notes.

 app/test-pmd/cmdline_flow.c                 | 10 ++++++++++
 doc/guides/prog_guide/rte_flow.rst          | 19 +++++++++++++++++++
 doc/guides/rel_notes/release_18_11.rst      |  5 +++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  3 +++
 lib/librte_ethdev/rte_flow.c                |  1 +
 lib/librte_ethdev/rte_flow.h                | 11 +++++++++++
 6 files changed, 49 insertions(+)

diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index a9888cacf..4a2764270 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -255,6 +255,7 @@ enum index {
 	ACTION_SET_TP_SRC_TP_SRC,
 	ACTION_SET_TP_DST,
 	ACTION_SET_TP_DST_TP_DST,
+	ACTION_MAC_SWAP,
 };
 
 /** Maximum size for pattern in struct rte_flow_item_raw. */
@@ -834,6 +835,7 @@ static const enum index next_action[] = {
 	ACTION_SET_IPV6_DST,
 	ACTION_SET_TP_SRC,
 	ACTION_SET_TP_DST,
+	ACTION_MAC_SWAP,
 	ZERO,
 };
 
@@ -2626,6 +2628,14 @@ static const struct token token_list[] = {
 			     (struct rte_flow_action_set_tp, port)),
 		.call = parse_vc_conf,
 	},
+	[ACTION_MAC_SWAP] = {
+		.name = "mac_swap",
+		.help = "Swap the source and destination MAC addresses"
+			" in the outermost Ethernet header",
+		.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/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
index 1c81a824d..a5ec441c9 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -2184,6 +2184,25 @@ flow pattern item. Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
    | ``port`` | new TCP/UDP destination port |
    +---------------+-------------------------+
 
+Action: ``MAC_SWAP``
+^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Swap the source and destination MAC addresses in the outermost Ethernet
+header.
+
+It must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
+Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error will be returned.
+
+.. _table_rte_flow_action_mac_swap:
+
+.. table:: MAC_SWAP
+
+   +---------------+
+   | Field         |
+   +===============+
+   | no properties |
+   +---------------+
+
 Negative types
 ~~~~~~~~~~~~~~
 
diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 9a135c89c..96e09b7b0 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -141,6 +141,11 @@ New Features
   * Modify source and destination port numbers in the outermost TCP/UDP
     headers.
 
+* **Added new Flow API action to swap MAC addresses in Ethernet header.**
+
+  Added new Flow API action to swap the source and destination MAC
+  addresses in the outermost Ethernet header.
+
 API Changes
 -----------
 
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index ffec7013b..ed8aa861b 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3732,6 +3732,9 @@ This section lists supported actions and their attributes, if any.
 
   - ``port``: New TCP/UDP destination port number.
 
+- ``mac_swap``: Swap the source and destination MAC addresses in the outermost
+  Ethernet header.
+
 Destroying flow rules
 ~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index 4eeb392b6..bc9e719dc 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -135,6 +135,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
 		       sizeof(struct rte_flow_action_set_tp)),
 	MK_FLOW_ACTION(SET_TP_DST,
 		       sizeof(struct rte_flow_action_set_tp)),
+	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 ce2021641..02c34917f 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1568,6 +1568,17 @@ enum rte_flow_action_type {
 	 * See struct rte_flow_action_set_tp.
 	 */
 	RTE_FLOW_ACTION_TYPE_SET_TP_DST,
+
+	/**
+	 * Swap the source and destination MAC addresses in the outermost
+	 * Ethernet header.
+	 *
+	 * If flow pattern does not define a valid RTE_FLOW_ITEM_TYPE_ETH,
+	 * then the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION error.
+	 *
+	 * No associated configuration structure.
+	 */
+	RTE_FLOW_ACTION_TYPE_MAC_SWAP,
 };
 
 /**
-- 
2.18.0

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

[Ferruh Yigit]

On 10/6/2018 4:45 PM, Rahul Lakkireddy wrote:
> This action is useful for offloading loopback mode, where the hardware
> will swap source and destination MAC addresses in the outermost Ethernet
> header 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.
> 
> Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
> Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
> PMDs.
> 
> Original work by Shagun Agrawal
> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

Applied to dpdk-next-net/master, thanks.

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

[Ferruh Yigit]

On 9/24/2018 9:29 AM, Rahul Lakkireddy wrote:
> This action is useful for offloading loopback mode, where the hardware
> will swap source and destination MAC addresses in the outermost Ethernet
> header 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.
> 
> Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
> Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
> PMDs.
> 
> Original work by Shagun Agrawal

Do you consider adding his sign off too, that is more formal way we do?

If would like to update we can do it on the next-net before it get pulled.

> 
> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>

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

[Rahul Lakkireddy]

On Tuesday, October 10/09/18, 2018 at 20:19:54 +0530, Ferruh Yigit wrote:
> On 9/24/2018 9:29 AM, Rahul Lakkireddy wrote:
> > This action is useful for offloading loopback mode, where the hardware
> > will swap source and destination MAC addresses in the outermost Ethernet
> > header 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.
> > 
> > Must be used with a valid RTE_FLOW_ITEM_TYPE_ETH flow pattern item.
> > Otherwise, RTE_FLOW_ERROR_TYPE_ACTION error should be returned by the
> > PMDs.
> > 
> > Original work by Shagun Agrawal
> 
> Do you consider adding his sign off too, that is more formal way we do?
> 
> If would like to update we can do it on the next-net before it get pulled.
> 

Shagun is no longer working for Chelsio. He had done the original
implementation and I've re-spun the series incorporating review
comments. I couldn't add his Signed-off-by without his permission on
the later revision series. Hence, the reason for "Original work by"
line above.

> > 
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> 
> 

Thanks,
Rahul