From 6f0fbfb81991654f52d0168c356e7d546131c0b3 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Sat, 2 Dec 2023 08:04:16 +0100 Subject: [PATCH] ovs: Support consuming OVS from snap At present netplan hard codes the location of the `ovs-vsctl` binary. This does not work well when trying to integrate with snaps. Determine path of `ovs-vsctl` at runtime by checking for both '/snap/bin/ovs-vsctl' and '/usr/bin/ovs-vsctl' with a preference of the former. This allows making use of the Netplan Open vSwitch integration with for example the MicroOVN snap (and its consumers such as MicroCloud). TODO: Figure out how to hanlde the ovsdb-server.service unit check TODO: Add test cases that checks the snap workflow Signed-off-by: Frode Nordahl --- netplan_cli/cli/ovs.py | 4 +- src/openvswitch.c | 105 ++++++++++++++++++++++++++++++++--------- src/openvswitch.h | 3 ++ src/util-internal.h | 2 - src/validation.c | 5 +- 5 files changed, 94 insertions(+), 25 deletions(-) diff --git a/netplan_cli/cli/ovs.py b/netplan_cli/cli/ovs.py index 0ba0482b6..8426ec360 100644 --- a/netplan_cli/cli/ovs.py +++ b/netplan_cli/cli/ovs.py @@ -22,7 +22,9 @@ from .utils import systemctl_is_active -OPENVSWITCH_OVS_VSCTL = '/usr/bin/ovs-vsctl' +OPENVSWITCH_OVS_VSCTL = ( + '/snap/bin/ovs-vsctl' if os.path.exists('/snap/bin/ovs-vsctl') else + '/usr/bin/ovs-vsctl') OPENVSWITCH_OVSDB_SERVER_UNIT = 'ovsdb-server.service' # Defaults for non-optional settings, as defined here: # http://www.openvswitch.org/ovs-vswitchd.conf.db.5.pdf diff --git a/src/openvswitch.c b/src/openvswitch.c index d4af8618f..846432e60 100644 --- a/src/openvswitch.c +++ b/src/openvswitch.c @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include #include #include @@ -29,6 +30,42 @@ #include "util.h" #include "util-internal.h" +/* 20 bytes for "/snap/bin/" or "/usr/bin/ovs-vsctl" + \0 */ +static char netplan_openvswitch_ovs_vsctl_path[20]; +#define OPENVSWITCH_OVS_VSCTL "ovs-vsctl" + +#ifndef BINDIR +#define BINDIR "/usr/bin" +#endif + +#ifndef SNAPBINDIR +#define SNAPBINDIR "/snap/bin" +#endif + +void +netplan_openvswitch_vsctl__(void) +{ + if (g_file_test(SNAPBINDIR"/"OPENVSWITCH_OVS_VSCTL, G_FILE_TEST_EXISTS)) { + snprintf(netplan_openvswitch_ovs_vsctl_path, + sizeof netplan_openvswitch_ovs_vsctl_path, + SNAPBINDIR"/"OPENVSWITCH_OVS_VSCTL); + } else { + snprintf(netplan_openvswitch_ovs_vsctl_path, + sizeof netplan_openvswitch_ovs_vsctl_path, + BINDIR"/"OPENVSWITCH_OVS_VSCTL); + } + +} + +const char * +netplan_openvswitch_ovs_vsctl(void) +{ + if (!*netplan_openvswitch_ovs_vsctl_path) { + netplan_openvswitch_vsctl__(); + } + return (const char *) &netplan_openvswitch_ovs_vsctl_path; +} + static gboolean write_ovs_systemd_unit(const char* id, const GString* cmds, const char* rootdir, gboolean physical, gboolean cleanup, const char* dependency, GError** error) { @@ -51,7 +88,8 @@ write_ovs_systemd_unit(const char* id, const GString* cmds, const char* rootdir, g_string_append_printf(s, "After=netplan-ovs-cleanup.service\n"); } else { /* The netplan-ovs-cleanup unit shall not run on systems where Open vSwitch is not installed. */ - g_string_append(s, "ConditionFileIsExecutable=" OPENVSWITCH_OVS_VSCTL "\n"); + g_string_append_printf(s, "ConditionFileIsExecutable=%s\n", + netplan_openvswitch_ovs_vsctl()); } g_string_append(s, "Before=network.target\nWants=network.target\n"); if (dependency) { @@ -127,7 +165,8 @@ write_ovs_tag_setting(const gchar* id, const char* type, const char* col, const if (key) g_string_append_printf(s, "/%s", key); g_string_append_printf(s, "=%s", clean_value); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s", type, id, s->str); + append_systemd_cmd(cmds, "%s set %s %s %s", + netplan_openvswitch_ovs_vsctl(), type, id, s->str); g_string_free(s, TRUE); } @@ -142,8 +181,9 @@ write_ovs_additional_data(GHashTable *data, const char* type, const gchar* id, G while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &value)) { /* XXX: we need to check what happens when an invalid key=value pair gets supplied here. We might want to handle this somehow. */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=%s", - type, id, setting, key, value); + append_systemd_cmd(cmds, "%s set %s %s %s:%s=%s", + netplan_openvswitch_ovs_vsctl(), type, id, setting, + key, value); write_ovs_tag_setting(id, type, setting, key, value, cmds); } } @@ -175,8 +215,9 @@ write_ovs_bond_interfaces(const NetplanState* np_state, const NetplanNetDefiniti return NULL; } - s = g_string_new(OPENVSWITCH_OVS_VSCTL " --may-exist add-bond"); - g_string_append_printf(s, " %s %s", def->bridge, def->id); + s = g_string_new(NULL); + g_string_printf(s, "%s --may-exist add-bond %s %s", + netplan_openvswitch_ovs_vsctl(), def->bridge, def->id); g_hash_table_iter_init(&iter, np_state->netdefs); while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &tmp_nd)) { @@ -202,8 +243,8 @@ static void write_ovs_tag_netplan(const gchar* id, const char* type, GString* cmds) { /* Mark this bridge/port/interface as created by netplan */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=true", - type, id); + append_systemd_cmd(cmds, "%s set %s %s external-ids:netplan=true", + netplan_openvswitch_ovs_vsctl(), type, id); } static gboolean @@ -216,7 +257,8 @@ write_ovs_bond_mode(const NetplanNetDefinition* def, GString* cmds, GError** err !strcmp(def->bond_params.mode, "balance-tcp") || !strcmp(def->bond_params.mode, "balance-slb")) { value = def->bond_params.mode; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Port %s bond_mode=%s", def->id, value); + append_systemd_cmd(cmds, "%s set Port %s bond_mode=%s", + netplan_openvswitch_ovs_vsctl(), def->id, value); write_ovs_tag_setting(def->id, "Port", "bond_mode", NULL, value, cmds); } else { g_set_error(error, NETPLAN_BACKEND_ERROR, NETPLAN_ERROR_VALIDATION, "%s: bond mode '%s' not supported by Open vSwitch\n", @@ -233,7 +275,8 @@ write_ovs_bridge_interfaces(const NetplanState* np_state, const NetplanNetDefini GHashTableIter iter; gchar* key; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-br %s", def->id); + append_systemd_cmd(cmds, "%s --may-exist add-br %s", + netplan_openvswitch_ovs_vsctl(), def->id); g_hash_table_iter_init(&iter, np_state->netdefs); while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &tmp_nd)) { @@ -243,8 +286,9 @@ write_ovs_bridge_interfaces(const NetplanState* np_state, const NetplanNetDefini GString * patch_ports = g_string_new(""); if (tmp_nd->type == NETPLAN_DEF_TYPE_PORT) setup_patch_port(patch_ports, tmp_nd); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-port %s %s%s", - def->id, tmp_nd->id, patch_ports->str); + append_systemd_cmd(cmds, "%s --may-exist add-port %s %s%s", + netplan_openvswitch_ovs_vsctl(), def->id, + tmp_nd->id, patch_ports->str); g_string_free(patch_ports, TRUE); } } @@ -259,7 +303,8 @@ write_ovs_protocols(const NetplanOVSSettings* ovs_settings, const gchar* bridge, for (unsigned i = 1; i < ovs_settings->protocols->len; ++i) g_string_append_printf(s, ",%s", g_array_index(ovs_settings->protocols, char*, i)); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s protocols=%s", bridge, s->str); + append_systemd_cmd(cmds, "%s set Bridge %s protocols=%s", + netplan_openvswitch_ovs_vsctl(), bridge, s->str); write_ovs_tag_setting(bridge, "Bridge", "protocols", NULL, s->str, cmds); g_string_free(s, TRUE); } @@ -301,7 +346,8 @@ write_ovs_bridge_controller_targets(const NetplanOVSSettings* settings, const Ne } g_string_erase(s, s->len-1, 1); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-controller %s %s", bridge, s->str); + append_systemd_cmd(cmds, "%s set-controller %s %s", + netplan_openvswitch_ovs_vsctl(), bridge, s->str); write_ovs_tag_setting(bridge, "Bridge", "global", "set-controller", s->str, cmds); cleanup: @@ -340,7 +386,9 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio write_ovs_tag_netplan(def->id, type, cmds); /* Set LACP mode, default to "off" */ value = def->ovs_settings.lacp? def->ovs_settings.lacp : "off"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Port %s lacp=%s", def->id, value); + append_systemd_cmd(cmds, "%s set Port %s lacp=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "lacp", NULL, value, cmds); if (def->bond_params.mode && !write_ovs_bond_mode(def, cmds, error)) return FALSE; @@ -351,15 +399,23 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio write_ovs_tag_netplan(def->id, type, cmds); /* Set fail-mode, default to "standalone" */ value = def->ovs_settings.fail_mode? def->ovs_settings.fail_mode : "standalone"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-fail-mode %s %s", def->id, value); + append_systemd_cmd(cmds, "%s set-fail-mode %s %s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "global", "set-fail-mode", value, cmds); /* Enable/disable mcast-snooping */ value = def->ovs_settings.mcast_snooping? "true" : "false"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s mcast_snooping_enable=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Bridge %s mcast_snooping_enable=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "mcast_snooping_enable", NULL, value, cmds); /* Enable/disable rstp */ value = def->ovs_settings.rstp? "true" : "false"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s rstp_enable=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Bridge %s rstp_enable=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "rstp_enable", NULL, value, cmds); /* Set protocols */ if (def->ovs_settings.protocols && def->ovs_settings.protocols->len > 0) @@ -374,7 +430,11 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio /* Set controller connection mode, only applicable if at least one controller target address was set */ if (def->ovs_settings.controller.connection_mode) { value = def->ovs_settings.controller.connection_mode; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Controller %s connection-mode=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Controller %s " + "connection-mode=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, "Controller", "connection-mode", NULL, value, cmds); } } @@ -400,7 +460,9 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio g_assert(def->vlan_link); dependency = def->vlan_link->id; /* Create a fake VLAN bridge */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-br %s %s %i", def->id, def->vlan_link->id, def->vlan_id) + append_systemd_cmd(cmds, "%s --may-exist add-br %s %s %i", + netplan_openvswitch_ovs_vsctl(), + def->id, def->vlan_link->id, def->vlan_id) write_ovs_tag_netplan(def->id, type, cmds); break; @@ -477,7 +539,8 @@ netplan_state_finish_ovs_write(const NetplanState* np_state, const char* rootdir settings->ssl.client_key, settings->ssl.client_certificate, settings->ssl.ca_certificate); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-ssl %s", value->str); + append_systemd_cmd(cmds, "%s set-ssl %s", + netplan_openvswitch_ovs_vsctl(), value->str); write_ovs_tag_setting(".", "open_vswitch", "global", "set-ssl", value->str, cmds); g_string_free(value, TRUE); } diff --git a/src/openvswitch.h b/src/openvswitch.h index 67aff9add..5982b11f8 100644 --- a/src/openvswitch.h +++ b/src/openvswitch.h @@ -29,3 +29,6 @@ netplan_netdef_write_ovs( NETPLAN_INTERNAL gboolean netplan_ovs_cleanup(const char* rootdir); + +NETPLAN_INTERNAL const char * +netplan_openvswitch_ovs_vsctl(void); diff --git a/src/util-internal.h b/src/util-internal.h index fe41c776e..0b3c66f2e 100644 --- a/src/util-internal.h +++ b/src/util-internal.h @@ -61,8 +61,6 @@ wifi_get_freq5(int channel); NETPLAN_ABI gchar* systemd_escape(char* string); -#define OPENVSWITCH_OVS_VSCTL "/usr/bin/ovs-vsctl" - void mark_data_as_dirty(NetplanParser* npp, const void* data_ptr); diff --git a/src/validation.c b/src/validation.c index 4e372135f..baaa381c7 100644 --- a/src/validation.c +++ b/src/validation.c @@ -31,6 +31,7 @@ #include "error.h" #include "util-internal.h" #include "validation.h" +#include "openvswitch.h" /* Check coherence for address types */ @@ -401,7 +402,9 @@ validate_netdef_grammar(const NetplanParser* npp, NetplanNetDefinition* nd, GErr if (nd->backend == NETPLAN_BACKEND_OVS) { // LCOV_EXCL_START - if (!g_file_test(OPENVSWITCH_OVS_VSCTL, G_FILE_TEST_EXISTS)) { + if (!g_file_test(netplan_openvswitch_ovs_vsctl(), + G_FILE_TEST_EXISTS)) + { /* Tested via integration test */ return yaml_error(npp, NULL, error, "%s: The 'ovs-vsctl' tool is required to setup OpenVSwitch interfaces.", nd->id); }