From 1d5847c8a6d96396ee660b6582216fbdc4c57b60 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 25 Feb 2014 17:56:06 -0600 Subject: [PATCH] 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. --- src/NetworkManagerUtils.c | 48 +++++++++++++++++++++++++++++++++++++-- src/NetworkManagerUtils.h | 1 + src/nm-manager.c | 1 + src/tests/test-general.c | 47 +++++++++++++++++++++++++++++++++++--- 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 264f8e2a71..64c31c89dc 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1705,6 +1705,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 (); }