Move the existing API for querying global datapath-specific
offload statistics from dpif to dpif-offload.

Note that this patch will temporarily remove the global dpif
offload stats feature for rte offload, but it will be
reintroduced in the next patch.

Signed-off-by: Eelco Chaudron <echau...@redhat.com>
---
 include/openvswitch/netdev.h |  1 +
 lib/dpctl.c                  | 42 +++++++++++++---
 lib/dpif-netdev.c            | 96 ------------------------------------
 lib/dpif-netlink.c           |  1 -
 lib/dpif-offload-dummy.c     | 55 ++++++++++++++-------
 lib/dpif-offload-provider.h  |  8 +++
 lib/dpif-offload.c           | 36 ++++++++++++++
 lib/dpif-offload.h           | 12 +++++
 lib/dpif-provider.h          |  7 ---
 lib/dpif.c                   |  8 ---
 lib/dpif.h                   |  8 ---
 lib/netdev.c                 |  2 +
 tests/ofproto-dpif.at        | 18 +++++++
 13 files changed, 148 insertions(+), 146 deletions(-)

diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
index 83e8633dd..f730b4f97 100644
--- a/include/openvswitch/netdev.h
+++ b/include/openvswitch/netdev.h
@@ -101,6 +101,7 @@ struct netdev_custom_counter {
 
 /* Structure representation of custom statistics */
 struct netdev_custom_stats {
+    char *label; /* Optional label/description for this set of counters. */
     uint16_t size;
     struct netdev_custom_counter *counters;
 };
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 1411c5a31..074e73922 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -33,6 +33,7 @@
 #include "dirs.h"
 #include "dpctl.h"
 #include "dpif.h"
+#include "dpif-offload.h"
 #include "dpif-provider.h"
 #include "openvswitch/dynamic-string.h"
 #include "flow.h"
@@ -1597,34 +1598,59 @@ dpctl_del_flows(int argc, const char *argv[], struct 
dpctl_params *dpctl_p)
     return error;
 }
 
+static int
+compare_custom_stats(const void *a, const void *b)
+{
+    const struct netdev_custom_stats *sa = a;
+    const struct netdev_custom_stats *sb = b;
+
+    return sa->label && sb->label ? strcmp(sa->label, sb->label)
+                                  : (sa->label != NULL) - (sb->label != NULL);
+}
+
 static int
 dpctl_offload_stats_show(int argc, const char *argv[],
                          struct dpctl_params *dpctl_p)
 {
-    struct netdev_custom_stats stats;
+    struct netdev_custom_stats *stats_array;
     struct dpif *dpif;
+    size_t n_stats;
     int error;
-    size_t i;
 
     error = opt_dpif_open(argc, argv, dpctl_p, 2, &dpif);
     if (error) {
         return error;
     }
 
-    memset(&stats, 0, sizeof(stats));
-    error = dpif_offload_stats_get(dpif, &stats);
+    error = dpif_offload_stats_get(dpif, &stats_array, &n_stats);
     if (error) {
         dpctl_error(dpctl_p, error, "retrieving offload statistics");
         goto close_dpif;
     }
 
+    if (stats_array && n_stats) {
+        qsort(stats_array, n_stats, sizeof *stats_array, compare_custom_stats);
+    }
+
     dpctl_print(dpctl_p, "HW Offload stats:\n");
-    for (i = 0; i < stats.size; i++) {
-        dpctl_print(dpctl_p, "   %s: %6" PRIu64 "\n",
-                    stats.counters[i].name, stats.counters[i].value);
+    for (size_t i = 0; i < n_stats; i++) {
+        struct netdev_custom_stats *stats = &stats_array[i];
+
+        if (!stats) {
+            continue;
+        }
+
+        if (stats->label) {
+            dpctl_print(dpctl_p, "  %s:\n", stats->label);
+        }
+        for (size_t j = 0; j < stats->size; j++) {
+            dpctl_print(dpctl_p, "    %s: %6" PRIu64 "\n",
+                        stats->counters[j].name, stats->counters[j].value);
+        }
+        netdev_free_custom_stats_counters(stats);
     }
 
-    netdev_free_custom_stats_counters(&stats);
+    free(stats_array);
 
 close_dpif:
     dpif_close(dpif);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d1fc0223c..813090995 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4735,101 +4735,6 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op 
**ops, size_t n_ops)
     }
 }
 
-static int
-dpif_netdev_offload_stats_get(struct dpif *dpif,
-                              struct netdev_custom_stats *stats)
-{
-    enum {
-        DP_NETDEV_HW_OFFLOADS_STATS_ENQUEUED,
-        DP_NETDEV_HW_OFFLOADS_STATS_INSERTED,
-        DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_MEAN,
-        DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_STDDEV,
-        DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_MEAN,
-        DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV,
-    };
-    struct {
-        const char *name;
-        uint64_t total;
-    } hwol_stats[] = {
-        [DP_NETDEV_HW_OFFLOADS_STATS_ENQUEUED] =
-            { "                Enqueued offloads", 0 },
-        [DP_NETDEV_HW_OFFLOADS_STATS_INSERTED] =
-            { "                Inserted offloads", 0 },
-        [DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_MEAN] =
-            { "  Cumulative Average latency (us)", 0 },
-        [DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_STDDEV] =
-            { "   Cumulative Latency stddev (us)", 0 },
-        [DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_MEAN] =
-            { " Exponential Average latency (us)", 0 },
-        [DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV] =
-            { "  Exponential Latency stddev (us)", 0 },
-    };
-
-    unsigned int nb_thread;
-    unsigned int tid;
-    size_t i;
-
-    if (!dpif_offload_is_offload_enabled()) {
-        return EINVAL;
-    }
-
-    nb_thread = rte_flow_offload_thread_nb();
-    if (!nb_thread) {
-        return EINVAL;
-    }
-
-    /* nb_thread counters for the overall total as well. */
-    stats->size = ARRAY_SIZE(hwol_stats) * (nb_thread + 1);
-    stats->counters = xcalloc(stats->size, sizeof *stats->counters);
-
-    for (tid = 0; tid < nb_thread; tid++) {
-        uint64_t counts[ARRAY_SIZE(hwol_stats)];
-        size_t idx = ((tid + 1) * ARRAY_SIZE(hwol_stats));
-
-        memset(counts, 0, sizeof counts);
-        if (dp_offload_threads != NULL) {
-            atomic_read_relaxed(&dp_offload_threads[tid].enqueued_item,
-                                &counts[DP_NETDEV_HW_OFFLOADS_STATS_ENQUEUED]);
-
-            counts[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_MEAN] =
-                mov_avg_cma(&dp_offload_threads[tid].cma);
-            counts[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_STDDEV] =
-                mov_avg_cma_std_dev(&dp_offload_threads[tid].cma);
-
-            counts[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_MEAN] =
-                mov_avg_ema(&dp_offload_threads[tid].ema);
-            counts[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV] =
-                mov_avg_ema_std_dev(&dp_offload_threads[tid].ema);
-        }
-
-        for (i = 0; i < ARRAY_SIZE(hwol_stats); i++) {
-            snprintf(stats->counters[idx + i].name,
-                     sizeof(stats->counters[idx + i].name),
-                     "  [%3u] %s", tid, hwol_stats[i].name);
-            stats->counters[idx + i].value = counts[i];
-            hwol_stats[i].total += counts[i];
-        }
-    }
-
-    /* Do an average of the average for the aggregate. */
-    hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_MEAN].total /= nb_thread;
-    hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_CMA_STDDEV].total /= nb_thread;
-    hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_MEAN].total /= nb_thread;
-    hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_LAT_EMA_STDDEV].total /= nb_thread;
-
-    /* Get the total offload count. */
-    hwol_stats[DP_NETDEV_HW_OFFLOADS_STATS_INSERTED].total =
-        dpif_offload_flow_get_n_offloaded(dpif);
-
-    for (i = 0; i < ARRAY_SIZE(hwol_stats); i++) {
-        snprintf(stats->counters[i].name, sizeof(stats->counters[i].name),
-                 "  Total %s", hwol_stats[i].name);
-        stats->counters[i].value = hwol_stats[i].total;
-    }
-
-    return 0;
-}
-
 /* Enable or Disable PMD auto load balancing. */
 static void
 set_pmd_auto_lb(struct dp_netdev *dp, bool state, bool always_log)
@@ -9985,7 +9890,6 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_flow_dump_thread_destroy,
     dpif_netdev_flow_dump_next,
     dpif_netdev_operate,
-    dpif_netdev_offload_stats_get,
     NULL,                       /* recv_set */
     NULL,                       /* handlers_set */
     dpif_netdev_number_handlers_required,
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 222dc8336..456da0ff5 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4038,7 +4038,6 @@ const struct dpif_class dpif_netlink_class = {
     dpif_netlink_flow_dump_thread_destroy,
     dpif_netlink_flow_dump_next,
     dpif_netlink_operate,
-    NULL,                       /* offload_stats_get */
     dpif_netlink_recv_set,
     dpif_netlink_handlers_set,
     dpif_netlink_number_handlers_required,
diff --git a/lib/dpif-offload-dummy.c b/lib/dpif-offload-dummy.c
index a289430e4..32360737f 100644
--- a/lib/dpif-offload-dummy.c
+++ b/lib/dpif-offload-dummy.c
@@ -273,6 +273,24 @@ dpif_offload_dummy_get_debug(const struct dpif_offload 
*offload, struct ds *ds,
     }
 }
 
+static int
+dpif_offload_dummy_get_global_stats(const struct dpif_offload *offload_,
+                                    struct netdev_custom_stats *stats)
+{
+    struct dpif_offload_dummy *offload = dpif_offload_dummy_cast(offload_);
+
+    /* Add a single counter telling how many ports we are servicing. */
+    stats->label = xstrdup(dpif_offload_name(offload_));
+    stats->size = 1;
+    stats->counters = xmalloc(sizeof(struct netdev_custom_counter) * 1);
+    stats->counters[0].value = dpif_offload_port_mgr_port_count(
+        offload->port_mgr);
+    ovs_strzcpy(stats->counters[0].name, "Offloaded port count",
+                sizeof stats->counters[0].name);
+
+    return 0;
+}
+
 static bool
 dpif_offload_dummy_can_offload(struct dpif_offload *dpif_offload OVS_UNUSED,
                                struct netdev *netdev)
@@ -280,24 +298,25 @@ dpif_offload_dummy_can_offload(struct dpif_offload 
*dpif_offload OVS_UNUSED,
     return is_dummy_netdev_class(netdev->netdev_class) ? true : false;
 }
 
-#define DEFINE_DPIF_DUMMY_CLASS(NAME, TYPE_STR)                 \
-    struct dpif_offload_class NAME = {                          \
-        .type = TYPE_STR,                                       \
-        .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY,                 \
-        .supported_dpif_types = (const char *const[]) {         \
-            "dummy",                                            \
-            NULL},                                              \
-        .open = dpif_offload_dummy_open,                        \
-        .close = dpif_offload_dummy_close,                      \
-        .set_config = dpif_offload_dummy_set_config,            \
-        .get_debug = dpif_offload_dummy_get_debug,              \
-        .can_offload = dpif_offload_dummy_can_offload,          \
-        .port_add = dpif_offload_dummy_port_add,                \
-        .port_del = dpif_offload_dummy_port_del,                \
-        .port_dump_start = dpif_offload_dummy_port_dump_start,  \
-        .port_dump_next = dpif_offload_dummy_port_dump_next,    \
-        .port_dump_done = dpif_offload_dummy_port_dump_done,    \
-        .get_netdev = dpif_offload_dummy_get_netdev,            \
+#define DEFINE_DPIF_DUMMY_CLASS(NAME, TYPE_STR)                  \
+    struct dpif_offload_class NAME = {                           \
+        .type = TYPE_STR,                                        \
+        .impl_type = DPIF_OFFLOAD_IMPL_HW_ONLY,                  \
+        .supported_dpif_types = (const char *const[]) {          \
+            "dummy",                                             \
+            NULL},                                               \
+        .open = dpif_offload_dummy_open,                         \
+        .close = dpif_offload_dummy_close,                       \
+        .set_config = dpif_offload_dummy_set_config,             \
+        .get_debug = dpif_offload_dummy_get_debug,               \
+        .get_global_stats = dpif_offload_dummy_get_global_stats, \
+        .can_offload = dpif_offload_dummy_can_offload,           \
+        .port_add = dpif_offload_dummy_port_add,                 \
+        .port_del = dpif_offload_dummy_port_del,                 \
+        .port_dump_start = dpif_offload_dummy_port_dump_start,   \
+        .port_dump_next = dpif_offload_dummy_port_dump_next,     \
+        .port_dump_done = dpif_offload_dummy_port_dump_done,     \
+        .get_netdev = dpif_offload_dummy_get_netdev,             \
     }
 
 DEFINE_DPIF_DUMMY_CLASS(dpif_offload_dummy_class, "dummy");
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
index deee44a61..d65d17947 100644
--- a/lib/dpif-offload-provider.h
+++ b/lib/dpif-offload-provider.h
@@ -132,6 +132,14 @@ struct dpif_offload_class {
     void (*get_debug)(const struct dpif_offload *offload, struct ds *ds,
                       struct json *json);
 
+    /* Get hardware offload activity counters from the data plane.
+     * These counters are not interface offload statistics, but rather a status
+     * report of hardware offload management: how many offloads are currently
+     * waiting, inserted, etc.  If this function returns an error, the 'stats'
+     * structure should not be touched, that is, it remains uninitialized. */
+    int (*get_global_stats)(const struct dpif_offload *offload,
+                            struct netdev_custom_stats *stats);
+
     /* Verifies whether the offload provider supports offloading flows for the
      * given 'netdev'.  Returns 'false' if the provider lacks the capabilities
      * to offload on this port, otherwise returns 'true'. */
diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
index 17d63c88d..0f2c02c84 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -424,6 +424,42 @@ bool dpif_offload_get_debug(const struct dpif_offload 
*offload,
     return true;
 }
 
+int dpif_offload_stats_get(struct dpif *dpif,
+                           struct netdev_custom_stats **stats_,
+                           size_t *n_stats)
+{
+    struct dp_offload *dp_offload = dpif_offload_get_dp_offload(dpif);
+    struct netdev_custom_stats *stats;
+    struct dpif_offload *offload;
+    size_t n_providers;
+    int error = 0;
+
+    *n_stats = 0;
+    if (!dp_offload || !dpif_offload_is_offload_enabled()) {
+        *stats_ = NULL;
+        return 0;
+    }
+
+    n_providers = ovs_list_size(&dp_offload->offload_providers);
+    stats = xcalloc(n_providers, sizeof(struct netdev_custom_stats));
+
+    LIST_FOR_EACH (offload, dpif_list_node, &dp_offload->offload_providers) {
+        if (!offload->class->get_global_stats) {
+            continue;
+        }
+
+        error = offload->class->get_global_stats(offload, &stats[(*n_stats)]);
+        if (error) {
+            break;
+        }
+
+        (*n_stats)++;
+    }
+
+    *stats_ = stats;
+    return error;
+}
+
 bool
 dpif_offload_is_offload_enabled(void)
 {
diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h
index 305b44188..279c7898b 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -138,6 +138,18 @@ int dpif_offload_port_dump_done(struct 
dpif_offload_port_dump *);
           : (dpif_offload_port_dump_done(DUMP), false));           \
         )
 
+/* Queries the datapath for hardware offload stats.
+ *
+ * On success, '*stats' will point to a heap-allocated array of
+ * 'netdev_custom_stats' structures, and '*n_stats' will be set to the
+ * number of statistics returned.
+ *
+ * The caller is responsible for freeing the memory using
+ * 'netdev_free_custom_stats_counters()' on each 'stats' object, and
+ * call free() on 'stats'. */
+int dpif_offload_stats_get(struct dpif *, struct netdev_custom_stats **stats,
+                           size_t *n_stats);
+
 
 /* Netdev specific function, which can be used in the fast path. */
 bool dpif_offload_netdev_same_offload(const struct netdev *,
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index e1c91caff..18878fd8e 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -366,13 +366,6 @@ struct dpif_class {
      * dpif_op. */
     void (*operate)(struct dpif *dpif, struct dpif_op **ops, size_t n_ops);
 
-    /* Get hardware-offloads activity counters from a dataplane.
-     * Those counters are not offload statistics (which are accessible through
-     * netdev statistics), but a status of hardware offload management:
-     * how many offloads are currently waiting, inserted, etc. */
-    int (*offload_stats_get)(struct dpif *dpif,
-                             struct netdev_custom_stats *stats);
-
     /* Enables or disables receiving packets with dpif_recv() for 'dpif'.
      * Turning packet receive off and then back on is allowed to change Netlink
      * PID assignments (see ->port_get_pid()).  The client is responsible for
diff --git a/lib/dpif.c b/lib/dpif.c
index 342de99b5..bbc2fd873 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1468,14 +1468,6 @@ dpif_operate(struct dpif *dpif, struct dpif_op **ops, 
size_t n_ops,
     }
 }
 
-int dpif_offload_stats_get(struct dpif *dpif,
-                           struct netdev_custom_stats *stats)
-{
-    return (dpif->dpif_class->offload_stats_get
-            ? dpif->dpif_class->offload_stats_get(dpif, stats)
-            : EOPNOTSUPP);
-}
-
 /* Returns a string that represents 'type', for use in log messages. */
 const char *
 dpif_upcall_type_to_string(enum dpif_upcall_type type)
diff --git a/lib/dpif.h b/lib/dpif.h
index c1b0daa3e..3e6a34a25 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -793,14 +793,6 @@ struct dpif_op {
 void dpif_operate(struct dpif *, struct dpif_op **ops, size_t n_ops,
                   enum dpif_offload_type);
 
-/* Queries the datapath for hardware offloads stats.
- *
- * Statistics are written in 'stats' following the 'netdev_custom_stats'
- * format. They are allocated on the heap and must be freed by the caller,
- * using 'netdev_free_custom_stats_counters'.
- */
-int dpif_offload_stats_get(struct dpif *dpif,
-                           struct netdev_custom_stats *stats);
 
 /* Upcalls. */
 
diff --git a/lib/netdev.c b/lib/netdev.c
index c5ebd58f0..dc3e6faac 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2415,8 +2415,10 @@ netdev_free_custom_stats_counters(struct 
netdev_custom_stats *custom_stats)
 {
     if (custom_stats) {
         if (custom_stats->counters) {
+            free(custom_stats->label);
             free(custom_stats->counters);
             custom_stats->counters = NULL;
+            custom_stats->label = NULL;
             custom_stats->size = 0;
         }
     }
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 775b7b823..2d3a2b77c 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -10217,6 +10217,24 @@ AT_CHECK([ovs-appctl --format json --pretty 
dpif/offload/show], [0], [dnl
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - offload - ovs-appctl dpctl/offload-stats-show])
+AT_KEYWORDS([dpif-offload])
+OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy], [], [],
+  [], [-- set Open_vSwitch . other_config:hw-offload=true])
+
+ovs-appctl dpctl/offload-stats-show
+
+AT_CHECK([ovs-appctl dpctl/offload-stats-show], [0], [dnl
+HW Offload stats:
+  dummy[[dummy@ovs-dummy]]:
+    Offloaded port count:      2
+  dummy_x[[dummy@ovs-dummy]]:
+    Offloaded port count:      0
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 dnl ----------------------------------------------------------------------
 AT_BANNER([ofproto-dpif -- megaflows])
 
-- 
2.50.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to