[dpdk-dev] [PATCH v3 2/7] ixgbe: add functions to get and reset xstats

Tahhan, Maryam maryam.tahhan at intel.com
Sun Jul 5 12:02:40 CEST 2015



> 
> 
> <snip>
> > >
> > >> +
> > >> +	total_missed_rx = 0;
> > >> +	total_qbrc = 0;
> > >> +	total_qprc = 0;
> > >> +	total_qprdc = 0;
> > >> +	rxnfgpc = 0;
> > >> +	txdgpc = 0;
> > >> +	count = 0;
> > >> +
> > >> +	ixgbe_read_stats_registers(hw, hw_stats, &total_missed_rx,
> > &total_qbrc,
> > >> +							   &total_qprc,
> > &rxnfgpc, &txdgpc, &total_qprdc);
> > >> +
> > >> +	if (!xstats)
> > >> +		return 0;
> > >
> > > this cannot happen except if n == 0.
> > > This condition is already tested above, and "count" should be returned.
> > >
> > >> +
> > >> +	/* Error stats */
> > >> +	for (i = 0; i < RTE_NB_XSTATS; i++) {
> > >> +		snprintf(xstats[count].name, sizeof(xstats[count].name),
> > >> +				"%s", rte_ixgbe_stats_strings[i].name);
> > >> +		xstats[count++].value = *(uint64_t *)(((char *)hw_stats) +
> > >> +
> > 	rte_ixgbe_stats_strings[i].offset);
> > >> +	}
> > >> +
> > >> +	return count;
> > >> +}
> > >
> > > Shouldn't it be xstats[i] instead of xstats[count] ?
> > >
> > > Does it work when using "show port in test-pmd"?
> >
> > ok I missed the 'count = 0' above.
> > So why using count instead of i ?
> >
> > Also, I think it would be clearer to use the constant IXGBE_NB_XSTATS
> > instead of count at the beginning of the function.
> >
> >
> 
> Hi Olivier
> 
> Actually, count should not be 0, it should be n, which is the passed in index
> from rte_eth_xstats_get()...
> 
> Because we fill out the generic stats first in rte_eth_xstats_get() then we call
> ixgbe_dev_xstats_get to fill out the rest.
> 
> I will fix this up
Hi Olivier, 

I confused this change with a subsequent patch in the patch set... so yes for this patch you are correct we can just use i... and leave count as 0. 

in a subsequent patch I modify count to start at n, which is a passed in index from in rte_eth_xstats_get()...

so I will change count for i in the loop here. 


> > >
> > >> +
> > >> +static void
> > >> +ixgbe_dev_xstats_reset(struct rte_eth_dev *dev) {
> > >> +	struct ixgbe_hw_stats *stats =
> > >> +			IXGBE_DEV_PRIVATE_TO_STATS(dev->data-
> > >dev_private);
> > >> +
> > >> +	/* HW registers are cleared on read */
> > >> +	ixgbe_dev_xstats_get(dev, NULL, RTE_NB_XSTATS);
> > >> +
> > >> +	/* Reset software totals */
> > >> +	memset(stats, 0, sizeof(*stats)); }
> > >> +
> > >>  static void
> > >>  ixgbevf_dev_stats_get(struct rte_eth_dev *dev, struct
> > >> rte_eth_stats
> > >> *stats)  {
> > >>


More information about the dev mailing list