Signed-off-by: Eelco Chaudron <[email protected]>
---
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 | 42 ++++++++++++++++
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 | 16 ++++++
13 files changed, 152 insertions(+), 146 deletions(-)
diff --git a/include/openvswitch/netdev.h b/include/openvswitch/netdev.h
index 8212ffb00..01161bdba 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 586b589a3..2aeef471f 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 = dpdk_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)
@@ -9990,7 +9895,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 d384e8f09..d76e2ccfb 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4071,7 +4071,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 3a0d51295..ce849e3dd 100644
--- a/lib/dpif-offload-dummy.c
+++ b/lib/dpif-offload-dummy.c
@@ -272,6 +272,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)
@@ -279,24 +297,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 29189d4b1..1e63e1199 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 198b36b0c..8a42d5f13 100644
--- a/lib/dpif-offload.c
+++ b/lib/dpif-offload.c
@@ -435,6 +435,48 @@ dpif_offload_get_debug(const struct dpif_offload *offload,
struct ds *ds,
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) {
+ for (int i = 0; i < *n_stats; i++) {
+ netdev_free_custom_stats_counters(&stats[i]);
+ }
+ *n_stats = 0;
+ free(stats);
+ stats = NULL;
+ 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 213897ca2..2ca790568 100644
--- a/lib/dpif-offload.h
+++ b/lib/dpif-offload.h
@@ -134,6 +134,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 089043ad4..bda11a41b 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1470,14 +1470,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 6b0ea8e11..ee5d68c99 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2440,6 +2440,8 @@ netdev_free_custom_stats_counters(struct
netdev_custom_stats *custom_stats)
custom_stats->counters = NULL;
custom_stats->size = 0;
}
+ free(custom_stats->label);
+ custom_stats->label = NULL;
}
}
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 10596a968..1099c6f71 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -10229,6 +10229,22 @@ 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])
+
+AT_CHECK([ovs-appctl dpctl/offload-stats-show], [0], [dnl
+HW Offload stats:
+ dummy[[dummy@ovs-dummy]]:
+ Offloaded port count: 3
+ dummy_x[[dummy@ovs-dummy]]:
+ Offloaded port count: 0
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
dnl ----------------------------------------------------------------------
AT_BANNER([ofproto-dpif -- megaflows])
