[dpdk-dev,v4] app/testpmd: print statistics periodically

Message ID 20170706015205.74325-1-pablo.de.lara.guarch@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

De Lara Guarch, Pablo July 6, 2017, 1:52 a.m. UTC
  Add parameter to print port statistics periodically
(disabled by default), if interactive mode is not enabled.

This is useful to allow the user to see port statistics
without having to get into the internal command line.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
---

Changes in v4:

- Removed CamelCase
- Used generic rte_get_timer_cycles() function, instead 
  of TSC API.

Changes in v3:

- Added missing "|" character in help

Changes in v2:

- Added extra argument in help 

 app/test-pmd/parameters.c             | 19 +++++++++++++++--
 app/test-pmd/testpmd.c                | 40 ++++++++++++++++++++++++++++++++++-
 app/test-pmd/testpmd.h                |  1 +
 doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
 4 files changed, 62 insertions(+), 3 deletions(-)
  

Comments

Jerin Jacob July 6, 2017, 10:06 a.m. UTC | #1
-----Original Message-----
> Date: Thu, 6 Jul 2017 02:52:05 +0100
> From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> To: jingjing.wu@intel.com, thomas@monjalon.net
> CC: dev@dpdk.org, Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev] [PATCH v4] app/testpmd: print statistics periodically
> X-Mailer: git-send-email 2.9.4
> 
> Add parameter to print port statistics periodically
> (disabled by default), if interactive mode is not enabled.
> 
> This is useful to allow the user to see port statistics
> without having to get into the internal command line.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
> 
> Changes in v4:
> 
> - Removed CamelCase
> - Used generic rte_get_timer_cycles() function, instead 
>   of TSC API.
> 
> Changes in v3:
> 
> - Added missing "|" character in help
> 
> Changes in v2:
> 
> - Added extra argument in help 
> 
>  app/test-pmd/parameters.c             | 19 +++++++++++++++--
>  app/test-pmd/testpmd.c                | 40 ++++++++++++++++++++++++++++++++++-
>  app/test-pmd/testpmd.h                |  1 +
>  doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
>  4 files changed, 62 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index fbe6284..41ed74c 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -88,7 +88,7 @@ usage(char* progname)
>  	       "[--interactive|-i] "
>  	       "[--cmdline-file=FILENAME] "
>  #endif
> -	       "[--help|-h] | [--auto-start|-a] | ["
> +	       "[--help|-h] | [--auto-start|-a] | [-T PERIOD] | ["
>  	       "--coremask=COREMASK --portmask=PORTMASK --numa "
>  	       "--mbuf-size= | --total-num-mbufs= | "
>  	       "--nb-cores= | --nb-ports= | "
> @@ -109,6 +109,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("  -T PERIOD: statistics will be shown every PERIOD seconds "

Considering all the option are in small letter, Does it makes sense to
make it as small letter? (ie -t instead of -T).

Other than it looks good and useful feature.

Tested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
  
Thomas Monjalon July 6, 2017, 10:16 a.m. UTC | #2
06/07/2017 03:52, Pablo de Lara:
> Add parameter to print port statistics periodically
> (disabled by default), if interactive mode is not enabled.
> 
> This is useful to allow the user to see port statistics
> without having to get into the internal command line.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
> 
> Changes in v4:
> 
> - Removed CamelCase
> - Used generic rte_get_timer_cycles() function, instead 
>   of TSC API.

Thank you for the update.
Please, may I request a v5 rebased on current master?
  
De Lara Guarch, Pablo July 6, 2017, 10:19 a.m. UTC | #3
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, July 6, 2017 11:16 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [PATCH v4] app/testpmd: print statistics periodically
> 
> 06/07/2017 03:52, Pablo de Lara:
> > Add parameter to print port statistics periodically (disabled by
> > default), if interactive mode is not enabled.
> >
> > This is useful to allow the user to see port statistics without having
> > to get into the internal command line.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >
> > Changes in v4:
> >
> > - Removed CamelCase
> > - Used generic rte_get_timer_cycles() function, instead
> >   of TSC API.
> 
> Thank you for the update.
> Please, may I request a v5 rebased on current master?

Oh... sure, sorry about that. Will send a v5, and probably change the parameter name,
as per Jerin's request.
  
De Lara Guarch, Pablo July 6, 2017, 10:43 a.m. UTC | #4
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jerin Jacob
> Sent: Thursday, July 6, 2017 11:07 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; thomas@monjalon.net;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4] app/testpmd: print statistics
> periodically
> 
> -----Original Message-----
> > Date: Thu, 6 Jul 2017 02:52:05 +0100
> > From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > To: jingjing.wu@intel.com, thomas@monjalon.net
> > CC: dev@dpdk.org, Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Subject: [dpdk-dev] [PATCH v4] app/testpmd: print statistics
> > periodically
> > X-Mailer: git-send-email 2.9.4
> >
> > Add parameter to print port statistics periodically (disabled by
> > default), if interactive mode is not enabled.
> >
> > This is useful to allow the user to see port statistics without having
> > to get into the internal command line.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > Acked-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >
> > Changes in v4:
> >
> > - Removed CamelCase
> > - Used generic rte_get_timer_cycles() function, instead
> >   of TSC API.
> >
> > Changes in v3:
> >
> > - Added missing "|" character in help
> >
> > Changes in v2:
> >
> > - Added extra argument in help
> >
> >  app/test-pmd/parameters.c             | 19 +++++++++++++++--
> >  app/test-pmd/testpmd.c                | 40
> ++++++++++++++++++++++++++++++++++-
> >  app/test-pmd/testpmd.h                |  1 +
> >  doc/guides/testpmd_app_ug/run_app.rst |  5 +++++
> >  4 files changed, 62 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index fbe6284..41ed74c 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -88,7 +88,7 @@ usage(char* progname)
> >  	       "[--interactive|-i] "
> >  	       "[--cmdline-file=FILENAME] "
> >  #endif
> > -	       "[--help|-h] | [--auto-start|-a] | ["
> > +	       "[--help|-h] | [--auto-start|-a] | [-T PERIOD] | ["
> >  	       "--coremask=COREMASK --portmask=PORTMASK --numa "
> >  	       "--mbuf-size= | --total-num-mbufs= | "
> >  	       "--nb-cores= | --nb-ports= | "
> > @@ -109,6 +109,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("  -T PERIOD: statistics will be shown every PERIOD seconds "
> 
> Considering all the option are in small letter, Does it makes sense to make it
> as small letter? (ie -t instead of -T).
> 
> Other than it looks good and useful feature.
> 
> Tested-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Thanks for the testing. I took the parameter name from L2fwd, but you are right,
it should be lowercase. Actually, to make it more explicit, I propose 
to change it to "--stats-period".

Will send a v5 soon.
Thanks,
Pablo
  

Patch

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index fbe6284..41ed74c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -88,7 +88,7 @@  usage(char* progname)
 	       "[--interactive|-i] "
 	       "[--cmdline-file=FILENAME] "
 #endif
-	       "[--help|-h] | [--auto-start|-a] | ["
+	       "[--help|-h] | [--auto-start|-a] | [-T PERIOD] | ["
 	       "--coremask=COREMASK --portmask=PORTMASK --numa "
 	       "--mbuf-size= | --total-num-mbufs= | "
 	       "--nb-cores= | --nb-ports= | "
@@ -109,6 +109,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("  -T PERIOD: statistics will be shown every PERIOD seconds "
+	       "(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 "
@@ -639,7 +641,7 @@  launch_args_parse(int argc, char** argv)
 #else
 #define SHORTOPTS ""
 #endif
-	while ((opt = getopt_long(argc, argvopt, SHORTOPTS "ah",
+	while ((opt = getopt_long(argc, argvopt, SHORTOPTS "ahT:",
 				 lgopts, &opt_idx)) != EOF) {
 		switch (opt) {
 #ifdef RTE_LIBRTE_CMDLINE
@@ -653,6 +655,19 @@  launch_args_parse(int argc, char** argv)
 			auto_start = 1;
 			break;
 
+		case 'T':
+		{
+			char *end = NULL;
+			unsigned int n;
+
+			n = strtoul(optarg, &end, 10);
+			if ((optarg[0] == '\0') || (end == NULL) ||
+					(*end != '\0'))
+				break;
+
+			stats_period = n;
+			break;
+		}
 		case 0: /*long options */
 			if (!strcmp(lgopts[opt_idx].name, "help")) {
 				usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 0a23d82..343b0f4 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -180,7 +180,7 @@  uint32_t burst_tx_retry_num = BURST_TX_RETRIES;
 uint16_t mbuf_data_size = DEFAULT_MBUF_DATA_SIZE; /**< Mbuf data space size. */
 uint32_t param_total_num_mbufs = 0;  /**< number of mbufs in all pools - if
                                       * specified on command-line. */
-
+uint16_t stats_period; /**< Period to show statistics (disabled by default) */
 /*
  * Configuration of packet segments used by the "txonly" processing engine.
  */
@@ -2235,6 +2235,21 @@  force_quit(void)
 }
 
 static void
+print_stats(void)
+{
+	uint8_t i;
+	const char clr[] = { 27, '[', '2', 'J', '\0' };
+	const char top_left[] = { 27, '[', '1', ';', '1', 'H', '\0' };
+
+	/* Clear screen and move to top left */
+	printf("%s%s", clr, top_left);
+
+	printf("\nPort statistics ====================================");
+	for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++)
+		nic_stats_display(fwd_ports_ids[i]);
+}
+
+static void
 signal_handler(int signum)
 {
 	if (signum == SIGINT || signum == SIGTERM) {
@@ -2357,8 +2372,31 @@  main(int argc, char** argv)
 
 		printf("No commandline core given, start packet forwarding\n");
 		start_packet_forwarding(0);
+		if (stats_period != 0) {
+			uint64_t prev_time = 0, cur_time, diff_time = 0;
+			uint64_t timer_period;
+
+			/* Convert to number of cycles */
+			timer_period = stats_period * rte_get_timer_hz();
+
+			while (1) {
+				cur_time = rte_get_timer_cycles();
+				diff_time += cur_time - prev_time;
+
+				if (diff_time >= timer_period) {
+					print_stats();
+					/* Reset the timer */
+					diff_time = 0;
+				}
+				/* Sleep to avoid unnecessary checks */
+				prev_time = cur_time;
+				sleep(1);
+			}
+		}
+
 		printf("Press enter to exit\n");
 		rc = read(0, &c, 1);
+
 		pmd_test_exit();
 		if (rc < 0)
 			return 1;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 364502d..bf9126c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -378,6 +378,7 @@  extern enum dcb_queue_mapping_mode dcb_q_mapping;
 extern uint16_t mbuf_data_size; /**< Mbuf data space size. */
 extern uint32_t param_total_num_mbufs;
 
+extern uint16_t stats_period;
 
 #ifdef RTE_LIBRTE_LATENCY_STATS
 extern uint8_t latencystats_enabled;
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 2a43214..bebe46a 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -188,6 +188,11 @@  The commandline options are:
 
     Start forwarding on initialization.
 
+*   ``-T PERIOD``
+
+    Display statistics every PERIOD seconds, if interactive mode is disabled.
+    The default value is 0, which means that the statistics will not be displayed.
+
 *   ``--nb-cores=N``
 
     Set the number of forwarding cores,