Re: [PATCH v6 0/5] Extended xstats API in ethdev library to allow grouping of stats

[Roger B Melton <rmelton@cisco.com>]

On 4/24/17 11:49 AM, Mcnamara, John wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>> Sent: Monday, April 24, 2017 1:41 PM
>> To: Kozak, KubaX <kubax.kozak@intel.com>
>> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>; Van Haaren, Harry
>> <harry.van.haaren@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
>> Piasecki, JacekX <jacekx.piasecki@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v6 0/5] Extended xstats API in ethdev
>> library to allow grouping of stats
>>
>> 24/04/2017 14:32, Olivier Matz:
>>> Hi,
>>>
>> ...
>>> So, I wonder if it wouldn't be more simple to keep the old API intact
>>> (it would avoid unannounced breakage). The new feature can be
>>> implemented in an additional API:
>>>
>>>   rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>>>         uint64_t *values, unsigned int size)
>>> rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat_name *xstats_names, unsigned int size)
>>>
>>> Or:
>>>
>>>   rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat *values, unsigned int size)
>>> rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat_name *xstats_names, unsigned int size)
>>>
>>>   (which would allow to deprecate the old API, but I'm not sure
>>>    we need to)
>>>
>>>
>>> Can we fix that for 17.05?
>> ...
>>
>> Back to the issues, please try to fix it quickly or we should revert it
>> for 17.05-rc3.
> Hi,
>
> We'll submit a patch to change the APIs for rte_eth_xstats_get() and
> rte_eth_xstats_get_names() back to their previous signature, without
> symbol versions, and add the APIs suggested by Olivier.
>
> We'll work on that as soon as possible.
>
> John
> .
>
Hi folks,

I adapted our application to the API changes presented in 17.05-rc2 and 
encountered problem that I don't think was pointed out in Olivier's list 
of issues.

In our application we call rte_eth_stats_get() with NULL pointers to 
determine the number of xstats.  We use the returned count  to 
pre-allocate buffers for the name strings and xstats values and then 
invoke rte_eth_xstats_get_names() and rte_eth_xstats_get() with pointers 
to appropriately sized buffers and the number of entries available in 
those buffers.

Things appeared to work fine until I requested stats for a net_ixgbe_vf 
port. In that case the initial call to rte_eth_xstats_get() did not 
account for the xstats extensions from the ixgbevf PMD.  Long story 
short, my buffers were undersized and bad things happened.

John,

If you intend to restore the previous API signature but retain the -rc2 
internal implementation, may I suggest that both APIs use the same 
internal function to determine stats count.  Otherwise the potential for 
disagreement between the APIs will remain.

Also while on this topic, IMO the above demonstrates that we really need 
an API to query the xstats count rather than relying on 
rte_eth_xstats_get() or rte_eth_xstats_get_names() with 0'd out arguments.

Regards,
Roger