From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shreyansh Jain <shreyansh.jain@nxp.com>
Subject: Re: [PATCH v4 41/41] net/dpaa: support for extended
	statistics
Date: Wed, 27 Sep 2017 13:56:44 +0530
Message-ID: <92166552-9ab5-db9a-d7f5-8fb2f4a8a3e7@nxp.com>
References: <20170823141213.25476-1-shreyansh.jain@nxp.com>
 <20170909112132.13936-1-shreyansh.jain@nxp.com>
 <20170909112132.13936-42-shreyansh.jain@nxp.com>
 <3332a9da-5e28-260a-68fa-ab665f907403@intel.com>
 <58ffe1c0-308a-f730-a6d1-9bcf8ddb3d57@nxp.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 8bit
Cc: <dev@dpdk.org>, <hemant.agrawal@nxp.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from NAM02-BL2-obe.outbound.protection.outlook.com
 (mail-bl2nam02on0085.outbound.protection.outlook.com [104.47.38.85])
 by dpdk.org (Postfix) with ESMTP id 769EC5599
 for <dev@dpdk.org>; Wed, 27 Sep 2017 10:15:54 +0200 (CEST)
In-Reply-To: <58ffe1c0-308a-f730-a6d1-9bcf8ddb3d57@nxp.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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@nxp.com>
>>>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@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;
>>> +}
>>
>> <...>
>>
> 
>