diff mbox series

[OpenWrt-Devel,uqmi] nas: add --get-plmn

Message ID 20190704113537.22078-1-ms@dev.tdt.de
State Under Review
Delegated to: Koen Vandeputte
Headers show
Series [OpenWrt-Devel,uqmi] nas: add --get-plmn | expand

Commit Message

Martin Schiller July 4, 2019, 11:35 a.m. UTC
This command is needed in the qmi proto handler to check if the plmn
is already set to 'auto'.

The reason for this is, that setting the plmn to 'auto' will implicitly
lead to a (delayed) network re-registration, which could further lead
to some timing related issues in the qmi proto handler.

Signed-off-by: Martin Schiller <ms@dev.tdt.de>
---
 commands-nas.c | 31 +++++++++++++++++++++++++++++++
 commands-nas.h |  2 ++
 2 files changed, 33 insertions(+)

Comments

Martin Schiller Aug. 26, 2019, 10:48 a.m. UTC | #1
Can somebody please take a look at this patch.

It's really necessary to fix the problem in the qmi proto handler.

Thanks,
Martin

On 2019-07-04 13:35, Martin Schiller wrote:
> This command is needed in the qmi proto handler to check if the plmn
> is already set to 'auto'.
> 
> The reason for this is, that setting the plmn to 'auto' will implicitly
> lead to a (delayed) network re-registration, which could further lead
> to some timing related issues in the qmi proto handler.
> 
> Signed-off-by: Martin Schiller <ms@dev.tdt.de>
> ---
>  commands-nas.c | 31 +++++++++++++++++++++++++++++++
>  commands-nas.h |  2 ++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/commands-nas.c b/commands-nas.c
> index 5874bfb..1f7445d 100644
> --- a/commands-nas.c
> +++ b/commands-nas.c
> @@ -293,6 +293,37 @@ cmd_nas_get_serving_system_prepare(struct qmi_dev
> *qmi, struct qmi_request *req,
>  }
> 
>  static void
> +cmd_nas_get_plmn_cb(struct qmi_dev *qmi, struct qmi_request *req,
> struct qmi_msg *msg)
> +{
> +	struct qmi_nas_get_system_selection_preference_response res;
> +	static const char *modes[] = {
> +		[QMI_NAS_NETWORK_SELECTION_PREFERENCE_AUTOMATIC] = "automatic",
> +		[QMI_NAS_NETWORK_SELECTION_PREFERENCE_MANUAL] = "manual",
> +	};
> +	void *c;
> +
> +	qmi_parse_nas_get_system_selection_preference_response(msg, &res);
> +
> +	c = blobmsg_open_table(&status, NULL);
> +	if (res.set.network_selection_preference) {
> +		blobmsg_add_string(&status, "mode",
> modes[res.data.network_selection_preference]);
> +	}
> +	if (res.set.manual_network_selection) {
> +		blobmsg_add_u32(&status, "mcc", 
> res.data.manual_network_selection.mcc);
> +		blobmsg_add_u32(&status, "mnc", 
> res.data.manual_network_selection.mnc);
> +	}
> +
> +	blobmsg_close_table(&status, c);
> +}
> +
> +static enum qmi_cmd_result
> +cmd_nas_get_plmn_prepare(struct qmi_dev *qmi, struct qmi_request
> *req, struct qmi_msg *msg, char *arg)
> +{
> +	qmi_set_nas_get_system_selection_preference_request(msg);
> +	return QMI_CMD_REQUEST;
> +}
> +
> +static void
>  cmd_nas_network_scan_cb(struct qmi_dev *qmi, struct qmi_request *req,
> struct qmi_msg *msg)
>  {
>  	static struct qmi_nas_network_scan_response res;
> diff --git a/commands-nas.h b/commands-nas.h
> index 9ebfa00..4b175f9 100644
> --- a/commands-nas.h
> +++ b/commands-nas.h
> @@ -24,6 +24,7 @@
>  	__uqmi_command(nas_set_network_modes, set-network-modes, required,
> CMD_TYPE_OPTION), \
>  	__uqmi_command(nas_initiate_network_register, network-register, no,
> QMI_SERVICE_NAS), \
>  	__uqmi_command(nas_set_plmn, set-plmn, no, QMI_SERVICE_NAS), \
> +	__uqmi_command(nas_get_plmn, get-plmn, no, QMI_SERVICE_NAS), \
>  	__uqmi_command(nas_set_mcc, mcc, required, CMD_TYPE_OPTION), \
>  	__uqmi_command(nas_set_mnc, mnc, required, CMD_TYPE_OPTION), \
>  	__uqmi_command(nas_network_scan, network-scan, no, QMI_SERVICE_NAS), 
> \
> @@ -44,6 +45,7 @@
>  		"  --set-plmn:                       Register at specified 
> network\n" \
>  		"    --mcc <mcc>:                    Mobile Country Code (0 - 
> auto)\n" \
>  		"    --mnc <mnc>:                    Mobile Network Code\n" \
> +		"  --get-plmn:                       Get preferred network
> selection info\n" \
>  		"  --get-signal-info:                Get signal strength info\n" \
>  		"  --get-serving-system:             Get serving system info\n" \
R. Diez via openwrt-devel Aug. 26, 2019, 7:12 p.m. UTC | #2
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
I think the ideology behind proto handler there is to "do whatever
told" despite of what the state is currently,
maybe there is a reason for such behaviour (searches some stuff from
network etc).
Martin Schiller Aug. 29, 2019, 7:29 a.m. UTC | #3
On 2019-08-26 21:12, Sami Olmari wrote:
> I think the ideology behind proto handler there is to "do whatever
> told" despite of what the state is currently,
> maybe there is a reason for such behaviour (searches some stuff from
> network etc).

There exist 2 problems in the qmi proto handler:

1. Setting the plmn to 'auto' will implicitly lead to a (delayed)
network re-registration, which could further lead to some timing
related issues in the qmi proto handler.
Let me explain this in more detail:
After successfully calling the uqmi --set-plmn (auto) command it takes
up to 5 (or maybe even more) seconds until the modem detaches from 
network
and start searching for new registration.

In the meantime the proto handler goes through the next steps ("Waiting 
for
network registration", "Start network $interface" etc.).

I hope you can see were this leads to.

This is really a problem in Roaming scenarios, where to provider maybe 
is
switched after the re-registration.


2. The plmn setting is permanently saved in the wwan modem:
This leads to the problem, that if you switch back from manual plmn 
selection
to auto mode you have to set it explicitly to 'auto'.

- Martin
Bjørn Mork Aug. 29, 2019, 8:35 a.m. UTC | #4
Martin Schiller <ms@dev.tdt.de> writes:

> On 2019-08-26 21:12, Sami Olmari wrote:
>> I think the ideology behind proto handler there is to "do whatever
>> told" despite of what the state is currently,
>> maybe there is a reason for such behaviour (searches some stuff from
>> network etc).
>
> There exist 2 problems in the qmi proto handler:
>
> 1. Setting the plmn to 'auto' will implicitly lead to a (delayed)
> network re-registration, which could further lead to some timing
> related issues in the qmi proto handler.
> Let me explain this in more detail:
> After successfully calling the uqmi --set-plmn (auto) command it takes
> up to 5 (or maybe even more) seconds until the modem detaches from
> network
> and start searching for new registration.
>
> In the meantime the proto handler goes through the next steps
> ("Waiting for
> network registration", "Start network $interface" etc.).
>
> I hope you can see were this leads to.
>
> This is really a problem in Roaming scenarios, where to provider maybe
> is
> switched after the re-registration.


FWIW, I also believe this is a real problem.

The modem firmware isn't always smart.  It will "do whatever told", even
if it is a completely unnecessary de-registration, network scan and
re-registration.

We can be smarter than that.  We should avoid changing any persistent
(in modem nvram) setting related to network registration, unless
absolutely necessary.

> 2. The plmn setting is permanently saved in the wwan modem:
> This leads to the problem, that if you switch back from manual plmn
> selection
> to auto mode you have to set it explicitly to 'auto'.

Yes, the current handler will use whatever is stored in the modem unless
'plmn' is explictly set.  This is very confusing if you set 'plmn'
temporarily, whether it is for roaming or just experimenting. Users will
rightfully assume that adding and then removing 'plmn' means 'no
change'.

Everything in the qmi proto handler should take into account that
settings might be stored in the modem nvram.  Optional settings need an
explicit default, and should always be verified against the value stored
in the modem.

I believe the 'plmn' default should be 'auto'. But we can only do that
if we first add the logic to verify the current setting and avoid any
unnecessary 'uqmi --set-plmn' commands.



Bjørn
Martin Schiller Oct. 22, 2019, 4:45 a.m. UTC | #5
Ping.

I still want to fix the existing problems in the qmi protohandler.

Therefore, this feature needs to be integrated into uqmi.

Martin

On 2019-08-29 10:35, Bjørn Mork wrote:
> Martin Schiller <ms@dev.tdt.de> writes:
> 
>> On 2019-08-26 21:12, Sami Olmari wrote:
>>> I think the ideology behind proto handler there is to "do whatever
>>> told" despite of what the state is currently,
>>> maybe there is a reason for such behaviour (searches some stuff from
>>> network etc).
>> 
>> There exist 2 problems in the qmi proto handler:
>> 
>> 1. Setting the plmn to 'auto' will implicitly lead to a (delayed)
>> network re-registration, which could further lead to some timing
>> related issues in the qmi proto handler.
>> Let me explain this in more detail:
>> After successfully calling the uqmi --set-plmn (auto) command it takes
>> up to 5 (or maybe even more) seconds until the modem detaches from
>> network
>> and start searching for new registration.
>> 
>> In the meantime the proto handler goes through the next steps
>> ("Waiting for
>> network registration", "Start network $interface" etc.).
>> 
>> I hope you can see were this leads to.
>> 
>> This is really a problem in Roaming scenarios, where to provider maybe
>> is
>> switched after the re-registration.
> 
> 
> FWIW, I also believe this is a real problem.
> 
> The modem firmware isn't always smart.  It will "do whatever told", 
> even
> if it is a completely unnecessary de-registration, network scan and
> re-registration.
> 
> We can be smarter than that.  We should avoid changing any persistent
> (in modem nvram) setting related to network registration, unless
> absolutely necessary.
> 
>> 2. The plmn setting is permanently saved in the wwan modem:
>> This leads to the problem, that if you switch back from manual plmn
>> selection
>> to auto mode you have to set it explicitly to 'auto'.
> 
> Yes, the current handler will use whatever is stored in the modem 
> unless
> 'plmn' is explictly set.  This is very confusing if you set 'plmn'
> temporarily, whether it is for roaming or just experimenting. Users 
> will
> rightfully assume that adding and then removing 'plmn' means 'no
> change'.
> 
> Everything in the qmi proto handler should take into account that
> settings might be stored in the modem nvram.  Optional settings need an
> explicit default, and should always be verified against the value 
> stored
> in the modem.
> 
> I believe the 'plmn' default should be 'auto'. But we can only do that
> if we first add the logic to verify the current setting and avoid any
> unnecessary 'uqmi --set-plmn' commands.
> 
> 
> 
> Bjørn
diff mbox series

Patch

diff --git a/commands-nas.c b/commands-nas.c
index 5874bfb..1f7445d 100644
--- a/commands-nas.c
+++ b/commands-nas.c
@@ -293,6 +293,37 @@  cmd_nas_get_serving_system_prepare(struct qmi_dev *qmi, struct qmi_request *req,
 }
 
 static void
+cmd_nas_get_plmn_cb(struct qmi_dev *qmi, struct qmi_request *req, struct qmi_msg *msg)
+{
+	struct qmi_nas_get_system_selection_preference_response res;
+	static const char *modes[] = {
+		[QMI_NAS_NETWORK_SELECTION_PREFERENCE_AUTOMATIC] = "automatic",
+		[QMI_NAS_NETWORK_SELECTION_PREFERENCE_MANUAL] = "manual",
+	};
+	void *c;
+
+	qmi_parse_nas_get_system_selection_preference_response(msg, &res);
+
+	c = blobmsg_open_table(&status, NULL);
+	if (res.set.network_selection_preference) {
+		blobmsg_add_string(&status, "mode", modes[res.data.network_selection_preference]);
+	}
+	if (res.set.manual_network_selection) {
+		blobmsg_add_u32(&status, "mcc", res.data.manual_network_selection.mcc);
+		blobmsg_add_u32(&status, "mnc", res.data.manual_network_selection.mnc);
+	}
+
+	blobmsg_close_table(&status, c);
+}
+
+static enum qmi_cmd_result
+cmd_nas_get_plmn_prepare(struct qmi_dev *qmi, struct qmi_request *req, struct qmi_msg *msg, char *arg)
+{
+	qmi_set_nas_get_system_selection_preference_request(msg);
+	return QMI_CMD_REQUEST;
+}
+
+static void
 cmd_nas_network_scan_cb(struct qmi_dev *qmi, struct qmi_request *req, struct qmi_msg *msg)
 {
 	static struct qmi_nas_network_scan_response res;
diff --git a/commands-nas.h b/commands-nas.h
index 9ebfa00..4b175f9 100644
--- a/commands-nas.h
+++ b/commands-nas.h
@@ -24,6 +24,7 @@ 
 	__uqmi_command(nas_set_network_modes, set-network-modes, required, CMD_TYPE_OPTION), \
 	__uqmi_command(nas_initiate_network_register, network-register, no, QMI_SERVICE_NAS), \
 	__uqmi_command(nas_set_plmn, set-plmn, no, QMI_SERVICE_NAS), \
+	__uqmi_command(nas_get_plmn, get-plmn, no, QMI_SERVICE_NAS), \
 	__uqmi_command(nas_set_mcc, mcc, required, CMD_TYPE_OPTION), \
 	__uqmi_command(nas_set_mnc, mnc, required, CMD_TYPE_OPTION), \
 	__uqmi_command(nas_network_scan, network-scan, no, QMI_SERVICE_NAS), \
@@ -44,6 +45,7 @@ 
 		"  --set-plmn:                       Register at specified network\n" \
 		"    --mcc <mcc>:                    Mobile Country Code (0 - auto)\n" \
 		"    --mnc <mnc>:                    Mobile Network Code\n" \
+		"  --get-plmn:                       Get preferred network selection info\n" \
 		"  --get-signal-info:                Get signal strength info\n" \
 		"  --get-serving-system:             Get serving system info\n" \