[v3,1/3] examples/l3fwd-acl: add source and destination MAC update

Message ID 20201006171618.19374-2-konstantin.ananyev@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series Few enhancements for l3fwd-acl |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ananyev, Konstantin Oct. 6, 2020, 5:16 p.m. UTC
  Introduces two changes into l3fwd-acl behaviour to make
it behave in the same way as l3fwd:
 - Add a command-line parameter to allow the user to specify the
   destination mac address for each ethernet port used.
 - While forwarding the packet update source and destination mac
   addresses.

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 doc/guides/rel_notes/release_20_11.rst        |  5 ++
 .../sample_app_ug/l3_forward_access_ctrl.rst  |  4 +-
 examples/l3fwd-acl/main.c                     | 51 +++++++++++++++++--
 3 files changed, 54 insertions(+), 6 deletions(-)
  

Comments

David Marchand Oct. 15, 2020, 11:58 a.m. UTC | #1
On Tue, Oct 6, 2020 at 7:17 PM Konstantin Ananyev
<konstantin.ananyev@intel.com> wrote:
>
> Introduces two changes into l3fwd-acl behaviour to make
> it behave in the same way as l3fwd:
>  - Add a command-line parameter to allow the user to specify the
>    destination mac address for each ethernet port used.
>  - While forwarding the packet update source and destination mac
>    addresses.

This new parameter is optional, but I see no default for l2
destination in the patch.
How does it work when you won't set this option?


Bonus question, what keeps us from merging this example with l3fwd?
  
Ananyev, Konstantin Oct. 15, 2020, 1:06 p.m. UTC | #2
> On Tue, Oct 6, 2020 at 7:17 PM Konstantin Ananyev
> <konstantin.ananyev@intel.com> wrote:
> >
> > Introduces two changes into l3fwd-acl behaviour to make
> > it behave in the same way as l3fwd:
> >  - Add a command-line parameter to allow the user to specify the
> >    destination mac address for each ethernet port used.
> >  - While forwarding the packet update source and destination mac
> >    addresses.
> 
> This new parameter is optional, but I see no default for l2
> destination in the patch.
> How does it work when you won't set this option?

Ah yes, forgot to init l2 dst array with some default values.
Will fix and re-spin then.

> 
> 
> Bonus question, what keeps us from merging this example with l3fwd?

Nothing, except the effort required.
  
David Marchand Oct. 16, 2020, 12:19 p.m. UTC | #3
On Thu, Oct 15, 2020 at 3:06 PM Ananyev, Konstantin
<konstantin.ananyev@intel.com> wrote:
> > On Tue, Oct 6, 2020 at 7:17 PM Konstantin Ananyev
> > <konstantin.ananyev@intel.com> wrote:
> > >
> > > Introduces two changes into l3fwd-acl behaviour to make
> > > it behave in the same way as l3fwd:
> > >  - Add a command-line parameter to allow the user to specify the
> > >    destination mac address for each ethernet port used.
> > >  - While forwarding the packet update source and destination mac
> > >    addresses.
> >
> > This new parameter is optional, but I see no default for l2
> > destination in the patch.
> > How does it work when you won't set this option?
>
> Ah yes, forgot to init l2 dst array with some default values.
> Will fix and re-spin then.

Ok, waiting for it, and squash patch 3 as suggested.
Thanks.
  
Ananyev, Konstantin Oct. 16, 2020, 1:14 p.m. UTC | #4
> On Thu, Oct 15, 2020 at 3:06 PM Ananyev, Konstantin
> <konstantin.ananyev@intel.com> wrote:
> > > On Tue, Oct 6, 2020 at 7:17 PM Konstantin Ananyev
> > > <konstantin.ananyev@intel.com> wrote:
> > > >
> > > > Introduces two changes into l3fwd-acl behaviour to make
> > > > it behave in the same way as l3fwd:
> > > >  - Add a command-line parameter to allow the user to specify the
> > > >    destination mac address for each ethernet port used.
> > > >  - While forwarding the packet update source and destination mac
> > > >    addresses.
> > >
> > > This new parameter is optional, but I see no default for l2
> > > destination in the patch.
> > > How does it work when you won't set this option?
> >
> > Ah yes, forgot to init l2 dst array with some default values.
> > Will fix and re-spin then.
> 
> Ok, waiting for it, and squash patch 3 as suggested.
> Thanks.
> 

Just sent.
Thanks.
  

Patch

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index fc6c5de8d9..53343c780a 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst
@@ -119,6 +119,11 @@  New Features
   * Added new ``RTE_ACL_CLASSIFY_AVX512X32`` vector implementation,
     which can process up to 32 flows in parallel. Requires AVX512 support.
 
+* **Add new command-line parameter for l3wfd-acl sample application.**
+
+  Added new optional parameter ``--eth-dest`` for the ``l3fwd-acl`` to allow
+  the user to specify the destination mac address for each ethernet port used.
+
 
 Removed Items
 -------------
diff --git a/doc/guides/sample_app_ug/l3_forward_access_ctrl.rst b/doc/guides/sample_app_ug/l3_forward_access_ctrl.rst
index a44fbcd52c..91004356ed 100644
--- a/doc/guides/sample_app_ug/l3_forward_access_ctrl.rst
+++ b/doc/guides/sample_app_ug/l3_forward_access_ctrl.rst
@@ -236,7 +236,7 @@  The application has a number of command line options:
 
 ..  code-block:: console
 
-    ./build/l3fwd-acl [EAL options] -- -p PORTMASK [-P] --config(port,queue,lcore)[,(port,queue,lcore)] --rule_ipv4 FILENAME rule_ipv6 FILENAME [--scalar] [--enable-jumbo [--max-pkt-len PKTLEN]] [--no-numa]
+    ./build/l3fwd-acl [EAL options] -- -p PORTMASK [-P] --config(port,queue,lcore)[,(port,queue,lcore)] --rule_ipv4 FILENAME rule_ipv6 FILENAME [--scalar] [--enable-jumbo [--max-pkt-len PKTLEN]] [--no-numa] [--eth-dest=X,MM:MM:MM:MM:MM:MM]
 
 
 where,
@@ -260,6 +260,8 @@  where,
 
 *   --no-numa: optional, disables numa awareness
 
+*   --eth-dest=X,MM:MM:MM:MM:MM:MM: optional, ethernet destination for port X
+
 For example, consider a dual processor socket platform with 8 physical cores, where cores 0-7 and 16-23 appear on socket 0,
 while cores 8-15 and 24-31 appear on socket 1.
 
diff --git a/examples/l3fwd-acl/main.c b/examples/l3fwd-acl/main.c
index 4386584667..a5b8b11ee1 100644
--- a/examples/l3fwd-acl/main.c
+++ b/examples/l3fwd-acl/main.c
@@ -39,6 +39,9 @@ 
 #include <rte_string_fns.h>
 #include <rte_acl.h>
 
+#include <cmdline_parse.h>
+#include <cmdline_parse_etheraddr.h>
+
 #if RTE_LOG_DP_LEVEL >= RTE_LOG_DEBUG
 #define L3FWDACL_DEBUG
 #endif
@@ -81,9 +84,6 @@ 
 static uint16_t nb_rxd = RTE_TEST_RX_DESC_DEFAULT;
 static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
 
-/* ethernet addresses of ports */
-static struct rte_ether_addr ports_eth_addr[RTE_MAX_ETHPORTS];
-
 /* mask of enabled ports */
 static uint32_t enabled_port_mask;
 static int promiscuous_on; /**< Ports set in promiscuous mode off by default. */
@@ -143,6 +143,9 @@  static struct rte_eth_conf port_conf = {
 
 static struct rte_mempool *pktmbuf_pool[NB_SOCKETS];
 
+/* ethernet addresses of ports */
+static struct rte_ether_hdr port_l2hdr[RTE_MAX_ETHPORTS];
+
 /***********************start of ACL part******************************/
 #ifdef DO_RFC_1812_CHECKS
 static inline int
@@ -164,6 +167,7 @@  send_single_packet(struct rte_mbuf *m, uint16_t port);
 #define OPTION_RULE_IPV4	"rule_ipv4"
 #define OPTION_RULE_IPV6	"rule_ipv6"
 #define OPTION_SCALAR		"scalar"
+#define OPTION_ETH_DEST		"eth-dest"
 #define ACL_DENY_SIGNATURE	0xf0000000
 #define RTE_LOGTYPE_L3FWDACL	RTE_LOGTYPE_USER3
 #define acl_log(format, ...)	RTE_LOG(ERR, L3FWDACL, format, ##__VA_ARGS__)
@@ -1275,9 +1279,14 @@  send_single_packet(struct rte_mbuf *m, uint16_t port)
 {
 	uint32_t lcore_id;
 	struct lcore_conf *qconf;
+	struct rte_ether_hdr *eh;
 
 	lcore_id = rte_lcore_id();
 
+	/* update src and dst mac*/
+	eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	memcpy(eh, &port_l2hdr[port], sizeof(eh->d_addr) + sizeof(eh->s_addr));
+
 	qconf = &lcore_conf[lcore_id];
 	rte_eth_tx_buffer(port, qconf->tx_queue_id[port],
 			qconf->tx_buffer[port], m);
@@ -1627,6 +1636,26 @@  parse_config(const char *q_arg)
 	return 0;
 }
 
+static const char *
+parse_eth_dest(const char *optarg)
+{
+	unsigned long portid;
+	char *port_end;
+
+	errno = 0;
+	portid = strtoul(optarg, &port_end, 0);
+	if (errno != 0 || port_end == optarg || *port_end++ != ',')
+		return "Invalid format";
+	else if (portid >= RTE_MAX_ETHPORTS)
+		return "port value exceeds RTE_MAX_ETHPORTS("
+			RTE_STR(RTE_MAX_ETHPORTS) ")";
+
+	if (cmdline_parse_etheraddr(NULL, port_end, &port_l2hdr[portid].d_addr,
+			sizeof(port_l2hdr[portid].d_addr)) < 0)
+		return "Invalid ethernet address";
+	return NULL;
+}
+
 /* Parse the argument given in the command line of the application */
 static int
 parse_args(int argc, char **argv)
@@ -1642,6 +1671,7 @@  parse_args(int argc, char **argv)
 		{OPTION_RULE_IPV4, 1, 0, 0},
 		{OPTION_RULE_IPV6, 1, 0, 0},
 		{OPTION_SCALAR, 0, 0, 0},
+		{OPTION_ETH_DEST, 1, 0, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -1737,6 +1767,16 @@  parse_args(int argc, char **argv)
 					OPTION_SCALAR, sizeof(OPTION_SCALAR)))
 				parm_config.scalar = 1;
 
+			if (!strncmp(lgopts[option_index].name, OPTION_ETH_DEST,
+					sizeof(OPTION_ETH_DEST))) {
+				const char *serr = parse_eth_dest(optarg);
+				if (serr != NULL) {
+					printf("invalid %s value:\"%s\": %s\n",
+						OPTION_ETH_DEST, optarg, serr);
+					print_usage(prgname);
+					return -1;
+				}
+			}
 
 			break;
 
@@ -1962,13 +2002,14 @@  main(int argc, char **argv)
 				"rte_eth_dev_adjust_nb_rx_tx_desc: err=%d, port=%d\n",
 				ret, portid);
 
-		ret = rte_eth_macaddr_get(portid, &ports_eth_addr[portid]);
+		ret = rte_eth_macaddr_get(portid, &port_l2hdr[portid].s_addr);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE,
 				"rte_eth_macaddr_get: err=%d, port=%d\n",
 				ret, portid);
 
-		print_ethaddr(" Address:", &ports_eth_addr[portid]);
+		print_ethaddr("Dst MAC:", &port_l2hdr[portid].d_addr);
+		print_ethaddr(", Src MAC:", &port_l2hdr[portid].s_addr);
 		printf(", ");
 
 		/* init memory */