[RFC PATCHv3 0/4] Plugin API for provisioning of GPRS context settings

[RFC PATCHv3 0/4] Plugin API for provisioning of GPRS context settings

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

Hello

Here is another RFC patchset about implementing automatic provisioning GPRS context settings. ("Internet Access Provider database" TODO item).

This time the approach is to add a new driver/plugin API for provisioning modules. Different kind of provisioning modules may be created for different platforms or use cases.
It is up to the module what settings database it uses, and how many, and what type of contexts (with settings) it returns.
Several plugins may be loaded in oFono, and they will be called in priority order until first plugin returns non-empty result.

Provisioning modules are called in case there are no previously configured contexts found during gprs atom registration.
 
These patches add new gprs-provision.[hc] API for provisioning plugins to register into, and get_settings() for gprs.c to call when needed.
gprs.c is modified to use provisioning if reading existing context settings fails.

A dummy example provisioning plugin is included. In case it is called when using phonesim with default.xml configuration, the plugin returns dummy settings for two contexts, one type Internet and one type MMS. 

Patches also add new function ofono_sim_get_mnc_length to SIM atom API for figuring out MNC value (typically needed by provisioning plugins).

--Jukka Saunamäki

[RFC PATCH 1/4] Added GPRS context provisioning driver API sources

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 7567 bytes --]

---
 Makefile.am              |    4 +-
 include/gprs-provision.h |   73 +++++++++++++++++++++++++
 src/gprs-provision.c     |  133 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 208 insertions(+), 2 deletions(-)
 create mode 100644 include/gprs-provision.h
 create mode 100644 src/gprs-provision.c

diff --git a/Makefile.am b/Makefile.am
index 8ad01cd..0f330a7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -15,7 +15,7 @@ include_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/radio-settings.h include/stk.h \
 			include/audio-settings.h include/nettime.h \
 			include/ctm.h include/cdma-voicecall.h \
-			include/cdma-sms.h
+			include/cdma-sms.h include/gprs-provision.h
 
 nodist_include_HEADERS = include/version.h
 
@@ -331,7 +331,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/nettime.c src/stkagent.c src/stkagent.h \
 			src/simfs.c src/simfs.h src/audio-settings.c \
 			src/smsagent.c src/smsagent.h src/ctm.c \
-			src/cdma-voicecall.c
+			src/cdma-voicecall.c src/gprs-provision.c
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/include/gprs-provision.h b/include/gprs-provision.h
new file mode 100644
index 0000000..97e356b
--- /dev/null
+++ b/include/gprs-provision.h
@@ -0,0 +1,73 @@
+/*
+ *
+ *  oFono - Open Telephony stack for Linux
+ *
+ *  Copyright (C) 2011  Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __OFONO_GPRS_PROVISION_H
+#define __OFONO_GPRS_PROVISION_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "gprs-context.h"
+
+struct ofono_gprs_provisioning_data {
+	enum ofono_gprs_context_type type;
+	enum ofono_gprs_proto proto;
+	gchar *name;
+	gchar *apn;
+	gchar *username;
+	gchar *password;
+	gchar *message_proxy;
+	gchar *message_center;
+};
+
+/*
+ * Callback from provisioning plugin.
+ * settings: list of struct ofono_gprs_provisioning_data
+ *
+ * It is responsibility of callback function to free settings-list
+ * settings-list elements must be freed with ofono_gprs_provisioning_data_free()
+ */
+typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
+
+struct ofono_gprs_provision_driver {
+	const char *name;
+	int priority;
+	void (*get_settings) (struct ofono_modem *modem,
+				ofono_gprs_provision_cb_t cb,
+				void *userdata);
+};
+
+/* For provisioning drivers/plugins */
+int ofono_gprs_provision_driver_register(const struct ofono_gprs_provision_driver *driver);
+void ofono_gprs_provision_driver_unregister(const struct ofono_gprs_provision_driver *driver);
+
+/* For gprs */
+void ofono_gprs_provision_get_settings(struct ofono_modem *modem,
+					ofono_gprs_provision_cb_t cb,
+					void *data);
+void ofono_gprs_provisioning_data_free(struct ofono_gprs_provisioning_data *data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GPRS_PROVISION_H */
diff --git a/src/gprs-provision.c b/src/gprs-provision.c
new file mode 100644
index 0000000..c143fb6
--- /dev/null
+++ b/src/gprs-provision.c
@@ -0,0 +1,133 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2011  Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <glib.h>
+
+#include "ofono.h"
+#include "gprs-provision.h"
+
+struct gprs_provisioning_request {
+	GSList *current_driver;
+	struct ofono_modem *modem;
+	ofono_gprs_provision_cb_t cb;
+	void *userdata;
+};
+
+static GSList *gprs_provision_drivers = NULL;
+
+
+void ofono_gprs_provisioning_data_free(struct ofono_gprs_provisioning_data *data)
+{
+	if (data == NULL)
+		return;
+
+	g_free(data->name);
+	g_free(data->apn);
+	g_free(data->username);
+	g_free(data->password);
+	g_free(data->message_proxy);
+	g_free(data->message_center);
+	g_free(data);
+}
+
+
+static void settings_cb(GSList *settings, void *userdata)
+{
+	GSList *d;
+	struct gprs_provisioning_request *req = userdata;
+
+	if (settings != NULL)
+		DBG("Provisioning plugin returned settings for %d contexts",
+			g_slist_length(settings));
+	else
+		DBG("Provisioning plugin returned no settings");
+
+	d = req->current_driver->next;
+	if (settings == NULL && d != NULL) {
+		/* No success from this driver, try next */
+		const struct ofono_gprs_provision_driver *driver = d->data;
+		req->current_driver = d;
+		DBG("Calling provisioning plugin '%s'", driver->name);
+		driver->get_settings(req->modem, settings_cb, req);
+		return;
+	}
+
+	req->cb(settings, req->userdata);
+	g_free(req);
+}
+
+
+void ofono_gprs_provision_get_settings(struct ofono_modem *modem,
+					ofono_gprs_provision_cb_t cb,
+					void *userdata)
+{
+	struct gprs_provisioning_request *req;
+	const struct ofono_gprs_provision_driver *driver;
+
+	if (gprs_provision_drivers == 0)
+		goto error;
+
+	req = g_try_new0(struct gprs_provisioning_request, 1);
+	if (req == NULL)
+		goto error;
+
+	req->modem = modem;
+	req->cb = cb;
+	req->userdata = userdata;
+	req->current_driver = gprs_provision_drivers;
+
+	driver = gprs_provision_drivers->data;
+	DBG("Calling provisioning plugin '%s'", driver->name);
+	driver->get_settings(modem, settings_cb, req);
+	return;
+
+error:
+	cb(NULL, userdata);
+}
+
+static gint compare_priority(gconstpointer a, gconstpointer b)
+{
+	const struct ofono_gprs_provision_driver *plugin1 = a;
+	const struct ofono_gprs_provision_driver *plugin2 = b;
+
+	return plugin2->priority - plugin1->priority;
+}
+
+int ofono_gprs_provision_driver_register(const struct ofono_gprs_provision_driver *driver)
+{
+	DBG("driver: %p name: %s", driver, driver->name);
+
+	gprs_provision_drivers = g_slist_insert_sorted(gprs_provision_drivers,
+							(void *) driver,
+							compare_priority);
+	return 0;
+}
+
+void ofono_gprs_provision_driver_unregister(const struct ofono_gprs_provision_driver *driver)
+{
+	DBG("driver: %p name: %s", driver, driver->name);
+
+	gprs_provision_drivers = g_slist_remove(gprs_provision_drivers, driver);
+}
-- 
1.7.1

Re: [RFC PATCH 1/4] Added GPRS context provisioning driver API sources

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 4976 bytes --]

Hi Jukka,

> +#ifndef __OFONO_GPRS_PROVISION_H
> +#define __OFONO_GPRS_PROVISION_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include "gprs-context.h"
> +
> +struct ofono_gprs_provisioning_data {
> +	enum ofono_gprs_context_type type;
> +	enum ofono_gprs_proto proto;
> +	gchar *name;
> +	gchar *apn;
> +	gchar *username;
> +	gchar *password;
> +	gchar *message_proxy;
> +	gchar *message_center;
> +};

don't bother with gchar *. Just use char * here. The gchar is
essentially bad idea. We only use it a few rare occasions or when it got
missed in a review.

> +
> +/*
> + * Callback from provisioning plugin.
> + * settings: list of struct ofono_gprs_provisioning_data
> + *
> + * It is responsibility of callback function to free settings-list
> + * settings-list elements must be freed with ofono_gprs_provisioning_data_free()
> + */
> +typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
> +
> +struct ofono_gprs_provision_driver {
> +	const char *name;
> +	int priority;
> +	void (*get_settings) (struct ofono_modem *modem,
> +				ofono_gprs_provision_cb_t cb,
> +				void *userdata);
> +};

So here is something we need to talk about. The nettime plugin
infrastructure using a "pseudo" atom. So do we wanna do the same here or
do we wanna change the nettime atom to something simple like this.

I like to have consistency here. Aki, Denis, thoughts?

> +struct gprs_provisioning_request {
> +	GSList *current_driver;
> +	struct ofono_modem *modem;
> +	ofono_gprs_provision_cb_t cb;
> +	void *userdata;

The sort of general style is to call this user_data.

> +};
> +
> +static GSList *gprs_provision_drivers = NULL;
> +
> +

We also normally don't do double empty lines.

> +void ofono_gprs_provisioning_data_free(struct ofono_gprs_provisioning_data *data)
> +{
> +	if (data == NULL)
> +		return;
> +
> +	g_free(data->name);
> +	g_free(data->apn);
> +	g_free(data->username);
> +	g_free(data->password);
> +	g_free(data->message_proxy);
> +	g_free(data->message_center);
> +	g_free(data);
> +}
> +
> +
> +static void settings_cb(GSList *settings, void *userdata)
> +{

Same here, please use user_data.

> +	GSList *d;
> +	struct gprs_provisioning_request *req = userdata;

And as another general rule, the variable that have an assignment like
the user_data come first.

> +
> +	if (settings != NULL)
> +		DBG("Provisioning plugin returned settings for %d contexts",
> +			g_slist_length(settings));
> +	else
> +		DBG("Provisioning plugin returned no settings");
> +
> +	d = req->current_driver->next;
> +	if (settings == NULL && d != NULL) {
> +		/* No success from this driver, try next */
> +		const struct ofono_gprs_provision_driver *driver = d->data;
> +		req->current_driver = d;
> +		DBG("Calling provisioning plugin '%s'", driver->name);
> +		driver->get_settings(req->modem, settings_cb, req);
> +		return;
> +	}
> +
> +	req->cb(settings, req->userdata);
> +	g_free(req);
> +}
> +
> +

Same here for the double empty lines. Please fix them all.

> +void ofono_gprs_provision_get_settings(struct ofono_modem *modem,
> +					ofono_gprs_provision_cb_t cb,
> +					void *userdata)
> +{
> +	struct gprs_provisioning_request *req;
> +	const struct ofono_gprs_provision_driver *driver;
> +
> +	if (gprs_provision_drivers == 0)

This needs to be a == NULL.

> +		goto error;
> +
> +	req = g_try_new0(struct gprs_provisioning_request, 1);
> +	if (req == NULL)
> +		goto error;
> +
> +	req->modem = modem;
> +	req->cb = cb;
> +	req->userdata = userdata;
> +	req->current_driver = gprs_provision_drivers;
> +
> +	driver = gprs_provision_drivers->data;
> +	DBG("Calling provisioning plugin '%s'", driver->name);

Normally we put empty lines on top and below the DBG statement. Just to
make the stand out a bit.

Also in this case we might can remove some of the debug statements once
the code has been tested.

> +	driver->get_settings(modem, settings_cb, req);
> +	return;
> +
> +error:
> +	cb(NULL, userdata);
> +}
> +
> +static gint compare_priority(gconstpointer a, gconstpointer b)
> +{
> +	const struct ofono_gprs_provision_driver *plugin1 = a;
> +	const struct ofono_gprs_provision_driver *plugin2 = b;
> +
> +	return plugin2->priority - plugin1->priority;
> +}
> +
> +int ofono_gprs_provision_driver_register(const struct ofono_gprs_provision_driver *driver)
> +{
> +	DBG("driver: %p name: %s", driver, driver->name);
> +
> +	gprs_provision_drivers = g_slist_insert_sorted(gprs_provision_drivers,
> +							(void *) driver,
> +							compare_priority);
> +	return 0;
> +}
> +
> +void ofono_gprs_provision_driver_unregister(const struct ofono_gprs_provision_driver *driver)
> +{
> +	DBG("driver: %p name: %s", driver, driver->name);
> +
> +	gprs_provision_drivers = g_slist_remove(gprs_provision_drivers, driver);
> +}

Regards

Marcel

Re: [RFC PATCH 1/4] Added GPRS context provisioning driver API sources

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]

Hi Marcel,

>> +
>> +/*
>> + * Callback from provisioning plugin.
>> + * settings: list of struct ofono_gprs_provisioning_data
>> + *
>> + * It is responsibility of callback function to free settings-list
>> + * settings-list elements must be freed with ofono_gprs_provisioning_data_free()
>> + */
>> +typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
>> +
>> +struct ofono_gprs_provision_driver {
>> +	const char *name;
>> +	int priority;
>> +	void (*get_settings) (struct ofono_modem *modem,
>> +				ofono_gprs_provision_cb_t cb,
>> +				void *userdata);
>> +};
> 
> So here is something we need to talk about. The nettime plugin
> infrastructure using a "pseudo" atom. So do we wanna do the same here or
> do we wanna change the nettime atom to something simple like this.
> 
> I like to have consistency here. Aki, Denis, thoughts?
> 

The nettime and history plugins can create their own D-Bus interfaces.
Hence the need for probe and remove.  Since the provisioning driver
might want to pre-parse its database, I believe it should follow the
same approach...

Regards,
-Denis

[RFC PATCH 2/4] gprs: add automatic context settings provisioning

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 4629 bytes --]

---
 src/gprs.c |  116 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 102 insertions(+), 14 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 58166f8..7d188a3 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -43,6 +43,7 @@
 #include "common.h"
 #include "storage.h"
 #include "idmap.h"
+#include "gprs-provision.h"
 
 #define GPRS_FLAG_ATTACHING 0x1
 #define GPRS_FLAG_RECHECK 0x2
@@ -2454,13 +2455,15 @@ remove:
 		storage_sync(imsi, SETTINGS_STORE, gprs->settings);
 }
 
-void ofono_gprs_register(struct ofono_gprs *gprs)
+static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
 	struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom);
 	const char *path = __ofono_atom_get_path(gprs->atom);
 	struct ofono_atom *netreg_atom;
-	struct ofono_atom *sim_atom;
+
+	if (gprs->contexts == NULL) /* Automatic provisioning failed */
+		add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
 
 	if (!g_dbus_register_interface(conn, path,
 					OFONO_CONNECTION_MANAGER_INTERFACE,
@@ -2475,18 +2478,6 @@ void ofono_gprs_register(struct ofono_gprs *gprs)
 	ofono_modem_add_interface(modem,
 				OFONO_CONNECTION_MANAGER_INTERFACE);
 
-	sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
-
-	if (sim_atom) {
-		struct ofono_sim *sim = __ofono_atom_get_data(sim_atom);
-		const char *imsi = ofono_sim_get_imsi(sim);
-
-		gprs_load_settings(gprs, imsi);
-	}
-
-	if (gprs->contexts == NULL)
-		add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
-
 	gprs->netreg_watch = __ofono_modem_add_atom_watch(modem,
 					OFONO_ATOM_TYPE_NETREG,
 					netreg_watch, gprs, NULL);
@@ -2500,6 +2491,103 @@ void ofono_gprs_register(struct ofono_gprs *gprs)
 	__ofono_atom_register(gprs->atom, gprs_unregister);
 }
 
+static void provision_context(gpointer data, gpointer user_data)
+{
+	struct ofono_gprs_provisioning_data *ap = data;
+	struct ofono_gprs *gprs = user_data;
+
+	unsigned int id;
+	struct pri_context *context = NULL;
+
+	/* Sanity check */
+	if (ap == NULL || ap->name == NULL)
+		return;
+
+	if (gprs->last_context_id)
+		id = idmap_alloc_next(gprs->pid_map, gprs->last_context_id);
+	else
+		id = idmap_alloc(gprs->pid_map);
+
+	if (id > idmap_get_max(gprs->pid_map))
+		return;
+
+	context = pri_context_create(gprs, ap->name, ap->type);
+	DBG("%s context%d '%s' %s", gprs_context_default_name(ap->type),
+		id, ap->name, context ? "created" : "creation failed");
+
+	if (context == NULL)
+		return;
+
+	context->id = id;
+
+	if (ap->username != NULL)
+		strncpy(context->context.username, ap->username,
+			OFONO_GPRS_MAX_USERNAME_LENGTH);
+
+	if (ap->password != NULL)
+		strncpy(context->context.password, ap->password,
+			OFONO_GPRS_MAX_PASSWORD_LENGTH);
+
+	if (ap->apn != NULL)
+		strncpy(context->context.apn, ap->apn,
+			OFONO_GPRS_MAX_APN_LENGTH);
+
+	context->context.proto = ap->proto;
+
+	if (ap->type == OFONO_GPRS_CONTEXT_TYPE_MMS &&
+			ap->message_proxy != NULL)
+		strncpy(context->message_proxy, ap->message_proxy,
+			MAX_MESSAGE_PROXY_LENGTH);
+
+	if (ap->type == OFONO_GPRS_CONTEXT_TYPE_MMS &&
+			ap->message_center != NULL)
+		strncpy(context->message_center, ap->message_center,
+			MAX_MESSAGE_CENTER_LENGTH);
+
+	if (context_dbus_register(context) == TRUE) {
+		gprs->last_context_id = id;
+		gprs->contexts = g_slist_append(gprs->contexts,
+						context);
+		write_context_settings(gprs, context);
+	}
+}
+
+
+static void provision_cb(GSList *settings, void *userdata)
+{
+	struct ofono_gprs *gprs = userdata;
+
+	g_slist_foreach(settings, provision_context, gprs);
+	g_slist_foreach(settings, (GFunc) ofono_gprs_provisioning_data_free,
+			NULL);
+	g_slist_free(settings);
+
+	ofono_gprs_finish_register(gprs);
+}
+
+void ofono_gprs_register(struct ofono_gprs *gprs)
+{
+	struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom);
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
+
+	sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
+
+	if (sim_atom != NULL) {
+		const char *imsi = ofono_sim_get_imsi(sim);
+		sim = __ofono_atom_get_data(sim_atom);
+		gprs_load_settings(gprs, imsi);
+	}
+
+	if (gprs->contexts == NULL) {
+		/* Start settings provisioning */
+		ofono_gprs_provision_get_settings(modem, provision_cb, gprs);
+		return;
+	}
+
+	ofono_gprs_finish_register(gprs);
+}
+
 void ofono_gprs_remove(struct ofono_gprs *gprs)
 {
 	__ofono_atom_free(gprs->atom);
-- 
1.7.1

Re: [RFC PATCH 2/4] gprs: add automatic context settings provisioning

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

Hi Jukka,

> diff --git a/src/gprs.c b/src/gprs.c
> index 58166f8..7d188a3 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -43,6 +43,7 @@
>  #include "common.h"
>  #include "storage.h"
>  #include "idmap.h"
> +#include "gprs-provision.h"
>  
>  #define GPRS_FLAG_ATTACHING 0x1
>  #define GPRS_FLAG_RECHECK 0x2
> @@ -2454,13 +2455,15 @@ remove:
>  		storage_sync(imsi, SETTINGS_STORE, gprs->settings);
>  }
>  
> -void ofono_gprs_register(struct ofono_gprs *gprs)
> +static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
>  {
>  	DBusConnection *conn = ofono_dbus_get_connection();
>  	struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom);
>  	const char *path = __ofono_atom_get_path(gprs->atom);
>  	struct ofono_atom *netreg_atom;
> -	struct ofono_atom *sim_atom;
> +
> +	if (gprs->contexts == NULL) /* Automatic provisioning failed */
> +		add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);

so I see a potential race here between the UI trying to provision and us
trying to provision.

We don't wanna end up with duplicated contexts here. Any ideas on how we
could prevent that nicely? Or should we just fail any attempts to create
new contexts if provisioning is still ongoing?

Regards

Marcel

Re: [RFC PATCH 2/4] gprs: add automatic context settings provisioning

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

Hello Marcel,

On Tue, 2011-01-11 at 22:51 -0800, ext Marcel Holtmann wrote:
> >  
> > -void ofono_gprs_register(struct ofono_gprs *gprs)
> > +static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
> >  {
> >  	DBusConnection *conn = ofono_dbus_get_connection();
> >  	struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom);
> >  	const char *path = __ofono_atom_get_path(gprs->atom);
> >  	struct ofono_atom *netreg_atom;
> > -	struct ofono_atom *sim_atom;
> > +
> > +	if (gprs->contexts == NULL) /* Automatic provisioning failed */
> > +		add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
> 
> so I see a potential race here between the UI trying to provision and us
> trying to provision.
> 
> We don't wanna end up with duplicated contexts here. Any ideas on how we
> could prevent that nicely? Or should we just fail any attempts to create
> new contexts if provisioning is still ongoing?

Code continues there with:
	if (!g_dbus_register_interface(conn, path,
					OFONO_CONNECTION_MANAGER_INTERFACE,
					manager_methods, manager_signals, NULL,
					gprs, NULL)) {
		ofono_error("Could not create %s interface",
				OFONO_CONNECTION_MANAGER_INTERFACE);

		return;
	}

	ofono_modem_add_interface(modem,
				OFONO_CONNECTION_MANAGER_INTERFACE);

Patch rearranges the initial context creation and interface
registration, so there is no DBUS interface for UI to use until
provisioning is ready. Or have misunderstood something?

--Jukka

Re: [RFC PATCH 2/4] gprs: add automatic context settings provisioning

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]

Hi Jukka,

> > > -void ofono_gprs_register(struct ofono_gprs *gprs)
> > > +static void ofono_gprs_finish_register(struct ofono_gprs *gprs)
> > >  {
> > >  	DBusConnection *conn = ofono_dbus_get_connection();
> > >  	struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom);
> > >  	const char *path = __ofono_atom_get_path(gprs->atom);
> > >  	struct ofono_atom *netreg_atom;
> > > -	struct ofono_atom *sim_atom;
> > > +
> > > +	if (gprs->contexts == NULL) /* Automatic provisioning failed */
> > > +		add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET);
> > 
> > so I see a potential race here between the UI trying to provision and us
> > trying to provision.
> > 
> > We don't wanna end up with duplicated contexts here. Any ideas on how we
> > could prevent that nicely? Or should we just fail any attempts to create
> > new contexts if provisioning is still ongoing?
> 
> Code continues there with:
> 	if (!g_dbus_register_interface(conn, path,
> 					OFONO_CONNECTION_MANAGER_INTERFACE,
> 					manager_methods, manager_signals, NULL,
> 					gprs, NULL)) {
> 		ofono_error("Could not create %s interface",
> 				OFONO_CONNECTION_MANAGER_INTERFACE);
> 
> 		return;
> 	}
> 
> 	ofono_modem_add_interface(modem,
> 				OFONO_CONNECTION_MANAGER_INTERFACE);
> 
> Patch rearranges the initial context creation and interface
> registration, so there is no DBUS interface for UI to use until
> provisioning is ready. Or have misunderstood something?

it could we well that I got lost in the re-arranged code ;)

If we wait with the Connection Manager interface registration until we
ran through all provision drivers, that seems fine. So no concerns from
my part then on the potential race conditions with the UI.

Regards

Marcel

[RFC PATCH 3/4] sim: add ofono_sim_get_mnc_length

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1084 bytes --]

---
 include/sim.h |    1 +
 src/sim.c     |    8 ++++++++
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 7860e24..9b21f2e 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -173,6 +173,7 @@ void ofono_sim_set_data(struct ofono_sim *sim, void *data);
 void *ofono_sim_get_data(struct ofono_sim *sim);
 
 const char *ofono_sim_get_imsi(struct ofono_sim *sim);
+unsigned int ofono_sim_get_mnc_length(struct ofono_sim *sim);
 enum ofono_sim_phase ofono_sim_get_phase(struct ofono_sim *sim);
 
 enum ofono_sim_cphs_phase ofono_sim_get_cphs_phase(struct ofono_sim *sim);
diff --git a/src/sim.c b/src/sim.c
index 335f611..e9b1688 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -1913,6 +1913,14 @@ const char *ofono_sim_get_imsi(struct ofono_sim *sim)
 	return sim->imsi;
 }
 
+unsigned int ofono_sim_get_mnc_length(struct ofono_sim *sim)
+{
+	if (sim == NULL)
+		return 0;
+
+	return sim->mnc_length;
+}
+
 enum ofono_sim_phase ofono_sim_get_phase(struct ofono_sim *sim)
 {
 	if (sim == NULL)
-- 
1.7.1

Re: [RFC PATCH 3/4] sim: add ofono_sim_get_mnc_length

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

Hi Jukka,

>  include/sim.h |    1 +
>  src/sim.c     |    8 ++++++++
>  2 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 7860e24..9b21f2e 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -173,6 +173,7 @@ void ofono_sim_set_data(struct ofono_sim *sim, void *data);
>  void *ofono_sim_get_data(struct ofono_sim *sim);
>  
>  const char *ofono_sim_get_imsi(struct ofono_sim *sim);
> +unsigned int ofono_sim_get_mnc_length(struct ofono_sim *sim);

so here is now the question. Do we wanna make this simple and do it like
this or just provide functions to the MCC and MNC like we do inside the
network registration interface?

Denis, any favorites?

Regards

Marcel

Re: [RFC PATCH 3/4] sim: add ofono_sim_get_mnc_length

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Hi Marcel,

On 01/10/2011 06:40 PM, Marcel Holtmann wrote:
> Hi Jukka,
> 
>>  include/sim.h |    1 +
>>  src/sim.c     |    8 ++++++++
>>  2 files changed, 9 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/sim.h b/include/sim.h
>> index 7860e24..9b21f2e 100644
>> --- a/include/sim.h
>> +++ b/include/sim.h
>> @@ -173,6 +173,7 @@ void ofono_sim_set_data(struct ofono_sim *sim, void *data);
>>  void *ofono_sim_get_data(struct ofono_sim *sim);
>>  
>>  const char *ofono_sim_get_imsi(struct ofono_sim *sim);
>> +unsigned int ofono_sim_get_mnc_length(struct ofono_sim *sim);
> 
> so here is now the question. Do we wanna make this simple and do it like
> this or just provide functions to the MCC and MNC like we do inside the
> network registration interface?
> 
> Denis, any favorites?
> 

From an API point of view providing the mnc and mcc directly is a better
idea.

Regards,
-Denis

[RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 6109 bytes --]

---
 Makefile.am          |    3 +
 examples/provision.c |  198 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 examples/provision.c

diff --git a/Makefile.am b/Makefile.am
index 0f330a7..d57509d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -305,6 +305,9 @@ builtin_sources += examples/history.c
 
 builtin_modules += example_nettime
 builtin_sources += examples/nettime.c
+
+builtin_modules += example_gprs_provision
+builtin_sources += examples/provision.c
 endif
 
 builtin_modules += smart_messaging
diff --git a/examples/provision.c b/examples/provision.c
new file mode 100644
index 0000000..f075734
--- /dev/null
+++ b/examples/provision.c
@@ -0,0 +1,198 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2011  Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <glib.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+
+#include <ofono/modem.h>
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/gprs-provision.h>
+#include <ofono/types.h>
+
+#include "ofono.h"
+#include "common.h"
+#include "sim.h"
+#include "simutil.h"
+#include "util.h"
+
+struct provisioning_request {
+	struct ofono_modem *modem;
+	ofono_gprs_provision_cb_t cb;
+	void *userdata;
+};
+
+static void sim_spn_read_cb(int ok, int length, int record,
+				const unsigned char *data,
+				int record_length, void *userdata)
+{
+	struct provisioning_request *req = userdata;
+	char *spn = NULL;
+
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim;
+	const char *imsi;
+
+	unsigned char mnc_length;
+	char mcc[OFONO_MAX_MCC_LENGTH + 1];
+	char mnc[OFONO_MAX_MNC_LENGTH + 1];
+
+	GSList *settings = NULL;
+	struct ofono_gprs_provisioning_data *entry;
+
+	sim_atom = __ofono_modem_find_atom(req->modem, OFONO_ATOM_TYPE_SIM);
+
+	if (sim_atom == NULL) {
+		ofono_debug("No SIM atom");
+		goto finish;
+	}
+
+	sim = __ofono_atom_get_data(sim_atom);
+	imsi = ofono_sim_get_imsi(sim);
+	if (imsi == NULL) {
+		ofono_debug("No IMSI available");
+		goto finish;
+	}
+
+	mnc_length = ofono_sim_get_mnc_length(sim);
+	if (mnc_length == 0) {
+		ofono_debug("No MNC length available");
+		goto finish;
+	}
+
+	strncpy(mcc, imsi, OFONO_MAX_MCC_LENGTH);
+	mcc[OFONO_MAX_MCC_LENGTH] = '\0';
+	strncpy(mnc, imsi + OFONO_MAX_MCC_LENGTH, mnc_length);
+	mnc[mnc_length] = '\0';
+
+	if (ok)
+		spn = sim_string_to_utf8(data + 1, length - 1);
+
+	ofono_debug("Finding settings for MCC %s, MNC %s, SPN '%s'",
+			mcc, mnc, spn);
+
+	if (spn == NULL || strcmp(spn, "oFono") != 0)
+		goto finish;
+
+	/* Example settings for Phonesim */
+	ofono_debug("Creating example settings for phonesim");
+
+	/* MMS context settings */
+	entry = g_try_new0(struct ofono_gprs_provisioning_data, 1);
+	if (entry == NULL)
+		goto finish;
+
+	entry->proto = OFONO_GPRS_PROTO_IP;
+	entry->type = OFONO_GPRS_CONTEXT_TYPE_MMS;
+	entry->name = g_strdup("Phonesim MMS");
+	entry->apn = g_strdup("mmsapn");
+	entry->username = g_strdup("mmsuser");
+	entry->password = g_strdup("mmspass");
+	entry->message_proxy = g_strdup("10.11.12.13:8080");
+	entry->message_center = g_strdup("http://mms.example.com:8000");
+	settings = g_slist_prepend(settings, entry);
+
+	/* Internet context settings */
+	entry = g_try_new0(struct ofono_gprs_provisioning_data, 1);
+	if (entry == NULL)
+		goto finish;
+
+	entry->proto = OFONO_GPRS_PROTO_IP;
+	entry->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
+	entry->name = g_strdup("Phonesim Internet");
+	entry->apn = g_strdup("internetapn");
+	settings = g_slist_prepend(settings, entry);
+
+finish:
+	g_free(spn);
+
+	if (req->cb != NULL)
+		req->cb(settings, req->userdata);
+
+	g_free(req);
+}
+
+
+
+static void example_gprs_provision_get_settings(
+	struct ofono_modem *modem,
+	ofono_gprs_provision_cb_t cb,
+	void *userdata)
+{
+	struct provisioning_request *req;
+	struct ofono_atom *sim_atom;
+	struct ofono_sim *sim = NULL;
+
+	ofono_debug("Provisioning...");
+
+	req = g_try_new0(struct provisioning_request, 1);
+	if (req == NULL)
+		goto error;
+
+	req->modem = modem;
+	req->cb = cb;
+	req->userdata = userdata;
+
+	/* Start SPN query from SIM */
+	sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
+
+	if (sim_atom != NULL)
+		sim = __ofono_atom_get_data(sim_atom);
+
+	if (sim != NULL) {
+		ofono_sim_read(sim, SIM_EFSPN_FILEID,
+				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+				sim_spn_read_cb, req);
+		return;
+	}
+
+error:
+	if (cb != NULL)
+		cb(NULL, userdata);
+}
+
+
+static struct ofono_gprs_provision_driver example_driver = {
+	.name		= "Example GPRS context provisioning",
+	.priority       = OFONO_PLUGIN_PRIORITY_LOW,
+	.get_settings	= example_gprs_provision_get_settings,
+};
+
+static int example_gprs_provision_init(void)
+{
+	return ofono_gprs_provision_driver_register(&example_driver);
+}
+
+static void example_gprs_provision_exit(void)
+{
+	ofono_gprs_provision_driver_unregister(&example_driver);
+}
+
+OFONO_PLUGIN_DEFINE(example_gprs_provision, "Example Network Time Plugin",
+			VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+			example_gprs_provision_init,
+			example_gprs_provision_exit)
-- 
1.7.1

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 3465 bytes --]

Hi Jukka,

>  Makefile.am          |    3 +
>  examples/provision.c |  198 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 201 insertions(+), 0 deletions(-)
>  create mode 100644 examples/provision.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 0f330a7..d57509d 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -305,6 +305,9 @@ builtin_sources += examples/history.c
>  
>  builtin_modules += example_nettime
>  builtin_sources += examples/nettime.c
> +
> +builtin_modules += example_gprs_provision
> +builtin_sources += examples/provision.c
>  endif

I like to have these match. So just call it example_provision.

> +struct provisioning_request {
> +	struct ofono_modem *modem;
> +	ofono_gprs_provision_cb_t cb;
> +	void *userdata;

Same as I mentioned in the other patch. Please use user_data.

> +};
> +
> +static void sim_spn_read_cb(int ok, int length, int record,
> +				const unsigned char *data,
> +				int record_length, void *userdata)
> +{
> +	struct provisioning_request *req = userdata;
> +	char *spn = NULL;
> +
> +	struct ofono_atom *sim_atom;
> +	struct ofono_sim *sim;
> +	const char *imsi;
> +
> +	unsigned char mnc_length;
> +	char mcc[OFONO_MAX_MCC_LENGTH + 1];
> +	char mnc[OFONO_MAX_MNC_LENGTH + 1];
> +
> +	GSList *settings = NULL;
> +	struct ofono_gprs_provisioning_data *entry;
> +
> +	sim_atom = __ofono_modem_find_atom(req->modem, OFONO_ATOM_TYPE_SIM);
> +
> +	if (sim_atom == NULL) {
> +		ofono_debug("No SIM atom");
> +		goto finish;
> +	}
> +
> +	sim = __ofono_atom_get_data(sim_atom);
> +	imsi = ofono_sim_get_imsi(sim);
> +	if (imsi == NULL) {
> +		ofono_debug("No IMSI available");
> +		goto finish;
> +	}
> +
> +	mnc_length = ofono_sim_get_mnc_length(sim);
> +	if (mnc_length == 0) {
> +		ofono_debug("No MNC length available");
> +		goto finish;
> +	}
> +
> +	strncpy(mcc, imsi, OFONO_MAX_MCC_LENGTH);
> +	mcc[OFONO_MAX_MCC_LENGTH] = '\0';
> +	strncpy(mnc, imsi + OFONO_MAX_MCC_LENGTH, mnc_length);
> +	mnc[mnc_length] = '\0';
> +
> +	if (ok)
> +		spn = sim_string_to_utf8(data + 1, length - 1);
> +
> +	ofono_debug("Finding settings for MCC %s, MNC %s, SPN '%s'",
> +			mcc, mnc, spn);
> +
> +	if (spn == NULL || strcmp(spn, "oFono") != 0)
> +		goto finish;

I think we need to also take some MCC and MNC into account. People will
be by accident compiling this plugin and I wanna make sure nothing bad
gets provisioned ;)

> +static void example_gprs_provision_get_settings(
> +	struct ofono_modem *modem,
> +	ofono_gprs_provision_cb_t cb,
> +	void *userdata)
> +{
> +	struct provisioning_request *req;
> +	struct ofono_atom *sim_atom;
> +	struct ofono_sim *sim = NULL;
> +
> +	ofono_debug("Provisioning...");
> +
> +	req = g_try_new0(struct provisioning_request, 1);
> +	if (req == NULL)
> +		goto error;
> +
> +	req->modem = modem;
> +	req->cb = cb;
> +	req->userdata = userdata;
> +
> +	/* Start SPN query from SIM */
> +	sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
> +
> +	if (sim_atom != NULL)
> +		sim = __ofono_atom_get_data(sim_atom);
> +
> +	if (sim != NULL) {
> +		ofono_sim_read(sim, SIM_EFSPN_FILEID,
> +				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +				sim_spn_read_cb, req);
> +		return;
> +	}

Should this not be provided somehow by the SIM atom?

Denis, any idea why we are not keeping this information available?

Regards

Marcel

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

Hello

On Tue, 2011-01-11 at 22:48 -0800, ext Marcel Holtmann wrote:
> > +	if (sim != NULL) {
> > +		ofono_sim_read(sim, SIM_EFSPN_FILEID,
> > +				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> > +				sim_spn_read_cb, req);
> > +		return;
> > +	}
> 
> Should this not be provided somehow by the SIM atom?
> 
> Denis, any idea why we are not keeping this information available?
> 

I would guess there was no need for it, since netreg was the only user
for EFSPN data.

I though of patching the SIM atom to read this during some phase of SIM
initialization, but was not sure exactly when, and what kind of
interface (syncronous, asyncronous?) to provide.

--Jukka

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

Hi Jukka,

On 01/12/2011 01:41 AM, Jukka Saunamaki wrote:
> Hello
> 
> On Tue, 2011-01-11 at 22:48 -0800, ext Marcel Holtmann wrote:
>>> +	if (sim != NULL) {
>>> +		ofono_sim_read(sim, SIM_EFSPN_FILEID,
>>> +				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>>> +				sim_spn_read_cb, req);
>>> +		return;
>>> +	}
>>
>> Should this not be provided somehow by the SIM atom?
>>
>> Denis, any idea why we are not keeping this information available?
>>
> 
> I would guess there was no need for it, since netreg was the only user
> for EFSPN data.
> 
> I though of patching the SIM atom to read this during some phase of SIM
> initialization, but was not sure exactly when, and what kind of
> interface (syncronous, asyncronous?) to provide.
> 

You're correct that netreg is the only consumer.  There was no need for
anyone else to see this information.  However, this begs the question,
why do you need the SPN data?

Regards,
-Denis

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]

Hello

On Wed, 2011-01-12 at 10:46 -0600, Denis Kenzior wrote:
> > ...SIM EF-SPN... 
> 
> You're correct that netreg is the only consumer.  There was no need for
> anyone else to see this information.  However, this begs the question,
> why do you need the SPN data?

Some virtual operators are using the same MCC/MNC as their host, or some
operators have several different trade names, and these can have
different access settings (at least different UI visible name). 
SPN in SIM typically tells these cases apart. This is why I included
reading SPN to that example provisioning.

All provisioning plugins might not care about SPN (e.g. the previously
discussed one using mobile-broadband-provider-info?), so I would suggest
not creating specific SIM API yet. Of cause it can be added later, if so
wished.

--Jukka

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

Hi Jukka,

On 01/13/2011 12:36 AM, Jukka Saunamaki wrote:
> Hello
> 
> On Wed, 2011-01-12 at 10:46 -0600, Denis Kenzior wrote:
>>> ...SIM EF-SPN... 
>>
>> You're correct that netreg is the only consumer.  There was no need for
>> anyone else to see this information.  However, this begs the question,
>> why do you need the SPN data?
> 
> Some virtual operators are using the same MCC/MNC as their host, or some
> operators have several different trade names, and these can have
> different access settings (at least different UI visible name). 
> SPN in SIM typically tells these cases apart. This is why I included
> reading SPN to that example provisioning.

Do you have specific examples?  To my knowledge the MVNOs should be
provisioning the SIM with a different MNC from the host but the network
used (and thus the network's MCC/MNC) are their host's.

> 
> All provisioning plugins might not care about SPN (e.g. the previously
> discussed one using mobile-broadband-provider-info?), so I would suggest
> not creating specific SIM API yet. Of cause it can be added later, if so
> wished.

You might be able to get away with reading of EFspn just because it is
cached nicely on disk.  But you will have to carefully consider your
plugin design if you wish to do so to avoid any race conditions and be
able to properly clean up.

Regards,
-Denis

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]

Hello Denis,

On Thu, 2011-01-13 at 09:57 -0600, Denis Kenzior wrote:
> > Some virtual operators are using the same MCC/MNC as their host, or some
> > operators have several different trade names, and these can have
> > different access settings (at least different UI visible name). 
> > SPN in SIM typically tells these cases apart. This is why I included
> > reading SPN to that example provisioning.
> 
> Do you have specific examples?  To my knowledge the MVNOs should be
> provisioning the SIM with a different MNC from the host but the network
> used (and thus the network's MCC/MNC) are their host's.

I was not sure if all MVNOs have their own MNC, but in any case some
operators use different trade names. Off the top of my hat I know our
local Finnish operators Elisa and Sonera use trade names like Kolumbus
and TeleFinland, and their name shown in UI needs to be correct.

> > All provisioning plugins might not care about SPN (e.g. the previously
> > discussed one using mobile-broadband-provider-info?), so I would suggest
> > not creating specific SIM API yet. Of cause it can be added later, if so
> > wished.
> 
> You might be able to get away with reading of EFspn just because it is
> cached nicely on disk.  But you will have to carefully consider your
> plugin design if you wish to do so to avoid any race conditions and be
> able to properly clean up.

You mean that if plugin gets removed/unregistered before SPN-reading
callback comes in? 
That is a good point, and I also have to check how to handle this in
GPRS atom, since calling provisioning is asynchronous, and GPRS might
get removed while provisioning is running... I might need some help
figuring out solution to that. 
Alternative is of cause to make provisioning synchronous, but that would
limit what plugin can do (like asking SPN with ofono_sim_read())

--Jukka
 

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Friday 14 January 2011 08:53:03 ext Jukka Saunamaki, you wrote:
> Alternative is of cause to make provisioning synchronous, but that would
> limit what plugin can do (like asking SPN with ofono_sim_read())

Just because the plugin would be synchronous won't magically fix the "physical" 
race conditions. The user can still take the SIM out, and the network can 
still fail.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

Hi Jukka,

> > > Some virtual operators are using the same MCC/MNC as their host, or some
> > > operators have several different trade names, and these can have
> > > different access settings (at least different UI visible name). 
> > > SPN in SIM typically tells these cases apart. This is why I included
> > > reading SPN to that example provisioning.
> > 
> > Do you have specific examples?  To my knowledge the MVNOs should be
> > provisioning the SIM with a different MNC from the host but the network
> > used (and thus the network's MCC/MNC) are their host's.
> 
> I was not sure if all MVNOs have their own MNC, but in any case some
> operators use different trade names. Off the top of my hat I know our
> local Finnish operators Elisa and Sonera use trade names like Kolumbus
> and TeleFinland, and their name shown in UI needs to be correct.

what does the name showing in the UI has to do with the provision data
for the GPRS contexts? I am missing your point here.

Regards

Marcel

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

Hi ,

> > I was not sure if all MVNOs have their own MNC, but in any case some
> > operators use different trade names. Off the top of my hat I know our
> > local Finnish operators Elisa and Sonera use trade names like Kolumbus
> > and TeleFinland, and their name shown in UI needs to be correct.
> 
> what does the name showing in the UI has to do with the provision data
> for the GPRS contexts? I am missing your point here.

Isn't the 'name' property of a context shown in some UI (when user
selects connection to open)?
And there might be cases where other context settings like APN is
different with different trade names.

--Jukka

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

Hi Jukka,

> > > I was not sure if all MVNOs have their own MNC, but in any case some
> > > operators use different trade names. Off the top of my hat I know our
> > > local Finnish operators Elisa and Sonera use trade names like Kolumbus
> > > and TeleFinland, and their name shown in UI needs to be correct.
> > 
> > what does the name showing in the UI has to do with the provision data
> > for the GPRS contexts? I am missing your point here.
> 
> Isn't the 'name' property of a context shown in some UI (when user
> selects connection to open)?

so you wanna use the SPN to set the nice friendly name of the Context.
So far we just defaulted to "Internet" and "MMS" and did not bother any
further.

For example ConnMan actually takes that information from the network
registration and not from the context.

> And there might be cases where other context settings like APN is
> different with different trade names.

Is that really the case? Can this be used reliably?

Regards

Marcel

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

Hi

On Fri, 2011-01-14 at 14:44 +0100, ext Marcel Holtmann wrote:
> Hi Jukka,
> 
> > > > I was not sure if all MVNOs have their own MNC, but in any case some
> > > > operators use different trade names. Off the top of my hat I know our
> > > > local Finnish operators Elisa and Sonera use trade names like Kolumbus
> > > > and TeleFinland, and their name shown in UI needs to be correct.
> > > 
> > > what does the name showing in the UI has to do with the provision data
> > > for the GPRS contexts? I am missing your point here.
> > 
> > Isn't the 'name' property of a context shown in some UI (when user
> > selects connection to open)?
> 
> so you wanna use the SPN to set the nice friendly name of the Context.
> So far we just defaulted to "Internet" and "MMS" and did not bother any
> further.

Yes, that is how our previous UIs have done it, and I guess our UX
designers like it in the future too.

> For example ConnMan actually takes that information from the network
> registration and not from the context.
>
> > And there might be cases where other context settings like APN is
> > different with different trade names.
> 
> Is that really the case? Can this be used reliably?

Quick look at our operator database does indeed confirm that, there are
cases where different SPN maps to different APN.

--Jukka

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]

Hi Jukka,

> > > > > I was not sure if all MVNOs have their own MNC, but in any case some
> > > > > operators use different trade names. Off the top of my hat I know our
> > > > > local Finnish operators Elisa and Sonera use trade names like Kolumbus
> > > > > and TeleFinland, and their name shown in UI needs to be correct.
> > > > 
> > > > what does the name showing in the UI has to do with the provision data
> > > > for the GPRS contexts? I am missing your point here.
> > > 
> > > Isn't the 'name' property of a context shown in some UI (when user
> > > selects connection to open)?
> > 
> > so you wanna use the SPN to set the nice friendly name of the Context.
> > So far we just defaulted to "Internet" and "MMS" and did not bother any
> > further.
> 
> Yes, that is how our previous UIs have done it, and I guess our UX
> designers like it in the future too.

I would advise the UI against using the name from the context
information and better using the one from the network registration
instead.

The context gets only provisioned once. So any update of the network
name can not be reflected later on by provisioning. It would have to be
a manual step. And operators are known to changing their names. When
using network registration this would just work fine.

Regards

Marcel

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[=?unknown-8bit?q?R=C3=A9mi?= Denis-Courmont]

[-- Attachment #1: Type: text/plain, Size: 818 bytes --]

On Friday 14 January 2011 16:10:27 ext Marcel Holtmann, you wrote:
> > Yes, that is how our previous UIs have done it, and I guess our UX
> > designers like it in the future too.
> 
> I would advise the UI against using the name from the context
> information and better using the one from the network registration
> instead.

Well that would make sense... Therefore the operators don't do it ;-)

For instance my provider shows as "Elisa" in the network registration but the 
access points are expected to be shown as "Elisa Internet" and "Elisa MMS WAP" 
if I recall correctly.

It only makes sense to have this data in the provisioning plugin. It would be 
silly to have to provision just the names in a different component.

-- 
Rémi Denis-Courmont
Nokia Devices R&D, Maemo Software, Helsinki

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

Hi Remi,

> > > Yes, that is how our previous UIs have done it, and I guess our UX
> > > designers like it in the future too.
> > 
> > I would advise the UI against using the name from the context
> > information and better using the one from the network registration
> > instead.
> 
> Well that would make sense... Therefore the operators don't do it ;-)
> 
> For instance my provider shows as "Elisa" in the network registration but the 
> access points are expected to be shown as "Elisa Internet" and "Elisa MMS WAP" 
> if I recall correctly.
> 
> It only makes sense to have this data in the provisioning plugin. It would be 
> silly to have to provision just the names in a different component.

just to make this clear, oFono takes care of potential operator renames
and displays them accordingly, but the provision names would stay the
same.

So if Elisa renames themselves into "Gwendula", the context names would
still say "Elisa Internet" etc. Has anybody thought about this part?

Regards

Marcel

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Jukka Saunamaki]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

Hello

On Sat, 2011-01-15 at 02:30 +0100, Marcel Holtmann wrote:
> > > I would advise the UI against using the name from the context
> > > information and better using the one from the network registration
> > > instead.
> > 
> > Well that would make sense... Therefore the operators don't do it ;-)
> > 
> > For instance my provider shows as "Elisa" in the network registration but the 
> > access points are expected to be shown as "Elisa Internet" and "Elisa MMS WAP" 
> > if I recall correctly.
> > 
> > It only makes sense to have this data in the provisioning plugin. It would be 
> > silly to have to provision just the names in a different component.
> 
> just to make this clear, oFono takes care of potential operator renames
> and displays them accordingly, but the provision names would stay the
> same.
> 
> So if Elisa renames themselves into "Gwendula", the context names would
> still say "Elisa Internet" etc. Has anybody thought about this part?

Well, there is always OMA Client Provisioning for that purpose.

However, I would leave this question to UX designers and someone that
actually knows operator requirements regarding which name (network
provided or provisioning/gprs-context name) to show for Internet access
selection.
Also, by the way, when roaming, network name is different from normal
home name, and this may or may not be desirable for internet access
name...

In any case, regarding the original issue if reading SPN is really
needed for provisioning, the answer is still yes. There are cases where
different SPN means different gprs context settings (other than 'Name'
too).

--Jukka

Re: [RFC PATCH 4/4] Dummy example GPRS context provisioning driver

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

Hi Jukka,

On 01/14/2011 12:53 AM, Jukka Saunamaki wrote:
> Hello Denis,
> 
> On Thu, 2011-01-13 at 09:57 -0600, Denis Kenzior wrote:
>>> Some virtual operators are using the same MCC/MNC as their host, or some
>>> operators have several different trade names, and these can have
>>> different access settings (at least different UI visible name). 
>>> SPN in SIM typically tells these cases apart. This is why I included
>>> reading SPN to that example provisioning.
>>
>> Do you have specific examples?  To my knowledge the MVNOs should be
>> provisioning the SIM with a different MNC from the host but the network
>> used (and thus the network's MCC/MNC) are their host's.
> 
> I was not sure if all MVNOs have their own MNC, but in any case some
> operators use different trade names. Off the top of my hat I know our
> local Finnish operators Elisa and Sonera use trade names like Kolumbus
> and TeleFinland, and their name shown in UI needs to be correct.
> 

Being shown on the UI is a bit different from needing this information
to provision.  oFono already takes care of reading EFspn in the netreg
atom and figuring out the proper display name.

If you can rely on just the MCC/MNC of the SIM/IMSI for provisioning
information, that would greatly simplify your database and your plugin
implementation.

So I suggest you double check the need of EFspn first.

Regards,
-Denis