[dpdk-dev,v5,1/3] ethdev: new xstats API add retrieving by ID

Message ID 1491928644-10383-2-git-send-email-michalx.k.jastrzebski@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Michal Jastrzebski April 11, 2017, 4:37 p.m. UTC
  From: Jacek Piasecki <jacekx.piasecki@intel.com>

Extended xstats API in ethdev library to allow grouping of stats
logically so they can be retrieved per logical grouping  managed
by the application.
Changed existing functions rte_eth_xstats_get_names and
rte_eth_xstats_get to use a new list of arguments: array of ids
and array of values. ABI versioning mechanism was used to
support backward compatibility.
Introduced two new functions rte_eth_xstats_get_all and
rte_eth_xstats_get_names_all which keeps functionality of the
previous ones (respectively rte_eth_xstats_get and
rte_eth_xstats_get_names) but use new API inside.
Both functions marked as deprecated.
Introduced new function: rte_eth_xstats_get_id_by_name
to retrieve xstats ids by its names.

test-pmd: add support for new xstats API retrieving by id in
testpmd application: xstats_get() and
xstats_get_names() call with modified parameters.

proc_info: add support for new xstats API retrieving by id to
proc_info application. There is a new argument --xstats-ids
in proc_info command line to retrieve statistics given by ids.
E.g. --xstats-ids="1,3,5,7,8"

doc: add description for modified xstats API
Documentation change for modified extended statistics API functions.
The old API only allows retrieval of *all* of the NIC statistics
at once. Given this requires a MMIO read PCI transaction per statistic
it is an inefficient way of retrieving just a few key statistics.
Often a monitoring agent only has an interest in a few key statistics,
and the old API forces wasting CPU time and PCIe bandwidth in retrieving
*all* statistics; even those that the application didn't explicitly
show an interest in.
The new, more flexible API allow retrieval of statistics per ID.
If a PMD wishes, it can be implemented to read just the required
NIC registers. As a result, the monitoring application no longer wastes
PCIe bandwidth and CPU time.

Signed-off-by: Jacek Piasecki <jacekx.piasecki@intel.com>
Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>
---
 app/proc_info/main.c                    | 150 ++++++++++-
 app/test-pmd/config.c                   |  19 +-
 doc/guides/prog_guide/poll_mode_drv.rst | 174 +++++++++++--
 lib/librte_ether/rte_ethdev.c           | 430 ++++++++++++++++++++++++--------
 lib/librte_ether/rte_ethdev.h           | 170 ++++++++++++-
 lib/librte_ether/rte_ether_version.map  |   5 +
 6 files changed, 794 insertions(+), 154 deletions(-)
  

Comments

Van Haaren, Harry April 12, 2017, 8:56 a.m. UTC | #1
> From: Jastrzebski, MichalX K
> Sent: Tuesday, April 11, 2017 5:37 PM
> To: dev@dpdk.org
> Cc: Jain, Deepak K <deepak.k.jain@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Piasecki, JacekX <jacekx.piasecki@intel.com>; Kozak, KubaX <kubax.kozak@intel.com>; Kulasek,
> TomaszX <tomaszx.kulasek@intel.com>
> Subject: [PATCH v5 1/3] ethdev: new xstats API add retrieving by ID
> 
> From: Jacek Piasecki <jacekx.piasecki@intel.com>
> 
> Extended xstats API in ethdev library to allow grouping of stats
> logically so they can be retrieved per logical grouping  managed
> by the application.
> Changed existing functions rte_eth_xstats_get_names and
> rte_eth_xstats_get to use a new list of arguments: array of ids
> and array of values. ABI versioning mechanism was used to
> support backward compatibility.
> Introduced two new functions rte_eth_xstats_get_all and
> rte_eth_xstats_get_names_all which keeps functionality of the
> previous ones (respectively rte_eth_xstats_get and
> rte_eth_xstats_get_names) but use new API inside.
> Both functions marked as deprecated.
> Introduced new function: rte_eth_xstats_get_id_by_name
> to retrieve xstats ids by its names.
> 
> test-pmd: add support for new xstats API retrieving by id in
> testpmd application: xstats_get() and
> xstats_get_names() call with modified parameters.
> 
> proc_info: add support for new xstats API retrieving by id to
> proc_info application. There is a new argument --xstats-ids
> in proc_info command line to retrieve statistics given by ids.
> E.g. --xstats-ids="1,3,5,7,8"
> 
> doc: add description for modified xstats API
> Documentation change for modified extended statistics API functions.
> The old API only allows retrieval of *all* of the NIC statistics
> at once. Given this requires a MMIO read PCI transaction per statistic
> it is an inefficient way of retrieving just a few key statistics.
> Often a monitoring agent only has an interest in a few key statistics,
> and the old API forces wasting CPU time and PCIe bandwidth in retrieving
> *all* statistics; even those that the application didn't explicitly
> show an interest in.
> The new, more flexible API allow retrieval of statistics per ID.
> If a PMD wishes, it can be implemented to read just the required
> NIC registers. As a result, the monitoring application no longer wastes
> PCIe bandwidth and CPU time.
> 
> Signed-off-by: Jacek Piasecki <jacekx.piasecki@intel.com>
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
> Signed-off-by: Tomasz Kulasek <tomaszx.kulasek@intel.com>


As part of this patchset 3 functions were added to struct eth_dev_ops, to provide more flexible xstats() APIs.

This patchset uses symbol versioning to keep ABI stable. I have checked ABI using ./devtools/validate-abi.sh script for both GCC 5.4.0 and Clang 3.8.0. GCC indicates Compatible, while Clang says there is a single Medium issue, which I believe to be a false positive (details below).

The clang Medium issue is described as follows by the ABI report;
- struct rte_eth_dev :
  Change: Size of field dev_ops has been changed from 624 bytes to 648 bytes [HvH: due to adding 3 xstats function pointers to end of struct] 
  Effect: Previous accesses of applications and library functions to this field and fields at higher positions of the structure definition may be broken. 

The reason I believe this is a false positive is that the "dev_ops" field is defined in the rte_eth_dev struct as follows:
  const struct eth_dev_ops *dev_ops;

Any accesses made to dev_ops will be by this pointer-dereference, so the *size* of dev_ops *in* rte_eth_dev struct is still a pointer - it hasn't changed. Hence "accesses to this field and fields at higher positions of the structure" will not be changed - the pointer in the rte_eth_dev struct remains a pointer.


Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
  
Thomas Monjalon April 12, 2017, 5:10 p.m. UTC | #2
> --- a/app/proc_info/main.c
> +++ b/app/proc_info/main.c
>  		"  --stats: to display port statistics, enabled by default\n"
>  		"  --xstats: to display extended port statistics, disabled by "
>  			"default\n"
> -		"  --metrics: to display derived metrics of the ports, disabled by "
> -			"default\n"
> +		"  --xstats-name NAME: to display single xstat value by NAME\n"
> +		"  --xstats-ids IDLIST: to display xstat values by id. "
> +			"The argument is comma-separated list of xstat ids to print out.\n"
>  		"  --stats-reset: to reset port statistics\n"
>  		"  --xstats-reset: to reset port extended statistics\n"

Why removing --metrics? Is it a rebase mistake?

Please, could you introduce these new proc_info options in a separate patch?

> --- a/doc/guides/prog_guide/poll_mode_drv.rst
> +++ b/doc/guides/prog_guide/poll_mode_drv.rst
> +The extended statistics API allows a PMD to expose all statistics that are
> +available to it, including statistics that are unique to the device.
> +Each statistic has three properties ``name``, ``id`` and ``value``:
> +
> +* ``name``: A human readable string formatted by the scheme detailed below.
> +* ``id``: An integer that represents only that statistic.

Suggestion: we could say that the id/name pair may change from a device
to another one.

> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
>  int
> -rte_eth_xstats_get_names(uint8_t port_id,
> -	struct rte_eth_xstat_name *xstats_names,
> -	unsigned size)
> +rte_eth_xstats_get_id_by_name(uint8_t port_id, const char *xstat_name,
> +		uint64_t *id)
[...]
> +/* retrieve ethdev extended statistics */
> +int
> +rte_eth_xstats_get_v22(uint8_t port_id, struct rte_eth_xstat *xstats,
> +	unsigned int n)

I do not manage to review the function changes because of the mix
of versioning old functions and introduce new ones.
It would have been simpler to separate these two steps in separate patches.

> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> -	eth_xstats_get_names_t     xstats_get_names;
> -	/**< Get names of extended statistics. */
> +	eth_xstats_get_names_t	   xstats_get_names;
> +	/**< Get names of extended device statistics. */

It seems a space alignment has been removed here.

> - * Retrieve names of extended statistics of an Ethernet device.
> +* Gets the ID of a statistic from its name.
> +*
> +* Note this function searches for the statistics using string compares, and
> +* as such should not be used on the fast-path. For fast-path retrieval of
> +* specific statistics, store the ID as provided in *id* from this function,
> +* and pass the ID to rte_eth_xstats_get()

Good to note :)

> -int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
> -		unsigned n);
[...]
> +int rte_eth_xstats_get_v1705(uint8_t port_id, uint64_t *ids, uint64_t *values,
> +	unsigned int n);
> +
> +int rte_eth_xstats_get(uint8_t port_id, uint64_t *ids, uint64_t *values,
> +	unsigned int n);

It is an API change.
Please reference it in the release notes.

> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -159,5 +159,10 @@ DPDK_17.05 {
>  	global:
>  
>  	rte_eth_find_next;
> +	rte_eth_xstats_get_names;
> +	rte_eth_xstats_get;
> +	rte_eth_xstats_get_all;
> +	rte_eth_xstats_get_names_all;
> +	rte_eth_xstats_get_id_by_name;

Please keep alphabetical order.


Conclusion: The API looks right.
I have not checked the implementation and hope there will be less bugs
than when introducing the old one ;)
  
John McNamara April 13, 2017, 10:52 a.m. UTC | #3
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Michal Jastrzebski
> Sent: Tuesday, April 11, 2017 5:37 PM
> To: dev@dpdk.org
> Cc: Jain, Deepak K <deepak.k.jain@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Piasecki, JacekX
> <jacekx.piasecki@intel.com>; Kozak, KubaX <kubax.kozak@intel.com>;
> Kulasek, TomaszX <tomaszx.kulasek@intel.com>
> Subject: [dpdk-dev] [PATCH v5 1/3] ethdev: new xstats API add retrieving
> by ID
> 
> From: Jacek Piasecki <jacekx.piasecki@intel.com>
> 
> ...
>
> +Scheme for Human Readable Names
> +_______________________________
> 
>  A naming scheme exists for the strings exposed to clients of the API.
> This is  to allow scraping of the API for statistics of interest. The
> naming scheme uses @@ -363,8 +359,8 @@ strings split by a single


Minor request, if you are updating this patchset could you use a different
title underline than ____.

The guidelines suggest:

    Level 1 Heading
    ===============


    Level 2 Heading
    ---------------


    Level 3 Heading
    ~~~~~~~~~~~~~~~


    Level 4 Heading
    ^^^^^^^^^^^^^^^

http://dpdk.org/doc/guides/contributing/documentation.html#section-headers


So in this case it should probably be ^^^^^^^^^^.

However, that is fairly minor and it is okay as is if you don't get to it.

John
  
Kuba Kozak April 13, 2017, 2:59 p.m. UTC | #4
Extended xstats API in ethdev library to allow grouping of stats logically
so they can be retrieved per logical grouping  managed by the application.
Changed existing functions rte_eth_xstats_get_names and rte_eth_xstats_get
to use a new list of arguments: array of ids and array of values.
ABI versioning mechanism was used to support backward compatibility.
Introduced two new functions rte_eth_xstats_get_all and
rte_eth_xstats_get_names_all which keeps functionality of the previous
ones (respectively rte_eth_xstats_get and rte_eth_xstats_get_names)
but use new API inside. Both functions marked as deprecated. 
Introduced new function: rte_eth_xstats_get_id_by_name to retrieve
xstats ids by its names.
Extended functionality of proc_info application:
--xstats-name NAME: to display single xstat value by NAME
Updated test-pmd application to use new API.

v6 changes:
* patches arrangement in patchset
* fixes spelling bugs
* release notes

v5 changes:
* fix clang shared build compilation
* remove wrong versioning macros
* Makefile LIBABIVER 6 change 

v4 changes:
* documentation change after API modification
* fix xstats display for PMD without _by_ids() functions
* fix ABI validator errors

v3 changes:
* checkpatch fixes
* removed malloc bug in ethdev
* add new command to proc_info and IDs parsing
* merged testpmd and proc_info patch with library patch

Jacek Piasecki (3):
  ethdev: new xstats API add retrieving by ID
  net/e1000: new xstats API add ID support for e1000
  net/ixgbe: new xstats API add ID support for ixgbe

Kuba Kozak (2):
  ethdev: added new function for xstats ID
  proc-info: add support for new xstats API

 app/proc_info/main.c                    | 148 ++++++++++-
 app/test-pmd/config.c                   |  19 +-
 doc/guides/prog_guide/poll_mode_drv.rst | 173 +++++++++++--
 doc/guides/rel_notes/release_17_05.rst  |   8 +
 drivers/net/e1000/igb_ethdev.c          |  92 ++++++-
 drivers/net/ixgbe/ixgbe_ethdev.c        | 179 +++++++++++++
 lib/librte_ether/rte_ethdev.c           | 430 ++++++++++++++++++++++++--------
 lib/librte_ether/rte_ethdev.h           | 167 ++++++++++++-
 lib/librte_ether/rte_ether_version.map  |   5 +
 9 files changed, 1070 insertions(+), 151 deletions(-)
  
Van Haaren, Harry April 13, 2017, 4:21 p.m. UTC | #5
> From: Kozak, KubaX
> Sent: Thursday, April 13, 2017 3:59 PM
> To: dev@dpdk.org
> Cc: Van Haaren, Harry <harry.van.haaren@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
> Kozak, KubaX <kubax.kozak@intel.com>
> Subject: [PATCH v6 0/5] Extended xstats API in ethdev library to allow grouping of stats
> 
> Extended xstats API in ethdev library to allow grouping of stats logically
> so they can be retrieved per logical grouping  managed by the application.
> Changed existing functions rte_eth_xstats_get_names and rte_eth_xstats_get
> to use a new list of arguments: array of ids and array of values.
> ABI versioning mechanism was used to support backward compatibility.
> Introduced two new functions rte_eth_xstats_get_all and
> rte_eth_xstats_get_names_all which keeps functionality of the previous
> ones (respectively rte_eth_xstats_get and rte_eth_xstats_get_names)
> but use new API inside. Both functions marked as deprecated.
> Introduced new function: rte_eth_xstats_get_id_by_name to retrieve
> xstats ids by its names.
> Extended functionality of proc_info application:
> --xstats-name NAME: to display single xstat value by NAME
> Updated test-pmd application to use new API.
> 
> v6 changes:
> * patches arrangement in patchset
> * fixes spelling bugs
> * release notes
> 
> v5 changes:
> * fix clang shared build compilation
> * remove wrong versioning macros
> * Makefile LIBABIVER 6 change
> 
> v4 changes:
> * documentation change after API modification
> * fix xstats display for PMD without _by_ids() functions
> * fix ABI validator errors
> 
> v3 changes:
> * checkpatch fixes
> * removed malloc bug in ethdev
> * add new command to proc_info and IDs parsing
> * merged testpmd and proc_info patch with library patch

Please keep Acks when only re-arranging patches and changes.

I'll re-Ack the patches on patchwork now :)

Thanks, -Harry
  
Thomas Monjalon April 20, 2017, 8:31 p.m. UTC | #6
13/04/2017 16:59, Kuba Kozak:
> Extended xstats API in ethdev library to allow grouping of stats logically
> so they can be retrieved per logical grouping  managed by the application.
> Changed existing functions rte_eth_xstats_get_names and rte_eth_xstats_get
> to use a new list of arguments: array of ids and array of values.
> ABI versioning mechanism was used to support backward compatibility.
> Introduced two new functions rte_eth_xstats_get_all and
> rte_eth_xstats_get_names_all which keeps functionality of the previous
> ones (respectively rte_eth_xstats_get and rte_eth_xstats_get_names)
> but use new API inside. Both functions marked as deprecated.
> Introduced new function: rte_eth_xstats_get_id_by_name to retrieve
> xstats ids by its names.
> Extended functionality of proc_info application:
> --xstats-name NAME: to display single xstat value by NAME
> Updated test-pmd application to use new API.

Applied, thanks
  
Olivier Matz April 24, 2017, 12:32 p.m. UTC | #7
Hi,

On Thu, 20 Apr 2017 22:31:35 +0200, Thomas Monjalon <thomas@monjalon.net> wrote:
> 13/04/2017 16:59, Kuba Kozak:
> > Extended xstats API in ethdev library to allow grouping of stats logically
> > so they can be retrieved per logical grouping  managed by the application.
> > Changed existing functions rte_eth_xstats_get_names and rte_eth_xstats_get
> > to use a new list of arguments: array of ids and array of values.
> > ABI versioning mechanism was used to support backward compatibility.
> > Introduced two new functions rte_eth_xstats_get_all and
> > rte_eth_xstats_get_names_all which keeps functionality of the previous
> > ones (respectively rte_eth_xstats_get and rte_eth_xstats_get_names)
> > but use new API inside. Both functions marked as deprecated.
> > Introduced new function: rte_eth_xstats_get_id_by_name to retrieve
> > xstats ids by its names.
> > Extended functionality of proc_info application:
> > --xstats-name NAME: to display single xstat value by NAME
> > Updated test-pmd application to use new API.  
> 
> Applied, thanks

I'm adapting my application to the upcoming dpdk 17.05. I see
several problems with this patchset:

- the API of rte_eth_xstats_get() and rte_eth_xstats_get_names()
  has been modified, and from what I see it was not announced.
  It looks that ABI is preserved however.

- the new functions rte_eth_xstats_get_all() and
  rte_eth_xstats_get_names_all() are marked as deprecated, which
  looks strange for new functions.

About the new api:

int rte_eth_xstats_get(uint8_t port_id, uint64_t *ids, uint64_t *values,
       unsigned int n);

int rte_eth_xstats_get_names(uint8_t port_id,
       struct rte_eth_xstat_name *xstats_names, unsigned int size,
       uint64_t *ids);

- the argument "id" is not at the same place

- why having "size" in one function and "n" in the second (it was
  renamed in the patch)?

- the argument "id" should be const

- a table of uint64_t is returned in place of the struct rte_eth_xstat
  table: if no ids are given, the driver cannot return partial or
  disordered stats anymore. See
  513c78ae3fd6 ("ethdev: fix extended statistics name index")


So, I wonder if it wouldn't be more simple to keep the old
API intact (it would avoid unannounced breakage). The new feature
can be implemented in an additional API:

 rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
       uint64_t *values, unsigned int size)
 rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
       struct rte_eth_xstat_name *xstats_names, unsigned int size)

Or:

 rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
       struct rte_eth_xstat *values, unsigned int size)
 rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
       struct rte_eth_xstat_name *xstats_names, unsigned int size)

 (which would allow to deprecate the old API, but I'm not sure
  we need to)


Can we fix that for 17.05?

Regards,
Olivier
  
Thomas Monjalon April 24, 2017, 12:41 p.m. UTC | #8
24/04/2017 14:32, Olivier Matz:
> Hi,
> 
> On Thu, 20 Apr 2017 22:31:35 +0200, Thomas Monjalon <thomas@monjalon.net> 
wrote:
> > 13/04/2017 16:59, Kuba Kozak:
> > > Extended xstats API in ethdev library to allow grouping of stats
> > > logically
> > > so they can be retrieved per logical grouping  managed by the
> > > application.
> > > Changed existing functions rte_eth_xstats_get_names and
> > > rte_eth_xstats_get
> > > to use a new list of arguments: array of ids and array of values.
> > > ABI versioning mechanism was used to support backward compatibility.
> > > Introduced two new functions rte_eth_xstats_get_all and
> > > rte_eth_xstats_get_names_all which keeps functionality of the previous
> > > ones (respectively rte_eth_xstats_get and rte_eth_xstats_get_names)
> > > but use new API inside. Both functions marked as deprecated.
> > > Introduced new function: rte_eth_xstats_get_id_by_name to retrieve
> > > xstats ids by its names.
> > > Extended functionality of proc_info application:
> > > --xstats-name NAME: to display single xstat value by NAME
> > > Updated test-pmd application to use new API.
> > 
> > Applied, thanks
> 
> I'm adapting my application to the upcoming dpdk 17.05. I see
> several problems with this patchset:
> 
> - the API of rte_eth_xstats_get() and rte_eth_xstats_get_names()
>   has been modified, and from what I see it was not announced.
>   It looks that ABI is preserved however.
> 
> - the new functions rte_eth_xstats_get_all() and
>   rte_eth_xstats_get_names_all() are marked as deprecated, which
>   looks strange for new functions.
> 
> About the new api:
> 
> int rte_eth_xstats_get(uint8_t port_id, uint64_t *ids, uint64_t *values,
>        unsigned int n);
> 
> int rte_eth_xstats_get_names(uint8_t port_id,
>        struct rte_eth_xstat_name *xstats_names, unsigned int size,
>        uint64_t *ids);
> 
> - the argument "id" is not at the same place
> 
> - why having "size" in one function and "n" in the second (it was
>   renamed in the patch)?
> 
> - the argument "id" should be const
> 
> - a table of uint64_t is returned in place of the struct rte_eth_xstat
>   table: if no ids are given, the driver cannot return partial or
>   disordered stats anymore. See
>   513c78ae3fd6 ("ethdev: fix extended statistics name index")
> 
> 
> So, I wonder if it wouldn't be more simple to keep the old
> API intact (it would avoid unannounced breakage). The new feature
> can be implemented in an additional API:
> 
>  rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>        uint64_t *values, unsigned int size)
>  rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>        struct rte_eth_xstat_name *xstats_names, unsigned int size)
> 
> Or:
> 
>  rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>        struct rte_eth_xstat *values, unsigned int size)
>  rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>        struct rte_eth_xstat_name *xstats_names, unsigned int size)
> 
>  (which would allow to deprecate the old API, but I'm not sure
>   we need to)
> 
> 
> Can we fix that for 17.05?

Bottom line is that I have skipped the complete review of this patchset
because it was not properly split, preventing a good review.
Lesson learned: I won't accept anymore a patchset which is not split
enough to allow a proper overview.

Back to the issues, please try to fix it quickly or we should revert
it for 17.05-rc3.
  
John McNamara April 24, 2017, 3:49 p.m. UTC | #9
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, April 24, 2017 1:41 PM
> To: Kozak, KubaX <kubax.kozak@intel.com>
> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
> Piasecki, JacekX <jacekx.piasecki@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v6 0/5] Extended xstats API in ethdev
> library to allow grouping of stats
> 
> 24/04/2017 14:32, Olivier Matz:
> > Hi,
> >
> ...
> > So, I wonder if it wouldn't be more simple to keep the old API intact
> > (it would avoid unannounced breakage). The new feature can be
> > implemented in an additional API:
> >
> >  rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
> >        uint64_t *values, unsigned int size)
> > rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
> >        struct rte_eth_xstat_name *xstats_names, unsigned int size)
> >
> > Or:
> >
> >  rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
> >        struct rte_eth_xstat *values, unsigned int size)
> > rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
> >        struct rte_eth_xstat_name *xstats_names, unsigned int size)
> >
> >  (which would allow to deprecate the old API, but I'm not sure
> >   we need to)
> >
> >
> > Can we fix that for 17.05?
> 
> ...
> 
> Back to the issues, please try to fix it quickly or we should revert it
> for 17.05-rc3.

Hi,

We'll submit a patch to change the APIs for rte_eth_xstats_get() and 
rte_eth_xstats_get_names() back to their previous signature, without
symbol versions, and add the APIs suggested by Olivier.

We'll work on that as soon as possible.

John
  
Roger Melton April 25, 2017, 10:49 p.m. UTC | #10
On 4/24/17 11:49 AM, Mcnamara, John wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>> Sent: Monday, April 24, 2017 1:41 PM
>> To: Kozak, KubaX <kubax.kozak@intel.com>
>> Cc: dev@dpdk.org; Olivier Matz <olivier.matz@6wind.com>; Van Haaren, Harry
>> <harry.van.haaren@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>;
>> Piasecki, JacekX <jacekx.piasecki@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v6 0/5] Extended xstats API in ethdev
>> library to allow grouping of stats
>>
>> 24/04/2017 14:32, Olivier Matz:
>>> Hi,
>>>
>> ...
>>> So, I wonder if it wouldn't be more simple to keep the old API intact
>>> (it would avoid unannounced breakage). The new feature can be
>>> implemented in an additional API:
>>>
>>>   rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>>>         uint64_t *values, unsigned int size)
>>> rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat_name *xstats_names, unsigned int size)
>>>
>>> Or:
>>>
>>>   rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat *values, unsigned int size)
>>> rte_eth_xstats_get_names_by_id(uint8_t port_id, const uint64_t *ids,
>>>         struct rte_eth_xstat_name *xstats_names, unsigned int size)
>>>
>>>   (which would allow to deprecate the old API, but I'm not sure
>>>    we need to)
>>>
>>>
>>> Can we fix that for 17.05?
>> ...
>>
>> Back to the issues, please try to fix it quickly or we should revert it
>> for 17.05-rc3.
> Hi,
>
> We'll submit a patch to change the APIs for rte_eth_xstats_get() and
> rte_eth_xstats_get_names() back to their previous signature, without
> symbol versions, and add the APIs suggested by Olivier.
>
> We'll work on that as soon as possible.
>
> John
> .
>
Hi folks,

I adapted our application to the API changes presented in 17.05-rc2 and 
encountered problem that I don't think was pointed out in Olivier's list 
of issues.

In our application we call rte_eth_stats_get() with NULL pointers to 
determine the number of xstats.  We use the returned count  to 
pre-allocate buffers for the name strings and xstats values and then 
invoke rte_eth_xstats_get_names() and rte_eth_xstats_get() with pointers 
to appropriately sized buffers and the number of entries available in 
those buffers.

Things appeared to work fine until I requested stats for a net_ixgbe_vf 
port. In that case the initial call to rte_eth_xstats_get() did not 
account for the xstats extensions from the ixgbevf PMD.  Long story 
short, my buffers were undersized and bad things happened.

John,

If you intend to restore the previous API signature but retain the -rc2 
internal implementation, may I suggest that both APIs use the same 
internal function to determine stats count.  Otherwise the potential for 
disagreement between the APIs will remain.

Also while on this topic, IMO the above demonstrates that we really need 
an API to query the xstats count rather than relying on 
rte_eth_xstats_get() or rte_eth_xstats_get_names() with 0'd out arguments.

Regards,
Roger
  

Patch

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index d576b42..d0eae4b 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -86,6 +86,14 @@ 
 static uint32_t reset_xstats;
 /**< Enable memory info. */
 static uint32_t mem_info;
+/**< Enable displaying xstat name. */
+static uint32_t enable_xstats_name;
+static char *xstats_name;
+
+/**< Enable xstats by ids. */
+#define MAX_NB_XSTATS_IDS 1024
+static uint32_t nb_xstats_ids;
+static uint64_t xstats_ids[MAX_NB_XSTATS_IDS];
 
 /**< display usage */
 static void
@@ -97,8 +105,9 @@ 
 		"  --stats: to display port statistics, enabled by default\n"
 		"  --xstats: to display extended port statistics, disabled by "
 			"default\n"
-		"  --metrics: to display derived metrics of the ports, disabled by "
-			"default\n"
+		"  --xstats-name NAME: to display single xstat value by NAME\n"
+		"  --xstats-ids IDLIST: to display xstat values by id. "
+			"The argument is comma-separated list of xstat ids to print out.\n"
 		"  --stats-reset: to reset port statistics\n"
 		"  --xstats-reset: to reset port extended statistics\n"
 		"  --collectd-format: to print statistics to STDOUT in expected by collectd format\n"
@@ -132,6 +141,33 @@ 
 
 }
 
+/*
+ * Parse ids value list into array
+ */
+static int
+parse_xstats_ids(char *list, uint64_t *ids, int limit) {
+	int length;
+	char *token;
+	char *ctx = NULL;
+	char *endptr;
+
+	length = 0;
+	token = strtok_r(list, ",", &ctx);
+	while (token != NULL) {
+		ids[length] = strtoull(token, &endptr, 10);
+		if (*endptr != '\0')
+			return -EINVAL;
+
+		length++;
+		if (length >= limit)
+			return -E2BIG;
+
+		token = strtok_r(NULL, ",", &ctx);
+	}
+
+	return length;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -178,7 +214,9 @@ 
 		{"xstats", 0, NULL, 0},
 		{"metrics", 0, NULL, 0},
 		{"xstats-reset", 0, NULL, 0},
+		{"xstats-name", required_argument, NULL, 1},
 		{"collectd-format", 0, NULL, 0},
+		{"xstats-ids", 1, NULL, 1},
 		{"host-id", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
@@ -224,7 +262,28 @@ 
 					MAX_LONG_OPT_SZ))
 				reset_xstats = 1;
 			break;
+		case 1:
+			/* Print xstat single value given by name*/
+			if (!strncmp(long_option[option_index].name,
+					"xstats-name", MAX_LONG_OPT_SZ)) {
+				enable_xstats_name = 1;
+				xstats_name = optarg;
+				printf("name:%s:%s\n",
+						long_option[option_index].name,
+						optarg);
+			} else if (!strncmp(long_option[option_index].name,
+					"xstats-ids",
+					MAX_LONG_OPT_SZ))	{
+				nb_xstats_ids = parse_xstats_ids(optarg,
+						xstats_ids, MAX_NB_XSTATS_IDS);
+
+				if (nb_xstats_ids <= 0) {
+					printf("xstats-id list parse error.\n");
+					return -1;
+				}
 
+			}
+			break;
 		default:
 			proc_info_usage(prgname);
 			return -1;
@@ -351,20 +410,82 @@  static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
 }
 
 static void
+nic_xstats_by_name_display(uint8_t port_id, char *name)
+{
+	uint64_t id;
+
+	printf("###### NIC statistics for port %-2d, statistic name '%s':\n",
+			   port_id, name);
+
+	if (rte_eth_xstats_get_id_by_name(port_id, name, &id) == 0)
+		printf("%s: %"PRIu64"\n", name, id);
+	else
+		printf("Statistic not found...\n");
+
+}
+
+static void
+nic_xstats_by_ids_display(uint8_t port_id, uint64_t *ids, int len)
+{
+	struct rte_eth_xstat_name *xstats_names;
+	uint64_t *values;
+	int ret, i;
+	static const char *nic_stats_border = "########################";
+
+	values = malloc(sizeof(values) * len);
+	if (values == NULL) {
+		printf("Cannot allocate memory for xstats\n");
+		return;
+	}
+
+	xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * len);
+	if (xstats_names == NULL) {
+		printf("Cannot allocate memory for xstat names\n");
+		free(values);
+		return;
+	}
+
+	if (len != rte_eth_xstats_get_names(
+			port_id, xstats_names, len, ids)) {
+		printf("Cannot get xstat names\n");
+		goto err;
+	}
+
+	printf("###### NIC extended statistics for port %-2d #########\n",
+			   port_id);
+	printf("%s############################\n", nic_stats_border);
+	ret = rte_eth_xstats_get(port_id, ids, values, len);
+	if (ret < 0 || ret > len) {
+		printf("Cannot get xstats\n");
+		goto err;
+	}
+
+	for (i = 0; i < len; i++)
+		printf("%s: %"PRIu64"\n",
+			xstats_names[i].name,
+			values[i]);
+
+	printf("%s############################\n", nic_stats_border);
+err:
+	free(values);
+	free(xstats_names);
+}
+
+static void
 nic_xstats_display(uint8_t port_id)
 {
 	struct rte_eth_xstat_name *xstats_names;
-	struct rte_eth_xstat *xstats;
+	uint64_t *values;
 	int len, ret, i;
 	static const char *nic_stats_border = "########################";
 
-	len = rte_eth_xstats_get_names(port_id, NULL, 0);
+	len = rte_eth_xstats_get_names(port_id, NULL, 0, NULL);
 	if (len < 0) {
 		printf("Cannot get xstats count\n");
 		return;
 	}
-	xstats = malloc(sizeof(xstats[0]) * len);
-	if (xstats == NULL) {
+	values = malloc(sizeof(values) * len);
+	if (values == NULL) {
 		printf("Cannot allocate memory for xstats\n");
 		return;
 	}
@@ -372,11 +493,11 @@  static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
 	xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * len);
 	if (xstats_names == NULL) {
 		printf("Cannot allocate memory for xstat names\n");
-		free(xstats);
+		free(values);
 		return;
 	}
 	if (len != rte_eth_xstats_get_names(
-			port_id, xstats_names, len)) {
+			port_id, xstats_names, len, NULL)) {
 		printf("Cannot get xstat names\n");
 		goto err;
 	}
@@ -385,7 +506,7 @@  static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
 			   port_id);
 	printf("%s############################\n",
 			   nic_stats_border);
-	ret = rte_eth_xstats_get(port_id, xstats, len);
+	ret = rte_eth_xstats_get(port_id, NULL, values, len);
 	if (ret < 0 || ret > len) {
 		printf("Cannot get xstats\n");
 		goto err;
@@ -401,18 +522,18 @@  static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
 						  xstats_names[i].name);
 			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
 				PRIu64"\n", host_id, port_id, counter_type,
-				xstats_names[i].name, xstats[i].value);
+				xstats_names[i].name, values[i]);
 			write(stdout_fd, buf, strlen(buf));
 		} else {
 			printf("%s: %"PRIu64"\n", xstats_names[i].name,
-			       xstats[i].value);
+					values[i]);
 		}
 	}
 
 	printf("%s############################\n",
 			   nic_stats_border);
 err:
-	free(xstats);
+	free(values);
 	free(xstats_names);
 }
 
@@ -551,6 +672,11 @@  static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
 				nic_stats_clear(i);
 			else if (reset_xstats)
 				nic_xstats_clear(i);
+			else if (enable_xstats_name)
+				nic_xstats_by_name_display(i, xstats_name);
+			else if (nb_xstats_ids > 0)
+				nic_xstats_by_ids_display(i, xstats_ids,
+						nb_xstats_ids);
 			else if (enable_metrics)
 				metrics_display(i);
 		}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 4d873cd..ef07925 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -264,9 +264,9 @@  struct rss_type_info {
 void
 nic_xstats_display(portid_t port_id)
 {
-	struct rte_eth_xstat *xstats;
 	int cnt_xstats, idx_xstat;
 	struct rte_eth_xstat_name *xstats_names;
+	uint64_t *values;
 
 	printf("###### NIC extended statistics for port %-2d\n", port_id);
 	if (!rte_eth_dev_is_valid_port(port_id)) {
@@ -275,7 +275,7 @@  struct rss_type_info {
 	}
 
 	/* Get count */
-	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0);
+	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0, NULL);
 	if (cnt_xstats  < 0) {
 		printf("Error: Cannot get count of xstats\n");
 		return;
@@ -288,23 +288,24 @@  struct rss_type_info {
 		return;
 	}
 	if (cnt_xstats != rte_eth_xstats_get_names(
-			port_id, xstats_names, cnt_xstats)) {
+			port_id, xstats_names, cnt_xstats, NULL)) {
 		printf("Error: Cannot get xstats lookup\n");
 		free(xstats_names);
 		return;
 	}
 
 	/* Get stats themselves */
-	xstats = malloc(sizeof(struct rte_eth_xstat) * cnt_xstats);
-	if (xstats == NULL) {
+	values = malloc(sizeof(values) * cnt_xstats);
+	if (values == NULL) {
 		printf("Cannot allocate memory for xstats\n");
 		free(xstats_names);
 		return;
 	}
-	if (cnt_xstats != rte_eth_xstats_get(port_id, xstats, cnt_xstats)) {
+	if (cnt_xstats != rte_eth_xstats_get(port_id, NULL, values,
+			cnt_xstats)) {
 		printf("Error: Unable to get xstats\n");
 		free(xstats_names);
-		free(xstats);
+		free(values);
 		return;
 	}
 
@@ -312,9 +313,9 @@  struct rss_type_info {
 	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
 		printf("%s: %"PRIu64"\n",
 			xstats_names[idx_xstat].name,
-			xstats[idx_xstat].value);
+			values[idx_xstat]);
 	free(xstats_names);
-	free(xstats);
+	free(values);
 }
 
 void
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index e48c121..3372f30 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -334,24 +334,20 @@  The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK
 Extended Statistics API
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-The extended statistics API allows each individual PMD to expose a unique set
-of statistics. Accessing these from application programs is done via two
-functions:
-
-* ``rte_eth_xstats_get``: Fills in an array of ``struct rte_eth_xstat``
-  with extended statistics.
-* ``rte_eth_xstats_get_names``: Fills in an array of
-  ``struct rte_eth_xstat_name`` with extended statistic name lookup
-  information.
-
-Each ``struct rte_eth_xstat`` contains an identifier and value pair, and
-each ``struct rte_eth_xstat_name`` contains a string. Each identifier
-within the ``struct rte_eth_xstat`` lookup array must have a corresponding
-entry in the ``struct rte_eth_xstat_name`` lookup array. Within the latter
-the index of the entry is the identifier the string is associated with.
-These identifiers, as well as the number of extended statistic exposed, must
-remain constant during runtime. Note that extended statistic identifiers are
-driver-specific, and hence might not be the same for different ports.
+The extended statistics API allows a PMD to expose all statistics that are
+available to it, including statistics that are unique to the device.
+Each statistic has three properties ``name``, ``id`` and ``value``:
+
+* ``name``: A human readable string formatted by the scheme detailed below.
+* ``id``: An integer that represents only that statistic.
+* ``value``: A unsigned 64-bit integer that is the value of the statistic.
+
+The API consists of various ``rte_eth_xstats_*()`` functions, and allows an
+application to be flexible in how it retrieves statistics.
+
+
+Scheme for Human Readable Names
+_______________________________
 
 A naming scheme exists for the strings exposed to clients of the API. This is
 to allow scraping of the API for statistics of interest. The naming scheme uses
@@ -363,8 +359,8 @@  strings split by a single underscore ``_``. The scheme is as follows:
 * detail n
 * unit
 
-Examples of common statistics xstats strings, formatted to comply to the scheme
-proposed above:
+Examples of common statistics xstats strings, formatted to comply to the
+above scheme:
 
 * ``rx_bytes``
 * ``rx_crc_errors``
@@ -378,7 +374,7 @@  associated with the receive side of the NIC.  The second component ``packets``
 indicates that the unit of measure is packets.
 
 A more complicated example: ``tx_size_128_to_255_packets``. In this example,
-``tx`` indicates transmission, ``size``  is the first detail, ``128`` etc are
+``tx`` indicates transmission, ``size``  is the first detail, ``128`` etc. are
 more details, and ``packets`` indicates that this is a packet counter.
 
 Some additions in the metadata scheme are as follows:
@@ -392,3 +388,139 @@  Some additions in the metadata scheme are as follows:
 An example where queue numbers are used is as follows: ``tx_q7_bytes`` which
 indicates this statistic applies to queue number 7, and represents the number
 of transmitted bytes on that queue.
+
+API Design
+__________
+
+The xstats API uses the ``name``, ``id``, and ``value`` to allow performant
+lookup of specific statistics. Performant lookup means two things;
+
+* No string comparisons with the ``name`` of the statistic in fast-path
+* Allow requesting of only the statistics of interest
+
+The API ensures these requirements are met by mapping the ``name`` of the
+statistic to a unique ``id``, which is used as a key for lookup in the fast-path.
+The API allows applications to request an array of ``id`` values, so that the
+PMD only performs the required calculations. Expected usage is that the
+application scans the ``name`` of each statistic, and caches the ``id``
+if it has an interest in that statistic. On the fast-path, the integer can be used
+to retrieve the actual ``value`` of the statistic that the ``id`` represents.
+
+API Functions
+_____________
+
+The API is built out of a small number of functions, which can be used to
+retrieve the number of statistics and the names, IDs and values of those
+statistics.
+
+* ``rte_eth_xstats_get_names()``: returns the names of the statistics. When given a
+  ``NULL`` parameter the function returns the number of statistics that are available.
+
+* ``rte_eth_xstats_get_id_by_name()``: Searches for the statistic ID that matches
+  ``xstat_name``. If found, the ``id`` integer is set.
+
+* ``rte_eth_xstats_get()``: Fills in an array of ``uint64_t`` values
+  with matching the provided ``ids`` array. If the ``ids`` array is NULL, it
+  returns all statistics that are available.
+
+
+Application Usage
+_________________
+
+Imagine an application that wants to view the dropped packet count. If no
+packets are dropped, the application does not read any other metrics for
+performance reasons. If packets are dropped, the application has a particular
+set of statistics that it requests. This "set" of statistics allows the app to
+decide what next steps to perform. The following code-snippets show how the
+xstats API can be used to achieve this goal.
+
+First step is to get all statistics names and list them:
+
+.. code-block:: c
+
+    struct rte_eth_xstat_name *xstats_names;
+    uint64_t *values;
+    int len, i;
+
+    /* Get number of stats */
+    len = rte_eth_xstats_get_names(port_id, NULL, NULL, 0);
+    if (len < 0) {
+        printf("Cannot get xstats count\n");
+        goto err;
+    }
+
+    xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * len);
+    if (xstats_names == NULL) {
+        printf("Cannot allocate memory for xstat names\n");
+        goto err;
+    }
+
+    /* Retrieve xstats names, passing NULL for IDs to return all statistics */
+    if (len != rte_eth_xstats_get_names(port_id, xstats_names, NULL, len)) {
+        printf("Cannot get xstat names\n");
+        goto err;
+    }
+
+    values = malloc(sizeof(values) * len);
+    if (values == NULL) {
+        printf("Cannot allocate memory for xstats\n");
+        goto err;
+    }
+
+    /* Getting xstats values */
+    if (len != rte_eth_xstats_get(port_id, NULL, values, len)) {
+        printf("Cannot get xstat values\n");
+        goto err;
+    }
+
+    /* Print all xstats names and values */
+    for (i = 0; i < len; i++) {
+        printf("%s: %"PRIu64"\n", xstats_names[i].name, values[i]);
+    }
+
+The application has access to the names of all of the statistics that the PMD
+exposes. The application can decide which statistics are of interest, cache the
+ids of those statistics by looking up the name as follows:
+
+.. code-block:: c
+
+    uint64_t id;
+    uint64_t value;
+    const char *xstat_name = "rx_errors";
+
+    if(!rte_eth_xstats_get_id_by_name(port_id, xstat_name, &id)) {
+        rte_eth_xstats_get(port_id, &id, &value, 1);
+        printf("%s: %"PRIu64"\n", xstat_name, value);
+    }
+    else {
+        printf("Cannot find xstats with a given name\n");
+        goto err;
+    }
+
+The API provides flexibility to the application so that it can look up multiple
+statistics using an array containing multiple ``id`` numbers. This reduces the
+function call overhead of retrieving statistics, and makes lookup of multiple
+statistics simpler for the application.
+
+.. code-block:: c
+
+    #define APP_NUM_STATS 4
+    /* application cached these ids previously; see above */
+    uint64_t ids_array[APP_NUM_STATS] = {3,4,7,21};
+    uint64_t value_array[APP_NUM_STATS];
+
+    /* Getting multiple xstats values from array of IDs */
+    rte_eth_xstats_get(port_id, ids_array, value_array, APP_NUM_STATS);
+
+    uint32_t i;
+    for(i = 0; i < APP_NUM_STATS; i++) {
+        printf("%d: %"PRIu64"\n", ids_array[i], value_array[i]);
+    }
+
+
+This array lookup API for xstats allows the application create multiple
+"groups" of statistics, and look up the values of those IDs using a single API
+call. As an end result, the application is able to achieve its goal of
+monitoring a single statistic ("rx_errors" in this case), and if that shows
+packets being dropped, it can easily retrieve a "set" of statistics using the
+IDs array parameter to ``rte_eth_xstats_get`` function.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 4e1e6dc..0653cf7 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1454,12 +1454,19 @@  struct rte_eth_dev *
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 	dev = &rte_eth_devices[port_id];
+	if (dev->dev_ops->xstats_get_names_by_ids != NULL) {
+		count = (*dev->dev_ops->xstats_get_names_by_ids)(dev, NULL,
+				NULL, 0);
+		if (count < 0)
+			return count;
+	}
 	if (dev->dev_ops->xstats_get_names != NULL) {
 		count = (*dev->dev_ops->xstats_get_names)(dev, NULL, 0);
 		if (count < 0)
 			return count;
 	} else
 		count = 0;
+
 	count += RTE_NB_STATS;
 	count += RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS) *
 		 RTE_NB_RXQ_STATS;
@@ -1469,150 +1476,367 @@  struct rte_eth_dev *
 }
 
 int
-rte_eth_xstats_get_names(uint8_t port_id,
-	struct rte_eth_xstat_name *xstats_names,
-	unsigned size)
+rte_eth_xstats_get_id_by_name(uint8_t port_id, const char *xstat_name,
+		uint64_t *id)
 {
-	struct rte_eth_dev *dev;
-	int cnt_used_entries;
-	int cnt_expected_entries;
-	int cnt_driver_entries;
-	uint32_t idx, id_queue;
-	uint16_t num_q;
+	int cnt_xstats, idx_xstat;
 
-	cnt_expected_entries = get_xstats_count(port_id);
-	if (xstats_names == NULL || cnt_expected_entries < 0 ||
-			(int)size < cnt_expected_entries)
-		return cnt_expected_entries;
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 
-	/* port_id checked in get_xstats_count() */
-	dev = &rte_eth_devices[port_id];
-	cnt_used_entries = 0;
+	if (!id) {
+		RTE_PMD_DEBUG_TRACE("Error: id pointer is NULL\n");
+		return -1;
+	}
 
-	for (idx = 0; idx < RTE_NB_STATS; idx++) {
-		snprintf(xstats_names[cnt_used_entries].name,
-			sizeof(xstats_names[0].name),
-			"%s", rte_stats_strings[idx].name);
-		cnt_used_entries++;
+	if (!xstat_name) {
+		RTE_PMD_DEBUG_TRACE("Error: xstat_name pointer is NULL\n");
+		return -1;
 	}
-	num_q = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (id_queue = 0; id_queue < num_q; id_queue++) {
-		for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
+
+	/* Get count */
+	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0, NULL);
+	if (cnt_xstats  < 0) {
+		RTE_PMD_DEBUG_TRACE("Error: Cannot get count of xstats\n");
+		return -1;
+	}
+
+	/* Get id-name lookup table */
+	struct rte_eth_xstat_name xstats_names[cnt_xstats];
+
+	if (cnt_xstats != rte_eth_xstats_get_names(
+			port_id, xstats_names, cnt_xstats, NULL)) {
+		RTE_PMD_DEBUG_TRACE("Error: Cannot get xstats lookup\n");
+		return -1;
+	}
+
+	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++) {
+		if (!strcmp(xstats_names[idx_xstat].name, xstat_name)) {
+			*id = idx_xstat;
+			return 0;
+		};
+	}
+
+	return -EINVAL;
+}
+
+int
+rte_eth_xstats_get_names_v1607(uint8_t port_id,
+	struct rte_eth_xstat_name *xstats_names,
+	unsigned int size)
+{
+	return rte_eth_xstats_get_names(port_id, xstats_names, size, NULL);
+}
+VERSION_SYMBOL(rte_eth_xstats_get_names, _v1607, 16.07);
+
+int
+rte_eth_xstats_get_names_v1705(uint8_t port_id,
+	struct rte_eth_xstat_name *xstats_names, unsigned int size,
+	uint64_t *ids)
+{
+	/* Get all xstats */
+	if (!ids) {
+		struct rte_eth_dev *dev;
+		int cnt_used_entries;
+		int cnt_expected_entries;
+		int cnt_driver_entries;
+		uint32_t idx, id_queue;
+		uint16_t num_q;
+
+		cnt_expected_entries = get_xstats_count(port_id);
+		if (xstats_names == NULL || cnt_expected_entries < 0 ||
+				(int)size < cnt_expected_entries)
+			return cnt_expected_entries;
+
+		/* port_id checked in get_xstats_count() */
+		dev = &rte_eth_devices[port_id];
+		cnt_used_entries = 0;
+
+		for (idx = 0; idx < RTE_NB_STATS; idx++) {
 			snprintf(xstats_names[cnt_used_entries].name,
 				sizeof(xstats_names[0].name),
-				"rx_q%u%s",
-				id_queue, rte_rxq_stats_strings[idx].name);
+				"%s", rte_stats_strings[idx].name);
 			cnt_used_entries++;
 		}
+		num_q = RTE_MIN(dev->data->nb_rx_queues,
+				RTE_ETHDEV_QUEUE_STAT_CNTRS);
+		for (id_queue = 0; id_queue < num_q; id_queue++) {
+			for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
+				snprintf(xstats_names[cnt_used_entries].name,
+					sizeof(xstats_names[0].name),
+					"rx_q%u%s",
+					id_queue,
+					rte_rxq_stats_strings[idx].name);
+				cnt_used_entries++;
+			}
+
+		}
+		num_q = RTE_MIN(dev->data->nb_tx_queues,
+				RTE_ETHDEV_QUEUE_STAT_CNTRS);
+		for (id_queue = 0; id_queue < num_q; id_queue++) {
+			for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
+				snprintf(xstats_names[cnt_used_entries].name,
+					sizeof(xstats_names[0].name),
+					"tx_q%u%s",
+					id_queue,
+					rte_txq_stats_strings[idx].name);
+				cnt_used_entries++;
+			}
+		}
+
+		if (dev->dev_ops->xstats_get_names_by_ids != NULL) {
+			/* If there are any driver-specific xstats, append them
+			 * to end of list.
+			 */
+			cnt_driver_entries =
+				(*dev->dev_ops->xstats_get_names_by_ids)(
+				dev,
+				xstats_names + cnt_used_entries,
+				NULL,
+				size - cnt_used_entries);
+			if (cnt_driver_entries < 0)
+				return cnt_driver_entries;
+			cnt_used_entries += cnt_driver_entries;
+
+		} else if (dev->dev_ops->xstats_get_names != NULL) {
+			/* If there are any driver-specific xstats, append them
+			 * to end of list.
+			 */
+			cnt_driver_entries = (*dev->dev_ops->xstats_get_names)(
+				dev,
+				xstats_names + cnt_used_entries,
+				size - cnt_used_entries);
+			if (cnt_driver_entries < 0)
+				return cnt_driver_entries;
+			cnt_used_entries += cnt_driver_entries;
+		}
 
+		return cnt_used_entries;
 	}
-	num_q = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	for (id_queue = 0; id_queue < num_q; id_queue++) {
-		for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
-			snprintf(xstats_names[cnt_used_entries].name,
-				sizeof(xstats_names[0].name),
-				"tx_q%u%s",
-				id_queue, rte_txq_stats_strings[idx].name);
-			cnt_used_entries++;
+	/* Get only xstats given by IDS */
+	else {
+		uint16_t len, i;
+		struct rte_eth_xstat_name *xstats_names_copy;
+
+		len = rte_eth_xstats_get_names_v1705(port_id, NULL, 0, NULL);
+
+		xstats_names_copy =
+				malloc(sizeof(struct rte_eth_xstat_name) * len);
+		if (!xstats_names_copy) {
+			RTE_PMD_DEBUG_TRACE(
+			     "ERROR: can't allocate memory for values_copy\n");
+			free(xstats_names_copy);
+			return -1;
 		}
+
+		rte_eth_xstats_get_names_v1705(port_id, xstats_names_copy,
+				len, NULL);
+
+		for (i = 0; i < size; i++) {
+			if (ids[i] >= len) {
+				RTE_PMD_DEBUG_TRACE(
+					"ERROR: id value isn't valid\n");
+				return -1;
+			}
+			strcpy(xstats_names[i].name,
+					xstats_names_copy[ids[i]].name);
+		}
+		free(xstats_names_copy);
+		return size;
 	}
+}
+BIND_DEFAULT_SYMBOL(rte_eth_xstats_get_names, _v1705, 17.05);
 
-	if (dev->dev_ops->xstats_get_names != NULL) {
-		/* If there are any driver-specific xstats, append them
-		 * to end of list.
-		 */
-		cnt_driver_entries = (*dev->dev_ops->xstats_get_names)(
-			dev,
-			xstats_names + cnt_used_entries,
-			size - cnt_used_entries);
-		if (cnt_driver_entries < 0)
-			return cnt_driver_entries;
-		cnt_used_entries += cnt_driver_entries;
+MAP_STATIC_SYMBOL(int
+		rte_eth_xstats_get_names(uint8_t port_id,
+			struct rte_eth_xstat_name *xstats_names,
+			unsigned int size,
+			uint64_t *ids), rte_eth_xstats_get_names_v1705);
+
+/* retrieve ethdev extended statistics */
+int
+rte_eth_xstats_get_v22(uint8_t port_id, struct rte_eth_xstat *xstats,
+	unsigned int n)
+{
+	uint64_t *values_copy;
+	uint16_t size, i;
+
+	values_copy = malloc(sizeof(values_copy) * n);
+	if (!values_copy) {
+		RTE_PMD_DEBUG_TRACE(
+				"ERROR: Cannot allocate memory for xstats\n");
+		return -1;
 	}
+	size = rte_eth_xstats_get(port_id, 0, values_copy, n);
 
-	return cnt_used_entries;
+	for (i = 0; i < n; i++) {
+		xstats[i].id = i;
+		xstats[i].value = values_copy[i];
+	}
+	free(values_copy);
+	return size;
 }
+VERSION_SYMBOL(rte_eth_xstats_get, _v22, 2.2);
 
 /* retrieve ethdev extended statistics */
 int
-rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
-	unsigned n)
+rte_eth_xstats_get_v1705(uint8_t port_id, uint64_t *ids, uint64_t *values,
+	unsigned int n)
 {
-	struct rte_eth_stats eth_stats;
-	struct rte_eth_dev *dev;
-	unsigned count = 0, i, q;
-	signed xcount = 0;
-	uint64_t val, *stats_ptr;
-	uint16_t nb_rxqs, nb_txqs;
+	/* If need all xstats */
+	if (!ids) {
+		struct rte_eth_stats eth_stats;
+		struct rte_eth_dev *dev;
+		unsigned int count = 0, i, q;
+		signed int xcount = 0;
+		uint64_t val, *stats_ptr;
+		uint16_t nb_rxqs, nb_txqs;
 
-	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
+		RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+		dev = &rte_eth_devices[port_id];
 
-	dev = &rte_eth_devices[port_id];
+		nb_rxqs = RTE_MIN(dev->data->nb_rx_queues,
+				RTE_ETHDEV_QUEUE_STAT_CNTRS);
+		nb_txqs = RTE_MIN(dev->data->nb_tx_queues,
+				RTE_ETHDEV_QUEUE_STAT_CNTRS);
 
-	nb_rxqs = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
-	nb_txqs = RTE_MIN(dev->data->nb_tx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
+		/* Return generic statistics */
+		count = RTE_NB_STATS + (nb_rxqs * RTE_NB_RXQ_STATS) +
+			(nb_txqs * RTE_NB_TXQ_STATS);
 
-	/* Return generic statistics */
-	count = RTE_NB_STATS + (nb_rxqs * RTE_NB_RXQ_STATS) +
-		(nb_txqs * RTE_NB_TXQ_STATS);
 
-	/* implemented by the driver */
-	if (dev->dev_ops->xstats_get != NULL) {
-		/* Retrieve the xstats from the driver at the end of the
-		 * xstats struct.
-		 */
-		xcount = (*dev->dev_ops->xstats_get)(dev,
-				     xstats ? xstats + count : NULL,
-				     (n > count) ? n - count : 0);
+		/* implemented by the driver */
+		if (dev->dev_ops->xstats_get_by_ids != NULL) {
+			/* Retrieve the xstats from the driver at the end of the
+			 * xstats struct. Retrieve all xstats.
+			 */
+			xcount = (*dev->dev_ops->xstats_get_by_ids)(dev,
+					NULL,
+					values ? values + count : NULL,
+					(n > count) ? n - count : 0);
+
+			if (xcount < 0)
+				return xcount;
+		/* implemented by the driver */
+		} else if (dev->dev_ops->xstats_get != NULL) {
+			/* Retrieve the xstats from the driver at the end of the
+			 * xstats struct. Retrieve all xstats.
+			 * Compatibility for PMD without xstats_get_by_ids
+			 */
+			unsigned int size = (n > count) ? n - count : 1;
+			struct rte_eth_xstat xstats[size];
 
-		if (xcount < 0)
-			return xcount;
-	}
+			xcount = (*dev->dev_ops->xstats_get)(dev,
+					values ? xstats : NULL,	size);
 
-	if (n < count + xcount || xstats == NULL)
-		return count + xcount;
+			if (xcount < 0)
+				return xcount;
+
+			if (values != NULL)
+				for (i = 0 ; i < (unsigned int)xcount; i++)
+					values[i + count] = xstats[i].value;
+		}
 
-	/* now fill the xstats structure */
-	count = 0;
-	rte_eth_stats_get(port_id, &eth_stats);
+		if (n < count + xcount || values == NULL)
+			return count + xcount;
 
-	/* global stats */
-	for (i = 0; i < RTE_NB_STATS; i++) {
-		stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_stats_strings[i].offset);
-		val = *stats_ptr;
-		xstats[count++].value = val;
-	}
+		/* now fill the xstats structure */
+		count = 0;
+		rte_eth_stats_get(port_id, &eth_stats);
 
-	/* per-rxq stats */
-	for (q = 0; q < nb_rxqs; q++) {
-		for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
+		/* global stats */
+		for (i = 0; i < RTE_NB_STATS; i++) {
 			stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_rxq_stats_strings[i].offset +
-					q * sizeof(uint64_t));
+						rte_stats_strings[i].offset);
 			val = *stats_ptr;
-			xstats[count++].value = val;
+			values[count++] = val;
 		}
+
+		/* per-rxq stats */
+		for (q = 0; q < nb_rxqs; q++) {
+			for (i = 0; i < RTE_NB_RXQ_STATS; i++) {
+				stats_ptr = RTE_PTR_ADD(&eth_stats,
+					    rte_rxq_stats_strings[i].offset +
+					    q * sizeof(uint64_t));
+				val = *stats_ptr;
+				values[count++] = val;
+			}
+		}
+
+		/* per-txq stats */
+		for (q = 0; q < nb_txqs; q++) {
+			for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
+				stats_ptr = RTE_PTR_ADD(&eth_stats,
+					    rte_txq_stats_strings[i].offset +
+					    q * sizeof(uint64_t));
+				val = *stats_ptr;
+				values[count++] = val;
+			}
+		}
+
+		return count + xcount;
 	}
+	/* Need only xstats given by IDS array */
+	else {
+		uint16_t i, size;
+		uint64_t *values_copy;
 
-	/* per-txq stats */
-	for (q = 0; q < nb_txqs; q++) {
-		for (i = 0; i < RTE_NB_TXQ_STATS; i++) {
-			stats_ptr = RTE_PTR_ADD(&eth_stats,
-					rte_txq_stats_strings[i].offset +
-					q * sizeof(uint64_t));
-			val = *stats_ptr;
-			xstats[count++].value = val;
+		size = rte_eth_xstats_get_v1705(port_id, NULL, NULL, 0);
+
+		values_copy = malloc(sizeof(values_copy) * size);
+		if (!values_copy) {
+			RTE_PMD_DEBUG_TRACE(
+			    "ERROR: can't allocate memory for values_copy\n");
+			return -1;
+		}
+
+		rte_eth_xstats_get_v1705(port_id, NULL, values_copy, size);
+
+		for (i = 0; i < n; i++) {
+			if (ids[i] >= size) {
+				RTE_PMD_DEBUG_TRACE(
+					"ERROR: id value isn't valid\n");
+				return -1;
+			}
+			values[i] = values_copy[ids[i]];
 		}
+		free(values_copy);
+		return n;
+	}
+}
+BIND_DEFAULT_SYMBOL(rte_eth_xstats_get, _v1705, 17.05);
+
+MAP_STATIC_SYMBOL(int
+		rte_eth_xstats_get(uint8_t port_id, uint64_t *ids,
+		uint64_t *values, unsigned int n), rte_eth_xstats_get_v1705);
+
+__rte_deprecated int
+rte_eth_xstats_get_all(uint8_t port_id, struct rte_eth_xstat *xstats,
+	unsigned int n)
+{
+	uint64_t *values_copy;
+	uint16_t size, i;
+
+	values_copy = malloc(sizeof(values_copy) * n);
+	if (!values_copy) {
+		RTE_PMD_DEBUG_TRACE(
+				"ERROR: Cannot allocate memory for xstats\n");
+		return -1;
 	}
+	size = rte_eth_xstats_get(port_id, 0, values_copy, n);
 
-	for (i = 0; i < count; i++)
+	for (i = 0; i < n; i++) {
 		xstats[i].id = i;
-	/* add an offset to driver-specific stats */
-	for ( ; i < count + xcount; i++)
-		xstats[i].id += count;
+		xstats[i].value = values_copy[i];
+	}
+	free(values_copy);
+	return size;
+}
 
-	return count + xcount;
+__rte_deprecated int
+rte_eth_xstats_get_names_all(uint8_t port_id,
+		struct rte_eth_xstat_name *xstats_names, unsigned int n)
+{
+	return rte_eth_xstats_get_names(port_id, xstats_names, n, NULL);
 }
 
 /* reset ethdev extended statistics */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index d072538..0ef9d20 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -186,6 +186,7 @@ 
 #include "rte_ether.h"
 #include "rte_eth_ctrl.h"
 #include "rte_dev_info.h"
+#include "rte_compat.h"
 
 struct rte_mbuf;
 
@@ -1118,6 +1119,10 @@  typedef int (*eth_xstats_get_t)(struct rte_eth_dev *dev,
 	struct rte_eth_xstat *stats, unsigned n);
 /**< @internal Get extended stats of an Ethernet device. */
 
+typedef int (*eth_xstats_get_by_ids_t)(struct rte_eth_dev *dev,
+		uint64_t *ids, uint64_t *values, unsigned int n);
+/**< @internal Get extended stats of an Ethernet device. */
+
 typedef void (*eth_xstats_reset_t)(struct rte_eth_dev *dev);
 /**< @internal Reset extended stats of an Ethernet device. */
 
@@ -1125,6 +1130,17 @@  typedef int (*eth_xstats_get_names_t)(struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, unsigned size);
 /**< @internal Get names of extended stats of an Ethernet device. */
 
+typedef int (*eth_xstats_get_names_by_ids_t)(struct rte_eth_dev *dev,
+	struct rte_eth_xstat_name *xstats_names, uint64_t *ids,
+	unsigned int size);
+/**< @internal Get names of extended stats of an Ethernet device. */
+
+typedef int (*eth_xstats_get_by_name_t)(struct rte_eth_dev *dev,
+		struct rte_eth_xstat_name *xstats_names,
+		struct rte_eth_xstat *xstat,
+		const char *name);
+/**< @internal Get xstat specified by name of an Ethernet device. */
+
 typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
@@ -1466,8 +1482,8 @@  struct eth_dev_ops {
 	eth_stats_reset_t          stats_reset;   /**< Reset generic device statistics. */
 	eth_xstats_get_t           xstats_get;    /**< Get extended device statistics. */
 	eth_xstats_reset_t         xstats_reset;  /**< Reset extended device statistics. */
-	eth_xstats_get_names_t     xstats_get_names;
-	/**< Get names of extended statistics. */
+	eth_xstats_get_names_t	   xstats_get_names;
+	/**< Get names of extended device statistics. */
 	eth_queue_stats_mapping_set_t queue_stats_mapping_set;
 	/**< Configure per queue stat counter mapping. */
 
@@ -1563,6 +1579,12 @@  struct eth_dev_ops {
 	eth_timesync_adjust_time   timesync_adjust_time; /** Adjust the device clock. */
 	eth_timesync_read_time     timesync_read_time; /** Get the device clock time. */
 	eth_timesync_write_time    timesync_write_time; /** Set the device clock time. */
+	eth_xstats_get_by_ids_t    xstats_get_by_ids;
+	/**< Get extended device statistics by ID. */
+	eth_xstats_get_names_by_ids_t xstats_get_names_by_ids;
+	/**< Get name of extended device statistics by ID. */
+	eth_xstats_get_by_name_t   xstats_get_by_name;
+	/**< Get extended device statistics by name. */
 };
 
 /**
@@ -2324,8 +2346,55 @@  int rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
  */
 void rte_eth_stats_reset(uint8_t port_id);
 
+
 /**
- * Retrieve names of extended statistics of an Ethernet device.
+* Gets the ID of a statistic from its name.
+*
+* Note this function searches for the statistics using string compares, and
+* as such should not be used on the fast-path. For fast-path retrieval of
+* specific statistics, store the ID as provided in *id* from this function,
+* and pass the ID to rte_eth_xstats_get()
+*
+* @param port_id The port to look up statistics from
+* @param xstat_name The name of the statistic to return
+* @param[out] id A pointer to an app-supplied uint64_t which should be
+*                set to the ID of the stat if the stat exists.
+* @return
+*    0 on success
+*    -ENODEV for invalid port_id,
+*    -EINVAL if the xstat_name doesn't exist in port_id
+*/
+int rte_eth_xstats_get_id_by_name(uint8_t port_id, const char *xstat_name,
+		uint64_t *id);
+
+/**
+ * Retrieve all extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param xstats
+ *   A pointer to a table of structure of type *rte_eth_xstat*
+ *   to be filled with device statistics ids and values: id is the
+ *   index of the name string in xstats_names (see rte_eth_xstats_get_names()),
+ *   and value is the statistic counter.
+ *   This parameter can be set to NULL if n is 0.
+ * @param n
+ *   The size of the xstats array (number of elements).
+ * @return
+ *   - A positive value lower or equal to n: success. The return value
+ *     is the number of entries filled in the stats table.
+ *   - A positive value higher than n: error, the given statistics table
+ *     is too small. The return value corresponds to the size that should
+ *     be given to succeed. The entries in the table are not valid and
+ *     shall not be used by the caller.
+ *   - A negative value on error (invalid port id).
+ */
+__rte_deprecated
+int rte_eth_xstats_get_all(uint8_t port_id, struct rte_eth_xstat *xstats,
+	unsigned int n);
+
+/**
+ * Retrieve names of all extended statistics of an Ethernet device.
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
@@ -2333,7 +2402,7 @@  int rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
  *   An rte_eth_xstat_name array of at least *size* elements to
  *   be filled. If set to NULL, the function returns the required number
  *   of elements.
- * @param size
+ * @param n
  *   The size of the xstats_names array (number of elements).
  * @return
  *   - A positive value lower or equal to size: success. The return value
@@ -2344,9 +2413,8 @@  int rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
  *     shall not be used by the caller.
  *   - A negative value on error (invalid port id).
  */
-int rte_eth_xstats_get_names(uint8_t port_id,
-		struct rte_eth_xstat_name *xstats_names,
-		unsigned size);
+__rte_deprecated int rte_eth_xstats_get_names_all(uint8_t port_id,
+		struct rte_eth_xstat_name *xstats_names, unsigned int n);
 
 /**
  * Retrieve extended statistics of an Ethernet device.
@@ -2370,8 +2438,92 @@  int rte_eth_xstats_get_names(uint8_t port_id,
  *     shall not be used by the caller.
  *   - A negative value on error (invalid port id).
  */
-int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
-		unsigned n);
+int rte_eth_xstats_get_v22(uint8_t port_id, struct rte_eth_xstat *xstats,
+	unsigned int n);
+
+/**
+ * Retrieve extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param ids
+ *   A pointer to an ids array passed by application. This tells wich
+ *   statistics values function should retrieve. This parameter
+ *   can be set to NULL if n is 0. In this case function will retrieve
+ *   all avalible statistics.
+ * @param values
+ *   A pointer to a table to be filled with device statistics values.
+ * @param n
+ *   The size of the ids array (number of elements).
+ * @return
+ *   - A positive value lower or equal to n: success. The return value
+ *     is the number of entries filled in the stats table.
+ *   - A positive value higher than n: error, the given statistics table
+ *     is too small. The return value corresponds to the size that should
+ *     be given to succeed. The entries in the table are not valid and
+ *     shall not be used by the caller.
+ *   - A negative value on error (invalid port id).
+ */
+int rte_eth_xstats_get_v1705(uint8_t port_id, uint64_t *ids, uint64_t *values,
+	unsigned int n);
+
+int rte_eth_xstats_get(uint8_t port_id, uint64_t *ids, uint64_t *values,
+	unsigned int n);
+
+/**
+ * Retrieve extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param xstats_names
+ *   A pointer to a table of structure of type *rte_eth_xstat*
+ *   to be filled with device statistics ids and values: id is the
+ *   index of the name string in xstats_names (see rte_eth_xstats_get_names()),
+ *   and value is the statistic counter.
+ *   This parameter can be set to NULL if n is 0.
+ * @param n
+ *   The size of the xstats array (number of elements).
+ * @return
+ *   - A positive value lower or equal to n: success. The return value
+ *     is the number of entries filled in the stats table.
+ *   - A positive value higher than n: error, the given statistics table
+ *     is too small. The return value corresponds to the size that should
+ *     be given to succeed. The entries in the table are not valid and
+ *     shall not be used by the caller.
+ *   - A negative value on error (invalid port id).
+ */
+int rte_eth_xstats_get_names_v1607(uint8_t port_id,
+		struct rte_eth_xstat_name *xstats_names, unsigned int n);
+
+/**
+ * Retrieve names of extended statistics of an Ethernet device.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param xstats_names
+ *   An rte_eth_xstat_name array of at least *size* elements to
+ *   be filled. If set to NULL, the function returns the required number
+ *   of elements.
+ * @param ids
+ *   IDs array given by app to retrieve specific statistics
+ * @param size
+ *   The size of the xstats_names array (number of elements).
+ * @return
+ *   - A positive value lower or equal to size: success. The return value
+ *     is the number of entries filled in the stats table.
+ *   - A positive value higher than size: error, the given statistics table
+ *     is too small. The return value corresponds to the size that should
+ *     be given to succeed. The entries in the table are not valid and
+ *     shall not be used by the caller.
+ *   - A negative value on error (invalid port id).
+ */
+int rte_eth_xstats_get_names_v1705(uint8_t port_id,
+	struct rte_eth_xstat_name *xstats_names, unsigned int size,
+	uint64_t *ids);
+
+int rte_eth_xstats_get_names(uint8_t port_id,
+	struct rte_eth_xstat_name *xstats_names, unsigned int size,
+	uint64_t *ids);
 
 /**
  * Reset extended statistics of an Ethernet device.
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 0ea3856..f4d0136 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -159,5 +159,10 @@  DPDK_17.05 {
 	global:
 
 	rte_eth_find_next;
+	rte_eth_xstats_get_names;
+	rte_eth_xstats_get;
+	rte_eth_xstats_get_all;
+	rte_eth_xstats_get_names_all;
+	rte_eth_xstats_get_id_by_name;
 
 } DPDK_17.02;