[dpdk-dev] [RFC PATCH v1 2/3] drivers/net/ixgbe: change xstats to use integers

[David Harton (dharton)]

> -----Original Message-----
> From: Remy Horton [mailto:remy.horton at intel.com]
> Sent: Tuesday, May 03, 2016 8:23 AM
> To: David Harton (dharton) <dharton at cisco.com>; dev at dpdk.org; Helin Zhang
> <helin.zhang at intel.com>
> Subject: Re: [dpdk-dev] [RFC PATCH v1 2/3] drivers/net/ixgbe: change
> xstats to use integers
> 
> 
> On 29/04/2016 14:43, David Harton (dharton) wrote:
> [..]
> >> +		/* RX Priority Stats */
> >> +		for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
> >> +			for (i = 0; i < 8; i++) {
> >
> > 8 seems magical.  Is there a constant somewhere that can be used?
> 
> Not that I'm aware of. I've made it a #define as a small cleanup.
> 
> 
> >> +static int ixgbevf_dev_xstats_names(__rte_unused struct rte_eth_dev
> *dev,
> >> +	struct rte_eth_xstats_name *ptr_names, __rte_unused unsigned limit)
> >> +{
> >> +	unsigned i;
> >> +
> >> +	if (limit < IXGBEVF_NB_XSTATS)
> >
> > Think this check has to be removed since rte_eth_xstats_count() calls
> > with limit == 0.
> 
> Well spotted. It should only error-out if ptr_names is non-NULL.
> 
> As an aside, I am wondering whether getting stats counts should itself be
> a seperate driver hook.

I personally think it would be cleaner.  Bug, I can also see the argument of keeping
the vector table smaller and would conceded to others that felt strongly about it.

I do really like having a separate external API to the user.  A user should explicitly
ask for the count.

Cheers,
Dave

> 
> 
> ..Remy