From dabf3668385518dcf9242663419ba940d1d45d21 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 7 Dec 2017 15:20:42 +0100 Subject: [PATCH 1/6] utils: extend binary-search to return the first/last index binary-search can find an index of a matching entry in a sorted list. However, if the list contains multiple entries that compare equal, it can be interesting to find the first/last entry. For example, if you want to append new items after the last. Extend binary search to optionally continue the binary search to determine the range that compares equal. (cherry picked from commit d83eee5d575ffaa42da77e06698f6a93e85b62b8) --- libnm-core/nm-core-internal.h | 8 ++- libnm-core/nm-utils.c | 76 ++++++++++++++++++++----- libnm-core/tests/test-general.c | 99 +++++++++++++++++++++++++++++++-- 3 files changed, 163 insertions(+), 20 deletions(-) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 5a27d43886..15e5dc2a14 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -241,7 +241,13 @@ GPtrArray *_nm_utils_copy_object_array (const GPtrArray *array); gssize _nm_utils_ptrarray_find_first (gconstpointer *list, gssize len, gconstpointer needle); -gssize _nm_utils_ptrarray_find_binary_search (gconstpointer *list, gsize len, gconstpointer needle, GCompareDataFunc cmpfcn, gpointer user_data); +gssize _nm_utils_ptrarray_find_binary_search (gconstpointer *list, + gsize len, + gconstpointer needle, + GCompareDataFunc cmpfcn, + gpointer user_data, + gssize *out_idx_first, + gssize *out_idx_last); gssize _nm_utils_array_find_binary_search (gconstpointer list, gsize elem_size, gsize len, gconstpointer needle, GCompareDataFunc cmpfcn, gpointer user_data); GSList * _nm_utils_strv_to_slist (char **strv, gboolean deep_copy); diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 9aafdd9148..df58409bf1 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -650,36 +650,82 @@ _nm_utils_ptrarray_find_first (gconstpointer *list, gssize len, gconstpointer ne } gssize -_nm_utils_ptrarray_find_binary_search (gconstpointer *list, gsize len, gconstpointer needle, GCompareDataFunc cmpfcn, gpointer user_data) +_nm_utils_ptrarray_find_binary_search (gconstpointer *list, + gsize len, + gconstpointer needle, + GCompareDataFunc cmpfcn, + gpointer user_data, + gssize *out_idx_first, + gssize *out_idx_last) { - gssize imin, imax, imid; + gssize imin, imax, imid, i2min, i2max, i2mid; int cmp; g_return_val_if_fail (list || !len, ~((gssize) 0)); g_return_val_if_fail (cmpfcn, ~((gssize) 0)); imin = 0; - if (len == 0) - return ~imin; + if (len > 0) { + imax = len - 1; - imax = len - 1; + while (imin <= imax) { + imid = imin + (imax - imin) / 2; - while (imin <= imax) { - imid = imin + (imax - imin) / 2; + cmp = cmpfcn (list[imid], needle, user_data); + if (cmp == 0) { + /* we found a matching entry at index imid. + * + * Does the caller request the first/last index as well (in case that + * there are multiple entries which compare equal). */ - cmp = cmpfcn (list[imid], needle, user_data); - if (cmp == 0) - return imid; + if (out_idx_first) { + i2min = imin; + i2max = imid + 1; + while (i2min <= i2max) { + i2mid = i2min + (i2max - i2min) / 2; - if (cmp < 0) - imin = imid + 1; - else - imax = imid - 1; + cmp = cmpfcn (list[i2mid], needle, user_data); + if (cmp == 0) + i2max = i2mid -1; + else { + nm_assert (cmp < 0); + i2min = i2mid + 1; + } + } + *out_idx_first = i2min; + } + if (out_idx_last) { + i2min = imid + 1; + i2max = imax; + while (i2min <= i2max) { + i2mid = i2min + (i2max - i2min) / 2; + + cmp = cmpfcn (list[i2mid], needle, user_data); + if (cmp == 0) + i2min = i2mid + 1; + else { + nm_assert (cmp > 0); + i2max = i2mid - 1; + } + } + *out_idx_last = i2min - 1; + } + return imid; + } + + if (cmp < 0) + imin = imid + 1; + else + imax = imid - 1; + } } /* return the inverse of @imin. This is a negative number, but * also is ~imin the position where the value should be inserted. */ - return ~imin; + imin = ~imin; + NM_SET_OUT (out_idx_first, imin); + NM_SET_OUT (out_idx_last, imin); + return imin; } gssize diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index d743fd224a..520ed5814c 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -6139,7 +6139,7 @@ static void _test_find_binary_search_do (const int *array, gsize len) { gsize i; - gssize idx; + gssize idx, idx_first, idx_last; gs_free gconstpointer *parray = g_new (gconstpointer, len); const int NEEDLE = 0; gconstpointer pneedle = GINT_TO_POINTER (NEEDLE); @@ -6150,10 +6150,10 @@ _test_find_binary_search_do (const int *array, gsize len) expected_result = _nm_utils_ptrarray_find_first (parray, len, pneedle); - idx = _nm_utils_ptrarray_find_binary_search (parray, len, pneedle, _test_find_binary_search_cmp, NULL); - if (expected_result >= 0) + idx = _nm_utils_ptrarray_find_binary_search (parray, len, pneedle, _test_find_binary_search_cmp, NULL, &idx_first, &idx_last); + if (expected_result >= 0) { g_assert_cmpint (expected_result, ==, idx); - else { + } else { gssize idx2 = ~idx; g_assert_cmpint (idx, <, 0); @@ -6162,6 +6162,8 @@ _test_find_binary_search_do (const int *array, gsize len) g_assert (idx2 - 1 < 0 || _test_find_binary_search_cmp (parray[idx2 - 1], pneedle, NULL) < 0); g_assert (idx2 >= len || _test_find_binary_search_cmp (parray[idx2], pneedle, NULL) > 0); } + g_assert_cmpint (idx, ==, idx_first); + g_assert_cmpint (idx, ==, idx_last); for (i = 0; i < len; i++) { int cmp; @@ -6262,6 +6264,94 @@ test_nm_utils_ptrarray_find_binary_search (void) test_find_binary_search_do (-3, -2, -1, 1, 2, 3, 4); } +/*****************************************************************************/ + +#define BIN_SEARCH_W_DUPS_LEN 100 +#define BIN_SEARCH_W_DUPS_JITTER 10 + +static int +_test_bin_search2_cmp (gconstpointer pa, + gconstpointer pb, + gpointer user_data) +{ + int a = GPOINTER_TO_INT (pa); + int b = GPOINTER_TO_INT (pb); + + g_assert (a >= 0 && a <= BIN_SEARCH_W_DUPS_LEN + BIN_SEARCH_W_DUPS_JITTER); + g_assert (b >= 0 && b <= BIN_SEARCH_W_DUPS_LEN + BIN_SEARCH_W_DUPS_JITTER); + NM_CMP_DIRECT (a, b); + return 0; +} + +static int +_test_bin_search2_cmp_p (gconstpointer pa, + gconstpointer pb, + gpointer user_data) +{ + return _test_bin_search2_cmp (*((gpointer *) pa), *((gpointer *) pb), NULL); +} + +static void +test_nm_utils_ptrarray_find_binary_search_with_duplicates (void) +{ + gssize idx, idx2, idx_first2, idx_first, idx_last; + int i_test, i_len, i; + gssize j; + gconstpointer arr[BIN_SEARCH_W_DUPS_LEN]; + const int N_TEST = 10; + + for (i_test = 0; i_test < N_TEST; i_test++) { + for (i_len = 0; i_len < BIN_SEARCH_W_DUPS_LEN; i_len++) { + + /* fill with random numbers... surely there are some duplicates + * there... or maybe even there are none... */ + for (i = 0; i < i_len; i++) + arr[i] = GINT_TO_POINTER (nmtst_get_rand_int () % (i_len + BIN_SEARCH_W_DUPS_JITTER)); + g_qsort_with_data (arr, + i_len, + sizeof (gpointer), + _test_bin_search2_cmp_p, + NULL); + for (i = 0; i < i_len + BIN_SEARCH_W_DUPS_JITTER; i++) { + gconstpointer p = GINT_TO_POINTER (i); + + idx = _nm_utils_ptrarray_find_binary_search (arr, i_len, p, _test_bin_search2_cmp, NULL, &idx_first, &idx_last); + + idx_first2 = _nm_utils_ptrarray_find_first (arr, i_len, p); + + idx2 = _nm_utils_array_find_binary_search (arr, sizeof (gpointer), i_len, &p, _test_bin_search2_cmp_p, NULL); + g_assert_cmpint (idx, ==, idx2); + + if (idx_first2 < 0) { + g_assert_cmpint (idx, <, 0); + g_assert_cmpint (idx, ==, idx_first); + g_assert_cmpint (idx, ==, idx_last); + idx = ~idx; + g_assert_cmpint (idx, >=, 0); + g_assert_cmpint (idx, <=, i_len); + if (i_len == 0) + g_assert_cmpint (idx, ==, 0); + else { + g_assert (idx == i_len || GPOINTER_TO_INT (arr[idx]) > i); + g_assert (idx == 0 || GPOINTER_TO_INT (arr[idx - 1]) < i); + } + } else { + g_assert_cmpint (idx_first, ==, idx_first2); + g_assert_cmpint (idx_first, >=, 0); + g_assert_cmpint (idx_last, <, i_len); + g_assert_cmpint (idx_first, <=, idx_last); + g_assert_cmpint (idx, >=, idx_first); + g_assert_cmpint (idx, <=, idx_last); + for (j = idx_first; j < idx_last; j++) + g_assert (GPOINTER_TO_INT (arr[j]) == i); + g_assert (idx_first == 0 || GPOINTER_TO_INT (arr[idx_first - 1]) < i); + g_assert (idx_last == i_len - 1 || GPOINTER_TO_INT (arr[idx_last + 1]) > i); + } + } + } + } +} + /*****************************************************************************/ static void test_nm_utils_enum_from_str_do (GType type, const char *str, @@ -6952,6 +7042,7 @@ int main (int argc, char **argv) g_test_add_func ("/core/general/_glib_compat_g_ptr_array_insert", test_g_ptr_array_insert); g_test_add_func ("/core/general/_glib_compat_g_hash_table_get_keys_as_array", test_g_hash_table_get_keys_as_array); g_test_add_func ("/core/general/_nm_utils_ptrarray_find_binary_search", test_nm_utils_ptrarray_find_binary_search); + g_test_add_func ("/core/general/_nm_utils_ptrarray_find_binary_search_with_duplicates", test_nm_utils_ptrarray_find_binary_search_with_duplicates); g_test_add_func ("/core/general/_nm_utils_strstrdictkey", test_nm_utils_strstrdictkey); g_test_add_func ("/core/general/nm_ptrarray_len", test_nm_ptrarray_len); From ea78f156f2ef3402a5c4dde9c395cbf720b7aa4c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Dec 2017 13:16:30 +0100 Subject: [PATCH 2/6] device: expose nm_device_get_route_metric_default() (cherry picked from commit 989b5fabaac95f9367fb5f1c730db5dca7eab0de) --- 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 afc81dcde7..f45f85549d 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 810a613dde..ac73ee0c4b 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, From 42fbc9410ba5c131e84c21d073d3d818feb2666d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Dec 2017 12:49:21 +0100 Subject: [PATCH 3/6] core: add nm_config_keyfile_get_int64() util (cherry picked from commit 3f38b765158c2ffcec7d1b314c3ba6aea3bc3e7b) --- 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 de727c9882..c344d5cd15 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 8bdd5002d2..5b2dc65c46 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, From 7b899334068c6945b0604b37510d91b02cbe62d9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Dec 2017 13:09:31 +0100 Subject: [PATCH 4/6] 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) --- src/devices/nm-device.c | 31 +++---- src/nm-config.c | 190 +++++++++++++++++++++++++++++----------- src/nm-config.h | 5 ++ src/nm-manager.c | 5 +- 4 files changed, 158 insertions(+), 73 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f45f85549d..27282ff492 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 c344d5cd15..771d74f2ac 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; - - 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); + 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; + + 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 5b2dc65c46..52a4099b2c 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 7f1b9a9d96..9e5f7ad4cc 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, From 282ed0d17501fda8b8c30020c029eb86f364e8bc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 6 Dec 2017 15:51:18 +0100 Subject: [PATCH 5/6] core: add read/write support for route-metric to NMConfig's device state (cherry picked from commit a90b523a3e09f68d5700e73981ba84d40e4682a5) --- 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 771d74f2ac..97ae3f6f43 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 52a4099b2c..4c6fcca2b3 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 9e5f7ad4cc..8b0b8013b3 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)); } From bd2d71754b770b43a71d85ca5f79832bbdb6b77a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 5 Dec 2017 16:32:04 +0100 Subject: [PATCH 6/6] 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) --- 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 27282ff492..f9b2217f9a 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 8b0b8013b3..4d3416b37c 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 2d463c718a..d825708893 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,