[dpdk-dev] [PATCH v4 41/41] net/dpaa: support for extended statistics

Shreyansh Jain shreyansh.jain at nxp.com
Wed Sep 27 10:26:44 CEST 2017


On Thursday 21 September 2017 06:56 PM, Shreyansh Jain wrote:
> On Monday 18 September 2017 08:27 PM, Ferruh Yigit wrote:
>> On 9/9/2017 12:21 PM, Shreyansh Jain wrote:
>>> From: Hemant Agrawal <hemant.agrawal at nxp.com>
>>>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>>
>> <...>
>>
>>> +static int
>>> +dpaa_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat 
>>> *xstats,
>>> +            unsigned int n)
>>> +{
>>> +    struct dpaa_if *dpaa_intf = dev->data->dev_private;
>>> +    unsigned int i = 0, num = RTE_DIM(dpaa_xstats_strings);
>>> +    uint64_t values[sizeof(struct dpaa_if_stats) / 8];
>>> +
>>> +    if (xstats == NULL)
>>> +        return 0;
>>
>> This is a little not clear from API definition, but I guess when xstats
>> is NULL, it should return num of available stats, "num" for this case. I
>> guess there are PMDs implements both, can you please double check?
> 
> Ok. I will check again.

I checked a number of other ethev implementations. Some like i40e/e1000 
also return 0 when xstats is NULL. Others, like bnx2x and qede don't 
handle this situation.
All return "num" when passed argument is larger than number of elements 
in the table.

Though, I think the logic that get_xstats should return its size (num) 
when passed with NULL, looks good to me.
How does one standardize such semantics for existing APIs?

(I can add this info to the API document that you created - but only 
once we know if others will agree to change)

> 
>>
>>> +
>>> +    if (n < num)
>>> +        return num;
>>> +
>>> +    fman_if_stats_get_all(dpaa_intf->fif, values,
>>> +                  sizeof(struct dpaa_if_stats) / 8);
>>> +
>>> +    for (i = 0; i < num; i++) {
>>> +        xstats[i].id = i;
>>> +        xstats[i].value = values[dpaa_xstats_strings[i].offset / 8];
>>> +    }
>>> +    return i;
>>> +}
>>
>> <...>
>>
> 
> 



More information about the dev mailing list