From 8775e71d12bf26f4153d12dcb20e8e92ba6f0189 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 29 Jul 2019 16:13:27 +0200 Subject: [PATCH 1/3] ovs: don't release slaves on quit An OVS bridge and its slaves can continue to work even after NM has quit. Keep the interface enslaved when the @configure argument of device->release_slave() is FALSE, which happens on quit and in other circumstances when we don't really want to release the slave from its master. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/215 (cherry picked from commit ccd4be4014f9f4cfdd0d298ff387ee7558d5f3a5) (cherry picked from commit a1f39b69e0b09b7ab02513e34bd41276808ad778) --- src/devices/ovs/nm-device-ovs-port.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/devices/ovs/nm-device-ovs-port.c b/src/devices/ovs/nm-device-ovs-port.c index 35eb739f9..8a93a5a9d 100644 --- a/src/devices/ovs/nm-device-ovs-port.c +++ b/src/devices/ovs/nm-device-ovs-port.c @@ -140,13 +140,18 @@ del_iface_cb (GError *error, gpointer user_data) static void release_slave (NMDevice *device, NMDevice *slave, gboolean configure) { - nm_ovsdb_del_interface (nm_ovsdb_get (), nm_device_get_iface (slave), - del_iface_cb, g_object_ref (slave)); - - /* Open VSwitch is going to delete this one. We must ignore what happens - * next with the interface. */ - if (NM_IS_DEVICE_OVS_INTERFACE (slave)) - nm_device_update_from_platform_link (slave, NULL); + NMDeviceOvsPort *self = NM_DEVICE_OVS_PORT (device); + + if (configure) { + _LOGI (LOGD_DEVICE, "releasing ovs interface %s", nm_device_get_ip_iface (slave)); + nm_ovsdb_del_interface (nm_ovsdb_get (), nm_device_get_iface (slave), + del_iface_cb, g_object_ref (slave)); + /* Open VSwitch is going to delete this one. We must ignore what happens + * next with the interface. */ + if (NM_IS_DEVICE_OVS_INTERFACE (slave)) + nm_device_update_from_platform_link (slave, NULL); + } else + _LOGI (LOGD_DEVICE, "ovs interface %s was released", nm_device_get_ip_iface (slave)); } /*****************************************************************************/ -- 2.21.0 From 846a67ab95bc5f3e36099a45b0f2131c6346ef6a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 30 Jul 2019 11:03:59 +0200 Subject: [PATCH 2/3] device: check platform link compatibility when setting nm-owned flag We set nm-owned to indicate whether a software device was created by NM or it was pre-existing. When checking the existence, we must verify also whether the link type is compatible with the device, otherwise it is possible to match unrelated interfaces. For example, when checking for the existence of an ovs-bridge (which is not compatible with any platform link) we could match a unrelated platform link with the same name. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 (cherry picked from commit 3cb4b36261684aa3d2676f922c6d53bc31085153) (cherry picked from commit cb20d0791a6daf20d64f4cd57d6bd4b60e35a9a0) (cherry picked from commit 511ef27d5eaf6fd0577b867d9d31de3bee0440fe) --- src/devices/nm-device.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9ad69998b..eaf72a7a0 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4106,13 +4106,14 @@ nm_device_create_and_realize (NMDevice *self, { nm_auto_nmpobj const NMPObject *plink_keep_alive = NULL; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const NMPlatformLink *plink = NULL; + const NMPlatformLink *plink; /* Must be set before device is realized */ - priv->nm_owned = !nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); - + plink = nm_platform_link_get_by_ifname (nm_device_get_platform (self), priv->iface); + priv->nm_owned = !plink || !link_type_compatible (self, plink->type, NULL, NULL); _LOGD (LOGD_DEVICE, "create (is %snm-owned)", priv->nm_owned ? "" : "not "); + plink = NULL; /* Create any resources the device needs */ if (NM_DEVICE_GET_CLASS (self)->create_and_realize) { if (!NM_DEVICE_GET_CLASS (self)->create_and_realize (self, connection, parent, &plink, error)) -- 2.21.0 From 70ada1a2c9e936ec5abe737e1eb80463ff4fba60 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 31 Jul 2019 11:40:35 +0200 Subject: [PATCH 3/3] device: fix releasing slaves Not all masters type have a platform link and so it's wrong to check for it to decide whether the slave should be really released. Move the check to master devices that need it (bond, bridge and team). OVS ports don't need the check because they don't call to platform to remove a slave. https://bugzilla.redhat.com/show_bug.cgi?id=1733709 (cherry picked from commit 57e3734b6cc1bb453216c7e2150a698114507a46) (cherry picked from commit ec1b5fb019929441fdcdf6bf7c54a2856ad61976) (cherry picked from commit f6a90b899ab01d6dc2635faf6bc8d72839a9064c) --- src/devices/nm-device-bond.c | 6 ++++++ src/devices/nm-device-bridge.c | 6 ++++++ src/devices/nm-device.c | 7 +------ src/devices/team/nm-device-team.c | 6 ++++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 37159fca8..790f00d44 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -413,6 +413,12 @@ release_slave (NMDevice *device, NMDeviceBond *self = NM_DEVICE_BOND (device); gboolean success; gs_free char *address = NULL; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { /* When the last slave is released the bond MAC will be set to a random diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 4275af912..e81bcd5d2 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -625,6 +625,12 @@ release_slave (NMDevice *device, { NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); gboolean success; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { success = nm_platform_link_release (nm_device_get_platform (device), diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index eaf72a7a0..5bb7a592e 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -4851,7 +4851,6 @@ nm_device_master_release_slaves (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMDeviceStateReason reason; - gboolean configure = TRUE; CList *iter, *safe; /* Don't release the slaves if this connection doesn't belong to NM. */ @@ -4862,14 +4861,10 @@ nm_device_master_release_slaves (NMDevice *self) if (priv->state == NM_DEVICE_STATE_FAILED) reason = NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED; - if ( priv->ifindex <= 0 - || !nm_platform_link_get (nm_device_get_platform (self), priv->ifindex)) - configure = FALSE; - c_list_for_each_safe (iter, safe, &priv->slaves) { SlaveInfo *info = c_list_entry (iter, SlaveInfo, lst_slave); - nm_device_master_release_one_slave (self, info->slave, configure, reason); + nm_device_master_release_one_slave (self, info->slave, TRUE, reason); } } diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 4ae276dbf..7afd18606 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -774,6 +774,12 @@ release_slave (NMDevice *device, NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gboolean success; + int ifindex; + + ifindex = nm_device_get_ifindex (device); + if ( ifindex <= 0 + || !nm_platform_link_get (nm_device_get_platform (device), ifindex)) + configure = FALSE; if (configure) { success = nm_platform_link_release (nm_device_get_platform (device), -- 2.21.0