Closed Bug 762426 Opened 12 years ago Closed 12 years ago

B2G network manager: support multiple simultaneously connected network interfaces

Categories

(Core :: DOM: Device Interfaces, defect, P1)

Product:
Core
Component:
DOM: Device Interfaces
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18
Project Flags:
blocking-kilimanjaro +
blocking-b2g shira+
blocking-basecamp +
Tracking Flags:
Tracking Status
b2g18 --- fixed
b2g18-v1.0.1 --- fixed

People

(Reporter: kanru, Assigned: swu)

References

Details

(Whiteboard: [LOE:M] [mno11])

Attachments

(1 file, 2 obsolete files)

WIP V3
14.18 KB, patch
Details | Diff | Splinter Review 
Some operators require MMS and A-GPS to use dedicated APN to use the service, which may create multiple data connections.

We need a way to initiate such connections and the network manager should handle such use case.
Default data connection on SGS2 (android)

  default via 42.72.99.1 dev rmnet0 
  42.72.99.0/24 dev rmnet0  proto kernel  scope link  src 42.72.99.222 
  42.72.99.1 dev rmnet0  scope link 

After open the second connection

  default via 42.72.99.1 dev rmnet0 
  10.1.7.1 via 10.34.87.1 dev rmnet1 
  10.1.7.2 via 10.34.87.1 dev rmnet1 
  10.34.87.0/24 dev rmnet1  proto kernel  scope link  src 10.34.87.137 
  10.34.87.1 dev rmnet1  scope link 
  42.72.99.0/24 dev rmnet0  proto kernel  scope link  src 42.72.99.222 
  42.72.99.1 dev rmnet0  scope link
Default wifi

  default via 10.247.32.1 dev wlan0 
  default via 10.247.32.1 dev wlan0  metric 308 
  10.247.32.0/21 dev wlan0  proto kernel  scope link  src 10.247.37.247  metric 308 
  10.247.32.1 dev wlan0  scope link 

Wifi + SUPL apn:

  default via 10.247.32.1 dev wlan0 
  default via 10.247.32.1 dev wlan0  metric 308 
  10.1.7.1 via 10.38.122.1 dev rmnet0 
  10.1.7.2 via 10.38.122.1 dev rmnet0 
  10.38.122.0/24 dev rmnet0  proto kernel  scope link  src 10.38.122.11 
  10.38.122.1 dev rmnet0  scope link 
  10.247.32.0/21 dev wlan0  proto kernel  scope link  src 10.247.37.247  metric 308 
  10.247.32.1 dev wlan0  scope link
The secondary APN has two additional host route rules, which are its DNS server.
The bionic DNS resolver can accept per-process DNS server list, using the properties 'net.dns1.<pid>', 'net.dns2.<pid>', etc... Where <pid> stands for the numerical ID of the current process.

And in ConnectivityService.java the DNS server list is set for each network type (mms, supl, etc.) users, in mNetRequestersPids.

This could be a problem because we have only one or two processes.
I will implement multiple APNs support first, without tackling the DNS problem. Let's figure out that later.
Assignee: nobody → kchen
Blocks a blocker.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
blocking-kilimanjaro: --- → +
Does this affect Telefonica in Brazil?
(In reply to Andrew Overholt [:overholt] from comment #7)
> Does this affect Telefonica in Brazil?

Pretty sure we need this for A-GPS and MMS.
Summary: Support multiple simultaneously connected network interfaces → B2G network manager: support multiple simultaneously connected network interfaces
(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> Pretty sure we need this for A-GPS and MMS.

Yeah, but Kan-Ru said "Some operators" so we wanted to be sure it was affecting Telefonica in Brazil :)

(In reply to Kan-Ru Chen [:kanru] from comment #0)
> Some operators require MMS and A-GPS to use dedicated APN to use the
> service, which may create multiple data connections.
Depends on: 772747
Currently we could only have one default route, set to the preferred
interface or the last connected one. I think the first step is to make
sure we don't call setDefaultRouteAndDNS when we connect to a secondary
APN and then add custom rules for the APN.

Example test cases:

 1. Wifi connected, set default route
 2. APN-internet connected, no action since wifi is preferred

and

 1. APN-internet connected, set default route
 2. Wifi connected, set default route since wifi is preferred
 3. Wifi disabled, set default route to APN-internet

and

 1. Wifi connected, set default route
 2. APN-mms connected, no action since it is a secondary APN

and

 1. APN-internet connected, set default route
 2. APN-mms connected, no action since it is a secondary APN
 3. (Add host-route rules to APN-mms)

etc.

To discover a secondary APN we will need some support from bug 772747.
I just spoke about this with Kan-Ru.  It appears that we have not yet determined if this affects MMS and A-GPS on Telefonica in Brazil.
(In reply to Andrew Overholt [:overholt] from comment #11)
> I just spoke about this with Kan-Ru.  It appears that we have not yet
> determined if this affects MMS and A-GPS on Telefonica in Brazil.

I am not aware about the requirements for MMS and A-GPS in Brazil but I could shed some light here. I'll do it as soon as possible.
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #12)
> (In reply to Andrew Overholt [:overholt] from comment #11)
> > I just spoke about this with Kan-Ru.  It appears that we have not yet
> > determined if this affects MMS and A-GPS on Telefonica in Brazil.
> 
> I am not aware about the requirements for MMS and A-GPS in Brazil but I
> could shed some light here. I'll do it as soon as possible.

Thanks, José!
(In reply to Andrew Overholt [:overholt] from comment #13)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #12)
> > (In reply to Andrew Overholt [:overholt] from comment #11)
> > > I just spoke about this with Kan-Ru.  It appears that we have not yet
> > > determined if this affects MMS and A-GPS on Telefonica in Brazil.
> > 
> > I am not aware about the requirements for MMS and A-GPS in Brazil but I
> > could shed some light here. I'll do it as soon as possible.
> 
> Thanks, José!

Let me introduce Beatriz (cc'ing) from Telefonica, she knows about operator requirement stuff. She will comment about the requirements.
This is the case of Brasil and many other operators:
The AGPS service goes through APN-internet (no dedicated APN here)
The MMS service goes through dedicated mms APN (APN-mms)

 
Other operators can use only ONE APN for all the services (but different proxy)
Thanks for the information, Beatriz.
This is the WIP code that does the following:

1. When MMS APN connected:
- Don't set default route and don't change system default DNS addresses
- Add routing rules for MMS APN DNS addresses

2. When MMS APN disconnected:
- Remove routing rules for MMS APN DNS addresses


It was tested by hardcoding default type of RILNetworkInterface to Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS in RadioInterfaceLayer.js.

Before MMS APN connected:

root@android:/ # ip route
default via 10.247.32.1 dev wlan0 
default via 10.247.32.1 dev wlan0  metric 313 
10.247.32.0/21 dev wlan0  proto kernel  scope link  src 10.247.37.238  metric 313 

After MMS APN connected:

root@android:/ # ip route
default via 10.49.49.1 dev rmnet0 
default via 10.247.32.1 dev wlan0 
default via 10.247.32.1 dev wlan0  metric 313 
10.1.7.1 via 10.49.49.1 dev rmnet0 
10.1.7.2 via 10.49.49.1 dev rmnet0 
10.49.49.0/26 dev rmnet0  proto kernel  scope link  src 10.49.49.31 
10.247.32.0/21 dev wlan0  proto kernel  scope link  src 10.247.37.238  metric 313 


One thing strange is, the default route for MMS APN is not supposed to be set.
There are two default routes, one for wlan0 and one for rmnet0.  
According to log, setDefaultRouteAndDNS() was called just one for wlan0.
It's set by somewhere else than setDefaultRouteAndDNS(), I'm still checking who sets the default route for rmnet0(maybe rild layer?).  This should be a different bug to be filed.
Attachment #646146 - Flags: feedback?(philipp)
Comment on attachment 646146 [details] [diff] [review]
WIP V1: Add host route rules for DNS on MMS APN

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

::: dom/system/gonk/NetworkManager.js
@@ +23,4 @@
>  const TOPIC_INTERFACE_UNREGISTERED   = "network-interface-unregistered";
>  const TOPIC_DEFAULT_ROUTE_CHANGED    = "network-default-route-changed";
>  
> +const DEBUG = true;

Remove.

@@ +71,5 @@
>      switch (network.state) {
>        case Ci.nsINetworkInterface.NETWORK_STATE_CONNECTED:
> +        // Add host route for DNS addresses on secondary APN
> +        if (network.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS) {
> +          this.addHostRouteForDNS(network);

Should we only ever do this for MMS or perhaps for any network, or at least any network that isn't 3G/Wifi? Reason I'm asking is that we might have more network links (e.g. for A-GPS).

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1722,4 @@
>    // Event timer for connection retries
>    timer: null,
>  
> +  type: Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS,

I don't think you intended this change permanently, did you?

::: dom/system/gonk/net_worker.js
@@ +92,5 @@
>  
> +/**
> + * Add host route for DNS addresses
> + */
> +function addHostRouteForDNS(options) {

So MMS only needs DNS routes? I'm curious to know how it establishes routes to the other hosts.

::: dom/system/gonk/ril_consts.js
@@ +14,4 @@
>   */
>  
>  // Set to true to debug all RIL layers
> +const DEBUG_ALL = true;

Remove.

::: dom/system/gonk/ril_worker.js
@@ +1836,4 @@
>        return;
>      }
>  
> +    let token = Buf.newParcel(REQUEST_DEACTIVATE_DATA_CALL, options);

This is already in the patch for bug 777284. Please remove this.

::: dom/system/gonk/systemlibs.js
@@ +167,5 @@
> +                                   ctypes.int,       // return value
> +                                   ctypes.char.ptr,  // ifname
> +                                   ctypes.char.ptr,  // dst
> +                                   ctypes.int,       // prefix_length
> +                                   ctypes.char.ptr), // gw

Nit: please align parameters
Attachment #646146 - Flags: feedback?(philipp) → feedback+
(In reply to Philipp von Weitershausen [:philikon] from comment #18)
> @@ +71,5 @@
> >      switch (network.state) {
> >        case Ci.nsINetworkInterface.NETWORK_STATE_CONNECTED:
> > +        // Add host route for DNS addresses on secondary APN
> > +        if (network.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS) {
> > +          this.addHostRouteForDNS(network);
> 
> Should we only ever do this for MMS or perhaps for any network, or at least
> any network that isn't 3G/Wifi? Reason I'm asking is that we might have more
> network links (e.g. for A-GPS).

Yes, A-GPS to be considered later.
Many ISPs has mixed types in one APN.  For example, many cases have the Internet and A-GPS using same APN.  A simple way is to add default route if the APN contains a Internet type, and add host route rules in addition when it contains type MMS or A-GPS.

> 
> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +1722,4 @@
> >    // Event timer for connection retries
> >    timer: null,
> >  
> > +  type: Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS,
> 
> I don't think you intended this change permanently, did you?
> 
No, not intended to change it.  It's for debugging purpose.  It will not be needed after Bug 772747 implemented.

> ::: dom/system/gonk/net_worker.js
> @@ +92,5 @@
> >  
> > +/**
> > + * Add host route for DNS addresses
> > + */
> > +function addHostRouteForDNS(options) {
> 
> So MMS only needs DNS routes? I'm curious to know how it establishes routes
> to the other hosts.

Besides DNS, below are the routing rules I think we should add:
1. DNS addresses
2. MMS Proxy address
3. HTTP Proxy address

> 
> ::: dom/system/gonk/systemlibs.js
> @@ +167,5 @@
> > +                                   ctypes.int,       // return value
> > +                                   ctypes.char.ptr,  // ifname
> > +                                   ctypes.char.ptr,  // dst
> > +                                   ctypes.int,       // prefix_length
> > +                                   ctypes.char.ptr), // gw
> 
> Nit: please align parameters

OK, thanks.
(In reply to Shian-Yow Wu from comment #19)
> > > +/**
> > > + * Add host route for DNS addresses
> > > + */
> > > +function addHostRouteForDNS(options) {
> > 
> > So MMS only needs DNS routes? I'm curious to know how it establishes routes
> > to the other hosts.
> 
> Besides DNS, below are the routing rules I think we should add:
> 1. DNS addresses
> 2. MMS Proxy address
> 3. HTTP Proxy address

Right, ok. We probably want to extend nsINetworkInterface to provide this info to the NetworkManager. We should also consider exposing the IP address, gateway, DNS servers, etc. through that interface.
Depends on: 767375
No longer depends on: 772747
Depends on: 772747
(In reply to Philipp von Weitershausen [:philikon] from comment #20)
> (In reply to Shian-Yow Wu from comment #19)
> > > > +/**
> > > > + * Add host route for DNS addresses
> > > > + */
> > > > +function addHostRouteForDNS(options) {
> > > 
> > > So MMS only needs DNS routes? I'm curious to know how it establishes routes
> > > to the other hosts.
> > 
> > Besides DNS, below are the routing rules I think we should add:
> > 1. DNS addresses
> > 2. MMS Proxy address
> > 3. HTTP Proxy address
> 
> Right, ok. We probably want to extend nsINetworkInterface to provide this
> info to the NetworkManager. We should also consider exposing the IP address,
> gateway, DNS servers, etc. through that interface.

Hi Shian-Yow & Philipp, MMS Proxy address is referenced internally by MmsService only for now and it should be already available through Settings API by the time the connection is setup. If there is another app trying to implement MMS protocol, it can still retrieve that address through the same way. Besides, you may also need a MMSC address to communicate with MMSC, does it mean we'll also add MMSC address to nsINetworkInterface? IMHO, we should add host routing for mms featured connections and expose generic http proxy address only.
Good point, Vicamo. So let's not add MMS proxy then.
Priority: -- → P1
Depends on: 782513
Attached patch WIP V2 (obsolete) — — Splinter Review 
Previous comments addressed.
Assignee: kchen → swu
Attachment #646146 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #652039 - Flags: feedback?(philipp)
Comment on attachment 652039 [details] [diff] [review]
WIP V2

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

::: dom/system/gonk/NetworkManager.js
@@ +235,5 @@
> +      dns1: network.dns1,
> +      dns2: network.dns2,
> +      gateway: network.gateway,
> +      httpproxy: network.httpProxyHost,
> +      mmsproxy: Services.prefs.getCharPref("ril.data.mmsproxy")

We should no longer use prefs as an intermediary settings storage. Also, I don't think "MMS Proxy" is best grouped under the "ril.data". "ril.mmsproxy" or even "ril.mms.proxy" seems better to me.

Vincent is adding settings handling to NetworkManager as part of bug 735547. You might want to coordinate with him, perhaps work on top of his patch. It's very close to being done.

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1746,5 @@
>  
> +  NETWORK_TYPE_WIFI:        Ci.nsINetworkInterface.NETWORK_TYPE_WIFI,
> +  NETWORK_TYPE_MOBILE:      Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE,
> +  NETWORK_TYPE_MOBILE_MMS:  Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS,
> +  NETWORK_TYPE_MOBILE_SUPL: Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL,

Also update WifiNetworkInterface, please.

::: dom/system/gonk/nsINetworkManager.idl
@@ +27,5 @@
>  
>    const long NETWORK_TYPE_WIFI        = 0;
>    const long NETWORK_TYPE_MOBILE      = 1;
>    const long NETWORK_TYPE_MOBILE_MMS  = 2;
> +  const long NETWORK_TYPE_MOBILE_SUPL = 3;

What's SUPL for?
Attachment #652039 - Flags: feedback?(philipp) → review+
(In reply to Philipp von Weitershausen [:philikon] from comment #24)
> We should no longer use prefs as an intermediary settings storage. Also, I
> don't think "MMS Proxy" is best grouped under the "ril.data". "ril.mmsproxy"
> or even "ril.mms.proxy" seems better to me.

Yes, will change to get from settings.
We may have to consider together with Bug 772747 for multiple APN types.  For MMS, there are mmsc, mmsproxy, and mmsport.  As these settings are related to APN, may I know why you think they shouldn't be grouped under "ril.data"?

> Vincent is adding settings handling to NetworkManager as part of bug 735547.
> You might want to coordinate with him, perhaps work on top of his patch.
> It's very close to being done.

Yes, will do.

> ::: dom/system/gonk/RadioInterfaceLayer.js
> @@ +1746,5 @@
> >  
> > +  NETWORK_TYPE_WIFI:        Ci.nsINetworkInterface.NETWORK_TYPE_WIFI,
> > +  NETWORK_TYPE_MOBILE:      Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE,
> > +  NETWORK_TYPE_MOBILE_MMS:  Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS,
> > +  NETWORK_TYPE_MOBILE_SUPL: Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL,
> 
> Also update WifiNetworkInterface, please.

Ok, thanks for your remind.

> >    const long NETWORK_TYPE_WIFI        = 0;
> >    const long NETWORK_TYPE_MOBILE      = 1;
> >    const long NETWORK_TYPE_MOBILE_MMS  = 2;
> > +  const long NETWORK_TYPE_MOBILE_SUPL = 3;
> 
> What's SUPL for?

It's network type for A-GPS.
Whiteboard: [LOE:M]
Is this ready land now?
(In reply to Philipp von Weitershausen [:philikon] from comment #26)
> Is this ready land now?

To change mmsproxy using settings on top of Vincent's setting handler in bug 735547, we need to wait bug 735547 to land first before landing final patch for this bug.  I suggest that we land prefs based patch for this bug first, then after bug 735547 landed we change mmsproxy to use settings in bug 772747.  How do you think?
Just provide what I knew on Android.

1. For each main radio type (TYPE_WIFI, TYPE_BT [PAN profile] and TYPE_MOBILE [default APN]), and it's sub-type (TYPE_WIFI_P2P, TYPE_MOBILE_FOTA, TYPE_MOBILE_MMS), the relationship of priorities will be defined on config.xml.

2. For each radio type, they will not be co-existed simutaneouly.
   And the existing one is decided by priority. (connectivity service)

3. Only main radio type's connection will trigger connectivity service set default routing via netd. 

4. The sub type of radio connections can be multiple eanbled and will only set specific routing. (Supporting multiple and simultaneous APNs)

5. In connection setup process, each radio service will set up it's connection individually then notify the connectivity service (no routing table change in this moment).
   Then connectivity service will decide to keep connection alive or not by priorities and whether it is sub type or not.

6. Ex: BT PAN > Wifi > Mobile
   a. Mobile Connected -> Routing from Mobile
   b. Wifi Connected -> Routing from Wifi and teardown Mobile
   c. Wifi Disconnected -> Re-connect Mobile then Routing from Mobile
   d. Mobile_MMS Connected -> Default Routing from Mobile and Specific routing for MMS.

7. No relationship with tethering, because tethering is treated device as a router for routing packets from outside to device's default routing.
Attached patch WIP V3 — — Splinter Review 
1. Rebase and address previous review comments.
2. Changing mmsproxy to get from settings to be handled in bug 772747.
Attachment #652039 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9da3b8110084
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [LOE:M] → [LOE:M] [mno11]
blocking-b2g: --- → shira+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: