Closed
Bug 957917
Opened 11 years ago
Closed 11 years ago
[IPv6] To support IPv6 in RIL.
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S3 (14mar)
People
(Reporter: wesley_huang, Assigned: vicamo)
References
Details
(Whiteboard: [ucid: , 1.4, FT:RIL][ipv6])
User Story
As a user, I should have my FireFoxOS device supporting IPv6.
Attachments
(2 files, 15 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
hsinyi
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•11 years ago
|
User Story: (updated)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → vyang
Flags: needinfo?(vyang)
Reporter | ||
Updated•11 years ago
|
Blocks: b2g-ipv6-1.4
Blocks: b2g--telephony-1.4
Assignee | ||
Comment 2•11 years ago
|
||
There is still dozens of networking errors in KitKat emulator. I should have them fixed first for a basic networking environment. For KitKat emulator builds, see bug 957526.
Updated•11 years ago
|
blocking-b2g: --- → 1.4+
Reporter | ||
Updated•11 years ago
|
Whiteboard: [ucid: , 1.4, FT:RIL]
Comment 3•11 years ago
|
||
This bug is supported to change the interface between ril and network manager.
Blocks: b2g-ril-interface
Reporter | ||
Updated•11 years ago
|
Target Milestone: --- → 1.4 S3 (14mar)
Reporter | ||
Updated•11 years ago
|
Flags: in-testsuite?
Flags: in-moztrap?(echang)
Reporter | ||
Comment 4•11 years ago
|
||
user story or feature work are not blocker.
blocking-b2g: 1.4+ → backlog
Updated•11 years ago
|
blocking-b2g: backlog → 1.3T+
Updated•11 years ago
|
Summary: [IPv6] Add IPv6 support for FireFox OS → [IPv6] Add IPv6 support for Firefox OS
Reporter | ||
Comment 6•11 years ago
|
||
Target milestone set to 2/28. Remove 1.4+ as this is user story.
blocking-b2g: 1.4+ → ---
Target Milestone: 1.4 S3 (14mar) → 1.4 S2 (28feb)
Assignee | ||
Comment 7•11 years ago
|
||
Assignee | ||
Comment 8•11 years ago
|
||
Depending on bug 871475 for tests. However it was backed-out because of causing perm-orange of one reftest and one crashtest. If that can't meet the schedule, I'll remove it from the dependency list and file a follow-up for test cases.
Depends on: 871475
Comment 10•11 years ago
|
||
Make this bug clear. This bug is to get IPv6 relevant information from network. And Bug 975813 is to configure IPv6 through network manager.
Depends on: 975813
Summary: [IPv6] Add IPv6 support for Firefox OS → [IPv6] To support IPv6 in RIL.
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 8379464 [details] Github pull request for hardware/ril No longer needed because <PDP_type> parsing is merged into bug 871475.
Attachment #8379464 -
Attachment is obsolete: true
Comment 12•11 years ago
|
||
Per comment 11 marking this resolved fixed.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 13•11 years ago
|
||
I think we still need this bug, but it isn't in the blocker list provided by partner.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
No longer blocks: b2g--telephony-1.4
Assignee | ||
Comment 14•11 years ago
|
||
Assignee | ||
Comment 15•11 years ago
|
||
https://github.com/vicamo/b2g_platform_hardware_ril/tree/bugzilla/957917/firefoxos-ipv6 https://github.com/vicamo/b2g_platform_external_qemu/tree/bugzilla/957917/firefoxos-ipv6 qemu gives ipv6 addresses as expected, rild crashes :X
Comment 16•11 years ago
|
||
http://mzl.la/1c73sCh IPv6+LTE
Flags: in-moztrap?(echang) → in-moztrap+
QA Contact: echang
Assignee | ||
Comment 17•11 years ago
|
||
This PR depends on bug 871475, which was backed out and is not fixed yet. The PR here is only for test purpose.
Assignee | ||
Comment 18•11 years ago
|
||
This PR depends on bug 871475, which was backed out and is not fixed yet. The PR here is only for test purpose.
Assignee | ||
Comment 19•11 years ago
|
||
Assignee | ||
Comment 20•11 years ago
|
||
So far we have two things to do: 1) handle undefined dns2, 2) handle undefined httpProxyHost.
Assignee | ||
Comment 21•11 years ago
|
||
Support roamingProtocol as well.
Attachment #8379454 -
Attachment is obsolete: true
Assignee | ||
Comment 22•11 years ago
|
||
Attachment #8382924 -
Attachment is obsolete: true
Attachment #8383095 -
Flags: review?(htsai)
Assignee | ||
Updated•11 years ago
|
Attachment #8382895 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8382899 -
Attachment is obsolete: true
Assignee | ||
Comment 23•11 years ago
|
||
The same patch set doesn't work on emulator-jb. :X D/NetUtils( 46): ifc_add_route(eth1, 2001:200::a00:202, 128, 2001:200::a00:202) = 0 E/NetUtils( 46): getaddrinfo failed: invalid gateway eth1 D/NetUtils( 46): ifc_remove_route(eth1, ::, 0, eth1) = -22 D/NetUtils( 46): ifc_init_returning 0 D/NetUtils( 46): ifc_close D/NetUtils( 46): ifc_add_route(eth1, 2001:200::a00:202, 128, 2001:200::a00:202) = -113 E/NetUtils( 46): getaddrinfo failed: invalid gateway eth1 D/NetUtils( 46): ifc_remove_route(eth1, ::, 0, eth1) = -22 E/NetUtils( 46): getaddrinfo failed: invalid gateway eth1 D/NetUtils( 46): ifc_add_route(eth1, ::, 0, eth1) = -22 D/NetUtils( 36): ifc_init_returning 0
Assignee | ||
Comment 24•11 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #23) > The same patch set doesn't work on emulator-jb. :X Looks like emulator rild has done his job well. Somehow that |aOptions.gateway| becomes "eth1". $ adb shell ip addr show 3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP qlen 1000 link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff inet6 2001:200::a00:264/64 scope global valid_lft forever preferred_lft forever inet6 fe80::5054:ff:fe12:3457/64 scope link valid_lft forever preferred_lft forever $ adb shell ip link show 3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP qlen 1000 link/ether 52:54:00:12:34:57 brd ff:ff:ff:ff:ff:ff $ adb shell ip route show 10.0.2.0/24 dev eth0 proto kernel scope link src 10.0.2.15 default via 10.0.2.2 dev eth0 metric 2
Assignee | ||
Comment 25•11 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #24) > (In reply to Vicamo Yang [:vicamo][:vyang] from comment #23) > > The same patch set doesn't work on emulator-jb. :X > > Looks like emulator rild has done his job well. Somehow that > |aOptions.gateway| becomes "eth1". Root cause found and fixed in bug 975813 attachment 8383200 [details] [diff] [review]. Basically GET_CHAR() marco returns a C string pointer owned by a temporarily, in-stack object.
Comment 26•11 years ago
|
||
Comment on attachment 8383095 [details] [diff] [review] [Gecko] read PDP_type from APN settings Review of attachment 8383095 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_consts.js @@ +2368,5 @@ > +this.RIL_DATACALL_PDP_TYPES = [ > + GECKO_DATACALL_PDP_TYPE_IP, > + GECKO_DATACALL_PDP_TYPE_IPV6, > +]; > + Vicamo, the valid values for the roaming and roaming_protocol properties in the APNs are 'none' (this one would means there is no value), 'IP', 'IPV6', and 'IPV4V6' as they are the possible valid values that these properties have in the AOSP database and in our apn.json database.
Assignee | ||
Comment 28•11 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #26) > Vicamo, the valid values for the roaming and roaming_protocol properties in > the APNs are 'none' (this one would means there is no value), 'IP', 'IPV6', > and 'IPV4V6' as they are the possible valid values that these properties > have in the AOSP database and in our apn.json database. Hi, we're not going to have IPV4V6 in v1.4, and we fallback to "IP" given "none" is passed.
Assignee | ||
Comment 30•11 years ago
|
||
Comment on attachment 8382886 [details] Github PR for platform/external/qemu [DO NOT SUBMIT] Move to bug 978071.
Attachment #8382886 -
Attachment is obsolete: true
Assignee | ||
Comment 31•11 years ago
|
||
Comment on attachment 8382889 [details] Github pull request for platform/hardware/ril [DO NOT SUBMIT] Move to bug 978071.
Attachment #8382889 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8383136 -
Attachment is obsolete: true
Assignee | ||
Comment 32•11 years ago
|
||
1) use 'roaming_protocol' as Gaia does. 2) fix missing a '!' in protocol/roaming_protocol selection
Attachment #8383095 -
Attachment is obsolete: true
Attachment #8383095 -
Flags: review?(htsai)
Attachment #8383701 -
Flags: review?(htsai)
Assignee | ||
Comment 33•11 years ago
|
||
This patch is currently disabled because of bug 978071. It passes with pull requests in that bug pulled into external/qemu and hardware/ril.
Attachment #8383550 -
Attachment is obsolete: true
Attachment #8383702 -
Flags: review?(htsai)
Comment 35•11 years ago
|
||
Comment on attachment 8383701 [details] [diff] [review] part 1/2: read PDP_type from APN settings Review of attachment 8383701 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +4513,5 @@ > } > authType = RIL.RIL_DATACALL_AUTH_TO_GECKO.indexOf(RIL.GECKO_DATACALL_AUTH_DEFAULT); > } > + let pdptype = !radioInterface.rilContext.data.roaming > + ? this.apnSetting.protocol : this.apnSetting.roaming_protocol; nit: the style usually used is making ? and : align ::: dom/system/gonk/ril_consts.js @@ +2365,5 @@ > +this.GECKO_DATACALL_PDP_TYPE_IPV6 = "IPV6"; > +this.GECKO_DATACALL_PDP_TYPE_DEFAULT = GECKO_DATACALL_PDP_TYPE_IP; > +this.RIL_DATACALL_PDP_TYPES = [ > + GECKO_DATACALL_PDP_TYPE_IP, > + GECKO_DATACALL_PDP_TYPE_IPV6, Looks like type 'IPV4V6' is missing? ::: dom/system/gonk/ril_worker.js @@ +3962,5 @@ > } > currentDataCall.gw = updatedDataCall.gw; > if (updatedDataCall.dns) { > + for (let i = 0; i < updatedDataCall.dns.length; i++) { > + currentDataCall.dns.push(updatedDataCall.dns[i]); This line makes updated dns attended to existing currentDataCall.dns. Is this expected behaviour?
Attachment #8383701 -
Flags: review?(htsai)
Assignee | ||
Comment 36•11 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #35) > Looks like type 'IPV4V6' is missing? We don't have IPV4V6 now. Can't find the correct format through AOSP source code. Need of real hardware references. Will file a follow-up and place a comment here instead. > ::: dom/system/gonk/ril_worker.js > @@ +3962,5 @@ > > } > > currentDataCall.gw = updatedDataCall.gw; > > if (updatedDataCall.dns) { > > + for (let i = 0; i < updatedDataCall.dns.length; i++) { > > + currentDataCall.dns.push(updatedDataCall.dns[i]); > > This line makes updated dns attended to existing currentDataCall.dns. Is > this expected behaviour? Oops! A reset to |currentDataCall.dns| is missed. :X
Comment 37•11 years ago
|
||
Comment on attachment 8383702 [details] [diff] [review] part 2/2: test cases Review of attachment 8383702 [details] [diff] [review]: ----------------------------------------------------------------- Please file a followup to replace mobile_head.js with head.js. ::: dom/mobileconnection/tests/marionette/head.js @@ +10,5 @@ > +let Promise = Cu.import("resource://gre/modules/Promise.jsm").Promise; > + > +let _pendingEmulatorCmdCount = 0; > + > +/* Send emulator command with safe guard. nit: the format we are using is /* * Comment starts... * ... * comment ends. */ Please make the style consistent all over the file. @@ +50,5 @@ > + * > + * Resolve if that mozSettings value is retrieved successfully, reject > + * otherwise. > + * > + * Forfill params: s/Forfill/Fulfill @@ +82,5 @@ > +/* Set mozSettings values. > + * > + * Resolve if that mozSettings value is set successfully, reject otherwise. > + * > + * Forfill params: (none) ditto. @@ +113,5 @@ > +/* Set mozSettings value with only one key. > + * > + * Resolve if that mozSettings value is set successfully, reject otherwise. > + * > + * Forfill params: (none) ditto. @@ +205,5 @@ > + SpecialPowers.pushPermissions(permissions, function() { > + ok(true, "permissions pushed: " + JSON.stringify(permissions)); > + > + // Permission changes can't change existing Navigator.prototype > + // objects, so grab our objects from a new Navigator nit: place a period at the end of the line @@ +237,5 @@ > +/* Wait for one named MobileConnection event. > + * > + * Resolve if that named event occurs. Never reject. > + * > + * Forfill params: the DOMEvent passed. ditto. @@ +261,5 @@ > +/* Set data connection enabling state and wait for "datachange" event. > + * > + * Resolve if data connection state changed to the expected one. Never reject. > + * > + * Forfill params: (none) ditto. @@ +281,5 @@ > + deferred.resolve(); > + return; > + } > + > + return waitForManagerEvent("datachange").then(keepWaiting); Any case that we don't treat it as a failure even we receive 'connected' not equal to expected 'aEnabled?' @@ +282,5 @@ > + return; > + } > + > + return waitForManagerEvent("datachange").then(keepWaiting); > + }) A semicolon is missing. @@ +289,5 @@ > +} > + > +/* Set voice/data roaming emulation and wait for state change. > + * > + * Forfill params: (none) ditto. @@ +366,5 @@ > + * This function ensures global |mobileConnection| variable is available during > + * the process and performs clean-ups as well. > + * > + * @param aTestCaseMain > + * A function that take one parameter -- mobileConnection. nit: s/take/takes/ ::: dom/mobileconnection/tests/marionette/test_mobile_data_ipv6.js @@ +12,5 @@ > + let nm = getNetworkManager(); > + let active = nm.active; > + ok(active, "Active network interface"); > + > + log(" Interface: " + active.name); nit: remove extra ws after " @@ +13,5 @@ > + let active = nm.active; > + ok(active, "Active network interface"); > + > + log(" Interface: " + active.name); > + log(" Address: " + active.ip); ditto. @@ +26,5 @@ > + > +function doTestHome(aApnSettings, aProtocol) { > + log("Testing \"" + aProtocol + "\"@HOME... "); > + > + aApnSettings[0][0].protocol = aProtocol; Please comment [0][0]. @@ +35,5 @@ > + > +function doTestRoaming(aApnSettings, aRoaminProtocol) { > + log("Testing \"" + aRoaminProtocol + "\"@ROMAING... "); > + > + delete aApnSettings[0][0].protocol; ditto. @@ +46,5 @@ > + let origApnSettings; > + > + return setDataRoamingEnabled(true) > + .then(getDataApnSettings) > + .then(function(aResult) { Not very big deal, but sometime verbose style is used in a promise chain while sometimes the compact one is. Any guideline of the distinction? Why not use the same one over the code?
Attachment #8383702 -
Flags: review?(htsai)
Comment 38•11 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #36) > We don't have IPV4V6 now. Can't find the correct format through AOSP source > code. Need of real hardware references. Will file a follow-up and place a > comment here instead. File a follow-up bug, bug 978711.
Assignee | ||
Comment 39•11 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #37) > ::: dom/mobileconnection/tests/marionette/head.js > @@ +10,5 @@ > > +let Promise = Cu.import("resource://gre/modules/Promise.jsm").Promise; > > + > > +let _pendingEmulatorCmdCount = 0; > > + > > +/* Send emulator command with safe guard. > > nit: the format we are using is > > /* > * Comment starts... > * ... > * comment ends. > */ > > Please make the style consistent all over the file. I think you mean: /** * Blah ... */ > @@ +50,5 @@ > > + * > > + * Resolve if that mozSettings value is retrieved successfully, reject > > + * otherwise. > > + * > > + * Forfill params: > > s/Forfill/Fulfill Will fix "dom/bluetooth/tests/marionette/head.js" as well. > @@ +281,5 @@ > > + deferred.resolve(); > > + return; > > + } > > + > > + return waitForManagerEvent("datachange").then(keepWaiting); > > Any case that we don't treat it as a failure even we receive 'connected' not > equal to expected 'aEnabled?' Don't get you quite well. I do check |connected == aEnabled| here. > ::: dom/mobileconnection/tests/marionette/test_mobile_data_ipv6.js > @@ +12,5 @@ > > + let nm = getNetworkManager(); > > + let active = nm.active; > > + ok(active, "Active network interface"); > > + > > + log(" Interface: " + active.name); > > nit: remove extra ws after " Why? > @@ +46,5 @@ > > + let origApnSettings; > > + > > + return setDataRoamingEnabled(true) > > + .then(getDataApnSettings) > > + .then(function(aResult) { > > Not very big deal, but sometime verbose style is used in a promise chain > while sometimes the compact one is. Any guideline of the distinction? Why > not use the same one over the code? I personally use compact style when the action to take is a simple, state-less call that doesn't depend on the resolved value of previous run.
Assignee | ||
Comment 40•11 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #37) > Please file a followup to replace mobile_head.js with head.js. Filed as bug 979134.
Assignee | ||
Comment 41•11 years ago
|
||
Address comment 35.
Attachment #8383701 -
Attachment is obsolete: true
Attachment #8385195 -
Flags: review?(htsai)
Assignee | ||
Comment 42•11 years ago
|
||
Address review comments.
Attachment #8383702 -
Attachment is obsolete: true
Attachment #8385197 -
Flags: review?(htsai)
Comment 43•11 years ago
|
||
Comment on attachment 8385195 [details] [diff] [review] part 1/2: read PDP_type from APN settings : v2 Review of attachment 8385195 [details] [diff] [review]: ----------------------------------------------------------------- Thank you!
Attachment #8385195 -
Flags: review?(htsai) → review+
Comment 44•11 years ago
|
||
Comment on attachment 8385197 [details] [diff] [review] part 2/2: test cases : v2 Review of attachment 8385197 [details] [diff] [review]: ----------------------------------------------------------------- Awesome!
Attachment #8385197 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 45•11 years ago
|
||
Add a few more comments in |setDataEnabledAndWait()| since hsinyi still raised a question about it.
Attachment #8385197 -
Attachment is obsolete: true
Assignee | ||
Comment 46•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/901717199390 https://hg.mozilla.org/integration/b2g-inbound/rev/a7ea2d51414d
Comment 47•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/901717199390 https://hg.mozilla.org/mozilla-central/rev/a7ea2d51414d
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Comment 48•11 years ago
|
||
Backed out for causing bug 980375. https://hg.mozilla.org/mozilla-central/rev/8095b7dd8f58
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 1.4 S2 (28feb) → ---
Comment 49•11 years ago
|
||
We should think about a case that a device may not support IPv6. See https://bugzilla.mozilla.org/show_bug.cgi?id=980375#c11. So, even apnSetting.protocol is IPv6, we should still use "IP" as default while the device itself is lack of ipv6 capability.
Assignee | ||
Comment 50•11 years ago
|
||
Only assign pdpType from protocol or roaming_protocol when a new RIL quirk "ro.moz.ril.ipv6" is set to "true".
Attachment #8385195 -
Attachment is obsolete: true
Updated•11 years ago
|
Whiteboard: [ucid: , 1.4, FT:RIL] → [ucid: , 1.4, FT:RIL][ipv6]
Assignee | ||
Updated•11 years ago
|
Attachment #8388338 -
Flags: review?(htsai)
Comment 51•11 years ago
|
||
Comment on attachment 8388338 [details] [diff] [review] part 1/2: read PDP_type from APN settings : v3 Review of attachment 8388338 [details] [diff] [review]: ----------------------------------------------------------------- Thank you :)
Attachment #8388338 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 52•11 years ago
|
||
Verified by applying PRs in bug 978071 with and without the third PR, attachment 8389016 [details]. When the third PR was applied, test cases here passed as before; when without, two test cases failed because it got an IPv4 address instead of the expected IPv6 one.
Assignee | ||
Comment 53•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/cbf30c2f215b https://hg.mozilla.org/integration/b2g-inbound/rev/765c7b0b461d
Comment 54•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cbf30c2f215b https://hg.mozilla.org/mozilla-central/rev/765c7b0b461d
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S3 (14mar)
You need to log in
before you can comment on or make changes to this bug.
Description
•