* [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, ð_stats);
+ dev = &rte_eth_devices[port_id];
- /* global stats */
- for (i = 0; i < RTE_NB_STATS; i++) {
- stats_ptr = RTE_PTR_ADD(ð_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(ð_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(ð_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, ð_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(ð_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(ð_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(ð_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, ð_stats);
+
+ /* global stats */
+ for (i = 0; i < RTE_NB_STATS; i++) {
+ stats_ptr = RTE_PTR_ADD(ð_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(ð_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(ð_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 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