[ovs-dev] [RFC PATCH] netdev-dpdk: Expose per rxq/txq basic statistics.

[David Marchand]

When troubleshooting multiqueue setups, having per queue statistics helps
checking packets repartition in rx and tx queues.

Per queue statistics are exported by most DPDK drivers (with capability
RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS). But since OVS only filters statistics
it exposes, there is nothing to request in DPDK API.

Extend existing filter with a regular expression.
string_ends_with() helper is not used anymore, and removed as a
consequence.

Querying statistics with
$ ovs-vsctl get interface dpdk0 statistics | \
  sed -e 's#[{}]##g' -e 's#, #\n#g'

and comparing gives:
@@ -13,7 +13,12 @@
 rx_phy_crc_errors=0
 rx_phy_in_range_len_errors=0
 rx_phy_symbol_errors=0
+rx_q0_bytes=0
 rx_q0_errors=0
+rx_q0_packets=0
+rx_q1_bytes=0
+rx_q1_errors=0
+rx_q1_packets=0
 rx_wqe_errors=0
 tx_broadcast_packets=0
 tx_bytes=0
@@ -27,3 +32,13 @@
 tx_pp_rearm_queue_errors=0
 tx_pp_timestamp_future_errors=0
 tx_pp_timestamp_past_errors=0
+tx_q0_bytes=0
+tx_q0_packets=0
+tx_q1_bytes=0
+tx_q1_packets=0
+tx_q2_bytes=0
+tx_q2_packets=0
+tx_q3_bytes=0
+tx_q3_packets=0
+tx_q4_bytes=0
+tx_q4_packets=0

Signed-off-by: David Marchand <david.marchand at redhat.com>
---
Sending this as a RFC for feedback.
vhost user ports are left untouched for now, but could be added in the
future.

---
 lib/netdev-dpdk.c | 32 ++++++++++++++++++++++++++------
 lib/util.c        | 13 -------------
 lib/util.h        |  2 --
 3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ca92c947a2..12447cc4b2 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -18,6 +18,7 @@
 #include "netdev-dpdk.h"
 
 #include <errno.h>
+#include <regex.h>
 #include <signal.h>
 #include <stdlib.h>
 #include <string.h>
@@ -1583,6 +1584,13 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
     return dev->rte_xstats_names[id].name;
 }
 
+/* We filter out everything except per rxq/txq basic stats, and dropped,
+ * error and management counters.
+ * Note: rx_qX_errors is handled by the _errors$ pattern.
+ */
+#define DPDK_STATS_REGEX_FILTER \
+    "(^(rx_q|tx_q)[0-9]*_(packets|bytes)$|_errors$|_dropped$|_management_)"
+
 static bool
 netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
     OVS_REQUIRES(dev->mutex)
@@ -1617,6 +1625,20 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
                                             sizeof *dev->rte_xstats_names);
 
             if (dev->rte_xstats_names) {
+                int err;
+                regex_t r;
+
+                err = regcomp(&r, DPDK_STATS_REGEX_FILTER, REG_EXTENDED);
+                if (err != 0) {
+                    size_t errbuf_size = regerror(err, &r, NULL, 0);
+                    char *errbuf = xcalloc(1, errbuf_size);
+
+                    regerror(err, &r, errbuf, errbuf_size);
+                    VLOG_DBG("Could not setup stats regexp: %s", errbuf);
+                    free(errbuf);
+                    goto out;
+                }
+
                 /* Retreive xstats names */
                 rte_xstats_len =
                         rte_eth_xstats_get_names(dev->port_id,
@@ -1626,10 +1648,12 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
                 if (rte_xstats_len < 0) {
                     VLOG_WARN("Cannot get XSTATS names for port: "
                               DPDK_PORT_ID_FMT, dev->port_id);
+                    regfree(&r);
                     goto out;
                 } else if (rte_xstats_len != dev->rte_xstats_names_size) {
                     VLOG_WARN("XSTATS size doesn't match for port: "
                               DPDK_PORT_ID_FMT, dev->port_id);
+                    regfree(&r);
                     goto out;
                 }
 
@@ -1648,12 +1672,7 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
                     for (uint32_t i = 0; i < rte_xstats_len; i++) {
                         id = rte_xstats[i].id;
                         name = netdev_dpdk_get_xstat_name(dev, id);
-                        /* We need to filter out everything except
-                         * dropped, error and management counters */
-                        if (string_ends_with(name, "_errors") ||
-                            strstr(name, "_management_") ||
-                            string_ends_with(name, "_dropped")) {
-
+                        if (regexec(&r, name, 0, NULL, 0) == 0) {
                             dev->rte_xstats_ids[xstats_no] = id;
                             xstats_no++;
                         }
@@ -1666,6 +1685,7 @@ netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
                 }
 
                 free(rte_xstats);
+                regfree(&r);
             }
         }
     } else {
diff --git a/lib/util.c b/lib/util.c
index 1195c79821..34755a4dad 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -405,19 +405,6 @@ ovs_strzcpy(char *dst, const char *src, size_t size)
     }
 }
 
-/*
- * Returns true if 'str' ends with given 'suffix'.
- */
-int
-string_ends_with(const char *str, const char *suffix)
-{
-    int str_len = strlen(str);
-    int suffix_len = strlen(suffix);
-
-    return (str_len >= suffix_len) &&
-           (0 == strcmp(str + (str_len - suffix_len), suffix));
-}
-
 /* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' is
  * nonzero, then it is formatted with ovs_retval_to_string() and appended to
  * the message inside parentheses.  Then, terminates with abort().
diff --git a/lib/util.h b/lib/util.h
index aea19d45fa..4ad6e28274 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -183,8 +183,6 @@ void free_cacheline(void *);
 void ovs_strlcpy(char *dst, const char *src, size_t size);
 void ovs_strzcpy(char *dst, const char *src, size_t size);
 
-int string_ends_with(const char *str, const char *suffix);
-
 void *xmalloc_pagealign(size_t) MALLOC_LIKE;
 void free_pagealign(void *);
 void *xmalloc_size_align(size_t, size_t) MALLOC_LIKE;
-- 
2.23.0