Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info

["Tahhan, Maryam" <maryam.tahhan@intel.com>]

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, July 9, 2015 2:40 AM
> To: Tahhan, Maryam
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 8/8] app: add a new app proc_info
> 
> 2015-07-05 18:40, Maryam Tahhan:
> > proc_info displays statistics information including extened stats for
> 
> typo: extended
> 
> > given DPDK ports and dumps the memory information for DPDK.
> >
> > Signed-off-by: Maryam Tahhan <maryam.tahhan@intel.com>
> > ---
> >  MAINTAINERS            |   4 +
> >  app/Makefile           |   1 +
> >  app/proc_info/Makefile |  45 +++++
> >  app/proc_info/main.c   | 512
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  mk/rte.sdktest.mk      |   4 +-
> >  5 files changed, 564 insertions(+), 2 deletions(-)
> 
> A doc would be useful.

I will add a doc
> 
> > +DIRS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += proc_info
> 
> Why only Linux?

Dump_cfg was only supported on Linux also, I've only tested it on Linux.
> 
> > +#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> 
> Too old :)
> 
> > +	if ((!port->rx_queue_stats_mapping_enabled) && (!port-
> >tx_queue_stats_mapping_enabled)) {
> > +		printf("  RX-packets: %-10"PRIu64" RX-missed: %-10"PRIu64"
> RX-bytes:  "
> > +		       "%-"PRIu64"\n",
> > +		       stats.ipackets, stats.imissed, stats.ibytes);
> > +		printf("  RX-badcrc:  %-10"PRIu64" RX-badlen: %-10"PRIu64"
> RX-errors: "
> > +		       "%-"PRIu64"\n",
> > +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > +		printf("  RX-nombuf:  %-10"PRIu64"\n",
> > +		       stats.rx_nombuf);
> > +		printf("  TX-packets: %-10"PRIu64" TX-errors: %-10"PRIu64"
> TX-bytes:  "
> > +		       "%-"PRIu64"\n",
> > +		       stats.opackets, stats.oerrors, stats.obytes);
> > +	} else {
> > +		printf("  RX-packets:              %10"PRIu64"    RX-errors:
> %10"PRIu64
> > +		       "    RX-bytes: %10"PRIu64"\n",
> > +		       stats.ipackets, stats.ierrors, stats.ibytes);
> > +		printf("  RX-badcrc:               %10"PRIu64"    RX-badlen:
> %10"PRIu64
> > +		       "  RX-errors:  %10"PRIu64"\n",
> > +		       stats.ibadcrc, stats.ibadlen, stats.ierrors);
> > +		printf("  RX-nombuf:               %10"PRIu64"\n",
> > +		       stats.rx_nombuf);
> > +		printf("  TX-packets:              %10"PRIu64"    TX-errors:
> %10"PRIu64
> > +		       "    TX-bytes: %10"PRIu64"\n",
> > +		       stats.opackets, stats.oerrors, stats.obytes);
> > +	}
> 
> Why the formatting is different depending of device-specific stats mapping?

I will update this and remove the deprecated stats from being printed as well.
> 
> > +	if (port->rx_queue_stats_mapping_enabled) {
> > +		printf("\n");
> > +		for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) {
> > +			printf("  Stats reg %2d RX-packets: %10"PRIu64
> > +			       "    RX-errors: %10"PRIu64
> > +			       "    RX-bytes: %10"PRIu64"\n",
> > +			       i, stats.q_ipackets[i], stats.q_errors[i],
> stats.q_ibytes[i]);
> > +		}
> > +	}
> 
> Why restricting stats per queue to devices requiring a mapping?

I will change this
> 
> > -		$(RTE_OUTPUT)/app/dump_cfg --file-prefix=ring_perf ; \
> 
> This should have been removed in previous patch.

I will remove this in the previous patch