From 0a22ce64a6a8c2ec0f8a71f1a59855593cdeb96a Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 13 Jan 2015 15:08:21 -0500 Subject: [PATCH 1/4] connectivity: improve debug logging nm-connectivity was logging both "started" and "finished" for periodic connectivity checks, but was only logging "finished" for manual ones, which made the logs look weird. Fix it to log both periodic and manual starts, and differentiate them. Also add some additional logging to indicate when set_online() is called, and when :state changes. (cherry picked from commit 53f2642c736ebb8466cbfd29c2ede2c3828a4728) --- src/nm-connectivity.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index 8a324a5c16..5913fd96c0 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -69,12 +69,33 @@ nm_connectivity_get_state (NMConnectivity *connectivity) return NM_CONNECTIVITY_GET_PRIVATE (connectivity)->state; } +static const char * +state_name (NMConnectivityState state) +{ + switch (state) { + case NM_CONNECTIVITY_UNKNOWN: + return "UNKNOWN"; + case NM_CONNECTIVITY_NONE: + return "NONE"; + case NM_CONNECTIVITY_LIMITED: + return "LIMITED"; + case NM_CONNECTIVITY_PORTAL: + return "PORTAL"; + case NM_CONNECTIVITY_FULL: + return "FULL"; + default: + return "???"; + } +} + static void update_state (NMConnectivity *self, NMConnectivityState state) { NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); if (priv->state != state) { + nm_log_dbg (LOGD_CONCHECK, "Connectivity state changed from %s to %s", + state_name (priv->state), state_name (state)); priv->state = state; g_object_notify (G_OBJECT (self), NM_CONNECTIVITY_STATE); } @@ -155,7 +176,6 @@ run_check (gpointer user_data) nm_connectivity_check_async (self, run_check_complete, NULL); priv->running = TRUE; - nm_log_dbg (LOGD_CONCHECK, "Connectivity check with uri '%s' started.", priv->uri); return TRUE; } @@ -167,7 +187,11 @@ nm_connectivity_set_online (NMConnectivity *self, { #if WITH_CONCHECK NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); +#endif + nm_log_dbg (LOGD_CONCHECK, "nm_connectivity_set_online(%s)", online ? "TRUE" : "FALSE"); + +#if WITH_CONCHECK if (online && priv->uri && priv->interval) { if (!priv->check_id) priv->check_id = g_timeout_add_seconds (priv->interval, run_check, self); @@ -201,6 +225,11 @@ nm_connectivity_check_async (NMConnectivity *self, g_return_if_fail (NM_IS_CONNECTIVITY (self)); priv = NM_CONNECTIVITY_GET_PRIVATE (self); + if (callback == run_check_complete) + nm_log_dbg (LOGD_CONCHECK, "Periodic connectivity check started with uri '%s'.", priv->uri); + else + nm_log_dbg (LOGD_CONCHECK, "Connectivity check started with uri '%s'.", priv->uri); + simple = g_simple_async_result_new (G_OBJECT (self), callback, user_data, nm_connectivity_check_async); From e042bb09a56511ffc6040f8e125863ebb5623974 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 13 Jan 2015 14:48:29 -0500 Subject: [PATCH 2/4] connectivity: fix manager state during connectivity check (bgo #742823) When a connection has finished activating, but we don't know yet that we have full connectivity, then find_best_device_state() should return CONNECTED_SITE, not CONNECTING. Fixes a bug where the manager state would repeatedly switch between those two states. (cherry picked from commit 5e182d55777b95886d39068821d1d6fa8298474d) --- src/nm-manager.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 65fb5682c4..fb4e0165ea 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -578,7 +578,7 @@ checked_connectivity (GObject *object, GAsyncResult *result, gpointer user_data) } static NMState -find_best_device_state (NMManager *manager, gboolean *want_connectivity_check) +find_best_device_state (NMManager *manager) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); NMState best_state = NM_STATE_DISCONNECTED; @@ -593,13 +593,10 @@ find_best_device_state (NMManager *manager, gboolean *want_connectivity_check) if ( nm_active_connection_get_default (ac) || nm_active_connection_get_default6 (ac)) { nm_connectivity_set_online (priv->connectivity, TRUE); - if (nm_connectivity_get_state (priv->connectivity) == NM_CONNECTIVITY_FULL) { - *want_connectivity_check = FALSE; + if (nm_connectivity_get_state (priv->connectivity) == NM_CONNECTIVITY_FULL) return NM_STATE_CONNECTED_GLOBAL; - } - best_state = NM_STATE_CONNECTING; - *want_connectivity_check = TRUE; + best_state = NM_STATE_CONNECTED_SITE; } else { if (best_state < NM_STATE_CONNECTING) best_state = NM_STATE_CONNECTED_LOCAL; @@ -630,7 +627,6 @@ nm_manager_update_state (NMManager *manager) { NMManagerPrivate *priv; NMState new_state = NM_STATE_DISCONNECTED; - gboolean want_connectivity_check = FALSE; g_return_if_fail (NM_IS_MANAGER (manager)); @@ -639,9 +635,9 @@ nm_manager_update_state (NMManager *manager) if (manager_sleeping (manager)) new_state = NM_STATE_ASLEEP; else - new_state = find_best_device_state (manager, &want_connectivity_check); + new_state = find_best_device_state (manager); - if (new_state == NM_STATE_CONNECTING && want_connectivity_check) { + if (new_state == NM_STATE_CONNECTED_SITE) { nm_connectivity_check_async (priv->connectivity, checked_connectivity, g_object_ref (manager)); From 57249a2d0513a51e7446b7058f9e24e69cabbf6c Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 13 Jan 2015 15:35:10 -0500 Subject: [PATCH 3/4] connectivity: simplify redundant code Merge the two nm_connectivity_set_online() calls into one, after tweaking NMConnectivity to always update its internal state before alerting callers to the new state. (cherry picked from commit 0997c4b245ca5638d0d27eee90146dc47c56130d) --- src/nm-connectivity.c | 4 ++-- src/nm-manager.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index 5913fd96c0..cf4552a8d7 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -145,10 +145,10 @@ nm_connectivity_check_cb (SoupSession *session, SoupMessage *msg, gpointer user_ } done: + update_state (self, new_state); + g_simple_async_result_set_op_res_gssize (simple, new_state); g_simple_async_result_complete (simple); - - update_state (self, new_state); } static void diff --git a/src/nm-manager.c b/src/nm-manager.c index fb4e0165ea..62a1e154a6 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -592,7 +592,6 @@ find_best_device_state (NMManager *manager) case NM_ACTIVE_CONNECTION_STATE_ACTIVATED: if ( nm_active_connection_get_default (ac) || nm_active_connection_get_default6 (ac)) { - nm_connectivity_set_online (priv->connectivity, TRUE); if (nm_connectivity_get_state (priv->connectivity) == NM_CONNECTIVITY_FULL) return NM_STATE_CONNECTED_GLOBAL; @@ -637,12 +636,13 @@ nm_manager_update_state (NMManager *manager) else new_state = find_best_device_state (manager); + nm_connectivity_set_online (priv->connectivity, new_state >= NM_STATE_CONNECTED_LOCAL); + if (new_state == NM_STATE_CONNECTED_SITE) { nm_connectivity_check_async (priv->connectivity, checked_connectivity, g_object_ref (manager)); - } else - nm_connectivity_set_online (priv->connectivity, new_state >= NM_STATE_CONNECTED_LOCAL); + } set_state (manager, new_state); } From 3d49585b377a578ce0195d74c5b5125a1101c688 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 13 Jan 2015 15:25:31 -0500 Subject: [PATCH 4/4] connectivity: avoid redundant connectivity checks Don't start an automatic connectivity check right when NMManager tells us we're online; only do it if the manager doesn't request an explicit connectivity check immediately afterward. (cherry picked from commit 66b8f2b7a0f9e1ce84d8a98863c88ea3dbed0a51) --- src/nm-connectivity.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/nm-connectivity.c b/src/nm-connectivity.c index cf4552a8d7..1a374ebe41 100644 --- a/src/nm-connectivity.c +++ b/src/nm-connectivity.c @@ -44,7 +44,7 @@ typedef struct { #if WITH_CONCHECK SoupSession *soup_session; - gboolean running; + guint pending_checks; guint check_id; #endif @@ -114,6 +114,7 @@ nm_connectivity_check_cb (SoupSession *session, SoupMessage *msg, gpointer user_ self = NM_CONNECTIVITY (g_async_result_get_source_object (G_ASYNC_RESULT (simple))); g_object_unref (self); priv = NM_CONNECTIVITY_GET_PRIVATE (self); + priv->pending_checks--; if (SOUP_STATUS_IS_TRANSPORT_ERROR (msg->status_code)) { nm_log_info (LOGD_CONCHECK, "Connectivity check for uri '%s' failed with '%s'.", @@ -157,11 +158,9 @@ run_check_complete (GObject *object, gpointer user_data) { NMConnectivity *self = NM_CONNECTIVITY (object); - NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); GError *error = NULL; nm_connectivity_check_finish (self, result, &error); - priv->running = FALSE; if (error) { nm_log_err (LOGD_CONCHECK, "Connectivity check failed: %s", error->message); g_error_free (error); @@ -172,13 +171,23 @@ static gboolean run_check (gpointer user_data) { NMConnectivity *self = NM_CONNECTIVITY (user_data); - NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); nm_connectivity_check_async (self, run_check_complete, NULL); - priv->running = TRUE; - return TRUE; } + +static gboolean +idle_start_periodic_checks (gpointer user_data) +{ + NMConnectivity *self = user_data; + NMConnectivityPrivate *priv = NM_CONNECTIVITY_GET_PRIVATE (self); + + priv->check_id = g_timeout_add_seconds (priv->interval, run_check, self); + if (!priv->pending_checks) + run_check (self); + + return FALSE; +} #endif void @@ -194,9 +203,7 @@ nm_connectivity_set_online (NMConnectivity *self, #if WITH_CONCHECK if (online && priv->uri && priv->interval) { if (!priv->check_id) - priv->check_id = g_timeout_add_seconds (priv->interval, run_check, self); - if (!priv->running) - run_check (self); + priv->check_id = g_timeout_add (0, idle_start_periodic_checks, self); return; } else if (priv->check_id) { @@ -241,6 +248,7 @@ nm_connectivity_check_async (NMConnectivity *self, msg, nm_connectivity_check_cb, simple); + priv->pending_checks++; return; }