[dpdk-dev] [PATCH] app/testpmd: refine xstats show

[dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Jianfeng Tan]

When using "show port xstats all" command to show xstats, the output
is usually too long to obtain what you really want, expecially when
multi-queue is enabled.

This patch refines this situation by skipping showing those with value
of zero.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 app/test-pmd/config.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 36c47ab..1adef29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -304,10 +304,13 @@ nic_xstats_display(portid_t port_id)
 	}
 
 	/* Display xstats */
-	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
+	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
+		if ((xstats[idx_xstat].value) == 0)
+			continue;
 		printf("%s: %"PRIu64"\n",
 			xstats_names[idx_xstat].name,
 			xstats[idx_xstat].value);
+	}
 	free(xstats_names);
 	free(xstats);
 }
-- 
2.7.4

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[De Lara Guarch, Pablo]

Hi Jianfeng,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
> Sent: Wednesday, December 21, 2016 9:10 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Tan, Jianfeng
> Subject: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> 
> When using "show port xstats all" command to show xstats, the output
> is usually too long to obtain what you really want, expecially when
> multi-queue is enabled.
> 
> This patch refines this situation by skipping showing those with value
> of zero.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  app/test-pmd/config.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 36c47ab..1adef29 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -304,10 +304,13 @@ nic_xstats_display(portid_t port_id)
>  	}
> 
>  	/* Display xstats */
> -	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
> +	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
> +		if ((xstats[idx_xstat].value) == 0)
> +			continue;
>  		printf("%s: %"PRIu64"\n",
>  			xstats_names[idx_xstat].name,
>  			xstats[idx_xstat].value);
> +	}
>  	free(xstats_names);
>  	free(xstats);
>  }
> --
> 2.7.4

I think this is a good idea, but I would give the user the option to do this, as sometimes it is useful to actually show statistics with value 0.
Could you extend the show port xstats command to accept a new parameter instead?
It would be nice to have something loke show port xstats all hide-zeros, maybe?

Thanks,
Pablo

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Tan, Jianfeng]

Hi Pablo,

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, January 18, 2017 12:04 AM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: Wu, Jingjing; Tan, Jianfeng
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> 
> Hi Jianfeng,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
> > Sent: Wednesday, December 21, 2016 9:10 AM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing; Tan, Jianfeng
> > Subject: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> >
> > When using "show port xstats all" command to show xstats, the output
> > is usually too long to obtain what you really want, expecially when
> > multi-queue is enabled.
> >
> > This patch refines this situation by skipping showing those with value
> > of zero.
> >
> > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > ---
> >  app/test-pmd/config.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > index 36c47ab..1adef29 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -304,10 +304,13 @@ nic_xstats_display(portid_t port_id)
> >  	}
> >
> >  	/* Display xstats */
> > -	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
> > +	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
> > +		if ((xstats[idx_xstat].value) == 0)
> > +			continue;
> >  		printf("%s: %"PRIu64"\n",
> >  			xstats_names[idx_xstat].name,
> >  			xstats[idx_xstat].value);
> > +	}
> >  	free(xstats_names);
> >  	free(xstats);
> >  }
> > --
> > 2.7.4
> 
> I think this is a good idea, but I would give the user the option to do this, as
> sometimes it is useful to actually show statistics with value 0.

Make sense.

> Could you extend the show port xstats command to accept a new parameter
> instead?

OK.

> It would be nice to have something loke show port xstats all hide-zeros,
> maybe?

How about "nz" for non-zero?

Thanks,
Jianfeng

> 
> Thanks,
> Pablo

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[De Lara Guarch, Pablo]

Hi Jianfeng,

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Wednesday, January 18, 2017 6:46 AM
> To: De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Wu, Jingjing
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> 
> Hi Pablo,
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Wednesday, January 18, 2017 12:04 AM
> > To: Tan, Jianfeng; dev@dpdk.org
> > Cc: Wu, Jingjing; Tan, Jianfeng
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> >
> > Hi Jianfeng,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
> > > Sent: Wednesday, December 21, 2016 9:10 AM
> > > To: dev@dpdk.org
> > > Cc: Wu, Jingjing; Tan, Jianfeng
> > > Subject: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> > >
> > > When using "show port xstats all" command to show xstats, the output
> > > is usually too long to obtain what you really want, expecially when
> > > multi-queue is enabled.
> > >
> > > This patch refines this situation by skipping showing those with value
> > > of zero.
> > >
> > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > > ---
> > >  app/test-pmd/config.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > > index 36c47ab..1adef29 100644
> > > --- a/app/test-pmd/config.c
> > > +++ b/app/test-pmd/config.c
> > > @@ -304,10 +304,13 @@ nic_xstats_display(portid_t port_id)
> > >  	}
> > >
> > >  	/* Display xstats */
> > > -	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
> > > +	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
> > > +		if ((xstats[idx_xstat].value) == 0)
> > > +			continue;
> > >  		printf("%s: %"PRIu64"\n",
> > >  			xstats_names[idx_xstat].name,
> > >  			xstats[idx_xstat].value);
> > > +	}
> > >  	free(xstats_names);
> > >  	free(xstats);
> > >  }
> > > --
> > > 2.7.4
> >
> > I think this is a good idea, but I would give the user the option to do this,
> as
> > sometimes it is useful to actually show statistics with value 0.
> 
> Make sense.
> 
> > Could you extend the show port xstats command to accept a new
> parameter
> > instead?
> 
> OK.
> 
> > It would be nice to have something loke show port xstats all hide-zeros,
> > maybe?
> 
> How about "nz" for non-zero?

I would prefer the full name. If you want nonzero, that's ok for me, but there is
no need to make it shorter and not readable enough (I wouldn't know what nz is).

Thanks,
Pablo


> 
> Thanks,
> Jianfeng
> 
> >
> > Thanks,
> > Pablo

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Tan, Jianfeng]

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, January 18, 2017 5:03 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: Wu, Jingjing
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> 
> Hi Jianfeng,
> 
> > -----Original Message-----
> > From: Tan, Jianfeng
> > Sent: Wednesday, January 18, 2017 6:46 AM
> > To: De Lara Guarch, Pablo; dev@dpdk.org
> > Cc: Wu, Jingjing
> > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> >
> > Hi Pablo,
> >
> > > -----Original Message-----
> > > From: De Lara Guarch, Pablo
> > > Sent: Wednesday, January 18, 2017 12:04 AM
> > > To: Tan, Jianfeng; dev@dpdk.org
> > > Cc: Wu, Jingjing; Tan, Jianfeng
> > > Subject: RE: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> > >
> > > Hi Jianfeng,
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jianfeng Tan
> > > > Sent: Wednesday, December 21, 2016 9:10 AM
> > > > To: dev@dpdk.org
> > > > Cc: Wu, Jingjing; Tan, Jianfeng
> > > > Subject: [dpdk-dev] [PATCH] app/testpmd: refine xstats show
> > > >
> > > > When using "show port xstats all" command to show xstats, the output
> > > > is usually too long to obtain what you really want, expecially when
> > > > multi-queue is enabled.
> > > >
> > > > This patch refines this situation by skipping showing those with value
> > > > of zero.
> > > >
> > > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > > > ---
> > > >  app/test-pmd/config.c | 5 ++++-
> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> > > > index 36c47ab..1adef29 100644
> > > > --- a/app/test-pmd/config.c
> > > > +++ b/app/test-pmd/config.c
> > > > @@ -304,10 +304,13 @@ nic_xstats_display(portid_t port_id)
> > > >  	}
> > > >
> > > >  	/* Display xstats */
> > > > -	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
> > > > +	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
> > > > +		if ((xstats[idx_xstat].value) == 0)
> > > > +			continue;
> > > >  		printf("%s: %"PRIu64"\n",
> > > >  			xstats_names[idx_xstat].name,
> > > >  			xstats[idx_xstat].value);
> > > > +	}
> > > >  	free(xstats_names);
> > > >  	free(xstats);
> > > >  }
> > > > --
> > > > 2.7.4
> > >
> > > I think this is a good idea, but I would give the user the option to do this,
> > as
> > > sometimes it is useful to actually show statistics with value 0.
> >
> > Make sense.
> >
> > > Could you extend the show port xstats command to accept a new
> > parameter
> > > instead?
> >
> > OK.
> >
> > > It would be nice to have something loke show port xstats all hide-zeros,
> > > maybe?
> >
> > How about "nz" for non-zero?
> 
> I would prefer the full name. If you want nonzero, that's ok for me, but there
> is
> no need to make it shorter and not readable enough (I wouldn't know what
> nz is).

Make sense. Will send out patch soon.

Thanks,
Jianfeng

> 
> Thanks,
> Pablo
> 
> 
> >
> > Thanks,
> > Jianfeng
> >
> > >
> > > Thanks,
> > > Pablo

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Thomas Monjalon]

2017-01-18 09:07, Tan, Jianfeng:
> > > How about "nz" for non-zero?
> > 
> > I would prefer the full name. If you want nonzero, that's ok for me, but there
> > is
> > no need to make it shorter and not readable enough (I wouldn't know what
> > nz is).
> 
> Make sense. Will send out patch soon.

Any news?

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Thomas Monjalon]

08/02/2017 17:30, Thomas Monjalon:
> 2017-01-18 09:07, Tan, Jianfeng:
> > > > How about "nz" for non-zero?
> > > 
> > > I would prefer the full name. If you want nonzero, that's ok for me, but there
> > > is
> > > no need to make it shorter and not readable enough (I wouldn't know what
> > > nz is).
> > 
> > Make sense. Will send out patch soon.
> 
> Any news?

Why this patch is forgotten?

[dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Elza Mathew]

When using "show port xstats all" command to show xstats, the output
is usually too long to obtain what you really want, especially when
multi-queue is enabled.

Added an option to set whether zero values should be displayed
or not for xstats. The "set xstats-hide-zero on|off" command enables
the user to decide if the zero values should be shown while
displaying xstats.

Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
Signed-off-by: Elza Mathew <elza.mathew@intel.com>
---
 app/test-pmd/cmdline.c                      | 47 +++++++++++++++++++++++++++++
 app/test-pmd/config.c                       | 11 ++++++-
 app/test-pmd/testpmd.c                      |  5 +++
 app/test-pmd/testpmd.h                      |  4 +++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 11 +++++++
 5 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index bb01e98..c631dc0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -507,6 +507,10 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    e.g., 'set stat_qmap rx 0 2 5' sets rx queue 2"
 			" on port 0 to mapping 5.\n\n"
 
+			"set xstats-hide-zero on|off\n"
+			"    Set the option to hide the zero values"
+			" for xstats display.\n"
+
 			"set port (port_id) vf (vf_id) rx|tx on|off\n"
 			"    Enable/Disable a VF receive/tranmit from a port\n\n"
 
@@ -7078,6 +7082,48 @@ struct cmd_set_qmap_result {
 	},
 };
 
+/* *** SET OPTION TO HIDE ZERO VALUES FOR XSTATS  DISPLAY *** */
+struct cmd_set_xstats_hide_zero_result {
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t name;
+	cmdline_fixed_string_t on_off;
+};
+
+static void
+cmd_set_xstats_hide_zero_parsed(void *parsed_result,
+			__attribute__((unused)) struct cmdline *cl,
+			__attribute__((unused)) void *data)
+{
+	struct cmd_set_xstats_hide_zero_result *res;
+	uint16_t on_off = 0;
+
+	res = parsed_result;
+	on_off = !strcmp(res->on_off, "on") ? 1 : 0;
+	set_xstats_hide_zero(on_off);
+}
+
+cmdline_parse_token_string_t cmd_set_xstats_hide_zero_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_xstats_hide_zero_result,
+				 keyword, "set");
+cmdline_parse_token_string_t cmd_set_xstats_hide_zero_name =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_xstats_hide_zero_result,
+				 name, "xstats-hide-zero");
+cmdline_parse_token_string_t cmd_set_xstats_hide_zero_on_off =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_xstats_hide_zero_result,
+				 on_off, "on#off");
+
+cmdline_parse_inst_t cmd_set_xstats_hide_zero = {
+	.f = cmd_set_xstats_hide_zero_parsed,
+	.data = NULL,
+	.help_str = "set xstats-hide-zero on|off",
+	.tokens = {
+		(void *)&cmd_set_xstats_hide_zero_keyword,
+		(void *)&cmd_set_xstats_hide_zero_name,
+		(void *)&cmd_set_xstats_hide_zero_on_off,
+		NULL,
+	},
+};
+
 /* *** CONFIGURE UNICAST HASH TABLE *** */
 struct cmd_set_uc_hash_table {
 	cmdline_fixed_string_t set;
@@ -15482,6 +15528,7 @@ struct cmd_cmdfile_result {
 	(cmdline_parse_inst_t *)&cmd_stop,
 	(cmdline_parse_inst_t *)&cmd_mac_addr,
 	(cmdline_parse_inst_t *)&cmd_set_qmap,
+	(cmdline_parse_inst_t *)&cmd_set_xstats_hide_zero,
 	(cmdline_parse_inst_t *)&cmd_operate_port,
 	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
 	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index bafe76c..1c8f542 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -292,10 +292,13 @@ struct rss_type_info {
 	}
 
 	/* Display xstats */
-	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
+	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
+		if (xstats_hide_zero && !xstats[idx_xstat].value)
+			continue;
 		printf("%s: %"PRIu64"\n",
 			xstats_names[idx_xstat].name,
 			xstats[idx_xstat].value);
+	}
 	free(xstats_names);
 	free(xstats);
 }
@@ -2866,6 +2869,12 @@ struct igb_ring_desc_16_bytes {
 	}
 }
 
+void
+set_xstats_hide_zero(uint8_t on_off)
+{
+	xstats_hide_zero = on_off;
+}
+
 static inline void
 print_fdir_mask(struct rte_eth_fdir_masks *mask)
 {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index a4d4a86..8b57aaf 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -384,6 +384,11 @@ struct rte_fdir_conf fdir_conf = {
 uint16_t nb_tx_queue_stats_mappings = 0;
 uint16_t nb_rx_queue_stats_mappings = 0;
 
+/*
+ * Display zero values by default for xstats
+ */
+uint8_t xstats_hide_zero;
+
 unsigned int num_sockets = 0;
 unsigned int socket_ids[RTE_MAX_NUMA_NODES];
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 265b75f..4e3cb57 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -348,6 +348,8 @@ struct queue_stats_mappings {
 extern uint16_t nb_tx_queue_stats_mappings;
 extern uint16_t nb_rx_queue_stats_mappings;
 
+extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
+
 /* globals used for configuration */
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern uint8_t  interactive;
@@ -650,6 +652,8 @@ void vlan_tpid_set(portid_t port_id, enum rte_vlan_type vlan_type,
 
 void set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value);
 
+void set_xstats_hide_zero(uint8_t on_off);
+
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void show_tx_pkt_segments(void);
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 46e4db5..5fa3d86 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1220,6 +1220,17 @@ For example, to set rx queue 2 on port 0 to mapping 5::
 
    testpmd>set stat_qmap rx 0 2 5
 
+set xstats-hide-zero
+~~~~~~~~~~~~~~~~~~~~
+
+Set the option to hide zero values for xstats display::
+
+	testpmd> set xstats-hide-zero on|off
+
+.. note::
+
+	By default, the zero values are displayed for xstats.
+
 set port - rx/tx (for VF)
 ~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Ferruh Yigit]

On 10/20/2017 10:09 AM, Elza Mathew wrote:
> When using "show port xstats all" command to show xstats, the output
> is usually too long to obtain what you really want, especially when
> multi-queue is enabled.
> 
> Added an option to set whether zero values should be displayed
> or not for xstats. The "set xstats-hide-zero on|off" command enables
> the user to decide if the zero values should be shown while
> displaying xstats.
> 
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> Signed-off-by: Elza Mathew <elza.mathew@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH] app/testpmd: refine xstats show

[Ferruh Yigit]

On 10/23/2017 4:47 PM, Ferruh Yigit wrote:
> On 10/20/2017 10:09 AM, Elza Mathew wrote:
>> When using "show port xstats all" command to show xstats, the output
>> is usually too long to obtain what you really want, especially when
>> multi-queue is enabled.
>>
>> Added an option to set whether zero values should be displayed
>> or not for xstats. The "set xstats-hide-zero on|off" command enables
>> the user to decide if the zero values should be shown while
>> displaying xstats.
>>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> Signed-off-by: Elza Mathew <elza.mathew@intel.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/master, thanks.

Welcome Elza!