ovs: merge branch 'bg/ovs-deactivate-async-pt2'

https://bugzilla.redhat.com/show_bug.cgi?id=1787989
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/412
(cherry picked from commit e24fd884941d40209bf6c159be6a4cb61812572a)

(cherry picked from commit 53b878818c4c08c5daa08095483c4c0e68f9b035)

(cherry picked from commit 505aab90e0328d497f533f3ba8ea7e92bde144aa)

3 files changed, 84 insertions, 27 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index cf3f0c4987..f966ae6f4c 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;
@@ -16973,6 +16982,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 bcbc00bbed..0eab3aca19 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 a6fc2656ce..9c74ffd1c2 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,10 +308,22 @@ 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);
 }
 
+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
@@ -309,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;