From 12787f85656830be55296e96eb4553f3f301ab69 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 8 Feb 2014 07:52:24 -0600 Subject: [PATCH 1/5] mobile: fix disconnection on deactivation When c4fc72c7 began using the DEACTIVATING state, the modem code wasn't updated to handle this. Because it only checked for activating or ACTIVATED states to determine whether the modem was previously connected, and thus when an MM disconnect was needed, when the device enters the DISCONNECTED state it was no longer considered previously active, and not disconnected. Also, remove the NEED_AUTH handling from the modem code's device state switch, because it does not appear to be needed. The modem will only enter NEED_AUTH when it requires PAP/CHAP secrets during the connection attempt or when a PIN is required before enabling the modem. In both cases the modem won't yet be connected, so this code will never be hit. --- src/modem-manager/nm-modem.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/modem-manager/nm-modem.c b/src/modem-manager/nm-modem.c index 998decaa4e..77ab710c7f 100644 --- a/src/modem-manager/nm-modem.c +++ b/src/modem-manager/nm-modem.c @@ -581,27 +581,21 @@ nm_modem_device_state_changed (NMModem *self, g_return_if_fail (NM_IS_MODEM (self)); - if (old_state >= NM_DEVICE_STATE_PREPARE && old_state <= NM_DEVICE_STATE_ACTIVATED) + if (old_state >= NM_DEVICE_STATE_PREPARE && old_state <= NM_DEVICE_STATE_DEACTIVATING) was_connected = TRUE; priv = NM_MODEM_GET_PRIVATE (self); /* Make sure we don't leave the serial device open */ switch (new_state) { - case NM_DEVICE_STATE_NEED_AUTH: - if (priv->ppp_manager) - break; - /* else fall through */ case NM_DEVICE_STATE_UNMANAGED: case NM_DEVICE_STATE_UNAVAILABLE: - case NM_DEVICE_STATE_FAILED: case NM_DEVICE_STATE_DISCONNECTED: - if (new_state != NM_DEVICE_STATE_NEED_AUTH) { - if (priv->act_request) { - cancel_get_secrets (self); - g_object_unref (priv->act_request); - priv->act_request = NULL; - } + case NM_DEVICE_STATE_FAILED: + if (priv->act_request) { + cancel_get_secrets (self); + g_object_unref (priv->act_request); + priv->act_request = NULL; } if (was_connected) { From 4611aec5c2412464184be16b736168d1bc73f2d3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 9 Feb 2014 03:25:48 -0600 Subject: [PATCH 2/5] mobile: consolidate secrets requests into NMDeviceModem Both old and new ModemManager classes were doing the same thing, so consolidate that into the superclass and save some LoC. --- src/modem-manager/nm-modem-broadband.c | 57 ++++++++++---------------- src/modem-manager/nm-modem-old.c | 30 ++++---------- src/modem-manager/nm-modem.c | 56 +++++++++++++------------ src/modem-manager/nm-modem.h | 4 +- 4 files changed, 62 insertions(+), 85 deletions(-) diff --git a/src/modem-manager/nm-modem-broadband.c b/src/modem-manager/nm-modem-broadband.c index 032557de99..611931f6b9 100644 --- a/src/modem-manager/nm-modem-broadband.c +++ b/src/modem-manager/nm-modem-broadband.c @@ -296,48 +296,35 @@ create_gsm_connect_properties (NMConnection *connection) static NMActStageReturn act_stage1_prepare (NMModem *_self, - NMActRequest *req, - GPtrArray **out_hints, - const char **out_setting_name, + NMConnection *connection, NMDeviceStateReason *reason) { NMModemBroadband *self = NM_MODEM_BROADBAND (_self); - NMConnection *connection; + MMModemCapability caps; - connection = nm_act_request_get_connection (req); - g_assert (connection); + g_clear_object (&self->priv->connect_properties); - *out_setting_name = nm_connection_need_secrets (connection, out_hints); - if (!*out_setting_name) { - MMModemCapability caps; - - caps = mm_modem_get_current_capabilities (self->priv->modem_iface); - - g_clear_object (&self->priv->connect_properties); - - if (MODEM_CAPS_3GPP (caps)) - self->priv->connect_properties = create_gsm_connect_properties (connection); - else if (MODEM_CAPS_3GPP2 (caps)) - self->priv->connect_properties = create_cdma_connect_properties (connection); - else { - nm_log_warn (LOGD_MB, "(%s) not a mobile broadband modem", - nm_modem_get_uid (NM_MODEM (self))); - return NM_ACT_STAGE_RETURN_FAILURE; - } - - if (!self->priv->simple_iface) - self->priv->simple_iface = mm_object_get_modem_simple (self->priv->modem_object); - - g_dbus_proxy_set_default_timeout (G_DBUS_PROXY (self->priv->simple_iface), MODEM_CONNECT_TIMEOUT_SECS * 1000); - mm_modem_simple_connect (self->priv->simple_iface, - self->priv->connect_properties, - NULL, - (GAsyncReadyCallback)connect_ready, - g_object_ref (self)); - } else { - /* NMModem will handle requesting secrets... */ + caps = mm_modem_get_current_capabilities (self->priv->modem_iface); + if (MODEM_CAPS_3GPP (caps)) + self->priv->connect_properties = create_gsm_connect_properties (connection); + else if (MODEM_CAPS_3GPP2 (caps)) + self->priv->connect_properties = create_cdma_connect_properties (connection); + else { + nm_log_warn (LOGD_MB, "(%s) not a mobile broadband modem", + nm_modem_get_uid (NM_MODEM (self))); + return NM_ACT_STAGE_RETURN_FAILURE; } + if (!self->priv->simple_iface) + self->priv->simple_iface = mm_object_get_modem_simple (self->priv->modem_object); + + g_dbus_proxy_set_default_timeout (G_DBUS_PROXY (self->priv->simple_iface), MODEM_CONNECT_TIMEOUT_SECS * 1000); + mm_modem_simple_connect (self->priv->simple_iface, + self->priv->connect_properties, + NULL, + (GAsyncReadyCallback)connect_ready, + g_object_ref (self)); + return NM_ACT_STAGE_RETURN_POSTPONE; } diff --git a/src/modem-manager/nm-modem-old.c b/src/modem-manager/nm-modem-old.c index 866a94a122..f8bbcebb85 100644 --- a/src/modem-manager/nm-modem-old.c +++ b/src/modem-manager/nm-modem-old.c @@ -538,33 +538,21 @@ G_GNUC_END_IGNORE_DEPRECATIONS static NMActStageReturn act_stage1_prepare (NMModem *modem, - NMActRequest *req, - GPtrArray **out_hints, - const char **out_setting_name, + NMConnection *connection, NMDeviceStateReason *reason) { NMModemOld *self = NM_MODEM_OLD (modem); NMModemOldPrivate *priv = NM_MODEM_OLD_GET_PRIVATE (self); - NMConnection *connection; + gboolean enabled = nm_modem_get_mm_enabled (modem); - connection = nm_act_request_get_connection (req); - g_assert (connection); + if (priv->connect_properties) + g_hash_table_destroy (priv->connect_properties); + priv->connect_properties = create_connect_properties (connection); - *out_setting_name = nm_connection_need_secrets (connection, out_hints); - if (!*out_setting_name) { - gboolean enabled = nm_modem_get_mm_enabled (modem); - - if (priv->connect_properties) - g_hash_table_destroy (priv->connect_properties); - priv->connect_properties = create_connect_properties (connection); - - if (enabled) - do_connect (self); - else - do_enable (self); - } else { - /* NMModem will handle requesting secrets... */ - } + if (enabled) + do_connect (self); + else + do_enable (self); return NM_ACT_STAGE_RETURN_POSTPONE; } diff --git a/src/modem-manager/nm-modem.c b/src/modem-manager/nm-modem.c index 77ab710c7f..f427142e2b 100644 --- a/src/modem-manager/nm-modem.c +++ b/src/modem-manager/nm-modem.c @@ -416,9 +416,7 @@ nm_modem_get_secrets (NMModem *self, static NMActStageReturn act_stage1_prepare (NMModem *modem, - NMActRequest *req, - GPtrArray **out_hints, - const char **out_setting_name, + NMConnection *connection, NMDeviceStateReason *reason) { *reason = NM_DEVICE_STATE_REASON_UNKNOWN; @@ -435,37 +433,43 @@ nm_modem_act_stage1_prepare (NMModem *self, GPtrArray *hints = NULL; const char *setting_name = NULL; NMSettingsGetSecretsFlags flags = NM_SETTINGS_GET_SECRETS_FLAG_ALLOW_INTERACTION; + NMConnection *connection; if (priv->act_request) g_object_unref (priv->act_request); priv->act_request = g_object_ref (req); - ret = NM_MODEM_GET_CLASS (self)->act_stage1_prepare (self, - req, - &hints, - &setting_name, - reason); - if ((ret == NM_ACT_STAGE_RETURN_POSTPONE) && setting_name) { - if (priv->secrets_tries++) - flags |= NM_SETTINGS_GET_SECRETS_FLAG_REQUEST_NEW; + connection = nm_act_request_get_connection (req); + g_assert (connection); - priv->secrets_id = nm_act_request_get_secrets (req, - setting_name, - flags, - hints ? g_ptr_array_index (hints, 0) : NULL, - modem_secrets_cb, - self); - if (priv->secrets_id) - g_signal_emit (self, signals[AUTH_REQUESTED], 0); - else { - *reason = NM_DEVICE_STATE_REASON_NO_SECRETS; - ret = NM_ACT_STAGE_RETURN_FAILURE; - } - - if (hints) - g_ptr_array_free (hints, TRUE); + setting_name = nm_connection_need_secrets (connection, &hints); + if (!setting_name) { + /* Ready to connect */ + g_assert (!hints); + return NM_MODEM_GET_CLASS (self)->act_stage1_prepare (self, connection, reason); } + /* Secrets required... */ + if (priv->secrets_tries++) + flags |= NM_SETTINGS_GET_SECRETS_FLAG_REQUEST_NEW; + + priv->secrets_id = nm_act_request_get_secrets (req, + setting_name, + flags, + hints ? g_ptr_array_index (hints, 0) : NULL, + modem_secrets_cb, + self); + if (priv->secrets_id) { + g_signal_emit (self, signals[AUTH_REQUESTED], 0); + ret = NM_ACT_STAGE_RETURN_POSTPONE; + } else { + *reason = NM_DEVICE_STATE_REASON_NO_SECRETS; + ret = NM_ACT_STAGE_RETURN_FAILURE; + } + + if (hints) + g_ptr_array_free (hints, TRUE); + return ret; } diff --git a/src/modem-manager/nm-modem.h b/src/modem-manager/nm-modem.h index 8d754eb35c..531c79388e 100644 --- a/src/modem-manager/nm-modem.h +++ b/src/modem-manager/nm-modem.h @@ -95,9 +95,7 @@ typedef struct { GError **error); NMActStageReturn (*act_stage1_prepare) (NMModem *modem, - NMActRequest *req, - GPtrArray **out_hints, - const char **out_setting_name, + NMConnection *connection, NMDeviceStateReason *reason); NMActStageReturn (*static_stage3_ip4_config_start) (NMModem *self, From 5c1dee10cde3cc7cf74718650702dd124d46aa75 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 9 Feb 2014 03:31:21 -0600 Subject: [PATCH 3/5] mobile: only change state to NEED_AUTH during activation (rh #1058308) Auth requests only happen during activation and there's no need to request secrets at any other time. Ensure that the device state won't change to NEED_AUTH except when activating. (There's a case in NMModemBroadband where set_mm_enabled() when the modem is locked may cause this, but we'll solve this a different way later.) https://bugzilla.redhat.com/show_bug.cgi?id=1058308 --- src/devices/nm-device-bt.c | 10 +++++++++- src/devices/nm-device-modem.c | 10 +++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device-bt.c b/src/devices/nm-device-bt.c index e22c96eee3..3e5b693354 100644 --- a/src/devices/nm-device-bt.c +++ b/src/devices/nm-device-bt.c @@ -423,7 +423,15 @@ ppp_failed (NMModem *modem, NMDeviceStateReason reason, gpointer user_data) static void modem_auth_requested (NMModem *modem, gpointer user_data) { - nm_device_state_changed (NM_DEVICE (user_data), + NMDevice *device = NM_DEVICE (user_data); + + /* Auth requests (PIN, PAP/CHAP passwords, etc) only get handled + * during activation. + */ + if (!nm_device_is_activating (device)) + return; + + nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); } diff --git a/src/devices/nm-device-modem.c b/src/devices/nm-device-modem.c index 0bed60cd80..e047c03e11 100644 --- a/src/devices/nm-device-modem.c +++ b/src/devices/nm-device-modem.c @@ -113,7 +113,15 @@ modem_prepare_result (NMModem *modem, static void modem_auth_requested (NMModem *modem, gpointer user_data) { - nm_device_state_changed (NM_DEVICE (user_data), + NMDevice *device = NM_DEVICE (user_data); + + /* Auth requests (PIN, PAP/CHAP passwords, etc) only get handled + * during activation. + */ + if (!nm_device_is_activating (device)) + return; + + nm_device_state_changed (device, NM_DEVICE_STATE_NEED_AUTH, NM_DEVICE_STATE_REASON_NONE); } From 9d50e9dbd9d84f5e8df32895bc245dadf133aa39 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 9 Feb 2014 03:49:55 -0600 Subject: [PATCH 4/5] mobile: fix removal of ethernet interfaces owned by modems If the kernel doesn't tag a modem's ethernet interface with DEVTYPE=wwan then NetworkManager has no idea that's a modem (and cannot be used until connected via the control port). Since DEVTYPE=wwan devices get ignored by NM, so should these interfaces when NM knows they are modems. That got broken at some point for ModemManager1, because the data port isn't read until the modem is connected. NM only looked for and removed the data-port-as-ethernet-device when the modem was added, long before the MM1 data port was found. ModemManager does provide a list of ports owned by the modem though, which we can use at modem addition time to remove an ethernet device that is controled by the modem. --- src/modem-manager/nm-modem-broadband.c | 15 +++++++++++++ src/modem-manager/nm-modem.c | 21 +++++++++++++++++++ src/modem-manager/nm-modem.h | 4 ++++ src/nm-manager.c | 29 ++++++++++++++++---------- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/modem-manager/nm-modem-broadband.c b/src/modem-manager/nm-modem-broadband.c index 611931f6b9..5acce68f79 100644 --- a/src/modem-manager/nm-modem-broadband.c +++ b/src/modem-manager/nm-modem-broadband.c @@ -124,6 +124,20 @@ get_capabilities (NMModem *_self, *current_caps = (NMDeviceModemCapabilities) mm_modem_get_current_capabilities (self->priv->modem_iface); } +static gboolean +owns_port (NMModem *_self, const char *iface) +{ + NMModemBroadband *self = NM_MODEM_BROADBAND (_self); + const MMModemPortInfo *ports = NULL; + guint n_ports = 0, i; + gboolean owns = FALSE; + + mm_modem_peek_ports (self->priv->modem_iface, &ports, &n_ports); + for (i = 0; i < n_ports && !owns; i++) + owns = (g_strcmp0 (iface, ports[i].name) == 0); + return owns; +} + /*****************************************************************************/ static void @@ -916,6 +930,7 @@ nm_modem_broadband_class_init (NMModemBroadbandClass *klass) modem_class->check_connection_compatible = check_connection_compatible; modem_class->complete_connection = complete_connection; modem_class->act_stage1_prepare = act_stage1_prepare; + modem_class->owns_port = owns_port; /* Properties */ g_object_class_install_property diff --git a/src/modem-manager/nm-modem.c b/src/modem-manager/nm-modem.c index f427142e2b..e88ebe1bd7 100644 --- a/src/modem-manager/nm-modem.c +++ b/src/modem-manager/nm-modem.c @@ -661,6 +661,27 @@ nm_modem_get_data_port (NMModem *self) NM_MODEM_GET_PRIVATE (self)->ppp_iface : NM_MODEM_GET_PRIVATE (self)->data_port; } +gboolean +nm_modem_owns_port (NMModem *self, const char *iface) +{ + NMModemPrivate *priv = NM_MODEM_GET_PRIVATE (self); + + g_return_val_if_fail (iface != NULL, FALSE); + + if (NM_MODEM_GET_CLASS (self)->owns_port) + return NM_MODEM_GET_CLASS (self)->owns_port (self, iface); + + /* Fall back to data/control ports */ + if (priv->ppp_iface && (strcmp (priv->ppp_iface, iface) == 0)) + return TRUE; + if (priv->data_port && (strcmp (priv->data_port, iface) == 0)) + return TRUE; + if (priv->control_port && (strcmp (priv->control_port, iface) == 0)) + return TRUE; + + return FALSE; +} + /*****************************************************************************/ void diff --git a/src/modem-manager/nm-modem.h b/src/modem-manager/nm-modem.h index 531c79388e..db20407006 100644 --- a/src/modem-manager/nm-modem.h +++ b/src/modem-manager/nm-modem.h @@ -108,6 +108,8 @@ typedef struct { void (*deactivate) (NMModem *self, NMDevice *device); + gboolean (*owns_port) (NMModem *self, const char *iface); + /* Signals */ void (*ppp_stats) (NMModem *self, guint32 in_bytes, guint32 out_bytes); void (*ppp_failed) (NMModem *self, NMDeviceStateReason reason); @@ -127,6 +129,8 @@ const char *nm_modem_get_control_port (NMModem *modem); const char *nm_modem_get_data_port (NMModem *modem); const char *nm_modem_get_driver (NMModem *modem); +gboolean nm_modem_owns_port (NMModem *modem, const char *iface); + void nm_modem_get_capabilities (NMModem *self, NMDeviceModemCapabilities *modem_caps, NMDeviceModemCapabilities *current_caps); diff --git a/src/nm-manager.c b/src/nm-manager.c index 3ae48c8538..7cb71a63c7 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -550,19 +550,22 @@ modem_added (NMModemManager *modem_manager, { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - NMDevice *replace_device, *device = NULL; + NMDevice *device = NULL; const char *modem_iface; - GSList *iter; + GSList *iter, *remove = NULL; - /* Don't rely only on the data port; use the control port if available */ - modem_iface = nm_modem_get_data_port (modem); - if (!modem_iface) - modem_iface = nm_modem_get_control_port (modem); - g_return_if_fail (modem_iface); - - replace_device = find_device_by_ip_iface (NM_MANAGER (user_data), modem_iface); - if (replace_device) - remove_device (NM_MANAGER (user_data), replace_device, FALSE); + /* Remove ethernet devices that are actually owned by the modem, since + * they cannot be used as normal ethernet. + */ + for (iter = priv->devices; iter; iter = iter->next) { + if (nm_device_get_device_type (iter->data) == NM_DEVICE_TYPE_ETHERNET) { + if (nm_modem_owns_port (modem, nm_device_get_ip_iface (iter->data))) + remove = g_slist_prepend (remove, iter->data); + } + } + for (iter = remove; iter; iter = iter->next) + remove_device (self, NM_DEVICE (iter->data), FALSE); + g_slist_free (remove); /* Give Bluetooth DUN devices first chance to claim the modem */ for (iter = priv->devices; iter; iter = g_slist_next (iter)) { @@ -577,6 +580,10 @@ modem_added (NMModemManager *modem_manager, * by the Bluetooth code during the connection process. */ if (driver && !strcmp (driver, "bluetooth")) { + modem_iface = nm_modem_get_data_port (modem); + if (!modem_iface) + modem_iface = nm_modem_get_control_port (modem); + nm_log_info (LOGD_MB, "ignoring modem '%s' (no associated Bluetooth device)", modem_iface); return; } From 19089fb9609c5a62a3cfe5ff81e3d3d93beabeb3 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 18 Feb 2014 13:51:03 -0600 Subject: [PATCH 5/5] mobile: use gateway returned from ModemManager If we get a gateway, use it. --- src/modem-manager/nm-modem-broadband.c | 27 ++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/modem-manager/nm-modem-broadband.c b/src/modem-manager/nm-modem-broadband.c index 5acce68f79..f368c2e0ac 100644 --- a/src/modem-manager/nm-modem-broadband.c +++ b/src/modem-manager/nm-modem-broadband.c @@ -615,14 +615,16 @@ static gboolean ip_string_to_network_address (const gchar *str, guint32 *out) { - guint32 addr; + guint32 addr = 0; + gboolean success = FALSE; - /* IP address */ - if (inet_pton (AF_INET, str, &addr) <= 0) - return FALSE; + if (!str || inet_pton (AF_INET, str, &addr) != 1) + addr = 0; + else + success = TRUE; *out = (guint32)addr; - return TRUE; + return success; } static gboolean @@ -631,7 +633,9 @@ static_stage3_done (NMModemBroadband *self) GError *error = NULL; NMIP4Config *config = NULL; const gchar *address_string; + const gchar *gw_string; guint32 address_network; + guint32 gw; NMPlatformIP4Address address; const gchar **dns; guint i; @@ -652,6 +656,10 @@ static_stage3_done (NMModemBroadband *self) goto out; } + /* Missing gateway not a hard failure */ + gw_string = mm_bearer_ip_config_get_gateway (self->priv->ipv4_config); + ip_string_to_network_address (gw_string, &gw); + config = nm_ip4_config_new (); memset (&address, 0, sizeof (address)); @@ -660,9 +668,12 @@ static_stage3_done (NMModemBroadband *self) address.source = NM_PLATFORM_SOURCE_WWAN; nm_ip4_config_add_address (config, &address); - nm_log_info (LOGD_MB, " address %s/%d", - mm_bearer_ip_config_get_address (self->priv->ipv4_config), - mm_bearer_ip_config_get_prefix (self->priv->ipv4_config)); + nm_log_info (LOGD_MB, " address %s/%d", address_string, address.plen); + + if (gw) { + nm_ip4_config_set_gateway (config, gw); + nm_log_info (LOGD_MB, " gateway %s", gw_string); + } /* DNS servers */ dns = mm_bearer_ip_config_get_dns (self->priv->ipv4_config);