policy: consider additional assumed routes when synchronizing the default route

Not only consider the best route of assumed devices when synching the route
metrics. This fixes the following scenario:

Have em1 assumed, with two default routes (metric 20 and 21).
When activating em2, NMDefaultRouteManager would  have determined
21 as the effective metric, thus overwriting the assumed route of em1.

Since we don't want to touch assumed interfaces, it is wrong to
overwrite the default route.

Instead, keep track of all the assumed default routes and consider their
metrics when choosing effective_metric.

Signed-off-by: Thomas Haller <thaller@redhat.com>

1 files changed, 96 insertions, 5 deletions

diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c
index 13b998011e..b8ee4247f9 100644
--- a/src/nm-default-route-manager.c
+++ b/src/nm-default-route-manager.c
@@ -153,7 +153,7 @@ _platform_route_sync_add (const VTableIP *vtable, NMDefaultRouteManager *self, g
 	NMDefaultRouteManagerPrivate *priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self);
 	GPtrArray *entries = vtable->get_entries (priv);
 	guint i;
-	gboolean has_unsynced_entry = FALSE;
+	Entry *entry_unsynced = NULL;
 	Entry *entry = NULL;
 	gboolean success;
 
@@ -174,11 +174,11 @@ _platform_route_sync_add (const VTableIP *vtable, NMDefaultRouteManager *self, g
 			if (!entry)
 				entry = e;
 		} else
-			has_unsynced_entry = TRUE;
+			entry_unsynced = e;
 	}
 
 	/* For synced entries, we expect that the metric is chosen uniquely. */
-	g_assert (!entry || !has_unsynced_entry || metric == G_MAXUINT32);
+	g_assert (!entry || !entry_unsynced || (entry->route.rx.ifindex == entry_unsynced->route.rx.ifindex) || metric == G_MAXUINT32);
 
 	/* we only add the route, if we have an (to be synced) entry for it. */
 	if (!entry)
@@ -294,21 +294,70 @@ _sort_entries_cmp (gconstpointer a, gconstpointer b, gpointer user_data)
 	return 0;
 }
 
+static GHashTable *
+_resync_all_construct_used_metric_index (const VTableIP *vtable, NMDefaultRouteManager *self, GArray *routes)
+{
+	NMDefaultRouteManagerPrivate *priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self);
+	GPtrArray *entries;
+	guint i, j;
+	GHashTable *result;
+
+	/* create a list of all metrics that are currently assigned on an interface
+	 * that is *not* already covered by one of our (synced) entries.
+	 * This effectivly returns the metrics that are in use by assumed interfaces
+	 * that we want to preserve. */
+
+	entries = vtable->get_entries (priv);
+
+	result = g_hash_table_new (NULL, NULL);
+
+	for (i = 0; i < routes->len; i++) {
+		gboolean ifindex_has_synced_entry = FALSE;
+		const NMPlatformIPRoute *route;
+
+		route = _vt_route_index (vtable, routes, i);
+
+		for (j = 0; j < entries->len; j++) {
+			Entry *e = g_ptr_array_index (entries, j);
+
+			if (   !e->never_default
+			    && e->synced
+			    && e->route.rx.ifindex == route->ifindex) {
+				ifindex_has_synced_entry = TRUE;
+				break;
+			}
+		}
+
+		if (!ifindex_has_synced_entry)
+			g_hash_table_add (result, GUINT_TO_POINTER (vtable->route_metric_normalize (route->metric)));
+	}
+
+	return result;
+}
+
 static void
 _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *changed_entry, const Entry *old_entry)
 {
 	NMDefaultRouteManagerPrivate *priv = NM_DEFAULT_ROUTE_MANAGER_GET_PRIVATE (self);
 	Entry *entry;
-	guint i;
+	guint i, j;
 	gint64 last_metric = -1;
 	guint32 expected_metric;
 	GPtrArray *entries;
 	GHashTableIter iter;
 	gpointer ptr;
 	GHashTable *changed_metrics = g_hash_table_new (NULL, NULL);
+	GHashTable *assumed_metrics;
+	GArray *routes;
 
 	entries = vtable->get_entries (priv);
 
+	routes = vtable->platform_route_get_all (0, NM_PLATFORM_GET_ROUTE_MODE_ONLY_DEFAULT);
+
+	/* construct a list of metrics that are reserved by assumed interfaces and
+	 * that we cannot reuse. */
+	assumed_metrics = _resync_all_construct_used_metric_index (vtable, self, routes);
+
 	if (old_entry && old_entry->synced) {
 		/* The old version obviously changed. */
 		g_hash_table_add (changed_metrics, GUINT_TO_POINTER (old_entry->effective_metric));
@@ -324,7 +373,24 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c
 			continue;
 
 		if (!entry->synced) {
-			last_metric = MAX (last_metric, (gint64) entry->effective_metric);
+			gboolean has_synced_entry = FALSE;
+
+			/* A non synced entry is completely ignored, if we have
+			 * a synced entry for the same if index.
+			 * Otherwise the metric of the entry is still remembered as
+			 * last_metric to avoid reusing it. */
+			for (j = 0; j < entries->len; j++) {
+				const Entry *e = g_ptr_array_index (entries, j);
+
+				if (   !e->never_default
+				    && e->synced
+					&& e->route.rx.ifindex == entry->route.rx.ifindex) {
+					has_synced_entry = TRUE;
+					break;
+				}
+			}
+			if (!has_synced_entry)
+				last_metric = MAX (last_metric, (gint64) entry->effective_metric);
 			continue;
 		}
 
@@ -332,6 +398,28 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c
 		if ((gint64) expected_metric <= last_metric)
 			expected_metric = last_metric == G_MAXUINT32 ? G_MAXUINT32 : last_metric + 1;
 
+		while (   expected_metric < G_MAXUINT32
+		       && g_hash_table_lookup_extended (assumed_metrics, GUINT_TO_POINTER (expected_metric), NULL, NULL)) {
+			gboolean has_metric_for_ifindex = FALSE;
+
+			/* Check if there are assumed devices that have default routes with this metric.
+			 * If there are any, we have to pick another effective_metric.
+			 *
+			 * If the assumed device has the same ifindex, we don't pick another metric an break the loop. */
+			for (j = 0; j < routes->len; j++) {
+				const NMPlatformIPRoute *r = _vt_route_index (vtable, routes, i);
+
+				if (   r->metric == expected_metric
+					&& r->ifindex == entry->route.rx.ifindex) {
+					has_metric_for_ifindex = TRUE;
+					break;
+				}
+			}
+			if (has_metric_for_ifindex)
+				break;
+			expected_metric++;
+		}
+
 		if (changed_entry == entry) {
 			/* for the changed entry, the previous metric was either old_entry->effective_metric,
 			 * or none. Hence, we only have to remember what is going to change. */
@@ -350,12 +438,15 @@ _resync_all (const VTableIP *vtable, NMDefaultRouteManager *self, const Entry *c
 		last_metric = expected_metric;
 	}
 
+	g_array_free (routes, TRUE);
+
 	g_hash_table_iter_init (&iter, changed_metrics);
 	while (g_hash_table_iter_next (&iter, &ptr, NULL))
 		_platform_route_sync_add (vtable, self, GPOINTER_TO_UINT (ptr));
 	_platform_route_sync_flush (vtable, self);
 
 	g_hash_table_unref (changed_metrics);
+	g_hash_table_unref (assumed_metrics);
 }
 
 static void