core: wait to manage !IFF_UP software devices until they come up (rh #1030947)dcbw/external-ifaces2

NM sets a device IFF_UP when it manages the device, to ensure things like link
detection work and that firmware is loaded for the device. Unfortunately, this
prevents adding bridge ports or bond slaves outside NetworkManager if the
bridge/bond was created outside NM. Since we'd really like to cooperate better
with external changes, don't begin to manage the interface until it's set
IFF_UP (and thus actually usable) by whatever created it.

3 files changed, 161 insertions, 60 deletions

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 6727e4ec4e..02ba943ead 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1204,7 +1204,7 @@ link_changed_cb (NMPlatform *platform, int ifindex, NMPlatformLink *info, NMPlat
 		if (klass->link_changed)
 			klass->link_changed (device, info);
 
-		if (priv->admin_up == !!info->up) {
+		if (priv->admin_up != !!info->up) {
 			priv->admin_up = !!info->up;
 			g_object_notify (G_OBJECT (device), NM_DEVICE_ADMIN_UP);
 		}
@@ -7140,6 +7140,25 @@ nm_device_queued_ip_config_change_clear (NMDevice *self)
 	}
 }
 
+static gboolean
+_get_managed_from_flags (NMDevice *device, NMUnmanagedFlags flags)
+{
+	gboolean managed;
+
+	g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
+
+	/* Return the composite of all managed flags.  However, if the device
+	 * is a default-unmanaged device, and would be managed except for the
+	 * default-unmanaged flag (eg, only NM_UNMANAGED_DEFAULT is set) then
+	 * the device is managed whenever it's not in the UNMANAGED state.
+	 */
+	managed = !(flags & ~NM_UNMANAGED_DEFAULT);
+	if (managed && (flags & NM_UNMANAGED_DEFAULT))
+		managed = (nm_device_get_state (device) != NM_DEVICE_STATE_UNMANAGED);
+
+	return managed;
+}
+
 /**
  * nm_device_get_managed():
  * @device: the #NMDevice
@@ -7149,23 +7168,39 @@ nm_device_queued_ip_config_change_clear (NMDevice *self)
 gboolean
 nm_device_get_managed (NMDevice *device)
 {
-	NMDevicePrivate *priv;
-	gboolean managed;
-
 	g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
 
-	priv = NM_DEVICE_GET_PRIVATE (device);
+	return _get_managed_from_flags (device, NM_DEVICE_GET_PRIVATE (device)->unmanaged_flags);
+}
 
-	/* Return the composite of all managed flags.  However, if the device
-	 * is a default-unmanaged device, and would be managed except for the
-	 * default-unmanaged flag (eg, only NM_UNMANAGED_DEFAULT is set) then
-	 * the device is managed whenever it's not in the UNMANAGED state.
-	 */
-	managed = !(priv->unmanaged_flags & ~NM_UNMANAGED_DEFAULT);
-	if (managed && (priv->unmanaged_flags & NM_UNMANAGED_DEFAULT))
-		managed = (priv->state != NM_DEVICE_STATE_UNMANAGED);
+/**
+ * nm_device_would_be_managed():
+ * @device: the #NMDevice
+ * @flag: the #NMUnmanageFlag to check
+ * @unmanaged: the value for @flag
+ *
+ * Checks if a device would be managed or not if the unmanaged flag @flag
+ * was changed to @value, without actually changing @flag to @value.
+ *
+ * Returns: %TRUE if the device would be managed if @flag was changed to
+ * @value, %FALSE if not
+ */
+gboolean
+nm_device_would_be_managed (NMDevice *device,
+                            NMUnmanagedFlags flag,
+                            gboolean unmanaged)
+{
+	NMUnmanagedFlags new_flags;
 
-	return managed;
+	g_return_val_if_fail (NM_IS_DEVICE (device), FALSE);
+
+	new_flags = NM_DEVICE_GET_PRIVATE (device)->unmanaged_flags;
+	if (unmanaged)
+		new_flags |= flag;
+	else
+		new_flags &= ~flag;
+
+	return _get_managed_from_flags (device, new_flags);
 }
 
 /**
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 4fc59e89af..86155a83c9 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -303,19 +303,25 @@ RfKillType nm_device_get_rfkill_type (NMDevice *device);
  * @NM_UNMANAGED_INTERNAL: %TRUE when unmanaged by internal decision (ie,
  *   because NM is sleeping or not managed for some other reason)
  * @NM_UNMANAGED_USER: %TRUE when unmanaged by user decision (via unmanaged-specs)
+ * @NM_MANAGED_ADMIN_DOWN: whether unmanaged because device is administratively
+ *   down (!IFF_UP)
  */
 typedef enum {
-	NM_UNMANAGED_NONE     = 0x00,
-	NM_UNMANAGED_DEFAULT  = 0x01,
-	NM_UNMANAGED_INTERNAL = 0x02,
-	NM_UNMANAGED_USER     = 0x04,
+	NM_UNMANAGED_NONE       = 0x00,
+	NM_UNMANAGED_DEFAULT    = 0x01,
+	NM_UNMANAGED_INTERNAL   = 0x02,
+	NM_UNMANAGED_USER       = 0x04,
+	NM_UNMANAGED_ADMIN_DOWN = 0x08,
 
 	/* Boundary value */
 	__NM_UNMANAGED_LAST,
-	NM_UNMANAGED_LAST     = __NM_UNMANAGED_LAST - 1,
+	NM_UNMANAGED_LAST       = __NM_UNMANAGED_LAST - 1,
 } NMUnmanagedFlags;
 
 gboolean nm_device_get_managed (NMDevice *device);
+gboolean nm_device_would_be_managed (NMDevice *device,
+                                     NMUnmanagedFlags flag,
+                                     gboolean unmanaged);
 gboolean nm_device_get_default_unmanaged (NMDevice *device);
 gboolean nm_device_get_unmanaged_flag (NMDevice *device, NMUnmanagedFlags flag);
 void nm_device_set_unmanaged (NMDevice *device,
diff --git a/src/nm-manager.c b/src/nm-manager.c
index ea0663f5be..2989c634e2 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1735,6 +1735,76 @@ get_existing_connection (NMManager *manager, NMDevice *device)
 	return added ? NM_CONNECTION (added) : NULL;
 }
 
+static void
+assume_connection (NMManager *self, NMDevice *device, NMConnection *connection)
+{
+	NMActiveConnection *active, *master_ac = NULL;
+	NMAuthSubject *subject;
+	GError *error = NULL;
+
+	nm_log_dbg (LOGD_DEVICE, "(%s): will attempt to assume connection '%s'",
+	            nm_device_get_iface (device),
+	            nm_connection_get_id (connection));
+
+	/* Move device to DISCONNECTED to activate the connection */
+	nm_device_state_changed (device,
+	                         NM_DEVICE_STATE_DISCONNECTED,
+	                         NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED);
+
+	subject = nm_auth_subject_new_internal ();
+	active = _new_active_connection (self, connection, NULL, device, subject, &error);
+	if (active) {
+		/* If the device is a slave or VLAN, find the master ActiveConnection */
+		if (find_master (self, connection, device, NULL, NULL, &master_ac, NULL) && master_ac)
+			nm_active_connection_set_master (active, master_ac);
+
+		nm_active_connection_set_assumed (active, TRUE);
+		nm_active_connection_export (active);
+		active_connection_add (self, active);
+		nm_device_queue_activation (device, NM_ACT_REQUEST (active));
+		g_object_unref (active);
+	} else {
+		nm_log_warn (LOGD_DEVICE, "(%s): assumed connection '%s' failed to activate: (%d) %s",
+		             nm_device_get_iface (device),
+		             nm_connection_get_id (connection),
+		             error ? error->code : -1,
+		             error && error->message ? error->message : "(unknown)");
+		g_error_free (error);
+	}
+	g_object_unref (subject);
+}
+
+static void
+device_admin_up (NMDevice *device,
+                 GParamSpec *pspec,
+                 NMManager *self)
+{
+	NMConnection *connection = NULL;
+
+	g_return_if_fail (nm_device_get_unmanaged_flag (device, NM_UNMANAGED_ADMIN_DOWN));
+	if (!nm_device_get_admin_up (device))
+		return;
+
+	/* Stop listening the first time the device comes up */
+	g_signal_handlers_disconnect_by_func (device, device_admin_up, self);
+
+	/* Generate a connection if the device will be managed when we remove
+	 * the UNMANAGED_ADMIN_DOWN flag.  We have to check this before clearing
+	 * the flag, since clearing the flag changes device state, and we send
+	 * a different device state reason if there's a connection we can assume.
+	 */
+	if (nm_device_would_be_managed (device, NM_UNMANAGED_ADMIN_DOWN, FALSE))
+		connection = get_existing_connection (self, device);
+
+	nm_device_set_unmanaged (device,
+	                         NM_UNMANAGED_ADMIN_DOWN,
+	                         FALSE,
+	                         connection ? NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED :
+	                                      NM_DEVICE_STATE_REASON_NOW_MANAGED);
+	if (connection)
+		assume_connection (self, device, connection);
+}
+
 /**
  * add_device:
  * @self: the #NMManager
@@ -1752,12 +1822,13 @@ add_device (NMManager *self, NMDevice *device, gboolean generate_con)
 	char *path;
 	static guint32 devcount = 0;
 	const GSList *unmanaged_specs;
-	gboolean user_unmanaged, sleeping;
+	gboolean user_unmanaged, sleeping, admin_down;
 	NMConnection *connection = NULL;
 	gboolean enabled = FALSE;
 	RfKillType rtype;
 	NMDeviceType devtype;
 	GSList *iter, *remove = NULL;
+	int ifindex;
 
 	devtype = nm_device_get_device_type (device);
 
@@ -1838,27 +1909,50 @@ add_device (NMManager *self, NMDevice *device, gboolean generate_con)
 	type_desc = nm_device_get_type_desc (device);
 	g_assert (type_desc);
 	driver = nm_device_get_driver (device);
+	ifindex = nm_device_get_ifindex (device);
 	if (!driver)
 		driver = "unknown";
 	nm_log_info (LOGD_HW, "(%s): new %s device (driver: '%s' ifindex: %d)",
-	             iface, type_desc, driver, nm_device_get_ifindex (device));
+	             iface, type_desc, driver, ifindex);
 
+	/* Unmanaged by user? */
 	unmanaged_specs = nm_settings_get_unmanaged_specs (priv->settings);
 	user_unmanaged = nm_device_spec_match_list (device, unmanaged_specs);
 	nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_USER, user_unmanaged);
 
+	/* Unmanaged by NM sleeping? */
 	sleeping = manager_sleeping (self);
 	nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_INTERNAL, sleeping);
 
+	/* Don't manage downed software devices until the thing that created them
+	 * brings them up; but for backwards compatibility's sake this doesn't
+	 * apply to default-unmanaged devices.
+	 */
+	admin_down = (generate_con &&
+	              nm_device_is_software (device) &&
+	              !nm_platform_link_is_up (ifindex) &&
+	              !nm_device_get_default_unmanaged (device));
+	if (admin_down) {
+		nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_ADMIN_DOWN, admin_down);
+		/* Wait for device to come up before managing it and generating a connection */
+		g_signal_connect (device, "notify::" NM_DEVICE_ADMIN_UP,
+				          G_CALLBACK (device_admin_up),
+				          self);
+	}
+
+	/* Export the device */
 	path = g_strdup_printf ("/org/freedesktop/NetworkManager/Devices/%d", devcount++);
 	nm_device_set_path (device, path);
 	nm_dbus_manager_register_object (priv->dbus_mgr, path, device);
 	nm_log_info (LOGD_CORE, "(%s): exported as %s", iface, path);
 	g_free (path);
 
-	/* Don't generate a connection e.g. for devices NM just created, or
-	 * for the loopback, or when we're sleeping. */
-	if (generate_con && !user_unmanaged && !sleeping)
+	/* Generate and assume connections for fully-managed devices, and for
+	 * default unmanaged devices that are not otherwise unmanaged.
+	 */
+	if (    generate_con
+	    && (nm_device_get_managed (device) ||
+	        nm_device_would_be_managed (device, NM_UNMANAGED_DEFAULT, FALSE)))
 		connection = get_existing_connection (self, device);
 
 	/* Start the device if it's supposed to be managed. Note that this will
@@ -1881,42 +1975,8 @@ add_device (NMManager *self, NMDevice *device, gboolean generate_con)
 	system_create_virtual_devices (self);
 
 	/* If the device has a connection it can assume, do that now */
-	if (connection) {
-		NMActiveConnection *active;
-		NMAuthSubject *subject;
-		GError *error = NULL;
-
-		nm_log_dbg (LOGD_DEVICE, "(%s): will attempt to assume connection",
-		            nm_device_get_iface (device));
-
-		/* Move device to DISCONNECTED to activate the connection */
-		nm_device_state_changed (device,
-		                         NM_DEVICE_STATE_DISCONNECTED,
-		                         NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED);
-
-		subject = nm_auth_subject_new_internal ();
-		active = _new_active_connection (self, connection, NULL, device, subject, &error);
-		if (active) {
-			NMActiveConnection *master_ac = NULL;
-
-			/* If the device is a slave or VLAN, find the master ActiveConnection */
-			if (find_master (self, connection, device, NULL, NULL, &master_ac, NULL) && master_ac)
-				nm_active_connection_set_master (active, master_ac);
-
-			nm_active_connection_set_assumed (active, TRUE);
-			nm_active_connection_export (active);
-			active_connection_add (self, active);
-			nm_device_queue_activation (device, NM_ACT_REQUEST (active));
-			g_object_unref (active);
-		} else {
-			nm_log_warn (LOGD_DEVICE, "assumed connection %s failed to activate: (%d) %s",
-			             nm_connection_get_path (connection),
-			             error ? error->code : -1,
-			             error && error->message ? error->message : "(unknown)");
-			g_error_free (error);
-		}
-		g_object_unref (subject);
-	}
+	if (connection)
+		assume_connection (self, device, connection);
 }
 
 static NMDevice *