core: persist aspired default route-metric in device's state file

NMManager tries to assign unique route-metrics in an increasing manner
so that the device which activates first keeps to have the best routes.

This information is also persisted in the device's state file, however
we not only need to persist the effective route-metric which was
eventually chosen by NMManager, but also the aspired metric.

The reason is that when a metric is chosen for a device, the entire
range between aspired and effective route-metric is reserved for that
device. We must remember the entire range so that after restart the
entire range is still considered to be in use.

Fixes: 6a32c64d8fb2a9c1cfb78ab7e2f0bb3a269c81d7

4 files changed, 66 insertions, 41 deletions

diff --git a/src/nm-config.c b/src/nm-config.c
index 01449a5fe4..a111054470 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -1929,7 +1929,8 @@ _nm_config_state_set (NMConfig *self,
 #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE   "perm-hw-addr-fake"
 #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_CONNECTION_UUID     "connection-uuid"
 #define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NM_OWNED            "nm-owned"
-#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT "route-metric-default"
+#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_ASPIRED   "route-metric-default-aspired"
+#define DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_EFFECTIVE "route-metric-default-effective"
 
 NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_device_state_managed_type_to_str, NMConfigDeviceStateManagedType,
 	NM_UTILS_LOOKUP_DEFAULT_NM_ASSERT ("unknown"),
@@ -1949,7 +1950,8 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf)
 	gsize perm_hw_addr_fake_len;
 	gint nm_owned = -1;
 	char *p;
-	guint32 route_metric_default;
+	guint32 route_metric_default_effective;
+	guint32 route_metric_default_aspired;
 
 	nm_assert (kf);
 	nm_assert (ifindex > 0);
@@ -1992,10 +1994,18 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf)
 
 	/* metric zero is not a valid metric. While zero valid for IPv4, for IPv6 it is an alias
 	 * for 1024. Since we handle here IPv4 and IPv6 the same, we cannot allow zero. */
-	route_metric_default = nm_config_keyfile_get_int64 (kf,
-	                                                    DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,
-	                                                    DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT,
-	                                                    10, 1, G_MAXUINT32, 0);
+	route_metric_default_effective = nm_config_keyfile_get_int64 (kf,
+	                                                              DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,
+	                                                              DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_EFFECTIVE,
+	                                                              10, 1, G_MAXUINT32, 0);
+	if (route_metric_default_effective) {
+		route_metric_default_aspired = nm_config_keyfile_get_int64 (kf,
+		                                                            DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,
+		                                                            DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_EFFECTIVE,
+		                                                            10, 1, route_metric_default_effective,
+		                                                            route_metric_default_effective);
+	} else
+		route_metric_default_aspired = 0;
 
 	connection_uuid_len = connection_uuid ? strlen (connection_uuid) + 1 : 0;
 	perm_hw_addr_fake_len = perm_hw_addr_fake ? strlen (perm_hw_addr_fake) + 1 : 0;
@@ -2009,7 +2019,8 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf)
 	device_state->connection_uuid = NULL;
 	device_state->perm_hw_addr_fake = NULL;
 	device_state->nm_owned = nm_owned;
-	device_state->route_metric_default = route_metric_default;
+	device_state->route_metric_default_aspired = route_metric_default_aspired;
+	device_state->route_metric_default_effective = route_metric_default_effective;
 
 	p = (char *) (&device_state[1]);
 	if (connection_uuid) {
@@ -2054,14 +2065,15 @@ nm_config_device_state_load (int ifindex)
 	               ", nm-owned=1" :
 	               (device_state->nm_owned == FALSE ? ", nm-owned=0" : "");
 
-	_LOGT ("device-state: %s #%d (%s); managed=%s%s%s%s%s%s%s%s, route-metric-default=%"G_GUINT32_FORMAT,
+	_LOGT ("device-state: %s #%d (%s); managed=%s%s%s%s%s%s%s%s, route-metric-default=%"G_GUINT32_FORMAT"-%"G_GUINT32_FORMAT"",
 	       kf ? "read" : "miss",
 	       ifindex, path,
 	       _device_state_managed_type_to_str (device_state->managed),
 	       NM_PRINT_FMT_QUOTED (device_state->connection_uuid, ", connection-uuid=", device_state->connection_uuid, "", ""),
 	       NM_PRINT_FMT_QUOTED (device_state->perm_hw_addr_fake, ", perm-hw-addr-fake=", device_state->perm_hw_addr_fake, "", ""),
 	       nm_owned_str,
-	       device_state->route_metric_default);
+	       device_state->route_metric_default_aspired,
+	       device_state->route_metric_default_effective);
 
 	return device_state;
 }
@@ -2115,7 +2127,8 @@ nm_config_device_state_write (int ifindex,
                               const char *perm_hw_addr_fake,
                               const char *connection_uuid,
                               gint nm_owned,
-                              guint32 route_metric_default)
+                              guint32 route_metric_default_aspired,
+                              guint32 route_metric_default_effective)
 {
 	char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60];
 	GError *local = NULL;
@@ -2157,11 +2170,17 @@ nm_config_device_state_write (int ifindex,
 		                        nm_owned);
 	}
 
-	if (route_metric_default != 0) {
+	if (route_metric_default_effective != 0) {
 		g_key_file_set_int64 (kf,
 		                      DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,
-		                      DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT,
-		                      route_metric_default);
+		                      DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_EFFECTIVE,
+		                      route_metric_default_effective);
+		if (route_metric_default_aspired != route_metric_default_effective) {
+			g_key_file_set_int64 (kf,
+			                      DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,
+			                      DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_ROUTE_METRIC_DEFAULT_ASPIRED,
+			                      route_metric_default_aspired);
+		}
 	}
 
 	if (!g_key_file_save_to_file (kf, path, &local)) {
@@ -2169,12 +2188,13 @@ nm_config_device_state_write (int ifindex,
 		g_error_free (local);
 		return FALSE;
 	}
-	_LOGT ("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s, route-metric-default=%"G_GUINT32_FORMAT,
+	_LOGT ("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s, route-metric-default=%"G_GUINT32_FORMAT"-%"G_GUINT32_FORMAT"",
 	       ifindex, path,
 	       _device_state_managed_type_to_str (managed),
 	       NM_PRINT_FMT_QUOTED (connection_uuid, ", connection-uuid=", connection_uuid, "", ""),
 	       NM_PRINT_FMT_QUOTED (perm_hw_addr_fake, ", perm-hw-addr-fake=", perm_hw_addr_fake, "", ""),
-	       route_metric_default);
+	       route_metric_default_aspired,
+	       route_metric_default_effective);
 	return TRUE;
 }
 
diff --git a/src/nm-config.h b/src/nm-config.h
index e9e000932c..6ef87f8da1 100644
--- a/src/nm-config.h
+++ b/src/nm-config.h
@@ -214,7 +214,8 @@ struct _NMConfigDeviceStateData {
 	NMConfigDeviceStateManagedType managed;
 
 	/* a value of zero means that no metric is set. */
-	guint32 route_metric_default;
+	guint32 route_metric_default_aspired;
+	guint32 route_metric_default_effective;
 
 	/* the UUID of the last settings-connection active
 	 * on the device. */
@@ -234,7 +235,8 @@ gboolean nm_config_device_state_write (int ifindex,
                                        const char *perm_hw_addr_fake,
                                        const char *connection_uuid,
                                        gint nm_owned,
-                                       guint32 route_metric_default);
+                                       guint32 route_metric_default_aspired,
+                                       guint32 route_metric_default_effective);
 
 void nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes);
 
diff --git a/src/nm-manager.c b/src/nm-manager.c
index 7161e87ca9..e124c4646a 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -334,7 +334,7 @@ typedef struct {
 } DeviceRouteMetricData;
 
 static DeviceRouteMetricData *
-_device_route_metric_data_new (int ifindex, guint32 metric)
+_device_route_metric_data_new (int ifindex, guint32 aspired_metric, guint32 effective_metric)
 {
 	DeviceRouteMetricData *data;
 
@@ -344,12 +344,13 @@ _device_route_metric_data_new (int ifindex, guint32 metric)
 	 * zero is treated like 1024. Since we handle IPv4 and IPv6 identically,
 	 * we cannot allow a zero metric here.
 	 */
-	nm_assert (metric > 0);
+	nm_assert (aspired_metric > 0);
+	nm_assert (effective_metric == 0 || aspired_metric <= effective_metric);
 
 	data = g_slice_new0 (DeviceRouteMetricData);
 	data->ifindex = ifindex;
-	data->aspired_metric = metric;
-	data->effective_metric = metric;
+	data->aspired_metric = aspired_metric;
+	data->effective_metric = effective_metric ?: aspired_metric;
 	return data;
 }
 
@@ -377,7 +378,8 @@ static guint32
 _device_route_metric_get (NMManager *self,
                           int ifindex,
                           NMDeviceType device_type,
-                          gboolean lookup_only)
+                          gboolean lookup_only,
+                          guint32 *out_aspired_metric)
 {
 	NMManagerPrivate *priv;
 	const DeviceRouteMetricData *d2;
@@ -388,13 +390,18 @@ _device_route_metric_get (NMManager *self,
 	guint n_links;
 	gboolean cleaned = FALSE;
 	GHashTableIter h_iter;
+	guint32 metric;
 
 	g_return_val_if_fail (NM_IS_MANAGER (self), 0);
 
+	NM_SET_OUT (out_aspired_metric, 0);
+
 	if (ifindex <= 0) {
 		if (lookup_only)
 			return 0;
-		return nm_device_get_route_metric_default (device_type);
+		metric = nm_device_get_route_metric_default (device_type);
+		NM_SET_OUT (out_aspired_metric, metric);
+		return metric;
 	}
 
 	priv = NM_MANAGER_GET_PRIVATE (self);
@@ -420,7 +427,7 @@ _device_route_metric_get (NMManager *self,
 
 		g_hash_table_iter_init (&h_iter, (GHashTable *) h);
 		while (g_hash_table_iter_next (&h_iter, NULL, (gpointer *) &device_state)) {
-			if (!device_state->route_metric_default)
+			if (!device_state->route_metric_default_effective)
 				continue;
 			if (!nm_platform_link_get (priv->platform, device_state->ifindex)) {
 				/* we have the entry in the state file, but (currently) no such
@@ -430,7 +437,8 @@ _device_route_metric_get (NMManager *self,
 			}
 			if (!nm_g_hash_table_add (priv->device_route_metrics,
 			                          _device_route_metric_data_new (device_state->ifindex,
-			                                                         device_state->route_metric_default)))
+			                                                         device_state->route_metric_default_aspired,
+			                                                         device_state->route_metric_default_effective)))
 				nm_assert_not_reached ();
 		}
 	}
@@ -440,7 +448,7 @@ initited:
 
 	data = g_hash_table_lookup (priv->device_route_metrics, &data_lookup);
 	if (data)
-		return data->effective_metric;
+		goto out;
 	if (lookup_only)
 		return 0;
 
@@ -468,7 +476,7 @@ initited:
 		}
 	}
 
-	data = _device_route_metric_data_new (ifindex, nm_device_get_route_metric_default (device_type));
+	data = _device_route_metric_data_new (ifindex, nm_device_get_route_metric_default (device_type), 0);
 
 	/* unfortunately, there is no stright forward way to lookup all reserved metrics.
 	 * Note, that we don't only have to know which metrics are currently reserved,
@@ -526,6 +534,8 @@ again:
 	if (!nm_g_hash_table_add (priv->device_route_metrics, data))
 		nm_assert_not_reached ();
 
+out:
+	NM_SET_OUT (out_aspired_metric, data->aspired_metric);
 	return data->effective_metric;
 }
 
@@ -536,18 +546,11 @@ nm_manager_device_route_metric_reserve (NMManager *self,
 {
 	guint32 metric;
 
-	metric = _device_route_metric_get (self, ifindex, device_type, FALSE);
+	metric = _device_route_metric_get (self, ifindex, device_type, FALSE, NULL);
 	nm_assert (metric != 0);
 	return metric;
 }
 
-guint32
-nm_manager_device_route_metric_get (NMManager *self,
-                                    int ifindex)
-{
-	return _device_route_metric_get (self, ifindex, NM_DEVICE_TYPE_UNKNOWN, TRUE);
-}
-
 void
 nm_manager_device_route_metric_clear (NMManager *self,
                                       int ifindex)
@@ -5439,7 +5442,8 @@ nm_manager_write_device_state (NMManager *self)
 		const char *uuid = NULL;
 		const char *perm_hw_addr_fake = NULL;
 		gboolean perm_hw_addr_is_fake;
-		guint32 route_metric_default;
+		guint32 route_metric_default_aspired;
+		guint32 route_metric_default_effective;
 
 		ifindex = nm_device_get_ip_ifindex (device);
 		if (ifindex <= 0)
@@ -5469,14 +5473,16 @@ nm_manager_write_device_state (NMManager *self)
 
 		nm_owned = nm_device_is_software (device) ? nm_device_is_nm_owned (device) : -1;
 
-		route_metric_default = nm_manager_device_route_metric_get (self, ifindex);
+		route_metric_default_effective = _device_route_metric_get (self, ifindex, NM_DEVICE_TYPE_UNKNOWN,
+		                                                           TRUE, &route_metric_default_aspired);
 
 		if (nm_config_device_state_write (ifindex,
 		                                  managed_type,
 		                                  perm_hw_addr_fake,
 		                                  uuid,
 		                                  nm_owned,
-		                                  route_metric_default))
+		                                  route_metric_default_aspired,
+		                                  route_metric_default_effective))
 			g_hash_table_add (seen_ifindexes, GINT_TO_POINTER (ifindex));
 	}
 
diff --git a/src/nm-manager.h b/src/nm-manager.h
index 34e868a9cb..1019022df4 100644
--- a/src/nm-manager.h
+++ b/src/nm-manager.h
@@ -119,9 +119,6 @@ guint32             nm_manager_device_route_metric_reserve (NMManager *self,
                                                             int ifindex,
                                                             NMDeviceType device_type);
 
-guint32             nm_manager_device_route_metric_get (NMManager *self,
-                                                        int ifindex);
-
 void                nm_manager_device_route_metric_clear (NMManager *self,
                                                           int ifindex);