804531 - B2G 3G: When primary APN available, default route should not be set to secondary APN

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Shian-Yow Wu [:swu]]

During testing two APNs, sometimes default route was set to secondary APN, instead of primary data APN.

Comment 1

[Shian-Yow Wu [:swu]]

This may affect bug 801540, which is blocking-basecamp+.

Comment 2

[Vincent Liu[:vliu]]

Hi Shian-Yow,

Can you please offer the way how you reproduce this bug? Thanks.

Comment 3

[Shian-Yow Wu [:swu]]

(In reply to vliu from comment #2)
> Hi Shian-Yow,
> 
> Can you please offer the way how you reproduce this bug? Thanks.

STR:
1. Connect to Data APN
2. Check /proc/net/route and default route is rmnet0.
3. Connect to MMS APN
4. Check /proc/net/route.  The default route should stay in rmnet0, but it changed to rmnet1.

Comment 4

[Shian-Yow Wu [:swu]]

Attachment: V1

The original implementation avoids setting default route and DNS to secondary APNs(MMS, SUPL).  There was a bug to use 'oldActive.type', should be changed to check 'this.active.type' instead.

Besides this fix, I think we can make it better by following further changes:
1. If only secondary APNs (MMS, SUPL) connected, we still set default route and DNS for the secondary APN network.  
2. We don't need to set this limitation when 'this._overriddenActive' is true.  But the value true here means we really want it to be overridden.

Comment 5

[Andrew Overholt [:overholt]]

(In reply to Shian-Yow Wu from comment #1)
> This may affect bug 801540, which is blocking-basecamp+.

I don't think we can block on it until we know if it does affect bug 801540.  Please re-nom when you find out.  Thanks :)

Comment 6

[Philipp von Weitershausen [:philikon]]

Comment on attachment 674577 [details] [diff] [review]
V1

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

::: dom/system/gonk/NetworkManager.js
@@ +374,5 @@
> +      // If default data APN is not connected, we still set default route
> +      // and DNS on seconary APN.
> +      if (defaultDataNetwork && 
> +          (this.active.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_MMS ||
> +          this.active.type == Ci.nsINetworkInterface.NETWORK_TYPE_MOBILE_SUPL) &&

Indent this line 1 more space to make it clear it's *inside* of the parentheses.

(Also, nit: trailing space two lines up)

Comment 7

[Shian-Yow Wu [:swu]]

(In reply to Andrew Overholt [:overholt] from comment #5)
> (In reply to Shian-Yow Wu from comment #1)
> > This may affect bug 801540, which is blocking-basecamp+.
> 
> I don't think we can block on it until we know if it does affect bug 801540.
> Please re-nom when you find out.  Thanks :)

If the secondary APN takes over default route from primary APN, user will not be able to access Internet.  So I should say that we need to fix this bug to avoid Internet access been affect by bug 801540.

Comment 8

[Shian-Yow Wu [:swu]]

Attachment: Patch V2, r=philikon

Review comments addressed.

Comment 9

[Andrew Overholt [:overholt]]

Blocking based on comment #7.

Comment 10

[Shian-Yow Wu [:swu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a43543174e89

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a43543174e89

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c85b9855d7e8