[dpdk-dev] [RFC PATCH v1 0/3] Remove string operations from xstats

[Remy Horton]

Morning,

On 28/04/2016 16:58, David Harton (dharton) wrote:
[..]
>>>> *) id-name & id-value pairs for both lookup and query Permits
>>>> out-of-order and non-contigious returning of names/ids/values, even
>>>> though expected implmentations would in practice return items in
>>>> sorted order by id. Is this sufficent/desirable future proofing?
>>>> Idea is to allow possibility of drivers returning partial statistics.
>>>
>>> I believe forcing drivers to match to a common id-space will become
>>> burdensome.  If the stats id-space isn't common then matching strings
>>> is probably just as sufficient as long as drivers don't add/remove
>>> stats ad hoc between the time the device is initialized and removed.
>>
>> I'm not aware of drivers adding/removing the stats ad hoc? The idea is to
>> have a common-id space otherwise it will be a free for all and we won't
>> have alignment across the drivers. I don't see it being any more
>> burdensome than having a common register naming across the board which is
>> what is there today. The advantage being that you don't have to pull the
>> strings every time.

Returning both stats (id,value) and names (id,string) as pairs would 
allow (amoung other things) common ids but actually having common ids is 
not an intended goal of mine. I think the whole idea of common ids was 
implicity vetoed when the idea of ENUMs was thrown out.

I opted for both stats and lookup to be provided as pairs because when 
it comes to APIs, I have a slight preference for having that bit of 
extra generality. Not sure its really worth it, so might change stats to 
just use id-indexed integer arrays (ethtool-like basically) rather than 
a typedef that includes the numeric id.


>>>> *) Bulk name-id mapping lookup only
[..]
>>> I'm not sure I see the value of looking up a single stat from a user
>>> perspective.  I can see where the drivers might say that some stats
>>> are less disruptive/etc but the user doesn't have that knowledge and
>>> wouldn't know how to take advantage.  Usually all stats are grabbed
>>> multiple times and the changes noted during debug sessions.
>>>
>>
>> I believe Remy's change doesn't suggest/support individual lookup. It is
>> just a statement that we don't want to burden drivers with individual
>> stats lookups.

Correct.


>>>> *) Replacement or additional API
[..]
>>> However, if
>>> we want to go forward with cleaning up in order to reduce the support
>>> drivers provide I'm all for it.

Whether we want to do such a cleanup is my open question.


> Maybe I misread the patch series or missed one but I don't see where
> stats can be obtained without copying strings?  This is the real issue I
> raised originally.

See http://dpdk.org/dev/patchwork/patch/12096/ where xstats[].key is 
used to lookup string from ptr_names[].name - I didn't delete the name 
field from rte_eth_xstats because doing so would cause a compile error 
with the drivers I've not yet converted.



> I did not add "get the count" because it wasn't provided in the current API
> and instead followed the convention but I do believe overtly getting the
> count it is the better approach.

I didn't either for the same reason, but if the API is going to be 
broken, I think it should be added.


..Remy