[PATCH] api: allow specifying the PDP CID to use
matthew stanger
stangerm2 at gmail.com
Sun May 14 17:07:27 UTC 2017
Logic is sound.
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)
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.
Code LGTM
On Sun, May 14, 2017 at 8:51 AM, Aleksander Morgado <
aleksander at aleksander.es> wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/modemmanager-devel/attachments/20170514/533f2d45/attachment-0001.html>
More information about the ModemManager-devel
mailing list