1333185 - [e10s] Default route detection fails to connect for IPv6

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, defect, P1)

Description

[Nils Ohlmeier [:drno]]

We try to figure out the default route by creating a UDP socket and connect that socket to Google's DNS servers. As destination we use port 53. This works fine in non-e10s, because we request from NSPR directly to connect the UDP socket. But in case of e10s we got through the UDP Socket server which apparently checks the destination port against the famous list of forbidden ports. And obviously a browser should not try to connect to a DNS server?!
So our default route detection is broken for e10s, because the socket never connects.

Comment 1

[Nils Ohlmeier [:drno]]

Looks like NSPR returns this:

#define PR_HOST_UNREACHABLE_ERROR                (-5927L)

So it's probably not due to the port number.

Comment 2

[Nils Ohlmeier [:drno]]

Looks like we get the unreachable error only for the IPv6 destination.

Comment 3

[Nils Ohlmeier [:drno]]

I still haven't been able to figure out why the connect request on IPv6 fails in nsUDPSocket.cpp. My current suspicion is that it is related to the fact that this: http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/netwerk/base/nsUDPSocket.cpp#666
always initializes |addr| as an IPv4 struct, no matter if the given |aAddr| is IPv4 or IPv6.

But my hacked attempts to "fix" this directly in nsUDPSocket.cpp have not lead to successful connect() call. As all of this work in non-e10s code, so in nr_socket_prsock.cpp I'm pretty sure the problem is somewhere in nsUDPSocket.cpp or UDPSocketParent.cpp.

@schien: do you have any idea what could be going on here?

Comment 4

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Yeah sounds like the same issue I found in bug 1326483 comment #24. I think the root cause is in https://searchfox.org/mozilla-central/rev/ed04f7a44925dace34f2d2e15ef9c0f2809d70b0/dom/network/UDPSocketParent.cpp#313, which we do string-to-PRNetAddress conversion.
You can double check if this is where the weird scopeId comes from.

Comment 5

[Nils Ohlmeier [:drno]]

Thanks schien, yes bug 1330490 fixes the problem.

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

you might want to do manual memset 0 in this bug before bug 1330490 landed. Dont' know how long it'll take for finishing review process.

Comment 7

[Nils Ohlmeier [:drno]]

Attachment: Bug 1333185: memset PRNetAddr for UDPSocket::connect().

Review commit: https://reviewboard.mozilla.org/r/107000/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/107000/

Comment 8

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8830089 [details]
Bug 1333185: memset PRNetAddr for UDPSocket::connect().

https://reviewboard.mozilla.org/r/107000/#review108100

::: netwerk/base/nsUDPSocket.cpp:668
(Diff revision 1)
> +  if (aAddr->raw.family == AF_INET) {
> -  PR_InitializeNetAddr(PR_IpAddrAny, 0, &addr);
> +    PR_InitializeNetAddr(PR_IpAddrAny, 0, &addr);
> +  }

Is this still necessary? since we are doing `NetAddrToPRNetAddr` right after memset zero.

Comment 9

[Nils Ohlmeier [:drno]]

Comment on attachment 8830089 [details]
Bug 1333185: memset PRNetAddr for UDPSocket::connect().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/107000/diff/1-2/

Comment 10

[Nils Ohlmeier [:drno]]

Comment on attachment 8830089 [details]
Bug 1333185: memset PRNetAddr for UDPSocket::connect().

https://reviewboard.mozilla.org/r/107000/#review108100

> Is this still necessary? since we are doing `NetAddrToPRNetAddr` right after memset zero.

You are right. Since this init's it only to IpAddrAny everything gets set to zero anyway.
I removed the PR_InitializeNetAddr() call for now.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0be13c603475
memset PRNetAddr for UDPSocket::connect(). r=schien

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/0be13c603475