Shian-Yow Wu [:swu] Assignee
	Description • 12 years ago

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

	Shian-Yow Wu [:swu] Assignee
	Comment 1 • 12 years ago

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

	Vincent Liu[:vliu]
	Comment 2 • 12 years ago

Hi Shian-Yow,

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

	Shian-Yow Wu [:swu] Assignee
	Comment 3 • 12 years ago

(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.

	Shian-Yow Wu [:swu] Assignee
	Comment 4 • 12 years ago

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.

	Andrew Overholt [:overholt]
	Comment 5 • 12 years ago

(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 :)

	Philipp von Weitershausen [:philikon]
	Comment 6 • 12 years ago

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)

	Shian-Yow Wu [:swu] Assignee
	Comment 7 • 12 years ago

(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.

	Shian-Yow Wu [:swu] Assignee
	Comment 8 • 12 years ago

Review comments addressed.

	Andrew Overholt [:overholt]
	Comment 9 • 12 years ago

Blocking based on comment #7.

	Shian-Yow Wu [:swu] Assignee
	Comment 10 • 12 years ago

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

	Ed Morley [:emorley]
	Comment 11 • 12 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 12 years ago

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