route-manager: merge branch 'th/direct-route-conflict-bgo752062'

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

(cherry picked from commit ad2809cf6a)

src/nm-route-manager.c

@@ -44,6 +44,13 @@ typedef struct {
 typedef struct {
 	GArray *entries;
 	RouteIndex *index;
+
+	/* list of effective metrics. The indexes of the array correspond to @index, not @entries. */
+	GArray *effective_metrics;
+
+	/* this array contains the effective metrics but using the reversed index that corresponds
+	 * to @entries, instead of @index. */
+	GArray *effective_metrics_reverse;
 } RouteEntries;
 
 typedef struct {
@@ -75,6 +82,12 @@ NM_DEFINE_SINGLETON_GETTER (NMRouteManager, nm_route_manager_get, NM_TYPE_ROUTE_
 typedef struct {
 	const NMPlatformVTableRoute *vt;
 
+	/* a compare function for two routes that considers only the destination fields network/plen.
+	 * It is a looser comparisong then @route_id_cmp(), that means that if @route_dest_cmp()
+	 * returns non-zero, also @route_id_cmp() returns the same value. It also means, that
+	 * sorting by @route_id_cmp() implicitly sorts by @route_dest_cmp() as well. */
+	int (*route_dest_cmp) (const NMPlatformIPXRoute *r1, const NMPlatformIPXRoute *r2);
+
 	/* a compare function for two routes that considers only the fields network/plen,metric. */
 	int (*route_id_cmp) (const NMPlatformIPXRoute *r1, const NMPlatformIPXRoute *r2);
 } VTableIP;
@@ -202,13 +215,22 @@ ASSERT_route_index_valid (const VTableIP *vtable, const GArray *entries, const R
 
 /*********************************************************************************************/
 
+static int
+_v4_route_dest_cmp (const NMPlatformIP4Route *r1, const NMPlatformIP4Route *r2)
+{
+	CMP_AND_RETURN_INT (r1->plen, r2->plen);
+	CMP_AND_RETURN_INT (nm_utils_ip4_address_clear_host_address (r1->network, r1->plen),
+	                    nm_utils_ip4_address_clear_host_address (r2->network, r2->plen));
+	return 0;
+}
+
 static int
 _v4_route_id_cmp (const NMPlatformIP4Route *r1, const NMPlatformIP4Route *r2)
 {
 	CMP_AND_RETURN_INT (r1->plen, r2->plen);
-	CMP_AND_RETURN_INT (r1->metric, r2->metric);
 	CMP_AND_RETURN_INT (nm_utils_ip4_address_clear_host_address (r1->network, r1->plen),
 	                    nm_utils_ip4_address_clear_host_address (r2->network, r2->plen));
+	CMP_AND_RETURN_INT (r1->metric, r2->metric);
 	return 0;
 }
 
@@ -216,10 +238,27 @@ static int
 _v6_route_id_cmp (const NMPlatformIP6Route *r1, const NMPlatformIP6Route *r2)
 {
 	struct in6_addr n1, n2;
+	int c;
 
 	CMP_AND_RETURN_INT (r1->plen, r2->plen);
+
+	nm_utils_ip6_address_clear_host_address (&n1, &r1->network, r1->plen);
+	nm_utils_ip6_address_clear_host_address (&n2, &r2->network, r2->plen);
+	c = memcmp (&n1, &n2, sizeof (n1));
+	if (c != 0)
+		return c;
+
 	CMP_AND_RETURN_INT (nm_utils_ip6_route_metric_normalize (r1->metric),
 	                    nm_utils_ip6_route_metric_normalize (r2->metric));
+	return 0;
+}
+
+static int
+_v6_route_dest_cmp (const NMPlatformIP6Route *r1, const NMPlatformIP6Route *r2)
+{
+	struct in6_addr n1, n2;
+
+	CMP_AND_RETURN_INT (r1->plen, r2->plen);
 
 	nm_utils_ip6_address_clear_host_address (&n1, &r1->network, r1->plen);
 	nm_utils_ip6_address_clear_host_address (&n2, &r2->network, r2->plen);
@@ -315,13 +354,16 @@ _route_index_reverse_idx (const VTableIP *vtable, const RouteIndex *index, guint
 /*********************************************************************************************/
 
 static gboolean
-_route_equals_ignoring_ifindex (const VTableIP *vtable, const NMPlatformIPXRoute *r1, const NMPlatformIPXRoute *r2)
+_route_equals_ignoring_ifindex (const VTableIP *vtable, const NMPlatformIPXRoute *r1, const NMPlatformIPXRoute *r2, gint64 r2_metric)
 {
 	NMPlatformIPXRoute r2_backup;
 
-	if (r1->rx.ifindex != r2->rx.ifindex) {
+	if (   r1->rx.ifindex != r2->rx.ifindex
+	    || (r2_metric >= 0 && ((guint32) r2_metric) != r2->rx.metric)) {
 		memcpy (&r2_backup, r2, vtable->vt->sizeof_route);
 		r2_backup.rx.ifindex = r1->rx.ifindex;
+		if (r2_metric >= 0)
+			r2_backup.rx.metric = (guint32) r2_metric;
 		r2 = &r2_backup;
 	}
 	return vtable->vt->route_cmp (r1, r2) == 0;
@@ -411,11 +453,14 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 	RouteIndex *plat_routes_idx, *known_routes_idx;
 	gboolean success = TRUE;
 	guint i, i_type;
-	GArray *to_delete_indexes = NULL, *to_restore_routes = NULL;
+	GArray *to_delete_indexes = NULL;
 	GPtrArray *to_add_routes = NULL;
 	guint i_known_routes, i_plat_routes, i_ipx_routes;
 	const NMPlatformIPXRoute *cur_known_route, *cur_plat_route;
 	NMPlatformIPXRoute *cur_ipx_route;
+	gint64 *p_effective_metric = NULL;
+	gboolean ipx_routes_changed = FALSE;
+	gint64 *effective_metrics = NULL;
 
 	nm_platform_process_events (priv->platform);
 
@@ -427,6 +472,8 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 	plat_routes_idx = _route_index_create (vtable, plat_routes);
 	known_routes_idx = _route_index_create (vtable, known_routes);
 
+	effective_metrics = &g_array_index (ipx_routes->effective_metrics, gint64, 0);
+
 	ASSERT_route_index_valid (vtable, plat_routes, plat_routes_idx, TRUE);
 	ASSERT_route_index_valid (vtable, known_routes, known_routes_idx, FALSE);
 
@@ -437,7 +484,10 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 			       ifindex, i, vtable->vt->route_to_string (VTABLE_ROUTE_INDEX (vtable, known_routes, i)));
 		}
 		for (i = 0; i < ipx_routes->index->len; i++)
-			_LOGT (vtable->vt->addr_family, "%3d: STATE: has    #%u - %s", ifindex, i, vtable->vt->route_to_string (ipx_routes->index->entries[i]));
+			_LOGT (vtable->vt->addr_family, "%3d: STATE: has    #%u - %s (%lld)",
+			       ifindex, i,
+			       vtable->vt->route_to_string (ipx_routes->index->entries[i]),
+			       (long long) g_array_index (ipx_routes->effective_metrics, gint64, i));
 	}
 
 	/***************************************************************************
@@ -462,32 +512,20 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 				to_delete_indexes = g_array_new (FALSE, FALSE, sizeof (guint));
 			g_array_append_val (to_delete_indexes, i_ipx_routes);
 
-			/* later we will delete @cur_ipx_route. See if @cur_ipx_route was shadowing another route, that
-			 * we must restore. */
-			if (i_ipx_routes + 1 < ipx_routes->index->len) {
-				const NMPlatformIPXRoute *next_route = ipx_routes->index->entries[i_ipx_routes + 1];
-
-				if (vtable->route_id_cmp (cur_ipx_route, next_route) == 0) {
-					if (!to_restore_routes)
-						to_restore_routes = g_array_new (FALSE, FALSE, vtable->vt->sizeof_route);
-					g_array_append_vals (to_restore_routes, next_route, 1);
-					g_assert (next_route->rx.ifindex != ifindex);
-				}
-			}
-
 			/* find the next @cur_ipx_route with matching ifindex. */
 			cur_ipx_route = _get_next_ipx_route (ipx_routes->index, FALSE, &i_ipx_routes, ifindex);
 		}
 		if (   cur_ipx_route
 		    && cur_known_route
 		    && route_id_cmp_result == 0) {
-			if (!_route_equals_ignoring_ifindex (vtable, cur_ipx_route, cur_known_route)) {
+			if (!_route_equals_ignoring_ifindex (vtable, cur_ipx_route, cur_known_route, -1)) {
 				/* The routes match. Update the entry in place. As this is an exact match of primary
 				 * fields, this only updates possibly modified fields such as @gateway or @mss.
 				 * Modifiying @cur_ipx_route this way does not invalidate @ipx_routes->index. */
 				memcpy (cur_ipx_route, cur_known_route, vtable->vt->sizeof_route);
 				cur_ipx_route->rx.ifindex = ifindex;
 				cur_ipx_route->rx.metric = vtable->vt->metric_normalize (cur_ipx_route->rx.metric);
+				ipx_routes_changed = TRUE;
 				_LOGT (vtable->vt->addr_family, "%3d: STATE: update #%u - %s", ifindex, i_ipx_routes, vtable->vt->route_to_string (cur_ipx_route));
 			}
 		} else if (cur_known_route) {
@@ -516,9 +554,14 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 			}
 			g_array_sort (to_delete_indexes, (GCompareFunc) _sort_indexes_cmp);
 			nm_utils_array_remove_at_indexes (ipx_routes->entries, &g_array_index (to_delete_indexes, guint, 0), to_delete_indexes->len);
+			nm_utils_array_remove_at_indexes (ipx_routes->effective_metrics_reverse, &g_array_index (to_delete_indexes, guint, 0), to_delete_indexes->len);
 			g_array_unref (to_delete_indexes);
 		}
 		if (to_add_routes) {
+			guint j = ipx_routes->effective_metrics_reverse->len;
+
+			g_array_set_size (ipx_routes->effective_metrics_reverse, j + to_add_routes->len);
+
 			for (i = 0; i < to_add_routes->len; i++) {
 				NMPlatformIPXRoute *ipx_route;
 
@@ -528,15 +571,100 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 				ipx_route->rx.ifindex = ifindex;
 				ipx_route->rx.metric = vtable->vt->metric_normalize (ipx_route->rx.metric);
 
+				g_array_index (ipx_routes->effective_metrics_reverse, gint64, j++) = -1;
+
 				_LOGT (vtable->vt->addr_family, "%3d: STATE: added  #%u - %s", ifindex, ipx_routes->entries->len - 1, vtable->vt->route_to_string (ipx_route));
 			}
 			g_ptr_array_unref (to_add_routes);
 		}
 		g_free (ipx_routes->index);
 		ipx_routes->index = _route_index_create (vtable, ipx_routes->entries);
+		ipx_routes_changed = TRUE;
 		ASSERT_route_index_valid (vtable, ipx_routes->entries, ipx_routes->index, TRUE);
 	}
 
+	if (ipx_routes_changed) {
+		/***************************************************************************
+		 * Rebuild the list of effective metrics. In case of conflicting routes,
+		 * we configure device routes with a bumped metric. We do this, because non-direct
+		 * routes might require this direct route to reach the gateway (e.g. the default
+		 * route).
+		 *
+		 * We determine the effective metrics only based on our internal list @ipx_routes
+		 * and don't consider @plat_routes. That means, we might bump the metric of a route
+		 * and thereby cause a conflict with an existing route on an unmanaged device (which
+		 * causes the route on the unmanaged device to be replaced).
+		 * Still, that is not much different then from messing with unmanaged routes when
+		 * the effective and the intended metrics equal. The rules is: NM will leave routes
+		 * on unmanged devices alone, unless they conflict with what NM wants to configure.
+		 ***************************************************************************/
+
+		g_array_set_size (ipx_routes->effective_metrics, ipx_routes->entries->len);
+		effective_metrics = &g_array_index (ipx_routes->effective_metrics, gint64, 0);
+
+		/* Completely regenerate the list of effective metrics by walking through
+		 * ipx_routes->index and determining the effective metric. */
+
+		for (i_ipx_routes = 0; i_ipx_routes < ipx_routes->index->len; i_ipx_routes++) {
+			gint64 *p_effective_metric_before;
+			gboolean is_shadowed;
+			guint i_ipx_routes_before;
+
+			cur_ipx_route = ipx_routes->index->entries[i_ipx_routes];
+			p_effective_metric = &effective_metrics[i_ipx_routes];
+
+			is_shadowed =    i_ipx_routes > 0
+			              && vtable->route_dest_cmp (cur_ipx_route, ipx_routes->index->entries[i_ipx_routes - 1]) == 0;
+
+			if (!is_shadowed) {
+				/* the route is not shadowed, the effective metric is just as specified. */
+				*p_effective_metric = cur_ipx_route->rx.metric;
+				goto next;
+			}
+			if (!VTABLE_IS_DEVICE_ROUTE (vtable, cur_ipx_route)) {
+				/* The route is not a device route. We want to add redundant device routes, because
+				 * we might need the direct routes to the gateway. For non-direct routes, there is not much
+				 * reason to do the metric increment. */
+				*p_effective_metric = -1;
+				goto next;
+			}
+
+			/* The current route might be shadowed by several other routes. Find the one with the highest metric,
+			 * i.e. the one with an effecive metric set and in the index before the current index. */
+			i_ipx_routes_before = i_ipx_routes;
+			while (TRUE) {
+				nm_assert (i_ipx_routes_before > 0);
+
+				i_ipx_routes_before--;
+
+				p_effective_metric_before = &effective_metrics[i_ipx_routes_before];
+
+				if (*p_effective_metric_before == -1) {
+					/* this route is also shadowed, continue search. */
+					continue;
+				}
+
+				if (*p_effective_metric_before < cur_ipx_route->rx.metric) {
+					/* the previous route has a lower metric. There is no conflict,
+					 * just use the original metric. */
+					*p_effective_metric = cur_ipx_route->rx.metric;
+				} else if (*p_effective_metric_before == G_MAXUINT32) {
+					/* we cannot bump the metric. Don't configure this route. */
+					*p_effective_metric = -1;
+				} else {
+					/* bump the metric by one. */
+					*p_effective_metric = *p_effective_metric_before + 1;
+				}
+				break;
+			}
+next:
+			_LOGT (vtable->vt->addr_family, "%3d: new metric    #%u - %s (%lld)",
+			       ifindex, i_ipx_routes,
+			       vtable->vt->route_to_string (cur_ipx_route),
+			       (long long) *p_effective_metric);
+		}
+	}
+
 	/***************************************************************************
 	 * Delete routes in platform, that no longer exist in @ipx_routes
 	 ***************************************************************************/
@@ -544,46 +672,90 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 	/* iterate over @plat_routes and @ipx_routes */
 	cur_plat_route = _get_next_plat_route (plat_routes_idx, TRUE, &i_plat_routes);
 	cur_ipx_route = _get_next_ipx_route (ipx_routes->index, TRUE, &i_ipx_routes, ifindex);
+	if (cur_ipx_route)
+		p_effective_metric = &effective_metrics[i_ipx_routes];
 	while (cur_plat_route) {
-		int route_id_cmp_result = 0;
+		int route_dest_cmp_result = 0;
 
 		g_assert (cur_plat_route->rx.ifindex == ifindex);
 
-		_LOGT (vtable->vt->addr_family, "%3d: platform rt   #%u - %s", ifindex, i_ipx_routes, vtable->vt->route_to_string (cur_plat_route));
+		_LOGT (vtable->vt->addr_family, "%3d: platform rt   #%u - %s", ifindex, i_plat_routes, vtable->vt->route_to_string (cur_plat_route));
 
 		/* skip over @cur_ipx_route that are ordered before @cur_plat_route */
 		while (   cur_ipx_route
-		       && ((route_id_cmp_result = vtable->route_id_cmp (cur_ipx_route, cur_plat_route)) < 0)) {
+		       && ((route_dest_cmp_result = vtable->route_dest_cmp (cur_ipx_route, cur_plat_route)) <= 0)) {
+			if (   route_dest_cmp_result == 0
+			    && *p_effective_metric != -1
+			    && *p_effective_metric >= cur_plat_route->rx.metric) {
+				break;
+			}
 			cur_ipx_route = _get_next_ipx_route (ipx_routes->index, FALSE, &i_ipx_routes, ifindex);
+			if (cur_ipx_route)
+				p_effective_metric = &effective_metrics[i_ipx_routes];
 		}
 
 		/* if @cur_ipx_route is not equal to @plat_route, the route must be deleted. */
-		if (!(cur_ipx_route && route_id_cmp_result == 0))
+		if (   !cur_ipx_route
+		    || route_dest_cmp_result != 0
+		    || *p_effective_metric != cur_plat_route->rx.metric)
 			vtable->vt->route_delete (priv->platform, ifindex, cur_plat_route);
 
 		cur_plat_route = _get_next_plat_route (plat_routes_idx, FALSE, &i_plat_routes);
 	}
 
 	/***************************************************************************
-	 * Restore shadowed routes. These routes are on an other @ifindex, but were
+	 * Restore shadowed routes. These routes are on an other @ifindex then what
-	 * shadowed before. Unshadow them now.
+	 * we are syncing now. But the current changes make it necessary to add those
+	 * routes.
+	 *
+	 * Only add some routes that might be necessary. We don't delete any routes
+	 * on other ifindexes here. I.e. we don't do a full sync, but only ~add~ routes
+	 * that were shadowed previously, but should be now present with a different
+	 * metric.
 	 **************************************************************************/
 
-	if (to_restore_routes) {
+	if (ipx_routes_changed) {
-		for (i_type = 0; i_type < 2; i_type++) {
+		/* @effective_metrics_reverse contains the list of assigned metrics from the last
-			for (i = 0; i < to_restore_routes->len; i++) {
+		 * sync. Walk through it and see what changes there are (and possibly restore a
-				const NMPlatformIPXRoute *rest_route = VTABLE_ROUTE_INDEX (vtable, to_restore_routes, i);
+		 * shadowed route).
+		 * Thereby also update @effective_metrics_reverse to be up-to-date again. */
+		for (i_ipx_routes = 0; i_ipx_routes < ipx_routes->entries->len; i_ipx_routes++) {
+			guint i_ipx_routes_reverse;
+			gint64 *p_effective_metric_reversed;
 
-				if (   (i_type == 0 && !VTABLE_IS_DEVICE_ROUTE (vtable, rest_route))
+			p_effective_metric = &effective_metrics[i_ipx_routes];
-				    || (i_type == 1 && VTABLE_IS_DEVICE_ROUTE (vtable, rest_route))) {
+
-					/* Make two runs over the list of @to_restore_routes. On the first, only add
+			i_ipx_routes_reverse = _route_index_reverse_idx (vtable, ipx_routes->index, i_ipx_routes, ipx_routes->entries);
-					 * device routes, on the second the others (gateway routes). */
+			p_effective_metric_reversed = &g_array_index (ipx_routes->effective_metrics_reverse, gint64, i_ipx_routes_reverse);
-					continue;
+
-				}
+			if (*p_effective_metric_reversed == *p_effective_metric) {
-				vtable->vt->route_add (priv->platform, 0, rest_route);
+				/* The entry is up to date. No change, continue with the next one. */
+				continue;
 			}
+			*p_effective_metric_reversed = *p_effective_metric;
+
+			if (*p_effective_metric == -1) {
+				/* the entry is shadowed. Nothing to do. */
+				continue;
+			}
+
+			cur_ipx_route = ipx_routes->index->entries[i_ipx_routes];
+			if (cur_ipx_route->rx.ifindex == ifindex) {
+				/* @cur_ipx_route is on the current @ifindex. No need to special handling them
+				 * because we are about to do a full sync of the ifindex. */
+				continue;
+			}
+
+			/* the effective metric from previous sync changed. While @cur_ipx_route is not on the
+			 * ifindex we are about to sync, we still must add this route. Possibly it was shadowed
+			 * before, and now we want to restore it.
+			 *
+			 * Note that we don't do a full sync on the other ifindex. Especially, we don't delete
+			 * or add any further routes then this. That means there might be some stale routes
+			 * (with a higher metric!). They will only be removed on the next sync of that other
+			 * ifindex. */
+			vtable->vt->route_add (priv->platform, 0, cur_ipx_route, *p_effective_metric);
 		}
-		g_array_unref (to_restore_routes);
 	}
 
 	/***************************************************************************
@@ -598,7 +770,7 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 		 * we need to know whether a route is shadowed by another route, and that
 		 * requires to look at @ipx_routes. */
 		for (; cur_ipx_route; cur_ipx_route = _get_next_ipx_route (ipx_routes->index, FALSE, &i_ipx_routes, ifindex)) {
-			int route_id_cmp_result = -1;
+			int route_id_dest_result = -1;
 
 			if (   (i_type == 0 && !VTABLE_IS_DEVICE_ROUTE (vtable, cur_ipx_route))
 			    || (i_type == 1 && VTABLE_IS_DEVICE_ROUTE (vtable, cur_ipx_route))) {
@@ -607,24 +779,29 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 				continue;
 			}
 
-			if (   i_ipx_routes > 0
+			p_effective_metric = &effective_metrics[i_ipx_routes];
-			    && vtable->route_id_cmp (cur_ipx_route, ipx_routes->index->entries[i_ipx_routes - 1]) == 0) {
+
+			if (*p_effective_metric == -1) {
 				/* @cur_ipx_route is shadewed by another route. */
 				continue;
 			}
 
 			/* skip over @plat_routes that are ordered before our @cur_ipx_route. */
 			while (   cur_plat_route
-			       && (route_id_cmp_result = vtable->route_id_cmp (cur_plat_route, cur_ipx_route)) < 0)
+			       && (route_id_dest_result = vtable->route_dest_cmp (cur_plat_route, cur_ipx_route)) <= 0) {
+				if (   route_id_dest_result == 0
+				    && cur_plat_route->rx.metric >= *p_effective_metric)
+					break;
 				cur_plat_route = _get_next_plat_route (plat_routes_idx, FALSE, &i_plat_routes);
+			}
 
 			/* only add the route if we don't have an identical route in @plat_routes,
 			 * i.e. if @cur_plat_route is different from @cur_ipx_route. */
 			if (   !cur_plat_route
-			    || route_id_cmp_result != 0
+			    || route_id_dest_result != 0
-			    || !_route_equals_ignoring_ifindex (vtable, cur_plat_route, cur_ipx_route)) {
+			    || !_route_equals_ignoring_ifindex (vtable, cur_plat_route, cur_ipx_route, *p_effective_metric)) {
 
-				if (!vtable->vt->route_add (priv->platform, ifindex, cur_ipx_route)) {
+				if (!vtable->vt->route_add (priv->platform, ifindex, cur_ipx_route, *p_effective_metric)) {
 					if (cur_ipx_route->rx.source < NM_IP_CONFIG_SOURCE_USER) {
 						_LOGD (vtable->vt->addr_family,
 						       "ignore error adding IPv%c route to kernel: %s",
@@ -874,11 +1051,13 @@ nm_route_manager_ip4_route_register_device_route_purge_list (NMRouteManager *sel
 
 static const VTableIP vtable_v4 = {
 	.vt                             = &nm_platform_vtable_route_v4,
+	.route_dest_cmp                 = (int (*) (const NMPlatformIPXRoute *, const NMPlatformIPXRoute *)) _v4_route_dest_cmp,
 	.route_id_cmp                   = (int (*) (const NMPlatformIPXRoute *, const NMPlatformIPXRoute *)) _v4_route_id_cmp,
 };
 
 static const VTableIP vtable_v6 = {
 	.vt                             = &nm_platform_vtable_route_v6,
+	.route_dest_cmp                 = (int (*) (const NMPlatformIPXRoute *, const NMPlatformIPXRoute *)) _v6_route_dest_cmp,
 	.route_id_cmp                   = (int (*) (const NMPlatformIPXRoute *, const NMPlatformIPXRoute *)) _v6_route_id_cmp,
 };
 
@@ -893,6 +1072,10 @@ nm_route_manager_init (NMRouteManager *self)
 
 	priv->ip4_routes.entries = g_array_new (FALSE, FALSE, sizeof (NMPlatformIP4Route));
 	priv->ip6_routes.entries = g_array_new (FALSE, FALSE, sizeof (NMPlatformIP6Route));
+	priv->ip4_routes.effective_metrics = g_array_new (FALSE, FALSE, sizeof (gint64));
+	priv->ip6_routes.effective_metrics = g_array_new (FALSE, FALSE, sizeof (gint64));
+	priv->ip4_routes.effective_metrics_reverse = g_array_new (FALSE, FALSE, sizeof (gint64));
+	priv->ip6_routes.effective_metrics_reverse = g_array_new (FALSE, FALSE, sizeof (gint64));
 	priv->ip4_routes.index = _route_index_create (&vtable_v4, priv->ip4_routes.entries);
 	priv->ip6_routes.index = _route_index_create (&vtable_v6, priv->ip6_routes.entries);
 	priv->ip4_device_routes.entries = g_hash_table_new_full ((GHashFunc) nmp_object_id_hash,
@@ -922,6 +1105,10 @@ finalize (GObject *object)
 
 	g_array_free (priv->ip4_routes.entries, TRUE);
 	g_array_free (priv->ip6_routes.entries, TRUE);
+	g_array_free (priv->ip4_routes.effective_metrics, TRUE);
+	g_array_free (priv->ip6_routes.effective_metrics, TRUE);
+	g_array_free (priv->ip4_routes.effective_metrics_reverse, TRUE);
+	g_array_free (priv->ip6_routes.effective_metrics_reverse, TRUE);
 	g_free (priv->ip4_routes.index);
 	g_free (priv->ip6_routes.index);
 

src/platform/nm-platform.c

@@ -2941,7 +2941,7 @@ log_ip6_route (NMPlatform *self, NMPObjectType obj_type, int ifindex, NMPlatform
 /******************************************************************/
 
 static gboolean
-_vtr_v4_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route)
+_vtr_v4_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route, gint64 metric)
 {
 	return nm_platform_ip4_route_add (self,
 	                                  ifindex > 0 ? ifindex : route->rx.ifindex,
@@ -2950,12 +2950,12 @@ _vtr_v4_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *rout
 	                                  route->rx.plen,
 	                                  route->r4.gateway,
 	                                  route->r4.pref_src,
-	                                  route->rx.metric,
+	                                  metric >= 0 ? (guint32) metric : route->rx.metric,
 	                                  route->rx.mss);
 }
 
 static gboolean
-_vtr_v6_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route)
+_vtr_v6_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route, gint64 metric)
 {
 	return nm_platform_ip6_route_add (self,
 	                                  ifindex > 0 ? ifindex : route->rx.ifindex,
@@ -2963,7 +2963,7 @@ _vtr_v6_route_add (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *rout
 	                                  route->r6.network,
 	                                  route->rx.plen,
 	                                  route->r6.gateway,
-	                                  route->rx.metric,
+	                                  metric >= 0 ? (guint32) metric : route->rx.metric,
 	                                  route->rx.mss);
 }
 

src/platform/nm-platform.h

@@ -315,7 +315,7 @@ typedef struct {
 	int (*route_cmp) (const NMPlatformIPXRoute *a, const NMPlatformIPXRoute *b);
 	const char *(*route_to_string) (const NMPlatformIPXRoute *route);
 	GArray *(*route_get_all) (NMPlatform *self, int ifindex, NMPlatformGetRouteFlags flags);
-	gboolean (*route_add) (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route);
+	gboolean (*route_add) (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route, gint64 metric);
 	gboolean (*route_delete) (NMPlatform *self, int ifindex, const NMPlatformIPXRoute *route);
 	gboolean (*route_delete_default) (NMPlatform *self, int ifindex, guint32 metric);
 	guint32 (*metric_normalize) (guint32 metric);

src/tests/test-route-manager.c

@@ -191,6 +191,26 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 			.mss = 0,
 			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("6.6.6.0"),
+			.plen = 24,
+			.ifindex = fixture->ifindex1,
+			.gateway = INADDR_ANY,
+			.metric = 21,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("8.0.0.0"),
+			.plen = 8,
+			.ifindex = fixture->ifindex1,
+			.gateway = nmtst_inet4_from_string ("6.6.6.2"),
+			.metric = 22,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_UNIVERSE),
+		},
 	};
 
 	NMPlatformIP4Route state2[] = {
@@ -224,6 +244,26 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 			.mss = 0,
 			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("6.6.6.0"),
+			.plen = 24,
+			.ifindex = fixture->ifindex1,
+			.gateway = INADDR_ANY,
+			.metric = 21,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("8.0.0.0"),
+			.plen = 8,
+			.ifindex = fixture->ifindex1,
+			.gateway = nmtst_inet4_from_string ("6.6.6.2"),
+			.metric = 22,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_UNIVERSE),
+		},
 	};
 
 	NMPlatformIP4Route state3[] = {
@@ -247,22 +287,44 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 			.mss = 0,
 			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("8.0.0.0"),
+			.plen = 8,
+			.ifindex = fixture->ifindex1,
+			.gateway = nmtst_inet4_from_string ("6.6.6.2"),
+			.metric = 22,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_UNIVERSE),
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = nmtst_inet4_from_string ("6.6.6.0"),
+			.plen = 24,
+			.ifindex = fixture->ifindex1,
+			.gateway = INADDR_ANY,
+			/* this is a ghost entry because we synced ifindex0 and restore the route
+			 * with metric 20 (above). But we don't remove the metric 21. */
+			.metric = 21,
+			.mss = 0,
+			.scope_inv = nm_platform_route_scope_inv (RT_SCOPE_LINK),
+		},
 	};
 
 	setup_dev0_ip4 (fixture->ifindex0, 1000, 21021);
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip4-route '*: 8.0.0.0/8 22': *");
 	setup_dev1_ip4 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
-	/* 6.6.6.0/24 on dev0 won over 6.6.6.0/24 on dev1
+	/* - 6.6.6.0/24 on dev0 won over 6.6.6.0/24 on dev1
-	 * 7.0.0.0/8 routes did not clash
+	 * - 6.6.6.0/24 on dev1 has metric bumped.
-	 * 8.0.0.0/8 could not be added. */
+	 * - 7.0.0.0/8 route, metric 21021 added
+	 * - 7.0.0.0/8 route, metric 22 added
+	 * - 8.0.0.0/8 could be added. */
 	routes = ip4_routes (fixture);
 	g_assert_cmpint (routes->len, ==, G_N_ELEMENTS (state1));
 	nmtst_platform_ip4_routes_equal ((NMPlatformIP4Route *) routes->data, state1, routes->len, TRUE);
 	g_array_free (routes, TRUE);
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip4-route '*: 8.0.0.0/8 22': *");
 	setup_dev1_ip4 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
@@ -278,7 +340,7 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 
 	update_dev0_ip4 (fixture->ifindex0);
 
-	/* 7.0.0.0/8 on dev0 was updated for gateway removal*/
+	/* minor changes in the routes. Quite similar to state1. */
 	routes = ip4_routes (fixture);
 	g_assert_cmpint (routes->len, ==, G_N_ELEMENTS (state2));
 	nmtst_platform_ip4_routes_equal ((NMPlatformIP4Route *) routes->data, state2, routes->len, TRUE);
@@ -287,8 +349,9 @@ test_ip4 (test_fixture *fixture, gconstpointer user_data)
 	nm_route_manager_route_flush (nm_route_manager_get (), fixture->ifindex0);
 
 	/* 6.6.6.0/24 is now on dev1
+	 * 6.6.6.0/24 is also still on dev1 with bumped metric 21.
 	 * 7.0.0.0/8 gone from dev0, still present on dev1
-	 * 8.0.0.0/8 is present on dev1 now that 6.6.6.0/24 is on dev1 too
+	 * 8.0.0.0/8 is present on dev1
 	 * No dev0 routes left. */
 	routes = ip4_routes (fixture);
 	g_assert_cmpint (routes->len, ==, G_N_ELEMENTS (state3));
@@ -512,6 +575,33 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 			.metric = 22,
 			.mss = 0,
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:1337::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 1025,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:8086::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 21,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:d34d::"),
+			.plen = 64,
+			.ifindex = fixture->ifindex1,
+			.gateway = *nmtst_inet6_from_string ("2001:db8:8086::2"),
+			.metric = 20,
+			.mss = 0,
+		},
 	};
 
 	NMPlatformIP6Route state2[] = {
@@ -551,6 +641,33 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 			.metric = 22,
 			.mss = 0,
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:1337::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 1025,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:8086::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 21,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:d34d::"),
+			.plen = 64,
+			.ifindex = fixture->ifindex1,
+			.gateway = *nmtst_inet6_from_string ("2001:db8:8086::2"),
+			.metric = 20,
+			.mss = 0,
+		},
 	};
 
 	NMPlatformIP6Route state3[] = {
@@ -581,10 +698,36 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 			.metric = 1024,
 			.mss = 0,
 		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:1337::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 1025,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:8086::"),
+			.plen = 48,
+			.ifindex = fixture->ifindex1,
+			.gateway = in6addr_any,
+			.metric = 21,
+			.mss = 0,
+		},
+		{
+			.source = NM_IP_CONFIG_SOURCE_USER,
+			.network = *nmtst_inet6_from_string ("2001:db8:d34d::"),
+			.plen = 64,
+			.ifindex = fixture->ifindex1,
+			.gateway = *nmtst_inet6_from_string ("2001:db8:8086::2"),
+			.metric = 20,
+			.mss = 0,
+		},
 	};
 
 	setup_dev0_ip6 (fixture->ifindex0);
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip6-route '*: 2001:db8:d34d::/64 20': *");
 	setup_dev1_ip6 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 
@@ -597,7 +740,7 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 	nmtst_platform_ip6_routes_equal ((NMPlatformIP6Route *) routes->data, state1, routes->len, TRUE);
 	g_array_free (routes, TRUE);
 
-	g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, "*failure adding ip6-route '*: 2001:db8:d34d::/64 20': *");
+
 	setup_dev1_ip6 (fixture->ifindex1);
 	g_test_assert_expected_messages ();
 	setup_dev0_ip6 (fixture->ifindex0);