From 8775e71d12bf26f4153d12dcb20e8e92ba6f0189 Mon Sep 17 00:00:00 2001

From: Beniamino Galvani <bgalvani@redhat.com>

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 <bgalvani@redhat.com>

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 <bgalvani@redhat.com>

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