Push IPv6 addresses to libjingle PortAllocator.

Push IPv6 addresses to libjingle PortAllocator.

Currently we have disabled IPv6 support in libjingle, but
there is effort going on to support it. This is the one of the first step in that process.

TBR=sergeyu@chromium.org
BUG=https://code.google.com/p/webrtc/issues/detail?id=1406

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233473

[Ronghua Wu (Left Chromium), 2013-10-18 23:38:28]

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
File content/renderer/p2p/ipc_network_manager.cc (right):

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
content/renderer/p2p/ipc_network_manager.cc:57: it->name, it->name,
talk_base::IPAddress(address), 32);
Should we use NetworkInterface::network_prefix as the prefix_length?

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
content/renderer/p2p/ipc_network_manager.cc:65: it->name, it->name, ip6_addr,
64);
dito

[Mallinath (Gone from Chromium), 2013-10-19 00:09:39]

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
File content/renderer/p2p/ipc_network_manager.cc (right):

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
content/renderer/p2p/ipc_network_manager.cc:57: it->name, it->name,
talk_base::IPAddress(address), 32);
Now it's more aligned with the libjingle native code and meaning of prefix and
prefix length.

On 2013/10/18 23:38:28, Ronghua Wu wrote:
> Should we use NetworkInterface::network_prefix as the prefix_length?

https://codereview.chromium.org/30193002/diff/1/content/renderer/p2p/ipc_netw...
content/renderer/p2p/ipc_network_manager.cc:65: it->name, it->name, ip6_addr,
64);
same as above.
On 2013/10/18 23:38:28, Ronghua Wu wrote:
> dito

[Ronghua Wu (Left Chromium), 2013-10-21 16:49:57]

The rest lg, but I think we need a different way to calculate prefix_length.

https://codereview.chromium.org/30193002/diff/170001/content/renderer/p2p/ipc...
File content/renderer/p2p/ipc_network_manager.cc (right):

https://codereview.chromium.org/30193002/diff/170001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_network_manager.cc:57: int prefix_length =
talk_base::CountIPMaskBits(ip4_addr);
Is the CountIPMaskBits (returns the number of contiguously set bits) the right
function for this?

Looking at:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

For example I think talk_base::CountIPMaskBits("192.168.1.2") will equal 2.

[Mallinath (Gone from Chromium), 2013-10-21 16:55:46]

https://codereview.chromium.org/30193002/diff/170001/content/renderer/p2p/ipc...
File content/renderer/p2p/ipc_network_manager.cc (right):

https://codereview.chromium.org/30193002/diff/170001/content/renderer/p2p/ipc...
content/renderer/p2p/ipc_network_manager.cc:57: int prefix_length =
talk_base::CountIPMaskBits(ip4_addr);
Agree. Prefix length should be used for only identify ip address types. Hence I
think it should work. 

As it's used for only identifying the IP address, we need to distinguish IP4 and
IP6, so it should not be a problem if we just use 32 and 64 value rather than
using CountIPMaskBits.

FYI googletalk plugin uses CountIPMaskBits and it seems to be working for IPv4
addresses so far.

On 2013/10/21 16:49:58, Ronghua Wu wrote:
> Is the CountIPMaskBits (returns the number of contiguously set bits) the right
> function for this?
> 
> Looking at:
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...
> 
> For example I think talk_base::CountIPMaskBits("192.168.1.2") will equal 2.

[Mallinath (Gone from Chromium), 2013-10-23 16:31:50]

Adding Justin, who can advice on prefix length in Network and it's role. Justin,
In this CL I am trying to do calculate prefix length based on plugin code using
CountIPMaskBits, but it doesn't seems to be right the one. Also can we just use
32 for IPv4 and 64 IPv6 as prefix length values?

[juberti, 2013-10-23 16:40:12]

I need to go back and refresh my memory on IPv6 prefixes, but IIRC the
actual prefix is meaningful, and not just for v4/v6 detection.


On Wed, Oct 23, 2013 at 9:31 AM, <mallinath@chromium.org> wrote:

> Adding Justin, who can advice on prefix length in Network and it's role.
> Justin,
> In this CL I am trying to do calculate prefix length based on plugin code
> using
> CountIPMaskBits, but it doesn't seems to be right the one. Also can we
> just use
> 32 for IPv4 and 64 IPv6 as prefix length values?
>
>
>
>
https://codereview.chromium.**org/30193002/<https://codereview.chromium.org/3...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Mallinath (Gone from Chromium), 2013-10-29 00:50:35]

On 2013/10/23 16:40:12, juberti wrote:
> I need to go back and refresh my memory on IPv6 prefixes, but IIRC the
> actual prefix is meaningful, and not just for v4/v6 detection.

True, But I don't think there is anything significant usage of that in
network.cc.
I could see it's only used in CompareNetworks
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

Either we can just do 32 for IPv4 and 64 for IPv6 or do similar to what's done 
in libjingle in network.cc

> 
> 
> On Wed, Oct 23, 2013 at 9:31 AM, <mailto:mallinath@chromium.org> wrote:
> 
> > Adding Justin, who can advice on prefix length in Network and it's role.
> > Justin,
> > In this CL I am trying to do calculate prefix length based on plugin code
> > using
> > CountIPMaskBits, but it doesn't seems to be right the one. Also can we
> > just use
> > 32 for IPv4 and 64 IPv6 as prefix length values?
> >
> >
> >
> >
>
https://codereview.chromium.**org/30193002/%3Chttps://codereview.chromium.org...>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Ronghua Wu (Left Chromium), 2013-10-29 17:57:34]

On 2013/10/23 16:31:50, mallinath2 wrote:
> Adding Justin, who can advice on prefix length in Network and it's role.
Justin,
> In this CL I am trying to do calculate prefix length based on plugin code
using
> CountIPMaskBits, but it doesn't seems to be right the one. Also can we just
use
> 32 for IPv4 and 64 IPv6 as prefix length values?

The different is in network.cc, the CountIPMaskBits is called with mask, not the
address. But it seems chrome's NetworkInterface gives network_prefix instead of
mask. So I think CountIPMaskBits is not a option here.

If NetworkInterface's network_prefix is not set, I agree to just use some fix
value like 32 64.

[Mallinath (Gone from Chromium), 2013-11-04 18:24:52]

On 2013/10/29 17:57:34, Ronghua Wu wrote:
> On 2013/10/23 16:31:50, mallinath2 wrote:
> > Adding Justin, who can advice on prefix length in Network and it's role.
> Justin,
> > In this CL I am trying to do calculate prefix length based on plugin code
> using
> > CountIPMaskBits, but it doesn't seems to be right the one. Also can we just
> use
> > 32 for IPv4 and 64 IPv6 as prefix length values?
> 
> The different is in network.cc, the CountIPMaskBits is called with mask, not
the
> address. But it seems chrome's NetworkInterface gives network_prefix instead
of
> mask. So I think CountIPMaskBits is not a option here.
> 
> If NetworkInterface's network_prefix is not set, I agree to just use some fix
> value like 32 64.

Done, using simple 32 and 64 to differentiate.

[Ronghua Wu (Left Chromium), 2013-11-04 21:57:21]

LGTM

Please make a comment in code what are the magic number 32 and 64.

[commit-bot: I haz the power, 2013-11-06 06:46:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mallinath@chromium.org/30193002/400001

[commit-bot: I haz the power, 2013-11-06 07:07:20]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[commit-bot: I haz the power, 2013-11-06 07:19:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mallinath@chromium.org/30193002/400001

[commit-bot: I haz the power, 2013-11-06 10:41:37]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-06 18:40:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mallinath@chromium.org/30193002/400001

[commit-bot: I haz the power, 2013-11-06 23:43:40]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-07 00:04:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mallinath@chromium.org/30193002/400001

[commit-bot: I haz the power, 2013-11-07 01:53:56]

Change committed as 233473