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

git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@4334 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

diff --git a/ChangeLog b/ChangeLog
index b91afeb..46211c7 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+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
+
 2008-11-23  Dan Williams  <dcbw@redhat.com>
 
        * callouts/nm-dispatcher-action.c

diff --git a/src/NetworkManager.c b/src/NetworkManager.c
index 2899f78..5ac2e95 100644 (file)
--- a/src/NetworkManager.c
+++ b/src/NetworkManager.c
@@ -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 @@ nm_monitor_setup (void)
 {
        GError *error = NULL;
        NMNetlinkMonitor *monitor;
+       MonitorInfo *info;
 
        monitor = nm_netlink_monitor_get ();
        nm_netlink_monitor_open_connection (monitor, &error);
@@ -87,11 +125,14 @@ nm_monitor_setup (void)
                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);

diff --git a/src/nm-netlink-monitor.c b/src/nm-netlink-monitor.c
index 3041e98..11acb1c 100644 (file)
--- a/src/nm-netlink-monitor.c
+++ b/src/nm-netlink-monitor.c
@@ -65,16 +65,15 @@ typedef struct {
        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 @@ nm_netlink_monitor_close_connection (NMNetlinkMonitor  *monitor)
        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 @@ nm_netlink_monitor_error_quark (void)
        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 @@ nm_netlink_monitor_detach (NMNetlinkMonitor *monitor)
        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 @@ nm_netlink_monitor_request_status (NMNetlinkMonitor  *monitor,
        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 @@ nm_netlink_monitor_request_status (NMNetlinkMonitor  *monitor,
 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 @@ nm_netlink_monitor_error_handler (GIOChannel       *channel,
                                   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;
 }
 

diff --git a/src/nm-netlink-monitor.h b/src/nm-netlink-monitor.h
index c557631..0cb4bea 100644 (file)
--- a/src/nm-netlink-monitor.h
+++ b/src/nm-netlink-monitor.h
@@ -70,8 +70,7 @@ NMNetlinkMonitor *nm_netlink_monitor_get (void);
 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);