From e7d51475a855f29f124b12e1de8709c2addb8be2 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 6 Dec 2017 13:16:30 +0100

Subject: [PATCH 1/7] device: expose nm_device_get_route_metric_default()

(cherry picked from commit 989b5fabaac95f9367fb5f1c730db5dca7eab0de)

(cherry picked from commit ea78f156f2ef3402a5c4dde9c395cbf720b7aa4c)

---

 src/devices/nm-device.c | 8 ++++----

 src/devices/nm-device.h | 2 ++

 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c

index f75cc86e3..8179dbd3b 100644

--- a/src/devices/nm-device.c

+++ b/src/devices/nm-device.c

@@ -1616,8 +1616,8 @@ nm_device_get_metered (NMDevice *self)

 	return NM_DEVICE_GET_PRIVATE (self)->metered;

 }

-static guint32

-_get_route_metric_default (NMDevice *self)

+guint32

+nm_device_get_route_metric_default (NMDeviceType device_type)

 {

 	/* Device 'priority' is used for the default route-metric and is based on

 	 * the device type. The settings ipv4.route-metric and ipv6.route-metric

@@ -1636,7 +1636,7 @@ _get_route_metric_default (NMDevice *self)

 	 * metrics (except for IPv6, where 0 means 1024).

 	 */

-	switch (nm_device_get_device_type (self)) {

+	switch (device_type) {

 	/* 50 is reserved for VPN (NM_VPN_ROUTE_METRIC_DEFAULT) */

 	case NM_DEVICE_TYPE_ETHERNET:

 	case NM_DEVICE_TYPE_VETH:

@@ -1765,7 +1765,7 @@ nm_device_get_route_metric (NMDevice *self,

 		if (route_metric >= 0)

 			goto out;

 	}

-	route_metric = _get_route_metric_default (self);

+	route_metric = nm_device_get_route_metric_default (nm_device_get_device_type (self));

 out:

 	return nm_utils_ip_route_metric_normalize (addr_family, route_metric);

 }

diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h

index 810a613dd..ac73ee0c4 100644

--- a/src/devices/nm-device.h

+++ b/src/devices/nm-device.h

@@ -450,6 +450,8 @@ NMMetered       nm_device_get_metered           (NMDevice *dev);

 guint32         nm_device_get_route_table       (NMDevice *self, int addr_family, gboolean fallback_main);

 guint32         nm_device_get_route_metric      (NMDevice *dev, int addr_family);

+guint32         nm_device_get_route_metric_default (NMDeviceType device_type);

+

 const char *    nm_device_get_hw_address        (NMDevice *dev);

 const char *    nm_device_get_permanent_hw_address (NMDevice *self);

 const char *    nm_device_get_permanent_hw_address_full (NMDevice *self,

-- 

2.14.3

From 009ce5a739ec760acc38e8265776570e5d1a6a34 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 6 Dec 2017 12:49:21 +0100

Subject: [PATCH 2/7] core: add nm_config_keyfile_get_int64() util

(cherry picked from commit 3f38b765158c2ffcec7d1b314c3ba6aea3bc3e7b)

(cherry picked from commit 42fbc9410ba5c131e84c21d073d3d818feb2666d)

---

 src/nm-config.c | 27 +++++++++++++++++++++++++++

 src/nm-config.h |  7 +++++++

 2 files changed, 34 insertions(+)

diff --git a/src/nm-config.c b/src/nm-config.c

index de727c988..c344d5cd1 100644

--- a/src/nm-config.c

+++ b/src/nm-config.c

@@ -182,6 +182,33 @@ nm_config_keyfile_get_boolean (const GKeyFile *keyfile,

 	return nm_config_parse_boolean (str, default_value);

 }

+gint64

+nm_config_keyfile_get_int64 (const GKeyFile *keyfile,

+                             const char *section,

+                             const char *key,

+                             guint base,

+                             gint64 min,

+                             gint64 max,

+                             gint64 fallback)

+{

+	gint64 v;

+	int errsv;

+	char *str;

+

+	g_return_val_if_fail (keyfile, fallback);

+	g_return_val_if_fail (section, fallback);

+	g_return_val_if_fail (key, fallback);

+

+	str = g_key_file_get_value ((GKeyFile *) keyfile, section, key, NULL);

+	v = _nm_utils_ascii_str_to_int64 (str, base, min, max, fallback);

+	if (str) {

+		errsv = errno;

+		g_free (str);

+		errno = errsv;

+	}

+	return v;

+}

+

 char *

 nm_config_keyfile_get_value (const GKeyFile *keyfile,

                              const char *section,

diff --git a/src/nm-config.h b/src/nm-config.h

index 8bdd5002d..5b2dc65c4 100644

--- a/src/nm-config.h

+++ b/src/nm-config.h

@@ -165,6 +165,13 @@ gint nm_config_keyfile_get_boolean (const GKeyFile *keyfile,

                                     const char *section,

                                     const char *key,

                                     gint default_value);

+gint64 nm_config_keyfile_get_int64 (const GKeyFile *keyfile,

+                                    const char *section,

+                                    const char *key,

+                                    guint base,

+                                    gint64 min,

+                                    gint64 max,

+                                    gint64 fallback);

 char *nm_config_keyfile_get_value (const GKeyFile *keyfile,

                                    const char *section,

                                    const char *key,

-- 

2.14.3

From 412200a581b00c76757afb2250c3a9e3e719ca6b Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 6 Dec 2017 13:09:31 +0100

Subject: [PATCH 3/7] core: cache device state in NMConfig and load all at once

NMManager will need to know the state of all device at once.

Hence, load it once and cache it in NMConfig.

Note that this wastes a bit of memory in the order of

O(number-of-interfaces). But each device state entry is

rather small, and we always consume memory in the order

of O(number-of-interfaces).

(cherry picked from commit ea08df925f6a01e30ddcea4c15cea98d532593c6)

(cherry picked from commit 7b899334068c6945b0604b37510d91b02cbe62d9)

---

 src/devices/nm-device.c |  31 ++++----

 src/nm-config.c         | 186 +++++++++++++++++++++++++++++++++++-------------

 src/nm-config.h         |   5 ++

 src/nm-manager.c        |   5 +-

 4 files changed, 156 insertions(+), 71 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c

index 8179dbd3b..755589674 100644

--- a/src/devices/nm-device.c

+++ b/src/devices/nm-device.c

@@ -13512,6 +13512,7 @@ nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze)

 	gboolean success_read;

 	int ifindex;

 	const NMPlatformLink *pllink;

+	const NMConfigDeviceStateData *dev_state;

 	if (priv->hw_addr_perm) {

 		/* the permanent hardware address is only read once and not

@@ -13571,23 +13572,19 @@ nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze)

 	/* We also persist our choice of the fake address to the device state

 	 * file to use the same address on restart of NetworkManager.

 	 * First, try to reload the address from the state file. */

-	{

-		gs_free NMConfigDeviceStateData *dev_state = NULL;

-

-		dev_state = nm_config_device_state_load (ifindex);

-		if (   dev_state

-		    && dev_state->perm_hw_addr_fake

-		    && nm_utils_hwaddr_aton (dev_state->perm_hw_addr_fake, buf, priv->hw_addr_len)

-		    && !nm_utils_hwaddr_matches (buf, priv->hw_addr_len, priv->hw_addr, -1)) {

-			_LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use from statefile: %s, current: %s)",

-			       success_read

-			           ? "read HW addr length of permanent MAC address differs"

-			           : "unable to read permanent MAC address",

-			       dev_state->perm_hw_addr_fake,

-			       priv->hw_addr);

-			priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, priv->hw_addr_len);

-			goto notify_and_out;

-		}

+	dev_state = nm_config_device_state_get (nm_config_get (), ifindex);

+	if (   dev_state

+	    && dev_state->perm_hw_addr_fake

+	    && nm_utils_hwaddr_aton (dev_state->perm_hw_addr_fake, buf, priv->hw_addr_len)

+	    && !nm_utils_hwaddr_matches (buf, priv->hw_addr_len, priv->hw_addr, -1)) {

+		_LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use from statefile: %s, current: %s)",

+		       success_read

+		           ? "read HW addr length of permanent MAC address differs"

+		           : "unable to read permanent MAC address",

+		       dev_state->perm_hw_addr_fake,

+		       priv->hw_addr);

+		priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, priv->hw_addr_len);

+		goto notify_and_out;

 	}

 	_LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use current: %s)",

diff --git a/src/nm-config.c b/src/nm-config.c

index c344d5cd1..771d74f2a 100644

--- a/src/nm-config.c

+++ b/src/nm-config.c

@@ -121,6 +121,14 @@ typedef struct {

 	 * because the state changes only on explicit actions from the daemon

 	 * itself. */

 	State *state;

+

+	/* the hash table of device states. It is only loaded from disk

+	 * once and kept immutable afterwards.

+	 *

+	 * We also read all state file at once. We don't want to support

+	 * that they are changed outside of NM (at least not while NM is running).

+	 * Hence, we read them once, that's it. */

+	GHashTable *device_states;

 } NMConfigPrivate;

 struct _NMConfig {

@@ -1945,46 +1953,45 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf)

 	gint nm_owned = -1;

 	char *p;

+	nm_assert (kf);

 	nm_assert (ifindex > 0);

-	if (kf) {

-		switch (nm_config_keyfile_get_boolean (kf,

-		                                       DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

-		                                       DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_MANAGED,

-		                                       -1)) {

-		case TRUE:

-			managed_type = NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_MANAGED;

-			connection_uuid = nm_config_keyfile_get_value (kf,

-			                                               DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

-			                                               DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_CONNECTION_UUID,

-			                                               NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY);

-			break;

-		case FALSE:

-			managed_type = NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_UNMANAGED;

-			break;

-		case -1:

-			/* missing property in keyfile. */

-			break;

-		}

-

-		perm_hw_addr_fake = nm_config_keyfile_get_value (kf,

-		                                                 DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

-		                                                 DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE,

-		                                                 NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY);

-		if (perm_hw_addr_fake) {

-			char *normalized;

+	switch (nm_config_keyfile_get_boolean (kf,

+	                                       DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

+	                                       DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_MANAGED,

+	                                       -1)) {

+	case TRUE:

+		managed_type = NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_MANAGED;

+		connection_uuid = nm_config_keyfile_get_value (kf,

+		                                               DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

+		                                               DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_CONNECTION_UUID,

+		                                               NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY);

+		break;

+	case FALSE:

+		managed_type = NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_UNMANAGED;

+		break;

+	case -1:

+		/* missing property in keyfile. */

+		break;

+	}

-			normalized = nm_utils_hwaddr_canonical (perm_hw_addr_fake, -1);

-			g_free (perm_hw_addr_fake);

-			perm_hw_addr_fake = normalized;

-		}

+	perm_hw_addr_fake = nm_config_keyfile_get_value (kf,

+	                                                 DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

+	                                                 DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_PERM_HW_ADDR_FAKE,

+	                                                 NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY);

+	if (perm_hw_addr_fake) {

+		char *normalized;

-		nm_owned = nm_config_keyfile_get_boolean (kf,

-		                                          DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

-		                                          DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NM_OWNED,

-		                                          -1);

+		normalized = nm_utils_hwaddr_canonical (perm_hw_addr_fake, -1);

+		g_free (perm_hw_addr_fake);

+		perm_hw_addr_fake = normalized;

 	}

+	nm_owned = nm_config_keyfile_get_boolean (kf,

+	                                          DEVICE_RUN_STATE_KEYFILE_GROUP_DEVICE,

+	                                          DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NM_OWNED,

+	                                          -1);

+

 	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;

@@ -2034,14 +2041,13 @@ nm_config_device_state_load (int ifindex)

 	kf = nm_config_create_keyfile ();

 	if (!g_key_file_load_from_file (kf, path, G_KEY_FILE_NONE, NULL))

-		g_clear_pointer (&kf, g_key_file_unref);

+		return NULL;

 	device_state = _config_device_state_data_new (ifindex, kf);

 	nm_owned_str = device_state->nm_owned == TRUE ?

 	               ", 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",

 	       kf ? "read" : "miss",

 	       ifindex, path,

@@ -2053,6 +2059,49 @@ nm_config_device_state_load (int ifindex)

 	return device_state;

 }

+static int

+_device_state_parse_filename (const char *filename)

+{

+	if (!filename || !filename[0])

+		return 0;

+	if (!NM_STRCHAR_ALL (filename, ch, g_ascii_isdigit (ch)))

+		return 0;

+	return _nm_utils_ascii_str_to_int64 (filename, 10, 1, G_MAXINT, 0);

+}

+

+GHashTable *

+nm_config_device_state_load_all (void)

+{

+	GHashTable *states;

+	GDir *dir;

+	const char *fn;

+	int ifindex;

+

+	states = g_hash_table_new_full (nm_direct_hash, NULL, NULL, g_free);

+

+	dir = g_dir_open (NM_CONFIG_DEVICE_STATE_DIR, 0, NULL);

+	if (!dir)

+		return states;

+

+	while ((fn = g_dir_read_name (dir))) {

+		NMConfigDeviceStateData *state;

+

+		ifindex = _device_state_parse_filename (fn);

+		if (ifindex <= 0)

+			continue;

+

+		state = nm_config_device_state_load (ifindex);

+		if (!state)

+			continue;

+

+		if (!nm_g_hash_table_insert (states, GINT_TO_POINTER (ifindex), state))

+			nm_assert_not_reached ();

+	}

+	g_dir_close (dir);

+

+	return states;

+}

+

 gboolean

 nm_config_device_state_write (int ifindex,

                               NMConfigDeviceStateManagedType managed,

@@ -2121,7 +2170,6 @@ nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes)

 	const char *fn;

 	int ifindex;

 	gsize fn_len;

-	gsize i;

 	char buf[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/") + 30 + 3] = NM_CONFIG_DEVICE_STATE_DIR"/";

 	char *buf_p = &buf[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/")];

@@ -2132,24 +2180,20 @@ nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes)

 		return;

 	while ((fn = g_dir_read_name (dir))) {

-		fn_len = strlen (fn);

-

-		/* skip over file names that are not plain integers. */

-		for (i = 0; i < fn_len; i++) {

-			if (!g_ascii_isdigit (fn[i]))

-				break;

-		}

-		if (fn_len == 0 || i != fn_len)

+		ifindex = _device_state_parse_filename (fn);

+		if (ifindex <= 0)

 			continue;

-

-		ifindex = _nm_utils_ascii_str_to_int64 (fn, 10, 1, G_MAXINT, 0);

-		if (!ifindex)

-			continue;

-

 		if (g_hash_table_contains (seen_ifindexes, GINT_TO_POINTER (ifindex)))

 			continue;

-		memcpy (buf_p, fn, fn_len + 1);

+		fn_len = strlen (fn) + 1;

+		nm_assert (&buf_p[fn_len] < &buf[G_N_ELEMENTS (buf)]);

+		memcpy (buf_p, fn, fn_len);

+		nm_assert (({

+		                char bb[30];

+		                nm_sprintf_buf (bb, "%d", ifindex);

+		                nm_streq0 (bb, buf_p);

+		           }));

 		_LOGT ("device-state: prune #%d (%s)", ifindex, buf);

 		(void) unlink (buf);

 	}

@@ -2159,6 +2203,46 @@ nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes)

 /*****************************************************************************/

+static GHashTable *

+_device_state_get_all (NMConfig *self)

+{

+	NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (self);

+

+	if (G_UNLIKELY (!priv->device_states))

+		priv->device_states = nm_config_device_state_load_all ();

+	return priv->device_states;

+}

+

+/**

+ * nm_config_device_state_get_all:

+ * @self: the #NMConfig

+ *

+ * This function exists to give convenient access to all

+ * device states. Do not ever try to modify the returned

+ * hash, it's supposed to be immutable.

+ *

+ * Returns: the internal #GHashTable object with all device states.

+ */

+const GHashTable *

+nm_config_device_state_get_all (NMConfig *self)

+{

+	g_return_val_if_fail (NM_IS_CONFIG (self), NULL);

+

+	return _device_state_get_all (self);

+}

+

+const NMConfigDeviceStateData *

+nm_config_device_state_get (NMConfig *self,

+                            int ifindex)

+{

+	g_return_val_if_fail (NM_IS_CONFIG (self), NULL);

+	g_return_val_if_fail (ifindex > 0 , NULL);

+

+	return g_hash_table_lookup (_device_state_get_all (self), GINT_TO_POINTER (ifindex));

+}

+

+/*****************************************************************************/

+

 void

 nm_config_reload (NMConfig *self, NMConfigChangeFlags reload_flags)

 {

diff --git a/src/nm-config.h b/src/nm-config.h

index 5b2dc65c4..52a4099b2 100644

--- a/src/nm-config.h

+++ b/src/nm-config.h

@@ -224,6 +224,7 @@ struct _NMConfigDeviceStateData {

 };

 NMConfigDeviceStateData *nm_config_device_state_load (int ifindex);

+GHashTable *nm_config_device_state_load_all (void);

 gboolean nm_config_device_state_write (int ifindex,

                                        NMConfigDeviceStateManagedType managed,

                                        const char *perm_hw_addr_fake,

@@ -231,6 +232,10 @@ gboolean nm_config_device_state_write (int ifindex,

                                        gint nm_owned);

 void nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes);

+const GHashTable *nm_config_device_state_get_all (NMConfig *self);

+const NMConfigDeviceStateData *nm_config_device_state_get (NMConfig *self,

+                                                           int ifindex);

+

 /*****************************************************************************/

 #endif /* __NETWORKMANAGER_CONFIG_H__ */

diff --git a/src/nm-manager.c b/src/nm-manager.c

index 7f1b9a9d9..9e5f7ad4c 100644

--- a/src/nm-manager.c

+++ b/src/nm-manager.c

@@ -2640,10 +2640,9 @@ platform_query_devices (NMManager *self)

 		return;

 	for (i = 0; i < links->len; i++) {

 		const NMPlatformLink *link = NMP_OBJECT_CAST_LINK (links->pdata[i]);

-		gs_free NMConfigDeviceStateData *dev_state = NULL;

-

-		dev_state = nm_config_device_state_load (link->ifindex);

+		const NMConfigDeviceStateData *dev_state;

+		dev_state = nm_config_device_state_get (priv->config, link->ifindex);

 		platform_link_added (self,

 		                     link->ifindex,

 		                     link,

-- 

2.14.3

From e6d73154f23bdd6e0c4e9b52a9ea64bd3c1d8a13 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Wed, 6 Dec 2017 15:51:18 +0100

Subject: [PATCH 4/7] core: add read/write support for route-metric to

 NMConfig's device state

(cherry picked from commit a90b523a3e09f68d5700e73981ba84d40e4682a5)

(cherry picked from commit 282ed0d17501fda8b8c30020c029eb86f364e8bc)

---

 src/nm-config.c  | 29 ++++++++++++++++++++++++-----

 src/nm-config.h  |  9 +++++++--

 src/nm-manager.c |  6 +++++-

 3 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/nm-config.c b/src/nm-config.c

index 771d74f2a..97ae3f6f4 100644

--- a/src/nm-config.c

+++ b/src/nm-config.c

@@ -1933,6 +1933,7 @@ _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"

 NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_device_state_managed_type_to_str, NMConfigDeviceStateManagedType,

 	NM_UTILS_LOOKUP_DEFAULT_NM_ASSERT ("unknown"),

@@ -1952,6 +1953,7 @@ _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;

 	nm_assert (kf);

 	nm_assert (ifindex > 0);

@@ -1992,6 +1994,13 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf)

 	                                          DEVICE_RUN_STATE_KEYFILE_KEY_DEVICE_NM_OWNED,

 	                                          -1);

+	/* 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);

+

 	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;

@@ -2004,6 +2013,7 @@ _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;

 	p = (char *) (&device_state[1]);

 	if (connection_uuid) {

@@ -2048,13 +2058,14 @@ 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",

+	_LOGT ("device-state: %s #%d (%s); managed=%s%s%s%s%s%s%s%s, route-metric-default=%"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);

+	       nm_owned_str,

+	       device_state->route_metric_default);

 	return device_state;

 }

@@ -2107,7 +2118,8 @@ nm_config_device_state_write (int ifindex,

                               NMConfigDeviceStateManagedType managed,

                               const char *perm_hw_addr_fake,

                               const char *connection_uuid,

-                              gint nm_owned)

+                              gint nm_owned,

+                              guint32 route_metric_default)

 {

 	char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60];

 	GError *local = NULL;

@@ -2149,17 +2161,24 @@ nm_config_device_state_write (int ifindex,

 		                        nm_owned);

 	}

+	if (route_metric_default != 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);

+	}

 	if (!g_key_file_save_to_file (kf, path, &local)) {

 		_LOGW ("device-state: write #%d (%s) failed: %s", ifindex, path, local->message);

 		g_error_free (local);

 		return FALSE;

 	}

-	_LOGT ("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s",

+	_LOGT ("device-state: write #%d (%s); managed=%s%s%s%s%s%s%s, route-metric-default=%"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, "", ""));

+	       NM_PRINT_FMT_QUOTED (perm_hw_addr_fake, ", perm-hw-addr-fake=", perm_hw_addr_fake, "", ""),

+	       route_metric_default);

 	return TRUE;

 }

diff --git a/src/nm-config.h b/src/nm-config.h

index 52a4099b2..4c6fcca2b 100644

--- a/src/nm-config.h

+++ b/src/nm-config.h

@@ -212,6 +212,9 @@ struct _NMConfigDeviceStateData {

 	int ifindex;

 	NMConfigDeviceStateManagedType managed;

+	/* a value of zero means that no metric is set. */

+	guint32 route_metric_default;

+

 	/* the UUID of the last settings-connection active

 	 * on the device. */

 	const char *connection_uuid;

@@ -220,7 +223,7 @@ struct _NMConfigDeviceStateData {

 	/* whether the device was nm-owned (0/1) or -1 for

 	 * non-software devices. */

-	gint nm_owned;

+	int nm_owned:3;

 };

 NMConfigDeviceStateData *nm_config_device_state_load (int ifindex);

@@ -229,7 +232,9 @@ gboolean nm_config_device_state_write (int ifindex,

                                        NMConfigDeviceStateManagedType managed,

                                        const char *perm_hw_addr_fake,

                                        const char *connection_uuid,

-                                       gint nm_owned);

+                                       gint nm_owned,

+                                       guint32 route_metric_default);

+

 void nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes);

 const GHashTable *nm_config_device_state_get_all (NMConfig *self);

diff --git a/src/nm-manager.c b/src/nm-manager.c

index 9e5f7ad4c..8b0b8013b 100644

--- a/src/nm-manager.c

+++ b/src/nm-manager.c

@@ -5198,6 +5198,7 @@ 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;

 		ifindex = nm_device_get_ip_ifindex (device);

 		if (ifindex <= 0)

@@ -5227,11 +5228,14 @@ nm_manager_write_device_state (NMManager *self)

 		nm_owned = nm_device_is_software (device) ? nm_device_is_nm_owned (device) : -1;

+		route_metric_default = 0;

+

 		if (nm_config_device_state_write (ifindex,

 		                                  managed_type,

 		                                  perm_hw_addr_fake,

 		                                  uuid,

-		                                  nm_owned))

+		                                  nm_owned,

+		                                  route_metric_default))

 			g_hash_table_add (seen_ifindexes, GINT_TO_POINTER (ifindex));

 	}

-- 

2.14.3

From 747eb4bbb0f69f106e857e37256f26770bd9d518 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

Date: Tue, 5 Dec 2017 16:32:04 +0100

Subject: [PATCH 5/7] device: generate unique default route-metrics per

 interface

In the past we had NMDefaultRouteManager which would coordinate adding

the default-route with identical metrics. That especially happened, when

activating two devices of the same type, without explicitly specifying

ipv4.route-metric. For example, with ethernet devices, the routes on

both interfaces would get a metric of 100.

Coordinating routes was especially necessary, because we added

routes with NLM_F_EXCL flag, akin to `ip route replace`. We not

only had to avoid that activating two devices in NetworkManager would

result in a fight over the default-route, but more importently

to preserve externally added default-routes on unmanaged interfaces.

NMDefaultRouteManager would ensure that in case of duplicate

metrics, that the device that activated first would keep the

best default-route. It would do so by bumping the metric

of the second device to find a unused metric. The bumping itself

was not very important -- MDefaultRouteManager could also just not

configure any default-routes that show up as second, the result

would be quite similar. More important was to keep the best

default-route on the first activating device until the device

deactivates or a device activates that really has a better

default-route..

Likewise, NMRouteManager would globally manage non-default-routes.

It would not do any bumping of metrics, but it would also ensure that the routes

of the device that activates first are not overwritten by a device activating

later.

However, the `ip route replace` approach has downsides, especially

that it messes with routes on other interfaces, interfaces that are

possibly not managed by NetworkManager. Another downside is, that

binding a socket to an interface might not result in correct

routes, because the route might just not be there (in case of