manager: fix preserving assume state during activation

Originally 850c977 "device: track system interface state in NMDevice",
intended that a connection can only be assumed initially when seeing
a device for the first time. Assuming a connection later was to be
prevented by setting device's sys-iface-state to MANAGED.

That changed too much in behavior, because we used to assume external
connections also when they are activated later on. So this was attempted
to get fixed by
  - acf1067 nm-manager: try assuming connections on managed devices
  - b6b7d90 manager: avoid generating in memory connections during startup for managed devices

It's probably just wrong to prevent assuming connections based on the
sys-iface-state. So drop the check for sys-iface-state from
recheck_assume_connection(). Now, we can assume anytime on managed,
disconnected interfaces, like previously.
Btw, note that priv->startup is totally wrong to check there, because
priv->startup has the sole purpose of tracking startup-complete property.
Startup, as far as NMManager is concerned, is platform_query_devices().

However, the problem is that we only assume connections (contrary to
doing external activation) when we have a connection-uuid from the state
file or with guess-assume during startup.

When assuming a master device, it can fail with

  (nm-bond): ignoring generated connection (IPv6LL-only and not in master-slave relationship)

thus, for internal reason the device cannot be assumed yet.

Fix that by attatching the assume-state to the device, so that on multiple
recheck_assume_connection() calls we still try to assume. Whenever we try
to assume the connection and it fails due to external reasons (like, the connection
no longer matching), we clear the assume state, so that we only try as
long as there are internal reasons why assuming fails.

https://bugzilla.redhat.com/show_bug.cgi?id=1452062
(cherry picked from commit 729de7d7f09c3ad813477b7a822155b4b95dc682)

3 files changed, 144 insertions, 50 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index d9bdfb069f..8f97041586 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -264,6 +264,9 @@ typedef struct _NMDevicePrivate {
 
 	bool          nm_owned:1; /* whether the device is a device owned and created by NM */
 
+	bool          assume_state_guess_assume:1;
+	char *        assume_state_connection_uuid;
+
 	GHashTable *  available_connections;
 	char *        hw_addr;
 	char *        hw_addr_perm;
@@ -527,6 +530,8 @@ static gboolean dhcp6_start (NMDevice *self, gboolean wait_for_ll);
 static void nm_device_start_ip_check (NMDevice *self);
 static void realize_start_setup (NMDevice *self,
                                  const NMPlatformLink *plink,
+                                 gboolean assume_state_guess_assume,
+                                 const char *assume_state_connection_uuid,
                                  gboolean set_nm_owned,
                                  NMUnmanFlagOp unmanaged_user_explicit);
 static void _commit_mtu (NMDevice *self, const NMIP4Config *config);
@@ -711,6 +716,52 @@ nm_device_sys_iface_state_set (NMDevice *self,
 
 /*****************************************************************************/
 
+void
+nm_device_assume_state_get (NMDevice *self,
+                            gboolean *out_assume_state_guess_assume,
+                            const char **out_assume_state_connection_uuid)
+{
+	NMDevicePrivate *priv;
+
+	g_return_if_fail (NM_IS_DEVICE (self));
+
+	priv = NM_DEVICE_GET_PRIVATE (self);
+	NM_SET_OUT (out_assume_state_guess_assume, priv->assume_state_guess_assume);
+	NM_SET_OUT (out_assume_state_connection_uuid, priv->assume_state_connection_uuid);
+}
+
+static void
+_assume_state_set (NMDevice *self,
+                   gboolean assume_state_guess_assume,
+                   const char *assume_state_connection_uuid)
+{
+	NMDevicePrivate *priv;
+
+	nm_assert (NM_IS_DEVICE (self));
+
+	priv = NM_DEVICE_GET_PRIVATE (self);
+	if (   priv->assume_state_guess_assume == !!assume_state_guess_assume
+	    && nm_streq0 (priv->assume_state_connection_uuid, assume_state_connection_uuid))
+		return;
+
+	_LOGD (LOGD_DEVICE, "assume-state: set guess-assume=%c, connection=%s%s%s",
+	       assume_state_guess_assume ? '1' : '0',
+	       NM_PRINT_FMT_QUOTE_STRING (assume_state_connection_uuid));
+	priv->assume_state_guess_assume = assume_state_guess_assume;
+	g_free (priv->assume_state_connection_uuid);
+	priv->assume_state_connection_uuid = g_strdup (assume_state_connection_uuid);
+}
+
+void
+nm_device_assume_state_reset (NMDevice *self)
+{
+	g_return_if_fail (NM_IS_DEVICE (self));
+
+	_assume_state_set (self, FALSE, NULL);
+}
+
+/*****************************************************************************/
+
 static void
 init_ip4_config_dns_priority (NMDevice *self, NMIP4Config *config)
 {
@@ -2741,6 +2792,8 @@ link_type_compatible (NMDevice *self,
  * nm_device_realize_start():
  * @self: the #NMDevice
  * @plink: an existing platform link or %NULL
+ * @assume_state_guess_assume: set the guess_assume state.
+ * @assume_state_connection_uuid: set the connection uuid to assume.
  * @set_nm_owned: for software device, if TRUE set nm-owned.
  * @unmanaged_user_explicit: the user-explicit unmanaged flag to apply
  *   on the device initially.
@@ -2759,6 +2812,8 @@ link_type_compatible (NMDevice *self,
 gboolean
 nm_device_realize_start (NMDevice *self,
                          const NMPlatformLink *plink,
+                         gboolean assume_state_guess_assume,
+                         const char *assume_state_connection_uuid,
                          gboolean set_nm_owned,
                          NMUnmanFlagOp unmanaged_user_explicit,
                          gboolean *out_compatible,
@@ -2784,8 +2839,11 @@ nm_device_realize_start (NMDevice *self,
 		plink_copy = *plink;
 		plink = &plink_copy;
 	}
-	realize_start_setup (self, plink, set_nm_owned, unmanaged_user_explicit);
-
+	realize_start_setup (self, plink,
+	                     assume_state_guess_assume,
+	                     assume_state_connection_uuid,
+	                     set_nm_owned,
+	                     unmanaged_user_explicit);
 	return TRUE;
 }
 
@@ -2824,7 +2882,10 @@ nm_device_create_and_realize (NMDevice *self,
 		plink = &plink_copy;
 	}
 
-	realize_start_setup (self, plink, FALSE, NM_UNMAN_FLAG_OP_FORGET);
+	realize_start_setup (self, plink,
+	                     FALSE, /* assume_state_guess_assume */
+	                     NULL,  /* assume_state_connection_uuid */
+	                     FALSE, NM_UNMAN_FLAG_OP_FORGET);
 	nm_device_realize_finish (self, plink);
 
 	if (nm_device_get_managed (self, FALSE)) {
@@ -2920,6 +2981,8 @@ realize_start_notify (NMDevice *self,
  * realize_start_setup():
  * @self: the #NMDevice
  * @plink: the #NMPlatformLink if backed by a kernel netdevice
+ * @assume_state_guess_assume: set the guess_assume state.
+ * @assume_state_connection_uuid: set the connection uuid to assume.
  * @set_nm_owned: if TRUE and device is a software-device, set nm-owned.
  *    TRUE.
  * @unmanaged_user_explicit: the user-explict unmanaged flag to set.
@@ -2933,6 +2996,8 @@ realize_start_notify (NMDevice *self,
 static void
 realize_start_setup (NMDevice *self,
                      const NMPlatformLink *plink,
+                     gboolean assume_state_guess_assume,
+                     const char *assume_state_connection_uuid,
                      gboolean set_nm_owned,
                      NMUnmanFlagOp unmanaged_user_explicit)
 {
@@ -2972,6 +3037,8 @@ realize_start_setup (NMDevice *self,
 		_notify (self, PROP_MTU);
 	}
 
+	_assume_state_set (self, assume_state_guess_assume, assume_state_connection_uuid);
+
 	nm_device_sys_iface_state_set (self, NM_DEVICE_SYS_IFACE_STATE_EXTERNAL);
 
 	if (plink) {
@@ -3193,6 +3260,8 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error)
 
 	_LOGD (LOGD_DEVICE, "unrealize (ifindex %d)", ifindex > 0 ? ifindex : 0);
 
+	nm_device_assume_state_reset (self);
+
 	if (remove_resources) {
 		if (NM_DEVICE_GET_CLASS (self)->unrealize) {
 			if (!NM_DEVICE_GET_CLASS (self)->unrealize (self, error))
@@ -4042,7 +4111,7 @@ nm_device_master_update_slave_connection (NMDevice *self,
 }
 
 NMConnection *
-nm_device_generate_connection (NMDevice *self, NMDevice *master)
+nm_device_generate_connection (NMDevice *self, NMDevice *master, gboolean *out_maybe_later)
 {
 	NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self);
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
@@ -4056,6 +4125,8 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
 	GError *error = NULL;
 	const NMPlatformLink *pllink;
 
+	NM_SET_OUT (out_maybe_later, FALSE);
+
 	/* If update_connection() is not implemented, just fail. */
 	if (!klass->update_connection)
 		return NULL;
@@ -4131,6 +4202,7 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
 	    && !nm_setting_connection_get_master (NM_SETTING_CONNECTION (s_con))
 	    && !priv->slaves) {
 		_LOGD (LOGD_DEVICE, "ignoring generated connection (no IP and not in master-slave relationship)");
+		NM_SET_OUT (out_maybe_later, TRUE);
 		g_object_unref (connection);
 		connection = NULL;
 	}
@@ -4145,6 +4217,7 @@ nm_device_generate_connection (NMDevice *self, NMDevice *master)
 	    && !priv->slaves
 	    && !nm_config_data_get_assume_ipv6ll_only (NM_CONFIG_GET_DATA, self)) {
 		_LOGD (LOGD_DEVICE, "ignoring generated connection (IPv6LL-only and not in master-slave relationship)");
+		NM_SET_OUT (out_maybe_later, TRUE);
 		g_object_unref (connection);
 		connection = NULL;
 	}
@@ -12499,6 +12572,9 @@ _set_state_full (NMDevice *self,
 	                        NM_DEVICE_SYS_IFACE_STATE_ASSUME))
 		nm_device_sys_iface_state_set (self, NM_DEVICE_SYS_IFACE_STATE_MANAGED);
 
+	if (state > NM_DEVICE_STATE_DISCONNECTED)
+		nm_device_assume_state_reset (self);
+
 	if (state <= NM_DEVICE_STATE_UNAVAILABLE) {
 		if (available_connections_del_all (self))
 			_notify (self, PROP_AVAILABLE_CONNECTIONS);
@@ -13772,6 +13848,8 @@ dispose (GObject *object)
 
 	nm_clear_g_cancellable (&priv->deactivating_cancellable);
 
+	nm_device_assume_state_reset (self);
+
 	_parent_set_ifindex (self, 0, FALSE);
 
 	platform = nm_device_get_platform (self);
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 74cc2303c8..01e3938a9e 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -487,7 +487,9 @@ void            nm_device_removed               (NMDevice *self, gboolean unconf
 gboolean        nm_device_is_available          (NMDevice *dev, NMDeviceCheckDevAvailableFlags flags);
 gboolean        nm_device_has_carrier           (NMDevice *dev);
 
-NMConnection * nm_device_generate_connection (NMDevice *self, NMDevice *master);
+NMConnection * nm_device_generate_connection (NMDevice *self,
+                                              NMDevice *master,
+                                              gboolean *out_maybe_later);
 
 gboolean nm_device_master_update_slave_connection (NMDevice *master,
                                                    NMDevice *slave,
@@ -609,8 +611,19 @@ gboolean nm_device_is_nm_owned (NMDevice *device);
 
 gboolean nm_device_has_capability (NMDevice *self, NMDeviceCapabilities caps);
 
+/*****************************************************************************/
+
+void nm_device_assume_state_get (NMDevice *self,
+                                 gboolean *out_assume_state_guess_assume,
+                                 const char **out_assume_state_connection_uuid);
+void nm_device_assume_state_reset (NMDevice *self);
+
+/*****************************************************************************/
+
 gboolean nm_device_realize_start      (NMDevice *device,
                                        const NMPlatformLink *plink,
+                                       gboolean assume_state_guess_assume,
+                                       const char *assume_state_connection_uuid,
                                        gboolean set_nm_owned,
                                        NMUnmanFlagOp unmanaged_user_explicit,
                                        gboolean *out_compatible,
diff --git a/src/nm-manager.c b/src/nm-manager.c
index bcfac44ddc..3653ae7a28 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1691,11 +1691,6 @@ done:
  * get_existing_connection:
  * @manager: #NMManager instance
  * @device: #NMDevice instance
- * @guess_assume: whether to employ a heuristic to search for a matching
- *   connection to assume.
- * @assume_connection_uuid: if present, try to assume a connection with this
- *   UUID. If no uuid is given or no matching connection is found, we
- *   only do external activation.
  * @out_generated: (allow-none): return TRUE, if the connection was generated.
  *
  * Returns: a #NMSettingsConnection to be assumed by the device, or %NULL if
@@ -1704,8 +1699,6 @@ done:
 static NMSettingsConnection *
 get_existing_connection (NMManager *self,
                          NMDevice *device,
-                         gboolean guess_assume,
-                         const char *assume_connection_uuid,
                          gboolean *out_generated)
 {
 	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
@@ -1716,6 +1709,9 @@ get_existing_connection (NMManager *self,
 	int ifindex = nm_device_get_ifindex (device);
 	NMSettingsConnection *matched;
 	NMSettingsConnection *connection_checked = NULL;
+	gboolean assume_state_guess_assume = FALSE;
+	const char *assume_state_connection_uuid = NULL;
+	gboolean maybe_later;
 
 	if (out_generated)
 		*out_generated = FALSE;
@@ -1746,9 +1742,16 @@ get_existing_connection (NMManager *self,
 	 * update_connection() implemented, otherwise nm_device_generate_connection()
 	 * returns NULL.
 	 */
-	connection = nm_device_generate_connection (device, master);
-	if (!connection)
+	connection = nm_device_generate_connection (device, master, &maybe_later);
+	if (!connection) {
+		if (!maybe_later)
+			nm_device_assume_state_reset (device);
 		return NULL;
+	}
+
+	nm_device_assume_state_get (device,
+	                            &assume_state_guess_assume,
+	                            &assume_state_connection_uuid);
 
 	/* Now we need to compare the generated connection to each configured
 	 * connection. The comparison function is the heart of the connection
@@ -1759,8 +1762,8 @@ get_existing_connection (NMManager *self,
 	 * When no configured connection matches the generated connection, we keep
 	 * the generated connection instead.
 	 */
-	if (   assume_connection_uuid
-	    && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_connection_uuid))
+	if (   assume_state_connection_uuid
+	    && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_state_connection_uuid))
 	    && !active_connection_find_first (self, connection_checked, NULL,
 	                                      NM_ACTIVE_CONNECTION_STATE_DEACTIVATING)
 	    && nm_device_check_connection_compatible (device, NM_CONNECTION (connection_checked))) {
@@ -1778,7 +1781,7 @@ get_existing_connection (NMManager *self,
 	} else
 		matched = NULL;
 
-	if (!matched && guess_assume) {
+	if (!matched && assume_state_guess_assume) {
 		gs_free NMSettingsConnection **connections = NULL;
 		guint len, i, j;
 
@@ -1812,9 +1815,10 @@ get_existing_connection (NMManager *self,
 		       nm_device_get_iface (device),
 		       nm_settings_connection_get_id (matched),
 		       nm_settings_connection_get_uuid (matched),
-		       assume_connection_uuid && nm_streq (assume_connection_uuid, nm_settings_connection_get_uuid (matched))
+		       assume_state_connection_uuid && nm_streq (assume_state_connection_uuid, nm_settings_connection_get_uuid (matched))
 		           ? " (indicated)" : " (guessed)");
 		g_object_unref (connection);
+		nm_device_assume_state_reset (device);
 		return matched;
 	}
 
@@ -1822,6 +1826,8 @@ get_existing_connection (NMManager *self,
 	       nm_device_get_iface (device),
 	       nm_connection_get_id (connection));
 
+	nm_device_assume_state_reset (device);
+
 	added = nm_settings_add_connection (priv->settings, connection, FALSE, &error);
 	if (added) {
 		nm_settings_connection_set_flags (NM_SETTINGS_CONNECTION (added),
@@ -1844,34 +1850,28 @@ get_existing_connection (NMManager *self,
 
 static gboolean
 recheck_assume_connection (NMManager *self,
-                           NMDevice *device,
-                           gboolean guess_assume,
-                           const char *assume_connection_uuid)
+                           NMDevice *device)
 {
 	NMSettingsConnection *connection;
 	gboolean was_unmanaged = FALSE;
 	gboolean generated = FALSE;
 	NMDeviceState state;
-	NMDeviceSysIfaceState if_state;
-	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
 
 	g_return_val_if_fail (NM_IS_MANAGER (self), FALSE);
 	g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
 
-	if (!nm_device_get_managed (device, FALSE))
+	if (!nm_device_get_managed (device, FALSE)) {
+		nm_device_assume_state_reset (device);
 		return FALSE;
+	}
 
 	state = nm_device_get_state (device);
-	if (state > NM_DEVICE_STATE_DISCONNECTED)
-		return FALSE;
-
-	if_state = nm_device_sys_iface_state_get (device);
-	if (!priv->startup  && (if_state == NM_DEVICE_SYS_IFACE_STATE_MANAGED))
-		nm_assert (!guess_assume && (assume_connection_uuid == NULL));
-	else if (if_state != NM_DEVICE_SYS_IFACE_STATE_EXTERNAL)
+	if (state > NM_DEVICE_STATE_DISCONNECTED) {
+		nm_device_assume_state_reset (device);
 		return FALSE;
+	}
 
-	connection = get_existing_connection (self, device, guess_assume, assume_connection_uuid, &generated);
+	connection = get_existing_connection (self, device, &generated);
 	if (!connection) {
 		_LOGD (LOGD_DEVICE, "(%s): can't assume; no connection",
 		       nm_device_get_iface (device));
@@ -1951,9 +1951,9 @@ recheck_assume_connection (NMManager *self,
 }
 
 static void
-recheck_assume_connection_cb (NMDevice *device, gpointer user_data)
+recheck_assume_connection_cb (NMManager *self, NMDevice *device)
 {
-	recheck_assume_connection (user_data, device, FALSE, NULL);
+	recheck_assume_connection (self, device);
 }
 
 static void
@@ -2046,19 +2046,19 @@ device_connectivity_changed (NMDevice *device,
 static void
 _device_realize_finish (NMManager *self,
                         NMDevice *device,
-                        const NMPlatformLink *plink,
-                        gboolean guess_assume,
-                        const char *connection_uuid_to_assume)
+                        const NMPlatformLink *plink)
 {
 	g_return_if_fail (NM_IS_MANAGER (self));
 	g_return_if_fail (NM_IS_DEVICE (device));
 
 	nm_device_realize_finish (device, plink);
 
-	if (!nm_device_get_managed (device, FALSE))
+	if (!nm_device_get_managed (device, FALSE)) {
+		nm_device_assume_state_reset (device);
 		return;
+	}
 
-	if (recheck_assume_connection (self, device, guess_assume, connection_uuid_to_assume))
+	if (recheck_assume_connection (self, device))
 		return;
 
 	/* if we failed to assume a connection for the managed device, but the device
@@ -2128,9 +2128,9 @@ add_device (NMManager *self, NMDevice *device, GError **error)
 	                  G_CALLBACK (device_removed_cb),
 	                  self);
 
-	g_signal_connect (device, NM_DEVICE_RECHECK_ASSUME,
-	                  G_CALLBACK (recheck_assume_connection_cb),
-	                  self);
+	g_signal_connect_data (device, NM_DEVICE_RECHECK_ASSUME,
+	                       G_CALLBACK (recheck_assume_connection_cb),
+	                       self, NULL, G_CONNECT_SWAPPED);
 
 	g_signal_connect (device, "notify::" NM_DEVICE_IP_IFACE,
 	                  G_CALLBACK (device_ip_iface_changed),
@@ -2212,12 +2212,14 @@ factory_device_added_cb (NMDeviceFactory *factory,
 
 	if (nm_device_realize_start (device,
 	                             NULL,
-	                             FALSE,
+	                             FALSE, /* assume_state_guess_assume */
+	                             NULL,  /* assume_state_connection_uuid */
+	                             FALSE, /* set_nm_owned */
 	                             NM_UNMAN_FLAG_OP_FORGET,
 	                             NULL,
 	                             &error)) {
 		add_device (self, device, NULL);
-		_device_realize_finish (self, device, NULL, FALSE, NULL);
+		_device_realize_finish (self, device, NULL);
 	} else {
 		_LOGW (LOGD_DEVICE, "(%s): failed to realize device: %s",
 		       nm_device_get_iface (device), error->message);
@@ -2291,12 +2293,13 @@ platform_link_added (NMManager *self,
 			return;
 		} else if (nm_device_realize_start (candidate,
 		                                    plink,
-		                                    FALSE,
+		                                    FALSE, /* assume_state_guess_assume */
+		                                    NULL,  /* assume_state_connection_uuid */
+		                                    FALSE, /* set_nm_owned */
 		                                    NM_UNMAN_FLAG_OP_FORGET,
 		                                    &compatible,
 		                                    &error)) {
-			/* Success */
-			_device_realize_finish (self, candidate, plink, FALSE, NULL);
+			_device_realize_finish (self, candidate, plink);
 			return;
 		}
 
@@ -2363,14 +2366,14 @@ platform_link_added (NMManager *self,
 
 		if (nm_device_realize_start (device,
 		                             plink,
+		                             guess_assume,
+		                             dev_state ? dev_state->connection_uuid : NULL,
 		                             dev_state ? (dev_state->nm_owned == 1) : FALSE,
 		                             unmanaged_user_explicit,
 		                             NULL,
 		                             &error)) {
 			add_device (self, device, NULL);
-			_device_realize_finish (self, device, plink,
-			                        guess_assume,
-			                        dev_state ? dev_state->connection_uuid : NULL);
+			_device_realize_finish (self, device, plink);
 		} else {
 			_LOGW (LOGD_DEVICE, "%s: failed to realize device: %s",
 			       plink->name, error->message);