core: merge branch 'bg/route-metric-compare-rh1302532'

https://bugzilla.redhat.com/show_bug.cgi?id=1302532

5 files changed, 289 insertions, 20 deletions

diff --git a/shared/nm-test-utils.h b/shared/nm-test-utils.h
index 89d32ccf8c..fcae761228 100644
--- a/shared/nm-test-utils.h
+++ b/shared/nm-test-utils.h
@@ -1325,6 +1325,56 @@ nmtst_ip6_config_clone (NMIP6Config *config)
 
 #endif
 
+#ifdef NM_SETTING_IP_CONFIG_H
+inline static void
+nmtst_setting_ip_config_add_address (NMSettingIPConfig *s_ip,
+                                     const char *address,
+                                     guint prefix)
+{
+	NMIPAddress *addr;
+	int family;
+
+	g_assert (s_ip);
+
+	if (nm_utils_ipaddr_valid (AF_INET, address))
+		family = AF_INET;
+	else if (nm_utils_ipaddr_valid (AF_INET6, address))
+		family = AF_INET6;
+	else
+		g_assert (FALSE);
+
+	addr = nm_ip_address_new (family, address, prefix, NULL);
+	g_assert (addr);
+	g_assert (nm_setting_ip_config_add_address (s_ip, addr));
+	nm_ip_address_unref (addr);
+}
+
+inline static void
+nmtst_setting_ip_config_add_route (NMSettingIPConfig *s_ip,
+                                   const char *dest,
+                                   guint prefix,
+                                   const char *next_hop,
+                                   gint64 metric)
+{
+	NMIPRoute *route;
+	int family;
+
+	g_assert (s_ip);
+
+	if (nm_utils_ipaddr_valid (AF_INET, dest))
+		family = AF_INET;
+	else if (nm_utils_ipaddr_valid (AF_INET6, dest))
+		family = AF_INET6;
+	else
+		g_assert (FALSE);
+
+	route = nm_ip_route_new (family, dest, prefix, next_hop, metric, NULL);
+	g_assert (route);
+	g_assert (nm_setting_ip_config_add_route (s_ip, route));
+	nm_ip_route_unref (route);
+}
+#endif /* NM_SETTING_IP_CONFIG_H */
+
 #if (defined(__NM_SIMPLE_CONNECTION_H__) && defined(__NM_SETTING_CONNECTION_H__)) || (defined(NM_CONNECTION_H))
 
 inline static NMConnection *
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index f549875aa2..d13158cc6f 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -2305,6 +2305,95 @@ check_ip6_method (NMConnection *orig,
 	return allow;
 }
 
+static int
+route_compare (NMIPRoute *route1, NMIPRoute *route2, gint64 default_metric)
+{
+	gint64 r, metric1, metric2;
+
+	r = g_strcmp0 (nm_ip_route_get_dest (route1), nm_ip_route_get_dest (route2));
+	if (r)
+		return r;
+
+	r = nm_ip_route_get_prefix (route1) - nm_ip_route_get_prefix (route2);
+	if (r)
+		return r > 0 ? 1 : -1;
+
+	r = g_strcmp0 (nm_ip_route_get_next_hop (route1), nm_ip_route_get_next_hop (route2));
+	if (r)
+		return r;
+
+	metric1 = nm_ip_route_get_metric (route1) == -1 ? default_metric : nm_ip_route_get_metric (route1);
+	metric2 = nm_ip_route_get_metric (route2) == -1 ? default_metric : nm_ip_route_get_metric (route2);
+
+	r = metric1 - metric2;
+	if (r)
+		return r > 0 ? 1 : -1;
+
+	r = nm_ip_route_get_family (route1) - nm_ip_route_get_family (route2);
+	if (r)
+		return r > 0 ? 1 : -1;
+
+	return 0;
+}
+
+static int
+route_ptr_compare (const void *a, const void *b)
+{
+	return route_compare (*(NMIPRoute **) a, *(NMIPRoute **) b, -1);
+}
+
+static gboolean
+check_ip_routes (NMConnection *orig,
+                 NMConnection *candidate,
+                 GHashTable *settings,
+                 gint64 default_metric,
+                 gboolean v4)
+{
+	gs_free NMIPRoute **routes1 = NULL, **routes2 = NULL;
+	NMSettingIPConfig *s_ip1, *s_ip2;
+	const char *s_name;
+	GHashTable *props;
+	guint i, num;
+
+	s_name = v4 ? NM_SETTING_IP4_CONFIG_SETTING_NAME :
+	              NM_SETTING_IP6_CONFIG_SETTING_NAME;
+
+	props = check_property_in_hash (settings,
+	                                s_name,
+	                                NM_SETTING_IP_CONFIG_ROUTES);
+	if (!props)
+		return TRUE;
+
+	s_ip1 = (NMSettingIPConfig *) nm_connection_get_setting_by_name (orig, s_name);
+	s_ip2 = (NMSettingIPConfig *) nm_connection_get_setting_by_name (candidate, s_name);
+
+	if (!s_ip1 || !s_ip2)
+		return FALSE;
+
+	num = nm_setting_ip_config_get_num_routes (s_ip1);
+	if (num != nm_setting_ip_config_get_num_routes (s_ip2))
+		return FALSE;
+
+	routes1 = g_new (NMIPRoute *, num);
+	routes2 = g_new (NMIPRoute *, num);
+
+	for (i = 0; i < num; i++) {
+		routes1[i] = nm_setting_ip_config_get_route (s_ip1, i);
+		routes2[i] = nm_setting_ip_config_get_route (s_ip2, i);
+	}
+
+	qsort (routes1, num, sizeof (NMIPRoute *), route_ptr_compare);
+	qsort (routes2, num, sizeof (NMIPRoute *), route_ptr_compare);
+
+	for (i = 0; i < num; i++) {
+		if (route_compare (routes1[i], routes2[i], default_metric))
+			return FALSE;
+	}
+
+	remove_from_hash (settings, props, s_name, NM_SETTING_IP_CONFIG_ROUTES);
+	return TRUE;
+}
+
 static gboolean
 check_ip4_method (NMConnection *orig,
                   NMConnection *candidate,
@@ -2488,7 +2577,9 @@ static NMConnection *
 check_possible_match (NMConnection *orig,
                       NMConnection *candidate,
                       GHashTable *settings,
-                      gboolean device_has_carrier)
+                      gboolean device_has_carrier,
+                      gint64 default_v4_metric,
+                      gint64 default_v6_metric)
 {
 	g_return_val_if_fail (settings != NULL, NULL);
 
@@ -2498,6 +2589,12 @@ check_possible_match (NMConnection *orig,
 	if (!check_ip4_method (orig, candidate, settings, device_has_carrier))
 		return NULL;
 
+	if (!check_ip_routes (orig, candidate, settings, default_v4_metric, TRUE))
+		return NULL;
+
+	if (!check_ip_routes (orig, candidate, settings, default_v6_metric, FALSE))
+		return NULL;
+
 	if (!check_connection_interface_name (orig, candidate, settings))
 		return NULL;
 
@@ -2541,6 +2638,8 @@ NMConnection *
 nm_utils_match_connection (GSList *connections,
                            NMConnection *original,
                            gboolean device_has_carrier,
+                           gint64 default_v4_metric,
+                           gint64 default_v6_metric,
                            NMUtilsMatchFilterFunc match_filter_func,
                            gpointer match_filter_data)
 {
@@ -2557,8 +2656,10 @@ 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, device_has_carrier);
+			if (!best_match) {
+				best_match = check_possible_match (original, candidate, diffs, device_has_carrier,
+				                                   default_v4_metric, default_v6_metric);
+			}
 
 			if (!best_match && nm_logging_enabled (LOGL_DEBUG, LOGD_CORE)) {
 				GString *diff_string;
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index a873629799..0cb5627f29 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -328,6 +328,8 @@ typedef gboolean (NMUtilsMatchFilterFunc) (NMConnection *connection, gpointer us
 NMConnection *nm_utils_match_connection (GSList *connections,
                                          NMConnection *original,
                                          gboolean device_has_carrier,
+                                         gint64 default_v4_metric,
+                                         gint64 default_v6_metric,
                                          NMUtilsMatchFilterFunc match_filter_func,
                                          gpointer match_filter_data);
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index f8d177b583..e8d73a5020 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -1582,6 +1582,8 @@ get_existing_connection (NMManager *manager, NMDevice *device, gboolean *out_gen
 	matched = NM_SETTINGS_CONNECTION (nm_utils_match_connection (connections,
 	                                                             connection,
 	                                                             nm_device_has_carrier (device),
+	                                                             nm_device_get_ip4_route_metric (device),
+	                                                             nm_device_get_ip6_route_metric (device),
 	                                                             match_connection_filter,
 	                                                             device));
 	if (matched) {
diff --git a/src/tests/test-general.c b/src/tests/test-general.c
index da68c9a97c..4d979942c0 100644
--- a/src/tests/test-general.c
+++ b/src/tests/test-general.c
@@ -205,7 +205,7 @@ test_connection_match_basic (void)
 	copy = nm_simple_connection_new_clone (orig);
 	connections = g_slist_append (connections, copy);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	/* Now change a material property like IPv4 method and ensure matching fails */
@@ -214,7 +214,7 @@ test_connection_match_basic (void)
 	g_object_set (G_OBJECT (s_ip4),
 	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL,
 	              NULL);
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == NULL);
 
 	g_slist_free (connections);
@@ -250,7 +250,7 @@ test_connection_match_ip6_method (void)
 	              NM_SETTING_IP_CONFIG_MAY_FAIL, TRUE,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -284,7 +284,7 @@ test_connection_match_ip6_method_ignore (void)
 	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -318,7 +318,7 @@ test_connection_match_ip6_method_ignore_auto (void)
 	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -326,7 +326,6 @@ test_connection_match_ip6_method_ignore_auto (void)
 	g_object_unref (copy);
 }
 
-
 static void
 test_connection_match_ip4_method (void)
 {
@@ -355,11 +354,11 @@ test_connection_match_ip4_method (void)
 	              NM_SETTING_IP_CONFIG_MAY_FAIL, TRUE,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, FALSE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, FALSE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	/* Ensure when carrier=true matching fails */
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == NULL);
 
 	g_slist_free (connections);
@@ -393,7 +392,7 @@ test_connection_match_interface_name (void)
 	              NM_SETTING_CONNECTION_INTERFACE_NAME, NULL,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -430,7 +429,7 @@ test_connection_match_wired (void)
 	              NM_SETTING_WIRED_S390_NETTYPE, "qeth",
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -462,7 +461,7 @@ test_connection_match_wired2 (void)
 	 * the connections match. It can happen if assuming VLAN devices. */
 	nm_connection_remove_setting (orig, NM_TYPE_SETTING_WIRED);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == copy);
 
 	g_slist_free (connections);
@@ -487,7 +486,7 @@ test_connection_match_cloned_mac (void)
 	              NM_SETTING_WIRED_CLONED_MAC_ADDRESS, "52:54:00:ab:db:23",
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == fuzzy);
 
 	exact = nm_simple_connection_new_clone (orig);
@@ -498,14 +497,14 @@ test_connection_match_cloned_mac (void)
 	              NM_SETTING_WIRED_CLONED_MAC_ADDRESS, "52:54:00:ab:db:23",
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == exact);
 
 	g_object_set (G_OBJECT (s_wired),
 	              NM_SETTING_WIRED_CLONED_MAC_ADDRESS, "52:54:00:ab:db:24",
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched == fuzzy);
 
 	g_slist_free (connections);
@@ -565,7 +564,7 @@ test_connection_no_match_ip4_addr (void)
 	nm_setting_ip_config_add_address (s_ip4, nm_addr);
 	nm_ip_address_unref (nm_addr);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched != copy);
 
 	g_slist_free (connections);
@@ -611,7 +610,7 @@ test_connection_no_match_vlan (void)
 	              NM_SETTING_VLAN_FLAGS, 0,
 	              NULL);
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched != copy);
 
 	/* Check that the connections do not match if VLAN priorities differ */
@@ -621,7 +620,7 @@ test_connection_no_match_vlan (void)
 	g_object_set (G_OBJECT (s_vlan_copy), NM_SETTING_VLAN_FLAGS, 0, NULL);
 	nm_setting_vlan_add_priority_str (s_vlan_copy, NM_VLAN_INGRESS_MAP, "4:2");
 
-	matched = nm_utils_match_connection (connections, orig, TRUE, NULL, NULL);
+	matched = nm_utils_match_connection (connections, orig, TRUE, 0, 0, NULL, NULL);
 	g_assert (matched != copy);
 
 	g_slist_free (connections);
@@ -629,6 +628,118 @@ test_connection_no_match_vlan (void)
 	g_object_unref (copy);
 }
 
+static void
+test_connection_match_ip4_routes1 (void)
+{
+	gs_unref_object NMConnection *orig = NULL, *copy = NULL;
+	NMConnection *matched;
+	gs_free_slist GSList *connections = NULL;
+	NMSettingIPConfig *s_ip4;
+
+	orig = _match_connection_new ();
+
+	s_ip4 = nm_connection_get_setting_ip4_config (orig);
+	g_assert (s_ip4);
+	g_object_set (G_OBJECT (s_ip4),
+	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL,
+	              NULL);
+
+	nmtst_setting_ip_config_add_address (s_ip4, "10.0.0.1", 8);
+
+	/* Clone connection */
+	copy = nm_simple_connection_new_clone (orig);
+	connections = g_slist_append (connections, copy);
+
+	/* Set routes on original connection */
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.16.0", 24, "10.0.0.2", -1);
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.17.0", 24, "10.0.0.3", 20);
+
+	/* Set single route on cloned connection */
+	s_ip4 = nm_connection_get_setting_ip4_config (copy);
+	g_assert (s_ip4);
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.17.0", 24, "10.0.0.3", 20);
+
+	/* Try to match the connections */
+	matched = nm_utils_match_connection (connections, orig, FALSE, 100, 0, NULL, NULL);
+	g_assert (matched == NULL);
+}
+
+static void
+test_connection_match_ip4_routes2 (void)
+{
+	gs_unref_object NMConnection *orig = NULL, *copy = NULL;
+	NMConnection *matched;
+	gs_free_slist GSList *connections = NULL;
+	NMSettingIPConfig *s_ip4;
+
+	orig = _match_connection_new ();
+
+	s_ip4 = nm_connection_get_setting_ip4_config (orig);
+	g_assert (s_ip4);
+	g_object_set (G_OBJECT (s_ip4),
+	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL,
+	              NULL);
+
+	nmtst_setting_ip_config_add_address (s_ip4, "10.0.0.1", 8);
+
+	/* Clone connection */
+	copy = nm_simple_connection_new_clone (orig);
+	connections = g_slist_append (connections, copy);
+
+	/* Set routes on original connection */
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.16.0", 24, "10.0.0.2", -1);
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.17.0", 24, "10.0.0.3", 20);
+
+	/* Set routes on cloned connection, changing order and using explicit metrics */
+	s_ip4 = nm_connection_get_setting_ip4_config (copy);
+	g_assert (s_ip4);
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.17.0", 24, "10.0.0.3", 20);
+	nmtst_setting_ip_config_add_route (s_ip4, "172.25.16.0", 24, "10.0.0.2", 100);
+
+	/* Try to match the connections using different default metrics */
+	matched = nm_utils_match_connection (connections, orig, FALSE, 100, 0, NULL, NULL);
+	g_assert (matched == copy);
+	matched = nm_utils_match_connection (connections, orig, FALSE, 500, 0, NULL, NULL);
+	g_assert (matched == NULL);
+}
+
+static void
+test_connection_match_ip6_routes (void)
+{
+	gs_unref_object NMConnection *orig = NULL, *copy = NULL;
+	NMConnection *matched;
+	gs_free_slist GSList *connections = NULL;
+	NMSettingIPConfig *s_ip6;
+
+	orig = _match_connection_new ();
+
+	s_ip6 = nm_connection_get_setting_ip6_config (orig);
+	g_assert (s_ip6);
+	g_object_set (G_OBJECT (s_ip6),
+	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_MANUAL,
+	              NULL);
+
+	nmtst_setting_ip_config_add_address (s_ip6, "fd01::15", 64);
+
+	/* Clone connection */
+	copy = nm_simple_connection_new_clone (orig);
+	connections = g_slist_append (connections, copy);
+
+	/* Set routes on original connection */
+	nmtst_setting_ip_config_add_route (s_ip6, "2001:db8:a:b:0:0:0:0", 64, "fd01::16", -1);
+
+	/* Set routes on cloned connection */
+	s_ip6 = nm_connection_get_setting_ip6_config (copy);
+	g_assert (s_ip6);
+	nmtst_setting_ip_config_add_route (s_ip6, "2001:db8:a:b:0:0:0:0", 64, "fd01::16", 50);
+
+	/* Try to match the connections */
+	matched = nm_utils_match_connection (connections, orig, FALSE, 0, 100, NULL, NULL);
+	g_assert (matched == NULL);
+	matched = nm_utils_match_connection (connections, orig, FALSE, 0, 50, NULL, NULL);
+	g_assert (matched == copy);
+}
+
 static NMConnection *
 _create_connection_autoconnect (const char *id, gboolean autoconnect, int autoconnect_priority)
 {
@@ -1136,6 +1247,9 @@ main (int argc, char **argv)
 	g_test_add_func ("/general/connection-match/cloned_mac", test_connection_match_cloned_mac);
 	g_test_add_func ("/general/connection-match/no-match-ip4-addr", test_connection_no_match_ip4_addr);
 	g_test_add_func ("/general/connection-match/no-match-vlan", test_connection_no_match_vlan);
+	g_test_add_func ("/general/connection-match/routes/ip4/1", test_connection_match_ip4_routes1);
+	g_test_add_func ("/general/connection-match/routes/ip4/2", test_connection_match_ip4_routes2);
+	g_test_add_func ("/general/connection-match/routes/ip6", test_connection_match_ip6_routes);
 
 	g_test_add_func ("/general/connection-sort/autoconnect-priority", test_connection_sort_autoconnect_priority);