From 9b82c62f33d53192cd89ae9c62f67ddad2c2df1f Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 9 Jun 2020 10:36:03 +0200 Subject: [PATCH 1/2] libnm-core: interpret ovs-patch.peer as an interface name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'peer' property of ovs-patch is inserted into the 'options' column of the ovsdb 'Interface' table. The ovs-vswitchd.conf.db man page says about it: options : peer: optional string The name of the Interface for the other side of the patch. The named Interface’s own peer option must specify this Interface’s name. That is, the two patch interfaces must have reversed name and peer values. Therefore, it is wrong to validate the peer property as an IP address and document it as such. Fixes: d4a7fe46797b ('libnm-core: add ovs-patch setting') (cherry picked from commit beb1dba8c145be0971a488dcabac241e1c3a363b) (cherry picked from commit 5598c039e4557510b567f12418601f5983fe5357) --- clients/common/settings-docs.h.in | 2 +- libnm-core/nm-setting-ovs-patch.c | 12 +++--------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in index cec6e485e2..c10b6975a6 100644 --- a/clients/common/settings-docs.h.in +++ b/clients/common/settings-docs.h.in @@ -270,7 +270,7 @@ #define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_STP_ENABLE N_("Enable or disable STP.") #define DESCRIBE_DOC_NM_SETTING_OVS_DPDK_DEVARGS N_("Open vSwitch DPDK device arguments.") #define DESCRIBE_DOC_NM_SETTING_OVS_INTERFACE_TYPE N_("The interface type. Either \"internal\", \"system\", \"patch\", \"dpdk\", or empty.") -#define DESCRIBE_DOC_NM_SETTING_OVS_PATCH_PEER N_("Specifies the unicast destination IP address of a remote Open vSwitch bridge port to connect to.") +#define DESCRIBE_DOC_NM_SETTING_OVS_PATCH_PEER N_("Specifies the name of the interface for the other side of the patch. The patch on the other side must also set this interface as peer.") #define DESCRIBE_DOC_NM_SETTING_OVS_PORT_BOND_DOWNDELAY N_("The time port must be inactive in order to be considered down.") #define DESCRIBE_DOC_NM_SETTING_OVS_PORT_BOND_MODE N_("Bonding mode. One of \"active-backup\", \"balance-slb\", or \"balance-tcp\".") #define DESCRIBE_DOC_NM_SETTING_OVS_PORT_BOND_UPDELAY N_("The time port must be active before it starts forwarding traffic.") diff --git a/libnm-core/nm-setting-ovs-patch.c b/libnm-core/nm-setting-ovs-patch.c index e74a35caf8..b70a1bd316 100644 --- a/libnm-core/nm-setting-ovs-patch.c +++ b/libnm-core/nm-setting-ovs-patch.c @@ -81,13 +81,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - if ( !nm_utils_ipaddr_valid (AF_INET, self->peer) - && !nm_utils_ipaddr_valid (AF_INET6, self->peer)) { - g_set_error (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("'%s' is not a valid IP address"), - self->peer); + if (!nm_utils_ifname_valid (self->peer, NMU_IFACE_OVS, error)) { g_prefix_error (error, "%s.%s: ", NM_SETTING_OVS_PATCH_SETTING_NAME, NM_SETTING_OVS_PATCH_PEER); @@ -179,8 +173,8 @@ nm_setting_ovs_patch_class_init (NMSettingOvsPatchClass *klass) /** * NMSettingOvsPatch:peer: * - * Specifies the unicast destination IP address of a remote Open vSwitch - * bridge port to connect to. + * Specifies the name of the interface for the other side of the patch. + * The patch on the other side must also set this interface as peer. * * Since: 1.10 **/ From 399aad15bf53420cc015c7a920856f88d5584dfc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 9 Jun 2020 10:44:01 +0200 Subject: [PATCH 2/2] ovs: ignore failures of patch interfaces When there are two patch ports connected, each of them must reference the other; however they can't be created in a single transaction because they are part of different bridges (so, different connections). Therefore, the first patch that gets activated will always fail with "No usable peer $x exists in 'system' datapath" until the second patch exists. In theory we could also match the error message, however this doesn't seem very robust as the message may slightly change in the future. (cherry picked from commit ffeac35f0409516aa2302189cca3f0b72518466a) (cherry picked from commit 75cbf2173862a00ff44342a4a676aa1f6f9ac78d) --- src/devices/ovs/nm-ovs-factory.c | 37 ++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/devices/ovs/nm-ovs-factory.c b/src/devices/ovs/nm-ovs-factory.c index d1d79a1cf0..d7bd0a0982 100644 --- a/src/devices/ovs/nm-ovs-factory.c +++ b/src/devices/ovs/nm-ovs-factory.c @@ -142,15 +142,40 @@ ovsdb_interface_failed (NMOvsdb *ovsdb, { NMDevice *device = NULL; NMSettingsConnection *connection = NULL; - - _LOGI (name, connection_uuid, "ovs interface \"%s\" (%s) failed: %s", name, connection_uuid, error); + NMConnection *c; + const char *type; + NMSettingOvsInterface *s_ovs_int; + gboolean is_patch = FALSE; + gboolean ignore; device = nm_manager_get_device (NM_MANAGER_GET, name, NM_DEVICE_TYPE_OVS_INTERFACE); - if (!device) - return; + if (device && connection_uuid) { + connection = nm_settings_get_connection_by_uuid (nm_device_get_settings (device), + connection_uuid); + } - if (connection_uuid) - connection = nm_settings_get_connection_by_uuid (nm_device_get_settings (device), connection_uuid); + /* The patch interface which gets created first is expected to + * fail because the second patch doesn't exist yet. Ignore all + * failures of patch interfaces. */ + if ( connection + && (c = nm_settings_connection_get_connection (connection)) + && (type = nm_connection_get_connection_type (c)) + && nm_streq0 (type, NM_SETTING_OVS_INTERFACE_SETTING_NAME) + && (s_ovs_int = nm_connection_get_setting_ovs_interface (c)) + && nm_streq0 (nm_setting_ovs_interface_get_interface_type (s_ovs_int), "patch")) + is_patch = TRUE; + + ignore = !device || is_patch; + + _NMLOG (ignore ? LOGL_DEBUG : LOGL_INFO, + name, connection_uuid, + "ovs interface \"%s\" (%s) failed%s: %s", + name, connection_uuid, + ignore ? " (ignored)" : "", + error); + + if (ignore) + return; if (connection) { nm_settings_connection_autoconnect_blocked_reason_set (connection,