Re: [MM] [PATCH] bearer: allow specifying default IP family for bearers
Thanks. Submitted a revised patch.
- Ben
On Mon, Apr 22, 2013 at 12:42 AM, Aleksander Morgado
wrote:
> On 04/22/2013 03:50 AM, Ben Chan wrote:
> > This patch adds a 'bearer-default-ip-family' property to MMBearer, which
> > specifies the default IP family to use for a bearer when no explicit
> > value is given via the simple connect properties. The default IP family
> > is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
> > which allows a modem plugin to specify an appropriate default value.
>
> The logic to use the new property was added only in the
> MMBroadbandBearer subclass; all the other available MMBearer subclasses
> (QMI, MBIM) should also get modified to use this new default value.
>
> Some more comments below.
>
>
> > ---
> > libmm-glib/mm-bearer-properties.c | 8 +---
> > src/mm-bearer.c | 25 +
> > src/mm-bearer.h | 2 ++
> > src/mm-broadband-bearer.c | 25 +
> > 4 files changed, 49 insertions(+), 11 deletions(-)
> >
> > diff --git a/libmm-glib/mm-bearer-properties.c
> b/libmm-glib/mm-bearer-properties.c
> > index 6526ed0..2f59fee 100644
> > --- a/libmm-glib/mm-bearer-properties.c
> > +++ b/libmm-glib/mm-bearer-properties.c
> > @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
> > self->priv->allow_roaming = TRUE;
> > self->priv->rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
> > self->priv->allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
> > -
> > -/* At some point in the future, this default should probably be
> changed
> > - * to IPV4V6. However, presently support for this PDP type is rare.
> An
> > - * even better approach would likely be to query which PDP types the
> > - * modem supports (using AT+CGDCONT=?), and set the default
> accordingly
> > - */
> > -self->priv->ip_type = MM_BEARER_IP_FAMILY_IPV4;
> > +self->priv->ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
> > }
> >
> > static void
> > diff --git a/src/mm-bearer.c b/src/mm-bearer.c
> > index 9e96bde..d047e11 100644
> > --- a/src/mm-bearer.c
> > +++ b/src/mm-bearer.c
> > @@ -57,6 +57,7 @@ enum {
> > PROP_MODEM,
> > PROP_STATUS,
> > PROP_CONFIG,
> > +PROP_DEFAULT_IP_FAMILY,
> > PROP_LAST
> > };
> >
> > @@ -73,6 +74,8 @@ struct _MMBearerPrivate {
> > MMBearerStatus status;
> > /* Configuration of the bearer */
> > MMBearerProperties *config;
> > +/* Default IP family of this bearer */
> > +MMBearerIpFamily default_ip_family;
> >
> > /* Cancellable for connect() */
> > GCancellable *connect_cancellable;
> > @@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self)
> > NULL);
> > }
> >
> > +MMBearerIpFamily
> > +mm_bearer_get_default_ip_family (MMBearer *self)
> > +{
> > +return self->priv->default_ip_family;
> > +}
> > +
> >
>
> /*/
> >
> > static void
> > @@ -969,6 +978,9 @@ set_property (GObject *object,
> > g_variant_unref (dictionary);
> > break;
> > }
> > +case PROP_DEFAULT_IP_FAMILY:
> > +self->priv->default_ip_family = g_value_get_enum (value);
> > +break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> > break;
> > @@ -999,6 +1011,9 @@ get_property (GObject *object,
> > case PROP_CONFIG:
> > g_value_set_object (value, self->priv->config);
> > break;
> > +case PROP_DEFAULT_IP_FAMILY:
> > +g_value_set_enum (value, self->priv->default_ip_family);
> > +break;
> > default:
> > G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> > break;
> > @@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self)
> > self->priv->status = MM_BEARER_STATUS_DISCONNECTED;
> > self->priv->reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE;
> > self->priv->reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE;
> > +self->priv->default_ip_family = MM_BEARER_IP_FAMILY_IPV4;
> >
> > /* Set defaults */
> > mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL);
> > @@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass)
> > MM_TYPE_BEARER_PROPERTIES,
> > G_PARAM_READWRITE);
> > g_object_class_install_property (object_class, PROP_CONFIG,
> properties[PROP_CONFIG]);
> > +
> > +properties[PROP_DEFAULT_IP_FAMILY] =
> > +g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY,
> > + "Bearer default IP family",
> > + "IP family to use for this bearer when no IP
> family is specified",
> > + MM_TYPE_BEARER_IP_FAMILY,
> > + MM_BEARER_IP_FAMILY_IPV4,
> > + G_PARAM_READWRITE);
> > +g_object_class_install_property (object_clas
[MM] [PATCH v2] bearer: allow specifying default IP family for bearers
This patch adds a 'bearer-default-ip-family' property to MMBearer, which
specifies the default IP family to use for a bearer when no explicit
value is given via the simple connect properties. The default IP family
is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
which allows a modem plugin to specify an appropriate default value.
---
libmm-glib/mm-bearer-properties.c | 8 +---
src/mm-bearer-mbim.c | 14 +++---
src/mm-bearer-qmi.c | 18 +-
src/mm-bearer.c | 25 +
src/mm-bearer.h | 2 ++
src/mm-broadband-bearer.c | 16
6 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/libmm-glib/mm-bearer-properties.c
b/libmm-glib/mm-bearer-properties.c
index 6526ed0..2f59fee 100644
--- a/libmm-glib/mm-bearer-properties.c
+++ b/libmm-glib/mm-bearer-properties.c
@@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
self->priv->allow_roaming = TRUE;
self->priv->rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
self->priv->allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
-
-/* At some point in the future, this default should probably be changed
- * to IPV4V6. However, presently support for this PDP type is rare. An
- * even better approach would likely be to query which PDP types the
- * modem supports (using AT+CGDCONT=?), and set the default accordingly
- */
-self->priv->ip_type = MM_BEARER_IP_FAMILY_IPV4;
+self->priv->ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
}
static void
diff --git a/src/mm-bearer-mbim.c b/src/mm-bearer-mbim.c
index d96a1b1..bddc9da 100644
--- a/src/mm-bearer-mbim.c
+++ b/src/mm-bearer-mbim.c
@@ -618,6 +618,7 @@ connect_context_step (ConnectContext *ctx)
const gchar *password;
MbimAuthProtocol auth;
MbimContextIpType ip_type;
+MMBearerIpFamily ip_family;
GError *error = NULL;
/* Setup parameters to use */
@@ -658,7 +659,14 @@ connect_context_step (ConnectContext *ctx)
}
}
-switch (mm_bearer_properties_get_ip_type (ctx->properties)) {
+ip_family = mm_bearer_properties_get_ip_type (ctx->properties);
+if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) {
+ip_family = mm_bearer_get_default_ip_family (MM_BEARER
(ctx->self));
+mm_dbg ("No specific IP family requested, defaulting to %s",
+mm_bearer_ip_family_get_string (ip_family));
+}
+
+switch (ip_family) {
case MM_BEARER_IP_FAMILY_IPV4:
ip_type = MBIM_CONTEXT_IP_TYPE_IPV4;
break;
@@ -670,8 +678,8 @@ connect_context_step (ConnectContext *ctx)
break;
case MM_BEARER_IP_FAMILY_UNKNOWN:
default:
-mm_dbg ("No specific IP family requested, defaulting to IPv4");
-ip_type = MBIM_CONTEXT_IP_TYPE_IPV4;
+/* A valid default IP family should have been specified */
+g_assert_not_reached ();
break;
}
diff --git a/src/mm-bearer-qmi.c b/src/mm-bearer-qmi.c
index 8f9568d..9e8d2bc 100644
--- a/src/mm-bearer-qmi.c
+++ b/src/mm-bearer-qmi.c
@@ -846,11 +846,21 @@ _connect (MMBearer *self,
if (properties) {
MMBearerAllowedAuth auth;
+MMBearerIpFamily ip_family;
ctx->apn = g_strdup (mm_bearer_properties_get_apn (properties));
ctx->user = g_strdup (mm_bearer_properties_get_user (properties));
ctx->password = g_strdup (mm_bearer_properties_get_password
(properties));
-switch (mm_bearer_properties_get_ip_type (properties)) {
+
+ip_family = mm_bearer_properties_get_ip_type (properties);
+if (ip_family == MM_BEARER_IP_FAMILY_UNKNOWN) {
+ip_family = mm_bearer_get_default_ip_family (self);
+mm_dbg ("No specific IP family requested, defaulting to %s",
+mm_bearer_ip_family_get_string (ip_family));
+ctx->no_ip_family_preference = TRUE;
+}
+
+switch (ip_family) {
case MM_BEARER_IP_FAMILY_IPV4:
ctx->ipv4 = TRUE;
ctx->ipv6 = FALSE;
@@ -865,10 +875,8 @@ _connect (MMBearer *self,
break;
case MM_BEARER_IP_FAMILY_UNKNOWN:
default:
-mm_dbg ("No specific IP family requested, defaulting to IPv4");
-ctx->no_ip_family_preference = TRUE;
-ctx->ipv4 = TRUE;
-ctx->ipv6 = FALSE;
+/* A valid default IP family should have been specified */
+g_assert_not_reached ();
break;
}
diff --git a/src/mm-bearer.c b/src/mm-bearer.c
index 9e96bde..d047e11 100644
--- a/src/mm-bearer.c
+++ b/src/mm-bearer.c
@@ -57,6 +57,7 @@ enum {
PROP_MODEM,
PROP_STATUS,
PROP_CONFIG,
+PROP_DEFAULT_IP_FAMILY,
PROP_LAST
};
@@ -73,6 +74,8 @@ struct _MMBearerPrivate {
MMBearerSt
Re: While waiting for MBIM support
>>> I was hoping to help finishing the MM MBIM support, but have to realize >>> that available spare time is insufficient... And with no one sponsoring >>> Aleksander's work I guess MBIM just will have to wait. >> >> I just merged the MBIM support in ModemManager git master, which >> requires the libmbim-glib I've been writing in the past months. We're >> still waiting for the new project request in freedesktop.org, so I put >> the tarball in libqmi's release path: >> >> http://www.freedesktop.org/software/libqmi/libmbim-0.0.1.tar.xz >> >> The libmbim repo is still in gitorious.org: >> https://gitorious.org/lanedo/libmbim >> >> This libmbim shouldn't be considered stable yet, I guess. There's also a >> mbimcli included in the sources, but didn't spend much time on it. >> >> So far, the support seems to be working nice for all MBIM modems that >> Bjørn has :) > > Yes. Great work! As usual, you work at a speed which makes me > feel...uhmm.. slow :) > > I'm impressed, yet again. > > AFAICS, the support is already fully functional when it comes to > establishing connections. So do grab and test if you have a MBIM > capable device! > > And if anyone on the list could verify that it works with the Ericsson > H5321gw/F5321gw, also branded as Dell 5560, then that would be greatly > appreciated. This device seems to be common in some newer Lenovo and > Dell laptops, and is the only one I know of which implements the "NCM > backwards compatibility" solution. Which really makes it *more* > difficult to support because it ends up requiring userspace support for > both NCM and MBIM to avoid failing if the system selects the "wrong" > protocol. > > For anyone interested in MBIM/libmbim, we now have a separate mailing list: http://lists.freedesktop.org/mailman/listinfo/libmbim-devel We also have a place for releases: http://www.freedesktop.org/software/libmbim A new official git repo: http://cgit.freedesktop.org/libmbim/libmbim/ And a bugzilla product: https://bugs.freedesktop.org/enter_bug.cgi?product=libmbim Thanks to fd.o admins for handling all this :) -- Aleksander ___ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
Re: [MM] [PATCH] bearer: allow specifying default IP family for bearers
On 04/22/2013 03:50 AM, Ben Chan wrote:
> This patch adds a 'bearer-default-ip-family' property to MMBearer, which
> specifies the default IP family to use for a bearer when no explicit
> value is given via the simple connect properties. The default IP family
> is set to IPv4 in MMBearer but can be overridden by a MMBearer subclass,
> which allows a modem plugin to specify an appropriate default value.
The logic to use the new property was added only in the
MMBroadbandBearer subclass; all the other available MMBearer subclasses
(QMI, MBIM) should also get modified to use this new default value.
Some more comments below.
> ---
> libmm-glib/mm-bearer-properties.c | 8 +---
> src/mm-bearer.c | 25 +
> src/mm-bearer.h | 2 ++
> src/mm-broadband-bearer.c | 25 +
> 4 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/libmm-glib/mm-bearer-properties.c
> b/libmm-glib/mm-bearer-properties.c
> index 6526ed0..2f59fee 100644
> --- a/libmm-glib/mm-bearer-properties.c
> +++ b/libmm-glib/mm-bearer-properties.c
> @@ -674,13 +674,7 @@ mm_bearer_properties_init (MMBearerProperties *self)
> self->priv->allow_roaming = TRUE;
> self->priv->rm_protocol = MM_MODEM_CDMA_RM_PROTOCOL_UNKNOWN;
> self->priv->allowed_auth = MM_BEARER_ALLOWED_AUTH_UNKNOWN;
> -
> -/* At some point in the future, this default should probably be changed
> - * to IPV4V6. However, presently support for this PDP type is rare. An
> - * even better approach would likely be to query which PDP types the
> - * modem supports (using AT+CGDCONT=?), and set the default accordingly
> - */
> -self->priv->ip_type = MM_BEARER_IP_FAMILY_IPV4;
> +self->priv->ip_type = MM_BEARER_IP_FAMILY_UNKNOWN;
> }
>
> static void
> diff --git a/src/mm-bearer.c b/src/mm-bearer.c
> index 9e96bde..d047e11 100644
> --- a/src/mm-bearer.c
> +++ b/src/mm-bearer.c
> @@ -57,6 +57,7 @@ enum {
> PROP_MODEM,
> PROP_STATUS,
> PROP_CONFIG,
> +PROP_DEFAULT_IP_FAMILY,
> PROP_LAST
> };
>
> @@ -73,6 +74,8 @@ struct _MMBearerPrivate {
> MMBearerStatus status;
> /* Configuration of the bearer */
> MMBearerProperties *config;
> +/* Default IP family of this bearer */
> +MMBearerIpFamily default_ip_family;
>
> /* Cancellable for connect() */
> GCancellable *connect_cancellable;
> @@ -844,6 +847,12 @@ mm_bearer_get_config (MMBearer *self)
> NULL);
> }
>
> +MMBearerIpFamily
> +mm_bearer_get_default_ip_family (MMBearer *self)
> +{
> +return self->priv->default_ip_family;
> +}
> +
>
> /*/
>
> static void
> @@ -969,6 +978,9 @@ set_property (GObject *object,
> g_variant_unref (dictionary);
> break;
> }
> +case PROP_DEFAULT_IP_FAMILY:
> +self->priv->default_ip_family = g_value_get_enum (value);
> +break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -999,6 +1011,9 @@ get_property (GObject *object,
> case PROP_CONFIG:
> g_value_set_object (value, self->priv->config);
> break;
> +case PROP_DEFAULT_IP_FAMILY:
> +g_value_set_enum (value, self->priv->default_ip_family);
> +break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -1015,6 +1030,7 @@ mm_bearer_init (MMBearer *self)
> self->priv->status = MM_BEARER_STATUS_DISCONNECTED;
> self->priv->reason_3gpp = CONNECTION_FORBIDDEN_REASON_NONE;
> self->priv->reason_cdma = CONNECTION_FORBIDDEN_REASON_NONE;
> +self->priv->default_ip_family = MM_BEARER_IP_FAMILY_IPV4;
>
> /* Set defaults */
> mm_gdbus_bearer_set_interface (MM_GDBUS_BEARER (self), NULL);
> @@ -,6 +1127,15 @@ mm_bearer_class_init (MMBearerClass *klass)
> MM_TYPE_BEARER_PROPERTIES,
> G_PARAM_READWRITE);
> g_object_class_install_property (object_class, PROP_CONFIG,
> properties[PROP_CONFIG]);
> +
> +properties[PROP_DEFAULT_IP_FAMILY] =
> +g_param_spec_enum (MM_BEARER_DEFAULT_IP_FAMILY,
> + "Bearer default IP family",
> + "IP family to use for this bearer when no IP
> family is specified",
> + MM_TYPE_BEARER_IP_FAMILY,
> + MM_BEARER_IP_FAMILY_IPV4,
> + G_PARAM_READWRITE);
> +g_object_class_install_property (object_class, PROP_DEFAULT_IP_FAMILY,
> properties[PROP_DEFAULT_IP_FAMILY]);
> }
>
>
> /*/
> diff --git a/src/mm-bearer.h b/src/mm-bearer.h
> index c617297..cc71bfd 100644
> --- a/src/mm-bearer.h
> +++ b/src/mm-bearer.h
> @@ -58,6 +58,7 @@ typedef stru
