All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] app/testpmd: add parameter to start forwarding sending
@ 2017-05-28 21:36 Pablo de Lara
  2017-06-07  9:17 ` Wu, Jingjing
  2017-06-09  2:30 ` [PATCH v2] " Pablo de Lara
  0 siblings, 2 replies; 18+ messages in thread
From: Pablo de Lara @ 2017-05-28 21:36 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Pablo de Lara

Add parameter to start forwarding sending first
a burst of packets, which is useful when testing
a loopback connection.

This was already implemented as an internal command,
but adding it as a parameter is interesting, as it
allows the user to test a loopback connection without
entering in the internal command line.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 app/test-pmd/parameters.c             | 5 +++++
 app/test-pmd/testpmd.c                | 5 +++--
 app/test-pmd/testpmd.h                | 1 +
 doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index fbe6284..0da4172 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -89,6 +89,7 @@ usage(char* progname)
 	       "[--cmdline-file=FILENAME] "
 #endif
 	       "[--help|-h] | [--auto-start|-a] | ["
+	       "--tx-first"
 	       "--coremask=COREMASK --portmask=PORTMASK --numa "
 	       "--mbuf-size= | --total-num-mbufs= | "
 	       "--nb-cores= | --nb-ports= | "
@@ -674,6 +675,10 @@ launch_args_parse(int argc, char** argv)
 				printf("Auto-start selected\n");
 				auto_start = 1;
 			}
+			if (!strcmp(lgopts[opt_idx].name, "tx-first")) {
+				printf("Start TX first\n");
+				tx_first = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name,
 				    "eth-peers-configfile")) {
 				if (init_peer_eth_addrs(optarg) != 0)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d1041af..39a1225 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -95,6 +95,7 @@ uint16_t verbose_level = 0; /**< Silent by default. */
 /* use master core for command line ? */
 uint8_t interactive = 0;
 uint8_t auto_start = 0;
+uint8_t tx_first;
 char cmdline_filename[PATH_MAX] = {0};
 
 /*
@@ -2339,7 +2340,7 @@ main(int argc, char** argv)
 	if (interactive == 1) {
 		if (auto_start) {
 			printf("Start automatic packet forwarding\n");
-			start_packet_forwarding(0);
+			start_packet_forwarding(tx_first);
 		}
 		prompt();
 		pmd_test_exit();
@@ -2350,7 +2351,7 @@ main(int argc, char** argv)
 		int rc;
 
 		printf("No commandline core given, start packet forwarding\n");
-		start_packet_forwarding(0);
+		start_packet_forwarding(tx_first);
 		printf("Press enter to exit\n");
 		rc = read(0, &c, 1);
 		pmd_test_exit();
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e6c43ba..348aa66 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -299,6 +299,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern uint8_t  interactive;
 extern uint8_t  auto_start;
+extern uint8_t  tx_first;
 extern char cmdline_filename[PATH_MAX]; /**< offline commands file */
 extern uint8_t  numa_support; /**< set by "--numa" parameter */
 extern uint16_t port_topology; /**< set by "--port-topology" parameter */
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 2a43214..4171fcb 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -188,6 +188,10 @@ The commandline options are:
 
     Start forwarding on initialization.
 
+*   ``--tx-first``
+
+    Start forwarding, after sending a burst of packets first.
+
 *   ``--nb-cores=N``
 
     Set the number of forwarding cores,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] app/testpmd: add parameter to start forwarding sending
  2017-05-28 21:36 [PATCH] app/testpmd: add parameter to start forwarding sending Pablo de Lara
@ 2017-06-07  9:17 ` Wu, Jingjing
  2017-06-07 13:20   ` De Lara Guarch, Pablo
  2017-06-09  2:30 ` [PATCH v2] " Pablo de Lara
  1 sibling, 1 reply; 18+ messages in thread
From: Wu, Jingjing @ 2017-06-07  9:17 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev



> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Monday, May 29, 2017 5:37 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH] app/testpmd: add parameter to start forwarding sending
> 
> Add parameter to start forwarding sending first
> a burst of packets, which is useful when testing
> a loopback connection.
> 
> This was already implemented as an internal command,
> but adding it as a parameter is interesting, as it
> allows the user to test a loopback connection without
> entering in the internal command line.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  app/test-pmd/parameters.c             | 5 +++++
>  app/test-pmd/testpmd.c                | 5 +++--
>  app/test-pmd/testpmd.h                | 1 +
>  doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
>  4 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index fbe6284..0da4172 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -89,6 +89,7 @@ usage(char* progname)
>  	       "[--cmdline-file=FILENAME] "
>  #endif
>  	       "[--help|-h] | [--auto-start|-a] | ["
> +	       "--tx-first"

Just consider about the interactive mode.
If using command start, will it still call start_packet_forwarding(0)?
And if start tx_first, still call start_packet_forwarding(1)?

It may cause confused whether this argu "--tx-first" works.
If it only works for non-interactive, you'd better to comment it.


Thanks
Jingjing

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] app/testpmd: add parameter to start forwarding sending
  2017-06-07  9:17 ` Wu, Jingjing
@ 2017-06-07 13:20   ` De Lara Guarch, Pablo
  2017-06-07 13:23     ` Bruce Richardson
  0 siblings, 1 reply; 18+ messages in thread
From: De Lara Guarch, Pablo @ 2017-06-07 13:20 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev

Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, June 07, 2017 10:18 AM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] app/testpmd: add parameter to start forwarding
> sending
> 
> 
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Monday, May 29, 2017 5:37 AM
> > To: Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> > Subject: [PATCH] app/testpmd: add parameter to start forwarding
> sending
> >
> > Add parameter to start forwarding sending first
> > a burst of packets, which is useful when testing
> > a loopback connection.
> >
> > This was already implemented as an internal command,
> > but adding it as a parameter is interesting, as it
> > allows the user to test a loopback connection without
> > entering in the internal command line.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > ---
> >  app/test-pmd/parameters.c             | 5 +++++
> >  app/test-pmd/testpmd.c                | 5 +++--
> >  app/test-pmd/testpmd.h                | 1 +
> >  doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
> >  4 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index fbe6284..0da4172 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -89,6 +89,7 @@ usage(char* progname)
> >  	       "[--cmdline-file=FILENAME] "
> >  #endif
> >  	       "[--help|-h] | [--auto-start|-a] | ["
> > +	       "--tx-first"
> 
> Just consider about the interactive mode.
> If using command start, will it still call start_packet_forwarding(0)?
> And if start tx_first, still call start_packet_forwarding(1)?
> 

If using the commands "start" and "start tx_first", the behaviour will remain the same.
The only behaviour that will change is the non-interactive forwarding mode,
and when using auto-start in interactive mode.

Do you think it should modify the behaviour of "start", in the command line,
so if "--tx-first" is passed, then it sends a burst regardless using "start" or "start tx_first"?

Thanks for the review!
Pablo

> It may cause confused whether this argu "--tx-first" works.
> If it only works for non-interactive, you'd better to comment it.
> 
> 
> Thanks
> Jingjing

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] app/testpmd: add parameter to start forwarding sending
  2017-06-07 13:20   ` De Lara Guarch, Pablo
@ 2017-06-07 13:23     ` Bruce Richardson
  0 siblings, 0 replies; 18+ messages in thread
From: Bruce Richardson @ 2017-06-07 13:23 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: Wu, Jingjing, dev

On Wed, Jun 07, 2017 at 01:20:45PM +0000, De Lara Guarch, Pablo wrote:
> Hi Jingjing,
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Wednesday, June 07, 2017 10:18 AM
> > To: De Lara Guarch, Pablo
> > Cc: dev@dpdk.org
> > Subject: RE: [PATCH] app/testpmd: add parameter to start forwarding
> > sending
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: De Lara Guarch, Pablo
> > > Sent: Monday, May 29, 2017 5:37 AM
> > > To: Wu, Jingjing <jingjing.wu@intel.com>
> > > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > > Subject: [PATCH] app/testpmd: add parameter to start forwarding
> > sending
> > >
> > > Add parameter to start forwarding sending first
> > > a burst of packets, which is useful when testing
> > > a loopback connection.
> > >
> > > This was already implemented as an internal command,
> > > but adding it as a parameter is interesting, as it
> > > allows the user to test a loopback connection without
> > > entering in the internal command line.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > ---
> > >  app/test-pmd/parameters.c             | 5 +++++
> > >  app/test-pmd/testpmd.c                | 5 +++--
> > >  app/test-pmd/testpmd.h                | 1 +
> > >  doc/guides/testpmd_app_ug/run_app.rst | 4 ++++
> > >  4 files changed, 13 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > > index fbe6284..0da4172 100644
> > > --- a/app/test-pmd/parameters.c
> > > +++ b/app/test-pmd/parameters.c
> > > @@ -89,6 +89,7 @@ usage(char* progname)
> > >  	       "[--cmdline-file=FILENAME] "
> > >  #endif
> > >  	       "[--help|-h] | [--auto-start|-a] | ["
> > > +	       "--tx-first"
> > 
> > Just consider about the interactive mode.
> > If using command start, will it still call start_packet_forwarding(0)?
> > And if start tx_first, still call start_packet_forwarding(1)?
> > 
> 
> If using the commands "start" and "start tx_first", the behaviour will remain the same.
> The only behaviour that will change is the non-interactive forwarding mode,
> and when using auto-start in interactive mode.
> 
> Do you think it should modify the behaviour of "start", in the command line,
> so if "--tx-first" is passed, then it sends a burst regardless using "start" or "start tx_first"?
> 
> Thanks for the review!
> Pablo
> 
> > It may cause confused whether this argu "--tx-first" works.
> > If it only works for non-interactive, you'd better to comment it.
> > 
> > 
My 2c is that this flag should be only used for non-interactive mode,
and you get an error if it is used with -i flag. That should minimize any
confusion.

/Bruce

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] app/testpmd: add parameter to start forwarding sending
  2017-05-28 21:36 [PATCH] app/testpmd: add parameter to start forwarding sending Pablo de Lara
  2017-06-07  9:17 ` Wu, Jingjing
@ 2017-06-09  2:30 ` Pablo de Lara
  2017-06-15  4:04   ` [PATCH v3] app/testpmd: add parameter to start forwarding TX first Pablo de Lara
  2017-06-19  0:53   ` [PATCH v2] app/testpmd: add parameter to start forwarding sending Wu, Jingjing
  1 sibling, 2 replies; 18+ messages in thread
From: Pablo de Lara @ 2017-06-09  2:30 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Pablo de Lara

Add parameter to start forwarding sending first
a burst of packets, which is useful when testing
a loopback connection.

This was already implemented as an internal command,
but adding it as a parameter is interesting, as it
allows the user to test a loopback connection without
entering in the internal command line.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---

Changes in v2:

- Added check to prevent user from using --tx-first in interactive mode,
  to avoid confusion
- Added extra information in testpmd help about the new parameter


 app/test-pmd/parameters.c             | 7 +++++++
 app/test-pmd/testpmd.c                | 6 +++++-
 app/test-pmd/testpmd.h                | 1 +
 doc/guides/testpmd_app_ug/run_app.rst | 8 ++++++++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index fbe6284..163fd01 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -89,6 +89,7 @@ usage(char* progname)
 	       "[--cmdline-file=FILENAME] "
 #endif
 	       "[--help|-h] | [--auto-start|-a] | ["
+	       "--tx-first"
 	       "--coremask=COREMASK --portmask=PORTMASK --numa "
 	       "--mbuf-size= | --total-num-mbufs= | "
 	       "--nb-cores= | --nb-ports= | "
@@ -109,6 +110,8 @@ usage(char* progname)
 	printf("  --auto-start: start forwarding on init "
 	       "[always when non-interactive].\n");
 	printf("  --help: display this message and quit.\n");
+	printf("  --tx-first: start forwarding sending a burst first "
+	       "(only if interactive is disabled).\n");
 	printf("  --nb-cores=N: set the number of forwarding cores "
 	       "(1 <= N <= %d).\n", nb_lcores);
 	printf("  --nb-ports=N: set the number of forwarding ports "
@@ -674,6 +677,10 @@ launch_args_parse(int argc, char** argv)
 				printf("Auto-start selected\n");
 				auto_start = 1;
 			}
+			if (!strcmp(lgopts[opt_idx].name, "tx-first")) {
+				printf("Start TX first\n");
+				tx_first = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name,
 				    "eth-peers-configfile")) {
 				if (init_peer_eth_addrs(optarg) != 0)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d1041af..af32397 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -95,6 +95,7 @@ uint16_t verbose_level = 0; /**< Silent by default. */
 /* use master core for command line ? */
 uint8_t interactive = 0;
 uint8_t auto_start = 0;
+uint8_t tx_first;
 char cmdline_filename[PATH_MAX] = {0};
 
 /*
@@ -2291,6 +2292,9 @@ main(int argc, char** argv)
 	if (argc > 1)
 		launch_args_parse(argc, argv);
 
+	if (tx_first && interactive)
+		rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
+				"interactive mode");
 	if (!nb_rxq && !nb_txq)
 		printf("Warning: Either rx or tx queues should be non-zero\n");
 
@@ -2350,7 +2354,7 @@ main(int argc, char** argv)
 		int rc;
 
 		printf("No commandline core given, start packet forwarding\n");
-		start_packet_forwarding(0);
+		start_packet_forwarding(tx_first);
 		printf("Press enter to exit\n");
 		rc = read(0, &c, 1);
 		pmd_test_exit();
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e6c43ba..348aa66 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -299,6 +299,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern uint8_t  interactive;
 extern uint8_t  auto_start;
+extern uint8_t  tx_first;
 extern char cmdline_filename[PATH_MAX]; /**< offline commands file */
 extern uint8_t  numa_support; /**< set by "--numa" parameter */
 extern uint16_t port_topology; /**< set by "--port-topology" parameter */
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 2a43214..3159398 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -188,6 +188,14 @@ The commandline options are:
 
     Start forwarding on initialization.
 
+*   ``--tx-first``
+
+    Start forwarding, after sending a burst of packets first.
+
+.. Note::
+
+   This flag should be only used in non-interactive mode.
+
 *   ``--nb-cores=N``
 
     Set the number of forwarding cores,
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-09  2:30 ` [PATCH v2] " Pablo de Lara
@ 2017-06-15  4:04   ` Pablo de Lara
  2017-06-15 12:05     ` De Lara Guarch, Pablo
  2017-06-19  0:53   ` [PATCH v2] app/testpmd: add parameter to start forwarding sending Wu, Jingjing
  1 sibling, 1 reply; 18+ messages in thread
From: Pablo de Lara @ 2017-06-15  4:04 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Pablo de Lara

Add parameter to start forwarding sending first
a burst of packets, which is useful when testing
a loopback connection.

This was already implemented as an internal command,
but adding it as a parameter is interesting, as it
allows the user to test a loopback connection without
entering in the internal command line.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---

Changes in v3:

-Added tx-first in long parameter list
-Reworded informational message when tx-first is enabled

Changes in v2:

- Added check to prevent user from using --tx-first in interactive mode,
  to avoid confusion
- Added extra information in testpmd help about the new parameter

 app/test-pmd/parameters.c             | 9 +++++++++
 app/test-pmd/testpmd.c                | 6 +++++-
 app/test-pmd/testpmd.h                | 1 +
 doc/guides/testpmd_app_ug/run_app.rst | 8 ++++++++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index fbe6284..0a88844 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -89,6 +89,7 @@ usage(char* progname)
 	       "[--cmdline-file=FILENAME] "
 #endif
 	       "[--help|-h] | [--auto-start|-a] | ["
+	       "--tx-first | "
 	       "--coremask=COREMASK --portmask=PORTMASK --numa "
 	       "--mbuf-size= | --total-num-mbufs= | "
 	       "--nb-cores= | --nb-ports= | "
@@ -109,6 +110,8 @@ usage(char* progname)
 	printf("  --auto-start: start forwarding on init "
 	       "[always when non-interactive].\n");
 	printf("  --help: display this message and quit.\n");
+	printf("  --tx-first: start forwarding sending a burst first "
+	       "(only if interactive is disabled).\n");
 	printf("  --nb-cores=N: set the number of forwarding cores "
 	       "(1 <= N <= %d).\n", nb_lcores);
 	printf("  --nb-ports=N: set the number of forwarding ports "
@@ -566,6 +569,7 @@ launch_args_parse(int argc, char** argv)
 		{ "eth-peers-configfile",	1, 0, 0 },
 		{ "eth-peer",			1, 0, 0 },
 #endif
+		{ "tx-first",			0, 0, 0 },
 		{ "ports",			1, 0, 0 },
 		{ "nb-cores",			1, 0, 0 },
 		{ "nb-ports",			1, 0, 0 },
@@ -674,6 +678,11 @@ launch_args_parse(int argc, char** argv)
 				printf("Auto-start selected\n");
 				auto_start = 1;
 			}
+			if (!strcmp(lgopts[opt_idx].name, "tx-first")) {
+				printf("Ports to start sending a burst of "
+						"packets first\n");
+				tx_first = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name,
 				    "eth-peers-configfile")) {
 				if (init_peer_eth_addrs(optarg) != 0)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d32cbb9..6001283 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -98,6 +98,7 @@ uint16_t verbose_level = 0; /**< Silent by default. */
 /* use master core for command line ? */
 uint8_t interactive = 0;
 uint8_t auto_start = 0;
+uint8_t tx_first;
 char cmdline_filename[PATH_MAX] = {0};
 
 /*
@@ -2294,6 +2295,9 @@ main(int argc, char** argv)
 	if (argc > 1)
 		launch_args_parse(argc, argv);
 
+	if (tx_first && interactive)
+		rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
+				"interactive mode.\n");
 	if (!nb_rxq && !nb_txq)
 		printf("Warning: Either rx or tx queues should be non-zero\n");
 
@@ -2353,7 +2357,7 @@ main(int argc, char** argv)
 		int rc;
 
 		printf("No commandline core given, start packet forwarding\n");
-		start_packet_forwarding(0);
+		start_packet_forwarding(tx_first);
 		printf("Press enter to exit\n");
 		rc = read(0, &c, 1);
 		pmd_test_exit();
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 364502d..5cabeef 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -299,6 +299,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern uint8_t  interactive;
 extern uint8_t  auto_start;
+extern uint8_t  tx_first;
 extern char cmdline_filename[PATH_MAX]; /**< offline commands file */
 extern uint8_t  numa_support; /**< set by "--numa" parameter */
 extern uint16_t port_topology; /**< set by "--port-topology" parameter */
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 2a43214..3159398 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -188,6 +188,14 @@ The commandline options are:
 
     Start forwarding on initialization.
 
+*   ``--tx-first``
+
+    Start forwarding, after sending a burst of packets first.
+
+.. Note::
+
+   This flag should be only used in non-interactive mode.
+
 *   ``--nb-cores=N``
 
     Set the number of forwarding cores,
-- 
2.9.4

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-15  4:04   ` [PATCH v3] app/testpmd: add parameter to start forwarding TX first Pablo de Lara
@ 2017-06-15 12:05     ` De Lara Guarch, Pablo
  2017-06-19  1:05       ` Wu, Jingjing
  2017-06-19 21:12       ` Thomas Monjalon
  0 siblings, 2 replies; 18+ messages in thread
From: De Lara Guarch, Pablo @ 2017-06-15 12:05 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev

Sending to right Jingjing mail address.

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Thursday, June 15, 2017 5:04 AM
> To: jingjing.wu@dpdk.org
> Cc: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: [PATCH v3] app/testpmd: add parameter to start forwarding TX
> first
> 
> Add parameter to start forwarding sending first
> a burst of packets, which is useful when testing
> a loopback connection.
> 
> This was already implemented as an internal command,
> but adding it as a parameter is interesting, as it
> allows the user to test a loopback connection without
> entering in the internal command line.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
> 
> Changes in v3:
> 
> -Added tx-first in long parameter list
> -Reworded informational message when tx-first is enabled
> 
> Changes in v2:
> 
> - Added check to prevent user from using --tx-first in interactive mode,
>   to avoid confusion
> - Added extra information in testpmd help about the new parameter
> 
>  app/test-pmd/parameters.c             | 9 +++++++++
>  app/test-pmd/testpmd.c                | 6 +++++-
>  app/test-pmd/testpmd.h                | 1 +
>  doc/guides/testpmd_app_ug/run_app.rst | 8 ++++++++
>  4 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index fbe6284..0a88844 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -89,6 +89,7 @@ usage(char* progname)
>  	       "[--cmdline-file=FILENAME] "
>  #endif
>  	       "[--help|-h] | [--auto-start|-a] | ["
> +	       "--tx-first | "
>  	       "--coremask=COREMASK --portmask=PORTMASK --numa "
>  	       "--mbuf-size= | --total-num-mbufs= | "
>  	       "--nb-cores= | --nb-ports= | "
> @@ -109,6 +110,8 @@ usage(char* progname)
>  	printf("  --auto-start: start forwarding on init "
>  	       "[always when non-interactive].\n");
>  	printf("  --help: display this message and quit.\n");
> +	printf("  --tx-first: start forwarding sending a burst first "
> +	       "(only if interactive is disabled).\n");
>  	printf("  --nb-cores=N: set the number of forwarding cores "
>  	       "(1 <= N <= %d).\n", nb_lcores);
>  	printf("  --nb-ports=N: set the number of forwarding ports "
> @@ -566,6 +569,7 @@ launch_args_parse(int argc, char** argv)
>  		{ "eth-peers-configfile",	1, 0, 0 },
>  		{ "eth-peer",			1, 0, 0 },
>  #endif
> +		{ "tx-first",			0, 0, 0 },
>  		{ "ports",			1, 0, 0 },
>  		{ "nb-cores",			1, 0, 0 },
>  		{ "nb-ports",			1, 0, 0 },
> @@ -674,6 +678,11 @@ launch_args_parse(int argc, char** argv)
>  				printf("Auto-start selected\n");
>  				auto_start = 1;
>  			}
> +			if (!strcmp(lgopts[opt_idx].name, "tx-first")) {
> +				printf("Ports to start sending a burst of "
> +						"packets first\n");
> +				tx_first = 1;
> +			}
>  			if (!strcmp(lgopts[opt_idx].name,
>  				    "eth-peers-configfile")) {
>  				if (init_peer_eth_addrs(optarg) != 0)
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index d32cbb9..6001283 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -98,6 +98,7 @@ uint16_t verbose_level = 0; /**< Silent by default. */
>  /* use master core for command line ? */
>  uint8_t interactive = 0;
>  uint8_t auto_start = 0;
> +uint8_t tx_first;
>  char cmdline_filename[PATH_MAX] = {0};
> 
>  /*
> @@ -2294,6 +2295,9 @@ main(int argc, char** argv)
>  	if (argc > 1)
>  		launch_args_parse(argc, argv);
> 
> +	if (tx_first && interactive)
> +		rte_exit(EXIT_FAILURE, "--tx-first cannot be used on "
> +				"interactive mode.\n");
>  	if (!nb_rxq && !nb_txq)
>  		printf("Warning: Either rx or tx queues should be non-
> zero\n");
> 
> @@ -2353,7 +2357,7 @@ main(int argc, char** argv)
>  		int rc;
> 
>  		printf("No commandline core given, start packet
> forwarding\n");
> -		start_packet_forwarding(0);
> +		start_packet_forwarding(tx_first);
>  		printf("Press enter to exit\n");
>  		rc = read(0, &c, 1);
>  		pmd_test_exit();
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 364502d..5cabeef 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -299,6 +299,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
>  extern uint16_t verbose_level; /**< Drives messages being displayed, if
> any. */
>  extern uint8_t  interactive;
>  extern uint8_t  auto_start;
> +extern uint8_t  tx_first;
>  extern char cmdline_filename[PATH_MAX]; /**< offline commands file */
>  extern uint8_t  numa_support; /**< set by "--numa" parameter */
>  extern uint16_t port_topology; /**< set by "--port-topology" parameter */
> diff --git a/doc/guides/testpmd_app_ug/run_app.rst
> b/doc/guides/testpmd_app_ug/run_app.rst
> index 2a43214..3159398 100644
> --- a/doc/guides/testpmd_app_ug/run_app.rst
> +++ b/doc/guides/testpmd_app_ug/run_app.rst
> @@ -188,6 +188,14 @@ The commandline options are:
> 
>      Start forwarding on initialization.
> 
> +*   ``--tx-first``
> +
> +    Start forwarding, after sending a burst of packets first.
> +
> +.. Note::
> +
> +   This flag should be only used in non-interactive mode.
> +
>  *   ``--nb-cores=N``
> 
>      Set the number of forwarding cores,
> --
> 2.9.4

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] app/testpmd: add parameter to start forwarding sending
  2017-06-09  2:30 ` [PATCH v2] " Pablo de Lara
  2017-06-15  4:04   ` [PATCH v3] app/testpmd: add parameter to start forwarding TX first Pablo de Lara
@ 2017-06-19  0:53   ` Wu, Jingjing
  1 sibling, 0 replies; 18+ messages in thread
From: Wu, Jingjing @ 2017-06-19  0:53 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev



> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Friday, June 9, 2017 10:31 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH v2] app/testpmd: add parameter to start forwarding sending
> 
> Add parameter to start forwarding sending first a burst of packets, which is
> useful when testing a loopback connection.
> 
> This was already implemented as an internal command, but adding it as a
> parameter is interesting, as it allows the user to test a loopback connection
> without entering in the internal command line.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-15 12:05     ` De Lara Guarch, Pablo
@ 2017-06-19  1:05       ` Wu, Jingjing
  2017-07-05 23:48         ` Thomas Monjalon
  2017-06-19 21:12       ` Thomas Monjalon
  1 sibling, 1 reply; 18+ messages in thread
From: Wu, Jingjing @ 2017-06-19  1:05 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev



> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Thursday, June 15, 2017 8:05 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3] app/testpmd: add parameter to start forwarding TX
> first
> 
> Sending to right Jingjing mail address.
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Thursday, June 15, 2017 5:04 AM
> > To: jingjing.wu@dpdk.org
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> > Subject: [PATCH v3] app/testpmd: add parameter to start forwarding TX
> > first
> >
> > Add parameter to start forwarding sending first a burst of packets,
> > which is useful when testing a loopback connection.
> >
> > This was already implemented as an internal command, but adding it as
> > a parameter is interesting, as it allows the user to test a loopback
> > connection without entering in the internal command line.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > ---
> >
> > Changes in v3:
> >
> > -Added tx-first in long parameter list -Reworded informational message
> > when tx-first is enabled

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-15 12:05     ` De Lara Guarch, Pablo
  2017-06-19  1:05       ` Wu, Jingjing
@ 2017-06-19 21:12       ` Thomas Monjalon
  2017-06-20  8:03         ` De Lara Guarch, Pablo
  2017-06-20  9:22         ` Bruce Richardson
  1 sibling, 2 replies; 18+ messages in thread
From: Thomas Monjalon @ 2017-06-19 21:12 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev, Wu, Jingjing

15/06/2017 14:05, De Lara Guarch, Pablo:
> > Add parameter to start forwarding sending first
> > a burst of packets, which is useful when testing
> > a loopback connection.
> > 
> > This was already implemented as an internal command,
> > but adding it as a parameter is interesting, as it
> > allows the user to test a loopback connection without
> > entering in the internal command line.
> > 
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > ---
> > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > @@ -188,6 +188,14 @@ The commandline options are:
> > 
> >      Start forwarding on initialization.
> > 
> > +*   ``--tx-first``
> > +
> > +    Start forwarding, after sending a burst of packets first.
> > +
> > +.. Note::
> > +
> > +   This flag should be only used in non-interactive mode.

I don't really understand the benefit of this option.
Why is it better than
	echo start tx_first | testpmd -i
?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-19 21:12       ` Thomas Monjalon
@ 2017-06-20  8:03         ` De Lara Guarch, Pablo
  2017-06-20  9:22         ` Bruce Richardson
  1 sibling, 0 replies; 18+ messages in thread
From: De Lara Guarch, Pablo @ 2017-06-20  8:03 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Wu, Jingjing



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, June 19, 2017 10:13 PM
> To: De Lara Guarch, Pablo
> Cc: dev@dpdk.org; Wu, Jingjing
> Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: add parameter to start
> forwarding TX first
> 
> 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > Add parameter to start forwarding sending first
> > > a burst of packets, which is useful when testing
> > > a loopback connection.
> > >
> > > This was already implemented as an internal command,
> > > but adding it as a parameter is interesting, as it
> > > allows the user to test a loopback connection without
> > > entering in the internal command line.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > ---
> > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > @@ -188,6 +188,14 @@ The commandline options are:
> > >
> > >      Start forwarding on initialization.
> > >
> > > +*   ``--tx-first``
> > > +
> > > +    Start forwarding, after sending a burst of packets first.
> > > +
> > > +.. Note::
> > > +
> > > +   This flag should be only used in non-interactive mode.
> 
> I don't really understand the benefit of this option.
> Why is it better than
> 	echo start tx_first | testpmd -i
> ?

This is a good way to test a loopback connection without having to get into interactive mode.
With the other patch that I sent, to show port statistics periodically
(http://dpdk.org/dev/patchwork/patch/25344/), the app can show how traffic is forwarded
having a simple loopback connection, without anything else required by the user.

Thanks,
Pablo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-19 21:12       ` Thomas Monjalon
  2017-06-20  8:03         ` De Lara Guarch, Pablo
@ 2017-06-20  9:22         ` Bruce Richardson
  2017-06-20  9:58           ` Thomas Monjalon
  1 sibling, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2017-06-20  9:22 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: De Lara Guarch, Pablo, dev, Wu, Jingjing

On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > Add parameter to start forwarding sending first
> > > a burst of packets, which is useful when testing
> > > a loopback connection.
> > > 
> > > This was already implemented as an internal command,
> > > but adding it as a parameter is interesting, as it
> > > allows the user to test a loopback connection without
> > > entering in the internal command line.
> > > 
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > ---
> > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > @@ -188,6 +188,14 @@ The commandline options are:
> > > 
> > >      Start forwarding on initialization.
> > > 
> > > +*   ``--tx-first``
> > > +
> > > +    Start forwarding, after sending a burst of packets first.
> > > +
> > > +.. Note::
> > > +
> > > +   This flag should be only used in non-interactive mode.
> 
> I don't really understand the benefit of this option.
> Why is it better than
> 	echo start tx_first | testpmd -i
> ?

The one big difference I see is normal vs abnormal termination. With the
echo command you suggest, the only way to terminate testpmd is to kill
it via signal. With the extra cmdline option, it will cleanly exit via
enter as with non-interactive mode right now. Not a huge difference, but
I think having the extra argument to enable tx-first is useful.

/Bruce

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-20  9:22         ` Bruce Richardson
@ 2017-06-20  9:58           ` Thomas Monjalon
  2017-06-20 10:19             ` Gaëtan Rivet
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Monjalon @ 2017-06-20  9:58 UTC (permalink / raw)
  To: Bruce Richardson, De Lara Guarch, Pablo; +Cc: dev, Wu, Jingjing

20/06/2017 11:22, Bruce Richardson:
> On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> > 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > > Add parameter to start forwarding sending first
> > > > a burst of packets, which is useful when testing
> > > > a loopback connection.
> > > > 
> > > > This was already implemented as an internal command,
> > > > but adding it as a parameter is interesting, as it
> > > > allows the user to test a loopback connection without
> > > > entering in the internal command line.
> > > > 
> > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > ---
> > > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > > @@ -188,6 +188,14 @@ The commandline options are:
> > > > 
> > > >      Start forwarding on initialization.
> > > > 
> > > > +*   ``--tx-first``
> > > > +
> > > > +    Start forwarding, after sending a burst of packets first.
> > > > +
> > > > +.. Note::
> > > > +
> > > > +   This flag should be only used in non-interactive mode.
> > 
> > I don't really understand the benefit of this option.
> > Why is it better than
> > 	echo start tx_first | testpmd -i
> > ?
> 
> The one big difference I see is normal vs abnormal termination. With the
> echo command you suggest, the only way to terminate testpmd is to kill
> it via signal. With the extra cmdline option, it will cleanly exit via
> enter as with non-interactive mode right now. Not a huge difference, but
> I think having the extra argument to enable tx-first is useful.

I do not see a big difference between "enter" and "ctrl+c".

I think it is more flexible to pipe commands instead of options.
We could combine the proposed options --tx-first and -T into this command:
( echo 'start tx_first' ; while true ; do echo 'show port stats all' ; sleep 1 ; done ) | testpmd -i 

It is even possible to add more actions in the loop, so it is less
limited than the -T options.

It is a matter of deciding whether we prefer to implement more restricted
options or leverage the shell to freely program non-interactive testpmd.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-20  9:58           ` Thomas Monjalon
@ 2017-06-20 10:19             ` Gaëtan Rivet
  2017-06-20 10:27               ` Van Haaren, Harry
  2017-06-20 11:20               ` Bruce Richardson
  0 siblings, 2 replies; 18+ messages in thread
From: Gaëtan Rivet @ 2017-06-20 10:19 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Bruce Richardson, De Lara Guarch, Pablo, dev, Wu, Jingjing

On Tue, Jun 20, 2017 at 11:58:54AM +0200, Thomas Monjalon wrote:
> 20/06/2017 11:22, Bruce Richardson:
> > On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> > > 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > > > Add parameter to start forwarding sending first
> > > > > a burst of packets, which is useful when testing
> > > > > a loopback connection.
> > > > > 
> > > > > This was already implemented as an internal command,
> > > > > but adding it as a parameter is interesting, as it
> > > > > allows the user to test a loopback connection without
> > > > > entering in the internal command line.
> > > > > 
> > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > > ---
> > > > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > > > @@ -188,6 +188,14 @@ The commandline options are:
> > > > > 
> > > > >      Start forwarding on initialization.
> > > > > 
> > > > > +*   ``--tx-first``
> > > > > +
> > > > > +    Start forwarding, after sending a burst of packets first.
> > > > > +
> > > > > +.. Note::
> > > > > +
> > > > > +   This flag should be only used in non-interactive mode.
> > > 
> > > I don't really understand the benefit of this option.
> > > Why is it better than
> > > 	echo start tx_first | testpmd -i
> > > ?
> > 
> > The one big difference I see is normal vs abnormal termination. With the
> > echo command you suggest, the only way to terminate testpmd is to kill
> > it via signal. With the extra cmdline option, it will cleanly exit via
> > enter as with non-interactive mode right now. Not a huge difference, but
> > I think having the extra argument to enable tx-first is useful.

Side note, one can:

(trap 'echo quit' SIGUSR1; echo 'start'; while true; do sleep 1; done) |testpmd -i &

and issue a SIGUSR1 to the subshell to cleanly quit testpmd.
Or:

(trap 'echo show port stats all' SIGUSR1; \
 trap 'echo quit' SIGUSR2; \
 echo 'start'; \
 while true; do :; done) |\
testpmd -i &

It's a little contrived, but it does the job and is easy enough to put
in scripts.

For automated testing, what I did was open a pipe to testpmd to issue
commands from wherever, allowing to keep testpmd running in the
background and dispatch commands when needed from other terminals. No
need to issue signals at all then.

> 
> I do not see a big difference between "enter" and "ctrl+c".
> 
> I think it is more flexible to pipe commands instead of options.
> We could combine the proposed options --tx-first and -T into this command:
> ( echo 'start tx_first' ; while true ; do echo 'show port stats all' ; sleep 1 ; done ) | testpmd -i 
> 
> It is even possible to add more actions in the loop, so it is less
> limited than the -T options.
> 
> It is a matter of deciding whether we prefer to implement more restricted
> options or leverage the shell to freely program non-interactive testpmd.

-- 
Gaëtan Rivet
6WIND

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-20 10:19             ` Gaëtan Rivet
@ 2017-06-20 10:27               ` Van Haaren, Harry
  2017-06-20 11:20               ` Bruce Richardson
  1 sibling, 0 replies; 18+ messages in thread
From: Van Haaren, Harry @ 2017-06-20 10:27 UTC (permalink / raw)
  To: Gaëtan Rivet, Thomas Monjalon
  Cc: Richardson, Bruce, De Lara Guarch, Pablo, dev, Wu, Jingjing

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaëtan Rivet
> Sent: Tuesday, June 20, 2017 11:19 AM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] app/testpmd: add parameter to start forwarding TX first
> 
> On Tue, Jun 20, 2017 at 11:58:54AM +0200, Thomas Monjalon wrote:
> > 20/06/2017 11:22, Bruce Richardson:
> > > On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> > > > 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > > > > Add parameter to start forwarding sending first
> > > > > > a burst of packets, which is useful when testing
> > > > > > a loopback connection.
> > > > > >
> > > > > > This was already implemented as an internal command,
> > > > > > but adding it as a parameter is interesting, as it
> > > > > > allows the user to test a loopback connection without
> > > > > > entering in the internal command line.
> > > > > >
> > > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > > > ---
> > > > > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > @@ -188,6 +188,14 @@ The commandline options are:
> > > > > >
> > > > > >      Start forwarding on initialization.
> > > > > >
> > > > > > +*   ``--tx-first``
> > > > > > +
> > > > > > +    Start forwarding, after sending a burst of packets first.
> > > > > > +
> > > > > > +.. Note::
> > > > > > +
> > > > > > +   This flag should be only used in non-interactive mode.
> > > >
> > > > I don't really understand the benefit of this option.
> > > > Why is it better than
> > > > 	echo start tx_first | testpmd -i
> > > > ?
> > >
> > > The one big difference I see is normal vs abnormal termination. With the
> > > echo command you suggest, the only way to terminate testpmd is to kill
> > > it via signal. With the extra cmdline option, it will cleanly exit via
> > > enter as with non-interactive mode right now. Not a huge difference, but
> > > I think having the extra argument to enable tx-first is useful.
> 
> Side note, one can:
> 
> (trap 'echo quit' SIGUSR1; echo 'start'; while true; do sleep 1; done) |testpmd -i &
> 
> and issue a SIGUSR1 to the subshell to cleanly quit testpmd.
> Or:
> 
> (trap 'echo show port stats all' SIGUSR1; \
>  trap 'echo quit' SIGUSR2; \
>  echo 'start'; \
>  while true; do :; done) |\
> testpmd -i &
> 
> It's a little contrived, but it does the job and is easy enough to put
> in scripts.
> 
> For automated testing, what I did was open a pipe to testpmd to issue
> commands from wherever, allowing to keep testpmd running in the
> background and dispatch commands when needed from other terminals. No
> need to issue signals at all then.

In an expert-user automated testing environment this is a solution yes..


> > I do not see a big difference between "enter" and "ctrl+c".
> >
> > I think it is more flexible to pipe commands instead of options.
> > We could combine the proposed options --tx-first and -T into this command:
> > ( echo 'start tx_first' ; while true ; do echo 'show port stats all' ; sleep 1 ; done ) |
> testpmd -i
> >
> > It is even possible to add more actions in the loop, so it is less
> > limited than the -T options.
> >
> > It is a matter of deciding whether we prefer to implement more restricted
> > options or leverage the shell to freely program non-interactive testpmd.


Lets keep in mind the beginner users, who probably don't have a traffic generator machine, perhaps just 2 ports and a loopback cable. IMO running   ./testpmd -- --tx-first   is the easiest way to learn to run traffic using testpmd, instead of various shell tricks to work-around a missing CLI flag?

A +1 from me to add   --tx-first  to the CLI, -Harry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-20 10:19             ` Gaëtan Rivet
  2017-06-20 10:27               ` Van Haaren, Harry
@ 2017-06-20 11:20               ` Bruce Richardson
  2017-06-20 12:30                 ` Gaëtan Rivet
  1 sibling, 1 reply; 18+ messages in thread
From: Bruce Richardson @ 2017-06-20 11:20 UTC (permalink / raw)
  To: Gaëtan Rivet
  Cc: Thomas Monjalon, De Lara Guarch, Pablo, dev, Wu, Jingjing

On Tue, Jun 20, 2017 at 12:19:01PM +0200, Gaëtan Rivet wrote:
> On Tue, Jun 20, 2017 at 11:58:54AM +0200, Thomas Monjalon wrote:
> > 20/06/2017 11:22, Bruce Richardson:
> > > On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> > > > 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > > > > Add parameter to start forwarding sending first
> > > > > > a burst of packets, which is useful when testing
> > > > > > a loopback connection.
> > > > > > 
> > > > > > This was already implemented as an internal command,
> > > > > > but adding it as a parameter is interesting, as it
> > > > > > allows the user to test a loopback connection without
> > > > > > entering in the internal command line.
> > > > > > 
> > > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > > > ---
> > > > > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > @@ -188,6 +188,14 @@ The commandline options are:
> > > > > > 
> > > > > >      Start forwarding on initialization.
> > > > > > 
> > > > > > +*   ``--tx-first``
> > > > > > +
> > > > > > +    Start forwarding, after sending a burst of packets first.
> > > > > > +
> > > > > > +.. Note::
> > > > > > +
> > > > > > +   This flag should be only used in non-interactive mode.
> > > > 
> > > > I don't really understand the benefit of this option.
> > > > Why is it better than
> > > > 	echo start tx_first | testpmd -i
> > > > ?
> > > 
> > > The one big difference I see is normal vs abnormal termination. With the
> > > echo command you suggest, the only way to terminate testpmd is to kill
> > > it via signal. With the extra cmdline option, it will cleanly exit via
> > > enter as with non-interactive mode right now. Not a huge difference, but
> > > I think having the extra argument to enable tx-first is useful.
> 
> Side note, one can:
> 
> (trap 'echo quit' SIGUSR1; echo 'start'; while true; do sleep 1; done) |testpmd -i &
> 
> and issue a SIGUSR1 to the subshell to cleanly quit testpmd.
> Or:
> 
> (trap 'echo show port stats all' SIGUSR1; \
>  trap 'echo quit' SIGUSR2; \
>  echo 'start'; \
>  while true; do :; done) |\
> testpmd -i &
> 
> It's a little contrived, but it does the job and is easy enough to put
> in scripts.
> 
Or we can just put in a --tx-first flag and save the user the pain of
doing multi-line bash commands. Usability is a constant complaint about
DPDK that we hear e.g. at userspace every year, so we need to stop
assuming everyone looking to play with DPDK is a shell power-user.

/Bruce

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-20 11:20               ` Bruce Richardson
@ 2017-06-20 12:30                 ` Gaëtan Rivet
  0 siblings, 0 replies; 18+ messages in thread
From: Gaëtan Rivet @ 2017-06-20 12:30 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Thomas Monjalon, De Lara Guarch, Pablo, dev, Wu, Jingjing

On Tue, Jun 20, 2017 at 12:20:34PM +0100, Bruce Richardson wrote:
> On Tue, Jun 20, 2017 at 12:19:01PM +0200, Gaëtan Rivet wrote:
> > On Tue, Jun 20, 2017 at 11:58:54AM +0200, Thomas Monjalon wrote:
> > > 20/06/2017 11:22, Bruce Richardson:
> > > > On Mon, Jun 19, 2017 at 11:12:53PM +0200, Thomas Monjalon wrote:
> > > > > 15/06/2017 14:05, De Lara Guarch, Pablo:
> > > > > > > Add parameter to start forwarding sending first
> > > > > > > a burst of packets, which is useful when testing
> > > > > > > a loopback connection.
> > > > > > > 
> > > > > > > This was already implemented as an internal command,
> > > > > > > but adding it as a parameter is interesting, as it
> > > > > > > allows the user to test a loopback connection without
> > > > > > > entering in the internal command line.
> > > > > > > 
> > > > > > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > > > > > ---
> > > > > > > --- a/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > > +++ b/doc/guides/testpmd_app_ug/run_app.rst
> > > > > > > @@ -188,6 +188,14 @@ The commandline options are:
> > > > > > > 
> > > > > > >      Start forwarding on initialization.
> > > > > > > 
> > > > > > > +*   ``--tx-first``
> > > > > > > +
> > > > > > > +    Start forwarding, after sending a burst of packets first.
> > > > > > > +
> > > > > > > +.. Note::
> > > > > > > +
> > > > > > > +   This flag should be only used in non-interactive mode.
> > > > > 
> > > > > I don't really understand the benefit of this option.
> > > > > Why is it better than
> > > > > 	echo start tx_first | testpmd -i
> > > > > ?
> > > > 
> > > > The one big difference I see is normal vs abnormal termination. With the
> > > > echo command you suggest, the only way to terminate testpmd is to kill
> > > > it via signal. With the extra cmdline option, it will cleanly exit via
> > > > enter as with non-interactive mode right now. Not a huge difference, but
> > > > I think having the extra argument to enable tx-first is useful.
> > 
> > Side note, one can:
> > 
> > (trap 'echo quit' SIGUSR1; echo 'start'; while true; do sleep 1; done) |testpmd -i &
> > 
> > and issue a SIGUSR1 to the subshell to cleanly quit testpmd.
> > Or:
> > 
> > (trap 'echo show port stats all' SIGUSR1; \
> >  trap 'echo quit' SIGUSR2; \
> >  echo 'start'; \
> >  while true; do :; done) |\
> > testpmd -i &
> > 
> > It's a little contrived, but it does the job and is easy enough to put
> > in scripts.
> > 
> Or we can just put in a --tx-first flag and save the user the pain of
> doing multi-line bash commands. Usability is a constant complaint about
> DPDK that we hear e.g. at userspace every year, so we need to stop
> assuming everyone looking to play with DPDK is a shell power-user.
> 
> /Bruce

Sure, but who are the users of testpmd, beside DPDK developers and
integration bots?

This fix IMO may be useful but it is a crutch for this usability
problem. The aimed functionality would result in the end in adding one
by one all interactive commands as parameters to have feature-parity
between interactive and non-interactive mode.

A better solution could be to open the cmdline from testpmd to a pipe
when not in interactive mode, instead of relying on the user to do it
artificially (as I was doing with my multiline shell command).
Then the parameters could be restricted to configuration items that ought
to be set before probing any device, instead of each possible elements
being added twice in the cmdline and in the parameters.

A kind of daemon / background mode if you want.
Anyway, that's just my limited experience with integrating testpmd to a
CI and using it, I reckon that it may not be the most common use, and I
have no opinion regarding this parameter ; I only wanted to give my PoV.

-- 
Gaëtan Rivet
6WIND

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v3] app/testpmd: add parameter to start forwarding TX first
  2017-06-19  1:05       ` Wu, Jingjing
@ 2017-07-05 23:48         ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2017-07-05 23:48 UTC (permalink / raw)
  To: De Lara Guarch, Pablo; +Cc: dev, Wu, Jingjing

> > > Add parameter to start forwarding sending first a burst of packets,
> > > which is useful when testing a loopback connection.
> > >
> > > This was already implemented as an internal command, but adding it as
> > > a parameter is interesting, as it allows the user to test a loopback
> > > connection without entering in the internal command line.
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > > ---
> > >
> > > Changes in v3:
> > >
> > > -Added tx-first in long parameter list -Reworded informational message
> > > when tx-first is enabled
> 
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>

It is said to be useful for beginners, so it is accepted.
However we are not going to copy every CLI commands in parameters.

Applied, thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-07-05 23:48 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-28 21:36 [PATCH] app/testpmd: add parameter to start forwarding sending Pablo de Lara
2017-06-07  9:17 ` Wu, Jingjing
2017-06-07 13:20   ` De Lara Guarch, Pablo
2017-06-07 13:23     ` Bruce Richardson
2017-06-09  2:30 ` [PATCH v2] " Pablo de Lara
2017-06-15  4:04   ` [PATCH v3] app/testpmd: add parameter to start forwarding TX first Pablo de Lara
2017-06-15 12:05     ` De Lara Guarch, Pablo
2017-06-19  1:05       ` Wu, Jingjing
2017-07-05 23:48         ` Thomas Monjalon
2017-06-19 21:12       ` Thomas Monjalon
2017-06-20  8:03         ` De Lara Guarch, Pablo
2017-06-20  9:22         ` Bruce Richardson
2017-06-20  9:58           ` Thomas Monjalon
2017-06-20 10:19             ` Gaëtan Rivet
2017-06-20 10:27               ` Van Haaren, Harry
2017-06-20 11:20               ` Bruce Richardson
2017-06-20 12:30                 ` Gaëtan Rivet
2017-06-19  0:53   ` [PATCH v2] app/testpmd: add parameter to start forwarding sending Wu, Jingjing

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.