This patch adds support for configuring the priority of offload providers. When multiple providers exist and support the same port, the 'hw-offload-priority' option allows specifying the order in which they are tried.
Signed-off-by: Eelco Chaudron <[email protected]> --- lib/dpif-offload.c | 125 ++++++++++++++++++++++++++++++++++------ lib/dpif-offload.h | 4 ++ tests/ofproto-dpif.at | 37 +++++++++++- tests/ofproto-macros.at | 9 ++- vswitchd/bridge.c | 2 + vswitchd/vswitch.xml | 18 ++++++ 6 files changed, 174 insertions(+), 21 deletions(-) diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c index 6c119f0ff..dff951b4f 100644 --- a/lib/dpif-offload.c +++ b/lib/dpif-offload.c @@ -43,10 +43,14 @@ static const struct dpif_offload_class *base_dpif_offload_classes[] = { #ifdef DPDK_NETDEV &dpif_offload_rte_flow_class, #endif + /* If you add an offload class to this structure, make sure you also + * update the dpif_offload_provider_priority_list below. */ &dpif_offload_dummy_class, &dpif_offload_dummy_x_class, }; +static char *dpif_offload_provider_priority_list = "tc,rte_flow,dummy,dummy_x"; + static int dpif_offload_register_provider__(const struct dpif_offload_class *class) OVS_REQUIRES(dpif_offload_mutex) @@ -125,6 +129,12 @@ dpif_offload_show_classes(struct unixctl_conn *conn, int argc OVS_UNUSED, void dp_offload_initialize(void) { + static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; + + if (!ovsthread_once_start(&once)) { + return; + } + unixctl_command_register("dpif/offload/classes", NULL, 0, 0, dpif_offload_show_classes, NULL); @@ -134,6 +144,7 @@ dp_offload_initialize(void) dpif_offload_register_provider(base_dpif_offload_classes[i]); } + ovsthread_once_done(&once); } static struct dp_offload* @@ -186,9 +197,12 @@ static int dpif_offload_attach_providers_(struct dpif *dpif) OVS_REQUIRES(dpif_offload_mutex) { + struct ovs_list provider_list = OVS_LIST_INITIALIZER(&provider_list); const char *dpif_type_str = dpif_normalize_type(dpif_type(dpif)); struct dp_offload *dp_offload; + struct dpif_offload *offload; struct shash_node *node; + char *tokens, *saveptr; /* Allocate and attach dp_offload to dpif. */ dp_offload = xmalloc(sizeof *dp_offload); @@ -198,36 +212,71 @@ dpif_offload_attach_providers_(struct dpif *dpif) ovs_list_init(&dp_offload->offload_providers); shash_add(&dpif_offload_providers, dp_offload->dpif_name, dp_offload); - /* Attach all the providers supporting this dpif type. */ + /* Open all the providers supporting this dpif type. */ SHASH_FOR_EACH (node, &dpif_offload_classes) { const struct dpif_offload_class *class = node->data; + for (size_t i = 0; class->supported_dpif_types[i] != NULL; i++) { if (!strcmp(class->supported_dpif_types[i], dpif_type_str)) { - struct dpif_offload *offload; - int error; - - error = class->open(class, dpif, &offload); - if (!error) { - - error = dpif_offload_attach_provider_to_dp_offload( - dp_offload, offload); - if (error) { - VLOG_WARN("failed to add dpif offload provider " - "%s to %s: %s", - class->type, dpif_name(dpif), - ovs_strerror(error)); - class->close(offload); - } - } else { + int error = class->open(class, dpif, &offload); + if (error) { VLOG_WARN("failed to initialize dpif offload provider " "%s for %s: %s", class->type, dpif_name(dpif), ovs_strerror(error)); + } else { + ovs_list_push_back(&provider_list, + &offload->dpif_list_node); + } + break; + } + } + } + + /* Attach all the providers based on the priority list. */ + tokens = xstrdup(dpif_offload_provider_priority_list); + + for (char *name = strtok_r(tokens, ",", &saveptr); + name; + name = strtok_r(NULL, ",", &saveptr)) { + + LIST_FOR_EACH_SAFE (offload, dpif_list_node, &provider_list) { + + if (!strcmp(name, offload->class->type)) { + int error; + + ovs_list_remove(&offload->dpif_list_node); + error = dpif_offload_attach_provider_to_dp_offload(dp_offload, + offload); + if (error) { + VLOG_WARN( + "failed to add dpif offload provider %s to %s: %s", + offload->class->type, dpif_name(dpif), + ovs_strerror(error)); + + offload->class->close(offload); } break; } } } + free(tokens); + + /* Add remaining entries in order. */ + LIST_FOR_EACH_SAFE (offload, dpif_list_node, &provider_list) { + int error; + + ovs_list_remove(&offload->dpif_list_node); + error = dpif_offload_attach_provider_to_dp_offload(dp_offload, + offload); + if (error) { + VLOG_WARN("failed to add dpif offload provider %s to %s: %s", + offload->class->type, dpif_name(dpif), + ovs_strerror(error)); + + offload->class->close(offload); + } + } /* Attach dp_offload to dpif. */ ovsrcu_set(&dpif->dp_offload, dp_offload); @@ -391,3 +440,45 @@ dpif_offload_dump_done(struct dpif_offload_dump *dump) { return dump->error == EOF ? 0 : dump->error; } + +void +dpif_offload_set_global_cfg(const struct smap *other_cfg) +{ + static struct ovsthread_once init_once = OVSTHREAD_ONCE_INITIALIZER; + const char *priority = smap_get(other_cfg, "hw-offload-priority"); + + /* The 'hw-offload-priority' parameter can only be set at startup, + * any successive change needs a restart. */ + if (ovsthread_once_start(&init_once)) { + /* Initialize the dpif-offload layer in case it's not yet initialized + * at the first invocation of setting the configuration. */ + dp_offload_initialize(); + + /* If priority is not set keep the default value. */ + if (priority) { + char *tokens = xstrdup(priority); + char *saveptr; + + dpif_offload_provider_priority_list = xstrdup(priority); + + /* Log a warning for unknown offload providers. */ + for (char *name = strtok_r(tokens, ",", &saveptr); + name; + name = strtok_r(NULL, ",", &saveptr)) { + + if (!shash_find(&dpif_offload_classes, name)) { + VLOG_WARN("'hw-offload-priority' configuration has an " + "unknown type; %s", name); + } + } + free(tokens); + } + ovsthread_once_done(&init_once); + } else { + if (priority && strcmp(priority, + dpif_offload_provider_priority_list)) { + VLOG_INFO_ONCE("'hw-offload-priority' configuration changed; " + "restart required"); + } + } +} diff --git a/lib/dpif-offload.h b/lib/dpif-offload.h index 66de104e8..5d9b46d10 100644 --- a/lib/dpif-offload.h +++ b/lib/dpif-offload.h @@ -30,6 +30,10 @@ struct dpif_offload_dump { void *state; }; + +/* Global functions. */ +void dpif_offload_set_global_cfg(const struct smap *other_cfg); + /* Per dpif specific functions. */ void dpif_offload_init(struct dpif_offload *, diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 79d9d683e..c941b3cf7 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -10075,6 +10075,41 @@ AT_SETUP([ofproto-dpif - offload - ovs-appctl dpif/offload/]) AT_KEYWORDS([dpif-offload]) OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy]) +AT_CHECK([ovs-appctl dpif/offload/show], [0], [dnl +dummy@ovs-dummy: + dummy + dummy_x +]) + +AT_CHECK([ovs-appctl --format json --pretty dpif/offload/show], [0], [dnl +{ + "dummy@ovs-dummy": { + "providers": [[ + "dummy", + "dummy_x"]]}} +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([ofproto-dpif - offload - priority order - warning]) +AT_KEYWORDS([dpif-offload]) +OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy], [], [], + [], [-- set Open_vSwitch . other_config:hw-offload-priority=none,dummy]) + +OVS_WAIT_UNTIL( + [grep "'hw-offload-priority' configuration has an unknown type; none" \ + ovs-vswitchd.log]) + +OVS_VSWITCHD_STOP( + ["/'hw-offload-priority' configuration has an unknown type;/d"]) +AT_CLEANUP + +AT_SETUP([ofproto-dpif - offload - priority order]) +AT_KEYWORDS([dpif-offload]) +OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy], [], [], + [], [-- set Open_vSwitch . other_config:hw-offload-priority=dummy_x,dummy]) + AT_CHECK([ovs-appctl dpif/offload/show], [0], [dnl dummy@ovs-dummy: dummy_x @@ -10089,7 +10124,7 @@ AT_CHECK([ovs-appctl --format json --pretty dpif/offload/show], [0], [dnl "dummy"]]}} ]) -OVS_VSWITCHD_STOP +OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP dnl ---------------------------------------------------------------------- diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index 3cd49d7a7..71cd2aed0 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -227,11 +227,12 @@ m4_define([_OVS_VSWITCHD_START], /netdev_offload|INFO|netdev: Flow API Enabled/d /probe tc:/d /setting extended ack support failed/d -/tc: Using policy/d']]) +/tc: Using policy/d +/'"'"'hw-offload-priority'"'"' configuration has an unknown type;/d']]) ]) # OVS_VSWITCHD_START([vsctl-args], [vsctl-output], [=override], -# [vswitchd-aux-args]) +# [vswitchd-aux-args] [dbinit-aux-args]) # # Creates a database and starts ovsdb-server, starts ovs-vswitchd # connected to that database, calls ovs-vsctl to create a bridge named @@ -248,7 +249,9 @@ m4_define([_OVS_VSWITCHD_START], # 'vswitchd-aux-args' provides a way to pass extra command line arguments # to ovs-vswitchd m4_define([OVS_VSWITCHD_START], - [_OVS_VSWITCHD_START([--enable-dummy$3 --disable-system --disable-system-route $4]) + [_OVS_VSWITCHD_START( + [--enable-dummy$3 --disable-system --disable-system-route $4], + [$5]) AT_CHECK([add_of_br 0 $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) ]) diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 456b784c0..1c8f06168 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -28,6 +28,7 @@ #include "daemon.h" #include "dirs.h" #include "dpif.h" +#include "dpif-offload.h" #include "dpdk.h" #include "hash.h" #include "openvswitch/hmap.h" @@ -3397,6 +3398,7 @@ bridge_run(void) cfg = ovsrec_open_vswitch_first(idl); if (cfg) { + dpif_offload_set_global_cfg(&cfg->other_config); netdev_set_flow_api_enabled(&cfg->other_config); dpdk_init(&cfg->other_config); userspace_tso_init(&cfg->other_config); diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index 3dcf74402..1c298b10e 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -254,6 +254,24 @@ </p> </column> + <column name="other_config" key="hw-offload-priority" + type='{"type": "string"}'> + <p> + This configuration sets the order of the hardware offload providers + to try when multiple exist for a given datapath implementation. The + The argument provided should be a list of comma-separated hardware + offload provider names. + </p> + <p> + If implementations are missing from the list, they are added at the + end in undefined order. + </p> + <p> + The default value is <code>none</code>. Changing this value requires + restarting the daemon if hardware offload is already enabled. + </p> + </column> + <column name="other_config" key="n-offload-threads" type='{"type": "integer", "minInteger": 1, "maxInteger": 10}'> <p> -- 2.50.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
