Re: [dpdk-dev] [PATCH v2 6/8] net/mrvl: add extended statistics

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/12/2018 8:42 AM, Tomasz Duszynski wrote:
> Add extended statistics implementation.
> 
> Signed-off-by: Natalie Samsonov <nsamsono@marvell.com>
> Signed-off-by: Tomasz Duszynski <tdu@semihalf.com>

<...>

> @@ -1674,6 +1784,94 @@ mrvl_eth_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
>  	}
>  }
>  
> +/**
> + * DPDK callback to get xstats by id.
> + *
> + * @param dev
> + *   Pointer to the device structure.
> + * @param ids
> + *   Pointer to the ids table.
> + * @param values
> + *   Pointer to the values table.
> + * @param n
> + *   Values table size.
> + * @returns
> + *   Number of read values, negative value otherwise.
> + */
> +static int
> +mrvl_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
> +		      uint64_t *values, unsigned int n)
> +{
> +	unsigned int i, num = RTE_DIM(mrvl_xstats_tbl);
> +	uint64_t vals[n];
> +	int ret;
> +
> +	if (!ids) {

You will not get NULL ids, this case covered by ethdev layer, for both by_id()
functions.

> +		struct rte_eth_xstat xstats[num];
> +		int j;
> +
> +		ret = mrvl_xstats_get(dev, xstats, num);
> +		for (j = 0; j < ret; i++)
> +			values[j] = xstats[j].value;
> +
> +		return ret;
> +	}
> +
> +	ret = mrvl_xstats_get_by_id(dev, NULL, vals, n);
> +	if (ret < 0)
> +		return ret;
> +
> +	for (i = 0; i < n; i++) {
> +		if (ids[i] >= num) {
> +			RTE_LOG(ERR, PMD, "id value is not valid\n");
> +			return -1;
> +		}
> +
> +		values[i] = vals[ids[i]];
> +	}
> +
> +	return n;
> +}
> +
> +/**
> + * DPDK callback to get xstats names by ids.
> + *
> + * @param dev
> + *   Pointer to the device structure.
> + * @param xstats_names
> + *   Pointer to table with xstats names.
> + * @param ids
> + *   Pointer to table with ids.
> + * @param size
> + *   Xstats names table size.
> + * @returns
> + *   Number of names read, negative value otherwise.
> + */
> +static int
> +mrvl_xstats_get_names_by_id(struct rte_eth_dev *dev,
> +			    struct rte_eth_xstat_name *xstats_names,
> +			    const uint64_t *ids, unsigned int size)
> +{
> +	unsigned int i, num = RTE_DIM(mrvl_xstats_tbl);
> +	struct rte_eth_xstat_name names[num];
> +
> +	if (!ids)
> +		return mrvl_xstats_get_names(dev, xstats_names, size);
> +
> +	mrvl_xstats_get_names(dev, names, size);
> +	for (i = 0; i < size; i++) {
> +		if (ids[i] >= num) {
> +			RTE_LOG(ERR, PMD, "id value is not valid");
> +			return -1;
> +		}
> +
> +		snprintf(xstats_names[i].name, RTE_ETH_XSTATS_NAME_SIZE,
> +			 "%s", names[ids[i]].name);
> +	}
> +
> +	return size;
> +}

Specific to *_by_id() implementations, please check ethdev layer APIs for these
devops, they already do same thing as you did here.

These devops are to access specific ids efficiently with support of PMD, if you
don't have a quick way to access to an extended stat by id, you may just left
these unimplemented and abstraction layer will do the work for you, it is up to you.


Thanks,
ferruh