[PATCH 1/2] netreg: Add CPHS CSP implementation

[PATCH 1/2] netreg: Add CPHS CSP implementation

[Aki Niemi]

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

---
 src/network.c |  121 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 95 insertions(+), 26 deletions(-)

diff --git a/src/network.c b/src/network.c
index bf61472..ef0b37f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -42,7 +42,8 @@
 
 enum network_registration_mode {
 	NETWORK_REGISTRATION_MODE_AUTO =	0,
-	NETWORK_REGISTRATION_MODE_MANUAL =	1,
+	NETWORK_REGISTRATION_MODE_AUTO_ONLY =	1,
+	NETWORK_REGISTRATION_MODE_MANUAL =	2,
 };
 
 #define SETTINGS_STORE "netreg"
@@ -98,6 +99,8 @@ static const char *registration_mode_to_string(int mode)
 	switch (mode) {
 	case NETWORK_REGISTRATION_MODE_AUTO:
 		return "auto";
+	case NETWORK_REGISTRATION_MODE_AUTO_ONLY:
+		return "auto-only";
 	case NETWORK_REGISTRATION_MODE_MANUAL:
 		return "manual";
 	}
@@ -143,6 +146,42 @@ static char **network_operator_technologies(struct network_operator_data *opd)
 	return techs;
 }
 
+static void registration_status_callback(const struct ofono_error *error,
+					int status, int lac, int ci, int tech,
+					void *data)
+{
+	struct ofono_netreg *netreg = data;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		DBG("Error during registration status query");
+		return;
+	}
+
+	ofono_netreg_status_notify(netreg, status, lac, ci, tech);
+}
+
+static void init_register(const struct ofono_error *error, void *data)
+{
+	struct ofono_netreg *netreg = data;
+
+	if (netreg->driver->registration_status == NULL)
+		return;
+
+	netreg->driver->registration_status(netreg,
+					registration_status_callback, netreg);
+}
+
+static void enforce_auto_only(struct ofono_netreg *netreg)
+{
+	if (netreg->mode != NETWORK_REGISTRATION_MODE_MANUAL)
+		return;
+
+	if (netreg->driver->register_auto == NULL)
+		return;
+
+	netreg->driver->register_auto(netreg, init_register, netreg);
+}
+
 static void set_registration_mode(struct ofono_netreg *netreg, int mode)
 {
 	DBusConnection *conn;
@@ -152,6 +191,9 @@ static void set_registration_mode(struct ofono_netreg *netreg, int mode)
 	if (netreg->mode == mode)
 		return;
 
+	if (mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+		enforce_auto_only(netreg);
+
 	netreg->mode = mode;
 
 	if (netreg->settings) {
@@ -170,20 +212,6 @@ static void set_registration_mode(struct ofono_netreg *netreg, int mode)
 					"Mode", DBUS_TYPE_STRING, &strmode);
 }
 
-static void registration_status_callback(const struct ofono_error *error,
-					int status, int lac, int ci, int tech,
-					void *data)
-{
-	struct ofono_netreg *netreg = data;
-
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		DBG("Error during registration status query");
-		return;
-	}
-
-	ofono_netreg_status_notify(netreg, status, lac, ci, tech);
-}
-
 static void register_callback(const struct ofono_error *error, void *data)
 {
 	struct ofono_netreg *netreg = data;
@@ -211,15 +239,6 @@ out:
 					registration_status_callback, netreg);
 }
 
-static void init_register(const struct ofono_error *error, void *data)
-{
-	struct ofono_netreg *netreg = data;
-
-	if (netreg->driver->registration_status)
-		netreg->driver->registration_status(netreg,
-					registration_status_callback, netreg);
-}
-
 static struct network_operator_data *
 	network_operator_create(const struct ofono_network_operator *op)
 {
@@ -586,6 +605,9 @@ static DBusMessage *network_operator_register(DBusConnection *conn,
 	struct network_operator_data *opd = data;
 	struct ofono_netreg *netreg = opd->netreg;
 
+	if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+		return __ofono_error_access_denied(msg);
+
 	if (netreg->pending)
 		return __ofono_error_busy(msg);
 
@@ -838,6 +860,9 @@ static DBusMessage *network_register(DBusConnection *conn,
 
 	netreg->driver->register_auto(netreg, register_callback, netreg);
 
+	if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+		return NULL;
+
 	set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);
 
 	return NULL;
@@ -947,6 +972,9 @@ static DBusMessage *network_scan(DBusConnection *conn,
 {
 	struct ofono_netreg *netreg = data;
 
+	if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
+		return __ofono_error_access_denied(msg);
+
 	if (netreg->pending)
 		return __ofono_error_busy(msg);
 
@@ -1364,7 +1392,7 @@ static void init_registration_status(const struct ofono_error *error,
 					signal_strength_callback, netreg);
 	}
 
-	if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO &&
+	if (netreg->mode != NETWORK_REGISTRATION_MODE_MANUAL &&
 		(status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED ||
 			status == NETWORK_REGISTRATION_STATUS_DENIED ||
 			status == NETWORK_REGISTRATION_STATUS_UNKNOWN)) {
@@ -1574,6 +1602,44 @@ static void sim_spn_read_cb(int ok, int length, int record,
 	}
 }
 
+static void sim_csp_read_cb(int ok, int length, int record,
+				const unsigned char *data,
+				int record_length, void *user_data)
+{
+	struct ofono_netreg *netreg = user_data;
+	int i;
+
+	if (!ok)
+		return;
+
+	if (length < 18 || record_length < 18 || length < record_length)
+		return;
+
+	/*
+	 * According to CPHS 4.2, EFcsp is an array of two-byte service
+	 * entries, each consisting of a one byte service group
+	 * identifier followed by 8 bits; each bit is indicating
+	 * availability of a specific service or feature.
+	 *
+	 * The PLMN mode bit, if present, indicates whether manual
+	 * operator selection should be disabled or enabled. When
+	 * unset, the device is forced to automatic mode; when set,
+	 * manual selection is to be enabled. The latter is also the
+	 * default.
+	 */
+	for (i = 0; i < record_length / 2; i++) {
+
+		if (data[i * 2] != SIM_CSP_ENTRY_VALUE_ADDED_SERVICES)
+			continue;
+
+		if ((data[i * 2 + 1] & 0x80) != 0)
+			return;
+
+		set_registration_mode(netreg,
+					NETWORK_REGISTRATION_MODE_AUTO_ONLY);
+	}
+}
+
 int ofono_netreg_get_location(struct ofono_netreg *netreg)
 {
 	if (netreg == NULL)
@@ -1769,7 +1835,7 @@ static void netreg_load_settings(struct ofono_netreg *netreg)
 	mode = g_key_file_get_integer(netreg->settings, SETTINGS_GROUP,
 					"Mode", NULL);
 
-	if (mode >= 0 && mode <= 1)
+	if (mode != NETWORK_REGISTRATION_MODE_AUTO_ONLY)
 		netreg->mode = mode;
 
 	g_key_file_set_integer(netreg->settings, SETTINGS_GROUP,
@@ -1816,6 +1882,9 @@ void ofono_netreg_register(struct ofono_netreg *netreg)
 		ofono_sim_read(netreg->sim, SIM_EFSPN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 				sim_spn_read_cb, netreg);
+		ofono_sim_read(netreg->sim, SIM_EF_CPHS_CSP_FILEID,
+				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+				sim_csp_read_cb, netreg);
 	}
 
 	__ofono_atom_register(netreg->atom, netreg_unregister);
-- 
1.7.1

[PATCH 2/2] TODO: Remove completed CPHS CSP task

[Aki Niemi]

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

---
 TODO |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/TODO b/TODO
index 13a6baa..82066fa 100644
--- a/TODO
+++ b/TODO
@@ -88,19 +88,6 @@ SIM / SIM File system
   Complexity: C1
   Owner: Marit Henriksen <marit.henriksen@stericsson.com>
 
-- Add support for CPHS Customer Service Profile (CSP).  This task adds support
-  for reading the EFcsp file and in particular, interpreting the PLMN mode bit
-  within the 'Value Added Services' service group.
-
-  Based on the PLMN mode value, manual network selection needs to either be
-  enabled or disabled in the netreg atom.  Also to accommodate this, there
-  probably needs to be a new 'forced' mode to indicate that manual network
-  selection is disabled.
-
-  Priority: Medium
-  Complexity: C1
-  Owner: Aki Niemi <aki.niemi@nokia.com>
-
 - Support SIM authentication: SIM and AKA suites.
 
   Priority: Medium
-- 
1.7.1

Re: [PATCH 1/2] netreg: Add CPHS CSP implementation

[Denis Kenzior]

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

Hi Aki,

> @@ -838,6 +860,9 @@ static DBusMessage *network_register(DBusConnection *conn,
>  
>  	netreg->driver->register_auto(netreg, register_callback, netreg);
>  
> +	if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
> +		return NULL;
> +

So we might just return access_denied here as well.

>  	set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);
>  
>  	return NULL;

<snip>

> @@ -1769,7 +1835,7 @@ static void netreg_load_settings(struct ofono_netreg *netreg)
>  	mode = g_key_file_get_integer(netreg->settings, SETTINGS_GROUP,
>  					"Mode", NULL);
>  
> -	if (mode >= 0 && mode <= 1)
> +	if (mode != NETWORK_REGISTRATION_MODE_AUTO_ONLY)
>  		netreg->mode = mode;
>  

This check is unnecessary, you already loaded the settings before you
managed to load the sim file.  You're also removing a sanity check for
no real reason.

This brings up another point, since the CPHS CSP file is actually
user-writeable (for some unknown reason) the user can remove the SIM,
put it in another phone and modify the forced-auto settings.  If you
store the forced-auto mode in the settings file, then there's no way to
ever go back to non-forced auto, even though he should be able to.

I suggest that when forced-auto is detected, that we only write
NETWORK_REGISTRATION_MODE_AUTO to the settings store.

Regards,
-Denis

Re: [PATCH 1/2] netreg: Add CPHS CSP implementation

[Aki Niemi]

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

Hi Denis,

2011/2/3 Denis Kenzior <denkenz@gmail.com>:
>> +     if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
>> +             return NULL;
>> +
>
> So we might just return access_denied here as well.

There is a reason for allowing this, namely that sometimes you want to
re-run the logic for automatically operator selection. Granted, this
is not your most common use case, but since it also does no harm to
allowing it, I didn't.

>>       set_registration_mode(netreg, NETWORK_REGISTRATION_MODE_AUTO);
>>
>>       return NULL;
>
> <snip>
>
>> @@ -1769,7 +1835,7 @@ static void netreg_load_settings(struct ofono_netreg *netreg)
>>       mode = g_key_file_get_integer(netreg->settings, SETTINGS_GROUP,
>>                                       "Mode", NULL);
>>
>> -     if (mode >= 0 && mode <= 1)
>> +     if (mode != NETWORK_REGISTRATION_MODE_AUTO_ONLY)
>>               netreg->mode = mode;
>>
>
> This check is unnecessary, you already loaded the settings before you
> managed to load the sim file.  You're also removing a sanity check for
> no real reason.

That's just a bug. The intention was to write:

if (mode == NEtWORK_REGISTRATION_MODE_AUTO ||
                mode == NETWORK_REGISTRATION_MODE_MANUAL)
        netreg->mode = mode;

The patch has a seemingly shorter way of describing the same, but as
you point out, misses the sanity check. Must have been lack of coffee
or something. ;)

> This brings up another point, since the CPHS CSP file is actually
> user-writeable (for some unknown reason) the user can remove the SIM,
> put it in another phone and modify the forced-auto settings.  If you
> store the forced-auto mode in the settings file, then there's no way to
> ever go back to non-forced auto, even though he should be able to.

I think the above way will in fact default to auto even if the
settings store has auto-only. And yes, EFcsp can definitely change
even at run-time, as manual mode really should be allowed when
roaming, at least in countries that the operator for instance has
multiple suitable roaming partners with varying coverage.

> I suggest that when forced-auto is detected, that we only write
> NETWORK_REGISTRATION_MODE_AUTO to the settings store.

That sounds reasonable, too.

Cheers,
Aki

Re: [PATCH 1/2] netreg: Add CPHS CSP implementation

[Denis Kenzior]

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

Hi Aki,

On 02/03/2011 03:22 PM, Aki Niemi wrote:
> Hi Denis,
> 
> 2011/2/3 Denis Kenzior <denkenz@gmail.com>:
>>> +     if (netreg->mode == NETWORK_REGISTRATION_MODE_AUTO_ONLY)
>>> +             return NULL;
>>> +
>>
>> So we might just return access_denied here as well.
> 
> There is a reason for allowing this, namely that sometimes you want to
> re-run the logic for automatically operator selection. Granted, this
> is not your most common use case, but since it also does no harm to
> allowing it, I didn't.
> 

This might not really work on all modem firmware though.  I'm not too
sure you want to do this.

>> This brings up another point, since the CPHS CSP file is actually
>> user-writeable (for some unknown reason) the user can remove the SIM,
>> put it in another phone and modify the forced-auto settings.  If you
>> store the forced-auto mode in the settings file, then there's no way to
>> ever go back to non-forced auto, even though he should be able to.
> 
> I think the above way will in fact default to auto even if the
> settings store has auto-only. And yes, EFcsp can definitely change
> even at run-time, as manual mode really should be allowed when
> roaming, at least in countries that the operator for instance has
> multiple suitable roaming partners with varying coverage.

CPHS 4.2 only says that the manual mode selection should be disabled, so
I'm assuming this magic happens via STK Refresh.  If so, then it might
be worth it to go back from forced-auto (if set) to auto when reading
the CSP file.

Regards,
-Denis