NetworkManager r4334 - in trunk: . src

[dcbw svn gnome org]

Author: dcbw
Date: Tue Nov 25 18:30:44 2008
New Revision: 4334
URL: http://svn.gnome.org/viewvc/NetworkManager?rev=4334&view=rev

Log:
2008-11-25  Dan Williams  <dcbw redhat com>

	Patch from Tambet Ingo <tambet gmail com>

	Fix mishandling of netlink error floods (rh #459205, novell #443429, lp #284507)

	* src/nm-netlink-monitor.c
		- Remove bits for using a non-default GMainContext, which weren't used
		- (nm_netlink_monitor_error_handler): don't leak the GError, and report
			the actual error code

	* src/NetworkManager.c
		- (nm_error_monitoring_device_link_state): disconnect error handler when
			an error flood occurs to avoid pegging the CPU



Modified:
   trunk/ChangeLog
   trunk/src/NetworkManager.c
   trunk/src/nm-netlink-monitor.c
   trunk/src/nm-netlink-monitor.h

Modified: trunk/src/NetworkManager.c
==============================================================================
--- trunk/src/NetworkManager.c	(original)
+++ trunk/src/NetworkManager.c	Tue Nov 25 18:30:44 2008
@@ -60,14 +60,51 @@
 static NMManager *manager = NULL;
 static GMainLoop *main_loop = NULL;
 
+typedef struct {
+	time_t time;
+	GQuark domain;
+	guint32 code;
+	guint32 count;
+} MonitorInfo;
+
+static gboolean
+detach_monitor (gpointer data)
+{
+	nm_info ("Detaching netlink event monitor");
+	nm_netlink_monitor_detach (NM_NETLINK_MONITOR (data));
+	return FALSE;
+}
+
 static void
 nm_error_monitoring_device_link_state (NMNetlinkMonitor *monitor,
 									   GError *error,
 									   gpointer user_data)
 {
-	/* FIXME: Try to handle the error instead of just printing it. */
-	nm_warning ("error monitoring wired ethernet link state: %s\n",
-				error->message);
+	MonitorInfo *info = (MonitorInfo *) user_data;
+	time_t now;
+
+	now = time (NULL);
+
+	if (info->domain != error->domain || info->code != error->code || (info->time && now > info->time + 10)) {
+		/* FIXME: Try to handle the error instead of just printing it. */
+		nm_warning ("error monitoring device for netlink events: %s\n",
+					error->message);
+
+		info->time = now;
+		info->domain = error->domain;
+		info->code = error->code;
+		info->count = 0;
+	}
+
+	info->count++;
+	if (info->count > 100) {
+		/* Broken drivers will sometimes cause a flood of netlink errors.
+		 * rh #459205, novell #443429, lp #284507
+		 */
+		nm_warning ("Excessive netlink errors ocurred, disabling netlink monitor.");
+		nm_warning ("Link change events will not be processed.");
+		g_idle_add_full (G_PRIORITY_HIGH, detach_monitor, monitor, NULL);
+	}
 }
 
 static gboolean
@@ -75,6 +112,7 @@
 {
 	GError *error = NULL;
 	NMNetlinkMonitor *monitor;
+	MonitorInfo *info;
 
 	monitor = nm_netlink_monitor_get ();
 	nm_netlink_monitor_open_connection (monitor, &error);
@@ -87,11 +125,14 @@
 		return FALSE;
 	}
 
-	g_signal_connect (G_OBJECT (monitor), "error",
-			  G_CALLBACK (nm_error_monitoring_device_link_state),
-			  NULL);
+	info = g_new0 (MonitorInfo, 1);
+	g_signal_connect_data (G_OBJECT (monitor), "error",
+						   G_CALLBACK (nm_error_monitoring_device_link_state),
+						   info,
+						   (GClosureNotify) g_free,
+						   0);
 
-	nm_netlink_monitor_attach (monitor, NULL);
+	nm_netlink_monitor_attach (monitor);
 
 	/* Request initial status of cards */
 	nm_netlink_monitor_request_status (monitor, NULL);

Modified: trunk/src/nm-netlink-monitor.c
==============================================================================
--- trunk/src/nm-netlink-monitor.c	(original)
+++ trunk/src/nm-netlink-monitor.c	Tue Nov 25 18:30:44 2008
@@ -65,16 +65,15 @@
 	struct nl_cb *    nlh_cb;
 	struct nl_cache * nlh_link_cache;
 
-	GMainContext *	  context;
 	GIOChannel *	  io_channel;
-	GSource *		  event_source;
+	guint             event_id;
 
 	guint request_status_id;
 } NMNetlinkMonitorPrivate;
 
 static gboolean nm_netlink_monitor_event_handler (GIOChannel       *channel,
                                                   GIOCondition      io_condition,
-                                                  NMNetlinkMonitor *monitor);
+                                                  gpointer          user_data);
 
 static gboolean nm_netlink_monitor_error_handler (GIOChannel       *channel,
                                                   GIOCondition      io_condition,
@@ -333,7 +332,7 @@
 	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
 	g_return_if_fail (priv->io_channel != NULL);
 
-	if (priv->event_source != NULL)
+	if (priv->event_id)
 		nm_netlink_monitor_detach (monitor);
 
 	g_io_channel_shutdown (priv->io_channel,
@@ -355,42 +354,23 @@
 	return error_quark;
 }
 
-static void
-nm_netlink_monitor_clear_event_source (NMNetlinkMonitor *monitor)
-{
-	NM_NETLINK_MONITOR_GET_PRIVATE (monitor)->event_source = NULL;
-}
-
 void
-nm_netlink_monitor_attach (NMNetlinkMonitor *monitor, 
-                           GMainContext     *context)
+nm_netlink_monitor_attach (NMNetlinkMonitor *monitor)
 {
 	NMNetlinkMonitorPrivate *priv;
-	GSource *event_source;
 
 	g_return_if_fail (NM_IS_NETLINK_MONITOR (monitor));
 
 	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
 	g_return_if_fail (priv->nlh != NULL);
+	g_return_if_fail (priv->event_id == 0);
 
-	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
-	g_return_if_fail (priv->context == NULL);
-
-	if (context == NULL)
-		context = g_main_context_default ();
-
-	priv->context = g_main_context_ref (context);
-
-	event_source = g_io_create_watch (priv->io_channel,
-	                                  NM_NETLINK_MONITOR_EVENT_CONDITIONS |
+	priv->event_id = g_io_add_watch (priv->io_channel,
+	                                 (NM_NETLINK_MONITOR_EVENT_CONDITIONS |
 	                                  NM_NETLINK_MONITOR_ERROR_CONDITIONS |
-	                                  NM_NETLINK_MONITOR_DISCONNECT_CONDITIONS);
-	g_source_set_callback (event_source, 
-	                       (GSourceFunc) nm_netlink_monitor_event_handler,
-	                       monitor, 
-	                       (GDestroyNotify) nm_netlink_monitor_clear_event_source);
-	g_source_attach (event_source, context);
-	priv->event_source = event_source;
+	                                  NM_NETLINK_MONITOR_DISCONNECT_CONDITIONS),
+	                                 nm_netlink_monitor_event_handler,
+	                                 monitor);
 }
 
 void
@@ -401,13 +381,10 @@
 	g_return_if_fail (NM_IS_NETLINK_MONITOR (monitor));
 
 	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
-	g_return_if_fail (priv->context != NULL);
-
-	g_source_destroy (priv->event_source);
-	priv->event_source = NULL;
+	g_return_if_fail (priv->event_id > 0);
 
-	g_main_context_unref (priv->context);
-	priv->context = NULL;
+	g_source_remove (priv->event_id);
+	priv->event_id = 0;
 }
 
 static gboolean
@@ -432,7 +409,7 @@
 	g_return_val_if_fail (NM_IS_NETLINK_MONITOR (monitor), FALSE);
 
 	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
-	g_return_val_if_fail (priv->context != NULL, FALSE);
+	g_return_val_if_fail (priv->event_id > 0, FALSE);
 
 	/* Update the link cache with latest state */
 	if (nl_cache_refill (priv->nlh, priv->nlh_link_cache)) {
@@ -453,15 +430,16 @@
 static gboolean
 nm_netlink_monitor_event_handler (GIOChannel       *channel,
                                   GIOCondition      io_condition,
-                                  NMNetlinkMonitor *monitor)
+                                  gpointer          user_data)
 {
+	NMNetlinkMonitor *monitor = (NMNetlinkMonitor *) user_data;
 	NMNetlinkMonitorPrivate *priv;
 	GError *error = NULL;
 
 	g_return_val_if_fail (NM_IS_NETLINK_MONITOR (monitor), TRUE);
 
 	priv = NM_NETLINK_MONITOR_GET_PRIVATE (monitor);
-	g_return_val_if_fail (priv->context != NULL, TRUE);
+	g_return_val_if_fail (priv->event_id > 0, TRUE);
 
 	if (io_condition & NM_NETLINK_MONITOR_ERROR_CONDITIONS)
 		return nm_netlink_monitor_error_handler (channel, io_condition, monitor);
@@ -491,16 +469,30 @@
                                   NMNetlinkMonitor *monitor)
 {
 	GError *socket_error;
+	const char *err_msg;
+	int err_code;
+	socklen_t err_len;
  
 	g_return_val_if_fail (io_condition & NM_NETLINK_MONITOR_ERROR_CONDITIONS, FALSE);
 
+	err_code = 0;
+	err_len = sizeof (err_code);
+	if (getsockopt (g_io_channel_unix_get_fd (channel), 
+					SOL_SOCKET, SO_ERROR, (void *) &err_code, &err_len))
+		err_msg = strerror (err_code);
+	else
+		err_msg = _("error occurred while waiting for data on socket");
+
 	socket_error = g_error_new (NM_NETLINK_MONITOR_ERROR,
 	                            NM_NETLINK_MONITOR_ERROR_WAITING_FOR_SOCKET_DATA,
-	                            _("error occurred while waiting for data on socket"));
+	                            err_msg);
 
 	g_signal_emit (G_OBJECT (monitor), 
 	               signals[ERROR],
 	               0, socket_error);
+
+	g_error_free (socket_error);
+
 	return TRUE;
 }
 

Modified: trunk/src/nm-netlink-monitor.h
==============================================================================
--- trunk/src/nm-netlink-monitor.h	(original)
+++ trunk/src/nm-netlink-monitor.h	Tue Nov 25 18:30:44 2008
@@ -70,8 +70,7 @@
 gboolean          nm_netlink_monitor_open_connection (NMNetlinkMonitor *monitor,
 													  GError **error);
 void              nm_netlink_monitor_close_connection (NMNetlinkMonitor *monitor);
-void              nm_netlink_monitor_attach	          (NMNetlinkMonitor	*monitor,
-													   GMainContext *context);
+void              nm_netlink_monitor_attach	          (NMNetlinkMonitor	*monitor);
 void              nm_netlink_monitor_detach	          (NMNetlinkMonitor *monitor);
 gboolean          nm_netlink_monitor_request_status   (NMNetlinkMonitor *monitor,
 													   GError **error);