dns: move rate limiting to NMDnsPlugin

When we'll have multiple active DNS plugins the rate limiting needs to be
per-plugin.

It may actually make sense to move this into the Dnsmasq plugin, as it's the
only one which makes use of the facility and it will probably stay that way.

4 files changed, 67 insertions, 66 deletions

diff --git a/src/dns/nm-dns-dnsmasq.c b/src/dns/nm-dns-dnsmasq.c
index 3c05374f18..df3ca11120 100644
--- a/src/dns/nm-dns-dnsmasq.c
+++ b/src/dns/nm-dns-dnsmasq.c
@@ -589,7 +589,7 @@ update (NMDnsPlugin *plugin,
 
 /*****************************************************************************/
 
-static void
+static gboolean
 child_quit (NMDnsPlugin *plugin, gint status)
 {
 	NMDnsDnsmasq *self = NM_DNS_DNSMASQ (plugin);
@@ -615,12 +615,9 @@ child_quit (NMDnsPlugin *plugin, gint status)
 
 	priv->running = FALSE;
 
-	if (failed)
-		g_signal_emit_by_name (self, NM_DNS_PLUGIN_FAILED);
+	return failed;
 }
 
-/*****************************************************************************/
-
 static const char *
 get_name (NMDnsPlugin *plugin)
 {
diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c
index cef65bd5de..f5776d13a7 100644
--- a/src/dns/nm-dns-manager.c
+++ b/src/dns/nm-dns-manager.c
@@ -66,10 +66,6 @@
 #define NETCONFIG_PATH "/sbin/netconfig"
 #endif
 
-#define PLUGIN_RATELIMIT_INTERVAL    30
-#define PLUGIN_RATELIMIT_BURST       5
-#define PLUGIN_RATELIMIT_DELAY       300
-
 enum {
 	CONFIG_CHANGED,
 
@@ -136,12 +132,6 @@ typedef struct {
 	NMDnsPlugin *plugin;
 
 	NMConfig *config;
-
-	struct {
-		guint64 ts;
-		guint num_restarts;
-		guint timer;
-	} plugin_ratelimit;
 } NMDnsManagerPrivate;
 
 struct _NMDnsManager {
@@ -1106,8 +1096,6 @@ update_dns (NMDnsManager *self,
 		return TRUE;
 	}
 
-	nm_clear_g_source (&priv->plugin_ratelimit.timer);
-
 	if (NM_IN_SET (priv->rc_manager, NM_DNS_MANAGER_RESOLV_CONF_MAN_UNMANAGED,
 	                                 NM_DNS_MANAGER_RESOLV_CONF_MAN_IMMUTABLE)) {
 		update = FALSE;
@@ -1228,47 +1216,17 @@ plugin_failed (NMDnsPlugin *plugin, gpointer user_data)
 	}
 }
 
-static gboolean
-plugin_child_quit_update_dns (gpointer user_data)
+static void
+plugin_child_quit (NMDnsPlugin *plugin, gpointer user_data)
 {
-	GError *error = NULL;
 	NMDnsManager *self = NM_DNS_MANAGER (user_data);
+	GError *error = NULL;
 
 	/* Let the plugin try to spawn the child again */
 	if (!update_dns (self, FALSE, &error)) {
 		_LOGW ("could not commit DNS changes: %s", error->message);
 		g_clear_error (&error);
 	}
-
-	return G_SOURCE_REMOVE;
-}
-
-static void
-plugin_child_quit (NMDnsPlugin *plugin, int exit_status, gpointer user_data)
-{
-	NMDnsManager *self = NM_DNS_MANAGER (user_data);
-	NMDnsManagerPrivate *priv = NM_DNS_MANAGER_GET_PRIVATE (self);
-	gint64 ts = nm_utils_get_monotonic_timestamp_ms ();
-
-	_LOGW ("plugin %s child quit unexpectedly", nm_dns_plugin_get_name (plugin));
-
-	if (   !priv->plugin_ratelimit.ts
-	    || (ts - priv->plugin_ratelimit.ts) / 1000 > PLUGIN_RATELIMIT_INTERVAL) {
-		priv->plugin_ratelimit.ts = ts;
-		priv->plugin_ratelimit.num_restarts = 0;
-	} else {
-		priv->plugin_ratelimit.num_restarts++;
-		if (priv->plugin_ratelimit.num_restarts > PLUGIN_RATELIMIT_BURST) {
-			_LOGW ("plugin %s child respawning too fast, delaying update for %u seconds",
-			        nm_dns_plugin_get_name (plugin), PLUGIN_RATELIMIT_DELAY);
-			priv->plugin_ratelimit.timer = g_timeout_add_seconds (PLUGIN_RATELIMIT_DELAY,
-			                                                      plugin_child_quit_update_dns,
-			                                                      self);
-			return;
-		}
-	}
-
-	plugin_child_quit_update_dns (self);
 }
 
 static void
@@ -1572,8 +1530,6 @@ _clear_plugin (NMDnsManager *self)
 		g_clear_object (&priv->plugin);
 		return TRUE;
 	}
-	priv->plugin_ratelimit.ts = 0;
-	nm_clear_g_source (&priv->plugin_ratelimit.timer);
 	return FALSE;
 }
 
@@ -2099,8 +2055,6 @@ dispose (GObject *object)
 		priv->configs = NULL;
 	}
 
-	nm_clear_g_source (&priv->plugin_ratelimit.timer);
-
 	G_OBJECT_CLASS (nm_dns_manager_parent_class)->dispose (object);
 }
 
diff --git a/src/dns/nm-dns-plugin.c b/src/dns/nm-dns-plugin.c
index 2f370c9a1f..6d65913d51 100644
--- a/src/dns/nm-dns-plugin.c
+++ b/src/dns/nm-dns-plugin.c
@@ -32,6 +32,10 @@
 
 /*****************************************************************************/
 
+#define PLUGIN_RATELIMIT_INTERVAL    30
+#define PLUGIN_RATELIMIT_BURST       5
+#define PLUGIN_RATELIMIT_DELAY       300
+
 enum {
 	FAILED,
 	CHILD_QUIT,
@@ -45,6 +49,12 @@ typedef struct _NMDnsPluginPrivate {
 	guint watch_id;
 	char *progname;
 	char *pidfile;
+
+	struct {
+		guint64 ts;
+		guint num_restarts;
+		guint timer;
+	} plugin_ratelimit;
 } NMDnsPluginPrivate;
 
 G_DEFINE_TYPE_EXTENDED (NMDnsPlugin, nm_dns_plugin, G_TYPE_OBJECT, G_TYPE_FLAG_ABSTRACT, {})
@@ -81,8 +91,12 @@ nm_dns_plugin_update (NMDnsPlugin *self,
                       const NMGlobalDnsConfig *global_config,
                       const char *hostname)
 {
+	NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
+
 	g_return_val_if_fail (NM_DNS_PLUGIN_GET_CLASS (self)->update != NULL, FALSE);
 
+	nm_clear_g_source (&priv->plugin_ratelimit.timer);
+
 	return NM_DNS_PLUGIN_GET_CLASS (self)->update (self,
 	                                               configs,
 	                                               global_config,
@@ -154,20 +168,58 @@ out:
 	unlink (pidfile);
 }
 
+static gboolean
+emit_ratelimited_child_quit (gpointer user_data)
+{
+	NMDnsPlugin *self = NM_DNS_PLUGIN (user_data);
+
+	g_signal_emit (self, signals[CHILD_QUIT], 0);
+
+	return G_SOURCE_REMOVE;
+}
+
 static void
 watch_cb (GPid pid, gint status, gpointer user_data)
 {
 	NMDnsPlugin *self = NM_DNS_PLUGIN (user_data);
 	NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
+	NMDnsPluginClass *klass = NM_DNS_PLUGIN_GET_CLASS (self);
+	gint64 ts = nm_utils_get_monotonic_timestamp_ms ();
+	gboolean failed = FALSE;
 
 	priv->pid = 0;
 	priv->watch_id = 0;
 	g_clear_pointer (&priv->progname, g_free);
 	_clear_pidfile (self);
 
-	g_signal_emit (self, signals[CHILD_QUIT], 0, status);
+	if (klass->child_quit)
+		failed = klass->child_quit (self, status);
+
+	if (   !priv->plugin_ratelimit.ts
+	    || (ts - priv->plugin_ratelimit.ts) / 1000 > PLUGIN_RATELIMIT_INTERVAL
+	    || failed) {
+	        priv->plugin_ratelimit.ts = ts;
+	        priv->plugin_ratelimit.num_restarts = 0;
+	} else {
+		priv->plugin_ratelimit.num_restarts++;
+		if (priv->plugin_ratelimit.num_restarts > PLUGIN_RATELIMIT_BURST) {
+			_LOGW ("plugin %s child respawning too fast, delaying update for %u seconds",
+			       nm_dns_plugin_get_name (self), PLUGIN_RATELIMIT_DELAY);
+			priv->plugin_ratelimit.timer = g_timeout_add_seconds (PLUGIN_RATELIMIT_DELAY,
+			                                                      emit_ratelimited_child_quit,
+			                                                      self);
+			return;
+		}
+	}
+
+	if (failed)
+		g_signal_emit (self, signals[FAILED], 0);
+	else
+		g_signal_emit (self, signals[CHILD_QUIT], 0);
 }
 
+/*****************************************************************************/
+
 GPid
 nm_dns_plugin_child_pid (NMDnsPlugin *self)
 {
@@ -233,6 +285,8 @@ nm_dns_plugin_child_kill (NMDnsPlugin *self)
 {
 	NMDnsPluginPrivate *priv = NM_DNS_PLUGIN_GET_PRIVATE (self);
 
+	priv->plugin_ratelimit.ts = 0;
+	nm_clear_g_source (&priv->plugin_ratelimit.timer);
 	nm_clear_g_source (&priv->watch_id);
 	if (priv->pid) {
 		nm_utils_kill_child_sync (priv->pid, SIGTERM, _NMLOG_DOMAIN,
@@ -295,8 +349,7 @@ nm_dns_plugin_class_init (NMDnsPluginClass *plugin_class)
 	    g_signal_new (NM_DNS_PLUGIN_CHILD_QUIT,
 	                  G_OBJECT_CLASS_TYPE (object_class),
 	                  G_SIGNAL_RUN_FIRST,
-	                  G_STRUCT_OFFSET (NMDnsPluginClass, child_quit),
-	                  NULL, NULL,
-	                  g_cclosure_marshal_VOID__INT,
-	                  G_TYPE_NONE, 1, G_TYPE_INT);
+	                  0, NULL, NULL,
+	                  g_cclosure_marshal_VOID__VOID,
+	                  G_TYPE_NONE, 0);
 }
diff --git a/src/dns/nm-dns-plugin.h b/src/dns/nm-dns-plugin.h
index adf3f1374c..c37c657e01 100644
--- a/src/dns/nm-dns-plugin.h
+++ b/src/dns/nm-dns-plugin.h
@@ -57,14 +57,11 @@ typedef struct {
 	/* Subclasses should override this and return their plugin name */
 	const char *(*get_name) (NMDnsPlugin *self);
 
-	/* Signals */
-
-	/* Emitted by the plugin base class when the nameserver subprocess
-	 * quits.  This signal is consumed by the plugin subclasses and not
-	 * by NMDnsManager.  If the subclass decides the exit status (as returned
-	 * by waitpid(2)) is fatal it should then emit the 'failed' signal.
+	/* Lets the subclass know when the nameserver subprocess
+	 * quits. If the subclass decides the exit status (as returned
+	 * by waitpid(2)) is fatal it returns TRUE.
 	 */
-	void (*child_quit) (NMDnsPlugin *self, gint status);
+	gboolean (*child_quit) (NMDnsPlugin *self, gint status);
 } NMDnsPluginClass;
 
 GType nm_dns_plugin_get_type (void);