From a6e366caea8654151fc482d4186e2a729a6de141 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 30 Apr 2013 17:29:40 +0100 Subject: [PATCH 4/4] Replace ConnMan connectivity support with GNetworkMonitor The information we're getting from ConnMan is no more informative than GNetworkMonitor, and a ConnMan GNetworkMonitor implementation exists (e.g. ), so we might as well go via that. On NetworkManager systems, we now use NetworkManager to detect transient states like DISCONNECTING, but use the GNetworkMonitor to detect whether the interface is up. If NetworkManager is the only thing managing an interface, that makes no difference, but if there is an interface not managed by NetworkManager, the default netlink GNetworkMonitor will say we're online even while NM is DISCONNECTED, which is what we want. On non-NetworkManager, non-ConnMan systems where netlink works, netlink will tell us at least the basics of what's going on, and we can avoid trying to connect while there is no Internet route. On systems without a working GNetworkMonitor, GIO's default implementation will say "yes, you are online" at all times, which is no worse than we had previously. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=56635 Signed-off-by: Simon McVittie --- configure.ac | 29 +--- src/connectivity-monitor.c | 182 ++++++------------------- tests/twisted/Makefile.am | 7 - tests/twisted/account-manager/connectivity.py | 4 - tests/twisted/fakeconnectivity.py | 9 +- 5 files changed, 54 insertions(+), 177 deletions(-) diff --git a/configure.ac b/configure.ac index 18a1a28..c7a3266 100644 --- a/configure.ac +++ b/configure.ac @@ -283,29 +283,15 @@ fi # Connectivity integration # ----------------------------------------------------------- AC_ARG_WITH(connectivity, - AS_HELP_STRING([--with-connectivity=@<:@nm/connman/auto/no@:>@], + AS_HELP_STRING([--with-connectivity=@<:@nm/auto/no@:>@], [build with connectivity support]), , with_connectivity=auto) -if test "x$with_connectivity" = "xno"; then +# --with-connectivity=connman always used to turn off NM support, +# so treat it as "no" now that ConnMan doesn't need anything special. +if test "x$with_connectivity" = "xno" || test "x$with_connectivity" = "xconnman"; then have_nm=no - have_connman=no - -elif test "x$with_connectivity" = "xconnman"; then - have_nm=no - - PKG_CHECK_MODULES(CONNMAN, - [ - dbus-glib-1 - ], have_connman="yes", have_connman="no") - - if test "x$have_connman" = "xyes"; then - AC_DEFINE(HAVE_CONNMAN, 1, [Define if you have connman dependencies]) - fi - else - have_connman=no - PKG_CHECK_MODULES(NETWORK_MANAGER, [ libnm-glib >= 0.7.0 @@ -316,12 +302,6 @@ else fi fi -if test "x$with_connectivity" = "xconnman" -a "x$have_connman" != "xyes"; then - AC_MSG_ERROR([Couldn't find connman dependencies: - -$CONNMAN_PKG_ERRORS]) -fi - if test "x$with_connectivity" = "xnm" -a "x$have_nm" != "xyes"; then AC_MSG_ERROR([Couldn't find Network Manager dependencies: @@ -329,7 +309,6 @@ $NETWORK_MANAGER_PKG_ERRORS]) fi AM_CONDITIONAL(HAVE_NM, test "x$have_nm" = "xyes") -AM_CONDITIONAL(HAVE_CONNMAN, test "x$have_connman" = "xyes") AC_ARG_ENABLE([conn-setting], [AS_HELP_STRING([--enable-conn-setting], diff --git a/src/connectivity-monitor.c b/src/connectivity-monitor.c index eb728ca..618aaac 100644 --- a/src/connectivity-monitor.c +++ b/src/connectivity-monitor.c @@ -1,6 +1,7 @@ /* -*- Mode: C; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2; -*- */ /* * Copyright © 2009–2011 Collabora Ltd. + * Copyright © 2013 Intel Corporation * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,18 +27,9 @@ #ifdef HAVE_NM -# ifdef HAVE_CONNMAN -# error tried to build both Network Manager and ConnMan support simultaneously! -/* I mean, we could support both one day, but not for now. */ -# endif -/* Moving swiftly on… */ #include #endif -#ifdef HAVE_CONNMAN -#include -#endif - #ifdef HAVE_UPOWER #include #endif @@ -57,15 +49,13 @@ typedef enum { } Connectivity; struct _McdConnectivityMonitorPrivate { + GNetworkMonitor *network_monitor; + #ifdef HAVE_NM NMClient *nm_client; gulong state_change_signal_id; #endif -#ifdef HAVE_CONNMAN - DBusGProxy *proxy; -#endif - #ifdef HAVE_UPOWER UpClient *upower_client; #endif @@ -138,112 +128,57 @@ connectivity_monitor_nm_state_change_cb (NMClient *client, state = nm_client_get_state (priv->nm_client); + /* Deliberately not checking for DISCONNECTED. If we are really disconnected, + * the netlink GNetworkMonitor will say so; or if we have non-NM connectivity + * with a default route, we might as well give it a try. */ if (state == NM_STATE_CONNECTING #if NM_CHECK_VERSION(0,8,992) || state == NM_STATE_DISCONNECTING #endif || state == NM_STATE_ASLEEP) { - DEBUG ("New NetworkManager network state %d (unstable)", state); + DEBUG ("New NetworkManager network state %d (unstable state)", state); + connectivity_monitor_change_states (connectivity_monitor, 0, CONNECTIVITY_STABLE); } - else if (state == NM_STATE_DISCONNECTED) - { - DEBUG ("New NetworkManager network state %d (stable, down)", state); - connectivity_monitor_change_states (connectivity_monitor, - CONNECTIVITY_STABLE, CONNECTIVITY_UP); - } else { - DEBUG ("New NetworkManager network state %d (stable, up)", state); + DEBUG ("New NetworkManager network state %d (stable state)", state); connectivity_monitor_change_states (connectivity_monitor, - CONNECTIVITY_STABLE|CONNECTIVITY_UP, 0); + CONNECTIVITY_STABLE, 0); } } #endif -#ifdef HAVE_CONNMAN static void -connectivity_monitor_connman_state_changed (DBusGProxy *proxy, - const gchar *new_state, +connectivity_monitor_network_changed (GNetworkMonitor *monitor, + gboolean available, McdConnectivityMonitor *connectivity_monitor) { McdConnectivityMonitorPrivate *priv; - gboolean new_connected; priv = connectivity_monitor->priv; if (!priv->use_conn) return; - new_connected = (!tp_strdiff (new_state, "online") - || !tp_strdiff (new_state, "ready")); - - DEBUG ("New ConnMan network state %s", new_state); - - connectivity_monitor_set_connected (connectivity_monitor, new_connected); -} - -static void -connectivity_monitor_connman_property_changed_cb (DBusGProxy *proxy, - const gchar *prop_name, - const GValue *new_value, - McdConnectivityMonitor *connectivity_monitor) -{ - if (!tp_strdiff (prop_name, "State")) - connectivity_monitor_connman_state_changed (proxy, - g_value_get_string (new_value), connectivity_monitor); -} - -static void -connectivity_monitor_connman_check_state_cb (DBusGProxy *proxy, - DBusGProxyCall *call_id, - gpointer user_data) -{ - McdConnectivityMonitor *connectivity_monitor = (McdConnectivityMonitor *) user_data; - GError *error = NULL; - GHashTable *props; - - if (dbus_g_proxy_end_call (proxy, call_id, &error, - dbus_g_type_get_map ("GHashTable", G_TYPE_STRING, G_TYPE_VALUE), - &props, G_TYPE_INVALID)) + if (available) { - const gchar *state = tp_asv_get_string (props, "State"); - - if (state != NULL) - { - connectivity_monitor_connman_state_changed (proxy, state, - connectivity_monitor); - } - else - { - DEBUG ("Failed to get State: not in GetProperties return"); - } - - g_hash_table_unref (props); + DEBUG ("GNetworkMonitor (%s) says we are at least partially online", + G_OBJECT_TYPE_NAME (monitor)); + connectivity_monitor_change_states (connectivity_monitor, + CONNECTIVITY_UP, 0); } else { - DEBUG ("Failed to call GetProperties: %s", error->message); - connectivity_monitor_connman_state_changed (proxy, "offline", - connectivity_monitor); + DEBUG ("GNetworkMonitor (%s) says we are offline", + G_OBJECT_TYPE_NAME (monitor)); + connectivity_monitor_change_states (connectivity_monitor, + 0, CONNECTIVITY_UP); } } -static void -connectivity_monitor_connman_check_state (McdConnectivityMonitor *connectivity_monitor) -{ - McdConnectivityMonitorPrivate *priv; - - priv = connectivity_monitor->priv; - - dbus_g_proxy_begin_call (priv->proxy, "GetProperties", - connectivity_monitor_connman_check_state_cb, connectivity_monitor, NULL, - G_TYPE_INVALID); -} -#endif - #ifdef HAVE_UPOWER static void connectivity_monitor_set_awake ( @@ -285,10 +220,6 @@ static void mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor) { McdConnectivityMonitorPrivate *priv; -#ifdef HAVE_CONNMAN - DBusGConnection *connection; - GError *error = NULL; -#endif priv = G_TYPE_INSTANCE_GET_PRIVATE (connectivity_monitor, MCD_TYPE_CONNECTIVITY_MONITOR, McdConnectivityMonitorPrivate); @@ -296,7 +227,18 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor) connectivity_monitor->priv = priv; priv->use_conn = TRUE; - priv->connectivity = CONNECTIVITY_AWAKE | CONNECTIVITY_STABLE; + /* Initially, assume everything is good. */ + priv->connectivity = CONNECTIVITY_AWAKE | CONNECTIVITY_STABLE | + CONNECTIVITY_UP; + + priv->network_monitor = g_network_monitor_get_default (); + + tp_g_signal_connect_object (priv->network_monitor, "network-changed", + G_CALLBACK (connectivity_monitor_network_changed), + connectivity_monitor, 0); + connectivity_monitor_network_changed (priv->network_monitor, + g_network_monitor_get_network_available (priv->network_monitor), + connectivity_monitor); #ifdef HAVE_NM priv->nm_client = nm_client_new (); @@ -314,38 +256,6 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor) } #endif -#ifdef HAVE_CONNMAN - connection = dbus_g_bus_get (DBUS_BUS_SYSTEM, &error); - if (connection != NULL) - { - priv->proxy = dbus_g_proxy_new_for_name (connection, - "net.connman", "/", - "net.connman.Manager"); - - dbus_g_object_register_marshaller ( - g_cclosure_marshal_generic, - G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOXED, G_TYPE_INVALID); - - dbus_g_proxy_add_signal (priv->proxy, "PropertyChanged", - G_TYPE_STRING, G_TYPE_VALUE, G_TYPE_INVALID); - - dbus_g_proxy_connect_signal (priv->proxy, "PropertyChanged", - G_CALLBACK (connectivity_monitor_connman_property_changed_cb), - connectivity_monitor, NULL); - - connectivity_monitor_connman_check_state (connectivity_monitor); - } - else - { - DEBUG ("Failed to get system bus connection: %s", error->message); - g_error_free (error); - } -#endif - -#if !defined(HAVE_NM) && !defined(HAVE_CONNMAN) - priv->connectivity |= CONNECTIVITY_UP; -#endif - #ifdef HAVE_UPOWER priv->upower_client = up_client_new (); tp_g_signal_connect_object (priv->upower_client, @@ -360,7 +270,7 @@ mcd_connectivity_monitor_init (McdConnectivityMonitor *connectivity_monitor) static void connectivity_monitor_finalize (GObject *object) { -#if defined(HAVE_NM) || defined(HAVE_CONNMAN) || defined(HAVE_UPOWER) +#if defined(HAVE_NM) || defined(HAVE_UPOWER) McdConnectivityMonitor *connectivity_monitor = MCD_CONNECTIVITY_MONITOR (object); McdConnectivityMonitorPrivate *priv = connectivity_monitor->priv; #endif @@ -376,18 +286,6 @@ connectivity_monitor_finalize (GObject *object) } #endif -#ifdef HAVE_CONNMAN - if (priv->proxy != NULL) - { - dbus_g_proxy_disconnect_signal (priv->proxy, "PropertyChanged", - G_CALLBACK (connectivity_monitor_connman_property_changed_cb), - connectivity_monitor); - - g_object_unref (priv->proxy); - priv->proxy = NULL; - } -#endif - #ifdef HAVE_UPOWER tp_clear_object (&priv->upower_client); #endif @@ -398,6 +296,10 @@ connectivity_monitor_finalize (GObject *object) static void connectivity_monitor_dispose (GObject *object) { + McdConnectivityMonitor *self = MCD_CONNECTIVITY_MONITOR (object); + + g_clear_object (&self->priv->network_monitor); + G_OBJECT_CLASS (mcd_connectivity_monitor_parent_class)->dispose (object); } @@ -534,17 +436,17 @@ mcd_connectivity_monitor_set_use_conn (McdConnectivityMonitor *connectivity_moni priv->use_conn = use_conn; -#if defined(HAVE_NM) || defined(HAVE_CONNMAN) if (use_conn) { #if defined(HAVE_NM) connectivity_monitor_nm_state_change_cb (priv->nm_client, NULL, connectivity_monitor); -#elif defined(HAVE_CONNMAN) - connectivity_monitor_connman_check_state (connectivity_monitor); #endif + + connectivity_monitor_network_changed (priv->network_monitor, + g_network_monitor_get_network_available (priv->network_monitor), + connectivity_monitor); } else -#endif { /* !use_conn basically means "always assume it's stable and up". */ connectivity_monitor_change_states (connectivity_monitor, diff --git a/tests/twisted/Makefile.am b/tests/twisted/Makefile.am index 7bb3a10..6a34128 100644 --- a/tests/twisted/Makefile.am +++ b/tests/twisted/Makefile.am @@ -146,22 +146,15 @@ HAVE_MCE_PYBOOL = False endif if HAVE_NM -HAVE_CONNECTIVITY_PYBOOL = True HAVE_NM_PYBOOL = True else HAVE_NM_PYBOOL = False -if HAVE_CONNMAN -HAVE_CONNECTIVITY_PYBOOL = True -else -HAVE_CONNECTIVITY_PYBOOL = False -endif endif config.py: Makefile $(AM_V_GEN) { \ echo "HAVE_MCE = $(HAVE_MCE_PYBOOL)"; \ echo "HAVE_NM = $(HAVE_NM_PYBOOL)"; \ - echo "HAVE_CONNECTIVITY = $(HAVE_CONNECTIVITY_PYBOOL)"; \ } > $@ BUILT_SOURCES = config.py diff --git a/tests/twisted/account-manager/connectivity.py b/tests/twisted/account-manager/connectivity.py index 0198545..a8b6514 100644 --- a/tests/twisted/account-manager/connectivity.py +++ b/tests/twisted/account-manager/connectivity.py @@ -30,10 +30,6 @@ import constants as cs import config -if not config.HAVE_CONNECTIVITY: - print "NOTE: built without ConnMan or NM support" - raise SystemExit(77) - def sync_connectivity_state(mc): # We cannot simply use sync_dbus here, because nm-glib reports property # changes in an idle (presumably to batch them all together). This is fine diff --git a/tests/twisted/fakeconnectivity.py b/tests/twisted/fakeconnectivity.py index ccc62e5..02d577f 100644 --- a/tests/twisted/fakeconnectivity.py +++ b/tests/twisted/fakeconnectivity.py @@ -17,6 +17,10 @@ class FakeConnectivity(object): NM_STATE_CONNECTED_SITE = 60 NM_STATE_CONNECTED_GLOBAL = 70 + # Our fake GNetworkMonitor uses the ConnMan 0.79 D-Bus API - we don't + # have any special support for ConnMan any more, but it's as good an + # API as any. The important thing is that it's not NM, because we *do* + # have a bit of special support for that. CONNMAN_BUS_NAME = 'net.connman' CONNMAN_PATH = '/' CONNMAN_INTERFACE = 'net.connman.Manager' @@ -101,7 +105,10 @@ class FakeConnectivity(object): def change_state(self, online, indeterminate=False): if indeterminate: self.nm_state = self.NM_STATE_DISCONNECTING - # keep the previous ConnMan state + # keep the previous "ConnMan" (GNetworkMonitor) state; + # any other GNetworkMonitor would probably do the same + # while trying to disconnect, because e.g. netlink will say the + # interface is still up elif online: self.nm_state = self.NM_STATE_CONNECTED_GLOBAL self.connman_state = self.CONNMAN_ONLINE -- 1.7.10.4