<div dir="ltr">Logic is sound.<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> the user ends up changing the CID that the device uses for default LTE attachment (e.g. CID=1 for Qualcomm or CID=16 for Huawei ME909u)</blockquote><div>Wouldn't this still end up being okay for users too. Once they realize they're using the wrong CDC & switch pdp-cid (& the lte back off timer expires), the next 4g connection attempt should re-set the LTE(1 /14) pdp-cid? That's how I read the Cinterion AT doc.</div><div> </div><div>Code LGTM</div></div><div class="gmail_extra"> <div class="gmail_quote">On Sun, May 14, 2017 at 8:51 AM, Aleksander Morgado <<a href="mailto:aleksander@aleksander.es" target="_blank">aleksander@aleksander.es</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Some operators, most notably Verizon, require connection to be performed through a specific PDP context (#3). This API update allows the user to specify the PDP CID via a new 'pdp-cid' parameter given in either the Simple.Connect() or the Modem.CreateBearer() methods. This implementation is currently bound to every modem that uses the AT protocol and the AT+CGDCONT command to manage available PDP contexts. --- Hey Dan, Reinhard and Matthew, Some months ago we discussed about how we could define the PDP context to be used in a connection, as some operators would require specific PDP contexts (e.g. Verizon would require PDP context #3 for the internet connection). This patch is an initial approach to the problem, by letting the user select in which PDP context ID the APN should be configured and selected for the connection. This parameter is optional, and if not given we'll default to the original logic of looking for the best CID to use, e.g. by re-using one with the same settings we're requesting, if available. This feature may be risky if, as Reinhard pointed out, the user ends up changing the CID that the device uses for default LTE attachment (e.g. CID=1 for Qualcomm or CID=16 for Huawei ME909u). But being it optional, I don't see why we shouldn't have this, if it helps setting up proper connections with operators that require a specific PDP context ID. What do you guys think? --- cli/mmcli-bearer.c | 9 ++++ docs/reference/libmm-glib/libmm-glib-sections.txt | 4 ++ .../org.freedesktop.ModemManager1.Modem.xml | 2 + libmm-glib/mm-bearer-properties.c | 59 +++++++++++++++++++++- libmm-glib/mm-bearer-properties.h | 3 ++ libmm-glib/mm-simple-connect-properties.c | 34 +++++++++++++ libmm-glib/mm-simple-connect-properties.h | 3 ++ src/mm-broadband-bearer.c | 41 ++++++++++----- src/mm-iface-modem-simple.c | 13 +++-- 9 files changed, 150 insertions(+), 18 deletions(-) diff --git a/cli/mmcli-bearer.c b/cli/mmcli-bearer.c index cd9ecca0..daa86148 100644 --- a/cli/mmcli-bearer.c +++ b/cli/mmcli-bearer.c @@ -163,11 +163,18 @@ print_bearer_info (MMBearer *bearer) if (properties) { gchar *ip_family_str; + gchar *pdp_cid_str = NULL; + guint pdp_cid; ip_family_str = (mm_bearer_ip_family_build_string_from_mask ( mm_bearer_properties_get_ip_type (properties))); + + if ((pdp_cid = mm_bearer_properties_get_pdp_cid (properties)) != 0) + pdp_cid_str = g_strdup_printf ("%u", pdp_cid); + g_print (" -------------------------\n" " Properties | apn: '%s'\n" + " | PDP CID: '%s'\n" " | roaming: '%s'\n" " | IP type: '%s'\n" " | user: '%s'\n" @@ -175,6 +182,7 @@ print_bearer_info (MMBearer *bearer) " | number: '%s'\n" " | Rm protocol: '%s'\n", VALIDATE_NONE (mm_bearer_properties_get_apn (properties)), + VALIDATE_NONE (pdp_cid_str), mm_bearer_properties_get_allow_roaming (properties) ? "allowed" : "forbidden", VALIDATE_UNKNOWN (ip_family_str), VALIDATE_NONE (mm_bearer_properties_get_user (properties)), @@ -183,6 +191,7 @@ print_bearer_info (MMBearer *bearer) VALIDATE_UNKNOWN (mm_modem_cdma_rm_protocol_get_string ( mm_bearer_properties_get_rm_protocol (properties)))); g_free (ip_family_str); + g_free (pdp_cid_str); } /* IPv4 */ diff --git a/docs/reference/libmm-glib/libmm-glib-sections.txt b/docs/reference/libmm-glib/libmm-glib-sections.txt index 46f2d923..077e7ee3 100644 --- a/docs/reference/libmm-glib/libmm-glib-sections.txt +++ b/docs/reference/libmm-glib/libmm-glib-sections.txt @@ -779,6 +779,8 @@ mm_simple_connect_properties_get_operator_id mm_simple_connect_properties_set_operator_id mm_simple_connect_properties_get_apn mm_simple_connect_properties_set_apn +mm_simple_connect_properties_get_pdp_cid +mm_simple_connect_properties_set_pdp_cid mm_simple_connect_properties_get_allowed_auth mm_simple_connect_properties_set_allowed_auth mm_simple_connect_properties_get_user @@ -1064,6 +1066,8 @@ mm_bearer_properties_new <SUBSECTION GettersSetters> mm_bearer_properties_get_apn mm_bearer_properties_set_apn +mm_bearer_properties_get_pdp_cid +mm_bearer_properties_set_pdp_cid mm_bearer_properties_get_allowed_auth mm_bearer_properties_set_allowed_auth mm_bearer_properties_get_user diff --git a/introspection/org.freedesktop.ModemManager1.Modem.xml b/introspection/org.freedesktop.ModemManager1.Modem.xml index a5a236c5..23c14ede 100644 --- a/introspection/org.freedesktop.ModemManager1.Modem.xml +++ b/introspection/org.freedesktop.ModemManager1.Modem.xml @@ -61,6 +61,8 @@ <variablelist> <varlistentry><term><literal>"apn"</literal></term> <listitem><para>Access Point Name, given as a string value (signature <literal>"s"</literal>). Required in 3GPP.</para></listitem></varlistentry> + <varlistentry><term><literal>"pdp-cid"</literal></term> + <listitem><para>PDP context identifier, given as an unsigned 32-bit integer value (signature <literal>"u"</literal>). Optional in 3GPP.</para></listitem></varlistentry> <varlistentry><term><literal>"ip-type"</literal></term> <listitem><para>Addressing type, given as a <link linkend="MMBearerIpFamily">MMBearerIpFamily</link> value (signature <literal>"u"</literal>). Optional in 3GPP and CDMA.</para></listitem></varlistentry> <varlistentry><term><literal>"allowed-auth"</literal></term> diff --git a/libmm-glib/mm-bearer-properties.c b/libmm-glib/mm-bearer-properties.c index c87068ff..8e54f108 100644 --- a/libmm-glib/mm-bearer-properties.c +++ b/libmm-glib/mm-bearer-properties.c @@ -34,6 +34,7 @@ G_DEFINE_TYPE (MMBearerProperties, mm_bearer_properties, G_TYPE_OBJECT); #define PROPERTY_APN "apn" +#define PROPERTY_PDP_CID "pdp-cid" #define PROPERTY_ALLOWED_AUTH "allowed-auth" #define PROPERTY_USER "user" #define PROPERTY_PASSWORD "password" @@ -45,6 +46,8 @@ G_DEFINE_TYPE (MMBearerProperties, mm_bearer_properties, G_TYPE_OBJECT); struct _MMBearerPropertiesPrivate { /* APN */ gchar *apn; + /* PDP CID */ + guint pdp_cid; /* IP type */ MMBearerIpFamily ip_type; /* Allowed auth */ @@ -100,6 +103,40 @@ mm_bearer_properties_get_apn (MMBearerProperties *self) /*****************************************************************************/ /** + * mm_bearer_properties_set_pdp_cid: + * @self: a #MMBearerProperties. + * @pdp_cid: PDP context ID. + * + * Sets the PDP context ID to use in the connection attempt. + */ +void +mm_bearer_properties_set_pdp_cid (MMBearerProperties *self, + guint pdp_cid) +{ + g_return_if_fail (MM_IS_BEARER_PROPERTIES (self)); + + self->priv->pdp_cid = pdp_cid; +} + +/** + * mm_bearer_properties_get_pdp_cid: + * @self: a #MMBearerProperties. + * + * Gets the PDP context ID to use in the connection attempt. + * + * Returns: the context number, or 0 if not set. + */ +guint +mm_bearer_properties_get_pdp_cid (MMBearerProperties *self) +{ + g_return_val_if_fail (MM_IS_BEARER_PROPERTIES (self), 0); + + return self->priv->pdp_cid; +} + +/*****************************************************************************/ + +/** * mm_bearer_properties_set_allowed_auth: * @self: a #MMBearerProperties. * @allowed_auth: a bitmask of #MMBearerAllowedAuth values. %MM_BEARER_ALLOWED_AUTH_UNKNOWN may be given to request the modem-default method. @@ -361,6 +398,12 @@ mm_bearer_properties_get_dictionary (MMBearerProperties *self) PROPERTY_APN, g_variant_new_string (self->priv->apn)); + if (self->priv->pdp_cid) + g_variant_builder_add (&builder, + "{sv}", + PROPERTY_PDP_CID, + g_variant_new_uint32 (self->priv->pdp_cid)); + if (self->priv->allowed_auth != MM_BEARER_ALLOWED_AUTH_UNKNOWN) g_variant_builder_add (&builder, "{sv}", @@ -428,7 +471,16 @@ mm_bearer_properties_consume_string (MMBearerProperties *self, if (g_str_equal (key, PROPERTY_APN)) mm_bearer_properties_set_apn (self, value); - else if (g_str_equal (key, PROPERTY_ALLOWED_AUTH)) { + else if (g_str_equal (key, PROPERTY_PDP_CID)) { + guint aux; + + if (!mm_get_uint_from_str (value, &aux)) { + g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED, + "Invalid PDP CID given: %s", value); + return FALSE; + } + mm_bearer_properties_set_pdp_cid (self, aux); + }else if (g_str_equal (key, PROPERTY_ALLOWED_AUTH)) { GError *inner_error = NULL; MMBearerAllowedAuth allowed_auth; @@ -548,6 +600,10 @@ mm_bearer_properties_consume_variant (MMBearerProperties *properties, mm_bearer_properties_set_apn ( properties, g_variant_get_string (value, NULL)); + else if (g_str_equal (key, PROPERTY_PDP_CID)) + mm_bearer_properties_set_pdp_cid ( + properties, + g_variant_get_uint32 (value)); else if (g_str_equal (key, PROPERTY_ALLOWED_AUTH)) mm_bearer_properties_set_allowed_auth ( properties, @@ -664,6 +720,7 @@ mm_bearer_properties_cmp (MMBearerProperties *a, MMBearerProperties *b) { return ((!g_strcmp0 (a->priv->apn, b->priv->apn)) && + (a->priv->pdp_cid == b->priv->pdp_cid) && (a->priv->ip_type == b->priv->ip_type) && (!g_strcmp0 (a->priv->number, b->priv->number)) && (a->priv->allowed_auth == b->priv->allowed_auth) && diff --git a/libmm-glib/mm-bearer-properties.h b/libmm-glib/mm-bearer-properties.h index 361c8678..6fc1347d 100644 --- a/libmm-glib/mm-bearer-properties.h +++ b/libmm-glib/mm-bearer-properties.h @@ -59,6 +59,8 @@ MMBearerProperties *mm_bearer_properties_new (void); void mm_bearer_properties_set_apn (MMBearerProperties *self, const gchar *apn); +void mm_bearer_properties_set_pdp_cid (MMBearerProperties *self, + guint pdp_cid); void mm_bearer_properties_set_allowed_auth (MMBearerProperties *self, MMBearerAllowedAuth allowed_auth); void mm_bearer_properties_set_user (MMBearerProperties *self, @@ -75,6 +77,7 @@ void mm_bearer_properties_set_rm_protocol (MMBearerProperties *self, MMModemCdmaRmProtocol protocol); const gchar *mm_bearer_properties_get_apn (MMBearerProperties *self); +guint mm_bearer_properties_get_pdp_cid (MMBearerProperties *self); MMBearerAllowedAuth mm_bearer_properties_get_allowed_auth (MMBearerProperties *self); const gchar *mm_bearer_properties_get_user (MMBearerProperties *self); const gchar *mm_bearer_properties_get_password (MMBearerProperties *self); diff --git a/libmm-glib/mm-simple-connect-properties.c b/libmm-glib/mm-simple-connect-properties.c index 200b7c38..c696f50c 100644 --- a/libmm-glib/mm-simple-connect-properties.c +++ b/libmm-glib/mm-simple-connect-properties.c @@ -153,6 +153,40 @@ mm_simple_connect_properties_get_apn (MMSimpleConnectProperties *self) /*****************************************************************************/ /** + * mm_simple_connect_properties_set_pdp_cid: + * @self: a #MMSimpleConnectProperties. + * @pdp_cid: PDP context ID. + * + * Sets the PDP context ID to use in the connection attempt. + */ +void +mm_simple_connect_properties_set_pdp_cid (MMSimpleConnectProperties *self, + guint pdp_cid) +{ + g_return_if_fail (MM_IS_SIMPLE_CONNECT_PROPERTIES (self)); + + mm_bearer_properties_set_pdp_cid (self->priv->bearer_properties, pdp_cid); +} + +/** + * mm_simple_connect_properties_get_pdp_cid: + * @self: a #MMSimpleConnectProperties. + * + * Gets the PDP context ID to use in the connection attempt. + * + * Returns: the context number, or 0 if not set. + */ +guint +mm_simple_connect_properties_get_pdp_cid (MMSimpleConnectProperties *self) +{ + g_return_val_if_fail (MM_IS_SIMPLE_CONNECT_PROPERTIES (self), 0); + + return mm_bearer_properties_get_pdp_cid (self->priv->bearer_properties); +} + +/*****************************************************************************/ + +/** * mm_simple_connect_properties_set_allowed_auth: * @self: a #MMSimpleConnectProperties. * @allowed_auth: a bitmask of #MMBearerAllowedAuth values. %MM_BEARER_ALLOWED_AUTH_UNKNOWN may be given to request the modem-default method. diff --git a/libmm-glib/mm-simple-connect-properties.h b/libmm-glib/mm-simple-connect-properties.h index 3167db0a..5378c5e9 100644 --- a/libmm-glib/mm-simple-connect-properties.h +++ b/libmm-glib/mm-simple-connect-properties.h @@ -65,6 +65,8 @@ void mm_simple_connect_properties_set_operator_id (MMSimpleConnectProperties * const gchar *operator_id); void mm_simple_connect_properties_set_apn (MMSimpleConnectProperties *self, const gchar *apn); +void mm_simple_connect_properties_set_pdp_cid (MMSimpleConnectProperties *self, + guint pdp_cid); void mm_simple_connect_properties_set_allowed_auth (MMSimpleConnectProperties *self, MMBearerAllowedAuth allowed_auth); void mm_simple_connect_properties_set_user (MMSimpleConnectProperties *self, @@ -81,6 +83,7 @@ void mm_simple_connect_properties_set_number (MMSimpleConnectProperties * const gchar *mm_simple_connect_properties_get_pin (MMSimpleConnectProperties *self); const gchar *mm_simple_connect_properties_get_operator_id (MMSimpleConnectProperties *self); const gchar *mm_simple_connect_properties_get_apn (MMSimpleConnectProperties *self); +guint mm_simple_connect_properties_get_pdp_cid (MMSimpleConnectProperties *self); MMBearerAllowedAuth mm_simple_connect_properties_get_allowed_auth (MMSimpleConnectProperties *self); const gchar *mm_simple_connect_properties_get_user (MMSimpleConnectProperties *self); const gchar *mm_simple_connect_properties_get_password (MMSimpleConnectProperties *self); diff --git a/src/mm-broadband-bearer.c b/src/mm-broadband-bearer.c index ac701733..de4fa72e 100644 --- a/src/mm-broadband-bearer.c +++ b/src/mm-broadband-bearer.c @@ -732,25 +732,17 @@ initialize_pdp_context_ready (MMBaseModem *modem, } static void -find_cid_ready (MMBaseModem *modem, - GAsyncResult *res, - GTask *task) +initialize_pdp_context (GTask *task) { gchar *apn; gchar *command; - GError *error = NULL; const gchar *pdp_type; CidSelection3gppContext *ctx; ctx = (CidSelection3gppContext *) g_task_get_task_data (task); - mm_base_modem_at_sequence_full_finish (modem, res, NULL, &error); - if (error) { - mm_warn ("Couldn't find best CID to use: '%s'", error->message); - g_task_return_error (task, error); - g_object_unref (task); - return; - } + /* If no error reported, we must have a valid CID to be used */ + g_assert (ctx->cid != 0); /* Validate requested PDP type */ pdp_type = mm_3gpp_get_pdp_type_from_ip_family (ctx->ip_family); @@ -766,9 +758,6 @@ find_cid_ready (MMBaseModem *modem, return; } - /* If no error reported, we must have a valid CID to be used */ - g_assert (ctx->cid != 0); - /* If there's already a PDP context defined, just use it */ if (ctx->use_existing_cid) { g_task_return_int (task, (gssize) ctx->cid); @@ -792,6 +781,24 @@ find_cid_ready (MMBaseModem *modem, g_free (command); } +static void +find_cid_ready (MMBaseModem *modem, + GAsyncResult *res, + GTask *task) +{ + GError *error = NULL; + + mm_base_modem_at_sequence_full_finish (modem, res, NULL, &error); + if (error) { + mm_warn ("Couldn't find best CID to use: '%s'", error->message); + g_task_return_error (task, error); + g_object_unref (task); + return; + } + + initialize_pdp_context (task); +} + static gboolean parse_cid_range (MMBaseModem *modem, CidSelection3gppContext *ctx, @@ -998,6 +1005,12 @@ cid_selection_3gpp (MMBroadbandBearer *self, task = g_task_new (self, cancellable, callback, user_data); g_task_set_task_data (task, ctx, (GDestroyNotify) cid_selection_3gpp_context_free); + if ((ctx->cid = mm_bearer_properties_get_pdp_cid (mm_base_bearer_peek_config (MM_BASE_BEARER (ctx->self)))) != 0) { + mm_dbg ("Explicit CID requested by the user: %u", ctx->cid); + initialize_pdp_context (task); + return; + } + mm_dbg ("Looking for best CID..."); mm_base_modem_at_sequence_full (ctx->modem, ctx->primary, diff --git a/src/mm-iface-modem-simple.c b/src/mm-iface-modem-simple.c index a16adb19..59d980e4 100644 --- a/src/mm-iface-modem-simple.c +++ b/src/mm-iface-modem-simple.c @@ -651,9 +651,10 @@ connect_auth_ready (MMBaseModem *self, /* Log about all the parameters being used for the simple connect */ { - MMBearerAllowedAuth allowed_auth; - gchar *str; - MMBearerIpFamily ip_family; + MMBearerAllowedAuth allowed_auth; + gchar *str; + MMBearerIpFamily ip_family; + guint aux_uint; #define VALIDATE_UNSPECIFIED(str) (str ? str : "unspecified") @@ -665,6 +666,12 @@ connect_auth_ready (MMBaseModem *self, mm_dbg (" APN: %s", VALIDATE_UNSPECIFIED (mm_simple_connect_properties_get_apn (ctx->properties))); + aux_uint = mm_simple_connect_properties_get_pdp_cid (ctx->properties); + if (aux_uint) + mm_dbg (" PDP CID: %u", aux_uint); + else + mm_dbg (" PDP CID: unspecified"); + ip_family = mm_simple_connect_properties_get_ip_type (ctx->properties); if (ip_family != MM_BEARER_IP_FAMILY_NONE) { str = mm_bearer_ip_family_build_string_from_mask (ip_family); -- 2.12.2 </blockquote></div> </div>