core: match IPv4 'disabled' method to 'auto' when device has no link

If IPv4 configuration did not succeed or the device has no IPv4 addresses
when NM restarts, it will detect the existing device configuration as
'disabled'.  This can happen when a bridge has no slaves and thus cannot
perform IPv4 addressing because it has no carrier (since bridge carrier
status depends on slave carriers).  When NM starts or restarts, it
sees the bridge has no IPv4 address and assumes the IPv4 method is
'disabled'.  This creates a new connection, which blocks any slave
connections from activating if they specify their master via UUID
 (since the bridge's active connection is generated).

Fix this by allowing matches from 'disabled' to 'auto' if the device
has no carrier, and there are no other differences between the
original and the candidate connections.

4 files changed, 92 insertions, 5 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index ecc9b60930..375bcaaa31 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -685,16 +685,57 @@ check_ip6_method_link_local_auto (NMConnection *orig,
 	return FALSE;
 }
 
+static gboolean
+check_ip4_method_disabled_auto (NMConnection *orig,
+                                NMConnection *candidate,
+                                GHashTable *settings,
+                                gboolean device_has_carrier)
+{
+	GHashTable *props;
+	const char *orig_ip4_method, *candidate_ip4_method;
+	NMSettingIP4Config *candidate_ip4;
+
+	props = g_hash_table_lookup (settings, NM_SETTING_IP4_CONFIG_SETTING_NAME);
+	if (   !props
+	    || (g_hash_table_size (props) != 1)
+	    || !g_hash_table_lookup (props, NM_SETTING_IP4_CONFIG_METHOD)) {
+		/* For now 'method' is the only difference we handle here */
+		return FALSE;
+	}
+
+	/* If the original connection is 'disabled' (device had no IP addresses)
+	 * but it has no carrier, that most likely means that IP addressing could
+	 * not complete and thus no IP addresses were assigned.  In that case, allow
+	 * matching to the "auto" method.
+	 */
+	orig_ip4_method = nm_utils_get_ip_config_method (orig, NM_TYPE_SETTING_IP4_CONFIG);
+	candidate_ip4_method = nm_utils_get_ip_config_method (candidate, NM_TYPE_SETTING_IP4_CONFIG);
+	candidate_ip4 = nm_connection_get_setting_ip4_config (candidate);
+
+	if (   strcmp (orig_ip4_method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0
+	    && strcmp (candidate_ip4_method, NM_SETTING_IP4_CONFIG_METHOD_AUTO) == 0
+	    && (!candidate_ip4 || nm_setting_ip4_config_get_may_fail (candidate_ip4))
+	    && (device_has_carrier == FALSE)) {
+		return TRUE;
+	}
+
+	return FALSE;
+}
+
 static NMConnection *
 check_possible_match (NMConnection *orig,
                       NMConnection *candidate,
-                      GHashTable *settings)
+                      GHashTable *settings,
+                      gboolean device_has_carrier)
 {
 	g_return_val_if_fail (settings != NULL, NULL);
 
 	if (check_ip6_method_link_local_auto (orig, candidate, settings))
 		return candidate;
 
+	if (check_ip4_method_disabled_auto (orig, candidate, settings, device_has_carrier))
+		return candidate;
+
 	return NULL;
 }
 
@@ -703,6 +744,8 @@ check_possible_match (NMConnection *orig,
  * @connections: a (optionally pre-sorted) list of connections from which to
  * find a matching connection to @original based on "inferrable" properties
  * @original: the #NMConnection to find a match for from @connections
+ * @device_has_carrier: pass %TRUE if the device that generated @original has
+ * a carrier, %FALSE if not
  * @match_filter_func: a function to check whether each connection from @connections
  * should be considered for matching.  This function should return %TRUE if the
  * connection should be considered, %FALSE if the connection should be ignored
@@ -720,6 +763,7 @@ check_possible_match (NMConnection *orig,
 NMConnection *
 nm_utils_match_connection (GSList *connections,
                            NMConnection *original,
+                           gboolean device_has_carrier,
                            NMUtilsMatchFilterFunc match_filter_func,
                            gpointer match_filter_data)
 {
@@ -737,7 +781,7 @@ nm_utils_match_connection (GSList *connections,
 
 		if (!nm_connection_diff (original, candidate, NM_SETTING_COMPARE_FLAG_INFERRABLE, &diffs)) {
 			if (!best_match)
-				best_match = check_possible_match (original, candidate, diffs);
+				best_match = check_possible_match (original, candidate, diffs, device_has_carrier);
 			g_hash_table_unref (diffs);
 			continue;
 		}
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index ee234dda80..a33c78e0aa 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -94,6 +94,7 @@ typedef gboolean (NMUtilsMatchFilterFunc) (NMConnection *connection, gpointer us
 
 NMConnection *nm_utils_match_connection (GSList *connections,
                                          NMConnection *original,
+                                         gboolean device_has_carrier,
                                          NMUtilsMatchFilterFunc match_filter_func,
                                          gpointer match_filter_data);
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index da99a02d77..49c20da354 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1784,6 +1784,7 @@ get_existing_connection (NMManager *manager, NMDevice *device)
 	connections = g_slist_sort (connections, nm_settings_sort_connections);
 	matched = nm_utils_match_connection (connections,
 	                                     connection,
+	                                     nm_device_has_carrier (device),
 	                                     match_connection_filter,
 	                                     device);
 	if (matched) {
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index 9fbefa2b70..192fd669b7 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -185,7 +185,7 @@ test_connection_match_basic (void)
 	copy = nm_connection_duplicate (orig);
 	connections = g_slist_append (connections, copy);
 
-	matched = nm_utils_match_connection (connections, orig, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
 	g_assert (matched == copy);
 
 	/* Now change a material property like IPv4 method and ensure matching fails */
@@ -194,7 +194,7 @@ test_connection_match_basic (void)
 	g_object_set (G_OBJECT (s_ip4),
 	              NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL,
 	              NULL);
-	matched = nm_utils_match_connection (connections, orig, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
 	g_assert (matched == NULL);
 
 	g_slist_free (connections);
@@ -230,7 +230,7 @@ test_connection_match_ip6_method (void)
 	              NM_SETTING_IP6_CONFIG_MAY_FAIL, TRUE,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -238,6 +238,46 @@ test_connection_match_ip6_method (void)
 	g_object_unref (copy);
 }
 
+static void
+test_connection_match_ip4_method (void)
+{
+	NMConnection *orig, *copy, *matched;
+	GSList *connections = NULL;
+	NMSettingIP4Config *s_ip4;
+
+	orig = _match_connection_new ();
+	copy = nm_connection_duplicate (orig);
+	connections = g_slist_append (connections, copy);
+
+	/* Check that if the original connection is IPv4 method=disabled, and the
+	 * candidate is both method=auto and may-faily=true, and the device has no
+	 * carrier that the candidate is matched.
+	 */
+	s_ip4 = nm_connection_get_setting_ip4_config (orig);
+	g_assert (s_ip4);
+	g_object_set (G_OBJECT (s_ip4),
+	              NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_DISABLED,
+	              NULL);
+
+	s_ip4 = nm_connection_get_setting_ip4_config (copy);
+	g_assert (s_ip4);
+	g_object_set (G_OBJECT (s_ip4),
+	              NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO,
+	              NM_SETTING_IP4_CONFIG_MAY_FAIL, TRUE,
+	              NULL);
+
+	matched = nm_utils_match_connection (connections, orig, FALSE, NULL, NULL);
+	g_assert (matched == copy);
+
+	/* Ensure when carrier=true matching fails */
+	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	g_assert (matched == NULL);
+
+	g_slist_free (connections);
+	g_object_unref (orig);
+	g_object_unref (copy);
+}
+
 /*******************************************/
 
 int
@@ -252,6 +292,7 @@ main (int argc, char **argv)
 
 	g_test_add_func ("/general/connection-match/basic", test_connection_match_basic);
 	g_test_add_func ("/general/connection-match/ip6-method", test_connection_match_ip6_method);
+	g_test_add_func ("/general/connection-match/ip4-method", test_connection_match_ip4_method);
 
 	return g_test_run ();
 }