From a2cd778f7d54de1cf9f173fff5f09fededf5f49e Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 21 Apr 2021 13:42:45 +0200
Subject: [PATCH 1/2] device: take reference to device object before
'delete_on_deactivate'
It's not clear why currently a weak reference is needed.
(cherry picked from commit a42682d44fe2220412574fb13128814e643ed775)
(cherry picked from commit 8cfbb73294e9eaa475d28a6eada2c5ab14f1d74a)
---
src/core/devices/nm-device.c | 44 ++++++++++++------------------------
1 file changed, 15 insertions(+), 29 deletions(-)
diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index 5eaf8c23e7..7c2a6d3250 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -125,7 +125,6 @@ typedef struct {
typedef struct {
NMDevice *device;
guint idle_add_id;
- int ifindex;
} DeleteOnDeactivateData;
typedef struct {
@@ -12141,28 +12140,19 @@ nm_device_is_nm_owned(NMDevice *self)
static gboolean
delete_on_deactivate_link_delete(gpointer user_data)
{
- DeleteOnDeactivateData *data = user_data;
- NMDevice * self = data->device;
+ DeleteOnDeactivateData *data = user_data;
+ nm_auto_unref_object NMDevice *self = data->device;
+ NMDevicePrivate * priv = NM_DEVICE_GET_PRIVATE(self);
+ gs_free_error GError *error = NULL;
_LOGD(LOGD_DEVICE,
- "delete_on_deactivate: cleanup and delete virtual link #%d (id=%u)",
- data->ifindex,
+ "delete_on_deactivate: cleanup and delete virtual link (id=%u)",
data->idle_add_id);
- if (data->device) {
- NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(data->device);
- gs_free_error GError *error = NULL;
+ priv->delete_on_deactivate_data = NULL;
- g_object_remove_weak_pointer(G_OBJECT(data->device), (void **) &data->device);
- priv->delete_on_deactivate_data = NULL;
-
- if (!nm_device_unrealize(data->device, TRUE, &error))
- _LOGD(LOGD_DEVICE,
- "delete_on_deactivate: unrealizing %d failed (%s)",
- data->ifindex,
- error->message);
- } else if (data->ifindex > 0)
- nm_platform_link_delete(nm_device_get_platform(self), data->ifindex);
+ if (!nm_device_unrealize(self, TRUE, &error))
+ _LOGD(LOGD_DEVICE, "delete_on_deactivate: unrealizing failed (%s)", error->message);
nm_device_emit_recheck_auto_activate(self);
@@ -12181,17 +12171,16 @@ delete_on_deactivate_unschedule(NMDevice *self)
priv->delete_on_deactivate_data = NULL;
g_source_remove(data->idle_add_id);
- g_object_remove_weak_pointer(G_OBJECT(self), (void **) &data->device);
_LOGD(LOGD_DEVICE,
- "delete_on_deactivate: cancel cleanup and delete virtual link #%d (id=%u)",
- data->ifindex,
+ "delete_on_deactivate: cancel cleanup and delete virtual link (id=%u)",
data->idle_add_id);
+ g_object_unref(data->device);
g_free(data);
}
}
static void
-delete_on_deactivate_check_and_schedule(NMDevice *self, int ifindex)
+delete_on_deactivate_check_and_schedule(NMDevice *self)
{
NMDevicePrivate * priv = NM_DEVICE_GET_PRIVATE(self);
DeleteOnDeactivateData *data;
@@ -12208,16 +12197,13 @@ delete_on_deactivate_check_and_schedule(NMDevice *self, int ifindex)
return;
delete_on_deactivate_unschedule(self); /* always cancel and reschedule */
- data = g_new(DeleteOnDeactivateData, 1);
- g_object_add_weak_pointer(G_OBJECT(self), (void **) &data->device);
- data->device = self;
- data->ifindex = ifindex;
+ data = g_new(DeleteOnDeactivateData, 1);
+ data->device = g_object_ref(self);
data->idle_add_id = g_idle_add(delete_on_deactivate_link_delete, data);
priv->delete_on_deactivate_data = data;
_LOGD(LOGD_DEVICE,
- "delete_on_deactivate: schedule cleanup and delete virtual link #%d (id=%u)",
- ifindex,
+ "delete_on_deactivate: schedule cleanup and delete virtual link (id=%u)",
data->idle_add_id);
}
@@ -15854,7 +15840,7 @@ _cleanup_generic_post(NMDevice *self, CleanupType cleanup_type)
/* Check if the device was deactivated, and if so, delete_link.
* Don't call delete_link synchronously because we are currently
* handling a state change -- which is not reentrant. */
- delete_on_deactivate_check_and_schedule(self, nm_device_get_ip_ifindex(self));
+ delete_on_deactivate_check_and_schedule(self);
}
/* ip_iface should be cleared after flushing all routes and addresses, since
--
2.26.3
From a8eac46cc6a0579cd1b17634c50d9a54518cb53e Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 31 Mar 2021 21:32:43 +0200
Subject: [PATCH 2/2] manager: ensure auto default connection is deleted when a
veth goes away
When the link goes away the manager keeps software devices alive as
unrealized because there is still a connection for them.
If the device is software and has a NM-generated connection, keeping
the device alive means that also the generated connection stays
alive. The result is that both stick around forever even if there is
no longer a kernel link.
Add a check to avoid this situation.
https://bugzilla.redhat.com/show_bug.cgi?id=1945282
Fixes: cd0cf9229d49 ('veth: add support to configure veth interfaces')
(cherry picked from commit d19773ecd4bee36f11749085a15d70a49168c0b7)
(cherry picked from commit 5279b85e02341d24a18fc8dd9238f9f68b733bff)
---
src/core/nm-manager.c | 45 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index c751b2db50..804e8db0f0 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -3519,6 +3519,45 @@ typedef struct {
guint idle_id;
} PlatformLinkCbData;
+static gboolean
+_check_remove_dev_on_link_deleted(NMManager *self, NMDevice *device)
+{
+ NMManagerPrivate * priv = NM_MANAGER_GET_PRIVATE(self);
+ NMSettingsConnection *const *scons = NULL;
+ NMConnection * con;
+ guint i;
+
+ nm_assert(nm_device_is_software(device));
+
+ /* In general, software devices stick around as unrealized
+ * until their connection is removed. However, we don't want
+ * that a NM-generated connection keeps the device alive.
+ * If there are no other compatible connections, the device
+ * should be also removed.
+ */
+
+ scons = nm_settings_get_connections(priv->settings, NULL);
+
+ for (i = 0; scons[i]; i++) {
+ con = nm_settings_connection_get_connection(scons[i]);
+ if (!nm_connection_is_virtual(con))
+ continue;
+
+ if (NM_FLAGS_HAS(nm_settings_connection_get_flags(scons[i]),
+ NM_SETTINGS_CONNECTION_INT_FLAGS_NM_GENERATED))
+ continue;
+
+ if (!nm_device_check_connection_compatible(device, con, NULL))
+ continue;
+
+ /* Found a virtual connection compatible, the device must
+ * stay around unrealized. */
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
static gboolean
_platform_link_cb_idle(PlatformLinkCbData *data)
{
@@ -3544,13 +3583,15 @@ _platform_link_cb_idle(PlatformLinkCbData *data)
if (device) {
if (nm_device_is_software(device)) {
nm_device_sys_iface_state_set(device, NM_DEVICE_SYS_IFACE_STATE_REMOVED);
- /* Our software devices stick around until their connection is removed */
if (!nm_device_unrealize(device, FALSE, &error)) {
_LOG2W(LOGD_DEVICE, device, "failed to unrealize: %s", error->message);
g_clear_error(&error);
remove_device(self, device, FALSE);
} else {
- nm_device_update_from_platform_link(device, NULL);
+ if (_check_remove_dev_on_link_deleted(self, device))
+ remove_device(self, device, FALSE);
+ else
+ nm_device_update_from_platform_link(device, NULL);
}
} else {
/* Hardware and external devices always get removed when their kernel link is gone */
--
2.26.3