core: fix ignore-carrier behavior (bgo #710216)

Previously, ignore-carrier devices were always in the unavailable state
until they were activated.  This required some complicated code to keep
track of whether the device was available or not based on what connections
existed, whether those connections were static-IP, and whether the device
was ignore-carrier.

This had the side-effect of auto-activating DHCP connections on
ignore-carrier devices that didn't have a carrier, if that device had at
least one static IP connection available.  Not good.

Remove that complexity and confusion by making ignore-carrier devices
always move to DISCONNECTED state, and simply refuse to activate
connections that require connectivity, but allow connections that don't
require connectivity.  Also, when the device has no carrier, don't
add connections that require connectivity to the AvailableConnections
device property.

https://bugzilla.gnome.org/show_bug.cgi?id=710216

8 files changed, 162 insertions, 157 deletions

diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c
index 82e5a9d342..923af8269d 100644
--- a/src/devices/nm-device-bond.c
+++ b/src/devices/nm-device-bond.c
@@ -86,6 +86,17 @@ is_available (NMDevice *dev)
 }
 
 static gboolean
+check_connection_available (NMDevice *device,
+                            NMConnection *connection,
+                            const char *specific_object)
+{
+	/* Connections are always available because the carrier state is determined
+	 * by the slave carrier states, not the bonds's state.
+	 */
+	return TRUE;
+}
+
+static gboolean
 check_connection_compatible (NMDevice *device,
                              NMConnection *connection,
                              GError **error)
@@ -515,6 +526,7 @@ nm_device_bond_class_init (NMDeviceBondClass *klass)
 	parent_class->get_generic_capabilities = get_generic_capabilities;
 	parent_class->is_available = is_available;
 	parent_class->check_connection_compatible = check_connection_compatible;
+	parent_class->check_connection_available = check_connection_available;
 	parent_class->complete_connection = complete_connection;
 
 	parent_class->match_l2_config = match_l2_config;
diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c
index 2018e93552..739e60021b 100644
--- a/src/devices/nm-device-bridge.c
+++ b/src/devices/nm-device-bridge.c
@@ -84,6 +84,17 @@ is_available (NMDevice *dev)
 }
 
 static gboolean
+check_connection_available (NMDevice *device,
+                            NMConnection *connection,
+                            const char *specific_object)
+{
+	/* Connections are always available because the carrier state is determined
+	 * by the bridge port carrier states, not the bridge's state.
+	 */
+	return TRUE;
+}
+
+static gboolean
 check_connection_compatible (NMDevice *device,
                              NMConnection *connection,
                              GError **error)
@@ -436,6 +447,7 @@ nm_device_bridge_class_init (NMDeviceBridgeClass *klass)
 	parent_class->get_generic_capabilities = get_generic_capabilities;
 	parent_class->is_available = is_available;
 	parent_class->check_connection_compatible = check_connection_compatible;
+	parent_class->check_connection_available = check_connection_available;
 	parent_class->complete_connection = complete_connection;
 
 	parent_class->match_l2_config = match_l2_config;
diff --git a/src/devices/nm-device-generic.c b/src/devices/nm-device-generic.c
index 103d76feea..f58c2ff79f 100644
--- a/src/devices/nm-device-generic.c
+++ b/src/devices/nm-device-generic.c
@@ -68,12 +68,6 @@ get_generic_capabilities (NMDevice *dev)
 }
 
 static gboolean
-is_available (NMDevice *device)
-{
-	return TRUE;
-}
-
-static gboolean
 check_connection_compatible (NMDevice *device,
                              NMConnection *connection,
                              GError **error)
@@ -196,7 +190,6 @@ nm_device_generic_class_init (NMDeviceGenericClass *klass)
 	object_class->set_property = set_property;
 
 	parent_class->get_generic_capabilities = get_generic_capabilities;
-	parent_class->is_available = is_available;
 	parent_class->check_connection_compatible = check_connection_compatible;
 
 	/* properties */
diff --git a/src/devices/nm-device-team.c b/src/devices/nm-device-team.c
index 31806bc928..734cf4d5b9 100644
--- a/src/devices/nm-device-team.c
+++ b/src/devices/nm-device-team.c
@@ -97,6 +97,17 @@ is_available (NMDevice *dev)
 }
 
 static gboolean
+check_connection_available (NMDevice *device,
+                            NMConnection *connection,
+                            const char *specific_object)
+{
+	/* Connections are always available because the carrier state is determined
+	 * by the team port carrier states, not the team's state.
+	 */
+	return TRUE;
+}
+
+static gboolean
 check_connection_compatible (NMDevice *device,
                              NMConnection *connection,
                              GError **error)
@@ -730,6 +741,7 @@ nm_device_team_class_init (NMDeviceTeamClass *klass)
 	parent_class->get_generic_capabilities = get_generic_capabilities;
 	parent_class->is_available = is_available;
 	parent_class->check_connection_compatible = check_connection_compatible;
+	parent_class->check_connection_available = check_connection_available;
 	parent_class->complete_connection = complete_connection;
 
 	parent_class->match_l2_config = match_l2_config;
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index a0bf46542a..04f41c66ba 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1020,41 +1020,6 @@ nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean master_fai
 	return success;
 }
 
-static gboolean
-connection_is_static (NMConnection *connection)
-{
-	const char *method;
-
-	method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP4_CONFIG);
-	if (   strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_MANUAL) != 0
-	    && strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) != 0)
-		return FALSE;
-
-	method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG);
-	if (   strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL) != 0
-	    && strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) != 0)
-		return FALSE;
-
-	return TRUE;
-}
-
-static gboolean
-has_static_connection (NMDevice *self)
-{
-	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
-	const GSList *connections, *iter;
-
-	connections = nm_connection_provider_get_connections (priv->con_provider);
-	for (iter = connections; iter; iter = iter->next) {
-		NMConnection *connection = iter->data;
-
-		if (   nm_device_check_connection_compatible (self, connection, NULL)
-		    && connection_is_static (connection))
-			return TRUE;
-	}
-	return FALSE;
-}
-
 static void
 carrier_changed (NMDevice *device, gboolean carrier)
 {
@@ -1063,6 +1028,8 @@ carrier_changed (NMDevice *device, gboolean carrier)
 	if (!nm_device_get_managed (device))
 		return;
 
+	nm_device_recheck_available_connections (device);
+
 	if (priv->ignore_carrier) {
 		/* Ignore all carrier-off, and ignore carrier-on on connected devices */
 		if (!carrier || priv->state > NM_DEVICE_STATE_DISCONNECTED)
@@ -1530,13 +1497,7 @@ is_available (NMDevice *device)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
 
-	if (!priv->carrier) {
-		if (priv->ignore_carrier && has_static_connection (device))
-			return TRUE;
-		return FALSE;
-	}
-
-	return TRUE;
+	return priv->carrier || priv->ignore_carrier;
 }
 
 /**
@@ -1567,56 +1528,6 @@ nm_device_is_available (NMDevice *self)
 	return NM_DEVICE_GET_CLASS (self)->is_available (self);
 }
 
-/**
- * nm_device_can_activate:
- * @self: the #NMDevice
- * @connection: (allow-none) an #NMConnection, or %NULL
- *
- * Checks if @self can currently activate @connection. In particular,
- * this requires that @self is available (per
- * nm_device_is_available()); that it is either managed or able to
- * become managed; and that it is able to activate @connection in its
- * current state (eg, if @connection requires carrier, then @self has
- * carrier).
- *
- * If @connection is %NULL, this just checks that @self could
- * theoretically activate *some* connection.
- *
- * Returns: %TRUE or %FALSE
- */
-gboolean
-nm_device_can_activate (NMDevice *self, NMConnection *connection)
-{
-	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
-
-	if (!priv->manager_managed)
-		return FALSE;
-
-	if (   connection
-	    && !nm_device_check_connection_compatible (self, connection, NULL))
-		return FALSE;
-
-	if (priv->default_unmanaged) {
-		if (!nm_device_is_available (self))
-			return FALSE;
-	} else if (priv->state < NM_DEVICE_STATE_DISCONNECTED) {
-		if (priv->state != NM_DEVICE_STATE_UNAVAILABLE || priv->carrier || !priv->ignore_carrier)
-			return FALSE;
-
-		/* @self is UNAVAILABLE because it doesn't have carrier, but
-		 * ignore-carrier is set, so we might be able to ignore that.
-		 */
-		if (connection && connection_is_static (connection))
-			return TRUE;
-		else if (!connection && has_static_connection (self))
-			return TRUE;
-		else
-			return FALSE;
-	}
-
-	return TRUE;
-}
-
 gboolean
 nm_device_ignore_carrier (NMDevice *dev)
 {
@@ -1693,7 +1604,7 @@ can_auto_connect (NMDevice *device,
 	if (!nm_setting_connection_get_autoconnect (s_con))
 		return FALSE;
 
-	return nm_device_can_activate (device, connection);
+	return nm_device_connection_is_available (device, connection);
 }
 
 static gboolean
@@ -4527,22 +4438,14 @@ nm_device_activate (NMDevice *self, NMActRequest *req)
 	             nm_device_get_iface (self),
 	             nm_connection_get_id (connection));
 
-	if (priv->state < NM_DEVICE_STATE_DISCONNECTED) {
-		g_return_if_fail (nm_device_can_activate (self, connection));
-
-		if (priv->state == NM_DEVICE_STATE_UNMANAGED) {
-			nm_device_state_changed (self,
-			                         NM_DEVICE_STATE_UNAVAILABLE,
-			                         NM_DEVICE_STATE_REASON_NONE);
-		}
-		if (priv->state == NM_DEVICE_STATE_UNAVAILABLE) {
-			nm_device_state_changed (self,
-			                         NM_DEVICE_STATE_DISCONNECTED,
-			                         NM_DEVICE_STATE_REASON_NONE);
-		}
+	/* Move default unmanaged devices to DISCONNECTED state here */
+	if (priv->default_unmanaged && priv->state == NM_DEVICE_STATE_UNMANAGED) {
+		nm_device_state_changed (self,
+		                         NM_DEVICE_STATE_DISCONNECTED,
+		                         NM_DEVICE_STATE_REASON_NOW_MANAGED);
 	}
 
-	g_warn_if_fail (priv->state == NM_DEVICE_STATE_DISCONNECTED);
+	g_assert (nm_device_connection_is_available (self, connection));
 
 	priv->act_request = g_object_ref (req);
 	g_object_notify (G_OBJECT (self), NM_DEVICE_ACTIVE_CONNECTION);
@@ -6102,7 +6005,7 @@ nm_device_state_changed (NMDevice *device,
 			nm_device_deactivate (device, reason);
 		break;
 	case NM_DEVICE_STATE_DISCONNECTED:
-		if (old_state != NM_DEVICE_STATE_UNAVAILABLE)
+		if (old_state > NM_DEVICE_STATE_UNAVAILABLE)
 			nm_device_deactivate (device, reason);
 		break;
 	default:
@@ -6674,6 +6577,23 @@ nm_device_get_autoconnect (NMDevice *device)
 	return NM_DEVICE_GET_PRIVATE (device)->autoconnect;
 }
 
+gboolean
+nm_device_connection_is_available (NMDevice *device, NMConnection *connection)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (device);
+
+	if (priv->default_unmanaged && (priv->state == NM_DEVICE_STATE_UNMANAGED)) {
+		/* default-unmanaged  devices in UNMANAGED state have no available connections
+		 * so we must manually check whether the connection is available here.
+		 */
+		if (   nm_device_check_connection_compatible (device, connection, NULL)
+		    && NM_DEVICE_GET_CLASS (device)->check_connection_available (device, connection, NULL))
+			return TRUE;
+	}
+
+	return !!g_hash_table_lookup (priv->available_connections, connection);
+}
+
 static void
 _signal_available_connections_changed (NMDevice *device)
 {
@@ -6695,13 +6615,10 @@ _try_add_available_connection (NMDevice *self, NMConnection *connection)
 		return FALSE;
 
 	if (nm_device_check_connection_compatible (self, connection, NULL)) {
-		/* Let subclasses implement additional checks on the connection */
-		if (   NM_DEVICE_GET_CLASS (self)->check_connection_available
-		    && NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NULL)) {
-
+		if (NM_DEVICE_GET_CLASS (self)->check_connection_available (self, connection, NULL)) {
 			g_hash_table_insert (NM_DEVICE_GET_PRIVATE (self)->available_connections,
-					             g_object_ref (connection),
-					             GUINT_TO_POINTER (1));
+			                     g_object_ref (connection),
+			                     GUINT_TO_POINTER (1));
 			return TRUE;
 		}
 	}
@@ -6715,13 +6632,65 @@ _del_available_connection (NMDevice *device, NMConnection *connection)
 }
 
 static gboolean
+connection_requires_carrier (NMConnection *connection)
+{
+	NMSettingIP4Config *s_ip4;
+	NMSettingIP6Config *s_ip6;
+	const char *method;
+	gboolean ip4_carrier_wanted = FALSE, ip6_carrier_wanted = FALSE;
+	gboolean ip4_used = FALSE, ip6_used = FALSE;
+
+	method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP4_CONFIG);
+	if (   strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_MANUAL) != 0
+	    && strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) != 0) {
+		ip4_carrier_wanted = TRUE;
+
+		/* If IPv4 wants a carrier and cannot fail, the whole connection
+		 * requires a carrier regardless of the IPv6 method.
+		 */
+		s_ip4 = nm_connection_get_setting_ip4_config (connection);
+		if (s_ip4 && !nm_setting_ip4_config_get_may_fail (s_ip4))
+			return TRUE;
+	}
+	ip4_used = (strcmp (method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) != 0);
+
+	method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG);
+	if (   strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_MANUAL) != 0
+	    && strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) != 0) {
+		ip6_carrier_wanted = TRUE;
+
+		/* If IPv6 wants a carrier and cannot fail, the whole connection
+		 * requires a carrier regardless of the IPv4 method.
+		 */
+		s_ip6 = nm_connection_get_setting_ip6_config (connection);
+		if (s_ip6 && !nm_setting_ip6_config_get_may_fail (s_ip6))
+			return TRUE;
+	}
+	ip6_used = (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) != 0);
+
+	/* If an IP version wants a carrier and and the other IP version isn't
+	 * used, the connection requires carrier since it will just fail without one.
+	 */
+	if (ip4_carrier_wanted && !ip6_used)
+		return TRUE;
+	if (ip6_carrier_wanted && !ip4_used)
+		return TRUE;
+
+	/* If both want a carrier, the whole connection wants a carrier */
+	return ip4_carrier_wanted && ip6_carrier_wanted;
+}
+
+static gboolean
 check_connection_available (NMDevice *device,
                             NMConnection *connection,
                             const char *specific_object)
 {
-	/* Default is to assume the connection is available unless a subclass
-	 * overrides this with more specific checks.
+	/* Connections which require a network connection are not available when
+	 * the device has no carrier, even with ignore-carrer=TRUE.
 	 */
+	if (NM_DEVICE_GET_PRIVATE (device)->carrier == FALSE)
+		return connection_requires_carrier (connection) ? FALSE : TRUE;
+
 	return TRUE;
 }
 
@@ -6735,13 +6704,15 @@ nm_device_recheck_available_connections (NMDevice *device)
 
 	priv = NM_DEVICE_GET_PRIVATE(device);
 
-	_clear_available_connections (device, FALSE);
+	if (priv->con_provider) {
+		_clear_available_connections (device, FALSE);
 
-	connections = nm_connection_provider_get_connections (priv->con_provider);
-	for (iter = connections; iter; iter = g_slist_next (iter))
-		_try_add_available_connection (device, NM_CONNECTION (iter->data));
+		connections = nm_connection_provider_get_connections (priv->con_provider);
+		for (iter = connections; iter; iter = g_slist_next (iter))
+			_try_add_available_connection (device, NM_CONNECTION (iter->data));
 
-	_signal_available_connections_changed (device);
+		_signal_available_connections_changed (device);
+	}
 }
 
 /**
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index b593d0ac22..63b0699572 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -254,12 +254,9 @@ gboolean        nm_device_is_master         (NMDevice *dev);
 NMActRequest *	nm_device_get_act_request	(NMDevice *dev);
 NMConnection *  nm_device_get_connection	(NMDevice *dev);
 
-gboolean		nm_device_is_available   (NMDevice     *dev);
-gboolean		nm_device_can_activate   (NMDevice     *dev,
-                                          NMConnection *connection);
-
+gboolean        nm_device_is_available   (NMDevice *dev);
 gboolean        nm_device_has_carrier    (NMDevice *dev);
-gboolean		nm_device_ignore_carrier (NMDevice *dev);
+gboolean        nm_device_ignore_carrier (NMDevice *dev);
 
 NMConnection * nm_device_generate_connection (NMDevice *device);
 
@@ -340,6 +337,8 @@ GPtrArray *nm_device_get_available_connections (NMDevice *device,
 
 const char *nm_device_get_physical_port_id (NMDevice *device);
 
+gboolean   nm_device_connection_is_available (NMDevice *device, NMConnection *connection);
+
 G_END_DECLS
 
 /* For testing only */
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 54e04df1e6..3e5f86c8e0 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1775,9 +1775,12 @@ local_slist_free (void *loc)
 static NMConnection *
 get_connection (NMManager *manager, NMDevice *device)
 {
+	NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager);
 	free_slist GSList *connections = nm_manager_get_activatable_connections (manager);
 	NMConnection *connection = NULL;
+	NMSettingsConnection *added = NULL;
 	GSList *iter;
+	GError *error = NULL;
 
 	/* We still support the older API to match a NMDevice object to an
 	 * existing connection using nm_device_find_assumable_connection().
@@ -1835,7 +1838,18 @@ get_connection (NMManager *manager, NMDevice *device)
 	nm_log_info (LOGD_DEVICE, "(%s): Using generated connection: '%s'",
 				 nm_device_get_iface (device),
 				 nm_connection_get_id (connection));
-	return connection;
+
+	added = nm_settings_add_connection (priv->settings, connection, FALSE, &error);
+	if (!added) {
+		nm_log_warn (LOGD_SETTINGS, "(%s) Couldn't save generated connection '%s': %s",
+		             nm_device_get_iface (device),
+		             nm_connection_get_id (connection),
+		             (error && error->message) ? error->message : "(unknown)");
+		g_clear_error (&error);
+	}
+	g_object_unref (connection);
+
+	return added ? NM_CONNECTION (added) : NULL;
 }
 
 static void
@@ -1953,7 +1967,9 @@ add_device (NMManager *self, NMDevice *device)
 	system_create_virtual_devices (self);
 
 	/* If the device has a connection it can assume, do that now */
-	if (connection && nm_device_can_activate (device, connection)) {
+	if (   connection
+	    && nm_device_is_available (device)
+	    && nm_device_connection_is_available (device, connection)) {
 		NMActiveConnection *active;
 		NMAuthSubject *subject;
 		GError *error = NULL;
@@ -1961,14 +1977,10 @@ add_device (NMManager *self, NMDevice *device)
 		nm_log_dbg (LOGD_DEVICE, "(%s): will attempt to assume connection",
 		            nm_device_get_iface (device));
 
-		/* Tear down any existing connection */
-		if (nm_device_get_act_request (device)) {
-			nm_log_info (LOGD_DEVICE, "(%s): disconnecting for new activation request.",
-				         nm_device_get_iface (device));
-			nm_device_state_changed (device,
-			                         NM_DEVICE_STATE_DISCONNECTED,
-			                         NM_DEVICE_STATE_REASON_NONE);
-		}
+		/* 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);
@@ -2618,7 +2630,7 @@ ensure_master_active_connection (NMManager *self,
 				if (!is_compatible_with_slave (candidate, connection))
 					continue;
 
-				if (nm_device_check_connection_compatible (master_device, candidate, NULL)) {
+				if (nm_device_connection_is_available (master_device, candidate)) {
 					master_ac = nm_manager_activate_connection (self,
 					                                            candidate,
 					                                            NULL,
@@ -2659,7 +2671,7 @@ ensure_master_active_connection (NMManager *self,
 				continue;
 			}
 
-			if (!nm_device_check_connection_compatible (candidate, master_connection, NULL))
+			if (!nm_device_connection_is_available (candidate, master_connection))
 				continue;
 
 			found_device = TRUE;
@@ -2772,8 +2784,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 
 		/* A newly created device, if allowed to be managed by NM, will be
 		 * in the UNAVAILABLE state here.  To ensure it can be activated
-		 * immediately, we transition it to DISCONNECTED so it passes the
-		 * nm_device_can_activate() check below.
+		 * immediately, we transition it to DISCONNECTED.
 		 */
 		if (   nm_device_is_available (device)
 			&& (nm_device_get_state (device) == NM_DEVICE_STATE_UNAVAILABLE)) {
@@ -2783,13 +2794,11 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError *
 		}
 	}
 
-	/* Final connection must be compatible with the device */
-	if (!nm_device_check_connection_compatible (device, connection, error))
-		return FALSE;
-
-	if (!nm_device_can_activate (device, connection)) {
-		g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNMANAGED_DEVICE,
-		                     "Device not managed by NetworkManager or unavailable");
+	/* Final connection must be available on device */
+	if (!nm_device_connection_is_available (device, connection)) {
+		g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_UNKNOWN_CONNECTION,
+		             "Connection '%s' is not available on the device %s at this time.",
+		             nm_connection_get_id (connection), nm_device_get_iface (device));
 		return FALSE;
 	}
 
diff --git a/src/nm-policy.c b/src/nm-policy.c
index 2bdcfa95b4..f219eb3f86 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -1237,9 +1237,6 @@ schedule_activate_check (NMPolicy *policy, NMDevice *device, guint delay_seconds
 	if (nm_manager_get_state (priv->manager) == NM_STATE_ASLEEP)
 		return;
 
-	if (!nm_device_can_activate (device, NULL))
-		return;
-
 	if (!nm_device_get_enabled (device))
 		return;