[PATCH 0/3] EF-SPN API to sim-atom

[PATCH 0/3] EF-SPN API to sim-atom

[Jukka Saunamaki]

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

Hello

Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
The trick is to delay setting sim ready until spn reading is finished.

Patch to use the new API in netreg is included.

--Jukka

Jukka Saunamaki (3):
      sim: add ofono_sim_get_spn API header
      sim: add ofono_sim_get_spn API implementation
      netreg: use ofono_sim_get_spn()

 include/sim.h |    2 +
 src/network.c |   46 +++++------------------------------
 src/sim.c     |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 81 insertions(+), 40 deletions(-)

[PATCH 1/3] sim: add ofono_sim_get_spn API header

[Jukka Saunamaki]

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

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

diff --git a/include/sim.h b/include/sim.h
index 5e3ba5b..7e573fd 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -180,6 +180,8 @@ void *ofono_sim_get_data(struct ofono_sim *sim);
 const char *ofono_sim_get_imsi(struct ofono_sim *sim);
 const char *ofono_sim_get_mcc(struct ofono_sim *sim);
 const char *ofono_sim_get_mnc(struct ofono_sim *sim);
+const char *ofono_sim_get_spn(struct ofono_sim *sim);
+unsigned char ofono_sim_get_spn_dc(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);
-- 
1.7.1

[PATCH 2/3] sim: add ofono_sim_get_spn API implementation

[Jukka Saunamaki]

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

---
 src/sim.c |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 72 insertions(+), 1 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 3c5db90..2c09c11 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -80,6 +80,8 @@ struct ofono_sim {
 	char *imsi;
 	char mcc[OFONO_MAX_MCC_LENGTH + 1];
 	char mnc[OFONO_MAX_MNC_LENGTH + 1];
+	char *spn;
+	unsigned char spn_dc;
 
 	GSList *own_numbers;
 	GSList *new_numbers;
@@ -1257,6 +1259,49 @@ static void sim_efimg_read_cb(int ok, int length, int record,
 	memcpy(efimg, &data[1], 9);
 }
 
+static void sim_spn_read_cb(int ok, int length, int record,
+				const unsigned char *data,
+				int record_length, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+
+	if (!ok)
+		goto finish;
+
+	sim->spn_dc = data[0];
+
+	/*
+	 * TS 31.102 says:
+	 *
+	 * the string shall use:
+	 *
+	 * - either the SMS default 7-bit coded alphabet as defined in
+	 *   TS 23.038 [5] with bit 8 set to 0. The string shall be left
+	 *   justified. Unused bytes shall be set to 'FF'.
+	 *
+	 * - or one of the UCS2 code options defined in the annex of TS
+	 *   31.101 [11].
+	 *
+	 * 31.101 has no such annex though.  51.101 refers to Annex B of
+	 * itself which is not there either.  11.11 contains the same
+	 * paragraph as 51.101 and has an Annex B which we implement.
+	 */
+	sim->spn = sim_string_to_utf8(data + 1, length - 1);
+	if (sim->spn == NULL) {
+		ofono_error("EFspn read successfully, but couldn't parse");
+		goto finish;
+	}
+
+	if (strlen(sim->spn) == 0) {
+		g_free(sim->spn);
+		sim->spn = NULL;
+	}
+
+finish:
+	sim_set_ready(sim);
+}
+
+
 static void sim_ready(enum ofono_sim_state new_state, void *user)
 {
 	struct ofono_sim *sim = user;
@@ -1313,7 +1358,10 @@ static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
 						DBUS_TYPE_STRING, &str);
 	}
 
-	sim_set_ready(sim);
+	/* Try to get SPN before declaring SIM ready */
+	ofono_sim_read(sim, SIM_EFSPN_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_spn_read_cb, sim);
 }
 
 static void sim_retrieve_imsi(struct ofono_sim *sim)
@@ -2005,6 +2053,22 @@ const char *ofono_sim_get_mnc(struct ofono_sim *sim)
 	return sim->mnc;
 }
 
+const char *ofono_sim_get_spn(struct ofono_sim *sim)
+{
+	if (sim == NULL)
+		return NULL;
+
+	return sim->spn;
+}
+
+unsigned char ofono_sim_get_spn_dc(struct ofono_sim *sim)
+{
+	if (sim == NULL)
+		return 0;
+
+	return sim->spn_dc;
+}
+
 enum ofono_sim_phase ofono_sim_get_phase(struct ofono_sim *sim)
 {
 	if (sim == NULL)
@@ -2071,6 +2135,13 @@ static void sim_free_state(struct ofono_sim *sim)
 	sim->mcc[0] = '\0';
 	sim->mnc[0] = '\0';
 
+	if (sim->spn) {
+		g_free(sim->spn);
+		sim->spn = NULL;
+	}
+
+	sim->spn_dc = 0;
+
 	if (sim->own_numbers) {
 		g_slist_foreach(sim->own_numbers, (GFunc)g_free, NULL);
 		g_slist_free(sim->own_numbers);
-- 
1.7.1

[PATCH 3/3] netreg: use ofono_sim_get_spn()

[Jukka Saunamaki]

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

---
 src/network.c |   46 +++++++---------------------------------------
 1 files changed, 7 insertions(+), 39 deletions(-)

diff --git a/src/network.c b/src/network.c
index b5450ee..f602db4 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1507,51 +1507,21 @@ static void sim_spdi_read_cb(int ok, int length, int record,
 	}
 }
 
-static void sim_spn_read_cb(int ok, int length, int record,
-				const unsigned char *data,
-				int record_length, void *user_data)
+static void get_spn(struct ofono_netreg *netreg)
 {
-	struct ofono_netreg *netreg = user_data;
 	unsigned char dcbyte;
-	char *spn;
-
-	if (!ok)
-		return;
-
-	dcbyte = data[0];
+	const char *spn = ofono_sim_get_spn(netreg->sim);
 
-	/*
-	 * TS 31.102 says:
-	 *
-	 * the string shall use:
-	 *
-	 * - either the SMS default 7-bit coded alphabet as defined in
-	 *   TS 23.038 [5] with bit 8 set to 0. The string shall be left
-	 *   justified. Unused bytes shall be set to 'FF'.
-	 *
-	 * - or one of the UCS2 code options defined in the annex of TS
-	 *   31.101 [11].
-	 *
-	 * 31.101 has no such annex though.  51.101 refers to Annex B of
-	 * itself which is not there either.  11.11 contains the same
-	 * paragraph as 51.101 and has an Annex B which we implement.
-	 */
-	spn = sim_string_to_utf8(data + 1, length - 1);
-	if (spn == NULL) {
-		ofono_error("EFspn read successfully, but couldn't parse");
+	if (spn == NULL)
 		return;
-	}
 
-	if (strlen(spn) == 0) {
-		g_free(spn);
-		return;
-	}
-
-	netreg->spname = spn;
+	netreg->spname = g_strdup(spn);
 	ofono_sim_read(netreg->sim, SIM_EFSPDI_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 			sim_spdi_read_cb, netreg);
 
+	dcbyte = ofono_sim_get_spn_dc(netreg->sim);
+
 	if (dcbyte & SIM_EFSPN_DC_HOME_PLMN_BIT)
 		netreg->flags |= NETWORK_REGISTRATION_FLAG_HOME_SHOW_PLMN;
 
@@ -1816,9 +1786,7 @@ void ofono_netreg_register(struct ofono_netreg *netreg)
 		ofono_sim_read(netreg->sim, SIM_EFPNN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_FIXED,
 				sim_pnn_read_cb, netreg);
-		ofono_sim_read(netreg->sim, SIM_EFSPN_FILEID,
-				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
-				sim_spn_read_cb, netreg);
+		get_spn(netreg);
 	}
 
 	__ofono_atom_register(netreg->atom, netreg_unregister);
-- 
1.7.1

Re: [PATCH 1/3] sim: add ofono_sim_get_spn API header

[Marcel Holtmann]

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

Hi Jukka,

>  include/sim.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 5e3ba5b..7e573fd 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -180,6 +180,8 @@ void *ofono_sim_get_data(struct ofono_sim *sim);
>  const char *ofono_sim_get_imsi(struct ofono_sim *sim);
>  const char *ofono_sim_get_mcc(struct ofono_sim *sim);
>  const char *ofono_sim_get_mnc(struct ofono_sim *sim);
> +const char *ofono_sim_get_spn(struct ofono_sim *sim);
> +unsigned char ofono_sim_get_spn_dc(struct ofono_sim *sim);

what is the spn_dc for? We are converting everything into UTF-8 anyway
and that should be sufficient.

Regards

Marcel

Re: [PATCH 1/3] sim: add ofono_sim_get_spn API header

[Jukka Saunamaki]

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

Hi

On Thu, 2011-01-27 at 14:56 +0100, ext Marcel Holtmann wrote:
> > +const char *ofono_sim_get_spn(struct ofono_sim *sim);
> > +unsigned char ofono_sim_get_spn_dc(struct ofono_sim *sim);
> 
> what is the spn_dc for? We are converting everything into UTF-8 anyway
> and that should be sufficient.

That is "Display Condition" byte of EF-SPN. Needed by netreg. Perhaps
better name would have been spn_flags.

--Jukka

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Jukka Saunamaki]

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

Hello

On Thu, 2011-01-27 at 15:22 +0200, Jukka Saunamaki wrote:
> Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
> The trick is to delay setting sim ready until spn reading is finished.

Any comments about this? 
So, yes, it slightly delays SIM initialisation (usually SPN value should
come from disk cache), but then netreg can use it immediately when it
registers. 
Alternative is still to create an asyncronous API now that there is a
patch for safe SIM file reading.

--Jukka

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Marcel Holtmann]

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

Hi Jukka,

> > Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
> > The trick is to delay setting sim ready until spn reading is finished.
> 
> Any comments about this? 
> So, yes, it slightly delays SIM initialisation (usually SPN value should
> come from disk cache), but then netreg can use it immediately when it
> registers. 

I am actually fine with this. Denis had a concern about it. Check with
him.

Regards

Marcel

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Denis Kenzior]

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

Hi Jukka,

On 01/31/2011 12:13 AM, Jukka Saunamaki wrote:
> Hello
> 
> On Thu, 2011-01-27 at 15:22 +0200, Jukka Saunamaki wrote:
>> Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
>> The trick is to delay setting sim ready until spn reading is finished.
> 
> Any comments about this? 
> So, yes, it slightly delays SIM initialisation (usually SPN value should
> come from disk cache), but then netreg can use it immediately when it
> registers. 
> Alternative is still to create an asyncronous API now that there is a
> patch for safe SIM file reading.

So my main problem here is that you require the SPN a handful of times
in the phone's lifetime, more likely once.  Yet you pay the cost of
reading and delaying the initialization every time.  In my view this is
not really acceptable.

Perhaps a better approach would be to put the spn parser into simutil so
it can be shared by netreg and gprs.  Then go back to your original
proposal of delaying the gprs atom registration if provisioning is
required and SPN is needed.

Regards,
-Denis

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Jukka Saunamaki]

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

Hi Denis, 

On Mon, 2011-01-31 at 14:22 -0600, Denis Kenzior wrote:
> So my main problem here is that you require the SPN a handful of times
> in the phone's lifetime, more likely once.  Yet you pay the cost of
> reading and delaying the initialization every time.  In my view this is
> not really acceptable.
> 
> Perhaps a better approach would be to put the spn parser into simutil so
> it can be shared by netreg and gprs.  Then go back to your original
> proposal of delaying the gprs atom registration if provisioning is
> required and SPN is needed.

OK.

SPN parser is basically a line 
spn = sim_string_to_utf8(data + 1, length - 1);
so I do not think it really needs a separate simutil routine.

For reading SPN in gprs atom registration, it would make sense to make
it with the new sim_context api. Any estimates when that will be in?

--Jukka
 

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Marcel Holtmann]

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

Hi Denis,

> > On Thu, 2011-01-27 at 15:22 +0200, Jukka Saunamaki wrote:
> >> Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
> >> The trick is to delay setting sim ready until spn reading is finished.
> > 
> > Any comments about this? 
> > So, yes, it slightly delays SIM initialisation (usually SPN value should
> > come from disk cache), but then netreg can use it immediately when it
> > registers. 
> > Alternative is still to create an asyncronous API now that there is a
> > patch for safe SIM file reading.
> 
> So my main problem here is that you require the SPN a handful of times
> in the phone's lifetime, more likely once.  Yet you pay the cost of
> reading and delaying the initialization every time.  In my view this is
> not really acceptable.

does it really matter in the end? It gets read once and after that we
are caching it on disk anyway.

Regards

Marcel

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Denis Kenzior]

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

Hi Marcel,

On 02/01/2011 04:38 AM, Marcel Holtmann wrote:
> Hi Denis,
> 
>>> On Thu, 2011-01-27 at 15:22 +0200, Jukka Saunamaki wrote:
>>>> Here is an asynchronous implementation of SIM Service Provider Name (EF-SPN) getter API.
>>>> The trick is to delay setting sim ready until spn reading is finished.
>>>
>>> Any comments about this? 
>>> So, yes, it slightly delays SIM initialisation (usually SPN value should
>>> come from disk cache), but then netreg can use it immediately when it
>>> registers. 
>>> Alternative is still to create an asyncronous API now that there is a
>>> patch for safe SIM file reading.
>>
>> So my main problem here is that you require the SPN a handful of times
>> in the phone's lifetime, more likely once.  Yet you pay the cost of
>> reading and delaying the initialization every time.  In my view this is
>> not really acceptable.
> 
> does it really matter in the end? It gets read once and after that we
> are caching it on disk anyway.
> 

Of course it does, while SIM is initializing everything else is blocked.
 Once we're in sim ready you can have n atoms initializing at the same
time.  On good hardware with proper muxing that can shave tremendous
amount of time off your startup cost.

Regards,
-Denis

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Aki Niemi]

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

Hi Denis,

On Tue, 2011-02-01 at 09:14 -0600, ext Denis Kenzior wrote:
> > does it really matter in the end? It gets read once and after that we
> > are caching it on disk anyway.
> > 
> 
> Of course it does, while SIM is initializing everything else is blocked.
>  Once we're in sim ready you can have n atoms initializing at the same
> time.  On good hardware with proper muxing that can shave tremendous
> amount of time off your startup cost.

I agree.

There is also this weird corner case when the SIM has no SPN available.
AFAIK, a failed read isn't cached, so bootstrapping that particular SIM
will incur a penalty every single time for information that isn't even
available, as opposed to the GPRS atom incurring it just once during its
lifetime.

That reminds me, I think there are SIMs out there that use the CPHS ONS
instead of SPN, and these can be used interchangeably.

Jukka, don't you actually also need the ONS?

Cheers,
Aki

> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Denis Kenzior]

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

Hi Aki,

On 02/01/2011 02:38 PM, Aki Niemi wrote:
> Hi Denis,
> 
> On Tue, 2011-02-01 at 09:14 -0600, ext Denis Kenzior wrote:
>>> does it really matter in the end? It gets read once and after that we
>>> are caching it on disk anyway.
>>>
>>
>> Of course it does, while SIM is initializing everything else is blocked.
>>  Once we're in sim ready you can have n atoms initializing at the same
>> time.  On good hardware with proper muxing that can shave tremendous
>> amount of time off your startup cost.
> 
> I agree.
> 
> There is also this weird corner case when the SIM has no SPN available.
> AFAIK, a failed read isn't cached, so bootstrapping that particular SIM
> will incur a penalty every single time for information that isn't even
> available, as opposed to the GPRS atom incurring it just once during its
> lifetime.

The proposal from Jukka reads EFspn from the sim atom before obtaining
the IMSI, so no caching is possible anyway.  But you are right that we
do not cache if an error occurs during a file info request, so that is
something else to watch out for.

Regards,
-Denis

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Jukka Saunamaki]

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

Hi

On Tue, 2011-02-01 at 22:38 +0200, ext Aki Niemi wrote:
> That reminds me, I think there are SIMs out there that use the CPHS ONS
> instead of SPN, and these can be used interchangeably.
> 
> Jukka, don't you actually also need the ONS?

In theory maybe, but in practice so far all cases where operator/service
provider name is needed for provisioning have had SPN.

--Jukka
 

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Jukka Saunamaki]

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

Hi Denis,

On Tue, 2011-02-01 at 19:20 -0600, ext Denis Kenzior wrote:
> > There is also this weird corner case when the SIM has no SPN available.
> > AFAIK, a failed read isn't cached, so bootstrapping that particular SIM
> > will incur a penalty every single time for information that isn't even
> > available, as opposed to the GPRS atom incurring it just once during its
> > lifetime.
> 
> The proposal from Jukka reads EFspn from the sim atom before obtaining
> the IMSI, so no caching is possible anyway.  But you are right that we
> do not cache if an error occurs during a file info request, so that is
> something else to watch out for.

Actually the patch does read SPN after IMSI.

But anyway, I agree, reading SPN is better done asynchronously in gprs
atom, since this provisioning is done only once.  

--Jukka

Re: [PATCH 0/3] EF-SPN API to sim-atom

[Denis Kenzior]

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

Hi Jukka,

On 02/02/2011 01:19 AM, Jukka Saunamaki wrote:
> Hi Denis,
> 
> On Tue, 2011-02-01 at 19:20 -0600, ext Denis Kenzior wrote:
>>> There is also this weird corner case when the SIM has no SPN available.
>>> AFAIK, a failed read isn't cached, so bootstrapping that particular SIM
>>> will incur a penalty every single time for information that isn't even
>>> available, as opposed to the GPRS atom incurring it just once during its
>>> lifetime.
>>
>> The proposal from Jukka reads EFspn from the sim atom before obtaining
>> the IMSI, so no caching is possible anyway.  But you are right that we
>> do not cache if an error occurs during a file info request, so that is
>> something else to watch out for.
> 
> Actually the patch does read SPN after IMSI.
> 

Ah yes you're right.

FYI: we have maintained an unwritten rule that the presence of IMSI
indicates the sim is initialized to D-Bus clients; so it should be
signaled immediately before post_sim atoms are added.

Regards,
-Denis