Closed Bug 772747 Opened 7 years ago Closed 7 years ago

B2G RIL: Add method to connect to different APN type

Categories

(Firefox OS Graveyard :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(blocking-b2g:shira+, blocking-basecamp:+, b2g18 fixed, b2g18-v1.0.1 fixed)

RESOLVED FIXED
B2G C1 (to 19nov)
blocking-b2g shira+
blocking-basecamp +
Tracking Status
b2g18 --- fixed
b2g18-v1.0.1 --- fixed

People

(Reporter: kanru, Assigned: swu)

References

Details

(Keywords: feature, Whiteboard: [LOE:M] [WebAPI:P0] [mno11] QARegressExclude)

Attachments

(1 file, 4 obsolete files)

In A-GPS and MMS, when we want to setup a data call, we should be able to request a data call of type "MMS" or "SUPL" instead of using SetupDataCall directly.

And in DataCallStateChanged callback, we should be able to know the connection type.
Assignee: nobody → htsai
Blocks: 762426
Hi there, I start working on this. Couple questions just came up. Maybe you could help answer them before I move on. Thanks.

I just discussed with Vicamo to clarify this issue. (Vicamo, please correct me if I misunderstood.) What mms and a-gps need is to search an available apn whose type is mms/a-gps, and then connect to that. However, the apn database is maintained in content instead of gecko per my understanding. Then, how to get the apn information from gecko?
See Also: → 729440
Shian-yow, please help this case because you're working on multiple APN. Thanks!
Assignee: htsai → swu
blocking-basecamp: --- → +
Depends on: 776294
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> I just discussed with Vicamo to clarify this issue. (Vicamo, please correct
> me if I misunderstood.) What mms and a-gps need is to search an available
> apn whose type is mms/a-gps, and then connect to that. However, the apn
> database is maintained in content instead of gecko per my understanding.
> Then, how to get the apn information from gecko?

The answer to everything these days: Settings API :)
(In reply to Philipp von Weitershausen [:philikon] from comment #3)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> > I just discussed with Vicamo to clarify this issue. (Vicamo, please correct
> > me if I misunderstood.) What mms and a-gps need is to search an available
> > apn whose type is mms/a-gps, and then connect to that. However, the apn
> > database is maintained in content instead of gecko per my understanding.
> > Then, how to get the apn information from gecko?
> 
> The answer to everything these days: Settings API :)

Hmmm ... let's dive into the problem we have in Settings API.

Use Case 1:
Without 3G connection pre-setup, MMS message app receives a WAP Push notification for MMS message and want to retrieve the message right away.

Tech Details:
The mms application should emit a request for data connection with MMS feature enabled. Solution of this issue has to find out an appropriate APN to connect by the MNC/MCC we got. In order to do such searching, there MUST be a certain APN database available in chrome process. However, by the design of Settings API, we can only get a string that represents the whole APN database. And, in order to fetch that string, Settings application in content process MUST had set this string to some predefined settings value. That is, the settings application must parse APN database xml, serialize it into a huge string, and set to a settings value.

For Android, APN database is owned by TelephonyProvider but publicly accessible as part of framework API. TelephonyProvider parses /system/etc/apns-conf.xml and populates the APN database.
(In reply to Vicamo Yang [:vicamo] from comment #4)
> > The answer to everything these days: Settings API :)

I discuss with Vicamo and Shian-Yow.

I think in gecko, we should read something like 
"ril.data.apn", 
"ril.mms.apn",
"ril.supl.apn"
from Settings API, 
instead of parsing the 'huge string'.

So the parsing and getting the correct APNs should be left to Settings APP to handle.

How do you think about this?

Thanks
(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #5)
> (In reply to Vicamo Yang [:vicamo] from comment #4)
> > > The answer to everything these days: Settings API :)
> 
> I discuss with Vicamo and Shian-Yow.
> 
> I think in gecko, we should read something like 
> "ril.data.apn", 
> "ril.mms.apn",
> "ril.supl.apn"
> from Settings API, 
> instead of parsing the 'huge string'.
> 
> So the parsing and getting the correct APNs should be left to Settings APP
> to handle.

I completely agree.
No longer blocks: 762426
Blocks: 762426
According to apns-conf.xml, almost all ISPs use same Internet APN for SUPL, and many have also same as MMS.  For example, the default(Internet) APN could be type="default,supl" or type="default,supl,mms".

In this case, how do we know handle APN connection for MMS or SUPL?  And when data call already established on Internet APN, we shouldn't set data call again for MMS/SUPL APN if they are sharing same APN name with Internet APN.

As discussed with Vicamo, we have two options, and currently we tend to consider option 1.

Option 1:
Read APN type from Settings API and base on it to determine which APN to use.  If the default APN contains the type that we want to connect, then connect to default APN(if the connection has not yet been set).  Otherwise, search for other APNs to corresponding APN type and connect.  The settings will be something like:

  Default APN:  ril.data.*
  Secondary APNs:  ril.data2.* and ril.data3.*

Option 2:
Let Settings APP handle everything.  The Settings APP need to read the whole APN settings for specific ISP, and base on the type settings for each APN to create settings for each APN type.
It will be something like what we discussed earlier in comment #4.  If MMS/SUPL uses same APN name with default APN, there will be redundant settings in secondary APNs.  And in gecko, we need to do something to prevent connecting to same APN name twice.

  Default APN: ril.data.*
  Secondary APNs: ril.mms.* and ril.supl.*
Depends on: 782513
Whiteboard: [LOE:M]
Whiteboard: [LOE:M] → [LOE:M] [WebAPI:P0]
Keywords: feature
Attached patch WIP V1 (obsolete) — Splinter Review
This patch is created on top of bug 782513, using option 1 in comment 7.

One thing to note is, when secondary types tries to connect using default APN, the connection totally depends on "ril.data.enabled".

Kanru, Vicamo: please see if it can meet requirements for A-GPS and MMS.
Attachment #660774 - Flags: feedback?
Comment on attachment 660774 [details] [diff] [review]
WIP V1

Review of attachment 660774 [details] [diff] [review]:
-----------------------------------------------------------------

I guess I have to place a minus sign here.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1844,5 @@
> +    // Do nothing if secondary APN shares connection with default APN
> +    if (apntype != "default" &&
> +        this.dataCallSettings["apntypes"] &&
> +        this.dataCallSettings["apntypes"].indexOf(apntype) != -1) {
> +      debug("APN type " + apntype + " goes through default APN, nothing to do.");

So if mms and default share the same apn, and default connection is not active for we're using wifi, then there is no way a mms message can be routed out?

@@ +1898,5 @@
> +    // Nothing to do if secondary APN shares connection with default APN.
> +    if (apntype != "default" &&
> +        this.dataCallSettings["apntypes"] &&
> +        this.dataCallSettings["apntypes"].indexOf(apntype) != -1) {
> +      debug("APN type " + apntype + " goes through default APN, nothing to do.");

ditto
Attachment #660774 - Flags: feedback? → feedback-
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #9)
> So if mms and default share the same apn, and default connection is not
> active for we're using wifi, then there is no way a mms message can be
> routed out?

Good point.  To make implementation simple for this stage, we can add related host route unconditionally to the interface.
Attached patch WIP V2 (obsolete) — Splinter Review
This patch always adds host route rules also for default APN.
Attachment #660774 - Attachment is obsolete: true
Attachment #661206 - Flags: feedback?
Attachment #661206 - Flags: feedback?(vyang)
Attachment #661206 - Flags: feedback?(kchen)
Attachment #661206 - Flags: feedback?
Comment on attachment 661206 [details] [diff] [review]
WIP V2

Review of attachment 661206 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +201,5 @@
>    lock.get("ril.data.httpProxyHost", this);
>    lock.get("ril.data.httpProxyPort", this);
>    lock.get("ril.data.roaming_enabled", this);
>    lock.get("ril.data.enabled", this);
> +  lock.get("ril.data.apntypes", this);

I found in gaia/app/settings/serviceproviders.xml uses the term "usage" instead of "types" or "apntypes". It also uses "internet" instead of "default" for the default mobile data connection. Which terminology will we use actually? Either way we also have to make Gaia set "ril.data.apntypes" or an equivalent one in Settings app. Is there already an issue open in Gaia?
Attachment #661206 - Flags: feedback?(vyang) → feedback+
Gaia issue to support multiple APN types in https://github.com/mozilla-b2g/gaia/issues/4717
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #12)
> I found in gaia/app/settings/serviceproviders.xml uses the term "usage"
> instead of "types" or "apntypes". It also uses "internet" instead of
> "default" for the default mobile data connection. Which terminology will we
> use actually? Either way we also have to make Gaia set "ril.data.apntypes"
> or an equivalent one in Settings app. Is there already an issue open in Gaia?

Yes, the "gaia/app/settings/serviceproviders.xml" uses different term from "apn-conf.xml" used by Android.  I think either way are ok.

Do you have any comments, Kaze?
No longer blocks: 792521
FTR, I chose serviceproviders.xml without any particular reason except that I knew it existed. If the Android database is more up-to-date (e.g. a few users are reporting that T-mobile is not properly configured at the moment), we could switch to this database in Gaia and adapt the code accordingly.

I have no preference regarding the terminology, but if we prefer using the Android database I guess it'd make sense to use the Android terminology.
We need to patch serviceproviders.xml to support some items only provided by apn-conf.xml:

1. Multiple APN types for specific APN, such as "default,mms,supl".
2. HTTP proxy and MMS settings: proxy, port, mmsc, mmsproxy, mmsport.
After discussion with Kaze, apn-conf.xml + settings of option 2 in comment 7 seems a better combination for the moment.

Already discussed with Vicamo, now checking for feedback from Kanru and Philikon.
Attachment #661206 - Attachment is obsolete: true
Attachment #661206 - Flags: feedback?(kchen)
Attachment #663006 - Flags: feedback?(philipp)
Attachment #663006 - Flags: feedback?(kchen)
Comment on attachment 663006 [details] [diff] [review]
WIP V3 (using option 2 in comment 7)

Review of attachment 663006 [details] [diff] [review]:
-----------------------------------------------------------------

How do I know if I am already connected to the correct APN type?
(In reply to Kan-Ru Chen [:kanru] from comment #19)
> How do I know if I am already connected to the correct APN type?

Expose a new API getDataCallStatusByType(in DOMString apntype) in nsIRadioInterfaceLayer?
Attached patch WIP V4 (obsolete) — Splinter Review
Hi Philikon,

Per discussion with Kanru, we'll provide following APIs for this stage.
- setupDataCallByType()
- deactivateDataCallByType()
- getDataCallStateByType()

And below enhancements to be planned in follow up bugs:
1. Array-ize APN Settings/RILNetworkInterface.  There is an initial implementation in https://bugzilla.mozilla.org/show_bug.cgi?id=782513#c12
2. Handle multiple sharing on one APN connection by reference count

Please advice.
Attachment #663006 - Attachment is obsolete: true
Attachment #663006 - Flags: feedback?(philipp)
Attachment #663006 - Flags: feedback?(kchen)
Attachment #663995 - Flags: review?(philipp)
Comment on attachment 663995 [details] [diff] [review]
WIP V4

Review of attachment 663995 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1864,5 @@
> +
> +  setupDataCallByType: function setupDataCallByType(apntype) {
> +    if (apntype != "default" && this.usingDefaultAPN(apntype)) {
> +      debug("Secondary APN type " + apntype + " goes through default APN, nothing to do.");
> +      return;

In case we want to connect to the MMS APN but the MMS and data APN are the same, we should make sure we connect to the data APN and not just bail out and assume that the data APN is connected. The user might have data disabled but might want to be able to send/receive MMS.

See also below.

@@ +1869,5 @@
> +    }
> +    switch (apntype) {
> +      case "default":
> +        this.dataNetworkInterface.connect(this.dataCallSettings);
> +        break;

This now makes me wish we had

  this.networkInterfaces.data
  this.networkInterfaces.mms

etc., then we could just do something like

  let networkInterface = this.networkInterfaces[apntype];
  if (!networkInterface) {
    debug("Unsupported APN type " + apntype);
    return;
  }
  networkInterface.connect(this.dataCallSettings[apntype]);

Note that I also assume to have

  this.dataCallSettings.data
  this.dataCallSettings.mms
  etc.

Also this means that we can map

  this.networkInterfaces.data = this.networkInterfaces.mms

in case this.dataCallSettings.data["apn"] == this.dataCallSettings.mms["apn"]. If we do this at the time either setting changes, we don't have to do any runtime checks. We can just go ahead and look up the network interface object and call .connect() on it. If it's already connected, it will be a no-op.

What do you think?
Attachment #663995 - Flags: review?(philipp) → feedback+
(In reply to Philipp von Weitershausen [:philikon] from comment #22)
> > +
> > +  setupDataCallByType: function setupDataCallByType(apntype) {
> > +    if (apntype != "default" && this.usingDefaultAPN(apntype)) {
> > +      debug("Secondary APN type " + apntype + " goes through default APN, nothing to do.");
> > +      return;
> 
> In case we want to connect to the MMS APN but the MMS and data APN are the
> same, we should make sure we connect to the data APN and not just bail out
> and assume that the data APN is connected. The user might have data disabled
> but might want to be able to send/receive MMS.

If user have data disabled, he may want to avoid any data call charge.
Should we take this into consideration?

> 
> See also below.
> 
> @@ +1869,5 @@
> > +    }
> > +    switch (apntype) {
> > +      case "default":
> > +        this.dataNetworkInterface.connect(this.dataCallSettings);
> > +        break;
> 
> This now makes me wish we had
> 
>   this.networkInterfaces.data
>   this.networkInterfaces.mms
> 
> etc., then we could just do something like
> 
>   let networkInterface = this.networkInterfaces[apntype];
>   if (!networkInterface) {
>     debug("Unsupported APN type " + apntype);
>     return;
>   }
>   networkInterface.connect(this.dataCallSettings[apntype]);
> 
> Note that I also assume to have
> 
>   this.dataCallSettings.data
>   this.dataCallSettings.mms
>   etc.
> 
> Also this means that we can map
> 
>   this.networkInterfaces.data = this.networkInterfaces.mms
> 
> in case this.dataCallSettings.data["apn"] ==
> this.dataCallSettings.mms["apn"]. If we do this at the time either setting
> changes, we don't have to do any runtime checks. We can just go ahead and
> look up the network interface object and call .connect() on it. If it's
> already connected, it will be a no-op.
> 
> What do you think?

This sounds good, thanks for this suggestion. :)
(In reply to Shian-Yow Wu from comment #23)
> (In reply to Philipp von Weitershausen [:philikon] from comment #22)
> > > +
> > > +  setupDataCallByType: function setupDataCallByType(apntype) {
> > > +    if (apntype != "default" && this.usingDefaultAPN(apntype)) {
> > > +      debug("Secondary APN type " + apntype + " goes through default APN, nothing to do.");
> > > +      return;
> > 
> > In case we want to connect to the MMS APN but the MMS and data APN are the
> > same, we should make sure we connect to the data APN and not just bail out
> > and assume that the data APN is connected. The user might have data disabled
> > but might want to be able to send/receive MMS.
> 
> If user have data disabled, he may want to avoid any data call charge.
> Should we take this into consideration?

That's a good question. I would assume that MMS and A-GPS and so on still work even if I have my mobile data connection disabled. I would not think that connecting to the MMS and A-GPS APN would incur charges. And when they use the same connection as the data APN, I would assume that connecting to the data APN wouldn't incur charges so long as you only created MMS and A-GPS traffic. jaoo, beatriz, can you guys shed some light on this perhaps?

Another point I just realized is that if e.g. the data APN is the same as the MMS APN and we connect to it for MMS functionality, but data is disabled, we should only enable the routes necessary for MMS. We should not route general web traffic, and we should not set navigator.mozMobileConnection.data.connected and navigator.onLine to true.
Comment on attachment 663995 [details] [diff] [review]
WIP V4

Let's deal with my points from comment 22 in separate follow-up bugs so that this feature blocker can land. Especially since at least one of those points needs further discussion. Shian-Yow, would you mind filing those bugs? Thanks!
Attachment #663995 - Flags: feedback+ → review+
(In reply to Shian-Yow Wu from comment #21)
> And below enhancements to be planned in follow up bugs:
> 1. Array-ize APN Settings/RILNetworkInterface.  There is an initial
> implementation in https://bugzilla.mozilla.org/show_bug.cgi?id=782513#c12
> 2. Handle multiple sharing on one APN connection by reference count

And of course, Shian-Yow already suggested it way earlier. I feel stupid now.
(In reply to Philipp von Weitershausen [:philikon] from comment #25)
> Comment on attachment 663995 [details] [diff] [review]
> WIP V4
> 
> Let's deal with my points from comment 22 in separate follow-up bugs so that
> this feature blocker can land. Especially since at least one of those points
> needs further discussion. Shian-Yow, would you mind filing those bugs?
> Thanks!

Two follow-up bugs has been filed:
https://bugzilla.mozilla.org/show_bug.cgi?id=794336
https://bugzilla.mozilla.org/show_bug.cgi?id=794342
Attached patch V4 (r=philikon)Splinter Review
Rebased.
Attachment #663995 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/b8fa9968ad3d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [LOE:M] [WebAPI:P0] → [LOE:M] [WebAPI:P0] [mno11]
blocking-b2g: --- → shira+
Target Milestone: --- → B2G C1 (to 19nov)
Do not have the SIMs at this time to test APNs.  Skipping verifications
Whiteboard: [LOE:M] [WebAPI:P0] [mno11] → [LOE:M] [WebAPI:P0] [mno11] QARegressExclude
Can you please describe specific steps with which we can test this resolved fix through the UI?  Thanks?
You need to log in before you can comment on or make changes to this bug.