This patch fixes three (re) connection problems with the ofono plugin: 1) If the modem is connected, and registrations drops, and then is restored, the connection isn't re-activated. The fix was simply to change modem_state_cb to not return after setting the state to failed, which allows nm_device_ queue_recheck_available to be called, which queues a state transition to UNAVAILABLE.
2) If ofono returns InProgress, don't treat as a PREPARE_FAILURE. 3) If the ofono conext is already active, use it's existing 'Settings' property to configure the device. Bug-Ubuntu: https://bugs.launchpad.net/bugs/1565717 Bug-Ubuntu: https://bugs.launchpad.net/bugs/1579098 Gbp-Pq: Name wwan-fix-ofono-connection-problems.patch --- src/devices/wwan/nm-device-modem.c | 4 +- src/devices/wwan/nm-modem-ofono.c | 193 +++++++++++++++++++++++++++---------- src/devices/wwan/nm-modem-ofono.h | 2 + 3 files changed, 145 insertions(+), 54 deletions(-) diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 3bbd170..22009f8 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -286,12 +286,10 @@ modem_state_cb (NMModem *modem, if (new_state < NM_MODEM_STATE_CONNECTING && old_state >= NM_MODEM_STATE_CONNECTING && dev_state >= NM_DEVICE_STATE_NEED_AUTH && - dev_state <= NM_DEVICE_STATE_ACTIVATED) { + dev_state <= NM_DEVICE_STATE_ACTIVATED) /* Fail the device if the modem disconnects unexpectedly while the * device is activating/activated. */ nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_MODEM_NO_CARRIER); - return; - } if (new_state > NM_MODEM_STATE_LOCKED && old_state == NM_MODEM_STATE_LOCKED) { /* If the modem is now unlocked, enable/disable it according to the diff --git a/src/devices/wwan/nm-modem-ofono.c b/src/devices/wwan/nm-modem-ofono.c index 4566be0..c09cfd5 100644 --- a/src/devices/wwan/nm-modem-ofono.c +++ b/src/devices/wwan/nm-modem-ofono.c @@ -789,42 +789,35 @@ stage1_prepare_done (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data g_dbus_proxy_call_finish (proxy, result, &error); if (error) { - nm_log_warn (LOGD_MB, "ofono: connection failed: (%d) %s", - error ? error->code : -1, - error && error->message ? error->message : "(unknown)"); + if (!g_strstr_len (error->message, NM_STRLEN (OFONO_ERROR_IN_PROGRESS), OFONO_ERROR_IN_PROGRESS)) { + nm_log_warn (LOGD_MB, "ofono: connection failed: (%d) %s", + error ? error->code : -1, + error && error->message ? error->message : "(unknown)"); - g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, - NM_DEVICE_STATE_REASON_MODEM_BUSY); - /* - * FIXME: add code to check for InProgress so that the - * connection doesn't continue to try and activate, - * leading to the connection being disabled, and a 5m - * timeout... - */ + g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, + NM_DEVICE_STATE_REASON_MODEM_BUSY); + } else + nm_log_warn (LOGD_MB, "ofono: connection activation returned Error.InProgress"); g_clear_error (&error); } } static void -context_property_changed (GDBusProxy *proxy, - const char *property, - GVariant *v, - gpointer user_data) +handle_settings (GVariant *v_dict, gpointer user_data) { NMModemOfono *self = NM_MODEM_OFONO (user_data); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); NMDeviceStateReason reason = NM_DEVICE_STATE_REASON_NONE; NMPlatformIP4Address addr; gboolean ret = FALSE; - GVariant *v_dict; GVariantIter i; const gchar *s, *addr_s; const gchar **array, **iter; guint32 address_network, gateway_network; guint prefix = 0; - nm_log_dbg (LOGD_MB, "PropertyChanged: %s", property); + nm_log_info (LOGD_MB, "ofono: (%s): IPv4 static Settings:", nm_modem_get_uid (NM_MODEM (self))); /* * TODO: might be a good idea and re-factor this to mimic bluez-device, @@ -832,18 +825,6 @@ context_property_changed (GDBusProxy *proxy, * handle the action. */ - if (g_strcmp0 (property, "Settings") != 0) - return; - - v_dict = g_variant_get_child_value (v, 0); - if (!v_dict) { - nm_log_warn (LOGD_MB, "ofono: (%s): error getting IPv4 Settings", - nm_modem_get_uid (NM_MODEM (self))); - goto out; - } - - nm_log_info (LOGD_MB, "ofono: (%s): IPv4 static Settings:", nm_modem_get_uid (NM_MODEM (self))); - if (g_variant_lookup (v_dict, "Interface", "&s", &s)) { nm_log_dbg (LOGD_MB, "(%s): Interface: %s", nm_modem_get_uid (NM_MODEM (self)), s); @@ -930,8 +911,7 @@ context_property_changed (GDBusProxy *proxy, } nm_log_info (LOGD_MB, "ofono (%s) Address: %s/%d", - nm_modem_get_uid (NM_MODEM (self)), addr_s, prefix); - + nm_modem_get_uid (NM_MODEM (self)), addr_s, prefix); nm_ip4_config_add_address (priv->ip4_config, &addr); if (g_variant_lookup (v_dict, "Gateway", "&s", &s)) { @@ -972,7 +952,7 @@ context_property_changed (GDBusProxy *proxy, if (iter == array) { nm_log_warn (LOGD_MB, "ofono: (%s): Settings: 'DomainNameServers': none specified", - nm_modem_get_uid (NM_MODEM (self))); + nm_modem_get_uid (NM_MODEM (self))); g_free (array); goto out; } @@ -1009,7 +989,7 @@ context_property_changed (GDBusProxy *proxy, out: if (nm_modem_get_state (NM_MODEM (self)) != NM_MODEM_STATE_CONNECTED) { nm_log_info (LOGD_MB, "ofono: (%s): emitting PREPARE_RESULT: %s", - nm_modem_get_uid (NM_MODEM (self)), ret ? "TRUE" : "FALSE"); + nm_modem_get_uid (NM_MODEM (self)), ret ? "TRUE" : "FALSE"); if (!ret) reason = NM_DEVICE_STATE_REASON_IP_CONFIG_UNAVAILABLE; @@ -1022,6 +1002,33 @@ out: } } +static void +context_property_changed (GDBusProxy *proxy, + const char *property, + GVariant *v, + gpointer user_data) +{ + NMModemOfono *self = NM_MODEM_OFONO (user_data); + GVariant *v_dict; + + nm_log_dbg (LOGD_MB, "PropertyChanged: %s", property); + + if (g_strcmp0 (property, "Settings") != 0) + return; + + v_dict = g_variant_get_child_value (v, 0); + if (!v_dict) { + nm_log_warn (LOGD_MB, "ofono: (%s): error getting IPv4 Settings", + nm_modem_get_uid (NM_MODEM (self))); + return; + } + + g_assert (g_variant_is_of_type (v_dict, G_VARIANT_TYPE_VARDICT)); + + handle_settings (v_dict, user_data); + g_variant_unref (v_dict); +} + static NMActStageReturn static_stage3_ip4_config_start (NMModem *_self, NMActRequest *req, @@ -1053,11 +1060,100 @@ static_stage3_ip4_config_start (NMModem *_self, } static void +context_properties_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +{ + NMModemOfono *self = NM_MODEM_OFONO (user_data); + NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + GError *error = NULL; + GVariant *properties, *settings; + GVariant *v_dict = NULL; + gboolean active; + + nm_log_dbg (LOGD_MB, "in %s", __func__); + + properties = g_dbus_proxy_call_finish (proxy, result, &error); + + if (error) { + nm_log_warn (LOGD_MB, "ofono: connection failed; couldn't read context properties (%d) %s", + error ? error->code : -1, + error && error->message ? error->message : "(unknown)"); + + g_clear_error (&error); + goto error; + } + + if (properties == NULL) { + nm_log_warn (LOGD_MB, "ofono: connection failed; no context properties returned"); + goto error; + } + + v_dict = g_variant_get_child_value (properties, 0); + g_assert (v_dict); + g_assert (g_variant_is_of_type (v_dict, G_VARIANT_TYPE_VARDICT)); + + g_variant_unref (properties); + + if (!g_variant_lookup (v_dict, "Active", "b", &active)) { + nm_log_warn (LOGD_MB, "ofono: connection failed; can't read 'Active' property"); + goto error; + } + + /* Watch for custom ofono PropertyChanged signals */ + _nm_dbus_signal_connect (priv->context_proxy, + "PropertyChanged", + G_VARIANT_TYPE ("(sv)"), + G_CALLBACK (context_property_changed), + self); + + if (active) { + nm_log_dbg (LOGD_MB, "connection is already Active"); + + settings = g_variant_lookup_value (v_dict, "Settings", G_VARIANT_TYPE_VARDICT); + if (settings == NULL) { + nm_log_warn (LOGD_MB, "ofono: connection failed; can't read 'Settings' property"); + goto error; + } + + handle_settings (settings, user_data); + g_variant_unref (settings); + + } else + g_dbus_proxy_call (priv->context_proxy, + "SetProperty", + g_variant_new ("(sv)", + "Active", + g_variant_new ("b", TRUE)), + G_DBUS_CALL_FLAGS_NONE, + 20000, + NULL, + (GAsyncReadyCallback) stage1_prepare_done, + g_object_ref (self)); + + g_variant_unref (v_dict); + + return; + +error: + if (properties) + g_variant_unref (properties); + + if (v_dict) + g_variant_unref (v_dict); + + g_signal_emit_by_name (self, NM_MODEM_PREPARE_RESULT, FALSE, + NM_DEVICE_STATE_REASON_MODEM_BUSY); +} + +static void context_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMModemOfono *self = NM_MODEM_OFONO (user_data); NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); + gboolean context_active; GError *error = NULL; + GVariant *v_dict; + GVariant *active_property; + GVariant *settings_property; nm_log_dbg (LOGD_MB, "%s:", __func__); @@ -1087,23 +1183,18 @@ context_proxy_new_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat if (priv->ip4_config) g_clear_object (&priv->ip4_config); - /* Watch for custom ofono PropertyChanged signals */ - _nm_dbus_signal_connect (priv->context_proxy, - "PropertyChanged", - G_VARIANT_TYPE ("(sv)"), - G_CALLBACK (context_property_changed), - self); - + /* As ofono doesn't properly implement DBus.Properties, we need to + * query the ConnectionContextinteface directly to get the current + * property values vs. using g_dbus_proxy_get_cached_property. + */ g_dbus_proxy_call (priv->context_proxy, - "SetProperty", - g_variant_new ("(sv)", - "Active", - g_variant_new ("b", TRUE)), - G_DBUS_CALL_FLAGS_NONE, - 20000, - NULL, - (GAsyncReadyCallback) stage1_prepare_done, - g_object_ref (self)); + "GetProperties", + NULL, + G_DBUS_CALL_FLAGS_NONE, + 20000, + NULL, + (GAsyncReadyCallback) context_properties_cb, + g_object_ref (self)); } static void @@ -1112,8 +1203,8 @@ do_context_activate (NMModemOfono *self) NMModemOfonoPrivate *priv = NM_MODEM_OFONO_GET_PRIVATE (self); GValue value = G_VALUE_INIT; - g_return_val_if_fail (self != NULL, FALSE); - g_return_val_if_fail (NM_IS_MODEM_OFONO (self), FALSE); + g_assert (self != NULL); + g_assert (NM_IS_MODEM_OFONO (self)); nm_log_dbg (LOGD_MB, "in %s", __func__); diff --git a/src/devices/wwan/nm-modem-ofono.h b/src/devices/wwan/nm-modem-ofono.h index fa79e15..02eca11 100644 --- a/src/devices/wwan/nm-modem-ofono.h +++ b/src/devices/wwan/nm-modem-ofono.h @@ -41,6 +41,8 @@ G_BEGIN_DECLS #define OFONO_DBUS_INTERFACE_CONNECTION_CONTEXT "org.ofono.ConnectionContext" #define OFONO_DBUS_INTERFACE_SIM_MANAGER "org.ofono.SimManager" +#define OFONO_ERROR_IN_PROGRESS "org.ofono.Error.InProgress" + typedef enum { NM_OFONO_ERROR_CONNECTION_NOT_OFONO = 0, /*< nick=ConnectionNotOfono >*/ NM_OFONO_ERROR_CONNECTION_INVALID, /*< nick=ConnectionInvalid >*/ -- 2.7.4 _______________________________________________ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list