From d07827dfe1c0d676e56dc1da20f2f96e76f4e6cb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 17 Apr 2023 11:35:06 +0200 Subject: [PATCH 1/3] device: minor cleanup of code path in delete_cb() (cherry picked from commit c68cbcb8fa8d73c4b6dffae1b3773901f8244b40) (cherry picked from commit ae430f37ed64f7a78ee6912b39687a174113714d) --- src/core/devices/nm-device.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 62a9ff1e84..0f081983ee 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -13429,10 +13429,13 @@ delete_cb(NMDevice *self, /* Authorized */ nm_audit_log_device_op(NM_AUDIT_OP_DEVICE_DELETE, self, TRUE, NULL, subject, NULL); - if (nm_device_unrealize(self, TRUE, &local)) - g_dbus_method_invocation_return_value(context, NULL); - else + + if (!nm_device_unrealize(self, TRUE, &local)) { g_dbus_method_invocation_take_error(context, local); + return; + } + + g_dbus_method_invocation_return_value(context, NULL); } static void From f3a68c88a482d24203384096cc74415a571d670b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 17 Apr 2023 11:42:19 +0200 Subject: [PATCH 2/3] device: block autoconnect of profile when deleting device Currently, when we delete a device then autoconnect does not kick in right away. But that is only, because we happen not to schedule a "autoactivate" recheck. What should be happen, is that rechecking whether to autoconnect is always allowed, and that we have the necessary state to know that autoconnect currently should not work. Instead, block autoconnect of the involved profile. That makes sense, because clearly we don't want to autoconnect right again after `nmcli device delete $iface`. (cherry picked from commit 14d429dd17dcc242f13403374a519d3ea17df261) (cherry picked from commit 53a3368bd600746c30798fb95113a8ccc5d48de3) --- src/core/devices/nm-device.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 0f081983ee..abbd2a05fb 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -13414,7 +13414,8 @@ delete_cb(NMDevice *self, GError *error, gpointer user_data) { - GError *local = NULL; + NMSettingsConnection *sett_conn; + GError *local = NULL; if (error) { g_dbus_method_invocation_return_gerror(context, error); @@ -13430,6 +13431,19 @@ delete_cb(NMDevice *self, /* Authorized */ nm_audit_log_device_op(NM_AUDIT_OP_DEVICE_DELETE, self, TRUE, NULL, subject, NULL); + sett_conn = nm_device_get_settings_connection(self); + if (sett_conn) { + /* Block profile from autoconnecting. We block the profile, which may + * be ugly/wrong with multi-connect profiles. However, it's not + * obviously wrong, because profiles for software devices tend not to + * work with multi-connect anyway, because they describe a (unique) + * interface by name. */ + nm_settings_connection_autoconnect_blocked_reason_set( + sett_conn, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, + TRUE); + } + if (!nm_device_unrealize(self, TRUE, &local)) { g_dbus_method_invocation_take_error(context, local); return; From a4bce9e89b1f5b84abcf12aa87ffc2805951df00 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 12 Apr 2023 11:18:29 +0200 Subject: [PATCH 3/3] device: trigger a recheck to autoconnect when unrealizing ovs-interface NM_reboot_openvswitch_vlan_configuration_var2 test exposes a race. What the test does, is to create OVS profiles and repeatedly restart NetworkManager, checking that those profiles autoconnect and the OVS configuration gets created. There is a race, where: - the OVS interface exists, and an NMDeviceOvsInterface is created - first ovsdb cleans up old interfaces, sending a json request. - OVS deletes the interface, and NetworkManager first picks up the platform signal (there is a race here, usually the ovsdb request completes first, which will cleanup the NMDeviceOvsInterface in a different way). - when the device gets unrealized, we don't schedule a check-autoactivate, so the device stays down. See https://bugzilla.redhat.com/show_bug.cgi?id=2152864#c5 for a log file with more details. What should instead happen, is to autoactivate the OVS interface, which then also fully configures the port and bridge interfaces. Explicitly schedule an autoactivate when unrealizing devices. Note that there are now several cases, where NetworkManager autoconnects more eagerly. This even affects some CI tests and user-visible behavior. But I think relying on "just don't call nm_device_emit_recheck_auto_activate() to hope that autoconnect doesn't happen is wrong. It must always be possible to trigger an autoconnect check, and the right thing must happen. We only don't trigger autoconnect checks *all* the time, because it would be a waste of CPU resources, but whenever we slightly suspect that an autoconnect may happen, we must be allowed to trigger a check. If a device is in a condition where it previously did not autoconnect, and it also *should* not autoconnect, then we need to fix the code that evaluates whether an autoconnect may happen (not avoid triggering a check). https://bugzilla.redhat.com/show_bug.cgi?id=2152864 Fixes-test: @NM_reboot_openvswitch_vlan_configuration_var2 (cherry picked from commit e699dff46a7b24be9974bb779fd23b0802ba7a39) (cherry picked from commit aac8d0cc0cda41da3ddbe952b658ed06a26369e0) --- src/core/devices/nm-device.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index abbd2a05fb..0e029de97e 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7701,6 +7701,10 @@ nm_device_unrealize(NMDevice *self, gboolean remove_resources, GError **error) /* Garbage-collect unneeded unrealized devices. */ nm_device_recheck_available_connections(self); + /* In case the unrealized device is not going away, it may need to + * autoactivate. Schedule also a check for that. */ + nm_device_emit_recheck_auto_activate(self); + return TRUE; }