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