core: merge branch 'th/prune-device-state-files'

https://bugzilla.redhat.com/show_bug.cgi?id=1810153

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/429

4 files changed, 70 insertions, 33 deletions

diff --git a/src/nm-config.c b/src/nm-config.c
index 4b4ffd3122..558dab7aec 100644
--- a/src/nm-config.c
+++ b/src/nm-config.c
@@ -2299,6 +2299,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
@@ -2310,7 +2312,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;
 
@@ -2394,7 +2396,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;
 
@@ -2477,35 +2479,43 @@ nm_config_device_state_write (int ifindex,
 }
 
 void
-nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes)
+nm_config_device_state_prune_stale (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;
+
+		if (   preserve_in_platform
+		    && nm_platform_link_get (preserve_in_platform, ifindex))
 			continue;
 
-		fn_len = strlen (fn) + 1;
+		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..b4478ceb04 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_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 e49c739a10..ee1feb941b 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;
 
@@ -1515,8 +1519,22 @@ 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_manager_write_device_state (self, device);
+	               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,
@@ -6484,7 +6502,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;
@@ -6500,6 +6518,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;
@@ -6540,34 +6560,40 @@ nm_manager_write_device_state (NMManager *self, NMDevice *device)
 		next_server = nm_dhcp_config_get_option (dhcp_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
 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,
-			                  GINT_TO_POINTER (nm_device_get_ip_ifindex (device)));
+		int ifindex;
+
+		if (nm_manager_write_device_state (self, device, &ifindex)) {
+			g_hash_table_add (preserve_ifindexes,
+			                  GINT_TO_POINTER (ifindex));
 		}
 	}
 
-	nm_config_device_state_prune_unseen (seen_ifindexes);
+	nm_config_device_state_prune_stale (preserve_ifindexes, NULL);
 }
 
 static gboolean
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 */