On Wed, Mar 14, 2018 at 05:21:07PM +0000, Ferruh Yigit wrote:
> On 3/12/2018 8:42 AM, Tomasz Duszynski wrote:
> > Add extended statistics implementation.
> >
> > Signed-off-by: Natalie Samsonov <nsams...@marvell.com>
> > Signed-off-by: Tomasz Duszynski <t...@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.
Good point. Since *_by_id() are using xstats_get_names()/xstats_get()
anyway they do not provide a real speedup. I'll drop that in v3.
>
>
> Thanks,
> ferruh
--
- Tomasz Duszyński
