code review 12416043: net: implement TCP connection setup with fast failover

net: implement TCP connection setup with fast failover

This CL adds minimal support of Happy Eyeballs-like TCP connection
setup to Dialer API. Happy Eyeballs and derivation techniques are
described in the following:

- Happy Eyeballs: Success with Dual-Stack Hosts
  http://tools.ietf.org/html/rfc6555

- Analysing Dual Stack Behaviour and IPv6 Quality
  http://www.potaroo.net/presentations/2012-04-17-dual-stack-quality.pdf

Usually, the techniques consist of three components below.

- DNS query racers, that run A and AAAA queries in parallel or series
- A short list of destination addresses
- TCP SYN racers, that run IPv4 and IPv6 transport in parallel or series

This CL implements only the latter two. The existing DNS query
component gathers together A and AAAA records in series, so we don't
touch it here. This CL just uses extended resolveInternetAddr and makes
it possible to run multiple Dial racers in parallel.

For example, when the given destination is a DNS name and the name has
multiple address family A and AAAA records, and it happens on the TCP
wildcard network "tcp" with DualStack=true like the following:

(&net.Dialer{DualStack: true}).Dial("tcp", "www.example.com:80")

The function will return a first established connection either TCP over
IPv4 or TCP over IPv6, and close the other connection internally.

Fixes issue 3610.
Fixes issue 5267.


Benchmark results on freebsd/amd64 virtual machine, tip vs. tip+12416043:

benchmark                           old ns/op    new ns/op    delta
BenchmarkTCP4OneShot                    50696        52141   +2.85%
BenchmarkTCP4OneShotTimeout             65775        66426   +0.99%
BenchmarkTCP4Persistent                 10986        10457   -4.82%
BenchmarkTCP4PersistentTimeout          11207        10445   -6.80%
BenchmarkTCP6OneShot                    62009        63718   +2.76%
BenchmarkTCP6OneShotTimeout             78351        79138   +1.00%
BenchmarkTCP6Persistent                 14695        14659   -0.24%
BenchmarkTCP6PersistentTimeout          15032        14646   -2.57%
BenchmarkTCP4ConcurrentReadWrite         7215         6217  -13.83%
BenchmarkTCP6ConcurrentReadWrite         7528         7493   -0.46%

benchmark                          old allocs   new allocs    delta
BenchmarkTCP4OneShot                       36           36    0.00%
BenchmarkTCP4OneShotTimeout                36           36    0.00%
BenchmarkTCP4Persistent                     0            0     n/a%
BenchmarkTCP4PersistentTimeout              0            0     n/a%
BenchmarkTCP6OneShot                       37           37    0.00%
BenchmarkTCP6OneShotTimeout                37           37    0.00%
BenchmarkTCP6Persistent                     0            0     n/a%
BenchmarkTCP6PersistentTimeout              0            0     n/a%
BenchmarkTCP4ConcurrentReadWrite            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite            0            0     n/a%

benchmark                           old bytes    new bytes    delta
BenchmarkTCP4OneShot                     2500         2503    0.12%
BenchmarkTCP4OneShotTimeout              2508         2505   -0.12%
BenchmarkTCP4Persistent                     0            0     n/a%
BenchmarkTCP4PersistentTimeout              0            0     n/a%
BenchmarkTCP6OneShot                     2713         2707   -0.22%
BenchmarkTCP6OneShotTimeout              2722         2720   -0.07%
BenchmarkTCP6Persistent                     0            0     n/a%
BenchmarkTCP6PersistentTimeout              0            0     n/a%
BenchmarkTCP4ConcurrentReadWrite            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite            0            0     n/a%

[mikio, 2013-08-28 08:39:28]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[bradfitz, 2013-08-29 00:24:06]

This CL has gone from big to split up into multiple medium ones, back to
one, but even bigger.

Please, let's just do one thing at a time:

1) send a CL just adding and using the internal "type netaddr interface"
but don't even define the type addrList []netaddr yet.

Then _after_:

2) Add type addrList []netaddr and return it from that one (or two?) places.

Then _after_:

Who knows. But I don't want to see happy eyeballs in the same CL yet.




On Wed, Aug 28, 2013 at 1:39 AM, <mikioh.mikioh@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> net: TCP connection setup with fast failover on dual IP stack node
>
> This CL adds minimal support of Happy Eyeballs-like TCP connection
> setup to Dial, DialTimeout and Dial of Dialer API. Happy Eyeballs and
> derivation techniques are described in the following:
>
> - Happy Eyeballs: Success with Dual-Stack Hosts
>   http://tools.ietf.org/html/**rfc6555<http://tools.ietf.org/html/rfc6555>
>
> - Analysing Dual Stack Behaviour and IPv6 Quality
>   http://www.potaroo.net/**presentations/2012-04-17-dual-**
>
stack-quality.pdf<http://www.potaroo.net/presentations/2012-04-17-dual-stack-quality.pdf>
>
> Usually, the techniques consist of three components below.
>
> - DNS query racers, that run A and AAAA queries in parallel or series
> - A short list of destination addresses
> - TCP SYN racers, that run IPv4 and IPv6 transport in parallel or series
>
> This CL implements only the latter two. The existing DNS query
> component gathers together A and AAAA records in series, so we don't
> touch it here. This CL just extends resolveInternetAddr for returning
> a short list of addresses that contain a pair of different address
> family IP addresses if possible, also makes it possible to run two
> Dial racers in parallel.
>
> For example, when the given destination is a DNS registered name and
> the name has multiple address family A and AAAA records, and it
> happens on the TCP wildcard network "tcp" like the following:
>
> net.Dial("tcp", "www.example.com:80")
> net.DialTimeout("tcp", "www.example.com:80", time.Second)
> (&net.Dialer{}).Dial("tcp", "www.example.com:80")
>
> Each of the functions will return a first established connection either
> TCP over IPv4 or TCP over IPv6, and close the other connection
> internally.
>
> Fixes issue 3610.
> Fixes issue 5267.
>
>
> Benchmark results on freebsd/amd64 virtual machine, tip vs.
> tip+12416043:
>
> benchmark                           old ns/op    new ns/op    delta
> BenchmarkTCP4OneShot                    53487        53575   +0.16%
> BenchmarkTCP4OneShotTimeout             67187        68320   +1.69%
> BenchmarkTCP4Persistent                 11155        11610   +4.08%
> BenchmarkTCP4PersistentTimeout          11203        11217   +0.12%
> BenchmarkTCP6OneShot                    65360        66376   +1.55%
> BenchmarkTCP6OneShotTimeout             80664        81265   +0.75%
> BenchmarkTCP6Persistent                 14324        14692   +2.57%
> BenchmarkTCP6PersistentTimeout          14355        14623   +1.87%
> BenchmarkTCP4ConcurrentReadWri**te         6400         6015   -6.02%
> BenchmarkTCP6ConcurrentReadWri**te         7428         7287   -1.90%
>
> benchmark                          old allocs   new allocs    delta
> BenchmarkTCP4OneShot                       36           36    0.00%
> BenchmarkTCP4OneShotTimeout                36           36    0.00%
> BenchmarkTCP4Persistent                     0            0     n/a%
> BenchmarkTCP4PersistentTimeout              0            0     n/a%
> BenchmarkTCP6OneShot                       37           37    0.00%
> BenchmarkTCP6OneShotTimeout                37           37    0.00%
> BenchmarkTCP6Persistent                     0            0     n/a%
> BenchmarkTCP6PersistentTimeout              0            0     n/a%
> BenchmarkTCP4ConcurrentReadWri**te            0            0     n/a%
> BenchmarkTCP6ConcurrentReadWri**te            0            0     n/a%
>
> benchmark                           old bytes    new bytes    delta
> BenchmarkTCP4OneShot                     2490         2498    0.32%
> BenchmarkTCP4OneShotTimeout              2504         2499   -0.20%
> BenchmarkTCP4Persistent                     0            0     n/a%
> BenchmarkTCP4PersistentTimeout              0            0     n/a%
> BenchmarkTCP6OneShot                     2720         2720    0.00%
> BenchmarkTCP6OneShotTimeout              2710         2722    0.44%
> BenchmarkTCP6Persistent                     0            0     n/a%
> BenchmarkTCP6PersistentTimeout              0            0     n/a%
> BenchmarkTCP4ConcurrentReadWri**te            0            0     n/a%
> BenchmarkTCP6ConcurrentReadWri**te            0            0     n/a%
>
>
> Benchmark results on freebsd/amd64 virtual machine, go1.1.2 vs.
> tip+12416043:
>
> benchmark                         old ns/op    new ns/op    delta
> BenchmarkTCP4OneShot                6801599        51255  -99.25%
> BenchmarkTCP4OneShotTimeout        11397359        67348  -99.41%
> BenchmarkTCP4Persistent            12798667        10669  -99.92%
> BenchmarkTCP4PersistentTimeout     12199423        11173  -99.91%
> BenchmarkTCP6OneShot                6990032        66149  -99.05%
> BenchmarkTCP6OneShotTimeout         8794427        77048  -99.12%
> BenchmarkTCP6Persistent            11520045        14405  -99.87%
> BenchmarkTCP6PersistentTimeout      6997559        14629  -99.79%
>
> benchmark                        old allocs   new allocs    delta
> BenchmarkTCP4OneShot                     43           36  -16.28%
> BenchmarkTCP4OneShotTimeout              44           36  -18.18%
> BenchmarkTCP4Persistent                   4            0  -100.00%
> BenchmarkTCP4PersistentTimeout            2            0  -100.00%
> BenchmarkTCP6OneShot                     44           36  -18.18%
> BenchmarkTCP6OneShotTimeout              44           37  -15.91%
> BenchmarkTCP6Persistent                   2            0  -100.00%
> BenchmarkTCP6PersistentTimeout            4            0  -100.00%
>
> benchmark                         old bytes    new bytes    delta
> BenchmarkTCP4OneShot                   3275         2498  -23.73%
> BenchmarkTCP4OneShotTimeout            3301         2500  -24.27%
> BenchmarkTCP4Persistent                 282            0  -100.00%
> BenchmarkTCP4PersistentTimeout          115            0  -100.00%
> BenchmarkTCP6OneShot                   3411         2700  -20.84%
> BenchmarkTCP6OneShotTimeout            3420         2710  -20.76%
> BenchmarkTCP6Persistent                 119            0  -100.00%
> BenchmarkTCP6PersistentTimeout          289            0  -100.00%
>
> Please review this at
https://codereview.appspot.**com/12416043/<https://codereview.appspot.com/124...
>
> Affected files:
>   M src/pkg/net/dial.go
>   M src/pkg/net/dial_gen.go
>   M src/pkg/net/dial_test.go
>   M src/pkg/net/dialgoogle_test.go
>   M src/pkg/net/fd_plan9.go
>   M src/pkg/net/fd_unix.go
>   M src/pkg/net/fd_windows.go
>   M src/pkg/net/ipraw_test.go
>   M src/pkg/net/iprawsock.go
>   M src/pkg/net/iprawsock_posix.go
>   M src/pkg/net/ipsock.go
>   M src/pkg/net/lookup.go
>   M src/pkg/net/sock_posix.go
>   M src/pkg/net/tcp_test.go
>   M src/pkg/net/tcpsock.go
>   M src/pkg/net/tcpsock_posix.go
>   M src/pkg/net/udp_test.go
>   M src/pkg/net/udpsock.go
>   M src/pkg/net/udpsock_posix.go
>   M src/pkg/net/unixsock.go
>   M src/pkg/net/unixsock_posix.go
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[mikio, 2013-08-29 00:55:53]

On Thu, Aug 29, 2013 at 9:24 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> This CL has gone from big to split up into multiple medium ones, back to
> one, but even bigger.

Yup, I was a bit confused, so reassembled CL frags, sent a dumb signal
to golang-dev and waited for someone.
Will do re-splitting. Thanks.

[mikio, 2013-08-31 02:22:31]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[mikio, 2013-08-31 02:52:09]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[mikio, 2013-09-02 07:56:55]

Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[bradfitz, 2013-09-04 17:50:59]

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go
File src/pkg/net/dial.go (right):

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:169: // the list of addresses. It will return a fisrt
established
typo

also: the first, not a first

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:178: // To avoid the situation of resource tempolarily
unavailale
why would we get that error?  This comment creates more questions than answers.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:179: // by using multiple Dial racers, we use both channels
signal
sig and lane

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:183: sig, lane := make(chan bool, 1), make(chan racer, 1)
break these up into separate lines and comment each separately.  maybe that
would help.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:194: defer close(sig)
This feels either racy or too complicated.  But I don't understand the comment
above yet.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/fd_unix.go
File src/pkg/net/fd_unix.go (right):

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/fd_unix.go#ne...
src/pkg/net/fd_unix.go:39: func dialAround(network string, ra Addr, dialer
func(time.Time) (Conn, error), deadline time.Time) (Conn, error) {
what does dialAround do?  I don't understand the name or meaning.

[bradfitz, 2013-09-04 17:51:06]

I think Happy Eyeballs should probably be opt-in (off by default).  You
should have to set a bool on the Dialer struct to get it enabled.



On Mon, Sep 2, 2013 at 12:56 AM, <mikioh.mikioh@gmail.com> wrote:

> Hello golang-dev@googlegroups.com, bradfitz@golang.org (cc:
> golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
https://codereview.appspot.**com/12416043/<https://codereview.appspot.com/124...
>

[ioe, 2013-09-04 20:25:43]

On Wednesday, September 4, 2013 7:51:04 PM UTC+2, Brad Fitzpatrick wrote:

> I think Happy Eyeballs should probably be opt-in (off by default).  You 
> should have to set a bool on the Dialer struct to get it enabled.
>
>
Could you elaborate a bit more, why? I have a feeling the preference 
depends on the program not on the connection.

[bradfitz, 2013-09-05 00:03:13]

On Wed, Sep 4, 2013 at 1:25 PM, Ingo Oeser <nightlyone@googlemail.com>wrote:

> On Wednesday, September 4, 2013 7:51:04 PM UTC+2, Brad Fitzpatrick wrote:
>
>> I think Happy Eyeballs should probably be opt-in (off by default).  You
>> should have to set a bool on the Dialer struct to get it enabled.
>>
>>
> Could you elaborate a bit more, why?
>

Because it changes the meaning of dialing "tcp" pretty significantly,
racing connections.


> I have a feeling the preference depends on the program not on the
> connection.
>

I don't follow. Putting it on the Dialer gives programs an option.

[rsc, 2013-09-06 20:31:01]

I agree with Brad. Please make this opt-in, by using something like

DualStack bool // use RFC 6555 "Happy Eyeballs" simultaneous connect

in the Dialer struct. It should be off by default.

[mikio, 2013-09-10 07:43:51]

PTAL

I named the opt-in knob "MultiAttempt" because we could use it not only for
multiple addresses on multiple address families but for multiple addresses on a
single address family near the future. But honestly, not sure. Suggestions for a
better name welcome.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go
File src/pkg/net/dial.go (right):

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:169: // the list of addresses. It will return a fisrt
established
On 2013/09/04 17:50:59, bradfitz wrote:
> typo
> 
> also: the first, not a first

Done.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:179: // by using multiple Dial racers, we use both channels
signal
On 2013/09/04 17:50:59, bradfitz wrote:
> sig and lane

Done.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/dial.go#newco...
src/pkg/net/dial.go:183: sig, lane := make(chan bool, 1), make(chan racer, 1)
On 2013/09/04 17:50:59, bradfitz wrote:
> break these up into separate lines and comment each separately.  maybe that
> would help.

Done.

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/fd_unix.go
File src/pkg/net/fd_unix.go (right):

https://codereview.appspot.com/12416043/diff/163001/src/pkg/net/fd_unix.go#ne...
src/pkg/net/fd_unix.go:39: func dialAround(network string, ra Addr, dialer
func(time.Time) (Conn, error), deadline time.Time) (Conn, error) {
changed to dial.

[rsc, 2013-09-10 15:42:42]

I think this should be DualStack, not MultiAttempt. MultiAttempt might just
as well mean trying multiple addresses in sequence. We should leave a
separate setting open for when we want to do that (or we might just do it
by default, as other things do). This weird racing two Dials against each
other is really specific to IPv6 and Happy Eyeballs (but we can't call the
bool HappyEyeballs).

Russ

[mikio, 2013-09-10 21:26:37]

PTAL

Changed to DualStack.

[rsc, 2013-09-11 14:43:29]

LGTM

Let's stop changing package net now, please.

[rsc, 2013-09-11 14:48:58]

*** Submitted as https://code.google.com/p/go/source/detail?r=1b195d6fe342 ***

net: implement TCP connection setup with fast failover

This CL adds minimal support of Happy Eyeballs-like TCP connection
setup to Dialer API. Happy Eyeballs and derivation techniques are
described in the following:

- Happy Eyeballs: Success with Dual-Stack Hosts
  http://tools.ietf.org/html/rfc6555

- Analysing Dual Stack Behaviour and IPv6 Quality
  http://www.potaroo.net/presentations/2012-04-17-dual-stack-quality.pdf

Usually, the techniques consist of three components below.

- DNS query racers, that run A and AAAA queries in parallel or series
- A short list of destination addresses
- TCP SYN racers, that run IPv4 and IPv6 transport in parallel or series

This CL implements only the latter two. The existing DNS query
component gathers together A and AAAA records in series, so we don't
touch it here. This CL just uses extended resolveInternetAddr and makes
it possible to run multiple Dial racers in parallel.

For example, when the given destination is a DNS name and the name has
multiple address family A and AAAA records, and it happens on the TCP
wildcard network "tcp" with DualStack=true like the following:

(&net.Dialer{DualStack: true}).Dial("tcp", "www.example.com:80")

The function will return a first established connection either TCP over
IPv4 or TCP over IPv6, and close the other connection internally.

Fixes issue 3610.
Fixes issue 5267.


Benchmark results on freebsd/amd64 virtual machine, tip vs. tip+12416043:

benchmark                           old ns/op    new ns/op    delta
BenchmarkTCP4OneShot                    50696        52141   +2.85%
BenchmarkTCP4OneShotTimeout             65775        66426   +0.99%
BenchmarkTCP4Persistent                 10986        10457   -4.82%
BenchmarkTCP4PersistentTimeout          11207        10445   -6.80%
BenchmarkTCP6OneShot                    62009        63718   +2.76%
BenchmarkTCP6OneShotTimeout             78351        79138   +1.00%
BenchmarkTCP6Persistent                 14695        14659   -0.24%
BenchmarkTCP6PersistentTimeout          15032        14646   -2.57%
BenchmarkTCP4ConcurrentReadWrite         7215         6217  -13.83%
BenchmarkTCP6ConcurrentReadWrite         7528         7493   -0.46%

benchmark                          old allocs   new allocs    delta
BenchmarkTCP4OneShot                       36           36    0.00%
BenchmarkTCP4OneShotTimeout                36           36    0.00%
BenchmarkTCP4Persistent                     0            0     n/a%
BenchmarkTCP4PersistentTimeout              0            0     n/a%
BenchmarkTCP6OneShot                       37           37    0.00%
BenchmarkTCP6OneShotTimeout                37           37    0.00%
BenchmarkTCP6Persistent                     0            0     n/a%
BenchmarkTCP6PersistentTimeout              0            0     n/a%
BenchmarkTCP4ConcurrentReadWrite            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite            0            0     n/a%

benchmark                           old bytes    new bytes    delta
BenchmarkTCP4OneShot                     2500         2503    0.12%
BenchmarkTCP4OneShotTimeout              2508         2505   -0.12%
BenchmarkTCP4Persistent                     0            0     n/a%
BenchmarkTCP4PersistentTimeout              0            0     n/a%
BenchmarkTCP6OneShot                     2713         2707   -0.22%
BenchmarkTCP6OneShotTimeout              2722         2720   -0.07%
BenchmarkTCP6Persistent                     0            0     n/a%
BenchmarkTCP6PersistentTimeout              0            0     n/a%
BenchmarkTCP4ConcurrentReadWrite            0            0     n/a%
BenchmarkTCP6ConcurrentReadWrite            0            0     n/a%

R=golang-dev, bradfitz, nightlyone, rsc
CC=golang-dev
https://codereview.appspot.com/12416043

Committer: Russ Cox <rsc@golang.org>