[RFC PATCH 1/1] ublox: create only 1 gprs context

[RFC PATCH 1/1] ublox: create only 1 gprs context

[Jonas Bonn]

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

---

Here's something that I'm having trouble understanding.  The ublox
plugin creates 8 gprs_context atoms.  As far as I can tell, this means
that ofono will allow it to activate 8 contexts simultaneously... right?

Why is the ublox plugin the only one that does this?  Do other modems
not support multiple active contexts?  Or is this plugin wrong?

If I were to follow the model of other plugins, the below patch would
seem appropriate...

A bit of insight here would be appreciated.

Thanks,
Jonas

 plugins/ublox.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/plugins/ublox.c b/plugins/ublox.c
index dc001765..3ce60236 100644
--- a/plugins/ublox.c
+++ b/plugins/ublox.c
@@ -384,8 +384,6 @@ static void ublox_post_sim(struct ofono_modem *modem)
 	struct ofono_gprs_context *gc;
 	GAtChat *chat = data->modem ? data->modem : data->aux;
 	const char *driver;
-	/* Toby L2: Create same number of contexts as supported PDP contexts. */
-	int ncontexts = data->flags & UBLOX_DEVICE_F_HIGH_THROUGHPUT_MODE ? 8 : 1;
 	int variant;
 
 	DBG("%p", modem);
@@ -409,14 +407,9 @@ static void ublox_post_sim(struct ofono_modem *modem)
 		variant = OFONO_VENDOR_UBLOX;
 	}
 
-	while (ncontexts) {
-		gc = ofono_gprs_context_create(modem, variant, driver, chat);
-
-		if (gprs && gc)
-			ofono_gprs_add_context(gprs, gc);
-
-		--ncontexts;
-	}
+	gc = ofono_gprs_context_create(modem, variant, driver, chat);
+	if (gprs && gc)
+		ofono_gprs_add_context(gprs, gc);
 
 	ofono_lte_create(modem,
 		ublox_model_to_id(data->model), "ubloxmodem", data->aux);
-- 
2.19.1

Re: [RFC PATCH 1/1] ublox: create only 1 gprs context

[Denis Kenzior]

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

Hi Jonas,

On 03/14/2019 05:37 PM, Jonas Bonn wrote:
> ---
> 

Funny, I was looking at this code today and thought it was wrong.  Was 
going to point this out to you, but you beat me to it ;)

> Here's something that I'm having trouble understanding.  The ublox
> plugin creates 8 gprs_context atoms.  As far as I can tell, this means
> that ofono will allow it to activate 8 contexts simultaneously... right?
>

Correct.  Each context driver added to gprs allows a concurrent context 
to be activated.  E.g. if you have 2 drivers, you can activate 2 
contexts simultaneously.  Obviously you need a high speed / ppp 
interface for each active context.

> Why is the ublox plugin the only one that does this?  Do other modems
> not support multiple active contexts?  Or is this plugin wrong?

 From what I recall, ublox does claim to support multiple PDP contexts 
active at the same time.  However, I don't know how this works in 
practice as you need a unique network interface for each one.  As it 
stands today, given the udevng detection logic, this plugin is wrong.

> 
> If I were to follow the model of other plugins, the below patch would
> seem appropriate...
> 
> A bit of insight here would be appreciated.

There are drivers for USB based modems that do this properly.  See 
xmm7xxx for example.  Multiple PDP context support was added to that 
recently.

Modems that used multiplexing had support for multiple PDP contexts for 
quite some time.  E.g. plugins/ifx, etc.

Anyway, patch looks fine to me.  Let me know if you want me to apply it 
or you want to take a stab at fixing the detection logic.

Regards,
-Denis

Re: [RFC PATCH 1/1] ublox: create only 1 gprs context

[Jonas Bonn]

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

Hi Denis,

On 15/03/2019 02:27, Denis Kenzior wrote:
> Hi Jonas,
> 
> On 03/14/2019 05:37 PM, Jonas Bonn wrote:
>> ---
>>
> 
> Funny, I was looking at this code today and thought it was wrong.  Was 
> going to point this out to you, but you beat me to it ;)
> 
>> Here's something that I'm having trouble understanding.  The ublox
>> plugin creates 8 gprs_context atoms.  As far as I can tell, this means
>> that ofono will allow it to activate 8 contexts simultaneously... right?
>>
> 
> Correct.  Each context driver added to gprs allows a concurrent context 
> to be activated.  E.g. if you have 2 drivers, you can activate 2 
> contexts simultaneously.  Obviously you need a high speed / ppp 
> interface for each active context.
> 
>> Why is the ublox plugin the only one that does this?  Do other modems
>> not support multiple active contexts?  Or is this plugin wrong?
> 
>  From what I recall, ublox does claim to support multiple PDP contexts 
> active at the same time.  However, I don't know how this works in 
> practice as you need a unique network interface for each one.  As it 
> stands today, given the udevng detection logic, this plugin is wrong.

Right, so the way this works is that the modem looks like either a 
"bridge" or a "router".  If configured as a "bridge", the outbound 
packet's source address is used to determine which context to use; if 
configured as a "router", the outbound packet's destination _network_ is 
used to determine which context to use and a default route is set via 
the _first activated context_ for packets that don't match the network 
of any active context.  That's a mouthful...!!!

The short of it is that this probably works in practice.

Is it ok for the connection manager interface to see multiple active 
contexts with the same Interface and the IP settings set to method 
"dhcp".  Will it run a DHCP client on each interface or is it expected 
to be smart enough to recognize the common interface???  What would the 
point be since the only thing that differs the contexts is the APN and 
the connection manager shouldn't care about that particular detail...??? 
  This all applies to "router" mode only, really.

In "bridge" mode, the above is probably moot since each context would 
announce different IP settings and the connection manager would be 
expected to apply those settings to the common interface.  No idea if 
this works in practice.

Do you know how these multiple, active contexts are used in practice? 
In particular with regard to ofono?

> 
>>
>> If I were to follow the model of other plugins, the below patch would
>> seem appropriate...
>>
>> A bit of insight here would be appreciated.
> 
> There are drivers for USB based modems that do this properly.  See 
> xmm7xxx for example.  Multiple PDP context support was added to that 
> recently.
> 
> Modems that used multiplexing had support for multiple PDP contexts for 
> quite some time.  E.g. plugins/ifx, etc.
> 
> Anyway, patch looks fine to me.  Let me know if you want me to apply it 
> or you want to take a stab at fixing the detection logic.

 From this, it sounds like the multiple gprs-context atoms are probably 
correct.  I'll take a look at the xmm7xxx driver and see how it handles 
things.

Thanks,
Jonas

Re: [RFC PATCH 1/1] ublox: create only 1 gprs context

[Denis Kenzior]

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

Hi Jonas,

>>  From what I recall, ublox does claim to support multiple PDP contexts 
>> active at the same time.  However, I don't know how this works in 
>> practice as you need a unique network interface for each one.  As it 
>> stands today, given the udevng detection logic, this plugin is wrong.
> 
> Right, so the way this works is that the modem looks like either a 
> "bridge" or a "router".  If configured as a "bridge", the outbound 
> packet's source address is used to determine which context to use; if 
> configured as a "router", the outbound packet's destination _network_ is 
> used to determine which context to use and a default route is set via 
> the _first activated context_ for packets that don't match the network 
> of any active context.  That's a mouthful...!!!

Ugh.  Both of these are really not ideal.  I'd really rather have the 
host manage all these details than having the modem do it.  Oh well.

I take it that in bridged mode we should be setting multiple IP 
addresses to the network interface?

> 
> The short of it is that this probably works in practice.
> 

That depends.  I suspect that it might not 'just work' for certain 
classes of applications running on the host / application processor side.

> Is it ok for the connection manager interface to see multiple active 
> contexts with the same Interface and the IP settings set to method 
> "dhcp".  Will it run a DHCP client on each interface or is it expected 
> to be smart enough to recognize the common interface???  What would the 
> point be since the only thing that differs the contexts is the APN and 
> the connection manager shouldn't care about that particular detail...??? 
>   This all applies to "router" mode only, really.

uBlox is just being too smart here.  The whole bridged vs router stuff 
was never intended by 27.007.  They made this all up themselves. 
Neither oFono nor ConnMan were designed to handle such a setup.

The main issue will be for contexts used for MMS / OMA DM.  My memory 
from the Meego / Tizen days is fuzzy, but from what I recall we designed 
oFono with the assumption that we will always get an IP address and a 
separate interface for the MMS context.

There's also the proxy.  MMS contexts frequently have a proxy associated 
with them.  Again, we assumed that if a proxy is set, then it is also in 
a form of an IP address.  The proxy is then added as a route on the 
interface by oFono (see pri_setproxy for details).  And in practical 
terms there is no domain name resolution for these contexts.  Everything 
just goes to the proxy.

This was how e.g. mmsd was designed to work.  See 
https://git.kernel.org/pub/scm/network/ofono/mmsd.git.  That project has 
not been touched in some time though, but there are others that act very 
similar.

> 
> In "bridge" mode, the above is probably moot since each context would 
> announce different IP settings and the connection manager would be 
> expected to apply those settings to the common interface.  No idea if 
> this works in practice.

Right.  Do note that oFono is responsible for setting the IP on the 
interface.  So in bridged mode, this would need to be fixed.  Also, I 
don't know whether connman supports multi-IP interfaces in the first place.

Another issue is that ConnMan ignores all contexts that are not of type 
Internet.  So for example, nobody (or at least neither oFono, nor 
ConnMan) would set the domain name servers for MMS contexts.

> 
> Do you know how these multiple, active contexts are used in practice? In 
> particular with regard to ofono?
> 

They are used, yes.  MMS, OMA DM/device provisioning, etc.

>>
>>>
>>> If I were to follow the model of other plugins, the below patch would
>>> seem appropriate...
>>>
>>> A bit of insight here would be appreciated.
>>
>> There are drivers for USB based modems that do this properly.  See 
>> xmm7xxx for example.  Multiple PDP context support was added to that 
>> recently.
>>
>> Modems that used multiplexing had support for multiple PDP contexts 
>> for quite some time.  E.g. plugins/ifx, etc.
>>
>> Anyway, patch looks fine to me.  Let me know if you want me to apply 
>> it or you want to take a stab at fixing the detection logic.
> 
>  From this, it sounds like the multiple gprs-context atoms are probably 
> correct.  I'll take a look at the xmm7xxx driver and see how it handles 
> things.
> 

So this is still debatable :)

Regards,
-Denis

Re: [RFC PATCH 1/1] ublox: create only 1 gprs context

[Jonas Bonn]

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

Hi Denis,

On 15/03/2019 17:28, Denis Kenzior wrote:

<snip>

> 
> uBlox is just being too smart here.  The whole bridged vs router stuff 
> was never intended by 27.007.  They made this all up themselves. Neither 
> oFono nor ConnMan were designed to handle such a setup.
> 
> The main issue will be for contexts used for MMS / OMA DM.  My memory 
> from the Meego / Tizen days is fuzzy, but from what I recall we designed 
> oFono with the assumption that we will always get an IP address and a 
> separate interface for the MMS context.
> 
> There's also the proxy.  MMS contexts frequently have a proxy associated 
> with them.  Again, we assumed that if a proxy is set, then it is also in 
> a form of an IP address.  The proxy is then added as a route on the 
> interface by oFono (see pri_setproxy for details).  And in practical 
> terms there is no domain name resolution for these contexts.  Everything 
> just goes to the proxy.
> 
> This was how e.g. mmsd was designed to work.  See 
> https://git.kernel.org/pub/scm/network/ofono/mmsd.git.  That project has 
> not been touched in some time though, but there are others that act very 
> similar.
> 

OK, thanks for digging that out despite the cobwebs!

So, this is where I end up then:

i)  MMS (and other) contexts are just IP endpoints, but they are all 
identified by a "type"
ii)  connman handles "Internet" contexts and ignores everything else...
iii) other applications (like mmsd) handle contexts of other types
iv)  mmsd, unlike connman, expects the network interface associated with 
the context to be fully configured... IP address, route, etc.

ofono is designed to expect a network interface for each context.  For 
this reason, it's pretty heavy-handed in its setup of the interface:  it 
assumes the interface isn't shared by other contexts and isn't careful 
about maintaining settings coming from other contexts.

So the real impedance mismatch here for the case where a modem puts 
multiple contexts behind a single network interface is the way ofono is 
implemented.

ofono could be re-implemented here to add multiple IP addresses to a 
single interface and to set routing accordingly (for MMS contexts only, 
apparently... it looks like ofono does nothing currently for other 
context types).

Another alternative is to have the gprs-context driver for affected 
devices create "virtual network devices" (is this still a thing?) with 
static IP addresses for all contexts.  For the u-blox "router mode" 
case, though, I can't figure out who runs the DHCP client... (and the 
TOBY L4 is router-mode-only, so requiring bridge-mode is not an option).

Either way, this is a reasonably big undertaking.  I don't think I want 
to take this on without a concrete use-case presenting itself, first! :)

So given that, the right thing to do here is probably to just drop 
multiple context support from the u-blox driver until ofono is modified 
(someday) to handle multiple contexts behind a single network interface. 
  I will send a patch.

/Jonas

Re: [RFC PATCH 1/1] ublox: create only 1 gprs context

[Denis Kenzior]

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

Hi Jonas,

> OK, thanks for digging that out despite the cobwebs!

No worries.  Sorry I forgot to reply to this earlier.

> 
> So, this is where I end up then:
> 
> i)  MMS (and other) contexts are just IP endpoints, but they are all 
> identified by a "type"

Right now yes.  In theory you could have a DNS, routing, etc for these 
contexts as well.  In fact, once we introduce IMS contexts, this has to 
be done anyway.

> ii)  connman handles "Internet" contexts and ignores everything else...

Correct.

> iii) other applications (like mmsd) handle contexts of other types
> iv)  mmsd, unlike connman, expects the network interface associated with 
> the context to be fully configured... IP address, route, etc.

Correct, at least at the moment

> 
> ofono is designed to expect a network interface for each context.  For 
> this reason, it's pretty heavy-handed in its setup of the interface:  it 
> assumes the interface isn't shared by other contexts and isn't careful 
> about maintaining settings coming from other contexts.

This is also what 3GPP intended, at least back in the multiplexing / 
27.010 days.  Now things become a bit more complex as you can tell. 
Intel modems do actually expose multiple pseudo-ethernet devices for this.

> 
> So the real impedance mismatch here for the case where a modem puts 
> multiple contexts behind a single network interface is the way ofono is 
> implemented.

Correct.

> 
> ofono could be re-implemented here to add multiple IP addresses to a 
> single interface and to set routing accordingly (for MMS contexts only, 
> apparently... it looks like ofono does nothing currently for other 
> context types).

This gets tricky, but I assume that for 'bridged' mode we would need to 
do this yes.

With router mode things become a bit simpler.  Likely the only thing 
needed on these is to just automagically setup routing for MMS/SIP 
proxies.  The only difference from what we have today would be that we 
need to remove the corresponding routes when the MMS context goes down 
(today oFono assumes that the entire interface is brought down) So it 
wouldn't be that hard to implement actually.  And since ConnMan ignores 
non-Internet contexts, things would probably just work.

Someone needs to test that theory though.  I don't have any ublox devices.

> 
> Another alternative is to have the gprs-context driver for affected 
> devices create "virtual network devices" (is this still a thing?) with 
> static IP addresses for all contexts.  For the u-blox "router mode" 
> case, though, I can't figure out who runs the DHCP client... (and the 
> TOBY L4 is router-mode-only, so requiring bridge-mode is not an option).

We have been contemplating for a long time that DHCP needs to move out 
of ConnMan and into the individual daemons, e.g. BlueZ, iwd, oFono.  I 
already started moving bits of DHCP into ell, but higher priority tasks 
came up.

There was an earlier thread where weird device behaviors were described 
(by Giacinto, I think?).  E.g. a device would not succeed in context 
activation until DHCP was run against the interface.  So supporting such 
devices with DHCP in ConnMan is a bit problematic and we have a fighting 
chance with DHCP in oFono.

> 
> Either way, this is a reasonably big undertaking.  I don't think I want 
> to take this on without a concrete use-case presenting itself, first! :)
> 

Device Provisioning, Firmware update, MMS, IMS :)

> So given that, the right thing to do here is probably to just drop 
> multiple context support from the u-blox driver until ofono is modified 
> (someday) to handle multiple contexts behind a single network interface. 
>   I will send a patch.

For now I think that is the right approach.

Regards,
-Denis