core: fuzzier matching of generated connections to persistent ones

When generating a connection, if the device has no non-link-local IPv6
address, then it's unclear whether (a) the connection was link-local
originally, or (b) the connection was 'auto' but IPv6 failed or timed
out.

In this case, if there is a persistent connection that is 'auto' but
the generated connection is 'link-local', the persistent connection
should be used.

Add a more-testable framework for doing the connection matching to
handle this.

5 files changed, 123 insertions, 23 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index 4d42ad2d9d..f15d18aaed 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -659,3 +659,93 @@ nm_utils_read_resolv_conf_nameservers (const char *rc_contents)
 	return nameservers;
 }
 
+static NMConnection *
+check_possible_match (NMConnection *orig,
+                      NMConnection *candidate,
+                      GHashTable *settings)
+{
+	GHashTable *props;
+	const char *orig_ip6_method, *candidate_ip6_method;
+	NMSettingIP6Config *candidate_ip6;
+
+	g_return_val_if_fail (settings != NULL, NULL);
+
+	props = g_hash_table_lookup (settings, NM_SETTING_IP6_CONFIG_SETTING_NAME);
+	if (   !props
+	    || (g_hash_table_size (props) != 1)
+	    || !g_hash_table_lookup (props, NM_SETTING_IP6_CONFIG_METHOD)) {
+		/* For now 'method' is the only difference we handle here */
+		return NULL;
+	}
+
+	/* If the original connection is 'link-local' and the candidate is both 'auto'
+	 * and may-fail=TRUE, then the candidate is OK to use.  may-fail is included
+	 * in the decision because if the candidate is 'auto' but may-fail=FALSE, then
+	 * the connection could not possibly have been previously activated on the
+	 * device if the device has no non-link-local IPv6 address.
+	 */
+	orig_ip6_method = nm_utils_get_ip_config_method (orig, NM_TYPE_SETTING_IP6_CONFIG);
+	candidate_ip6_method = nm_utils_get_ip_config_method (candidate, NM_TYPE_SETTING_IP6_CONFIG);
+	candidate_ip6 = nm_connection_get_setting_ip6_config (candidate);
+
+	if (   strcmp (orig_ip6_method, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL) == 0
+	    && strcmp (candidate_ip6_method, NM_SETTING_IP6_CONFIG_METHOD_AUTO) == 0
+	    && (!candidate_ip6 || nm_setting_ip6_config_get_may_fail (candidate_ip6))) {
+		return candidate;
+	}
+
+	return NULL;
+}
+
+/**
+ * nm_utils_match_connection:
+ * @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
+ * @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
+ * @match_compat_data: data pointer passed to @match_filter_func
+ *
+ * Checks each connection from @connections until a matching connection is found
+ * considering only setting properties marked with %NM_SETTING_PARAM_INFERRABLE
+ * and checking a few other characteristics like IPv6 method.  If the caller
+ * desires some priority order of the connections, @connections should be
+ * sorted before calling this function.
+ *
+ * Returns: the best #NMConnection matching @original, or %NULL if no connection
+ * matches well enough.
+ */
+NMConnection *
+nm_utils_match_connection (GSList *connections,
+                           NMConnection *original,
+                           NMUtilsMatchFilterFunc match_filter_func,
+                           gpointer match_filter_data)
+{
+	NMConnection *best_match = NULL;
+	GSList *iter;
+
+	for (iter = connections; iter; iter = iter->next) {
+		NMConnection *candidate = NM_CONNECTION (iter->data);
+		GHashTable *diffs = NULL;
+
+		if (match_filter_func) {
+			if (!match_filter_func (candidate, match_filter_data))
+				continue;
+		}
+
+		if (!nm_connection_diff (original, candidate, NM_SETTING_COMPARE_FLAG_INFERRABLE, &diffs)) {
+			if (!best_match)
+				best_match = check_possible_match (original, candidate, diffs);
+			g_hash_table_unref (diffs);
+			continue;
+		}
+
+		/* Exact match */
+		return candidate;
+	}
+
+	/* Best match (if any) */
+	return best_match;
+}
+
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index a84ba10741..06058a5612 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -31,6 +31,7 @@
 #include "nm-ip6-config.h"
 #include "nm-setting-ip6-config.h"
 #include "nm-connection.h"
+#include "nm-setting-private.h"
 
 gboolean nm_ethernet_address_is_valid (const struct ether_addr *test_addr);
 
@@ -86,4 +87,11 @@ char *nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id);
 
 GPtrArray *nm_utils_read_resolv_conf_nameservers (const char *rc_contents);
 
+typedef gboolean (NMUtilsMatchFilterFunc) (NMConnection *connection, gpointer user_data);
+
+NMConnection *nm_utils_match_connection (GSList *connections,
+                                         NMConnection *original,
+                                         NMUtilsMatchFilterFunc match_filter_func,
+                                         gpointer match_filter_data);
+
 #endif /* NETWORK_MANAGER_UTILS_H */
diff --git a/src/nm-manager.c b/src/nm-manager.c
index aa7f918ba2..875464eb8d 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -57,7 +57,6 @@
 #include "nm-device-tun.h"
 #include "nm-device-macvlan.h"
 #include "nm-device-gre.h"
-#include "nm-setting-private.h"
 #include "nm-setting-bluetooth.h"
 #include "nm-setting-connection.h"
 #include "nm-setting-wireless.h"
@@ -1713,6 +1712,12 @@ local_slist_free (void *loc)
 		g_slist_free (*location);
 }
 
+static gboolean
+match_connection_filter (NMConnection *connection, gpointer user_data)
+{
+	return nm_device_check_connection_compatible (NM_DEVICE (user_data), connection, NULL);
+}
+
 /**
  * get_existing_connection:
  * @manager: #NMManager instance
@@ -1726,9 +1731,8 @@ get_existing_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;
+	NMConnection *connection = NULL, *matched;
 	NMSettingsConnection *added = NULL;
-	GSList *iter;
 	GError *error = NULL;
 
 	nm_device_capture_initial_config (device);
@@ -1752,20 +1756,17 @@ get_existing_connection (NMManager *manager, NMDevice *device)
 	 * When no configured connection matches the generated connection, we keep
 	 * the generated connection instead.
 	 */
-	for (iter = connections; iter; iter = iter->next) {
-		NMConnection *candidate = NM_CONNECTION (iter->data);
-
-		if (!nm_device_check_connection_compatible (device, candidate, NULL))
-			continue;
-
-		if (!nm_connection_compare (connection, candidate, NM_SETTING_COMPARE_FLAG_INFERRABLE))
-			continue;
-
+	connections = g_slist_sort (connections, nm_settings_sort_connections);
+	matched = nm_utils_match_connection (connections,
+	                                     connection,
+	                                     match_connection_filter,
+	                                     device);
+	if (matched) {
 		nm_log_info (LOGD_DEVICE, "(%s): found matching connection '%s'",
 		             nm_device_get_iface (device),
-		             nm_connection_get_id (candidate));
+		             nm_connection_get_id (matched));
 		g_object_unref (connection);
-		return candidate;
+		return matched;
 	}
 
 	nm_log_dbg (LOGD_DEVICE, "(%s): generated connection '%s'",
diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 057dc45382..a940656eed 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -1660,21 +1660,20 @@ nm_settings_device_removed (NMSettings *self, NMDevice *device, gboolean quittin
 
 /***************************************************************/
 
-static gint
-best_connection_sort (gconstpointer a, gconstpointer b, gpointer user_data)
+/* GCompareFunc helper for sorting "best" connections */
+gint
+nm_settings_sort_connections (gconstpointer a, gconstpointer b)
 {
 	NMSettingsConnection *ac = (NMSettingsConnection *) a;
 	NMSettingsConnection *bc = (NMSettingsConnection *) b;
 	guint64 ats = 0, bts = 0;
 
-	if (!ac && bc)
+	if (ac == bc)
+		return 0;
+	if (!ac)
 		return -1;
-	else if (ac && !bc)
+	if (!bc)
 		return 1;
-	else if (!ac && !bc)
-		return 0;
-
-	g_assert (ac && bc);
 
 	/* In the future we may use connection priorities in addition to timestamps */
 	nm_settings_connection_get_timestamp (ac, &ats);
@@ -1722,7 +1721,7 @@ get_best_connections (NMConnectionProvider *provider,
 		}
 
 		/* List is sorted with oldest first */
-		sorted = g_slist_insert_sorted_with_data (sorted, connection, best_connection_sort, NULL);
+		sorted = g_slist_insert_sorted (sorted, connection, nm_settings_sort_connections);
 		added++;
 
 		if (max_requested && added > max_requested) {
diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h
index 345eb04d44..7e699c2820 100644
--- a/src/settings/nm-settings.h
+++ b/src/settings/nm-settings.h
@@ -122,4 +122,6 @@ void nm_settings_device_added (NMSettings *self, NMDevice *device);
 
 void nm_settings_device_removed (NMSettings *self, NMDevice *device, gboolean quitting);
 
+gint nm_settings_sort_connections (gconstpointer a, gconstpointer b);
+
 #endif  /* __NM_SETTINGS_H__ */