From 0b4ebda859a3dc1c8ec4d8c887383d04bcd249f9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Mar 2020 13:21:48 +0100 Subject: [PATCH 1/4] core: cleanup nm_config_device_state_prune_unseen() and accept NMPlatform for skipping pruning (cherry picked from commit ad9e7488167ab25a5915040e813e76a5b669257b) --- src/nm-config.c | 36 +++++++++++++++++++++++------------- src/nm-config.h | 3 ++- src/nm-manager.c | 8 ++++---- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 063cbb746c..29e7cbc095 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -2298,6 +2298,8 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf) return device_state; } +#define DEVICE_STATE_FILENAME_LEN_MAX 60 + /** * nm_config_device_state_load: * @ifindex: the ifindex for which the state is to load @@ -2309,7 +2311,7 @@ NMConfigDeviceStateData * nm_config_device_state_load (int ifindex) { NMConfigDeviceStateData *device_state; - char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60]; + char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/") + DEVICE_STATE_FILENAME_LEN_MAX + 1]; gs_unref_keyfile GKeyFile *kf = NULL; const char *nm_owned_str; @@ -2393,7 +2395,7 @@ nm_config_device_state_write (int ifindex, const char *next_server, const char *root_path) { - char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60]; + char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/") + DEVICE_STATE_FILENAME_LEN_MAX + 1]; GError *local = NULL; gs_unref_keyfile GKeyFile *kf = NULL; @@ -2476,35 +2478,43 @@ nm_config_device_state_write (int ifindex, } void -nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes) +nm_config_device_state_prune_unseen (GHashTable *preserve_ifindexes, + NMPlatform *preserve_in_platform) { GDir *dir; const char *fn; - int ifindex; - gsize fn_len; - char buf[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/") + 30 + 3] = NM_CONFIG_DEVICE_STATE_DIR"/"; + char buf[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/") + DEVICE_STATE_FILENAME_LEN_MAX + 1] = NM_CONFIG_DEVICE_STATE_DIR"/"; char *buf_p = &buf[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR"/")]; - g_return_if_fail (seen_ifindexes); - dir = g_dir_open (NM_CONFIG_DEVICE_STATE_DIR, 0, NULL); if (!dir) return; while ((fn = g_dir_read_name (dir))) { + int ifindex; + gsize fn_len; + ifindex = _device_state_parse_filename (fn); if (ifindex <= 0) continue; - if (g_hash_table_contains (seen_ifindexes, GINT_TO_POINTER (ifindex))) + + if ( preserve_ifindexes + && g_hash_table_contains (preserve_ifindexes, GINT_TO_POINTER (ifindex))) continue; - fn_len = strlen (fn) + 1; + if ( preserve_in_platform + && nm_platform_link_get (preserve_in_platform, ifindex)) + continue; + + fn_len = strlen (fn); + nm_assert (fn_len > 0); nm_assert (&buf_p[fn_len] < &buf[G_N_ELEMENTS (buf)]); - memcpy (buf_p, fn, fn_len); + memcpy (buf_p, fn, fn_len + 1u); nm_assert (({ char bb[30]; - nm_sprintf_buf (bb, "%d", ifindex); - nm_streq0 (bb, buf_p); + + nm_streq0 (nm_sprintf_buf (bb, "%d", ifindex), + buf_p); })); _LOGT ("device-state: prune #%d (%s)", ifindex, buf); (void) unlink (buf); diff --git a/src/nm-config.h b/src/nm-config.h index d9460ebb46..048d64f41f 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -258,7 +258,8 @@ gboolean nm_config_device_state_write (int ifindex, const char *next_server, const char *root_path); -void nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes); +void nm_config_device_state_prune_unseen (GHashTable *preserve_ifindexes, + NMPlatform *preserve_in_platform); const GHashTable *nm_config_device_state_get_all (NMConfig *self); const NMConfigDeviceStateData *nm_config_device_state_get (NMConfig *self, diff --git a/src/nm-manager.c b/src/nm-manager.c index dbba5e4d17..c58d60a88f 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -6585,19 +6585,19 @@ void nm_manager_write_device_state_all (NMManager *self) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - gs_unref_hashtable GHashTable *seen_ifindexes = NULL; + gs_unref_hashtable GHashTable *preserve_ifindexes = NULL; NMDevice *device; - seen_ifindexes = g_hash_table_new (nm_direct_hash, NULL); + preserve_ifindexes = g_hash_table_new (nm_direct_hash, NULL); c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { if (nm_manager_write_device_state (self, device)) { - g_hash_table_add (seen_ifindexes, + g_hash_table_add (preserve_ifindexes, GINT_TO_POINTER (nm_device_get_ip_ifindex (device))); } } - nm_config_device_state_prune_unseen (seen_ifindexes); + nm_config_device_state_prune_unseen (preserve_ifindexes, NULL); } static gboolean From 7fa1e825450ce4b69670b5bfd5a9c705d9c05920 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Mar 2020 16:52:57 +0100 Subject: [PATCH 2/4] core/trivial: rename nm_config_device_state_prune_unseen() to nm_config_device_state_prune_stale() It's just a more fitting name after the previous change. (cherry picked from commit ecb0210e7a225f2b73149229cc96ac84f93404d1) --- src/nm-config.c | 4 ++-- src/nm-config.h | 4 ++-- src/nm-manager.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 29e7cbc095..4ea976574f 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -2478,8 +2478,8 @@ nm_config_device_state_write (int ifindex, } void -nm_config_device_state_prune_unseen (GHashTable *preserve_ifindexes, - NMPlatform *preserve_in_platform) +nm_config_device_state_prune_stale (GHashTable *preserve_ifindexes, + NMPlatform *preserve_in_platform) { GDir *dir; const char *fn; diff --git a/src/nm-config.h b/src/nm-config.h index 048d64f41f..b4478ceb04 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -258,8 +258,8 @@ gboolean nm_config_device_state_write (int ifindex, const char *next_server, const char *root_path); -void nm_config_device_state_prune_unseen (GHashTable *preserve_ifindexes, - NMPlatform *preserve_in_platform); +void nm_config_device_state_prune_stale (GHashTable *preserve_ifindexes, + NMPlatform *preserve_in_platform); const GHashTable *nm_config_device_state_get_all (NMConfig *self); const NMConfigDeviceStateData *nm_config_device_state_get (NMConfig *self, diff --git a/src/nm-manager.c b/src/nm-manager.c index c58d60a88f..1022e866f5 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -6597,7 +6597,7 @@ nm_manager_write_device_state_all (NMManager *self) } } - nm_config_device_state_prune_unseen (preserve_ifindexes, NULL); + nm_config_device_state_prune_stale (preserve_ifindexes, NULL); } static gboolean From fb6e14cf3f602a840bee66c27dfcb754872f1525 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Mar 2020 13:38:49 +0100 Subject: [PATCH 3/4] core: return ifindex from nm_manager_write_device_state() nm_manager_write_device_state() writes the device state to a file. The ifindex is here important, because that is the identifier for the device and is also used as file name. Return the ifindex that was used, instead of letting the caller reimplement the knowledge which ifindex was used. (cherry picked from commit 5477847eed9654727df5b70767a2a6498da1cb67) --- src/nm-manager.c | 34 +++++++++++++++++++++------------- src/nm-manager.h | 2 +- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 1022e866f5..3649c87e0f 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1515,7 +1515,7 @@ manager_device_state_changed (NMDevice *device, NM_DEVICE_STATE_UNMANAGED, NM_DEVICE_STATE_DISCONNECTED, NM_DEVICE_STATE_ACTIVATED)) - nm_manager_write_device_state (self, device); + nm_manager_write_device_state (self, device, NULL); if (NM_IN_SET (new_state, NM_DEVICE_STATE_UNAVAILABLE, @@ -6514,7 +6514,7 @@ start_factory (NMDeviceFactory *factory, gpointer user_data) } gboolean -nm_manager_write_device_state (NMManager *self, NMDevice *device) +nm_manager_write_device_state (NMManager *self, NMDevice *device, int *out_ifindex) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); int ifindex; @@ -6530,6 +6530,8 @@ nm_manager_write_device_state (NMManager *self, NMDevice *device) const char *next_server = NULL; const char *root_path = NULL; + NM_SET_OUT (out_ifindex, 0); + ifindex = nm_device_get_ip_ifindex (device); if (ifindex <= 0) return FALSE; @@ -6570,15 +6572,19 @@ nm_manager_write_device_state (NMManager *self, NMDevice *device) next_server = nm_dhcp4_config_get_option (dhcp4_config, "next_server"); } - return nm_config_device_state_write (ifindex, - managed_type, - perm_hw_addr_fake, - uuid, - nm_owned, - route_metric_default_aspired, - route_metric_default_effective, - next_server, - root_path); + if (!nm_config_device_state_write (ifindex, + managed_type, + perm_hw_addr_fake, + uuid, + nm_owned, + route_metric_default_aspired, + route_metric_default_effective, + next_server, + root_path)) + return FALSE; + + NM_SET_OUT (out_ifindex, ifindex); + return TRUE; } void @@ -6591,9 +6597,11 @@ nm_manager_write_device_state_all (NMManager *self) preserve_ifindexes = g_hash_table_new (nm_direct_hash, NULL); c_list_for_each_entry (device, &priv->devices_lst_head, devices_lst) { - if (nm_manager_write_device_state (self, device)) { + int ifindex; + + if (nm_manager_write_device_state (self, device, &ifindex)) { g_hash_table_add (preserve_ifindexes, - GINT_TO_POINTER (nm_device_get_ip_ifindex (device))); + GINT_TO_POINTER (ifindex)); } } diff --git a/src/nm-manager.h b/src/nm-manager.h index ad06e318f4..5873abd24b 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -103,7 +103,7 @@ NMSettingsConnection **nm_manager_get_activatable_connections (NMManager *manage guint *out_len); void nm_manager_write_device_state_all (NMManager *manager); -gboolean nm_manager_write_device_state (NMManager *manager, NMDevice *device); +gboolean nm_manager_write_device_state (NMManager *manager, NMDevice *device, int *out_ifindex); /* Device handling */ From d65b5c2e81f69ddc9217159a16f49d23d965da1c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 4 Mar 2020 15:44:53 +0100 Subject: [PATCH 4/4] core: periodically cleanup unused device state files from /run Otherwise, we only prune unused files when the service terminates. Usually, NetworkManager service doesn't get restarted before shutdown of the system (nor should it be). That means, if you create (and destroy) a large number of software devices, the state files pile up. From time to time, go through the files on disk and delete those that are no longer relevant. In this case, "from time to time" means after we write/update state files 100 times. (cherry picked from commit 332df7a58e86ce08cfd9331897d8b928ae6d267e) --- src/nm-manager.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 3649c87e0f..d7b2211e60 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -50,6 +50,8 @@ #include "nm-dispatcher.h" #include "NetworkManagerUtils.h" +#define DEVICE_STATE_PRUNE_RATELIMIT_MAX 100u + /*****************************************************************************/ typedef struct { @@ -191,6 +193,8 @@ typedef struct { NMConnectivityState connectivity_state; + guint8 device_state_prune_ratelimit_count; + bool startup:1; bool devices_inited:1; @@ -1514,9 +1518,23 @@ manager_device_state_changed (NMDevice *device, if (NM_IN_SET (new_state, NM_DEVICE_STATE_UNMANAGED, NM_DEVICE_STATE_DISCONNECTED, - NM_DEVICE_STATE_ACTIVATED)) + NM_DEVICE_STATE_ACTIVATED)) { nm_manager_write_device_state (self, device, NULL); + G_STATIC_ASSERT_EXPR (DEVICE_STATE_PRUNE_RATELIMIT_MAX < G_MAXUINT8); + if (priv->device_state_prune_ratelimit_count++ > DEVICE_STATE_PRUNE_RATELIMIT_MAX) { + /* We write the device state to /run. The state files are named after the + * ifindex (which is assumed to be unique and not repeat -- in practice + * it may repeat). So from time to time, we prune device state files + * for interfaces that no longer exist. + * + * Otherwise, the files might pile up if you create (and destroy) a large + * number of software devices. */ + priv->device_state_prune_ratelimit_count = 0; + nm_config_device_state_prune_stale (NULL, priv->platform); + } + } + if (NM_IN_SET (new_state, NM_DEVICE_STATE_UNAVAILABLE, NM_DEVICE_STATE_DISCONNECTED))