From 00b62f66a30dcbf2821f5ed2113a246ea42bdf33 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 28 Jul 2019 14:37:44 +0200
Subject: [PATCH 01/25] libnm: add internal _nm_connection_get_setting()
accessor
nm_connection_get_setting() returns a pointer of type NMSetting.
That is very inconvenient, because most callers will need the
the result pointer as a setting subtype (like NMSettingConnection).
That would be like g_object_new() returning a "GObject *" pointer,
which is technically correct but annoying.
In the past that problem was avoided by having countless accessors
like nm_connection_get_setting_ip4_config(), etc. But that just blows
up the API and also is not generic. Meaning: the type is not a function
argument but the function itself. That makes composing the code harder
as the setting type cannot be treated generically (as a function argument).
Anyway. Add an internal wrapper that returns a void pointer.
(cherry picked from commit c4788e611edfa897860494398de394c4a6254d6e)
(cherry picked from commit 9e32b33d05c58c0224028d80d25580ce7cbdaad6)
---
libnm-core/nm-core-internal.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 825355821345..039e2c29820d 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -443,6 +443,13 @@ gboolean _nm_utils_generate_mac_address_mask_parse (const char *value,
/*****************************************************************************/
+static inline gpointer
+_nm_connection_get_setting (NMConnection *connection,
+ GType type)
+{
+ return (gpointer) nm_connection_get_setting (connection, type);
+}
+
NMSettingIPConfig *nm_connection_get_setting_ip_config (NMConnection *connection,
int addr_family);
--
2.26.2
From bcf293b0074826e7ed3de44d2ed50292910a9f4d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 23 Jul 2019 23:25:03 +0200
Subject: [PATCH 02/25] shared: add nm_g_slice_free() helper
How odd that such a macro does not exist yet. It seems like
the majorities of calls to g_slice_free() could be replaced
by this.
(cherry picked from commit dcdbe984065ed06753a0ce542c1aaaa9336d8424)
(cherry picked from commit 8f644537a4c4d001ec219c82cc906dcd610f08a5)
---
shared/nm-glib-aux/nm-macros-internal.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h
index 1ca89787a693..ec78ef585424 100644
--- a/shared/nm-glib-aux/nm-macros-internal.h
+++ b/shared/nm-glib-aux/nm-macros-internal.h
@@ -1500,6 +1500,11 @@ nm_strcmp_p (gconstpointer a, gconstpointer b)
/*****************************************************************************/
+#define nm_g_slice_free(ptr) \
+ g_slice_free (typeof (*(ptr)), ptr)
+
+/*****************************************************************************/
+
/* like g_memdup(). The difference is that the @size argument is of type
* gsize, while g_memdup() has type guint. Since, the size of container types
* like GArray is guint as well, this means trying to g_memdup() an
--
2.26.2
From 285790e0dd450c817bda9f2ed576335f9a9644dc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Sun, 7 Jun 2020 15:10:36 +0200
Subject: [PATCH 03/25] tests: suppress valgrind warning about unsupported
syscall for "test-config"
(cherry picked from commit d507563a80d3e171017700f7b268940b63f5708d)
(cherry picked from commit 4036bc48e4a5b4989a789c74b124e76ae0286ea8)
(cherry picked from commit 9ff160d155da29b02890ce6ae433b2f2a39c579e)
(cherry picked from commit 1b9da1869382185fe4f03c17fd7612ce1809e0f3)
(cherry picked from commit a1f4bcd6111adb026dc010fbd1071822e4248bb3)
---
tools/run-nm-test.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/run-nm-test.sh b/tools/run-nm-test.sh
index e8af231883a0..09c735c129bf 100755
--- a/tools/run-nm-test.sh
+++ b/tools/run-nm-test.sh
@@ -297,6 +297,7 @@ if [ $HAS_ERRORS -eq 0 ]; then
# valgrind doesn't support setns syscall and spams the logfile.
# hack around it...
case "$TEST_NAME" in
+ 'test-config' | \
'test-link-linux' | \
'test-acd' | \
'test-service-providers' | \
--
2.26.2
From 6c5b3350527f2d358ed107f4f3943eb26eb5357d Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 12 Jun 2019 17:58:49 +0200
Subject: [PATCH 04/25] ovs/ovsdb: guard against OVSDB integrity issues
Don't crash in situations, where the bridge or a port has a child with
UUID we don't know. This could happen if we mess up the parsing of
messages from OVSDB, but could also theoretically happen in OVSDB sends
us bad data.
(cherry picked from commit 99c7adc1e12862f22026bb8f806c9223db9a78ec)
(cherry picked from commit 3455c30a58ff14952acf86f8ea068ef8f3b6d889)
---
src/devices/ovs/nm-ovsdb.c | 40 +++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 82e25f396ccf..5f46c23a260b 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -584,9 +584,14 @@ _add_interface (NMOvsdb *self, json_t *params,
json_array_append_new (ports, json_pack ("[s, s]", "uuid", port_uuid));
- if ( g_strcmp0 (ovs_port->name, nm_connection_get_interface_name (port)) != 0
- || g_strcmp0 (ovs_port->connection_uuid, nm_connection_get_uuid (port)) != 0)
+ if (!ovs_port) {
+ /* This would be a violation of ovsdb's reference integrity (a bug). */
+ _LOGW ("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid);
continue;
+ } else if ( strcmp (ovs_port->name, nm_connection_get_interface_name (port)) != 0
+ || g_strcmp0 (ovs_port->connection_uuid, nm_connection_get_uuid (port)) != 0) {
+ continue;
+ }
for (ii = 0; ii < ovs_port->interfaces->len; ii++) {
interface_uuid = g_ptr_array_index (ovs_port->interfaces, ii);
@@ -594,9 +599,13 @@ _add_interface (NMOvsdb *self, json_t *params,
json_array_append_new (interfaces, json_pack ("[s, s]", "uuid", interface_uuid));
- if ( g_strcmp0 (ovs_interface->name, nm_connection_get_interface_name (interface)) == 0
- && g_strcmp0 (ovs_interface->connection_uuid, nm_connection_get_uuid (interface)) == 0)
+ if (!ovs_interface) {
+ /* This would be a violation of ovsdb's reference integrity (a bug). */
+ _LOGW ("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid);
+ } else if ( strcmp (ovs_interface->name, nm_connection_get_interface_name (interface)) == 0
+ && g_strcmp0 (ovs_interface->connection_uuid, nm_connection_get_uuid (interface)) == 0) {
has_interface = TRUE;
+ }
}
break;
@@ -692,16 +701,27 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname)
interfaces_changed = FALSE;
+ if (!ovs_port) {
+ /* This would be a violation of ovsdb's reference integrity (a bug). */
+ _LOGW ("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid);
+ continue;
+ }
+
for (ii = 0; ii < ovs_port->interfaces->len; ii++) {
interface_uuid = g_ptr_array_index (ovs_port->interfaces, ii);
ovs_interface = g_hash_table_lookup (priv->interfaces, interface_uuid);
json_array_append_new (interfaces, json_pack ("[s,s]", "uuid", interface_uuid));
- if (strcmp (ovs_interface->name, ifname) == 0) {
- /* skip the interface */
- interfaces_changed = TRUE;
- continue;
+ if (ovs_interface) {
+ if (strcmp (ovs_interface->name, ifname) == 0) {
+ /* skip the interface */
+ interfaces_changed = TRUE;
+ continue;
+ }
+ } else {
+ /* This would be a violation of ovsdb's reference integrity (a bug). */
+ _LOGW ("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid);
}
json_array_append_new (new_interfaces, json_pack ("[s,s]", "uuid", interface_uuid));
@@ -929,7 +949,9 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
if (old) {
ovs_interface = g_hash_table_lookup (priv->interfaces, key);
- if (!new || g_strcmp0 (ovs_interface->name, name) != 0) {
+ if (!ovs_interface) {
+ _LOGW ("Interface '%s' was not seen", key);
+ } else if (!new || strcmp (ovs_interface->name, name) != 0) {
old = FALSE;
_LOGT ("removed an '%s' interface: %s%s%s",
ovs_interface->type, ovs_interface->name,
--
2.26.2
From 22903209fe8d1a3e9e3b6c9fa8c0178c9e6d2551 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 12 Jun 2019 17:10:09 +0200
Subject: [PATCH 05/25] ovs/ovsdb: remove the device-changes signal
It doesn't communicate anything about the nature of the change and
indeed nothing uses it.
(cherry picked from commit b1feebc43aafd5e40c371c2a971cdefd5f8acae6)
(cherry picked from commit 7448ad84679188157eda4ad3ba0289af4918d384)
---
src/devices/ovs/nm-ovsdb.c | 14 --------------
src/devices/ovs/nm-ovsdb.h | 1 -
2 files changed, 15 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 5f46c23a260b..9ea78a39b9b8 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -59,7 +59,6 @@ typedef struct {
enum {
DEVICE_ADDED,
DEVICE_REMOVED,
- DEVICE_CHANGED,
LAST_SIGNAL
};
@@ -976,8 +975,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
_LOGT ("changed an '%s' interface: %s%s%s", type, ovs_interface->name,
ovs_interface->connection_uuid ? ", " : "",
ovs_interface->connection_uuid ?: "");
- g_signal_emit (self, signals[DEVICE_CHANGED], 0,
- "ovs-interface", ovs_interface->name);
} else {
_LOGT ("added an '%s' interface: %s%s%s",
ovs_interface->type, ovs_interface->name,
@@ -1031,8 +1028,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
_LOGT ("changed a port: %s%s%s", ovs_port->name,
ovs_port->connection_uuid ? ", " : "",
ovs_port->connection_uuid ?: "");
- g_signal_emit (self, signals[DEVICE_CHANGED], 0,
- NM_SETTING_OVS_PORT_SETTING_NAME, ovs_port->name);
} else {
_LOGT ("added a port: %s%s%s", ovs_port->name,
ovs_port->connection_uuid ? ", " : "",
@@ -1081,8 +1076,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
_LOGT ("changed a bridge: %s%s%s", ovs_bridge->name,
ovs_bridge->connection_uuid ? ", " : "",
ovs_bridge->connection_uuid ?: "");
- g_signal_emit (self, signals[DEVICE_CHANGED], 0,
- NM_SETTING_OVS_BRIDGE_SETTING_NAME, ovs_bridge->name);
} else {
_LOGT ("added a bridge: %s%s%s", ovs_bridge->name,
ovs_bridge->connection_uuid ? ", " : "",
@@ -1648,11 +1641,4 @@ nm_ovsdb_class_init (NMOvsdbClass *klass)
G_SIGNAL_RUN_LAST,
0, NULL, NULL, NULL,
G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
-
- signals[DEVICE_CHANGED] =
- g_signal_new (NM_OVSDB_DEVICE_CHANGED,
- G_OBJECT_CLASS_TYPE (object_class),
- G_SIGNAL_RUN_LAST,
- 0, NULL, NULL, NULL,
- G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
}
diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h
index f9dbabbb7ad8..e403b0e447c1 100644
--- a/src/devices/ovs/nm-ovsdb.h
+++ b/src/devices/ovs/nm-ovsdb.h
@@ -29,7 +29,6 @@
#define NM_OVSDB_DEVICE_ADDED "device-added"
#define NM_OVSDB_DEVICE_REMOVED "device-removed"
-#define NM_OVSDB_DEVICE_CHANGED "device-changed"
typedef struct _NMOvsdb NMOvsdb;
typedef struct _NMOvsdbClass NMOvsdbClass;
--
2.26.2
From a7b2da1d28c725143a0c5370d2836f17e84194e7 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Fri, 14 Jun 2019 10:31:15 +0200
Subject: [PATCH 06/25] ovs/ovsdb: fix signal handler argument types
(cherry picked from commit dedc0cba23b5d51773f88d8bc06424eb589083cd)
(cherry picked from commit 4107490c892ee90e87d6231557a01501cf340bd1)
---
src/devices/ovs/nm-ovsdb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 9ea78a39b9b8..a9c209a2f408 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -1633,12 +1633,12 @@ nm_ovsdb_class_init (NMOvsdbClass *klass)
G_OBJECT_CLASS_TYPE (object_class),
G_SIGNAL_RUN_LAST,
0, NULL, NULL, NULL,
- G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
+ G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_UINT);
signals[DEVICE_REMOVED] =
g_signal_new (NM_OVSDB_DEVICE_REMOVED,
G_OBJECT_CLASS_TYPE (object_class),
G_SIGNAL_RUN_LAST,
0, NULL, NULL, NULL,
- G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
+ G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_UINT);
}
--
2.26.2
From 65b6544d76aac604fe8259fa78161dc23e08e5f3 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 12 Jun 2019 17:15:13 +0200
Subject: [PATCH 07/25] ovs/ovsdb: signal a failure when an error column is set
When an interface (other OVS device types can not fail) encounters an error
it indicates it by changing the error column. Watch for those changes so
that we can eventually communicate them to the OVS factory to deal with
them.
(cherry picked from commit f2c066e1046cc8da5a277b1528a59bf2653e17c8)
(cherry picked from commit cb19680d342d30043b6c7534cab4c8a12d3e797f)
---
src/devices/ovs/nm-ovsdb.c | 24 +++++++++++++++++++++---
src/devices/ovs/nm-ovsdb.h | 5 +++--
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index a9c209a2f408..a9644ce9e13c 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -59,6 +59,7 @@ typedef struct {
enum {
DEVICE_ADDED,
DEVICE_REMOVED,
+ INTERFACE_FAILED,
LAST_SIGNAL
};
@@ -787,14 +788,14 @@ ovsdb_next_command (NMOvsdb *self)
msg = json_pack ("{s:i, s:s, s:[s, n, {"
" s:[{s:[s, s, s]}],"
" s:[{s:[s, s, s]}],"
- " s:[{s:[s, s, s]}],"
+ " s:[{s:[s, s, s, s]}],"
" s:[{s:[]}]"
"}]}",
"id", call->id,
"method", "monitor", "params", "Open_vSwitch",
"Bridge", "columns", "name", "ports", "external_ids",
"Port", "columns", "name", "interfaces", "external_ids",
- "Interface", "columns", "name", "type", "external_ids",
+ "Interface", "columns", "name", "type", "external_ids", "error",
"Open_vSwitch", "columns");
break;
case OVSDB_ADD_INTERFACE:
@@ -934,15 +935,17 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
/* Interfaces */
json_object_foreach (interface, key, value) {
+ json_t *error = NULL;
gboolean old = FALSE;
gboolean new = FALSE;
if (json_unpack (value, "{s:{}}", "old") == 0)
old = TRUE;
- if (json_unpack (value, "{s:{s:s, s:s, s:o}}", "new",
+ if (json_unpack (value, "{s:{s:s, s:s, s?:o, s:o}}", "new",
"name", &name,
"type", &type,
+ "error", &error,
"external_ids", &external_ids) == 0)
new = TRUE;
@@ -987,6 +990,14 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
ovs_interface->name, NM_DEVICE_TYPE_OVS_INTERFACE);
}
}
+ /* The error is a string. No error is indicated by an empty set,
+ * because why the fuck not: [ "set": [] ] */
+ if (error && json_is_string (error)) {
+ g_signal_emit (self, signals[INTERFACE_FAILED], 0,
+ ovs_interface->name,
+ ovs_interface->connection_uuid,
+ json_string_value (error));
+ }
g_hash_table_insert (priv->interfaces, g_strdup (key), ovs_interface);
}
}
@@ -1641,4 +1652,11 @@ nm_ovsdb_class_init (NMOvsdbClass *klass)
G_SIGNAL_RUN_LAST,
0, NULL, NULL, NULL,
G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_UINT);
+
+ signals[INTERFACE_FAILED] =
+ g_signal_new (NM_OVSDB_INTERFACE_FAILED,
+ G_OBJECT_CLASS_TYPE (object_class),
+ G_SIGNAL_RUN_LAST,
+ 0, NULL, NULL, NULL,
+ G_TYPE_NONE, 3, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING);
}
diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h
index e403b0e447c1..e7f9d71984c2 100644
--- a/src/devices/ovs/nm-ovsdb.h
+++ b/src/devices/ovs/nm-ovsdb.h
@@ -27,8 +27,9 @@
#define NM_IS_OVSDB_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_OVSDB))
#define NM_OVSDB_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_OVSDB, NMOvsdbClass))
-#define NM_OVSDB_DEVICE_ADDED "device-added"
-#define NM_OVSDB_DEVICE_REMOVED "device-removed"
+#define NM_OVSDB_DEVICE_ADDED "device-added"
+#define NM_OVSDB_DEVICE_REMOVED "device-removed"
+#define NM_OVSDB_INTERFACE_FAILED "interface-failed"
typedef struct _NMOvsdb NMOvsdb;
typedef struct _NMOvsdbClass NMOvsdbClass;
--
2.26.2
From 358db39dc1e0d9b29dd4e6d39dbb5b8ab8f45de6 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 12 Jun 2019 18:30:36 +0200
Subject: [PATCH 08/25] ovs/ovsdb: track the devices before we signal addition
This doesn't make any difference in practice, but it seems more correct.
It would cause issues if we decided to remove an interface from the
signal handler.
(cherry picked from commit e948ce7debb4f2da2db9c19ab4f980eac5415b9d)
(cherry picked from commit 83a42ef8ad8a3edd95eceff1a3ea69989d78baa4)
---
src/devices/ovs/nm-ovsdb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index a9644ce9e13c..c85115201eac 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -974,6 +974,7 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
ovs_interface->name = g_strdup (name);
ovs_interface->type = g_strdup (type);
ovs_interface->connection_uuid = _connection_uuid_from_external_ids (external_ids);
+ g_hash_table_insert (priv->interfaces, g_strdup (key), ovs_interface);
if (old) {
_LOGT ("changed an '%s' interface: %s%s%s", type, ovs_interface->name,
ovs_interface->connection_uuid ? ", " : "",
@@ -998,7 +999,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
ovs_interface->connection_uuid,
json_string_value (error));
}
- g_hash_table_insert (priv->interfaces, g_strdup (key), ovs_interface);
}
}
@@ -1035,6 +1035,7 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
ovs_port->connection_uuid = _connection_uuid_from_external_ids (external_ids);
ovs_port->interfaces = g_ptr_array_new_with_free_func (g_free);
_uuids_to_array (ovs_port->interfaces, items);
+ g_hash_table_insert (priv->ports, g_strdup (key), ovs_port);
if (old) {
_LOGT ("changed a port: %s%s%s", ovs_port->name,
ovs_port->connection_uuid ? ", " : "",
@@ -1046,7 +1047,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
g_signal_emit (self, signals[DEVICE_ADDED], 0,
ovs_port->name, NM_DEVICE_TYPE_OVS_PORT);
}
- g_hash_table_insert (priv->ports, g_strdup (key), ovs_port);
}
}
@@ -1083,6 +1083,7 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
ovs_bridge->connection_uuid = _connection_uuid_from_external_ids (external_ids);
ovs_bridge->ports = g_ptr_array_new_with_free_func (g_free);
_uuids_to_array (ovs_bridge->ports, items);
+ g_hash_table_insert (priv->bridges, g_strdup (key), ovs_bridge);
if (old) {
_LOGT ("changed a bridge: %s%s%s", ovs_bridge->name,
ovs_bridge->connection_uuid ? ", " : "",
@@ -1094,7 +1095,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
g_signal_emit (self, signals[DEVICE_ADDED], 0,
ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE);
}
- g_hash_table_insert (priv->bridges, g_strdup (key), ovs_bridge);
}
}
--
2.26.2
From 1e056bd5efc42fa753e61bfad5e5f1d88177bb46 Mon Sep 17 00:00:00 2001
From: Lubomir Rintel <lkundrak@v3.sk>
Date: Wed, 12 Jun 2019 18:38:58 +0200
Subject: [PATCH 09/25] ovs/factory: fail the NMDevice if there's an error in
OVSDB
(cherry picked from commit 02950ec600d07647dadabdf08b151d5c4f5f8985)
(cherry picked from commit ba2c71a01db83a786ec4d1fe10b42574cd45e2c2)
---
src/devices/ovs/nm-ovs-factory.c | 41 +++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/src/devices/ovs/nm-ovs-factory.c b/src/devices/ovs/nm-ovs-factory.c
index 2124b2a0b1cd..fdf07bd3750a 100644
--- a/src/devices/ovs/nm-ovs-factory.c
+++ b/src/devices/ovs/nm-ovs-factory.c
@@ -26,7 +26,9 @@
#include "nm-device-ovs-bridge.h"
#include "platform/nm-platform.h"
#include "nm-core-internal.h"
+#include "settings/nm-settings.h"
#include "devices/nm-device-factory.h"
+#include "devices/nm-device-private.h"
/*****************************************************************************/
@@ -51,7 +53,13 @@ G_DEFINE_TYPE (NMOvsFactory, nm_ovs_factory, NM_TYPE_DEVICE_FACTORY)
/*****************************************************************************/
#define _NMLOG_DOMAIN LOGD_DEVICE
-#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "ovs", __VA_ARGS__)
+#define _NMLOG(level, ifname, con_uuid, ...) \
+ G_STMT_START { \
+ nm_log ((level), _NMLOG_DOMAIN, (ifname), (con_uuid), \
+ "ovs: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__) \
+ _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
+ } G_STMT_END
+
/*****************************************************************************/
@@ -138,6 +146,36 @@ ovsdb_device_removed (NMOvsdb *ovsdb, const char *name, NMDeviceType device_type
}
}
+static void
+ovsdb_interface_failed (NMOvsdb *ovsdb,
+ const char *name,
+ const char *connection_uuid,
+ const char *error,
+ NMDeviceFactory *self)
+{
+ NMDevice *device = NULL;
+ NMSettingsConnection *connection = NULL;
+
+ _LOGI (name, connection_uuid, "ovs interface \"%s\" (%s) failed: %s", name, connection_uuid, error);
+
+ device = nm_manager_get_device (nm_manager_get (), name, NM_DEVICE_TYPE_OVS_INTERFACE);
+ if (!device)
+ return;
+
+ if (connection_uuid)
+ connection = nm_settings_get_connection_by_uuid (nm_device_get_settings (device), connection_uuid);
+
+ if (connection) {
+ nm_settings_connection_autoconnect_blocked_reason_set (connection,
+ NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED,
+ TRUE);
+ }
+
+ nm_device_state_changed (device,
+ NM_DEVICE_STATE_FAILED,
+ NM_DEVICE_STATE_REASON_OVSDB_FAILED);
+}
+
static void
start (NMDeviceFactory *self)
{
@@ -147,6 +185,7 @@ start (NMDeviceFactory *self)
g_signal_connect_object (ovsdb, NM_OVSDB_DEVICE_ADDED, G_CALLBACK (ovsdb_device_added), self, (GConnectFlags) 0);
g_signal_connect_object (ovsdb, NM_OVSDB_DEVICE_REMOVED, G_CALLBACK (ovsdb_device_removed), self, (GConnectFlags) 0);
+ g_signal_connect_object (ovsdb, NM_OVSDB_INTERFACE_FAILED, G_CALLBACK (ovsdb_interface_failed), self, (GConnectFlags) 0);
}
static NMDevice *
--
2.26.2
From 8c9716c53e232a18249d9d07b52f0d06084660b8 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 15 Jan 2020 18:30:17 +0100
Subject: [PATCH 10/25] ovs: wait that link disappears before continuing with
deactivation
When we deactivate a virtual device, we usually schedule the deletion
of the link in an idle handler. That action will be executed at a
later time when the device is already in the disconnected state.
Similarly, for ovs interfaces we send the deletion command to the
ovsdb and then proceed to the disconnected state.
However, in the first case there is the guarantee that the link will
be deleted at some point, while for ovs interfaces it may happen that
ovs decides to reuse the same link if there is an addition
queued. Since reusing the same link confuses NM, let's implement
deactivate_async() for ovs-interfaces and wait that the link actually
goes away before proceeding.
https://bugzilla.redhat.com/show_bug.cgi?id=1782701
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/402
(cherry picked from commit 623a1e1f993b3166509202bc580c30e3daf9c67b)
(cherry picked from commit a1b0edd24b078294cd3959044a7ae35287d55f87)
(cherry picked from commit cb7c7c29bdde6654b3a5d498c592033772592ad0)
(cherry picked from commit b0aad945b4a1e6bdcf72a3a799f55cf5c72f411a)
---
src/devices/ovs/nm-device-ovs-interface.c | 115 ++++++++++++++++++++++
1 file changed, 115 insertions(+)
diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c
index 86d0b8fbd19c..a6fc2656ce7a 100644
--- a/src/devices/ovs/nm-device-ovs-interface.c
+++ b/src/devices/ovs/nm-device-ovs-interface.c
@@ -35,6 +35,7 @@ _LOG_DECLARE_SELF(NMDeviceOvsInterface);
typedef struct {
bool waiting_for_interface:1;
+ int link_ifindex;
} NMDeviceOvsInterfacePrivate;
struct _NMDeviceOvsInterface {
@@ -111,6 +112,9 @@ link_changed (NMDevice *device,
{
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
+ if (pllink)
+ priv->link_ifindex = pllink->ifindex;
+
if ( pllink
&& priv->waiting_for_interface
&& nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
@@ -166,6 +170,116 @@ deactivate (NMDevice *device)
priv->waiting_for_interface = FALSE;
}
+typedef struct {
+ NMDeviceOvsInterface *self;
+ GCancellable *cancellable;
+ NMDeviceDeactivateCallback callback;
+ gpointer callback_user_data;
+ gulong link_changed_id;
+ gulong cancelled_id;
+} DeactivateData;
+
+static void
+deactivate_invoke_cb (DeactivateData *data, GError *error)
+{
+ data->callback (NM_DEVICE (data->self),
+ error,
+ data->callback_user_data);
+
+ nm_clear_g_signal_handler (nm_device_get_platform (NM_DEVICE (data->self)),
+ &data->link_changed_id);
+ nm_clear_g_signal_handler (data->cancellable,
+ &data->cancelled_id);
+ g_object_unref (data->self);
+ g_object_unref (data->cancellable);
+ nm_g_slice_free (data);
+}
+
+static void
+link_changed_cb (NMPlatform *platform,
+ int obj_type_i,
+ int ifindex,
+ NMPlatformLink *info,
+ int change_type_i,
+ DeactivateData *data)
+{
+ NMDeviceOvsInterface *self = data->self;
+ NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
+ const NMPlatformSignalChangeType change_type = change_type_i;
+
+ if ( change_type == NM_PLATFORM_SIGNAL_REMOVED
+ && ifindex == priv->link_ifindex) {
+ _LOGT (LOGD_DEVICE,
+ "link %d gone, proceeding with deactivation",
+ priv->link_ifindex);
+ priv->link_ifindex = 0;
+ deactivate_invoke_cb (data, NULL);
+ return;
+ }
+}
+
+static void
+deactivate_cancelled_cb (GCancellable *cancellable,
+ gpointer user_data)
+{
+ gs_free_error GError *error = NULL;
+
+ nm_utils_error_set_cancelled (&error, FALSE, NULL);
+ deactivate_invoke_cb ((DeactivateData *) user_data, error);
+}
+
+static void
+deactivate_cb_on_idle (gpointer user_data,
+ GCancellable *cancellable)
+{
+ DeactivateData *data = user_data;
+ gs_free_error GError *cancelled_error = NULL;
+
+ g_cancellable_set_error_if_cancelled (data->cancellable, &cancelled_error);
+ deactivate_invoke_cb (data, cancelled_error);
+}
+
+static void
+deactivate_async (NMDevice *device,
+ GCancellable *cancellable,
+ NMDeviceDeactivateCallback callback,
+ gpointer callback_user_data) {
+
+ NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE (device);
+ NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
+ DeactivateData *data;
+
+ priv->waiting_for_interface = FALSE;
+
+ data = g_slice_new (DeactivateData);
+ *data = (DeactivateData) {
+ .self = g_object_ref (self),
+ .cancellable = g_object_ref (cancellable),
+ .callback = callback,
+ .callback_user_data = callback_user_data,
+ };
+
+ if ( !priv->link_ifindex
+ || !nm_platform_link_get (nm_device_get_platform (device), priv->link_ifindex)) {
+ priv->link_ifindex = 0;
+ nm_utils_invoke_on_idle (deactivate_cb_on_idle, data, cancellable);
+ return;
+ }
+
+ _LOGT (LOGD_DEVICE,
+ "async deactivation: waiting for link %d to disappear",
+ priv->link_ifindex);
+
+ data->cancelled_id = g_cancellable_connect (cancellable,
+ G_CALLBACK (deactivate_cancelled_cb),
+ data,
+ NULL);
+ data->link_changed_id = g_signal_connect (nm_device_get_platform (device),
+ NM_PLATFORM_SIGNAL_LINK_CHANGED,
+ G_CALLBACK (link_changed_cb),
+ data);
+}
+
/*****************************************************************************/
static void
@@ -196,6 +310,7 @@ nm_device_ovs_interface_class_init (NMDeviceOvsInterfaceClass *klass)
device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_OPENVSWITCH);
device_class->deactivate = deactivate;
+ device_class->deactivate_async = deactivate_async;
device_class->get_type_description = get_type_description;
device_class->create_and_realize = create_and_realize;
device_class->get_generic_capabilities = get_generic_capabilities;
--
2.26.2
From 91149efd8f6cecbc56e4c76c81c11db73f48b00c Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 13 Feb 2020 14:52:11 +0100
Subject: [PATCH 11/25] ovs: rework asynchronous deactivation of ovs interfaces
Tracking the deletion of link by ifindex is difficult because the
ifindex of the device is updated through delayed (idle) calls in
NMDevice and so there is the possibility that at a certain time the
device ifindex is not in sync with platform state. It seems simpler to
watch instead the interface name. The ugly thing is that the interface
name can be changed externally, but if users do that on an activating
device they are looking for trouble.
Also change the deactivate code to deal with the scenario where we
already created the interface in the ovsdb but the link didn't show up
yet. To ensure a proper cleanup we must wait that the link appears and
then goes away; however the link may never appear if vswitchd sees
only the last state in ovsdb, and so we must use a ugly timeout to
avoid waiting forever.
https://bugzilla.redhat.com/show_bug.cgi?id=1787989
(cherry picked from commit 9c49f8a87962d8009dc18973845a1c46aba38d00)
(cherry picked from commit 2e5e409bf2cf825af46c46d110b0502050adcb3c)
(cherry picked from commit 628706fab592f6c5611438a21742391ea1a2e8cc)
(cherry picked from commit 296d4e392649250e08735418e9e9d2cd416d0fe1)
---
src/devices/ovs/nm-device-ovs-interface.c | 86 ++++++++++++++++-------
1 file changed, 59 insertions(+), 27 deletions(-)
diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c
index a6fc2656ce7a..726637864e69 100644
--- a/src/devices/ovs/nm-device-ovs-interface.c
+++ b/src/devices/ovs/nm-device-ovs-interface.c
@@ -35,7 +35,6 @@ _LOG_DECLARE_SELF(NMDeviceOvsInterface);
typedef struct {
bool waiting_for_interface:1;
- int link_ifindex;
} NMDeviceOvsInterfacePrivate;
struct _NMDeviceOvsInterface {
@@ -112,13 +111,12 @@ link_changed (NMDevice *device,
{
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
- if (pllink)
- priv->link_ifindex = pllink->ifindex;
+ if (!pllink || !priv->waiting_for_interface)
+ return;
+
+ priv->waiting_for_interface = FALSE;
- if ( pllink
- && priv->waiting_for_interface
- && nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
- priv->waiting_for_interface = FALSE;
+ if (nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
nm_device_bring_up (device, TRUE, NULL);
nm_device_activate_schedule_stage3_ip_config_start (device);
}
@@ -142,12 +140,14 @@ act_stage3_ip_config_start (NMDevice *device,
gpointer *out_config,
NMDeviceStateReason *out_failure_reason)
{
+ NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE (device);
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
if (!_is_internal_interface (device))
return NM_ACT_STAGE_RETURN_IP_FAIL;
if (nm_device_get_ip_ifindex (device) <= 0) {
+ _LOGT (LOGD_DEVICE, "waiting for link to appear");
priv->waiting_for_interface = TRUE;
return NM_ACT_STAGE_RETURN_POSTPONE;
}
@@ -177,11 +177,17 @@ typedef struct {
gpointer callback_user_data;
gulong link_changed_id;
gulong cancelled_id;
+ guint link_timeout_id;
} DeactivateData;
static void
deactivate_invoke_cb (DeactivateData *data, GError *error)
{
+ NMDeviceOvsInterface *self = data->self;
+
+ _LOGT (LOGD_CORE,
+ "deactivate: async callback (%s)",
+ error ? error->message : "success");
data->callback (NM_DEVICE (data->self),
error,
data->callback_user_data);
@@ -190,34 +196,43 @@ deactivate_invoke_cb (DeactivateData *data, GError *error)
&data->link_changed_id);
nm_clear_g_signal_handler (data->cancellable,
&data->cancelled_id);
+ nm_clear_g_source (&data->link_timeout_id);
g_object_unref (data->self);
g_object_unref (data->cancellable);
nm_g_slice_free (data);
}
static void
-link_changed_cb (NMPlatform *platform,
- int obj_type_i,
- int ifindex,
- NMPlatformLink *info,
- int change_type_i,
- DeactivateData *data)
+deactivate_link_changed_cb (NMPlatform *platform,
+ int obj_type_i,
+ int ifindex,
+ NMPlatformLink *info,
+ int change_type_i,
+ DeactivateData *data)
{
NMDeviceOvsInterface *self = data->self;
- NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
const NMPlatformSignalChangeType change_type = change_type_i;
if ( change_type == NM_PLATFORM_SIGNAL_REMOVED
- && ifindex == priv->link_ifindex) {
- _LOGT (LOGD_DEVICE,
- "link %d gone, proceeding with deactivation",
- priv->link_ifindex);
- priv->link_ifindex = 0;
+ && nm_streq0 (info->name, nm_device_get_iface (NM_DEVICE (self)))) {
+ _LOGT (LOGD_DEVICE, "deactivate: link removed, proceeding");
+ nm_device_update_from_platform_link (NM_DEVICE (self), NULL);
deactivate_invoke_cb (data, NULL);
return;
}
}
+static gboolean
+deactivate_link_timeout (gpointer user_data)
+{
+ DeactivateData *data = user_data;
+ NMDeviceOvsInterface *self = data->self;
+
+ _LOGT (LOGD_DEVICE, "deactivate: timeout waiting link removal");
+ deactivate_invoke_cb (data, NULL);
+ return G_SOURCE_REMOVE;
+}
+
static void
deactivate_cancelled_cb (GCancellable *cancellable,
gpointer user_data)
@@ -249,7 +264,14 @@ deactivate_async (NMDevice *device,
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
DeactivateData *data;
- priv->waiting_for_interface = FALSE;
+ _LOGT (LOGD_CORE, "deactivate: start async");
+
+ /* We want to ensure that the kernel link for this device is
+ * removed upon disconnection so that it will not interfere with
+ * later activations of the same device. Unfortunately there is
+ * no synchronization mechanism with vswitchd, we only update
+ * ovsdb and wait that changes are picked up.
+ */
data = g_slice_new (DeactivateData);
*data = (DeactivateData) {
@@ -259,16 +281,26 @@ deactivate_async (NMDevice *device,
.callback_user_data = callback_user_data,
};
- if ( !priv->link_ifindex
- || !nm_platform_link_get (nm_device_get_platform (device), priv->link_ifindex)) {
- priv->link_ifindex = 0;
+ if ( !priv->waiting_for_interface
+ && !nm_platform_link_get_by_ifname (nm_device_get_platform (device),
+ nm_device_get_iface (device))) {
+ _LOGT (LOGD_CORE, "deactivate: link not present, proceeding");
+ nm_device_update_from_platform_link (NM_DEVICE (self), NULL);
nm_utils_invoke_on_idle (deactivate_cb_on_idle, data, cancellable);
return;
}
- _LOGT (LOGD_DEVICE,
- "async deactivation: waiting for link %d to disappear",
- priv->link_ifindex);
+ if (priv->waiting_for_interface) {
+ /* At this point we have issued an INSERT and a DELETE
+ * command for the interface to ovsdb. We don't know if
+ * vswitchd will see the two updates or only one. We
+ * must add a timeout to avoid waiting forever in case
+ * the link doesn't appear.
+ */
+ data->link_timeout_id = g_timeout_add (6000, deactivate_link_timeout, data);
+ _LOGT (LOGD_DEVICE, "deactivate: waiting for link to disappear in 6 seconds");
+ } else
+ _LOGT (LOGD_DEVICE, "deactivate: waiting for link to disappear");
data->cancelled_id = g_cancellable_connect (cancellable,
G_CALLBACK (deactivate_cancelled_cb),
@@ -276,7 +308,7 @@ deactivate_async (NMDevice *device,
NULL);
data->link_changed_id = g_signal_connect (nm_device_get_platform (device),
NM_PLATFORM_SIGNAL_LINK_CHANGED,
- G_CALLBACK (link_changed_cb),
+ G_CALLBACK (deactivate_link_changed_cb),
data);
}
--
2.26.2
From e9e1736dfc0c829074b90982ce44e6beb7266b4e Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Fri, 14 Feb 2020 17:46:31 +0100
Subject: [PATCH 12/25] ovs: discard link updates when deactivating
When the ovs interface gets deactivated, it is released from the
master port and we call nm_device_update_from_platform_link (dev,
NULL) to ignore any later event for the interface. This is important
especially because it sets a zero ifindex on the interface and so,
later when the link disappears, we don't unmanage the device but
directly remove it.
However, since ovs commands are queued, the link could appear during
the deactivation and we need to ignore such events. Add a new device
method can_update_from_platform_link() for such purpose.
(cherry picked from commit e9fc1dea437a3f78212143e8d2b14bcea8926f90)
(cherry picked from commit c4eb0c6852d96b82081babe2c16a54df75717aba)
(cherry picked from commit 34a9247a640504b15daeab578f569a4a151bfa7d)
(cherry picked from commit b22a20880e697febc3000de60321106a227a64bf)
---
src/devices/nm-device.c | 10 ++++++++++
src/devices/nm-device.h | 2 ++
src/devices/ovs/nm-device-ovs-interface.c | 13 +++++++++++++
3 files changed, 25 insertions(+)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 08ee2aef33a4..4bd93a510ee4 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -4142,6 +4142,12 @@ nm_device_create_and_realize (NMDevice *self,
return TRUE;
}
+static gboolean
+can_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink)
+{
+ return TRUE;
+}
+
void
nm_device_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink)
{
@@ -4150,6 +4156,9 @@ nm_device_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink
int ifindex;
guint32 mtu;
+ if (!NM_DEVICE_GET_CLASS (self)->can_update_from_platform_link (self, plink))
+ return;
+
g_return_if_fail (plink == NULL || link_type_compatible (self, plink->type, NULL, NULL));
str = plink ? nm_platform_link_get_udi (nm_device_get_platform (self), plink->ifindex) : NULL;
@@ -16974,6 +16983,7 @@ nm_device_class_init (NMDeviceClass *klass)
klass->get_type_description = get_type_description;
klass->can_auto_connect = can_auto_connect;
+ klass->can_update_from_platform_link = can_update_from_platform_link;
klass->check_connection_compatible = check_connection_compatible;
klass->check_connection_available = check_connection_available;
klass->can_unmanaged_external_down = can_unmanaged_external_down;
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index bcbc00bbedfa..0eab3aca19db 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -460,6 +460,8 @@ typedef struct _NMDeviceClass {
guint32 (* get_dhcp_timeout) (NMDevice *self,
int addr_family);
+ gboolean (* can_update_from_platform_link) (NMDevice *self, const NMPlatformLink *plink);
+
/* Controls, whether to call act_stage2_config() callback also for assuming
* a device or for external activations. In this case, act_stage2_config() must
* take care not to touch the device's configuration. */
diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c
index 726637864e69..9c74ffd1c22f 100644
--- a/src/devices/ovs/nm-device-ovs-interface.c
+++ b/src/devices/ovs/nm-device-ovs-interface.c
@@ -312,6 +312,18 @@ deactivate_async (NMDevice *device,
data);
}
+static gboolean
+can_update_from_platform_link (NMDevice *device, const NMPlatformLink *plink)
+{
+ /* If the device is deactivating, we already sent the
+ * deletion command to ovsdb and we don't want to deal
+ * with any new link appearing from the previous
+ * activation.
+ */
+ return !plink
+ || nm_device_get_state (device) != NM_DEVICE_STATE_DEACTIVATING;
+}
+
/*****************************************************************************/
static void
@@ -341,6 +353,7 @@ nm_device_ovs_interface_class_init (NMDeviceOvsInterfaceClass *klass)
device_class->connection_type_check_compatible = NM_SETTING_OVS_INTERFACE_SETTING_NAME;
device_class->link_types = NM_DEVICE_DEFINE_LINK_TYPES (NM_LINK_TYPE_OPENVSWITCH);
+ device_class->can_update_from_platform_link = can_update_from_platform_link;
device_class->deactivate = deactivate;
device_class->deactivate_async = deactivate_async;
device_class->get_type_description = get_type_description;
--
2.26.2
From 6bf0d9d596d04c37ffa03d634f844a770d120454 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 9 Mar 2020 11:19:47 +0100
Subject: [PATCH 13/25] ovs/ovsdb: support changing the MTU of an ovs interface
Introduce a nm_ovsdb_set_interface_mtu() function to update the MTU of
an ovs interface in the ovsdb.
(cherry picked from commit a4c2c1a843ff1492c1bfae2455a334b0d1c8389c)
(cherry picked from commit c1be15a66edf1ef1e7de1cc7a0f6e134b8dbdcb7)
(cherry picked from commit 990f46505d6d9bd4de40db458978b6c94664de13)
(cherry picked from commit 021141481e3cb7f747111dad67323f5d1a608bf6)
---
src/devices/ovs/nm-ovsdb.c | 77 ++++++++++++++++++++++++++++++--------
src/devices/ovs/nm-ovsdb.h | 3 ++
2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index c85115201eac..9ed8e06f5259 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -118,6 +118,7 @@ typedef enum {
OVSDB_MONITOR,
OVSDB_ADD_INTERFACE,
OVSDB_DEL_INTERFACE,
+ OVSDB_SET_INTERFACE_MTU,
} OvsdbCommand;
typedef struct {
@@ -127,7 +128,10 @@ typedef struct {
OvsdbMethodCallback callback;
gpointer user_data;
union {
- char *ifname;
+ struct {
+ char *ifname;
+ guint32 mtu;
+ };
struct {
NMConnection *bridge;
NMConnection *port;
@@ -169,6 +173,13 @@ _call_trace (const char *comment, OvsdbMethodCall *call, json_t *msg)
msg ? ": " : "",
msg ? str : "");
break;
+ case OVSDB_SET_INTERFACE_MTU:
+ _LOGT ("%s: set-iface-mtu interface=%s%s%s mtu=%u",
+ comment, call->ifname,
+ msg ? ": " : "",
+ msg ? str : "",
+ call->mtu);
+ break;
}
if (msg)
@@ -187,7 +198,7 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command,
const char *ifname,
NMConnection *bridge, NMConnection *port, NMConnection *interface,
NMDevice *bridge_device, NMDevice *interface_device,
- OvsdbMethodCallback callback, gpointer user_data)
+ guint32 mtu, OvsdbMethodCallback callback, gpointer user_data)
{
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
OvsdbMethodCall *call;
@@ -215,6 +226,10 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command,
case OVSDB_DEL_INTERFACE:
call->ifname = g_strdup (ifname);
break;
+ case OVSDB_SET_INTERFACE_MTU:
+ call->ifname = g_strdup (ifname);
+ call->mtu = mtu;
+ break;
}
_call_trace ("enqueue", call, NULL);
@@ -817,6 +832,22 @@ ovsdb_next_command (NMOvsdb *self)
_delete_interface (self, params, call->ifname);
+ msg = json_pack ("{s:i, s:s, s:o}",
+ "id", call->id,
+ "method", "transact", "params", params);
+ break;
+ case OVSDB_SET_INTERFACE_MTU:
+ params = json_array ();
+ json_array_append_new (params, json_string ("Open_vSwitch"));
+ json_array_append_new (params, _inc_next_cfg (priv->db_uuid));
+
+ json_array_append_new (params,
+ json_pack ("{s:s, s:s, s:{s: i}, s:[[s, s, s]]}",
+ "op", "update",
+ "table", "Interface",
+ "row", "mtu_request", call->mtu,
+ "where", "name", "==", call->ifname));
+
msg = json_pack ("{s:i, s:s, s:o}",
"id", call->id,
"method", "transact", "params", params);
@@ -1462,7 +1493,7 @@ ovsdb_try_connect (NMOvsdb *self)
/* Queue a monitor call before any other command, ensuring that we have an up
* to date view of existing bridged that we need for add and remove ops. */
ovsdb_call_method (self, OVSDB_MONITOR, NULL,
- NULL, NULL, NULL, NULL, NULL, _monitor_bridges_cb, NULL);
+ NULL, NULL, NULL, NULL, NULL, 0, _monitor_bridges_cb, NULL);
}
/*****************************************************************************/
@@ -1500,11 +1531,8 @@ out:
g_slice_free (OvsdbCall, call);
}
-void
-nm_ovsdb_add_interface (NMOvsdb *self,
- NMConnection *bridge, NMConnection *port, NMConnection *interface,
- NMDevice *bridge_device, NMDevice *interface_device,
- NMOvsdbCallback callback, gpointer user_data)
+static OvsdbCall *
+ovsdb_call_new (NMOvsdbCallback callback, gpointer user_data)
{
OvsdbCall *call;
@@ -1512,24 +1540,40 @@ nm_ovsdb_add_interface (NMOvsdb *self,
call->callback = callback;
call->user_data = user_data;
+ return call;
+}
+
+void
+nm_ovsdb_add_interface (NMOvsdb *self,
+ NMConnection *bridge, NMConnection *port, NMConnection *interface,
+ NMDevice *bridge_device, NMDevice *interface_device,
+ NMOvsdbCallback callback, gpointer user_data)
+{
ovsdb_call_method (self, OVSDB_ADD_INTERFACE, NULL,
bridge, port, interface,
bridge_device, interface_device,
- _transact_cb, call);
+ 0,
+ _transact_cb,
+ ovsdb_call_new (callback, user_data));
}
void
nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname,
NMOvsdbCallback callback, gpointer user_data)
{
- OvsdbCall *call;
-
- call = g_slice_new (OvsdbCall);
- call->callback = callback;
- call->user_data = user_data;
-
ovsdb_call_method (self, OVSDB_DEL_INTERFACE, ifname,
- NULL, NULL, NULL, NULL, NULL, _transact_cb, call);
+ NULL, NULL, NULL, NULL, NULL, 0,
+ _transact_cb,
+ ovsdb_call_new (callback, user_data));
+}
+
+void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu,
+ NMOvsdbCallback callback, gpointer user_data)
+{
+ ovsdb_call_method (self, OVSDB_SET_INTERFACE_MTU, ifname,
+ NULL, NULL, NULL, NULL, NULL, mtu,
+ _transact_cb,
+ ovsdb_call_new (callback, user_data));
}
/*****************************************************************************/
@@ -1550,6 +1594,7 @@ _clear_call (gpointer data)
g_clear_object (&call->interface_device);
break;
case OVSDB_DEL_INTERFACE:
+ case OVSDB_SET_INTERFACE_MTU:
g_clear_pointer (&call->ifname, g_free);
break;
}
diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h
index e7f9d71984c2..9552dcca3226 100644
--- a/src/devices/ovs/nm-ovsdb.h
+++ b/src/devices/ovs/nm-ovsdb.h
@@ -48,4 +48,7 @@ void nm_ovsdb_add_interface (NMOvsdb *self,
void nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname,
NMOvsdbCallback callback, gpointer user_data);
+void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu,
+ NMOvsdbCallback callback, gpointer user_data);
+
#endif /* __NETWORKMANAGER_OVSDB_H__ */
--
2.26.2
From 449dd37015d890ddf9385ad062dc4204974179ac Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 27 Feb 2020 13:54:43 +0100
Subject: [PATCH 14/25] ovs: set MTU from connection when creating an internal
interface
The ovs-vswitchd.conf.db(5) man page says about the the mtu_request
column in the Interface table:
"Requested MTU (Maximum Transmission Unit) for the interface. A
client can fill this column to change the MTU of an
interface [...] If this is not set and if the interface has
internal type, Open vSwitch will change the MTU to match the
minimum of the other interfaces in the bridge."
Therefore, if the connection specifies a MTU, set it early when adding
the interface to the ovsdb so that it will not be changed to the
minimum of other interfaces.
(cherry picked from commit ad12f26312bb4ccc73076f39bd2a618094af6e7e)
(cherry picked from commit 7311d5e2943931a70fb427096ea934072aed63b3)
(cherry picked from commit b81370f70b538688a62ad4a29482dbf9b34c69bc)
(cherry picked from commit 59787018c983e433da6172944e60246961c38d71)
---
src/devices/ovs/nm-ovsdb.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 9ed8e06f5259..753f5f172a09 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -367,11 +367,20 @@ _insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_
gs_free char *cloned_mac = NULL;
gs_free_error GError *error = NULL;
json_t *row;
+ guint32 mtu = 0;
s_ovs_iface = nm_connection_get_setting_ovs_interface (interface);
if (s_ovs_iface)
type = nm_setting_ovs_interface_get_interface_type (s_ovs_iface);
+ if (nm_streq0 (type, "internal")) {
+ NMSettingWired *s_wired;
+
+ s_wired = _nm_connection_get_setting (interface, NM_TYPE_SETTING_WIRED);
+ if (s_wired)
+ mtu = nm_setting_wired_get_mtu (s_wired);
+ }
+
if (!nm_device_hw_addr_get_cloned (interface_device,
interface,
FALSE,
@@ -404,6 +413,9 @@ _insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_
if (cloned_mac)
json_object_set_new (row, "mac", json_string (cloned_mac));
+ if (mtu != 0)
+ json_object_set_new (row, "mtu_request", json_integer (mtu));
+
json_array_append_new (params,
json_pack ("{s:s, s:s, s:o, s:s}",
"op", "insert",
--
2.26.2
From a337a7d144b2bc25f75a838042e540a3f7518105 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Mon, 16 Mar 2020 10:53:06 +0100
Subject: [PATCH 15/25] ovs: set the MTU in ovsdb when changing platform MTU of
ovs-interface
If we change the the MTU of an ovs interface only through netlink, the
change could be overridden by ovs-vswitchd at any time when other
interfaces change. Set the MTU also in the ovsdb to prevent such
changes.
Note that if the MTU comes from the connection, we already set the
ovsdb MTU at creation time and so this other update becomes
useless. But it is needed when changing the MTU at runtime (reapply)
or when the MTU comes from a different source (e.g. DHCP).
(cherry picked from commit c2a97129453c49f86976b42c29b793a87d9d2e41)
(cherry picked from commit e27a59c69e956961efe99f83068b760380b77066)
(cherry picked from commit 99ef891db6a561d6b3dcaec9ead2b2158b43ae8d)
(cherry picked from commit 04264be151f323a78dea23e58311c90d9928bd9e)
---
src/devices/nm-device.c | 17 +++++++---
src/devices/nm-device.h | 2 ++
src/devices/ovs/nm-device-ovs-interface.c | 39 +++++++++++++++++++++++
3 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 4bd93a510ee4..9bfd3704edc9 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -9152,6 +9152,17 @@ _set_mtu (NMDevice *self, guint32 mtu)
}
}
+static gboolean
+set_platform_mtu (NMDevice *self, guint32 mtu)
+{
+ int r;
+
+ r = nm_platform_link_set_mtu (nm_device_get_platform (self),
+ nm_device_get_ip_ifindex (self),
+ mtu);
+ return (r != -NME_PL_CANT_SET_MTU);
+}
+
static void
_commit_mtu (NMDevice *self, const NMIP4Config *config)
{
@@ -9310,10 +9321,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
}
if (mtu_desired && mtu_desired != mtu_plat) {
- int r;
-
- r = nm_platform_link_set_mtu (nm_device_get_platform (self), ifindex, mtu_desired);
- if (r == -NME_PL_CANT_SET_MTU) {
+ if (!NM_DEVICE_GET_CLASS (self)->set_platform_mtu (self, mtu_desired)) {
anticipated_failure = TRUE;
success = FALSE;
_LOGW (LOGD_DEVICE, "mtu: failure to set MTU. %s",
@@ -16996,6 +17004,7 @@ nm_device_class_init (NMDeviceClass *klass)
klass->parent_changed_notify = parent_changed_notify;
klass->can_reapply_change = can_reapply_change;
klass->reapply_connection = reapply_connection;
+ klass->set_platform_mtu = set_platform_mtu;
obj_properties[PROP_UDI] =
g_param_spec_string (NM_DEVICE_UDI, "", "",
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 0eab3aca19db..5a9339d03d1d 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -462,6 +462,8 @@ typedef struct _NMDeviceClass {
gboolean (* can_update_from_platform_link) (NMDevice *self, const NMPlatformLink *plink);
+ gboolean (* set_platform_mtu) (NMDevice *self, guint32 mtu);
+
/* Controls, whether to call act_stage2_config() callback also for assuming
* a device or for external activations. In this case, act_stage2_config() must
* take care not to touch the device's configuration. */
diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c
index 9c74ffd1c22f..f2919b488190 100644
--- a/src/devices/ovs/nm-device-ovs-interface.c
+++ b/src/devices/ovs/nm-device-ovs-interface.c
@@ -134,6 +134,43 @@ _is_internal_interface (NMDevice *device)
return nm_streq (nm_setting_ovs_interface_get_interface_type (s_ovs_iface), "internal");
}
+static void
+set_platform_mtu_cb (GError *error, gpointer user_data)
+{
+ NMDevice *device = user_data;
+ NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE (device);
+
+ if ( error
+ && !g_error_matches (error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING)) {
+ _LOGW (LOGD_DEVICE, "could not change mtu of '%s': %s",
+ nm_device_get_iface (device), error->message);
+ }
+
+ g_object_unref (device);
+}
+
+static gboolean
+set_platform_mtu (NMDevice *device, guint32 mtu)
+{
+ /*
+ * If the MTU is not set in ovsdb, Open vSwitch will change
+ * the MTU of an internal interface to match the minimum of
+ * the other interfaces in the bridge.
+ */
+ /* FIXME(shutdown): the function should become cancellable so
+ * that it doesn't need to hold a reference to the device, and
+ * it can be stopped during shutdown.
+ */
+ if (_is_internal_interface (device)) {
+ nm_ovsdb_set_interface_mtu (nm_ovsdb_get (),
+ nm_device_get_ip_iface (device),
+ mtu, set_platform_mtu_cb,
+ g_object_ref (device));
+ }
+
+ return NM_DEVICE_CLASS (nm_device_ovs_interface_parent_class)->set_platform_mtu (device, mtu);
+}
+
static NMActStageReturn
act_stage3_ip_config_start (NMDevice *device,
int addr_family,
@@ -364,4 +401,6 @@ nm_device_ovs_interface_class_init (NMDeviceOvsInterfaceClass *klass)
device_class->link_changed = link_changed;
device_class->act_stage3_ip_config_start = act_stage3_ip_config_start;
device_class->can_unmanaged_external_down = can_unmanaged_external_down;
+ device_class->set_platform_mtu = set_platform_mtu;
+ device_class->get_configured_mtu = nm_device_get_configured_mtu_for_wired;
}
--
2.26.2
From 8d3cd486012fdd04a1f9d2e976b7cf9f869b0205 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Fri, 3 Apr 2020 09:13:51 +0200
Subject: [PATCH 16/25] ovsdb: retry calls in case of communication error with
server
When the server is restarted the write to unix socket fails with
EPIPE. In such case, don't fail all the calls in queue; instead, after
a sync of the ovsdb state (through a monitor call), start processing
the queue again, including the call that previously failed.
Add a retry counter to avoid that calls are stuck in the queue forever
in a hypothetical scenario in which the write always fails.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/459
(cherry picked from commit db37e530e8f03b7882997194bb325a206555c44d)
(cherry picked from commit 54254bf6fe869b83189f43cdf13652c165d5aa71)
(cherry picked from commit 166ad887f9d2fa361dcc6e0926e21e25c7a4ce2d)
(cherry picked from commit b2d597695073866aa646e76df99170bc14a47cbe)
---
src/devices/ovs/nm-ovsdb.c | 81 ++++++++++++++++++++++++++------------
1 file changed, 55 insertions(+), 26 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 753f5f172a09..7e9214e8676c 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -79,6 +79,7 @@ typedef struct {
GHashTable *ports; /* port uuid => OpenvswitchPort */
GHashTable *bridges; /* bridge uuid => OpenvswitchBridge */
char *db_uuid;
+ guint num_failures;
} NMOvsdbPrivate;
struct _NMOvsdb {
@@ -102,7 +103,7 @@ NM_DEFINE_SINGLETON_GETTER (NMOvsdb, nm_ovsdb_get, NM_TYPE_OVSDB);
/*****************************************************************************/
static void ovsdb_try_connect (NMOvsdb *self);
-static void ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing);
+static void ovsdb_disconnect (NMOvsdb *self, gboolean retry, gboolean is_disposing);
static void ovsdb_read (NMOvsdb *self);
static void ovsdb_write (NMOvsdb *self);
static void ovsdb_next_command (NMOvsdb *self);
@@ -142,6 +143,8 @@ typedef struct {
};
} OvsdbMethodCall;
+#define OVSDB_MAX_FAILURES 3
+
static void
_call_trace (const char *comment, OvsdbMethodCall *call, json_t *msg)
{
@@ -198,7 +201,8 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command,
const char *ifname,
NMConnection *bridge, NMConnection *port, NMConnection *interface,
NMDevice *bridge_device, NMDevice *interface_device,
- guint32 mtu, OvsdbMethodCallback callback, gpointer user_data)
+ guint32 mtu, OvsdbMethodCallback callback, gpointer user_data,
+ gboolean add_first)
{
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
OvsdbMethodCall *call;
@@ -206,8 +210,13 @@ ovsdb_call_method (NMOvsdb *self, OvsdbCommand command,
/* Ensure we're not unsynchronized before we queue the method call. */
ovsdb_try_connect (self);
- g_array_set_size (priv->calls, priv->calls->len + 1);
- call = &g_array_index (priv->calls, OvsdbMethodCall, priv->calls->len - 1);
+ if (add_first) {
+ g_array_prepend_val (priv->calls, (OvsdbMethodCall) {});
+ call = &g_array_index (priv->calls, OvsdbMethodCall, 0);
+ } else {
+ g_array_set_size (priv->calls, priv->calls->len + 1);
+ call = &g_array_index (priv->calls, OvsdbMethodCall, priv->calls->len - 1);
+ }
call->id = COMMAND_PENDING;
call->command = command;
call->callback = callback;
@@ -1197,7 +1206,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
"result", &result,
"error", &error) == -1) {
_LOGW ("couldn't grok the message: %s", json_error.text);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
return;
}
@@ -1208,7 +1217,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
/* It's a method call! */
if (!params) {
_LOGW ("a method call with no params: '%s'", method);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
return;
}
@@ -1228,13 +1237,13 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
/* This is a response to a method call. */
if (!priv->calls->len) {
_LOGE ("there are no queued calls expecting response %" G_GUINT64_FORMAT, id);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
return;
}
call = &g_array_index (priv->calls, OvsdbMethodCall, 0);
if (call->id != id) {
_LOGE ("expected a response to call %" G_GUINT64_FORMAT ", not %" G_GUINT64_FORMAT, call->id, id);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
return;
}
/* Cool, we found a corresponding call. Finish it. */
@@ -1252,6 +1261,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
user_data = call->user_data;
g_array_remove_index (priv->calls, 0);
callback (self, result, local, user_data);
+ priv->num_failures = 0;
/* Don't progress further commands in case the callback hit an error
* and disconnected us. */
@@ -1310,9 +1320,11 @@ ovsdb_read_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
size = g_input_stream_read_finish (stream, res, &error);
if (size == -1) {
+ /* ovsdb-server was possibly restarted */
_LOGW ("short read from ovsdb: %s", error->message);
+ priv->num_failures++;
g_clear_error (&error);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, priv->num_failures <= OVSDB_MAX_FAILURES, FALSE);
return;
}
@@ -1358,9 +1370,11 @@ ovsdb_write_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
size = g_output_stream_write_finish (stream, res, &error);
if (size == -1) {
+ /* ovsdb-server was possibly restarted */
_LOGW ("short write to ovsdb: %s", error->message);
+ priv->num_failures++;
g_clear_error (&error);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, priv->num_failures <= OVSDB_MAX_FAILURES, FALSE);
return;
}
@@ -1403,7 +1417,7 @@ ovsdb_write (NMOvsdb *self)
* puts us back in sync.
*/
static void
-ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing)
+ovsdb_disconnect (NMOvsdb *self, gboolean retry, gboolean is_disposing)
{
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
OvsdbMethodCall *call;
@@ -1411,18 +1425,26 @@ ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing)
gpointer user_data;
gs_free_error GError *error = NULL;
+ nm_assert (!retry || !is_disposing);
+
if (!priv->client)
return;
- _LOGD ("disconnecting from ovsdb");
- nm_utils_error_set_cancelled (&error, is_disposing, "NMOvsdb");
+ _LOGD ("disconnecting from ovsdb, retry %d", retry);
- while (priv->calls->len) {
- call = &g_array_index (priv->calls, OvsdbMethodCall, priv->calls->len - 1);
- callback = call->callback;
- user_data = call->user_data;
- g_array_remove_index (priv->calls, priv->calls->len - 1);
- callback (self, NULL, error, user_data);
+ if (retry) {
+ if (priv->calls->len != 0)
+ g_array_index (priv->calls, OvsdbMethodCall, 0).id = COMMAND_PENDING;
+ } else {
+ nm_utils_error_set_cancelled (&error, is_disposing, "NMOvsdb");
+
+ while (priv->calls->len) {
+ call = &g_array_index (priv->calls, OvsdbMethodCall, priv->calls->len - 1);
+ callback = call->callback;
+ user_data = call->user_data;
+ g_array_remove_index (priv->calls, priv->calls->len - 1);
+ callback (self, NULL, error, user_data);
+ }
}
priv->bufp = 0;
@@ -1432,6 +1454,9 @@ ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing)
g_clear_object (&priv->conn);
g_clear_pointer (&priv->db_uuid, g_free);
nm_clear_g_cancellable (&priv->cancellable);
+
+ if (retry)
+ ovsdb_try_connect (self);
}
static void
@@ -1440,7 +1465,7 @@ _monitor_bridges_cb (NMOvsdb *self, json_t *result, GError *error, gpointer user
if (error) {
if (!nm_utils_error_is_cancelled (error, TRUE)) {
_LOGI ("%s", error->message);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
}
return;
}
@@ -1464,7 +1489,7 @@ _client_connect_cb (GObject *source_object, GAsyncResult *res, gpointer user_dat
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
_LOGI ("%s", error->message);
- ovsdb_disconnect (self, FALSE);
+ ovsdb_disconnect (self, FALSE, FALSE);
g_clear_error (&error);
return;
}
@@ -1505,7 +1530,8 @@ ovsdb_try_connect (NMOvsdb *self)
/* Queue a monitor call before any other command, ensuring that we have an up
* to date view of existing bridged that we need for add and remove ops. */
ovsdb_call_method (self, OVSDB_MONITOR, NULL,
- NULL, NULL, NULL, NULL, NULL, 0, _monitor_bridges_cb, NULL);
+ NULL, NULL, NULL, NULL, NULL, 0,
+ _monitor_bridges_cb, NULL, TRUE);
}
/*****************************************************************************/
@@ -1566,7 +1592,8 @@ nm_ovsdb_add_interface (NMOvsdb *self,
bridge_device, interface_device,
0,
_transact_cb,
- ovsdb_call_new (callback, user_data));
+ ovsdb_call_new (callback, user_data),
+ FALSE);
}
void
@@ -1576,7 +1603,8 @@ nm_ovsdb_del_interface (NMOvsdb *self, const char *ifname,
ovsdb_call_method (self, OVSDB_DEL_INTERFACE, ifname,
NULL, NULL, NULL, NULL, NULL, 0,
_transact_cb,
- ovsdb_call_new (callback, user_data));
+ ovsdb_call_new (callback, user_data),
+ FALSE);
}
void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu,
@@ -1585,7 +1613,8 @@ void nm_ovsdb_set_interface_mtu (NMOvsdb *self, const char *ifname, guint32 mtu,
ovsdb_call_method (self, OVSDB_SET_INTERFACE_MTU, ifname,
NULL, NULL, NULL, NULL, NULL, mtu,
_transact_cb,
- ovsdb_call_new (callback, user_data));
+ ovsdb_call_new (callback, user_data),
+ FALSE);
}
/*****************************************************************************/
@@ -1667,7 +1696,7 @@ dispose (GObject *object)
NMOvsdb *self = NM_OVSDB (object);
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
- ovsdb_disconnect (self, TRUE);
+ ovsdb_disconnect (self, FALSE, TRUE);
if (priv->input) {
g_string_free (priv->input, TRUE);
--
2.26.2
From f6be05697997595cc844b827d0ad11c66b7482ed Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Tue, 9 Jun 2020 10:44:01 +0200
Subject: [PATCH 17/25] 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)
(cherry picked from commit 399aad15bf53420cc015c7a920856f88d5584dfc)
(cherry picked from commit 692689ead8aaf4f1441a82e546649d996bd812d4)
(cherry picked from commit 4c8edaedc7a1cf6af91edb56c8b7f8db1b28c704)
---
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 fdf07bd3750a..f83c9e582730 100644
--- a/src/devices/ovs/nm-ovs-factory.c
+++ b/src/devices/ovs/nm-ovs-factory.c
@@ -155,15 +155,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,
--
2.26.2
From ceaeab87773f830f91ef2de84be9e1d12bce76a8 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 2 Jul 2020 17:38:10 +0200
Subject: [PATCH 18/25] device: restart DHCP only for devices that are active
or activating
do_sleep_wake() tries to restart DHCP for all devices, even ones that
are disconnecting. When a device is disconnecting, it still has a DHCP
client instance but we shouldn't restart it because it makes no sense;
and especially, the device could be already removed.
https://bugzilla.redhat.com/show_bug.cgi?id=1852612
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/561
(cherry picked from commit 2c50438987527a30d99c07601b8f1d1c9557cdaf)
(cherry picked from commit 53214901800ebcd1e1fabf98442983939af979bc)
(cherry picked from commit ef755588ad55261c6ade4ba0278d51d777022183)
(cherry picked from commit da54b35af3b0a5120b4703060ee0b95cbc4fd0bd)
(cherry picked from commit b0be1285cceff5de2eef44a56abda6a98d51c91a)
(cherry picked from commit 03b963451952435b9607c2335b24d9da6fd73745)
---
src/devices/nm-device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 9bfd3704edc9..c2eb815b3b3a 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -3412,6 +3412,10 @@ nm_device_update_dynamic_ip_setup (NMDevice *self)
priv = NM_DEVICE_GET_PRIVATE (self);
+ if ( priv->state < NM_DEVICE_STATE_IP_CONFIG
+ || priv->state > NM_DEVICE_STATE_ACTIVATED)
+ return;
+
g_hash_table_remove_all (priv->ip6_saved_properties);
if (priv->dhcp4.client) {
--
2.26.2
From 7f44570ee67d33450f55402120682ae4d1a9a314 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 16 Jul 2020 11:57:14 +0200
Subject: [PATCH 19/25] manager: fix race condition when resuming from sleep
If the device state change (to disconnected or unmanaged) triggered by
a sleep event happens after the wake, the devices becomes wrongly
unmanaged and it's necessary to manually manage it again, or restart
NM.
During the wake event we should disconnect the device_sleep_cb()
callback for all devices because we don't want to react to state
changes anymore; in particular we don't need to detect when the device
becomes disconnected to unmanage it.
(cherry picked from commit fe2d93980bd5b61c55a8b65a55f7aad35042e691)
(cherry picked from commit 971897195a8218cb0ec08ae95a7210fce73f0b03)
(cherry picked from commit 7913275b02c60803f21609168f9c16d2f2a4be2f)
(cherry picked from commit 6d0e8a2acfa6b7b6a2af1834d8873fc18fc2c55a)
(cherry picked from commit 61c44dad91733bc09e20752915476a7678a73b90)
(cherry picked from commit b3ec3fa5abecdffadaaef59e1f8401cf90caf884)
---
src/nm-manager.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 68eff1e466c0..e702fd90a55a 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -5904,8 +5904,9 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed)
} else {
_LOGD (LOGD_SUSPEND, "sleep: %s...", waking_from_suspend ? "waking up" : "re-enabling");
+ sleep_devices_clear (self);
+
if (waking_from_suspend) {
- sleep_devices_clear (self);
c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) {
if (nm_device_is_software (device))
continue;
--
2.26.2
From ab2b474f7feefd61da5ee5f2cfc5623e07e48300 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 16 Jul 2020 11:58:20 +0200
Subject: [PATCH 20/25] policy: block connection from autoconnect in case of
failed dependency
A connection that fails due to dependency-failed is not able to
reconnect until the master connection activates again; when this
happens, the master clears the blocked reason for all its slaves in
activate_slave_connections() and tries to reconnect them. For this to
work, the slave should be marked as blocked when it fails with
dependency-failed.
(cherry picked from commit 725fed01cf7c8508cf426897340b2a4113406aab)
(cherry picked from commit e1755048e35aca682c7d0d233122d4ddaf3bb089)
(cherry picked from commit ecb134ac349a0bf6582833253b6d453d6bf997de)
(cherry picked from commit bb4781cc581a3c9fcefbfc1ea0be5e95aa3e287b)
(cherry picked from commit 70c642325f90f767954aa86079b1bc6a4946f15f)
(cherry picked from commit 8d87f752db68652b4be1d1e3e4f03f82a0b14e83)
---
src/nm-policy.c | 33 +++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/src/nm-policy.c b/src/nm-policy.c
index 1faba5c7be04..5625986d1d85 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -1807,7 +1807,7 @@ device_state_changed (NMDevice *device,
if ( sett_conn
&& old_state >= NM_DEVICE_STATE_PREPARE
&& old_state <= NM_DEVICE_STATE_ACTIVATED) {
- gboolean block_no_secrets = FALSE;
+ gboolean blocked = FALSE;
int tries;
guint64 con_v;
@@ -1827,15 +1827,32 @@ device_state_changed (NMDevice *device,
*/
con_v = nm_settings_connection_get_last_secret_agent_version_id (sett_conn);
if ( con_v == 0
- || con_v == nm_agent_manager_get_agent_version_id (priv->agent_mgr))
- block_no_secrets = TRUE;
+ || con_v == nm_agent_manager_get_agent_version_id (priv->agent_mgr)) {
+ _LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets",
+ nm_settings_connection_get_id (sett_conn));
+ nm_settings_connection_autoconnect_blocked_reason_set (sett_conn,
+ NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS,
+ TRUE);
+ blocked = TRUE;
+ }
+ } else if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) {
+ /* A connection that fails due to dependency-failed is not
+ * able to reconnect until the master connection activates
+ * again; when this happens, the master clears the blocked
+ * reason for all its slaves in activate_slave_connections()
+ * and tries to reconnect them. For this to work, the slave
+ * should be marked as blocked when it fails with
+ * dependency-failed.
+ */
+ _LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to failed dependency",
+ nm_settings_connection_get_id (sett_conn));
+ nm_settings_connection_autoconnect_blocked_reason_set (sett_conn,
+ NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED,
+ TRUE);
+ blocked = TRUE;
}
- if (block_no_secrets) {
- _LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets",
- nm_settings_connection_get_id (sett_conn));
- nm_settings_connection_autoconnect_blocked_reason_set (sett_conn, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, TRUE);
- } else {
+ if (!blocked) {
tries = nm_settings_connection_autoconnect_retries_get (sett_conn);
if (tries > 0) {
_LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left",
--
2.26.2
From a19fe2332ad4c9344c6c880ec7fa65318bedd50c Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 1 Jul 2020 10:55:01 +0200
Subject: [PATCH 21/25] ovs: set MAC address on the bridge for local interfaces
When a user creates a ovs-interface with the same name of the parent
ovs-bridge, openvswitch considers the interface as the "local
interface" [1] and assigns the MAC address of the bridge to the
interface [2].
This is confusing for users, as the cloned MAC property is ignored in
some cases, depending on the ovs-interface name.
Instead, detect when the interface is local and set the MAC from the
ovs-interface connection in the bridge table.
[1] https://github.com/openvswitch/ovs/blob/v2.13.0/vswitchd/vswitch.xml#L2546
[2] https://github.com/openvswitch/ovs/blob/v2.13.0/vswitchd/bridge.c#L4744
(cherry picked from commit 5d4c8521a38c166a9a0aafe5be2bd0545084e154)
(cherry picked from commit 7548c29a89e12349fdfe196e96d65a6abae74cb5)
(cherry picked from commit 127294babc23782781c17192c52a2d57f7916170)
(cherry picked from commit f54c5400c8e43e71fb7c4e1e705866862af69ccd)
(cherry picked from commit 1a08885080855c3ee14f181c7bd236680663c97f)
(cherry picked from commit 9493f1f151cb7647f69af790d31086748f98424e)
---
src/devices/ovs/nm-ovsdb.c | 114 +++++++++++++++++++++++++------------
1 file changed, 77 insertions(+), 37 deletions(-)
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 7e9214e8676c..cedcb1d9af9b 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -324,6 +324,18 @@ _set_bridge_ports (json_t *params, const char *ifname, json_t *new_ports)
);
}
+static void
+_set_bridge_mac (json_t *params, const char *ifname, const char *mac)
+{
+ json_array_append_new (params,
+ json_pack ("{s:s, s:s, s:{s:[s, [[s, s]]]}, s:[[s, s, s]]}",
+ "op", "update", "table", "Bridge",
+ "row", "other_config", "map",
+ "hwaddr", mac,
+ "where", "name", "==", ifname)
+ );
+}
+
/**
* _expect_port_interfaces:
*
@@ -367,14 +379,15 @@ _set_port_interfaces (json_t *params, const char *ifname, json_t *new_interfaces
* Returns an commands that adds new interface from a given connection.
*/
static void
-_insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_device)
+_insert_interface (json_t *params,
+ NMConnection *interface,
+ NMDevice *interface_device,
+ const char *cloned_mac)
{
const char *type = NULL;
NMSettingOvsInterface *s_ovs_iface;
NMSettingOvsPatch *s_ovs_patch;
json_t *options = json_array ();
- gs_free char *cloned_mac = NULL;
- gs_free_error GError *error = NULL;
json_t *row;
guint32 mtu = 0;
@@ -390,18 +403,6 @@ _insert_interface (json_t *params, NMConnection *interface, NMDevice *interface_
mtu = nm_setting_wired_get_mtu (s_wired);
}
- if (!nm_device_hw_addr_get_cloned (interface_device,
- interface,
- FALSE,
- &cloned_mac,
- NULL,
- &error)) {
- _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s",
- "interface",
- nm_connection_get_interface_name (interface),
- error->message);
- }
-
json_array_append_new (options, json_string ("map"));
s_ovs_patch = nm_connection_get_setting_ovs_patch (interface);
if (s_ovs_patch) {
@@ -494,7 +495,11 @@ _insert_port (json_t *params, NMConnection *port, json_t *new_interfaces)
* Returns an commands that adds new bridge from a given connection.
*/
static void
-_insert_bridge (json_t *params, NMConnection *bridge, NMDevice *bridge_device, json_t *new_ports)
+_insert_bridge (json_t *params,
+ NMConnection *bridge,
+ NMDevice *bridge_device,
+ json_t *new_ports,
+ const char *cloned_mac)
{
NMSettingOvsBridge *s_ovs_bridge;
const char *fail_mode = NULL;
@@ -502,23 +507,9 @@ _insert_bridge (json_t *params, NMConnection *bridge, NMDevice *bridge_device, j
gboolean rstp_enable = FALSE;
gboolean stp_enable = FALSE;
json_t *row;
- gs_free_error GError *error = NULL;
- gs_free char *cloned_mac = NULL;
s_ovs_bridge = nm_connection_get_setting_ovs_bridge (bridge);
- if (!nm_device_hw_addr_get_cloned (bridge_device,
- bridge,
- FALSE,
- &cloned_mac,
- NULL,
- &error)) {
- _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s",
- "bridge",
- nm_connection_get_interface_name (bridge),
- error->message);
- }
-
row = json_object ();
if (s_ovs_bridge) {
@@ -586,6 +577,9 @@ _add_interface (NMOvsdb *self, json_t *params,
const char *bridge_uuid;
const char *port_uuid;
const char *interface_uuid;
+ const char *bridge_name;
+ const char *port_name;
+ const char *interface_name;
OpenvswitchBridge *ovs_bridge = NULL;
OpenvswitchPort *ovs_port = NULL;
OpenvswitchInterface *ovs_interface = NULL;
@@ -596,6 +590,10 @@ _add_interface (NMOvsdb *self, json_t *params,
nm_auto_decref_json json_t *interfaces = NULL;
nm_auto_decref_json json_t *new_interfaces = NULL;
gboolean has_interface = FALSE;
+ gboolean interface_is_internal;
+ gs_free char *bridge_cloned_mac = NULL;
+ gs_free char *interface_cloned_mac = NULL;
+ GError *error = NULL;
int pi;
int ii;
@@ -606,11 +604,51 @@ _add_interface (NMOvsdb *self, json_t *params,
new_ports = json_array ();
new_interfaces = json_array ();
+ bridge_name = nm_connection_get_interface_name (bridge);
+ port_name = nm_connection_get_interface_name (port);
+ interface_name = nm_connection_get_interface_name (interface);
+ interface_is_internal = nm_streq0 (bridge_name, interface_name);
+
+ /* Determine cloned MAC addresses */
+ if (!nm_device_hw_addr_get_cloned (bridge_device,
+ bridge,
+ FALSE,
+ &bridge_cloned_mac,
+ NULL,
+ &error)) {
+ _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s",
+ "bridge",
+ bridge_name,
+ error->message);
+ g_clear_error (&error);
+ }
+
+ if (!nm_device_hw_addr_get_cloned (interface_device,
+ interface,
+ FALSE,
+ &interface_cloned_mac,
+ NULL,
+ &error)) {
+ _LOGW ("Cannot determine cloned mac for OVS %s '%s': %s",
+ "interface",
+ interface_name,
+ error->message);
+ g_clear_error (&error);
+ }
+
+ if ( interface_is_internal
+ && !bridge_cloned_mac
+ && interface_cloned_mac) {
+ _LOGT ("'%s' is a local ovs-interface, the MAC will be set on ovs-bridge '%s'",
+ interface_name, bridge_name);
+ bridge_cloned_mac = g_steal_pointer (&interface_cloned_mac);
+ }
+
g_hash_table_iter_init (&iter, priv->bridges);
while (g_hash_table_iter_next (&iter, (gpointer) &bridge_uuid, (gpointer) &ovs_bridge)) {
json_array_append_new (bridges, json_pack ("[s, s]", "uuid", bridge_uuid));
- if ( g_strcmp0 (ovs_bridge->name, nm_connection_get_interface_name (bridge)) != 0
+ if ( g_strcmp0 (ovs_bridge->name, bridge_name) != 0
|| g_strcmp0 (ovs_bridge->connection_uuid, nm_connection_get_uuid (bridge)) != 0)
continue;
@@ -624,7 +662,7 @@ _add_interface (NMOvsdb *self, json_t *params,
/* This would be a violation of ovsdb's reference integrity (a bug). */
_LOGW ("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid);
continue;
- } else if ( strcmp (ovs_port->name, nm_connection_get_interface_name (port)) != 0
+ } else if ( strcmp (ovs_port->name, port_name) != 0
|| g_strcmp0 (ovs_port->connection_uuid, nm_connection_get_uuid (port)) != 0) {
continue;
}
@@ -638,7 +676,7 @@ _add_interface (NMOvsdb *self, json_t *params,
if (!ovs_interface) {
/* This would be a violation of ovsdb's reference integrity (a bug). */
_LOGW ("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid);
- } else if ( strcmp (ovs_interface->name, nm_connection_get_interface_name (interface)) == 0
+ } else if ( strcmp (ovs_interface->name, interface_name) == 0
&& g_strcmp0 (ovs_interface->connection_uuid, nm_connection_get_uuid (interface)) == 0) {
has_interface = TRUE;
}
@@ -661,12 +699,14 @@ _add_interface (NMOvsdb *self, json_t *params,
_expect_ovs_bridges (params, priv->db_uuid, bridges);
json_array_append_new (new_bridges, json_pack ("[s, s]", "named-uuid", "rowBridge"));
_set_ovs_bridges (params, priv->db_uuid, new_bridges);
- _insert_bridge (params, bridge, bridge_device, new_ports);
+ _insert_bridge (params, bridge, bridge_device, new_ports, bridge_cloned_mac);
} else {
/* Bridge already exists. */
g_return_if_fail (ovs_bridge);
_expect_bridge_ports (params, ovs_bridge->name, ports);
- _set_bridge_ports (params, nm_connection_get_interface_name (bridge), new_ports);
+ _set_bridge_ports (params, bridge_name, new_ports);
+ if (bridge_cloned_mac && interface_is_internal)
+ _set_bridge_mac (params, bridge_name, bridge_cloned_mac);
}
json_array_append_new (new_ports, json_pack ("[s, s]", "named-uuid", "rowPort"));
@@ -675,11 +715,11 @@ _add_interface (NMOvsdb *self, json_t *params,
/* Port already exists */
g_return_if_fail (ovs_port);
_expect_port_interfaces (params, ovs_port->name, interfaces);
- _set_port_interfaces (params, nm_connection_get_interface_name (port), new_interfaces);
+ _set_port_interfaces (params, port_name, new_interfaces);
}
if (!has_interface) {
- _insert_interface (params, interface, interface_device);
+ _insert_interface (params, interface, interface_device, interface_cloned_mac);
json_array_append_new (new_interfaces, json_pack ("[s, s]", "named-uuid", "rowInterface"));
}
}
--
2.26.2
From c31781abf41bc3be2f81d3a164522ae17251a9d5 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 1 Jul 2020 09:01:10 +0200
Subject: [PATCH 22/25] ovs: also set cloned MAC address via netlink
We already set the MAC of OVS interfaces in the ovsdb. Unfortunately,
vswitchd doesn't create the interface with the given MAC from the
beginning, but first creates it with a random MAC and then changes it.
This causes a race condition: as soon as NM sees the new link, it
starts IP configuration on it and (possibly later) vswitchd will
change the MAC.
To avoid this, also set the desired MAC via netlink before starting IP
configuration.
https://bugzilla.redhat.com/show_bug.cgi?id=1852106
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/483
(cherry picked from commit 47ec3d14d49eb9e9db956b3efd495d5d696da996)
(cherry picked from commit 60d10b146d57290f146536705d744e67925de90e)
(cherry picked from commit 01399955906aa1b1e52998e8daf487b30fa6eb81)
(cherry picked from commit 69c5c5e7670e8199829df308316d055be049c978)
(cherry picked from commit 91d2b0fd5ae4960420da0321dd1b11f8333f7d4d)
(cherry picked from commit 037ff2870842e7aa4d2d763134015a8e9abb6327)
---
src/devices/ovs/nm-device-ovs-interface.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/src/devices/ovs/nm-device-ovs-interface.c b/src/devices/ovs/nm-device-ovs-interface.c
index f2919b488190..5d103c16fec9 100644
--- a/src/devices/ovs/nm-device-ovs-interface.c
+++ b/src/devices/ovs/nm-device-ovs-interface.c
@@ -117,6 +117,14 @@ link_changed (NMDevice *device,
priv->waiting_for_interface = FALSE;
if (nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
+ if (!nm_device_hw_addr_set_cloned (device,
+ nm_device_get_applied_connection (device),
+ FALSE)) {
+ nm_device_state_changed (device,
+ NM_DEVICE_STATE_FAILED,
+ NM_DEVICE_STATE_REASON_CONFIG_FAILED);
+ return;
+ }
nm_device_bring_up (device, TRUE, NULL);
nm_device_activate_schedule_stage3_ip_config_start (device);
}
@@ -189,6 +197,13 @@ act_stage3_ip_config_start (NMDevice *device,
return NM_ACT_STAGE_RETURN_POSTPONE;
}
+ if (!nm_device_hw_addr_set_cloned (device,
+ nm_device_get_applied_connection (device),
+ FALSE)) {
+ *out_failure_reason = NM_DEVICE_STATE_REASON_CONFIG_FAILED;
+ return NM_ACT_STAGE_RETURN_FAILURE;
+ }
+
return NM_DEVICE_CLASS (nm_device_ovs_interface_parent_class)->act_stage3_ip_config_start (device, addr_family, out_config, out_failure_reason);
}
--
2.26.2
From b7a6efed8e5c35527f9a4d9ccb4e871202e9dbd6 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Thu, 2 Jul 2020 13:47:22 +0200
Subject: [PATCH 23/25] device: don't reset the MAC without ifindex
nm_device_cleanup() can be called when the device no longer has an
ifindex. In such case, don't try to reset the MAC address as that
would lead to an assertion failure.
(cherry picked from commit 77b6ce7d04f6c88e78fb7f1972549956e00e1f4b)
(cherry picked from commit 791a888cad3d260675781c0ed30acf13cc1194f7)
(cherry picked from commit e1f76e70447049487b2d8240ce12007624ab3c29)
(cherry picked from commit 5f22c06c53d9ae9003cd17603d6d6ff3e16f195b)
(cherry picked from commit 6beaa83d32d4c2784771b114725c34056f7083bf)
(cherry picked from commit baf91970b3954eb229cb8c980295ef273b0a2b31)
---
src/devices/nm-device.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index c2eb815b3b3a..346b8e743299 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -14649,17 +14649,19 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean
nm_device_update_metered (self);
- /* during device cleanup, we want to reset the MAC address of the device
- * to the initial state.
- *
- * We certainly want to do that when reaching the UNMANAGED state... */
- if (nm_device_get_state (self) <= NM_DEVICE_STATE_UNMANAGED)
- nm_device_hw_addr_reset (self, "unmanage");
- else {
- /* for other device states (UNAVAILABLE, DISCONNECTED), allow the
- * device to overwrite the reset behavior, so that Wi-Fi can set
- * a randomized MAC address used during scanning. */
- NM_DEVICE_GET_CLASS (self)->deactivate_reset_hw_addr (self);
+ if (ifindex > 0) {
+ /* during device cleanup, we want to reset the MAC address of the device
+ * to the initial state.
+ *
+ * We certainly want to do that when reaching the UNMANAGED state... */
+ if (nm_device_get_state (self) <= NM_DEVICE_STATE_UNMANAGED)
+ nm_device_hw_addr_reset (self, "unmanage");
+ else {
+ /* for other device states (UNAVAILABLE, DISCONNECTED), allow the
+ * device to overwrite the reset behavior, so that Wi-Fi can set
+ * a randomized MAC address used during scanning. */
+ NM_DEVICE_GET_CLASS (self)->deactivate_reset_hw_addr (self);
+ }
}
priv->mtu_source = NM_DEVICE_MTU_SOURCE_NONE;
--
2.26.2
From 4a4ad9bbd9393b3ef21911ea801d14d3d157dc08 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Tue, 9 Jun 2020 10:36:03 +0200
Subject: [PATCH 24/25] 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.
Backport: note that on nm-1-22, we have nm_utils_ifname_valid() function
for validating OVS interface names. We don't have that here, so we
re-implement the name validation differently.
Fixes: d4a7fe46797b ('libnm-core: add ovs-patch setting')
(cherry picked from commit beb1dba8c145be0971a488dcabac241e1c3a363b)
(cherry picked from commit 5598c039e4557510b567f12418601f5983fe5357)
(cherry picked from commit 9b82c62f33d53192cd89ae9c62f67ddad2c2df1f)
(cherry picked from commit 87e79d214e45cee4e19455724ebf5485149dd6cf)
(cherry picked from commit 3b51a05187250ee62a1f9379058f138f95c97529)
---
clients/common/settings-docs.h.in | 2 +-
libnm-core/nm-setting-ovs-patch.c | 28 +++++++++++++++++++---------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in
index a571d2f697a8..111e324d043f 100644
--- a/clients/common/settings-docs.h.in
+++ b/clients/common/settings-docs.h.in
@@ -260,7 +260,7 @@
#define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_RSTP_ENABLE N_("Enable or disable RSTP.")
#define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_STP_ENABLE N_("Enable or disable STP.")
#define DESCRIBE_DOC_NM_SETTING_OVS_INTERFACE_TYPE N_("The interface type. Either \"internal\", 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 2a46810c3290..6a320b7e61fc 100644
--- a/libnm-core/nm-setting-ovs-patch.c
+++ b/libnm-core/nm-setting-ovs-patch.c
@@ -96,13 +96,23 @@ 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_is_valid_iface_name (self->peer, error)) {
+ g_prefix_error (error, "%s.%s: ",
+ NM_SETTING_OVS_PATCH_SETTING_NAME,
+ NM_SETTING_OVS_PATCH_PEER);
+ return FALSE;
+ }
+
+ /* on nm-1-22, we use here nm_utils_ifname_valid(). This change was not backported to
+ * nm-1-20, hence reimplement the check here. We don't want to accept values, that
+ * would be rejected on newer versions of NetworkManager. */
+ if (!NM_STRCHAR_ALL (self->peer,
+ ch,
+ !NM_IN_SET (ch, '\\', '/')
+ && g_ascii_isgraph (ch))) {
+ g_set_error_literal (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY,
+ _("interface name must be alphanumerical with "
+ "no forward or backward slashes"));
g_prefix_error (error, "%s.%s: ",
NM_SETTING_OVS_PATCH_SETTING_NAME,
NM_SETTING_OVS_PATCH_PEER);
@@ -194,8 +204,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
**/
--
2.26.2
From b2be7659d79856e7b60e71b4ea13e4834c1e5b11 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Wed, 30 Sep 2020 10:36:11 +0200
Subject: [PATCH 25/25] libnm: revert changes to translated strings for
ovs-patch (rhel-7.9-only)
For rhel-7.9, we don't want to change translated strings. Revert that
part from upstream in this downstream-only patch.
---
clients/common/settings-docs.h.in | 2 +-
libnm-core/nm-setting-ovs-patch.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/clients/common/settings-docs.h.in b/clients/common/settings-docs.h.in
index 111e324d043f..a571d2f697a8 100644
--- a/clients/common/settings-docs.h.in
+++ b/clients/common/settings-docs.h.in
@@ -260,7 +260,7 @@
#define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_RSTP_ENABLE N_("Enable or disable RSTP.")
#define DESCRIBE_DOC_NM_SETTING_OVS_BRIDGE_STP_ENABLE N_("Enable or disable STP.")
#define DESCRIBE_DOC_NM_SETTING_OVS_INTERFACE_TYPE N_("The interface type. Either \"internal\", or empty.")
-#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_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_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 6a320b7e61fc..613a4c1da859 100644
--- a/libnm-core/nm-setting-ovs-patch.c
+++ b/libnm-core/nm-setting-ovs-patch.c
@@ -204,8 +204,8 @@ nm_setting_ovs_patch_class_init (NMSettingOvsPatchClass *klass)
/**
* NMSettingOvsPatch:peer:
*
- * 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.
+ * Specifies the unicast destination IP address of a remote Open vSwitch
+ * bridge port to connect to.
*
* Since: 1.10
**/
--
2.26.2