[dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: add sanity checks when retrieving xstats

[Remy Horton]

On 14/06/2018 11:55, Ferruh Yigit wrote:
> On 6/14/2018 7:39 AM, Remy Horton wrote:
>>
>> On 13/06/2018 16:39, Ferruh Yigit wrote:
>>> On 6/7/2018 9:15 AM, David Marchand wrote:
>>>> Testpmd should not expect the xstats names and values arrays to be
>>>> aligned: neither the arrays sizes, nor the order in which the values are.
[..]

> APIs exist for getting subset of values (.._by_id) but they both assume
> requested ids are array index.
> As you said this works fine because of xstats[idx].id==idx

Changing that coincidence into an assumption looks like a bug to me.


> struct rte_eth_xstat_name { char name[]; }
> struct rte_eth_xstat { uint64_t id; uint64_t value; }
>
> These two structs are for basic key-value match.
> But one has the "id" field, but other doesn't. If we use "id" as match, this
> will be the index of xstat_name[]. This is extra complexity, and xstats is
> already unnecessarily complex.
>
> I am for documenting that "xstat_name" and "xstat" are aligned, both in size and
> order, and array indexes are ids, clearly in API doc and continue with existing
> implementation. What do you think?

As long as none of the PMD vendors intend to take advantage of 
xstats[idx].id!=idx (e.g. to allow omitted values) then I'm OK with it.


..Remy