DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats
@ 2017-04-27 14:42 Michal Jastrzebski
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev Michal Jastrzebski
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

This patchset fixes following patches:

    commit ea85e7d711b6 ("ethdev: retrieve xstats by ID")
    commit a954495245c4 ("ethdev: get xstats ID by name")
    commit 1223608adb9b ("app/proc-info: support xstats by ID")
    commit 25e38f09af9c ("net/e1000: support xstats by ID")
    commit 923419333f5a ("net/ixgbe: support xstats by ID")

This patch addresses some API concerns with the xstat patchset
applied in DPDK 17.05 RC2.

For clarity the first patch reverts all changes from the above
commits.

The subsequent patches extends the xstats API in ethdev library
to allow grouping of stats logically so they can be retrieved per
logical grouping managed by the application.

The patchset adds new functions rte_eth_xstats_get_names_by_id()
and rte_eth_xstats_get_by_id() to use a new list of arguments:
array of ids and array of values. It also introduces a new
function, rte_eth_xstats_get_id_by_name(), to retrieve xstats ids
by their names.

It also extends the functionality of the proc_info application:
    --xstats-name NAME: to display single xstat value by NAME

Finally the test-pmd application is updated to use new API.


Kuba Kozak (6):
  ethdev: revert patches extending xstats API in ethdev
  ethdev: retrieve xstats by ID
  ethdev: get xstats ID by name
  app/proc-info: support xstats by ID and by name
  net/e1000: support xstats by ID
  net/ixgbe: support xstats by ID

 app/proc_info/main.c                    |  12 +-
 app/test-pmd/config.c                   |  19 ++-
 doc/guides/prog_guide/poll_mode_drv.rst |  22 ++--
 doc/guides/rel_notes/release_17_05.rst  |   5 +-
 drivers/net/e1000/igb_ethdev.c          |  26 ++--
 drivers/net/ixgbe/ixgbe_ethdev.c        |  25 ++--
 lib/librte_ether/rte_ethdev.c           | 224 +++++++++++++++++++++-----------
 lib/librte_ether/rte_ethdev.h           | 175 ++++++++-----------------
 lib/librte_ether/rte_ether_version.map  |   6 +-
 9 files changed, 259 insertions(+), 255 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:09   ` Van Haaren, Harry
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID Michal Jastrzebski
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

Revert patches to provide clear view for
upcoming changes. Reverted patches are listed below:
commit ea85e7d711b6 ("ethdev: retrieve xstats by ID")
commit a954495245c4 ("ethdev: get xstats ID by name")
commit 1223608adb9b ("app/proc-info: support xstats by ID")
commit 25e38f09af9c ("net/e1000: support xstats by ID")
commit 923419333f5a ("net/ixgbe: support xstats by ID")

Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
---
 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  |   9 -
 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, 151 insertions(+), 1071 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index 16b27b2..d576b42 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -86,14 +86,6 @@ static uint32_t reset_stats;
 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
@@ -107,9 +99,6 @@ proc_info_usage(const char *prgname)
 			"default\n"
 		"  --metrics: to display derived metrics of the ports, disabled by "
 			"default\n"
-		"  --xstats-name NAME: displays the ID of a single xstats 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"
@@ -143,33 +132,6 @@ parse_portmask(const char *portmask)
 
 }
 
-/*
- * 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)
 {
@@ -216,9 +178,7 @@ proc_info_parse_args(int argc, char **argv)
 		{"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}
 	};
@@ -264,28 +224,7 @@ proc_info_parse_args(int argc, char **argv)
 					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;
@@ -412,82 +351,20 @@ 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;
-	uint64_t *values;
+	struct rte_eth_xstat *xstats;
 	int len, ret, i;
 	static const char *nic_stats_border = "########################";
 
-	len = rte_eth_xstats_get_names(port_id, NULL, 0, NULL);
+	len = rte_eth_xstats_get_names(port_id, NULL, 0);
 	if (len < 0) {
 		printf("Cannot get xstats count\n");
 		return;
 	}
-	values = malloc(sizeof(values) * len);
-	if (values == NULL) {
+	xstats = malloc(sizeof(xstats[0]) * len);
+	if (xstats == NULL) {
 		printf("Cannot allocate memory for xstats\n");
 		return;
 	}
@@ -495,11 +372,11 @@ nic_xstats_display(uint8_t port_id)
 	xstats_names = malloc(sizeof(struct rte_eth_xstat_name) * len);
 	if (xstats_names == NULL) {
 		printf("Cannot allocate memory for xstat names\n");
-		free(values);
+		free(xstats);
 		return;
 	}
 	if (len != rte_eth_xstats_get_names(
-			port_id, xstats_names, len, NULL)) {
+			port_id, xstats_names, len)) {
 		printf("Cannot get xstat names\n");
 		goto err;
 	}
@@ -508,7 +385,7 @@ nic_xstats_display(uint8_t port_id)
 			   port_id);
 	printf("%s############################\n",
 			   nic_stats_border);
-	ret = rte_eth_xstats_get(port_id, NULL, values, len);
+	ret = rte_eth_xstats_get(port_id, xstats, len);
 	if (ret < 0 || ret > len) {
 		printf("Cannot get xstats\n");
 		goto err;
@@ -524,18 +401,18 @@ nic_xstats_display(uint8_t port_id)
 						  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, values[i]);
+				xstats_names[i].name, xstats[i].value);
 			write(stdout_fd, buf, strlen(buf));
 		} else {
 			printf("%s: %"PRIu64"\n", xstats_names[i].name,
-					values[i]);
+			       xstats[i].value);
 		}
 	}
 
 	printf("%s############################\n",
 			   nic_stats_border);
 err:
-	free(values);
+	free(xstats);
 	free(xstats_names);
 }
 
@@ -674,11 +551,6 @@ main(int argc, char **argv)
 				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 ef07925..4d873cd 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -264,9 +264,9 @@ nic_stats_clear(portid_t port_id)
 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 @@ nic_xstats_display(portid_t port_id)
 	}
 
 	/* Get count */
-	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0, NULL);
+	cnt_xstats = rte_eth_xstats_get_names(port_id, NULL, 0);
 	if (cnt_xstats  < 0) {
 		printf("Error: Cannot get count of xstats\n");
 		return;
@@ -288,24 +288,23 @@ nic_xstats_display(portid_t port_id)
 		return;
 	}
 	if (cnt_xstats != rte_eth_xstats_get_names(
-			port_id, xstats_names, cnt_xstats, NULL)) {
+			port_id, xstats_names, cnt_xstats)) {
 		printf("Error: Cannot get xstats lookup\n");
 		free(xstats_names);
 		return;
 	}
 
 	/* Get stats themselves */
-	values = malloc(sizeof(values) * cnt_xstats);
-	if (values == NULL) {
+	xstats = malloc(sizeof(struct rte_eth_xstat) * cnt_xstats);
+	if (xstats == NULL) {
 		printf("Cannot allocate memory for xstats\n");
 		free(xstats_names);
 		return;
 	}
-	if (cnt_xstats != rte_eth_xstats_get(port_id, NULL, values,
-			cnt_xstats)) {
+	if (cnt_xstats != rte_eth_xstats_get(port_id, xstats, cnt_xstats)) {
 		printf("Error: Unable to get xstats\n");
 		free(xstats_names);
-		free(values);
+		free(xstats);
 		return;
 	}
 
@@ -313,9 +312,9 @@ nic_xstats_display(portid_t port_id)
 	for (idx_xstat = 0; idx_xstat < cnt_xstats; idx_xstat++)
 		printf("%s: %"PRIu64"\n",
 			xstats_names[idx_xstat].name,
-			values[idx_xstat]);
+			xstats[idx_xstat].value);
 	free(xstats_names);
-	free(values);
+	free(xstats);
 }
 
 void
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index a1a758b..e48c121 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -334,21 +334,24 @@ The Ethernet device API exported by the Ethernet PMDs is described in the *DPDK
 Extended Statistics API
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-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.
-
-Note that extended statistic identifiers are
+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 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
@@ -360,8 +363,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
-above scheme:
+Examples of common statistics xstats strings, formatted to comply to the scheme
+proposed above:
 
 * ``rx_bytes``
 * ``rx_crc_errors``
@@ -375,7 +378,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:
@@ -389,139 +392,3 @@ 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/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index ad20e86..dcd55ff 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -450,15 +450,6 @@ API Changes
   * The vhost public header file ``rte_virtio_net.h`` is renamed to
     ``rte_vhost.h``
 
-* **Reworked rte_ethdev library**
-
-  * Changed set of input parameters for ``rte_eth_xstats_get`` and ``rte_eth_xstats_get_names`` functions.
-
-  * Added new functions ``rte_eth_xstats_get_all`` and ``rte_eth_xstats_get_names_all to provide backward compatibility for
-    ``rte_eth_xstats_get`` and ``rte_eth_xstats_get_names``
-
-  * Added new function ``rte_eth_xstats_get_id_by_name``
-
 
 ABI Changes
 -----------
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index ca9f98c..137780e 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -116,13 +116,9 @@ static void eth_igb_stats_get(struct rte_eth_dev *dev,
 				struct rte_eth_stats *rte_stats);
 static int eth_igb_xstats_get(struct rte_eth_dev *dev,
 			      struct rte_eth_xstat *xstats, unsigned n);
-static int eth_igb_xstats_get_by_ids(struct rte_eth_dev *dev, uint64_t *ids,
-		uint64_t *values, unsigned int n);
 static int eth_igb_xstats_get_names(struct rte_eth_dev *dev,
-		struct rte_eth_xstat_name *xstats_names, unsigned int limit);
-static int eth_igb_xstats_get_names_by_ids(struct rte_eth_dev *dev,
-		struct rte_eth_xstat_name *xstats_names, uint64_t *ids,
-		unsigned int limit);
+				    struct rte_eth_xstat_name *xstats_names,
+				    unsigned int limit);
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_xstats_reset(struct rte_eth_dev *dev);
 static int eth_igb_fw_version_get(struct rte_eth_dev *dev,
@@ -393,8 +389,6 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.link_update          = eth_igb_link_update,
 	.stats_get            = eth_igb_stats_get,
 	.xstats_get           = eth_igb_xstats_get,
-	.xstats_get_by_ids    = eth_igb_xstats_get_by_ids,
-	.xstats_get_names_by_ids = eth_igb_xstats_get_names_by_ids,
 	.xstats_get_names     = eth_igb_xstats_get_names,
 	.stats_reset          = eth_igb_stats_reset,
 	.xstats_reset         = eth_igb_xstats_reset,
@@ -1868,41 +1862,6 @@ static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	return IGB_NB_XSTATS;
 }
 
-static int eth_igb_xstats_get_names_by_ids(struct rte_eth_dev *dev,
-		struct rte_eth_xstat_name *xstats_names, uint64_t *ids,
-		unsigned int limit)
-{
-	unsigned int i;
-
-	if (!ids) {
-		if (xstats_names == NULL)
-			return IGB_NB_XSTATS;
-
-		for (i = 0; i < IGB_NB_XSTATS; i++)
-			snprintf(xstats_names[i].name,
-					sizeof(xstats_names[i].name),
-					"%s", rte_igb_stats_strings[i].name);
-
-		return IGB_NB_XSTATS;
-
-	} else {
-		struct rte_eth_xstat_name xstats_names_copy[IGB_NB_XSTATS];
-
-		eth_igb_xstats_get_names_by_ids(dev, xstats_names_copy, NULL,
-				IGB_NB_XSTATS);
-
-		for (i = 0; i < limit; i++) {
-			if (ids[i] >= IGB_NB_XSTATS) {
-				PMD_INIT_LOG(ERR, "id value isn't valid");
-				return -1;
-			}
-			strcpy(xstats_names[i].name,
-					xstats_names_copy[ids[i]].name);
-		}
-		return limit;
-	}
-}
-
 static int
 eth_igb_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		   unsigned n)
@@ -1933,53 +1892,6 @@ eth_igb_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	return IGB_NB_XSTATS;
 }
 
-static int
-eth_igb_xstats_get_by_ids(struct rte_eth_dev *dev, uint64_t *ids,
-		uint64_t *values, unsigned int n)
-{
-	unsigned int i;
-
-	if (!ids) {
-		struct e1000_hw *hw =
-			E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-		struct e1000_hw_stats *hw_stats =
-			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
-
-		if (n < IGB_NB_XSTATS)
-			return IGB_NB_XSTATS;
-
-		igb_read_stats_registers(hw, hw_stats);
-
-		/* If this is a reset xstats is NULL, and we have cleared the
-		 * registers by reading them.
-		 */
-		if (!values)
-			return 0;
-
-		/* Extended stats */
-		for (i = 0; i < IGB_NB_XSTATS; i++)
-			values[i] = *(uint64_t *)(((char *)hw_stats) +
-					rte_igb_stats_strings[i].offset);
-
-		return IGB_NB_XSTATS;
-
-	} else {
-		uint64_t values_copy[IGB_NB_XSTATS];
-
-		eth_igb_xstats_get_by_ids(dev, NULL, values_copy,
-				IGB_NB_XSTATS);
-
-		for (i = 0; i < n; i++) {
-			if (ids[i] >= IGB_NB_XSTATS) {
-				PMD_INIT_LOG(ERR, "id value isn't valid");
-				return -1;
-			}
-			values[i] = values_copy[ids[i]];
-		}
-		return n;
-	}
-}
-
 static void
 igbvf_read_stats_registers(struct e1000_hw *hw, struct e1000_vf_stats *hw_stats)
 {
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7b856bb..c226e0a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -182,20 +182,12 @@ static int ixgbe_dev_xstats_get(struct rte_eth_dev *dev,
 				struct rte_eth_xstat *xstats, unsigned n);
 static int ixgbevf_dev_xstats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_xstat *xstats, unsigned n);
-static int
-ixgbe_dev_xstats_get_by_ids(struct rte_eth_dev *dev, uint64_t *ids,
-		uint64_t *values, unsigned int n);
 static void ixgbe_dev_stats_reset(struct rte_eth_dev *dev);
 static void ixgbe_dev_xstats_reset(struct rte_eth_dev *dev);
 static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned limit);
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned limit);
-static int ixgbe_dev_xstats_get_names_by_ids(
-	__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names,
-	uint64_t *ids,
-	unsigned int limit);
 static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
@@ -531,11 +523,9 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.link_update          = ixgbe_dev_link_update,
 	.stats_get            = ixgbe_dev_stats_get,
 	.xstats_get           = ixgbe_dev_xstats_get,
-	.xstats_get_by_ids    = ixgbe_dev_xstats_get_by_ids,
 	.stats_reset          = ixgbe_dev_stats_reset,
 	.xstats_reset         = ixgbe_dev_xstats_reset,
 	.xstats_get_names     = ixgbe_dev_xstats_get_names,
-	.xstats_get_names_by_ids = ixgbe_dev_xstats_get_names_by_ids,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
 	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
@@ -3192,84 +3182,6 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	return cnt_stats;
 }
 
-static int ixgbe_dev_xstats_get_names_by_ids(
-	__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names,
-	uint64_t *ids,
-	unsigned int limit)
-{
-	if (!ids) {
-		const unsigned int cnt_stats = ixgbe_xstats_calc_num();
-		unsigned int stat, i, count;
-
-		if (xstats_names != NULL) {
-			count = 0;
-
-			/* Note: limit >= cnt_stats checked upstream
-			 * in rte_eth_xstats_names()
-			 */
-
-			/* Extended stats from ixgbe_hw_stats */
-			for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
-				snprintf(xstats_names[count].name,
-					sizeof(xstats_names[count].name),
-					"%s",
-					rte_ixgbe_stats_strings[i].name);
-				count++;
-			}
-
-			/* MACsec Stats */
-			for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
-				snprintf(xstats_names[count].name,
-					sizeof(xstats_names[count].name),
-					"%s",
-					rte_ixgbe_macsec_strings[i].name);
-				count++;
-			}
-
-			/* RX Priority Stats */
-			for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
-				for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
-					snprintf(xstats_names[count].name,
-					    sizeof(xstats_names[count].name),
-					    "rx_priority%u_%s", i,
-					    rte_ixgbe_rxq_strings[stat].name);
-					count++;
-				}
-			}
-
-			/* TX Priority Stats */
-			for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
-				for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
-					snprintf(xstats_names[count].name,
-					    sizeof(xstats_names[count].name),
-					    "tx_priority%u_%s", i,
-					    rte_ixgbe_txq_strings[stat].name);
-					count++;
-				}
-			}
-		}
-		return cnt_stats;
-	}
-
-	uint16_t i;
-	uint16_t size = ixgbe_xstats_calc_num();
-	struct rte_eth_xstat_name xstats_names_copy[size];
-
-	ixgbe_dev_xstats_get_names_by_ids(dev, xstats_names_copy, NULL,
-			size);
-
-	for (i = 0; i < limit; i++) {
-		if (ids[i] >= size) {
-			PMD_INIT_LOG(ERR, "id value isn't valid");
-			return -1;
-		}
-		strcpy(xstats_names[i].name,
-				xstats_names_copy[ids[i]].name);
-	}
-	return limit;
-}
-
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, unsigned limit)
 {
@@ -3360,97 +3272,6 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	return count;
 }
 
-static int
-ixgbe_dev_xstats_get_by_ids(struct rte_eth_dev *dev, uint64_t *ids,
-		uint64_t *values, unsigned int n)
-{
-	if (!ids) {
-		struct ixgbe_hw *hw =
-				IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-		struct ixgbe_hw_stats *hw_stats =
-				IXGBE_DEV_PRIVATE_TO_STATS(
-						dev->data->dev_private);
-		struct ixgbe_macsec_stats *macsec_stats =
-				IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
-					dev->data->dev_private);
-		uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
-		unsigned int i, stat, count = 0;
-
-		count = ixgbe_xstats_calc_num();
-
-		if (!ids && n < count)
-			return count;
-
-		total_missed_rx = 0;
-		total_qbrc = 0;
-		total_qprc = 0;
-		total_qprdc = 0;
-
-		ixgbe_read_stats_registers(hw, hw_stats, macsec_stats,
-				&total_missed_rx, &total_qbrc, &total_qprc,
-				&total_qprdc);
-
-		/* If this is a reset xstats is NULL, and we have cleared the
-		 * registers by reading them.
-		 */
-		if (!ids && !values)
-			return 0;
-
-		/* Extended stats from ixgbe_hw_stats */
-		count = 0;
-		for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
-			values[count] = *(uint64_t *)(((char *)hw_stats) +
-					rte_ixgbe_stats_strings[i].offset);
-			count++;
-		}
-
-		/* MACsec Stats */
-		for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
-			values[count] = *(uint64_t *)(((char *)macsec_stats) +
-					rte_ixgbe_macsec_strings[i].offset);
-			count++;
-		}
-
-		/* RX Priority Stats */
-		for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
-			for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
-				values[count] =
-					*(uint64_t *)(((char *)hw_stats) +
-					rte_ixgbe_rxq_strings[stat].offset +
-					(sizeof(uint64_t) * i));
-				count++;
-			}
-		}
-
-		/* TX Priority Stats */
-		for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
-			for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
-				values[count] =
-					*(uint64_t *)(((char *)hw_stats) +
-					rte_ixgbe_txq_strings[stat].offset +
-					(sizeof(uint64_t) * i));
-				count++;
-			}
-		}
-		return count;
-	}
-
-	uint16_t i;
-	uint16_t size = ixgbe_xstats_calc_num();
-	uint64_t values_copy[size];
-
-	ixgbe_dev_xstats_get_by_ids(dev, NULL, values_copy, size);
-
-	for (i = 0; i < n; i++) {
-		if (ids[i] >= size) {
-			PMD_INIT_LOG(ERR, "id value isn't valid");
-			return -1;
-		}
-		values[i] = values_copy[ids[i]];
-	}
-	return n;
-}
-
 static void
 ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 {
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 89f6514..9922430 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1360,19 +1360,12 @@ get_xstats_count(uint8_t port_id)
 
 	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;
@@ -1382,367 +1375,150 @@ get_xstats_count(uint8_t port_id)
 }
 
 int
-rte_eth_xstats_get_id_by_name(uint8_t port_id, const char *xstat_name,
-		uint64_t *id)
-{
-	int cnt_xstats, idx_xstat;
-
-	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
-	if (!id) {
-		RTE_PMD_DEBUG_TRACE("Error: id pointer is NULL\n");
-		return -1;
-	}
-
-	if (!xstat_name) {
-		RTE_PMD_DEBUG_TRACE("Error: xstat_name pointer is NULL\n");
-		return -1;
-	}
-
-	/* 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,
+rte_eth_xstats_get_names(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;
+	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;
+	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;
+	/* 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++) {
+	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++;
+	}
+	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),
-				"%s", rte_stats_strings[idx].name);
+				"rx_q%u%s",
+				id_queue, rte_rxq_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;
 	}
-	/* 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);
+	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++;
 		}
-		free(xstats_names_copy);
-		return size;
 	}
-}
-BIND_DEFAULT_SYMBOL(rte_eth_xstats_get_names, _v1705, 17.05);
-
-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;
+	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;
 	}
-	size = rte_eth_xstats_get(port_id, 0, values_copy, n);
 
-	for (i = 0; i < n; i++) {
-		xstats[i].id = i;
-		xstats[i].value = values_copy[i];
-	}
-	free(values_copy);
-	return size;
+	return cnt_used_entries;
 }
-VERSION_SYMBOL(rte_eth_xstats_get, _v22, 2.2);
 
 /* retrieve ethdev extended statistics */
 int
-rte_eth_xstats_get_v1705(uint8_t port_id, uint64_t *ids, uint64_t *values,
+rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 	unsigned int n)
 {
-	/* 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, -ENODEV);
-		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);
-
-		/* 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_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];
-
-			xcount = (*dev->dev_ops->xstats_get)(dev,
-					values ? xstats : NULL,	size);
-
-			if (xcount < 0)
-				return xcount;
-
-			if (values != NULL)
-				for (i = 0 ; i < (unsigned int)xcount; i++)
-					values[i + count] = xstats[i].value;
-		}
+	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;
 
-		if (n < count + xcount || values == NULL)
-			return count + xcount;
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
-		/* now fill the xstats structure */
-		count = 0;
-		rte_eth_stats_get(port_id, &eth_stats);
+	dev = &rte_eth_devices[port_id];
 
-		/* 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;
-			values[count++] = val;
-		}
+	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);
 
-		/* 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;
-			}
-		}
+	/* Return generic statistics */
+	count = RTE_NB_STATS + (nb_rxqs * RTE_NB_RXQ_STATS) +
+		(nb_txqs * RTE_NB_TXQ_STATS);
 
-		/* 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;
-			}
-		}
+	/* 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);
 
-		return count + xcount;
+		if (xcount < 0)
+			return xcount;
 	}
-	/* Need only xstats given by IDS array */
-	else {
-		uint16_t i, size;
-		uint64_t *values_copy;
 
-		size = rte_eth_xstats_get_v1705(port_id, NULL, NULL, 0);
+	if (n < count + xcount || xstats == NULL)
+		return count + xcount;
 
-		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;
-		}
+	/* now fill the xstats structure */
+	count = 0;
+	rte_eth_stats_get(port_id, &eth_stats);
 
-		rte_eth_xstats_get_v1705(port_id, NULL, values_copy, size);
+	/* 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;
+	}
 
-		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]];
+	/* 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;
+			xstats[count++].value = val;
 		}
-		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;
+	/* 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(port_id, 0, values_copy, n);
 
-	for (i = 0; i < n; i++) {
+	for (i = 0; i < count; i++)
 		xstats[i].id = i;
-		xstats[i].value = values_copy[i];
-	}
-	free(values_copy);
-	return size;
-}
+	/* add an offset to driver-specific stats */
+	for ( ; i < count + xcount; i++)
+		xstats[i].id += count;
 
-__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);
+	return count + xcount;
 }
 
 /* reset ethdev extended statistics */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index e0f7ee5..a46290c 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -185,7 +185,6 @@ extern "C" {
 #include "rte_ether.h"
 #include "rte_eth_ctrl.h"
 #include "rte_dev_info.h"
-#include "rte_compat.h"
 
 struct rte_mbuf;
 
@@ -1122,10 +1121,6 @@ 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. */
 
@@ -1133,17 +1128,6 @@ 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,
@@ -1582,12 +1566,6 @@ 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. */
 };
 
 /**
@@ -2291,55 +2269,8 @@ int rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats);
  */
 void rte_eth_stats_reset(uint8_t port_id);
 
-
-/**
- * Gets the ID of a statistic from its name.
- *
- * 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.
+ * Retrieve names of extended statistics of an Ethernet device.
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
@@ -2347,7 +2278,7 @@ int rte_eth_xstats_get_all(uint8_t port_id, struct rte_eth_xstat *xstats,
  *   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 n
+ * @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
@@ -2358,9 +2289,9 @@ int rte_eth_xstats_get_all(uint8_t port_id, struct rte_eth_xstat *xstats,
  *     shall not be used by the caller.
  *   - A negative value on error (invalid port id).
  */
-__rte_deprecated
-int rte_eth_xstats_get_names_all(uint8_t port_id,
-		struct rte_eth_xstat_name *xstats_names, unsigned int n);
+int rte_eth_xstats_get_names(uint8_t port_id,
+		struct rte_eth_xstat_name *xstats_names,
+		unsigned int size);
 
 /**
  * Retrieve extended statistics of an Ethernet device.
@@ -2384,92 +2315,8 @@ int rte_eth_xstats_get_names_all(uint8_t port_id,
  *     shall not be used by the caller.
  *   - A negative value on error (invalid port id).
  */
-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);
+int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
+		unsigned int n);
 
 /**
  * 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 6e118c4..238c2a1 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -151,10 +151,5 @@ DPDK_17.05 {
 
 	rte_eth_dev_attach_secondary;
 	rte_eth_find_next;
-	rte_eth_xstats_get;
-	rte_eth_xstats_get_all;
-	rte_eth_xstats_get_id_by_name;
-	rte_eth_xstats_get_names;
-	rte_eth_xstats_get_names_all;
 
 } DPDK_17.02;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:09   ` Van Haaren, Harry
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name Michal Jastrzebski
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@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.
Added new functions rte_eth_xstats_get_names_by_id and
rte_eth_xstats_get_by_id using additional arguments (in compare
to rte_eth_xstats_get_names and rte_eth_xstats_get) - array of ids
and array of values.

doc: add description for modified xstats API
Documentation change for new 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: Kuba Kozak <kubax.kozak@intel.com>
---
 doc/guides/prog_guide/poll_mode_drv.rst | 167 ++++++++++++++++++---
 doc/guides/rel_notes/release_17_05.rst  |   4 +
 lib/librte_ether/rte_ethdev.c           | 250 ++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h           |  70 +++++++++
 lib/librte_ether/rte_ether_version.map  |   2 +
 5 files changed, 476 insertions(+), 17 deletions(-)

diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index e48c121..4987f70 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -334,24 +334,21 @@ 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
+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.
+
+Note that extended statistic identifiers are
 driver-specific, and hence might not be the same for different ports.
+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
@@ -392,3 +389,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_by_id()``: 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_by_id()``: 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_by_id(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_by_id(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_by_id(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_by_id(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_by_id(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_by_id`` function.
diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index dcd55ff..12d6b49 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -450,6 +450,10 @@ API Changes
   * The vhost public header file ``rte_virtio_net.h`` is renamed to
     ``rte_vhost.h``
 
+* **Reworked rte_ethdev library**
+
+  * Added new functions ``rte_eth_xstats_get_by_id`` and ``rte_eth_xstats_get_names_by_id``
+
 
 ABI Changes
 -----------
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 9922430..331d2e3 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1360,12 +1360,19 @@ get_xstats_count(uint8_t port_id)
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 	dev = &rte_eth_devices[port_id];
+	if (dev->dev_ops->xstats_get_names_by_id != NULL) {
+		count = (*dev->dev_ops->xstats_get_names_by_id)(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;
@@ -1375,6 +1382,123 @@ get_xstats_count(uint8_t port_id)
 }
 
 int
+rte_eth_xstats_get_names_by_id(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),
+				"%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_id != NULL) {
+			/* If there are any driver-specific xstats, append them
+			 * to end of list.
+			 */
+			cnt_driver_entries =
+				(*dev->dev_ops->xstats_get_names_by_id)(
+				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;
+	}
+	/* 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_by_id(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_by_id(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;
+	}
+}
+
+int
 rte_eth_xstats_get_names(uint8_t port_id,
 	struct rte_eth_xstat_name *xstats_names,
 	unsigned int size)
@@ -1441,6 +1565,132 @@ rte_eth_xstats_get_names(uint8_t port_id,
 
 /* retrieve ethdev extended statistics */
 int
+rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids, uint64_t *values,
+	unsigned int n)
+{
+	/* 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, -ENODEV);
+		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);
+
+		/* 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_by_id != NULL) {
+			/* Retrieve the xstats from the driver at the end of the
+			 * xstats struct. Retrieve all xstats.
+			 */
+			xcount = (*dev->dev_ops->xstats_get_by_id)(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];
+
+			xcount = (*dev->dev_ops->xstats_get)(dev,
+					values ? xstats : NULL,	size);
+
+			if (xcount < 0)
+				return xcount;
+
+			if (values != NULL)
+				for (i = 0 ; i < (unsigned int)xcount; i++)
+					values[i + count] = xstats[i].value;
+		}
+
+		if (n < count + xcount || values == NULL)
+			return count + xcount;
+
+		/* now fill the xstats structure */
+		count = 0;
+		rte_eth_stats_get(port_id, &eth_stats);
+
+		/* 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;
+			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;
+
+		size = rte_eth_xstats_get_by_id(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_by_id(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;
+	}
+}
+
+int
 rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 	unsigned int n)
 {
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index a46290c..8594416 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -185,6 +185,7 @@ extern "C" {
 #include "rte_ether.h"
 #include "rte_eth_ctrl.h"
 #include "rte_dev_info.h"
+#include "rte_compat.h"
 
 struct rte_mbuf;
 
@@ -1121,6 +1122,12 @@ 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_id_t)(struct rte_eth_dev *dev,
+				      const 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. */
 
@@ -1128,6 +1135,11 @@ 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_id_t)(struct rte_eth_dev *dev,
+	struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
+	unsigned int size);
+/**< @internal Get names of extended stats 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,
@@ -1566,6 +1578,11 @@ 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_id_t     xstats_get_by_id;
+	/**< Get extended device statistic values by ID. */
+	eth_xstats_get_names_by_id_t xstats_get_names_by_id;
+	/**< Get name of extended device statistics by ID. */
 };
 
 /**
@@ -2319,6 +2336,59 @@ int rte_eth_xstats_get(uint8_t port_id, struct rte_eth_xstat *xstats,
 		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_by_id(uint8_t port_id,
+	struct rte_eth_xstat_name *xstats_names, unsigned int size,
+	uint64_t *ids);
+
+/**
+ * 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_by_id(uint8_t port_id, const uint64_t *ids,
+			     uint64_t *values, unsigned int n);
+
+/**
  * Reset extended statistics of an Ethernet device.
  *
  * @param port_id
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 238c2a1..1abd717 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -151,5 +151,7 @@ DPDK_17.05 {
 
 	rte_eth_dev_attach_secondary;
 	rte_eth_find_next;
+	rte_eth_xstats_get_by_id;
+	rte_eth_xstats_get_names_by_id;
 
 } DPDK_17.02;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev Michal Jastrzebski
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:10   ` Van Haaren, Harry
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and " Michal Jastrzebski
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

Introduced new function: rte_eth_xstats_get_id_by_name
to retrieve xstats ids by its names.

doc: added release note

Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
---
 doc/guides/rel_notes/release_17_05.rst |  2 ++
 lib/librte_ether/rte_ethdev.c          | 44 ++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h          | 20 ++++++++++++++++
 lib/librte_ether/rte_ether_version.map |  1 +
 4 files changed, 67 insertions(+)

diff --git a/doc/guides/rel_notes/release_17_05.rst b/doc/guides/rel_notes/release_17_05.rst
index 12d6b49..2e1463f 100644
--- a/doc/guides/rel_notes/release_17_05.rst
+++ b/doc/guides/rel_notes/release_17_05.rst
@@ -454,6 +454,8 @@ API Changes
 
   * Added new functions ``rte_eth_xstats_get_by_id`` and ``rte_eth_xstats_get_names_by_id``
 
+  * Added new function ``rte_eth_xstats_get_id_by_name``
+
 
 ABI Changes
 -----------
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 331d2e3..8aa6331 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -1382,6 +1382,50 @@ get_xstats_count(uint8_t port_id)
 }
 
 int
+rte_eth_xstats_get_id_by_name(uint8_t port_id, const char *xstat_name,
+		uint64_t *id)
+{
+	int cnt_xstats, idx_xstat;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+
+	if (!id) {
+		RTE_PMD_DEBUG_TRACE("Error: id pointer is NULL\n");
+		return -ENOMEM;
+	}
+
+	if (!xstat_name) {
+		RTE_PMD_DEBUG_TRACE("Error: xstat_name pointer is NULL\n");
+		return -ENOMEM;
+	}
+
+	/* Get count */
+	cnt_xstats = rte_eth_xstats_get_names_by_id(port_id, NULL, 0, NULL);
+	if (cnt_xstats  < 0) {
+		RTE_PMD_DEBUG_TRACE("Error: Cannot get count of xstats\n");
+		return -ENODEV;
+	}
+
+	/* Get id-name lookup table */
+	struct rte_eth_xstat_name xstats_names[cnt_xstats];
+
+	if (cnt_xstats != rte_eth_xstats_get_names_by_id(
+			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_by_id(uint8_t port_id,
 	struct rte_eth_xstat_name *xstats_names, unsigned int size,
 	uint64_t *ids)
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 8594416..782a6a8 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2389,6 +2389,26 @@ int rte_eth_xstats_get_by_id(uint8_t port_id, const uint64_t *ids,
 			     uint64_t *values, unsigned int n);
 
 /**
+ * Gets the ID of a statistic from its name.
+ *
+ * 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);
+
+/**
  * Reset extended statistics of an Ethernet device.
  *
  * @param port_id
diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
index 1abd717..d6726bb 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -152,6 +152,7 @@ DPDK_17.05 {
 	rte_eth_dev_attach_secondary;
 	rte_eth_find_next;
 	rte_eth_xstats_get_by_id;
+	rte_eth_xstats_get_id_by_name;
 	rte_eth_xstats_get_names_by_id;
 
 } DPDK_17.02;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and by name
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
                   ` (2 preceding siblings ...)
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:10   ` Van Haaren, Harry
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID Michal Jastrzebski
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

There are new arguments --xstats-ids and --xstats-name
in proc_info command line to retrieve statistics given by ids
and by name.
E.g. --xstats-ids="1,3,5,7,8"
E.g. --xstats-name rx_errors

Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
---
 app/proc_info/main.c | 150 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 139 insertions(+), 11 deletions(-)

diff --git a/app/proc_info/main.c b/app/proc_info/main.c
index d576b42..17a1c87 100644
--- a/app/proc_info/main.c
+++ b/app/proc_info/main.c
@@ -86,6 +86,14 @@ static uint32_t reset_stats;
 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
@@ -99,6 +107,9 @@ proc_info_usage(const char *prgname)
 			"default\n"
 		"  --metrics: to display derived metrics of the ports, disabled by "
 			"default\n"
+		"  --xstats-name NAME: to display single xstat id 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 +143,33 @@ parse_portmask(const char *portmask)
 
 }
 
+/*
+ * 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 +216,9 @@ proc_info_parse_args(int argc, char **argv)
 		{"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 +264,28 @@ proc_info_parse_args(int argc, char **argv)
 					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 +412,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_by_id(
+			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_by_id(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_by_id(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 +495,11 @@ nic_xstats_display(uint8_t port_id)
 	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)) {
+	if (len != rte_eth_xstats_get_names_by_id(
+			port_id, xstats_names, len, NULL)) {
 		printf("Cannot get xstat names\n");
 		goto err;
 	}
@@ -385,7 +508,7 @@ nic_xstats_display(uint8_t port_id)
 			   port_id);
 	printf("%s############################\n",
 			   nic_stats_border);
-	ret = rte_eth_xstats_get(port_id, xstats, len);
+	ret = rte_eth_xstats_get_by_id(port_id, NULL, values, len);
 	if (ret < 0 || ret > len) {
 		printf("Cannot get xstats\n");
 		goto err;
@@ -401,18 +524,18 @@ nic_xstats_display(uint8_t port_id)
 						  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 +674,11 @@ main(int argc, char **argv)
 				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);
 		}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
                   ` (3 preceding siblings ...)
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and " Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:10   ` Van Haaren, Harry
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 6/6] net/ixgbe: " Michal Jastrzebski
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

To achieve functionality of retrieving only specific statistics
given by application there are two new functions added:
eth_igb_xstats_get_by_id() which retrieve
values of statistics specified by ids array
and eth_igb_xstats_get_names_by_id() which retrieve
names of statistics specified by ids array.

Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
---
 drivers/net/e1000/igb_ethdev.c | 94 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 92 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 137780e..ca7c4a8 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -116,9 +116,15 @@ static void eth_igb_stats_get(struct rte_eth_dev *dev,
 				struct rte_eth_stats *rte_stats);
 static int eth_igb_xstats_get(struct rte_eth_dev *dev,
 			      struct rte_eth_xstat *xstats, unsigned n);
+static int eth_igb_xstats_get_by_id(struct rte_eth_dev *dev,
+		const uint64_t *ids,
+		uint64_t *values, unsigned int n);
 static int eth_igb_xstats_get_names(struct rte_eth_dev *dev,
 				    struct rte_eth_xstat_name *xstats_names,
-				    unsigned int limit);
+				    unsigned int size);
+static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
+		struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
+		unsigned int limit);
 static void eth_igb_stats_reset(struct rte_eth_dev *dev);
 static void eth_igb_xstats_reset(struct rte_eth_dev *dev);
 static int eth_igb_fw_version_get(struct rte_eth_dev *dev,
@@ -389,6 +395,8 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.link_update          = eth_igb_link_update,
 	.stats_get            = eth_igb_stats_get,
 	.xstats_get           = eth_igb_xstats_get,
+	.xstats_get_by_id    = eth_igb_xstats_get_by_id,
+	.xstats_get_names_by_id = eth_igb_xstats_get_names_by_id,
 	.xstats_get_names     = eth_igb_xstats_get_names,
 	.stats_reset          = eth_igb_stats_reset,
 	.xstats_reset         = eth_igb_xstats_reset,
@@ -1845,7 +1853,7 @@ eth_igb_xstats_reset(struct rte_eth_dev *dev)
 
 static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names,
-	__rte_unused unsigned limit)
+	__rte_unused unsigned int size)
 {
 	unsigned i;
 
@@ -1862,6 +1870,41 @@ static int eth_igb_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	return IGB_NB_XSTATS;
 }
 
+static int eth_igb_xstats_get_names_by_id(struct rte_eth_dev *dev,
+		struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
+		unsigned int limit)
+{
+	unsigned int i;
+
+	if (!ids) {
+		if (xstats_names == NULL)
+			return IGB_NB_XSTATS;
+
+		for (i = 0; i < IGB_NB_XSTATS; i++)
+			snprintf(xstats_names[i].name,
+					sizeof(xstats_names[i].name),
+					"%s", rte_igb_stats_strings[i].name);
+
+		return IGB_NB_XSTATS;
+
+	} else {
+		struct rte_eth_xstat_name xstats_names_copy[IGB_NB_XSTATS];
+
+		eth_igb_xstats_get_names_by_id(dev, xstats_names_copy, NULL,
+				IGB_NB_XSTATS);
+
+		for (i = 0; i < limit; i++) {
+			if (ids[i] >= IGB_NB_XSTATS) {
+				PMD_INIT_LOG(ERR, "id value isn't valid");
+				return -1;
+			}
+			strcpy(xstats_names[i].name,
+					xstats_names_copy[ids[i]].name);
+		}
+		return limit;
+	}
+}
+
 static int
 eth_igb_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 		   unsigned n)
@@ -1892,6 +1935,53 @@ eth_igb_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	return IGB_NB_XSTATS;
 }
 
+static int
+eth_igb_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
+		uint64_t *values, unsigned int n)
+{
+	unsigned int i;
+
+	if (!ids) {
+		struct e1000_hw *hw =
+			E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+		struct e1000_hw_stats *hw_stats =
+			E1000_DEV_PRIVATE_TO_STATS(dev->data->dev_private);
+
+		if (n < IGB_NB_XSTATS)
+			return IGB_NB_XSTATS;
+
+		igb_read_stats_registers(hw, hw_stats);
+
+		/* If this is a reset xstats is NULL, and we have cleared the
+		 * registers by reading them.
+		 */
+		if (!values)
+			return 0;
+
+		/* Extended stats */
+		for (i = 0; i < IGB_NB_XSTATS; i++)
+			values[i] = *(uint64_t *)(((char *)hw_stats) +
+					rte_igb_stats_strings[i].offset);
+
+		return IGB_NB_XSTATS;
+
+	} else {
+		uint64_t values_copy[IGB_NB_XSTATS];
+
+		eth_igb_xstats_get_by_id(dev, NULL, values_copy,
+				IGB_NB_XSTATS);
+
+		for (i = 0; i < n; i++) {
+			if (ids[i] >= IGB_NB_XSTATS) {
+				PMD_INIT_LOG(ERR, "id value isn't valid");
+				return -1;
+			}
+			values[i] = values_copy[ids[i]];
+		}
+		return n;
+	}
+}
+
 static void
 igbvf_read_stats_registers(struct e1000_hw *hw, struct e1000_vf_stats *hw_stats)
 {
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [PATCH v1 6/6] net/ixgbe: support xstats by ID
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
                   ` (4 preceding siblings ...)
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID Michal Jastrzebski
@ 2017-04-27 14:42 ` Michal Jastrzebski
  2017-04-28 11:10   ` Van Haaren, Harry
  2017-05-01 21:34 ` [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Thomas Monjalon
  2017-05-01 21:50 ` Thomas Monjalon
  7 siblings, 1 reply; 16+ messages in thread
From: Michal Jastrzebski @ 2017-04-27 14:42 UTC (permalink / raw)
  To: dev; +Cc: harry.van.haaren, deepak.k.jain, Kuba Kozak

From: Kuba Kozak <kubax.kozak@intel.com>

To achieve functionality of retrieving only specific statistics
given by application there are two new functions added:
ixgbe_dev_xstats_get_by_ids() which retrieve
values of statistics specified by ids array
and ixgbe_dev_xstats_get_names_by_ids() which retrieve
names of statistics specified by ids array.

Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 184 ++++++++++++++++++++++++++++++++++++++-
 lib/librte_ether/rte_ethdev.c    |   2 +-
 2 files changed, 183 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c226e0a..2197290 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -182,12 +182,21 @@ static int ixgbe_dev_xstats_get(struct rte_eth_dev *dev,
 				struct rte_eth_xstat *xstats, unsigned n);
 static int ixgbevf_dev_xstats_get(struct rte_eth_dev *dev,
 				  struct rte_eth_xstat *xstats, unsigned n);
+static int
+ixgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
+		uint64_t *values, unsigned int n);
 static void ixgbe_dev_stats_reset(struct rte_eth_dev *dev);
 static void ixgbe_dev_xstats_reset(struct rte_eth_dev *dev);
 static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned limit);
+	struct rte_eth_xstat_name *xstats_names,
+	__rte_unused unsigned int size);
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned limit);
+static int ixgbe_dev_xstats_get_names_by_id(
+	__rte_unused struct rte_eth_dev *dev,
+	struct rte_eth_xstat_name *xstats_names,
+	const uint64_t *ids,
+	unsigned int limit);
 static int ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev,
 					     uint16_t queue_id,
 					     uint8_t stat_idx,
@@ -523,9 +532,11 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
 	.link_update          = ixgbe_dev_link_update,
 	.stats_get            = ixgbe_dev_stats_get,
 	.xstats_get           = ixgbe_dev_xstats_get,
+	.xstats_get_by_id    = ixgbe_dev_xstats_get_by_id,
 	.stats_reset          = ixgbe_dev_stats_reset,
 	.xstats_reset         = ixgbe_dev_xstats_reset,
 	.xstats_get_names     = ixgbe_dev_xstats_get_names,
+	.xstats_get_names_by_id = ixgbe_dev_xstats_get_names_by_id,
 	.queue_stats_mapping_set = ixgbe_dev_queue_stats_mapping_set,
 	.fw_version_get       = ixgbe_fw_version_get,
 	.dev_infos_get        = ixgbe_dev_info_get,
@@ -3127,7 +3138,7 @@ ixgbe_xstats_calc_num(void) {
 }
 
 static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
-	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned limit)
+	struct rte_eth_xstat_name *xstats_names, __rte_unused unsigned int size)
 {
 	const unsigned cnt_stats = ixgbe_xstats_calc_num();
 	unsigned stat, i, count;
@@ -3182,6 +3193,84 @@ static int ixgbe_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	return cnt_stats;
 }
 
+static int ixgbe_dev_xstats_get_names_by_id(
+	__rte_unused struct rte_eth_dev *dev,
+	struct rte_eth_xstat_name *xstats_names,
+	const uint64_t *ids,
+	unsigned int limit)
+{
+	if (!ids) {
+		const unsigned int cnt_stats = ixgbe_xstats_calc_num();
+		unsigned int stat, i, count;
+
+		if (xstats_names != NULL) {
+			count = 0;
+
+			/* Note: limit >= cnt_stats checked upstream
+			 * in rte_eth_xstats_names()
+			 */
+
+			/* Extended stats from ixgbe_hw_stats */
+			for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
+				snprintf(xstats_names[count].name,
+					sizeof(xstats_names[count].name),
+					"%s",
+					rte_ixgbe_stats_strings[i].name);
+				count++;
+			}
+
+			/* MACsec Stats */
+			for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
+				snprintf(xstats_names[count].name,
+					sizeof(xstats_names[count].name),
+					"%s",
+					rte_ixgbe_macsec_strings[i].name);
+				count++;
+			}
+
+			/* RX Priority Stats */
+			for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
+				for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
+					snprintf(xstats_names[count].name,
+					    sizeof(xstats_names[count].name),
+					    "rx_priority%u_%s", i,
+					    rte_ixgbe_rxq_strings[stat].name);
+					count++;
+				}
+			}
+
+			/* TX Priority Stats */
+			for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
+				for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
+					snprintf(xstats_names[count].name,
+					    sizeof(xstats_names[count].name),
+					    "tx_priority%u_%s", i,
+					    rte_ixgbe_txq_strings[stat].name);
+					count++;
+				}
+			}
+		}
+		return cnt_stats;
+	}
+
+	uint16_t i;
+	uint16_t size = ixgbe_xstats_calc_num();
+	struct rte_eth_xstat_name xstats_names_copy[size];
+
+	ixgbe_dev_xstats_get_names_by_id(dev, xstats_names_copy, NULL,
+			size);
+
+	for (i = 0; i < limit; i++) {
+		if (ids[i] >= size) {
+			PMD_INIT_LOG(ERR, "id value isn't valid");
+			return -1;
+		}
+		strcpy(xstats_names[i].name,
+				xstats_names_copy[ids[i]].name);
+	}
+	return limit;
+}
+
 static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	struct rte_eth_xstat_name *xstats_names, unsigned limit)
 {
@@ -3272,6 +3361,97 @@ ixgbe_dev_xstats_get(struct rte_eth_dev *dev, struct rte_eth_xstat *xstats,
 	return count;
 }
 
+static int
+ixgbe_dev_xstats_get_by_id(struct rte_eth_dev *dev, const uint64_t *ids,
+		uint64_t *values, unsigned int n)
+{
+	if (!ids) {
+		struct ixgbe_hw *hw =
+				IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+		struct ixgbe_hw_stats *hw_stats =
+				IXGBE_DEV_PRIVATE_TO_STATS(
+						dev->data->dev_private);
+		struct ixgbe_macsec_stats *macsec_stats =
+				IXGBE_DEV_PRIVATE_TO_MACSEC_STATS(
+					dev->data->dev_private);
+		uint64_t total_missed_rx, total_qbrc, total_qprc, total_qprdc;
+		unsigned int i, stat, count = 0;
+
+		count = ixgbe_xstats_calc_num();
+
+		if (!ids && n < count)
+			return count;
+
+		total_missed_rx = 0;
+		total_qbrc = 0;
+		total_qprc = 0;
+		total_qprdc = 0;
+
+		ixgbe_read_stats_registers(hw, hw_stats, macsec_stats,
+				&total_missed_rx, &total_qbrc, &total_qprc,
+				&total_qprdc);
+
+		/* If this is a reset xstats is NULL, and we have cleared the
+		 * registers by reading them.
+		 */
+		if (!ids && !values)
+			return 0;
+
+		/* Extended stats from ixgbe_hw_stats */
+		count = 0;
+		for (i = 0; i < IXGBE_NB_HW_STATS; i++) {
+			values[count] = *(uint64_t *)(((char *)hw_stats) +
+					rte_ixgbe_stats_strings[i].offset);
+			count++;
+		}
+
+		/* MACsec Stats */
+		for (i = 0; i < IXGBE_NB_MACSEC_STATS; i++) {
+			values[count] = *(uint64_t *)(((char *)macsec_stats) +
+					rte_ixgbe_macsec_strings[i].offset);
+			count++;
+		}
+
+		/* RX Priority Stats */
+		for (stat = 0; stat < IXGBE_NB_RXQ_PRIO_STATS; stat++) {
+			for (i = 0; i < IXGBE_NB_RXQ_PRIO_VALUES; i++) {
+				values[count] =
+					*(uint64_t *)(((char *)hw_stats) +
+					rte_ixgbe_rxq_strings[stat].offset +
+					(sizeof(uint64_t) * i));
+				count++;
+			}
+		}
+
+		/* TX Priority Stats */
+		for (stat = 0; stat < IXGBE_NB_TXQ_PRIO_STATS; stat++) {
+			for (i = 0; i < IXGBE_NB_TXQ_PRIO_VALUES; i++) {
+				values[count] =
+					*(uint64_t *)(((char *)hw_stats) +
+					rte_ixgbe_txq_strings[stat].offset +
+					(sizeof(uint64_t) * i));
+				count++;
+			}
+		}
+		return count;
+	}
+
+	uint16_t i;
+	uint16_t size = ixgbe_xstats_calc_num();
+	uint64_t values_copy[size];
+
+	ixgbe_dev_xstats_get_by_id(dev, NULL, values_copy, size);
+
+	for (i = 0; i < n; i++) {
+		if (ids[i] >= size) {
+			PMD_INIT_LOG(ERR, "id value isn't valid");
+			return -1;
+		}
+		values[i] = values_copy[ids[i]];
+	}
+	return n;
+}
+
 static void
 ixgbe_dev_xstats_reset(struct rte_eth_dev *dev)
 {
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 8aa6331..8edcfb4 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -188,7 +188,7 @@ rte_eth_dev_allocated(const char *name)
 static uint8_t
 rte_eth_dev_find_free_port(void)
 {
-	unsigned i;
+	unsigned int i;
 
 	for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
 		if (rte_eth_devices[i].state == RTE_ETH_DEV_UNUSED)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev Michal Jastrzebski
@ 2017-04-28 11:09   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:09 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 1/6] ethdev: revert patches extending xstats API in ethdev
> 
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> Revert patches to provide clear view for
> upcoming changes. Reverted patches are listed below:
> commit ea85e7d711b6 ("ethdev: retrieve xstats by ID")
> commit a954495245c4 ("ethdev: get xstats ID by name")
> commit 1223608adb9b ("app/proc-info: support xstats by ID")
> commit 25e38f09af9c ("net/e1000: support xstats by ID")
> commit 923419333f5a ("net/ixgbe: support xstats by ID")
> 
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID Michal Jastrzebski
@ 2017-04-28 11:09   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:09 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 2/6] ethdev: retrieve xstats by ID
> 
> From: Kuba Kozak <kubax.kozak@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.
> Added new functions rte_eth_xstats_get_names_by_id and
> rte_eth_xstats_get_by_id using additional arguments (in compare
> to rte_eth_xstats_get_names and rte_eth_xstats_get) - array of ids
> and array of values.
> 
> doc: add description for modified xstats API
> Documentation change for new 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: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name Michal Jastrzebski
@ 2017-04-28 11:10   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:10 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 3/6] ethdev: get xstats ID by name
> 
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> Introduced new function: rte_eth_xstats_get_id_by_name
> to retrieve xstats ids by its names.
> 
> doc: added release note
> 
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and by name
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and " Michal Jastrzebski
@ 2017-04-28 11:10   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:10 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 4/6] app/proc-info: support xstats by ID and by name
> 
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> There are new arguments --xstats-ids and --xstats-name
> in proc_info command line to retrieve statistics given by ids
> and by name.
> E.g. --xstats-ids="1,3,5,7,8"
> E.g. --xstats-name rx_errors
> 
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID Michal Jastrzebski
@ 2017-04-28 11:10   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:10 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 5/6] net/e1000: support xstats by ID
> 
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> To achieve functionality of retrieving only specific statistics
> given by application there are two new functions added:
> eth_igb_xstats_get_by_id() which retrieve
> values of statistics specified by ids array
> and eth_igb_xstats_get_names_by_id() which retrieve
> names of statistics specified by ids array.
> 
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 6/6] net/ixgbe: support xstats by ID
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 6/6] net/ixgbe: " Michal Jastrzebski
@ 2017-04-28 11:10   ` Van Haaren, Harry
  0 siblings, 0 replies; 16+ messages in thread
From: Van Haaren, Harry @ 2017-04-28 11:10 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, dev; +Cc: Jain, Deepak K, Kozak, KubaX

> From: Jastrzebski, MichalX K
> Sent: Thursday, April 27, 2017 3:43 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 v1 6/6] net/ixgbe: support xstats by ID
> 
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> To achieve functionality of retrieving only specific statistics
> given by application there are two new functions added:
> ixgbe_dev_xstats_get_by_ids() which retrieve
> values of statistics specified by ids array
> and ixgbe_dev_xstats_get_names_by_ids() which retrieve
> names of statistics specified by ids array.
> 
> Signed-off-by: Kuba Kozak <kubax.kozak@intel.com>

Acked-by: Harry van Haaren <harry.van.haaren@intel.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
                   ` (5 preceding siblings ...)
  2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 6/6] net/ixgbe: " Michal Jastrzebski
@ 2017-05-01 21:34 ` Thomas Monjalon
  2017-05-01 21:49   ` Thomas Monjalon
  2017-05-01 21:50 ` Thomas Monjalon
  7 siblings, 1 reply; 16+ messages in thread
From: Thomas Monjalon @ 2017-05-01 21:34 UTC (permalink / raw)
  To: Michal Jastrzebski, Kuba Kozak; +Cc: dev, harry.van.haaren, deepak.k.jain

27/04/2017 16:42, Michal Jastrzebski:
> From: Kuba Kozak <kubax.kozak@intel.com>
> 
> This patchset fixes following patches:
> 
>     commit ea85e7d711b6 ("ethdev: retrieve xstats by ID")
>     commit a954495245c4 ("ethdev: get xstats ID by name")
>     commit 1223608adb9b ("app/proc-info: support xstats by ID")
>     commit 25e38f09af9c ("net/e1000: support xstats by ID")
>     commit 923419333f5a ("net/ixgbe: support xstats by ID")
> 
> This patch addresses some API concerns with the xstat patchset
> applied in DPDK 17.05 RC2.
> 
> For clarity the first patch reverts all changes from the above
> commits.

It looks crazy to restart the feature from scratch, not showing the fixes.
But after a quick look, there was so many lines and functions to fix
that it is maybe a good idea.
At least, the history will be clear.

[...]
> Kuba Kozak (6):
>   ethdev: revert patches extending xstats API in ethdev
>   ethdev: retrieve xstats by ID
>   ethdev: get xstats ID by name
>   app/proc-info: support xstats by ID and by name
>   net/e1000: support xstats by ID
>   net/ixgbe: support xstats by ID

I am doing these small changes in your patches:

--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -395,7 +395,7 @@ static const struct eth_dev_ops eth_igb_ops = {
        .link_update          = eth_igb_link_update,
        .stats_get            = eth_igb_stats_get,
        .xstats_get           = eth_igb_xstats_get,
-       .xstats_get_by_id    = eth_igb_xstats_get_by_id,
+       .xstats_get_by_id     = eth_igb_xstats_get_by_id,
        .xstats_get_names_by_id = eth_igb_xstats_get_names_by_id,
        .xstats_get_names     = eth_igb_xstats_get_names,
        .stats_reset          = eth_igb_stats_reset,

--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -532,7 +532,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
        .link_update          = ixgbe_dev_link_update,
        .stats_get            = ixgbe_dev_stats_get,
        .xstats_get           = ixgbe_dev_xstats_get,
-       .xstats_get_by_id    = ixgbe_dev_xstats_get_by_id,
+       .xstats_get_by_id     = ixgbe_dev_xstats_get_by_id,
        .stats_reset          = ixgbe_dev_stats_reset,
        .xstats_reset         = ixgbe_dev_xstats_reset,
        .xstats_get_names     = ixgbe_dev_xstats_get_names,

--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -188,7 +188,7 @@ rte_eth_dev_allocated(const char *name)
 static uint8_t
 rte_eth_dev_find_free_port(void)
 {
-       unsigned int i;
+       unsigned i;
 
        for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
                if (rte_eth_devices[i].state == RTE_ETH_DEV_UNUSED)

--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -185,7 +185,6 @@ extern "C" {
 #include "rte_ether.h"
 #include "rte_eth_ctrl.h"
 #include "rte_dev_info.h"
-#include "rte_compat.h"
 
 struct rte_mbuf;
 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats
  2017-05-01 21:34 ` [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Thomas Monjalon
@ 2017-05-01 21:49   ` Thomas Monjalon
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2017-05-01 21:49 UTC (permalink / raw)
  To: Michal Jastrzebski, Kuba Kozak; +Cc: dev, harry.van.haaren, deepak.k.jain

01/05/2017 23:34, Thomas Monjalon:
> I am doing these small changes in your patches:

More explanations below

> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -395,7 +395,7 @@ static const struct eth_dev_ops eth_igb_ops = {
>         .link_update          = eth_igb_link_update,
>         .stats_get            = eth_igb_stats_get,
>         .xstats_get           = eth_igb_xstats_get,
> -       .xstats_get_by_id    = eth_igb_xstats_get_by_id,
> +       .xstats_get_by_id     = eth_igb_xstats_get_by_id,
>         .xstats_get_names_by_id = eth_igb_xstats_get_names_by_id,
>         .xstats_get_names     = eth_igb_xstats_get_names,
>         .stats_reset          = eth_igb_stats_reset,
> 
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -532,7 +532,7 @@ static const struct eth_dev_ops ixgbe_eth_dev_ops = {
>         .link_update          = ixgbe_dev_link_update,
>         .stats_get            = ixgbe_dev_stats_get,
>         .xstats_get           = ixgbe_dev_xstats_get,
> -       .xstats_get_by_id    = ixgbe_dev_xstats_get_by_id,
> +       .xstats_get_by_id     = ixgbe_dev_xstats_get_by_id,
>         .stats_reset          = ixgbe_dev_stats_reset,
>         .xstats_reset         = ixgbe_dev_xstats_reset,
>         .xstats_get_names     = ixgbe_dev_xstats_get_names,

It is just for correct alignment.

> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -188,7 +188,7 @@ rte_eth_dev_allocated(const char *name)
>  static uint8_t
>  rte_eth_dev_find_free_port(void)
>  {
> -       unsigned int i;
> +       unsigned i;
>  
>         for (i = 0; i < RTE_MAX_ETHPORTS; i++) {
>                 if (rte_eth_devices[i].state == RTE_ETH_DEV_UNUSED)

It reverts a change you made in ixgbe patch.

> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -185,7 +185,6 @@ extern "C" {
>  #include "rte_ether.h"
>  #include "rte_eth_ctrl.h"
>  #include "rte_dev_info.h"
> -#include "rte_compat.h"
>  
>  struct rte_mbuf;
>  

This include is not needed in this new version of xstats.

I will remove also the changes to release notes (API section),
as the API is not modified. They are just additional functions.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats
  2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
                   ` (6 preceding siblings ...)
  2017-05-01 21:34 ` [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Thomas Monjalon
@ 2017-05-01 21:50 ` Thomas Monjalon
  7 siblings, 0 replies; 16+ messages in thread
From: Thomas Monjalon @ 2017-05-01 21:50 UTC (permalink / raw)
  To: Michal Jastrzebski, Kuba Kozak; +Cc: dev, harry.van.haaren, deepak.k.jain

27/04/2017 16:42, Michal Jastrzebski:
> Kuba Kozak (6):
>   ethdev: revert patches extending xstats API in ethdev
>   ethdev: retrieve xstats by ID
>   ethdev: get xstats ID by name
>   app/proc-info: support xstats by ID and by name
>   net/e1000: support xstats by ID
>   net/ixgbe: support xstats by ID

Applied with small changes.
Thanks for this better version.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2017-05-01 21:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 14:42 [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Michal Jastrzebski
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 1/6] ethdev: revert patches extending xstats API in ethdev Michal Jastrzebski
2017-04-28 11:09   ` Van Haaren, Harry
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 2/6] ethdev: retrieve xstats by ID Michal Jastrzebski
2017-04-28 11:09   ` Van Haaren, Harry
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 3/6] ethdev: get xstats ID by name Michal Jastrzebski
2017-04-28 11:10   ` Van Haaren, Harry
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 4/6] app/proc-info: support xstats by ID and " Michal Jastrzebski
2017-04-28 11:10   ` Van Haaren, Harry
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 5/6] net/e1000: support xstats by ID Michal Jastrzebski
2017-04-28 11:10   ` Van Haaren, Harry
2017-04-27 14:42 ` [dpdk-dev] [PATCH v1 6/6] net/ixgbe: " Michal Jastrzebski
2017-04-28 11:10   ` Van Haaren, Harry
2017-05-01 21:34 ` [dpdk-dev] [PATCH v1 0/6] Extended xstats API in ethdev library to allow grouping of stats Thomas Monjalon
2017-05-01 21:49   ` Thomas Monjalon
2017-05-01 21:50 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).