Looks okay. 

Regards
Sunil Kumar Kori

>-----Original Message-----
>From: pbhagavat...@marvell.com <pbhagavat...@marvell.com>
>Sent: Tuesday, April 28, 2020 12:01 AM
>To: Jerin Jacob Kollanukkaran <jer...@marvell.com>; tho...@monjalon.net;
>John McNamara <john.mcnam...@intel.com>; Marko Kovacevic
><marko.kovace...@intel.com>; Ori Kam <or...@mellanox.com>; Bruce
>Richardson <bruce.richard...@intel.com>; Radu Nicolau
><radu.nico...@intel.com>; Akhil Goyal <akhil.go...@nxp.com>; Tomasz
>Kantecki <tomasz.kante...@intel.com>; Sunil Kumar Kori
><sk...@marvell.com>; Pavan Nikhilesh Bhagavatula
><pbhagavat...@marvell.com>
>Cc: Andrzej Ostruszka [C] <aostrus...@marvell.com>; dev@dpdk.org; Vamsi
>Krishna Attunuru <vattun...@marvell.com>
>Subject: [dpdk-dev] [PATCH v4] examples/l2fwd: add cmdline option for
>forwarding port info
>
>From: Vamsi Attunuru <vattun...@marvell.com>
>
>Current l2fwd application statically configures adjacent ports as destination
>ports for forwarding the traffic.
>
>Add a portmap option to pass the forwarding port pair mapping which allows
>the user to configure forwarding port mapping.
>
>If no portmap argument is specified, destination port map is not changed and
>traffic gets forwarded with existing mapping.
>
>To align port/queue configuration of each lcore with destination port map,
>port/queue configuration of each lcore gets modified when portmap option is
>specified.
>
>Ex: ./l2fwd -c 0xff -- -p 0x3f -q 2 --portmap="(0,3)(1,4)(2,5)"
>
>With above portmap option, traffic received from portid = 0 gets forwarded to
>port = 3 and vice versa, similarly traffic gets forwarded on other port pairs
>(1,4) and (2,5)
>
>Signed-off-by: Vamsi Attunuru <vattun...@marvell.com>
>Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
>Acked-by: Andrzej Ostruszka <aostrus...@marvell.com>
Acked-by: Sunil Kumar Kori <sk...@marvell.com>

>---
> v4 Changes:
> * Documentation changes. (Sunil)
> * reduce port_pair_params_array size. (Sunil)
>
> v3 Changes:
> * s/config/portmap/
> * rebase on master
>
> v2 Changes:
> * Fix command option format in docs.
> * Use memcpy instead of snprintf.
> * Rephrase if-else condition.
>
> doc/guides/rel_notes/release_20_05.rst        |   7 +
> .../sample_app_ug/l2_forward_real_virtual.rst |  17 +-
> examples/l2fwd/main.c                         | 182 +++++++++++++++---
> 3 files changed, 180 insertions(+), 26 deletions(-)
>
>diff --git a/doc/guides/rel_notes/release_20_05.rst
>b/doc/guides/rel_notes/release_20_05.rst
>index b124c3f28..019bec5d7 100644
>--- a/doc/guides/rel_notes/release_20_05.rst
>+++ b/doc/guides/rel_notes/release_20_05.rst
>@@ -212,6 +212,13 @@ New Features
>   * Added IPsec inbound load-distribution support for ipsec-secgw application
>     using NIC load distribution feature(Flow Director).
>
>+* **Added --portmap command line parameter to l2fwd example.**
>+
>+  Added new command line option ``--portmap="(port, port)[,(port,
>+ port)]"`` to  pass forwarding port details.
>+  See the :doc:`doc/guides/sample_app_ug/l2_forward_real_virtual` for
>+ more  details of this parameter usage.
>+
>
> Removed Items
> -------------
>diff --git a/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
>b/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
>index 39d6b0067..90ca609d6 100644
>--- a/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
>+++ b/doc/guides/sample_app_ug/l2_forward_real_virtual.rst
>@@ -91,7 +91,10 @@ The application requires a number of command line
>options:
>
> .. code-block:: console
>
>-    ./build/l2fwd [EAL options] -- -p PORTMASK [-q NQ] --[no-]mac-updating
>+    ./build/l2fwd [EAL options] -- -p PORTMASK
>+                                   [-q NQ]
>+                                   --[no-]mac-updating
>+                                   [--portmap="(port, port)[,(port,
>+ port)]"]
>
> where,
>
>@@ -99,7 +102,9 @@ where,
>
> *   q NQ: A number of queues (=ports) per lcore (default is 1)
>
>-*   --[no-]mac-updating: Enable or disable MAC addresses updating (enabled
>by default).
>+*   --[no-]mac-updating: Enable or disable MAC addresses updating (enabled
>by default)
>+
>+*   --portmap="(port,port)[,(port,port)]": Determines forwarding ports
>mapping.
>
> To run the application in linux environment with 4 lcores, 16 ports and 8 RX
>queues per lcore and MAC address  updating enabled, issue the command:
>@@ -108,6 +113,14 @@ updating enabled, issue the command:
>
>     $ ./build/l2fwd -l 0-3 -n 4 -- -q 8 -p ffff
>
>+To run the application in linux environment with 4 lcores, 4 ports, 8
>+RX queues per lcore, to forward RX traffic of ports 0 & 1 on ports 2 &
>+3 respectively and vice versa, issue the command:
>+
>+.. code-block:: console
>+
>+    $ ./build/l2fwd -l 0-3 -n 4 -- -q 8 -p f --portmap="(0,2)(1,3)"
>+
> Refer to the *DPDK Getting Started Guide* for general information on running
>applications  and the Environment Abstraction Layer (EAL) options.
>
>diff --git a/examples/l2fwd/main.c b/examples/l2fwd/main.c index
>88ddfe589..1d84cd789 100644
>--- a/examples/l2fwd/main.c
>+++ b/examples/l2fwd/main.c
>@@ -38,6 +38,7 @@
> #include <rte_ethdev.h>
> #include <rte_mempool.h>
> #include <rte_mbuf.h>
>+#include <rte_string_fns.h>
>
> static volatile bool force_quit;
>
>@@ -67,6 +68,15 @@ static uint32_t l2fwd_enabled_port_mask = 0;
> /* list of enabled ports */
> static uint32_t l2fwd_dst_ports[RTE_MAX_ETHPORTS];
>
>+struct port_pair_params {
>+#define NUM_PORTS     2
>+      uint16_t port[NUM_PORTS];
>+} __rte_cache_aligned;
>+
>+static struct port_pair_params
>port_pair_params_array[RTE_MAX_ETHPORTS
>+/ 2]; static struct port_pair_params *port_pair_params; static uint16_t
>+nb_port_pair_params;
>+
> static unsigned int l2fwd_rx_queue_per_lcore = 1;
>
> #define MAX_RX_QUEUE_PER_LCORE 16
>@@ -294,11 +304,13 @@ l2fwd_usage(const char *prgname)
>       printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>              "  -p PORTMASK: hexadecimal bitmask of ports to configure\n"
>              "  -q NQ: number of queue (=ports) per lcore (default is 1)\n"
>-                 "  -T PERIOD: statistics will be refreshed each PERIOD
>seconds (0 to disable, 10 default, 86400 maximum)\n"
>-                 "  --[no-]mac-updating: Enable or disable MAC addresses
>updating (enabled by default)\n"
>-                 "      When enabled:\n"
>-                 "       - The source MAC address is replaced by the TX port
>MAC address\n"
>-                 "       - The destination MAC address is replaced by
>02:00:00:00:00:TX_PORT_ID\n",
>+             "  -T PERIOD: statistics will be refreshed each PERIOD seconds 
>(0 to
>disable, 10 default, 86400 maximum)\n"
>+             "  --[no-]mac-updating: Enable or disable MAC addresses updating
>(enabled by default)\n"
>+             "      When enabled:\n"
>+             "       - The source MAC address is replaced by the TX port MAC
>address\n"
>+             "       - The destination MAC address is replaced by
>02:00:00:00:00:TX_PORT_ID\n"
>+             "  --portmap: Configure forwarding port pair mapping\n"
>+             "              Default: alternate port pairs\n\n",
>              prgname);
> }
>
>@@ -319,6 +331,61 @@ l2fwd_parse_portmask(const char *portmask)
>       return pm;
> }
>
>+static int
>+l2fwd_parse_port_pair_config(const char *q_arg) {
>+      enum fieldnames {
>+              FLD_PORT1 = 0,
>+              FLD_PORT2,
>+              _NUM_FLD
>+      };
>+      unsigned long int_fld[_NUM_FLD];
>+      const char *p, *p0 = q_arg;
>+      char *str_fld[_NUM_FLD];
>+      unsigned int size;
>+      char s[256];
>+      char *end;
>+      int i;
>+
>+      nb_port_pair_params = 0;
>+
>+      while ((p = strchr(p0, '(')) != NULL) {
>+              ++p;
>+              p0 = strchr(p, ')');
>+              if (p0 == NULL)
>+                      return -1;
>+
>+              size = p0 - p;
>+              if (size >= sizeof(s))
>+                      return -1;
>+
>+              memcpy(s, p, size);
>+              s[size] = '\0';
>+              if (rte_strsplit(s, sizeof(s), str_fld,
>+                               _NUM_FLD, ',') != _NUM_FLD)
>+                      return -1;
>+              for (i = 0; i < _NUM_FLD; i++) {
>+                      errno = 0;
>+                      int_fld[i] = strtoul(str_fld[i], &end, 0);
>+                      if (errno != 0 || end == str_fld[i] ||
>+                          int_fld[i] >= RTE_MAX_ETHPORTS)
>+                              return -1;
>+              }
>+              if (nb_port_pair_params >= RTE_MAX_ETHPORTS/2) {
>+                      printf("exceeded max number of port pair params:
>%hu\n",
>+                              nb_port_pair_params);
>+                      return -1;
>+              }
>+              port_pair_params_array[nb_port_pair_params].port[0] =
>+                              (uint16_t)int_fld[FLD_PORT1];
>+              port_pair_params_array[nb_port_pair_params].port[1] =
>+                              (uint16_t)int_fld[FLD_PORT2];
>+              ++nb_port_pair_params;
>+      }
>+      port_pair_params = port_pair_params_array;
>+      return 0;
>+}
>+
> static unsigned int
> l2fwd_parse_nqueue(const char *q_arg)
> {
>@@ -361,6 +428,7 @@ static const char short_options[] =
>
> #define CMD_LINE_OPT_MAC_UPDATING "mac-updating"
> #define CMD_LINE_OPT_NO_MAC_UPDATING "no-mac-updating"
>+#define CMD_LINE_OPT_PORTMAP_CONFIG "portmap"
>
> enum {
>       /* long options mapped to a short option */ @@ -368,11 +436,13 @@
>enum {
>       /* first long only option value must be >= 256, so that we won't
>        * conflict with short options */
>       CMD_LINE_OPT_MIN_NUM = 256,
>+      CMD_LINE_OPT_PORTMAP_NUM,
> };
>
> static const struct option lgopts[] = {
>       { CMD_LINE_OPT_MAC_UPDATING, no_argument, &mac_updating,
>1},
>       { CMD_LINE_OPT_NO_MAC_UPDATING, no_argument,
>&mac_updating, 0},
>+      { CMD_LINE_OPT_PORTMAP_CONFIG, 1, 0,
>CMD_LINE_OPT_PORTMAP_NUM},
>       {NULL, 0, 0, 0}
> };
>
>@@ -386,6 +456,7 @@ l2fwd_parse_args(int argc, char **argv)
>       char *prgname = argv[0];
>
>       argvopt = argv;
>+      port_pair_params = NULL;
>
>       while ((opt = getopt_long(argc, argvopt, short_options,
>                                 lgopts, &option_index)) != EOF) { @@ -423,7
>+494,13 @@ l2fwd_parse_args(int argc, char **argv)
>                       break;
>
>               /* long options */
>-              case 0:
>+              case CMD_LINE_OPT_PORTMAP_NUM:
>+                      ret = l2fwd_parse_port_pair_config(optarg);
>+                      if (ret) {
>+                              fprintf(stderr, "Invalid config\n");
>+                              l2fwd_usage(prgname);
>+                              return -1;
>+                      }
>                       break;
>
>               default:
>@@ -440,6 +517,48 @@ l2fwd_parse_args(int argc, char **argv)
>       return ret;
> }
>
>+/*
>+ * Check port pair config with enabled port mask,
>+ * and for valid port pair combinations.
>+ */
>+static int
>+check_port_pair_config(void)
>+{
>+      uint32_t port_pair_config_mask = 0;
>+      uint32_t port_pair_mask = 0;
>+      uint16_t index, i, portid;
>+
>+      for (index = 0; index < nb_port_pair_params; index++) {
>+              port_pair_mask = 0;
>+
>+              for (i = 0; i < NUM_PORTS; i++)  {
>+                      portid = port_pair_params[index].port[i];
>+                      if ((l2fwd_enabled_port_mask & (1 << portid)) == 0) {
>+                              printf("port %u is not enabled in port
>mask\n",
>+                                     portid);
>+                              return -1;
>+                      }
>+                      if (!rte_eth_dev_is_valid_port(portid)) {
>+                              printf("port %u is not present on the
>board\n",
>+                                     portid);
>+                              return -1;
>+                      }
>+
>+                      port_pair_mask |= 1 << portid;
>+              }
>+
>+              if (port_pair_config_mask & port_pair_mask) {
>+                      printf("port %u is used in other port pairs\n", portid);
>+                      return -1;
>+              }
>+              port_pair_config_mask |= port_pair_mask;
>+      }
>+
>+      l2fwd_enabled_port_mask &= port_pair_config_mask;
>+
>+      return 0;
>+}
>+
> /* Check the link status of all ports in up to 9s, and print them finally */  
> static
>void  check_all_ports_link_status(uint32_t port_mask) @@ -555,6 +674,11
>@@ main(int argc, char **argv)
>       if (nb_ports == 0)
>               rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
>
>+      if (port_pair_params != NULL) {
>+              if (check_port_pair_config() < 0)
>+                      rte_exit(EXIT_FAILURE, "Invalid port pair config\n");
>+      }
>+
>       /* check port mask to possible port mask */
>       if (l2fwd_enabled_port_mask & ~((1 << nb_ports) - 1))
>               rte_exit(EXIT_FAILURE, "Invalid portmask; possible (0x%x)\n",
>@@ -565,26 +689,35 @@ main(int argc, char **argv)
>               l2fwd_dst_ports[portid] = 0;
>       last_port = 0;
>
>-      /*
>-       * Each logical core is assigned a dedicated TX queue on each port.
>-       */
>-      RTE_ETH_FOREACH_DEV(portid) {
>-              /* skip ports that are not enabled */
>-              if ((l2fwd_enabled_port_mask & (1 << portid)) == 0)
>-                      continue;
>+      /* populate destination port details */
>+      if (port_pair_params != NULL) {
>+              uint16_t idx, p;
>
>-              if (nb_ports_in_mask % 2) {
>-                      l2fwd_dst_ports[portid] = last_port;
>-                      l2fwd_dst_ports[last_port] = portid;
>+              for (idx = 0; idx < (nb_port_pair_params << 1); idx++) {
>+                      p = idx & 1;
>+                      portid = port_pair_params[idx >> 1].port[p];
>+                      l2fwd_dst_ports[portid] =
>+                              port_pair_params[idx >> 1].port[p ^ 1];
>               }
>-              else
>-                      last_port = portid;
>+      } else {
>+              RTE_ETH_FOREACH_DEV(portid) {
>+                      /* skip ports that are not enabled */
>+                      if ((l2fwd_enabled_port_mask & (1 << portid)) == 0)
>+                              continue;
>
>-              nb_ports_in_mask++;
>-      }
>-      if (nb_ports_in_mask % 2) {
>-              printf("Notice: odd number of ports in portmask.\n");
>-              l2fwd_dst_ports[last_port] = last_port;
>+                      if (nb_ports_in_mask % 2) {
>+                              l2fwd_dst_ports[portid] = last_port;
>+                              l2fwd_dst_ports[last_port] = portid;
>+                      } else {
>+                              last_port = portid;
>+                      }
>+
>+                      nb_ports_in_mask++;
>+              }
>+              if (nb_ports_in_mask % 2) {
>+                      printf("Notice: odd number of ports in portmask.\n");
>+                      l2fwd_dst_ports[last_port] = last_port;
>+              }
>       }
>
>       rx_lcore_id = 0;
>@@ -613,7 +746,8 @@ main(int argc, char **argv)
>
>               qconf->rx_port_list[qconf->n_rx_port] = portid;
>               qconf->n_rx_port++;
>-              printf("Lcore %u: RX port %u\n", rx_lcore_id, portid);
>+              printf("Lcore %u: RX port %u TX port %u\n", rx_lcore_id,
>+                     portid, l2fwd_dst_ports[portid]);
>       }
>
>       nb_mbufs = RTE_MAX(nb_ports * (nb_rxd + nb_txd +
>MAX_PKT_BURST +
>--
>2.17.1

Reply via email to