Re: [dpdk-dev] [PATCH v3 01/10] rte: change xstats to use integer ids

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-06-08 12:16, Remy Horton:
> 'noon,
> 
> On 08/06/2016 10:37, Thomas Monjalon wrote:
> > 2016-05-30 11:48, Remy Horton:
> >>   struct rte_eth_xstats {
> >> +	/* FIXME: Remove name[] once remaining drivers converted */
> >>   	char name[RTE_ETH_XSTATS_NAME_SIZE];
> >
> > What is the plan? This field must be deprecated with an attribute.
> > We cannot have 2 different APIs depending of the driver.
> 
> This is where it gets logistically tricky..
> 
> Since there's an API/ABI breakage notice in place on this, my own 
> preference would be to have the entire patchset quashed into a single 
> patch. Problem is that rte/app changes (patches 1 & 7-9) are normally 
> applied via master whereas driver changes (patches 2-6) go in via 
> dpdk-next-net - it is not clear to me how patches should be submitted 
> for this case..

Misunderstanding here. Patches are fine and will be integrated in the
main tree because they are not only some drivers changes.
I was talking about the old API with name in rte_eth_xstats.
I have not seen the patch 9 which removes it.

> >> +int rte_eth_xstats_count(uint8_t port_id);
> >
> > This function is useless because we can have the count with
> > rte_eth_xstats_get(p, NULL, 0)
> > By the way it would be more consistent to have the same behaviour
> > in rte_eth_xstats_names().
> 
> Feedback I got with earlier patches was that a seperate count function 
> was preferable to overloading the fetch function using *data==NULL - is 
> the use of the latter specifically preferred?

I prefer the fetch/NULL style to get a count.
It also handles nicely the fetch error because of a too small buffer.