From 457fae22aea770e2bf6553630caba645b6e1796d Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

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)

(cherry picked from commit 0b4ebda859a3dc1c8ec4d8c887383d04bcd249f9)

---

 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 a0995cf932b8..0fccca4e22dc 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;

+

+		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 d9460ebb46e3..048d64f41f47 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 7f6657fd70e2..35127fa6cd25 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

-- 

2.24.1

From af10b35dabc0b542eba8117bb32b3fee3ff207f9 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

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)

(cherry picked from commit 7fa1e825450ce4b69670b5bfd5a9c705d9c05920)

---

 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 0fccca4e22dc..0a6082d71a63 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 048d64f41f47..b4478ceb04af 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 35127fa6cd25..c9ef20a526d2 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

-- 

2.24.1

From 509b555a45b170f843242e3d92b229936457ba75 Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

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)

(cherry picked from commit fb6e14cf3f602a840bee66c27dfcb754872f1525)

---

 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 c9ef20a526d2..74776a24acf2 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 ad06e318f4dc..5873abd24b8e 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 */

-- 

2.24.1

From d60b888a07d69e155a7bea9dead1376df1a21bfb Mon Sep 17 00:00:00 2001

From: Thomas Haller <thaller@redhat.com>

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)

(cherry picked from commit d65b5c2e81f69ddc9217159a16f49d23d965da1c)

---

 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 74776a24acf2..931d1068f160 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))

-- 

2.24.1