diff mbox

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

Message ID 1482311376-38091-1-git-send-email-jianfeng.tan@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers

Checks

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

Commit Message

Jianfeng Tan Dec. 21, 2016, 9:09 a.m. UTC 
  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(-)

Comments

De Lara Guarch, Pablo Jan. 17, 2017, 4:04 p.m. UTC | #1 
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
Jianfeng Tan Jan. 18, 2017, 6:46 a.m. UTC | #2 
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
De Lara Guarch, Pablo Jan. 18, 2017, 9:02 a.m. UTC | #3 
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
Jianfeng Tan Jan. 18, 2017, 9:07 a.m. UTC | #4 
> -----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
Thomas Monjalon Feb. 8, 2017, 4:30 p.m. UTC | #5 
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?
Thomas Monjalon April 30, 2017, 4 p.m. UTC | #6 
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?
diff mbox

Patch

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);
 }