route-manager: always add conflicting direct routes and bump the route-metric if necessaryth/direct-route-conflict-bgo752062

Kernel does not allow to add the same route (as determined by network/plen,metric)
on two different interfaces (ifindex). In case of conflict, NMRouteManager used to
ignore any but the firstly added route.

On the other hand, we cannot add a gateway-route, if there is no direct
route to the gateway. Hence, skipping duplicate routes can mean that we
skip a direct route what was necessary to add other gateway-route,
which then leads to errors adding that route.

This is an even more serious issue since NMRouteManager manages the
device routes for IPv4 addresses.

For example, say you connect two interfaces to the same IP subnet.
The route-metric can conflict if the interfaces are of the same type
or if the users configured a conflict explicitly.
In case of conflicts, NMRouteManager would only configure the first
appearing route and skip the shadowed route on the second interface.
Now we cannot configure gateway-routes on the second interface because
the gateway is unreachable.

There are many scenarios where this issue can happen, especially with
default-routes and user-configured-routes.
For example with default-routes, ip4_config_merge_and_apply() would check
if the default-gateway requires an explict route and possibly add it.
But then NMRouteManager might not add the route because it is shadowed
by a route on an other interface.

This patch solves the issue by having NMRouteManager configure shadowed
routes too, similar to what NMDefaultRouteManager does.
It does that by searching for an unused, non-conflicting, higher metric
for the route, i.e. we bump the metric by 1 until we can add it without
conflict.

Also note that NMRouteManager preserves the order of activating routes.
That means, if you configure a conflicting route, the metric of that
route will be bumped. The effect is that the best route to a destination
stays on the first interface until it gets disconnected.

2 files changed, 347 insertions, 46 deletions

diff --git a/src/nm-route-manager.c b/src/nm-route-manager.c
index 44b855cb39..674c249181 100644
--- a/src/nm-route-manager.c
+++ b/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 {
@@ -443,11 +450,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);
 
@@ -459,6 +469,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);
 
@@ -469,7 +481,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));
 	}
 
 	/***************************************************************************
@@ -494,19 +509,6 @@ _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);
 		}
@@ -520,6 +522,7 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 				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) {
@@ -548,9 +551,13 @@ _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) {
+
+			g_array_set_size (ipx_routes->effective_metrics_reverse, ipx_routes->effective_metrics_reverse->len + to_add_routes->len);
+
 			for (i = 0; i < to_add_routes->len; i++) {
 				NMPlatformIPXRoute *ipx_route;
 
@@ -560,15 +567,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, i) = -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
 	 ***************************************************************************/
@@ -576,8 +668,10 @@ _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);
 
@@ -585,37 +679,90 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 
 		/* 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 (   *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
-	 * shadowed before. Unshadow them now.
+	 * Restore shadowed routes. These routes are on an other @ifindex then what
+	 * 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) {
-		for (i_type = 0; i_type < 2; i_type++) {
-			for (i = 0; i < to_restore_routes->len; i++) {
-				const NMPlatformIPXRoute *rest_route = VTABLE_ROUTE_INDEX (vtable, to_restore_routes, i);
+	if (ipx_routes_changed) {
 
-				if (   (i_type == 0 && !VTABLE_IS_DEVICE_ROUTE (vtable, rest_route))
-				    || (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
-					 * device routes, on the second the others (gateway routes). */
-					continue;
-				}
-				vtable->vt->route_add (priv->platform, 0, rest_route, -1);
+		/* If there were more entries in the last run, remove them from @effective_metrics_reverse
+		 * as they are no longer valid.
+		 * Otherwise, new entries are initialized with NUL, which will be properly behave with
+		 * _effective_metric_entry_update() to signal a change. */
+		g_array_set_size (ipx_routes->effective_metrics_reverse, ipx_routes->index->len);
+
+		/* @effective_metrics_reverse contains the list of assigned metrics from the last
+		 * sync. Walk through it and see what changes there are (and possibly restore a
+		 * 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;
+
+			p_effective_metric = &effective_metrics[i_ipx_routes];
+			cur_ipx_route = ipx_routes->index->entries[i_ipx_routes];
+
+			_LOGT (vtable_v4.vt->addr_family, ">>> UPDATE %3d - %s (%lld)",
+			       i_ipx_routes,
+			       vtable->vt->route_to_string (cur_ipx_route),
+			       (long long) *p_effective_metric);
+
+			i_ipx_routes_reverse = _route_index_reverse_idx (vtable, ipx_routes->index, i_ipx_routes, ipx_routes->entries);
+			p_effective_metric_reversed = &g_array_index (ipx_routes->effective_metrics_reverse, gint64, i_ipx_routes_reverse);
+
+			if (*p_effective_metric_reversed == *p_effective_metric) {
+				/* 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;
 			}
+
+			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!) on that interface. 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);
 	}
 
 	/***************************************************************************
@@ -639,24 +786,28 @@ _vx_route_sync (const VTableIP *vtable, NMRouteManager *self, int ifindex, const
 				continue;
 			}
 
-			if (   i_ipx_routes > 0
-			    && vtable->route_id_cmp (cur_ipx_route, ipx_routes->index->entries[i_ipx_routes - 1]) == 0) {
+			p_effective_metric = &effective_metrics[i_ipx_routes];
+
+			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_cmp_result = vtable->route_dest_cmp (cur_plat_route, cur_ipx_route)) <= 0) {
+				if (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_equals_ignoring_ifindex (vtable, cur_plat_route, cur_ipx_route, -1)) {
+			    || !_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, -1)) {
+				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",
@@ -927,6 +1078,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,
@@ -956,6 +1111,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);
 
diff --git a/src/tests/test-route-manager.c b/src/tests/test-route-manager.c
index d62ffea33d..5325b83fae 100644
--- a/src/tests/test-route-manager.c
+++ b/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
-	 * 7.0.0.0/8 routes did not clash
-	 * 8.0.0.0/8 could not be added. */
+	/* - 6.6.6.0/24 on dev0 won over 6.6.6.0/24 on dev1
+	 * - 6.6.6.0/24 on dev1 has metric bumped.
+	 * - 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 ();
 
@@ -598,7 +741,6 @@ test_ip6 (test_fixture *fixture, gconstpointer user_data)
 	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);