DPDK patches and discussions
 help / color / mirror / Atom feed
From: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
To: Sunil Kumar Kori <skori@marvell.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"John McNamara" <john.mcnamara@intel.com>,
	Marko Kovacevic <marko.kovacevic@intel.com>,
	Ori Kam <orika@mellanox.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Radu Nicolau <radu.nicolau@intel.com>,
	"Akhil Goyal" <akhil.goyal@nxp.com>,
	Tomasz Kantecki <tomasz.kantecki@intel.com>
Cc: "Andrzej Ostruszka [C]" <aostruszka@marvell.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	Vamsi Krishna Attunuru <vattunuru@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v3] examples/l2fwd: add cmdline option for forwarding port info
Date: Mon, 27 Apr 2020 16:38:40 +0000	[thread overview]
Message-ID: <BYAPR18MB25185ADCB5A4394C79D6CA9FDEAF0@BYAPR18MB2518.namprd18.prod.outlook.com> (raw)
In-Reply-To: <BY5PR18MB31057CA03F4D81D0085C554DB4AF0@BY5PR18MB3105.namprd18.prod.outlook.com>

>>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..b54321b5b 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)]"
>
>Will it be better to represent as [--portmap="(port, port)[,(port, port)]]
>as it is a optional parameter ?
>

Will change in v4.

>>
>> where,
>>
>>@@ -99,7 +102,10 @@ 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 which ports are
>>mapped to
>>+    which ports for packet forwarding.
>
>May be rephrased a bit shorter as "Determines forwarding ports
>mapping".

Will change in v4.

>>
>> 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 +114,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 and MAC address updating enabled, to forward
>RX
>>+traffic of ports 0 & 1 on ports 2 & 3 respectively and vice versa, issue
>the
>>command:
>
>IMO, No need to mentioned information about MAC address update
>enabled because it
>is enabled by default and no param is passed in below command. So
>information is not
>relevant here.
>

Will change in v4.

>>+
>>+.. code-block:: console
>>+
>>+    $ ./build/l2fwd -l 0-3 -n 4 -- -q 8 -p f --portmap="(0,2)(1,3)"
>It looks like comma is missed in between two port map information.
>>+
>> 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..81861a22a 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;
>
>Is there any specific reason to use this syntax to declare two ports
>instead of following
>struct port_pair_params {
>     uint16_t port1;
>     uint16_t port2;
>};
>

It was done to reduce code duplication. Will leave as is.

>>+
>>+static struct port_pair_params
>>+port_pair_params_array[RTE_MAX_ETHPORTS];
>Should not be RTE_MAX_ETHPORTS/2 if only 1:1 mapping is allowed  ?
>As I understood it is used to store CLI port mapping, 

RTE_MAX_ETHPORTS/2 will be sufficient.

>I think it is better to
>Use any other MACRO which define maximum number of port map
>parameter.
>If you consider it, then it can also be updated in documentation.
>
>>+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",
>
>IMO, Indentation changes for other parameters should in separate
>patch.

It's not a functionality change it just minor code formatting change and doesn't 
deserve a separate patch.

>> 	       prgname);
>> }
>>
>>@@ -319,6 +331,61 @@ l2fwd_parse_portmask(const char
>*portmask)
>> 	return pm;
>> }
>>

  parent reply	other threads:[~2020-04-27 16:38 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-23 11:56 [dpdk-dev] [PATCH v1 1/1] bus/pci: probe PCI devices in whitelisted order vattunuru
2019-09-25  6:41 ` Slava Ovsiienko
2019-09-25  9:07   ` Gaëtan Rivet
2019-09-26  4:15     ` Vamsi Krishna Attunuru
2019-09-26  8:04       ` Gaëtan Rivet
2019-09-26  9:39         ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru
2019-09-30 12:51           ` [dpdk-dev] [PATCH v1] eal: add manual probing option Gaetan Rivet
2019-09-30 17:51             ` Aaron Conole
2019-10-01  7:28               ` Gaëtan Rivet
2019-10-01 12:57                 ` Aaron Conole
2019-09-30 18:53             ` Stephen Hemminger
2019-10-01  9:10               ` Gaëtan Rivet
2019-10-01  9:49                 ` Jerin Jacob
2019-10-01 14:09                   ` Gaëtan Rivet
2019-10-01 14:26                     ` Jerin Jacob
2019-10-03  7:58             ` [dpdk-dev] [PATCH v2] " Gaetan Rivet
2019-10-04 12:55               ` [dpdk-dev] [PATCH v3] " Gaetan Rivet
2019-10-07  1:27                 ` Vamsi Krishna Attunuru
2019-10-23  8:44                   ` Gaëtan Rivet
2019-10-25 11:59                 ` Jerin Jacob
2019-10-25 12:50                   ` Gaëtan Rivet
2019-10-25 13:24                     ` Jerin Jacob
2019-10-25 14:41                 ` [dpdk-dev] [PATCH v4] " Gaetan Rivet
2019-10-25 15:01                   ` Jerin Jacob
2019-10-25 15:46                   ` [dpdk-dev] [PATCH v5] " Gaetan Rivet
2019-10-25 15:51                     ` Jerin Jacob
2020-01-22 16:51                     ` Pavan Nikhilesh Bhagavatula
2020-01-23  9:20                       ` Gaetan Rivet
2020-01-23  9:58                     ` [dpdk-dev] [PATCH v7] " Gaetan Rivet
2020-02-03  5:16                       ` Pavan Nikhilesh Bhagavatula
2020-02-03 22:21                         ` Thomas Monjalon
2020-02-04 10:03                           ` Gaetan Rivet
2020-02-04 11:07                             ` Thomas Monjalon
2020-02-04 12:43                               ` Gaetan Rivet
2020-02-04 15:06                                 ` Thomas Monjalon
2020-02-04 16:02                                   ` Gaetan Rivet
2020-02-10 14:51                                     ` Jerin Jacob
2020-02-10 15:27                                       ` Thomas Monjalon
2020-02-10 16:33                                         ` Jerin Jacob
2020-04-03  3:30                                           ` [dpdk-dev] [PATCH] [v1 1/1] examples/l2fwd: add cmdline option for forwarding port info vattunuru
2020-04-03 12:51                                             ` Andrzej Ostruszka [C]
2020-04-05  3:49                                               ` Vamsi Krishna Attunuru
2020-04-05  3:52                                             ` [dpdk-dev] [PATCH] [v2 " vattunuru
2020-04-06  9:32                                               ` Andrzej Ostruszka [C]
2020-04-26 21:19                                               ` Thomas Monjalon
2020-04-27  7:59                                               ` [dpdk-dev] [PATCH v3] " pbhagavatula
2020-04-27  9:19                                                 ` Sunil Kumar Kori
2020-04-27  9:36                                                   ` Andrzej Ostruszka [C]
2020-04-27 10:14                                                     ` Sunil Kumar Kori
2020-04-27 16:38                                                   ` Pavan Nikhilesh Bhagavatula [this message]
2020-04-27 16:49                                                     ` Sunil Kumar Kori
2020-04-27 18:31                                                 ` [dpdk-dev] [PATCH v4] " pbhagavatula
2020-04-28  5:54                                                   ` Sunil Kumar Kori
2020-05-01 14:00                                                   ` Varghese, Vipin
2020-05-01 15:14                                                     ` Pavan Nikhilesh Bhagavatula
2020-05-02  4:34                                                       ` Varghese, Vipin
2020-05-11  0:23                                                         ` Pavan Nikhilesh Bhagavatula
2020-05-24 16:13                                                           ` Thomas Monjalon
2020-05-25  9:29                                                             ` Bruce Richardson
2020-07-04 13:36                                                               ` Jerin Jacob
2020-07-05 12:23                                                                 ` Thomas Monjalon
2020-04-04 16:34                                           ` [dpdk-dev] [EXT] Re: [PATCH v7] eal: add manual probing option Jerin Jacob Kollanukkaran
2023-06-14 19:33                       ` [dpdk-dev] " Stephen Hemminger
2023-06-26 16:12                         ` Gaëtan Rivet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR18MB25185ADCB5A4394C79D6CA9FDEAF0@BYAPR18MB2518.namprd18.prod.outlook.com \
    --to=pbhagavatula@marvell.com \
    --cc=akhil.goyal@nxp.com \
    --cc=aostruszka@marvell.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=john.mcnamara@intel.com \
    --cc=marko.kovacevic@intel.com \
    --cc=orika@mellanox.com \
    --cc=radu.nicolau@intel.com \
    --cc=skori@marvell.com \
    --cc=thomas@monjalon.net \
    --cc=tomasz.kantecki@intel.com \
    --cc=vattunuru@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).