From e7d51475a855f29f124b12e1de8709c2addb8be2 Mon Sep 17 00:00:00 2001 From: Thomas Haller 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 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 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 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 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 NMRouteManager, which wouldn't configure duplicate routes by bumping their metric). Since commit 77ec302714795f905301d500b9aab6c88001f32e we would no longer use NLM_F_EXCL, but add routes akin to `ip route append`. When activating for example two ethernet devices with no explict route metric configuration, there are two routes like default via 10.16.122.254 dev eth0 proto dhcp metric 100 default via 192.168.100.1 dev eth1 proto dhcp metric 100 This does not only affect default routes. In case of a multi-homing setup you'd get 192.168.100.0/24 dev eth0 proto kernel scope link src 192.168.100.1 metric 100 192.168.100.0/24 dev eth1 proto kernel scope link src 192.168.100.1 metric 100 but it's visible the most for default-routes. Note that we would append the routes that are activated later, as the order of `ip route show` confirms. One might hence expect, that kernel selects a route based on the order in the routing tables. However, that isn't the case, and activating the second interface will non-deterministically re-route traffic via the new interface. That will interfere badly with with NAT, stateful firewalls, and existing connections (like TCP). The solution is to have NMManager keep a global index of the default route-metrics currently in use. So, instead of determining the default-route metric based solely on the device-type, we now in addition generate default metrics that do not overlap. For example, if you activate eth0 first, it gets route-metric 100, and if you then activate eth1, it gets 101. Note that if you deactivate and re-activate eth0, then it will get route-metric 102, because the best route should stick on eth1 (which reserves the range 100 to 101). Note that when a connection explititly selects a particular metric, then that choice is honored (contrary to NMDefaultRouteManager which was more concerned with avoiding conflicts, then keeping the exact metric). https://bugzilla.redhat.com/show_bug.cgi?id=1505893 (cherry picked from commit 6a32c64d8fb2a9c1cfb78ab7e2f0bb3a269c81d7) (cherry picked from commit bd2d71754b770b43a71d85ca5f79832bbdb6b77a) --- src/devices/nm-device.c | 10 +- src/nm-manager.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++- src/nm-manager.h | 10 ++ 3 files changed, 255 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 755589674..40c425a5b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1765,7 +1765,10 @@ nm_device_get_route_metric (NMDevice *self, if (route_metric >= 0) goto out; } - route_metric = nm_device_get_route_metric_default (nm_device_get_device_type (self)); + + route_metric = nm_manager_device_route_metric_reserve (nm_manager_get (), + nm_device_get_ip_ifindex (self), + nm_device_get_device_type (self)); out: return nm_utils_ip_route_metric_normalize (addr_family, route_metric); } @@ -12482,6 +12485,11 @@ _cleanup_generic_pre (NMDevice *self, CleanupType cleanup_type) _cancel_activation (self); + if (cleanup_type != CLEANUP_TYPE_KEEP) { + nm_manager_device_route_metric_clear (nm_manager_get (), + nm_device_get_ip_ifindex (self)); + } + if ( cleanup_type == CLEANUP_TYPE_DECONFIGURE && priv->fw_state >= FIREWALL_STATE_INITIALIZED && priv->fw_mgr diff --git a/src/nm-manager.c b/src/nm-manager.c index 8b0b8013b..4d3416b37 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -160,6 +160,8 @@ typedef struct { NMAuthManager *auth_mgr; + GHashTable *device_route_metrics; + GSList *auth_chains; GHashTable *sleep_devices; @@ -324,6 +326,237 @@ static NM_CACHED_QUARK_FCN ("autoconnect-root", autoconnect_root_quark) /*****************************************************************************/ +typedef struct { + int ifindex; + guint32 aspired_metric; + guint32 effective_metric; +} DeviceRouteMetricData; + +static DeviceRouteMetricData * +_device_route_metric_data_new (int ifindex, guint32 metric) +{ + DeviceRouteMetricData *data; + + nm_assert (ifindex > 0); + + /* For IPv4, metrics can use the entire uint32 bit range. For IPv6, + * zero is treated like 1024. Since we handle IPv4 and IPv6 identically, + * we cannot allow a zero metric here. + */ + nm_assert (metric > 0); + + data = g_slice_new0 (DeviceRouteMetricData); + data->ifindex = ifindex; + data->aspired_metric = metric; + data->effective_metric = metric; + return data; +} + +static guint +_device_route_metric_data_by_ifindex_hash (gconstpointer p) +{ + const DeviceRouteMetricData *data = p; + NMHashState h; + + nm_hash_init (&h, 1030338191); + nm_hash_update_vals (&h, data->ifindex); + return nm_hash_complete (&h); +} + +static gboolean +_device_route_metric_data_by_ifindex_equal (gconstpointer pa, gconstpointer pb) +{ + const DeviceRouteMetricData *a = pa; + const DeviceRouteMetricData *b = pb; + + return a->ifindex == b->ifindex; +} + +static guint32 +_device_route_metric_get (NMManager *self, + int ifindex, + NMDeviceType device_type, + gboolean lookup_only) +{ + NMManagerPrivate *priv; + const DeviceRouteMetricData *d2; + DeviceRouteMetricData *data; + DeviceRouteMetricData data_lookup; + const NMDedupMultiHeadEntry *all_links_head; + NMPObject links_needle; + guint n_links; + gboolean cleaned = FALSE; + GHashTableIter h_iter; + + g_return_val_if_fail (NM_IS_MANAGER (self), 0); + + if (ifindex <= 0) { + if (lookup_only) + return 0; + return nm_device_get_route_metric_default (device_type); + } + + priv = NM_MANAGER_GET_PRIVATE (self); + + if ( lookup_only + && !priv->device_route_metrics) + return 0; + + if (G_UNLIKELY (!priv->device_route_metrics)) { + const GHashTable *h; + const NMConfigDeviceStateData *device_state; + + priv->device_route_metrics = g_hash_table_new_full (_device_route_metric_data_by_ifindex_hash, + _device_route_metric_data_by_ifindex_equal, + NULL, + nm_g_slice_free_fcn (DeviceRouteMetricData)); + cleaned = TRUE; + + /* we need to pre-populate the cache for all (still existing) devices from the state-file */ + h = nm_config_device_state_get_all (priv->config); + if (!h) + goto initited; + + 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) + continue; + if (!nm_platform_link_get (priv->platform, device_state->ifindex)) { + /* we have the entry in the state file, but (currently) no such + * ifindex exists in platform. Most likely the entry is obsolete, + * hence we skip it. */ + continue; + } + if (!nm_g_hash_table_add (priv->device_route_metrics, + _device_route_metric_data_new (device_state->ifindex, + device_state->route_metric_default))) + nm_assert_not_reached (); + } + } + +initited: + data_lookup.ifindex = ifindex; + + data = g_hash_table_lookup (priv->device_route_metrics, &data_lookup); + if (data) + return data->effective_metric; + if (lookup_only) + return 0; + + if (!cleaned) { + /* get the number of all links in the platform cache. */ + all_links_head = nm_platform_lookup_all (priv->platform, + NMP_CACHE_ID_TYPE_OBJECT_TYPE, + nmp_object_stackinit_id_link (&links_needle, 1)); + n_links = all_links_head ? all_links_head->len : 0; + + /* on systems where a lot of devices are created and go away, the index contains + * a lot of stale entries. We must from time to time clean them up. + * + * Do do this cleanup, whenever we have more enties then 2 times the number of links. */ + if (G_UNLIKELY (g_hash_table_size (priv->device_route_metrics) > NM_MAX (20, n_links * 2))) { + /* from time to time, we need to do some house-keeping and prune stale entries. + * Otherwise, on a system where interfaces frequently come and go (docker), we + * keep growing this cache for ifindexes that no longer exist. */ + g_hash_table_iter_init (&h_iter, priv->device_route_metrics); + while (g_hash_table_iter_next (&h_iter, NULL, (gpointer *) &d2)) { + if (!nm_platform_link_get (priv->platform, d2->ifindex)) + g_hash_table_iter_remove (&h_iter); + } + cleaned = TRUE; + } + } + + data = _device_route_metric_data_new (ifindex, nm_device_get_route_metric_default (device_type)); + + /* 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, + * but also, which metrics are now seemingly un-used but caused another reserved + * metric to be bumped. Hence, the naive O(n^2) search :( */ +again: + g_hash_table_iter_init (&h_iter, priv->device_route_metrics); + while (g_hash_table_iter_next (&h_iter, NULL, (gpointer *) &d2)) { + if ( data->effective_metric < d2->aspired_metric + || data->effective_metric > d2->effective_metric) { + /* no overlap. Skip. */ + continue; + } + if ( !cleaned + && !nm_platform_link_get (priv->platform, d2->ifindex)) { + /* the metric seems taken, but there is no such interface. This entry + * is stale, forget about it. */ + g_hash_table_iter_remove (&h_iter); + continue; + } + data->effective_metric = d2->effective_metric; + if (data->effective_metric == G_MAXUINT32) { + /* we cannot bump any further. Done. */ + break; + } + + if (data->effective_metric - data->aspired_metric > 50) { + /* as one active interface reserves an entire range of metrics + * (from aspired_metric to effective_metric), that means if you + * alternatingly activate two interfaces, their metric will + * juggle up. + * + * Limit this, don't bump the metric more then 50 times. */ + break; + } + + /* bump the metric, and search again. */ + data->effective_metric++; + goto again; + } + + _LOGT (LOGD_DEVICE, "default-route-metric: ifindex %d reserves metric %u (aspired %u)", + data->ifindex, data->effective_metric, data->aspired_metric); + + if (!nm_g_hash_table_add (priv->device_route_metrics, data)) + nm_assert_not_reached (); + + return data->effective_metric; +} + +guint32 +nm_manager_device_route_metric_reserve (NMManager *self, + int ifindex, + NMDeviceType device_type) +{ + guint32 metric; + + metric = _device_route_metric_get (self, ifindex, device_type, FALSE); + 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) +{ + NMManagerPrivate *priv; + DeviceRouteMetricData data_lookup; + + priv = NM_MANAGER_GET_PRIVATE (self); + + if (!priv->device_route_metrics) + return; + data_lookup.ifindex = ifindex; + if (g_hash_table_remove (priv->device_route_metrics, &data_lookup)) { + _LOGT (LOGD_DEVICE, "default-route-metric: ifindex %d released", + ifindex); + } +} + +/*****************************************************************************/ + static void _delete_volatile_connection_do (NMManager *self, NMSettingsConnection *connection) @@ -5228,7 +5461,7 @@ 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; + route_metric_default = nm_manager_device_route_metric_get (self, ifindex); if (nm_config_device_state_write (ifindex, managed_type, @@ -6607,6 +6840,8 @@ dispose (GObject *object) nm_clear_g_source (&priv->timestamp_update_id); + g_clear_pointer (&priv->device_route_metrics, g_hash_table_destroy); + G_OBJECT_CLASS (nm_manager_parent_class)->dispose (object); } diff --git a/src/nm-manager.h b/src/nm-manager.h index 2d463c718..d82570889 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -114,6 +114,16 @@ NMDevice * nm_manager_get_device_by_ifindex (NMManager *manager, NMDevice * nm_manager_get_device_by_path (NMManager *manager, const char *path); +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); + char * nm_manager_get_connection_iface (NMManager *self, NMConnection *connection, NMDevice **out_parent, -- 2.14.3 From f1728bdeac80fdc45ed969b8821a207383962296 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 19 Dec 2017 10:10:15 +0100 Subject: [PATCH 6/7] core: ensure that the default route-metric bumps at most 50 points First check that the limit of 50 metric points is not surpassed. Otherwise, if you have an ethernet device (aspired 100, effective 130) and a MACSec devic (aspired 125, effective 155), activating a new ethernet device would bump it's metric to 155 -- more then the 50 points limit. It doesn't matter too much, because the cases where the limit of 50 could have been surpassed were very specific. Still, change it to ensure that the limit is always honored as one would expect. Fixes: 6a32c64d8fb2a9c1cfb78ab7e2f0bb3a269c81d7 (cherry picked from commit 2499d3bdc6007308bf282cb44462990a4cd03b0e) (cherry picked from commit 5fd91fb67d3e3865545f5cbc1f39292635440cd3) --- src/nm-manager.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 4d3416b37..fad4f64b8 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -472,7 +472,11 @@ initited: /* 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, * but also, which metrics are now seemingly un-used but caused another reserved - * metric to be bumped. Hence, the naive O(n^2) search :( */ + * metric to be bumped. Hence, the naive O(n^2) search :( + * + * Well, technically, since we limit bumping the metric to 50, this entire + * loop runs at most 50 times, so it's still O(n). Let's just say, it's not + * very efficient. */ again: g_hash_table_iter_init (&h_iter, priv->device_route_metrics); while (g_hash_table_iter_next (&h_iter, NULL, (gpointer *) &d2)) { @@ -488,24 +492,30 @@ again: g_hash_table_iter_remove (&h_iter); continue; } - data->effective_metric = d2->effective_metric; - if (data->effective_metric == G_MAXUINT32) { - /* we cannot bump any further. Done. */ + + if (d2->effective_metric == G_MAXUINT32) { + /* we cannot bump the metric any further. Done. + * + * Actually, this can currently not happen because the aspired_metric + * are small numbers and we limit the bumping to 50. Still, for + * completeness... */ + data->effective_metric = G_MAXUINT32; break; } - if (data->effective_metric - data->aspired_metric > 50) { + if (d2->effective_metric - data->aspired_metric >= 50) { /* as one active interface reserves an entire range of metrics * (from aspired_metric to effective_metric), that means if you * alternatingly activate two interfaces, their metric will - * juggle up. + * bump each other. * - * Limit this, don't bump the metric more then 50 times. */ + * Limit this, bump the metric at most 50 points. */ + data->effective_metric = data->aspired_metric + 50; break; } /* bump the metric, and search again. */ - data->effective_metric++; + data->effective_metric = d2->effective_metric + 1; goto again; } -- 2.14.3 From a245f5eb91e8c30ae19403586f5ef9c14ef5f6d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Dec 2017 12:45:02 +0100 Subject: [PATCH 7/7] 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 (cherry picked from commit 4277bc0ee0479ad62369c3a261ea8d098e5e25ad) (cherry picked from commit fa53c715d1b929516e5e32e13ebc630a2c339394) --- src/nm-config.c | 50 +++++++++++++++++++++++++++++++++++--------------- src/nm-config.h | 6 ++++-- src/nm-manager.c | 48 +++++++++++++++++++++++++++--------------------- src/nm-manager.h | 3 --- 4 files changed, 66 insertions(+), 41 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 97ae3f6f4..29f0d5177 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1933,7 +1933,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"), @@ -1953,7 +1954,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); @@ -1996,10 +1998,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; @@ -2013,7 +2023,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) { @@ -2058,14 +2069,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; } @@ -2119,7 +2131,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; @@ -2161,11 +2174,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)) { @@ -2173,12 +2192,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 4c6fcca2b..42ab4682d 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -213,7 +213,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. */ @@ -233,7 +234,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 fad4f64b8..c011bfd0e 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -333,7 +333,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; @@ -343,12 +343,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; } @@ -376,7 +377,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; @@ -387,13 +389,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); @@ -419,7 +426,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 @@ -429,7 +436,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 (); } } @@ -439,7 +447,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; @@ -467,7 +475,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, @@ -525,6 +533,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; } @@ -535,18 +545,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) @@ -5441,7 +5444,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) @@ -5471,14 +5475,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 d82570889..b4587e088 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -118,9 +118,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); -- 2.14.3