From 72e544c7eca495d0857d0710cc77161cd3b145d0 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 7 Jun 2017 17:34:47 +0200

Subject: [PATCH 1/1] 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)

(cherry picked from commit 06db38b91d627f897b5bdd0de4a06f7b8a220902)

---

 src/devices/nm-device.c | 86 ++++++++++++++++++++++++++++++++++++++++++---

 src/devices/nm-device.h | 15 +++++++-

 src/nm-manager.c        | 93 +++++++++++++++++++++++++------------------------

 3 files changed, 144 insertions(+), 50 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c

index 1aea8cb..404dcf7 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;

 	}

@@ -12504,6 +12577,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);

@@ -13778,6 +13854,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 74cc230..01e3938 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 1daf633..9a7b123 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);

-- 

2.9.4