Closed
Bug 975813
Opened 11 years ago
Closed 11 years ago
[IPv6] To support IPv6 in network manager.
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S2 (28feb)
People
(Reporter: kchang, Assigned: chucklee)
References
Details
(Whiteboard: [FT:RIL])
Attachments
(3 files, 13 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
To support IPv6 for partner's requirements, we need to support IPv6 in network manager. RIL needs to get the the relevant information of IPv6 from carrier's network and pass this information to network manager, and then network manager will do the configuration accordingly.
Reporter | ||
Comment 2•11 years ago
|
||
Anshul, Can you provide the information of IPv6 which commercial RIL wants to pass to network manager for this bug? Thanks.
Flags: needinfo?(anshulj)
Reporter | ||
Comment 3•11 years ago
|
||
(In reply to Ken Chang[:ken] from comment #2) > Anshul, Can you provide the information of IPv6 which commercial RIL wants > to pass to network manager for this bug? Thanks. We already support IPv6 feature in Necko. Currently, we are not sure if IPv6 are supported in network manager. But we think it dons. May you have a trial? If there is any problem occurred, please let us know what problem you have in network manager for IPv6.
Updated•11 years ago
|
Whiteboard: [FT:RIL]
Target Milestone: --- → 1.4 S2 (28feb)
Assignee | ||
Comment 4•11 years ago
|
||
It seems that gecko do nothing about settings IP of data call, maybe done by rild. In Network Utils: |setDefaultRouteAndDNS()| and |removeNetworkRoute()| might fail because it uses inet_addr(). DHCP and DNS related functions might fail if gonk doesn't support. Others like |SetHostRoute()| seems fine. So for Network Manager, the main problem is handling IPv6 routing rules, like set default route.
Reporter | ||
Comment 5•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #4) > It seems that gecko do nothing about settings IP of data call, maybe done by > rild. > In Network Utils: > |setDefaultRouteAndDNS()| and |removeNetworkRoute()| might fail because it > uses inet_addr(). Thanks for this analysation, but let's check what we have to do after getting the test result which partner provide.
Status: NEW → ASSIGNED
> D/DHCP ( 216): ifc_add_route: different address families: 0.0.0.0 and 2002:c023:9c17:4135:5c23:4743:df47:0eca
> D/DHCP ( 216): getaddrinfo failed: invalid address
> D/NetUtils( 216): failed to remove default route for rmnet0: No such process
> D/NetUtils( 216): failed to add 255.255.255.255 as default route for rmnet0: No such process
> D/DHCP ( 216): ifc_add_route: different address families: 0.0.0.0 and 2002:c023:9c17:4135:5c23:4743:df47:0eca
> D/DHCP ( 216): getaddrinfo failed: invalid address
> D/NetUtils( 216): failed to remove default route for rmnet0: No such process
> D/NetUtils( 216): failed to add 255.255.255.255 as default route for rmnet0: No such process
> E/BandwidthController( 209): No such iface rmnet0 to delete
As :chucklee mentioned, some of the network utils code expects an ipv4 address and doesn't know what to do with ipv6-formatted addresses.
To answer an earlier question, RIL will be providing the same information as when on IPv4, which is IP address, gateways, DNS, netmask, and broadcast.
The network manage could distinguish between IPv4 and IPv6 simply based on the format & length of the IP address, or nsIRilNetworkInterface could be expanded to include an explicit GetVersion() function.
Flags: needinfo?(pgravel)
One additional note: APN information will need to be expanded to allow users to explicitly set IPv4 vs IPv6.
Reporter | ||
Comment 9•11 years ago
|
||
Thanks for your help. Chuck will base on your result and review his patch. May need your help to test his patch. Thanks. (In reply to pgravel from comment #8) > One additional note: APN information will need to be expanded to allow users > to explicitly set IPv4 vs IPv6. Carrier said they didn't need seeting for configuring ipv4/ipv6, so we don't plan to have this in 1.4.
Comment 10•11 years ago
|
||
(In reply to Ken Chang[:ken] from comment #9) > Thanks for your help. > Chuck will base on your result and review his patch. May need your help to > test his patch. Thanks. > > (In reply to pgravel from comment #8) > > One additional note: APN information will need to be expanded to allow users > > to explicitly set IPv4 vs IPv6. > Carrier said they didn't need seeting for configuring ipv4/ipv6, so we don't > plan to have this in 1.4. Sure, but just to make it clear, this information still needs to be passed to gecko as part of the APN settings so modem know to bring up an IPv4, IPv6 or both
Reporter | ||
Comment 11•11 years ago
|
||
(In reply to Anshul from comment #10) > to gecko as part of the APN settings so modem know to bring up an IPv4, IPv6 > or both. For now, gecko only sets both for this setting.
Reporter | ||
Comment 12•11 years ago
|
||
(In reply to Ken Chang[:ken] from comment #11) > For now, gecko only sets both for this setting. By the way, protocol setting have been passed to RIL now, commercial RIL can directly use this protocol setting.
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Anshul from comment #10) > (In reply to Ken Chang[:ken] from comment #9) > > Thanks for your help. > > Chuck will base on your result and review his patch. May need your help to > > test his patch. Thanks. > > > > (In reply to pgravel from comment #8) > > > One additional note: APN information will need to be expanded to allow users > > > to explicitly set IPv4 vs IPv6. > > Carrier said they didn't need seeting for configuring ipv4/ipv6, so we don't > > plan to have this in 1.4. > Sure, but just to make it clear, this information still needs to be passed > to gecko as part of the APN settings so modem know to bring up an IPv4, IPv6 > or both Hi Anshul, If it's set to both, does modem bring up both IPv4 and IPv6 information at the same time if carrier supports both, or just one of them?
Flags: needinfo?(anshulj)
Assignee | ||
Comment 14•11 years ago
|
||
Attachment #8381335 -
Attachment is obsolete: true
Attachment #8381950 -
Flags: feedback?(vyang)
Attachment #8381950 -
Flags: feedback?(anshulj)
Assignee | ||
Comment 16•11 years ago
|
||
Assignee | ||
Comment 17•11 years ago
|
||
Assignee | ||
Comment 18•11 years ago
|
||
I overwrite the IP, prefix, and gateway of data connection interface while it's connected in the Test patch(attachment 8381951 [details] [diff] [review]), and apply test on unagi. IP and Routing table of IPv6 for data connection(rmnet0) are not set before enabling data connection , as attachment 8381952 [details]. After enabling data connection, IPv6 address "20010db80000f1010000000000000002" is assigned to rmnet0(I did this in test patch, should be rild I think), and two routing rules(subnet route 20010db80000f1010000000000000000 and default 00000000000000000000000000000000) are added to IPv6 routing table through rmnet0. I think the WIP patch might work with IPv6 data connection.
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #14) > Created attachment 8381950 [details] [diff] [review] > WIP - 0001. Support IPv6 in NetworkUtils. Note: In this patch, IPv6 address expects prefix of subnet instead of netmask.
Comment 20•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #13) > (In reply to Anshul from comment #10) > > (In reply to Ken Chang[:ken] from comment #9) > > > Thanks for your help. > > > Chuck will base on your result and review his patch. May need your help to > > > test his patch. Thanks. > > > > > > (In reply to pgravel from comment #8) > > > > One additional note: APN information will need to be expanded to allow users > > > > to explicitly set IPv4 vs IPv6. > > > Carrier said they didn't need seeting for configuring ipv4/ipv6, so we don't > > > plan to have this in 1.4. > > Sure, but just to make it clear, this information still needs to be passed > > to gecko as part of the APN settings so modem know to bring up an IPv4, IPv6 > > or both > Hi Anshul, > If it's set to both, does modem bring up both IPv4 and IPv6 information at > the same time if carrier supports both, or just one of them? Depends on the network, but both is possible. Example: > UNSOL_DATA_CALL_LIST_CHANGED { ver=9, num=1, connection[0]:{ver : 0x9, status : 0, retry : -1, cid : 0, active : 1, type : 'IPV4V6', ifname : 'rmnet0', addresses : '["172.20.184.116/29","2002:c023:9c17:41aa:2285:0e0d:f30d:5726/64"]', dnses : '["192.35.156.212","192.35.156.19","2001:04f8:0000:0002:0000:0000:0000:0014"]', gateways : '["172.20.184.117","2002:c023:9c17:41aa:68bf:8e28:689e:4cf5"]'}
Flags: needinfo?(anshulj)
Assignee | ||
Comment 21•11 years ago
|
||
Set IPv6 DNS based on https://code.google.com/p/android/issues/detail?id=3389#c30.
Attachment #8381950 -
Attachment is obsolete: true
Attachment #8381950 -
Flags: feedback?(vyang)
Attachment #8381950 -
Flags: feedback?(anshulj)
Attachment #8382740 -
Flags: review?(vyang)
Attachment #8382740 -
Flags: review?(anshulj)
Assignee | ||
Comment 22•11 years ago
|
||
Add IPv6 DNS address.
Attachment #8381951 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Attachment #8382740 -
Flags: review?(vyang)
Attachment #8382740 -
Flags: review?(anshulj)
Attachment #8382740 -
Flags: feedback?(vyang)
Attachment #8382740 -
Flags: feedback?(anshulj)
Assignee | ||
Comment 23•11 years ago
|
||
Modify based on bug 976959
Attachment #8382740 -
Attachment is obsolete: true
Attachment #8382740 -
Flags: feedback?(vyang)
Attachment #8382740 -
Flags: feedback?(anshulj)
Attachment #8382867 -
Flags: feedback?(vyang)
Attachment #8382867 -
Flags: feedback?(anshulj)
Assignee | ||
Comment 24•11 years ago
|
||
Modify IPv6 simulation parameter.
Attachment #8382741 -
Attachment is obsolete: true
Comment 25•11 years ago
|
||
Comment on attachment 8382867 [details] [diff] [review] WIP - 0001. Support IPv6 in NetworkUtils. Review of attachment 8382867 [details] [diff] [review]: ----------------------------------------------------------------- Need to handle IPv6 and empty hostname in addHostRoute/removeHostRoute as well.
Attachment #8382867 -
Flags: feedback?(vyang) → feedback+
Assignee | ||
Comment 26•11 years ago
|
||
Add hostname chekc per comment 25.
Attachment #8382867 -
Attachment is obsolete: true
Attachment #8382867 -
Flags: feedback?(anshulj)
Attachment #8382930 -
Flags: review?(vyang)
Attachment #8382930 -
Flags: feedback?(anshulj)
Comment 28•11 years ago
|
||
Comment on attachment 8382930 [details] [diff] [review] Support IPv6 in NetworkUtils. Review of attachment 8382930 [details] [diff] [review]: ----------------------------------------------------------------- D/NetUtils( 45): failed to remove default route for eth1: No such process Looks like we need to support IPv6 in NetworkUtils::removeDefaultRoute, too. We'll need an additional parameter 'gateway', so probably we should have nsINetworkService::removeDefaultRoute accept a nsINetworkInterface instead.
Attachment #8382930 -
Flags: review?(vyang)
Updated•11 years ago
|
Blocks: b2g-ril-interface
Assignee | ||
Comment 29•11 years ago
|
||
Fix NetworkUtils::removeDefaultRoute()
Attachment #8382930 -
Attachment is obsolete: true
Attachment #8382930 -
Flags: feedback?(anshulj)
Assignee | ||
Updated•11 years ago
|
Attachment #8382971 -
Flags: review?(vyang)
Reporter | ||
Comment 30•11 years ago
|
||
Comment on attachment 8382971 [details] [diff] [review] Support IPv6 in NetworkUtils. V2 Philippe, could you please also test this patch to see if it is workable?
Attachment #8382971 -
Flags: feedback?(pgravel)
Reporter | ||
Comment 31•11 years ago
|
||
(In reply to Ken Chang[:ken] from comment #30) > Comment on attachment 8382971 [details] [diff] [review] > Support IPv6 in NetworkUtils. V2 > > Philippe, could you please also test this patch to see if it is workable? You also need to base on the patch of bug 976959. Thanks.
Comment 32•11 years ago
|
||
Comment on attachment 8382971 [details] [diff] [review] Support IPv6 in NetworkUtils. V2 Review of attachment 8382971 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkUtils.cpp @@ +1162,5 @@ > */ > bool NetworkUtils::removeDefaultRoute(NetworkParams& aOptions) > { > + // Get gateway address. > + const char *gateway = GET_CHAR(mGateway); nit: append "removeDefaultRoute" to the comments of NetworkCommandOptions::gateway in "dom/webidl/NetworkOptions.webidl". @@ +1261,5 @@ > + > + if (type == AF_INET6) { > + // Calculate subnet. > + unsigned char ipBuf[16] = {0}, subnetBuf[16] = {0}; > + if (inet_pton(AF_INET6, ipStr, ipBuf) != 1) { struct in6_addr in6; if (inet_pton(AF_INET6, ipStr, &in6) != 1) { @@ +1274,5 @@ > + subnetBuf[i] |= (mask & ipBuf[i]); > + mask >>= 1; > + prefixLength--; > + } > + } uint32_t p, i, p1, mask; p = prefixLength; i = 0; while (i < 4) { p1 = p > 32 ? 32 : p; p -= p1; mask = p1 ? ~0x0 << (32 - p1) : 0; in6.s6_addr32[i++] &= htonl(mask); } @@ +1277,5 @@ > + } > + } > + > + char subnet[INET6_ADDRSTRLEN]; > + if (!inet_ntop(AF_INET6, subnetBuf, subnet, INET6_ADDRSTRLEN)) { char subnetStr[INET6_ADDRSTRLEN]; if (!inet_ntop(AF_INET6, &in6, subnetStr, sizeof subnetStr)) { @@ +1285,5 @@ > + // Remove default route. > + mNetUtils->do_ifc_remove_route(GET_CHAR(mIfname), "::", 0, NULL); > + > + // Remove subnet route. > + mNetUtils->do_ifc_remove_route(GET_CHAR(mIfname), subnet, prefixLength, NULL); nit: subnetStr
Attachment #8382971 -
Flags: review?(vyang) → feedback+
Comment 33•11 years ago
|
||
Address my previous comments and fix the problem reported in bug 957917 comment 23.
Attachment #8382971 -
Attachment is obsolete: true
Attachment #8382971 -
Flags: feedback?(pgravel)
Attachment #8383200 -
Flags: review?(fabrice)
Comment 34•11 years ago
|
||
Comment on attachment 8383200 [details] [diff] [review] Support IPv6 in NetworkUtils. V3 Review of attachment 8383200 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nits addressed ::: dom/system/gonk/NetworkService.js @@ +11,5 @@ > Cu.import("resource://gre/modules/NetUtil.jsm"); > Cu.import("resource://gre/modules/FileUtils.jsm"); > > const NETWORKSERVICE_CONTRACTID = "@mozilla.org/network/service;1"; > +const NETWORKSERVICE_CID = Components.ID("{f96461fa-e844-45d2-a6c3-8cd23ab0916b}"); no need to change that. ::: dom/system/gonk/NetworkService.manifest @@ +1,3 @@ > # NetworkService.js > +component {f96461fa-e844-45d2-a6c3-8cd23ab0916b} NetworkService.js > +contract @mozilla.org/network/service;1 {f96461fa-e844-45d2-a6c3-8cd23ab0916b} No need to change these. ::: dom/system/gonk/NetworkUtils.cpp @@ +289,5 @@ > snprintf(key, PROPERTY_KEY_MAX - 1, "net.%s.dns2", ifname); > property_get(key, prop.dns2, ""); > } > > +static int getIpType(const char *ip) { nit: s/ip/aIp
Attachment #8383200 -
Flags: review?(fabrice) → review+
Comment 35•11 years ago
|
||
Comment on attachment 8383200 [details] [diff] [review] Support IPv6 in NetworkUtils. V3 Review of attachment 8383200 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkUtils.cpp @@ +1126,5 @@ > + snprintf(key, sizeof key - 1, "net.%s.gw", autoIfname.get()); > + property_get(key, gateway, ""); > + } else { > + MOZ_ASSERT(strlen(GET_CHAR(mGateway_str)) < sizeof gateway); > + strcpy(gateway, GET_CHAR(mGateway_str)); Please use strncpy as the MOZ_ASSERT is a no-op in release builds.
Assignee | ||
Comment 36•11 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #34) > Comment on attachment 8383200 [details] [diff] [review] > Support IPv6 in NetworkUtils. V3 > > Review of attachment 8383200 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me with nits addressed > > ::: dom/system/gonk/NetworkService.js > @@ +11,5 @@ > > Cu.import("resource://gre/modules/NetUtil.jsm"); > > Cu.import("resource://gre/modules/FileUtils.jsm"); > > > > const NETWORKSERVICE_CONTRACTID = "@mozilla.org/network/service;1"; > > +const NETWORKSERVICE_CID = Components.ID("{f96461fa-e844-45d2-a6c3-8cd23ab0916b}"); > > no need to change that. > > ::: dom/system/gonk/NetworkService.manifest > @@ +1,3 @@ > > # NetworkService.js > > +component {f96461fa-e844-45d2-a6c3-8cd23ab0916b} NetworkService.js > > +contract @mozilla.org/network/service;1 {f96461fa-e844-45d2-a6c3-8cd23ab0916b} > > No need to change these. If I don't change these values, I got black screen on device (Unagi).
Assignee | ||
Comment 38•11 years ago
|
||
Address comment 34 and comment 35, except the uuid part.
Attachment #8383200 -
Attachment is obsolete: true
Comment 40•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #36) > If I don't change these values, I got black screen on device (Unagi). @@ -242,7 +242,7 @@ SystemWorkerManager::RegisterNfcWorker(JS::Handle<JS::Value> aWorker, nsresult SystemWorkerManager::InitNetd(JSContext *cx) { - nsCOMPtr<nsIWorkerHolder> worker = do_GetService(kNetworkServiceCID); + nsCOMPtr<nsIWorkerHolder> worker = do_GetService("@mozilla.org/network/service;1"); NS_ENSURE_TRUE(worker, NS_ERROR_FAILURE); mNetdWorker = worker; return NS_OK;
Comment 41•11 years ago
|
||
Comment on attachment 8383480 [details] [diff] [review] Support IPv6 in Network Manager. V4 Review of attachment 8383480 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/NetworkService.js @@ +309,1 @@ > if(DEBUG) debug("Remove default route for " + ifname); nit: network.name
Comment 42•11 years ago
|
||
(In reply to Chuck Lee [:chucklee] from comment #36) > If I don't change these values, I got black screen on device (Unagi). Filed bug 977995 to completely remove NetworkService stuff from SystemWorkerManager.
Assignee | ||
Comment 43•11 years ago
|
||
Address comment 40 and comment 41, thanks to Vicamo!
Attachment #8383480 -
Attachment is obsolete: true
Assignee | ||
Comment 45•11 years ago
|
||
Rebase based on bug 960865.
Attachment #8382934 -
Attachment is obsolete: true
Attachment #8383521 -
Attachment is obsolete: true
Assignee | ||
Comment 46•11 years ago
|
||
I think the oranges are not related to this patch, let's check in.
Keywords: checkin-needed
Comment 47•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/e9b4e110a6a7
Keywords: checkin-needed
Reporter | ||
Comment 48•11 years ago
|
||
(In reply to Edgar Chen [:edgar][:echen] from comment #47) > https://hg.mozilla.org/integration/b2g-inbound/rev/e9b4e110a6a7 Thanks, Edgar.
Comment 49•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e9b4e110a6a7
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Blocks: b2g-ipv6-1.4
You need to log in
before you can comment on or make changes to this bug.
Description
•