Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Dial should follow getaddrinfo address ordering #8453

Closed
pmarks-net opened this issue Jul 31, 2014 · 19 comments
Closed

net: Dial should follow getaddrinfo address ordering #8453

pmarks-net opened this issue Jul 31, 2014 · 19 comments
Milestone

Comments

@pmarks-net
Copy link
Contributor

Forking this from issue #3610.

My experiments show that net.Dial has two modes of operation when connecting to a
dual-stack hostname:

1. By default, it prefers IPv4 :(
2. When using a net.Dialer{DualStack: true}, IPv4 and IPv6 SYNs are sent simultaneously,
and the resulting address family is unpredictable.

The latter violates a "MUST" in the Happy Eyeballs RFC:
http://tools.ietf.org/html/rfc6555#section-4.1

This is harmful for two reasons:
- It increases server load in the common case, by always sending two SYNs.
- Dual-stack clients cannot reliably use IPv6 to route around NATs.  The primary
incentive for a network operator to deploy IPv6 is to reduce NAT load (and therefore
save money), but this sort of client behavior invalidates that incentive.
@pmarks-net
Copy link
Contributor Author

Comment 1:

I'm attaching my simple test client, for reference.

Attachments:

  1. dial.go (300 bytes)

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 2:

related issue: issue #8124. yup, I've been hoping to see some reasonable solution for
dual ip stack related issues but unfortunately the issues never stop growing, sigh.
i guess a few options out there;
a) escaping to another layer: stop trying to solve the issues within the conventional IP
world, and move to the ICN/CCN world ;),
b) burdening users with taxes: introducing user injectable app-network namespace
preference and make use of it in dialing,
c) determining target reachability from app-level through network-layer namespaces: as
suggested in issue #8124, introducing l7-to-l3 reachability poking stuff (like OS X doing
a bit) and use of it in dialing.
ps: you can use "tcp4" or "tcp6" instead of "tcp" if you are sure what's the reliable
address family for packet transportation; yup, usually not sure.

@pmarks-net
Copy link
Contributor Author

Comment 3:

I think you're overcomplicating matters; there's no need for applications to worry about
stuff like reachability.  getaddrinfo() will automatically sort its output based on the
available routes and configured address preference.
Without Happy Eyeballs, net.Dial should behave like a standard unix client:
  Call getaddrinfo()
  for each address {
    Connect to it
    If it fails (e.g. the port is closed), continue
  }
Happy Eyeballs also needn't be complicated:
  Call getaddrinfo()
  Note the family of address[0], and start a routine which connects to each address of that family, in order.
  After ~300ms, start a second routine which connects to addresses *not* of that family, in order.
  Meanwhile:
  - If the first routine terminates before 300ms, start the second routine immediately.
  - When one routine succeeds, cancel the other.
For my purposes, I have no need for Happy Eyeballs, but the glaring problem with
net.Dial is that it subverts getaddrinfo() by always sorting IPv4 first.

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 4:

> you're overcomplicating matters
perhaps, what I'm concerning about is routing, multihoming.
> net.Dial is that it subverts getaddrinfo() by always sorting IPv4 first
i see, probably it's time to replace the current Dial("tcp", name) and Dial("udp", name)
behaviors with "rfc3493 way" or "lesser happy eyeballs". let's focus on introducing
DNS/other name to network locator address mapping feature into Dial. If you still think
it's worth to replace existing "happi{sh,er} eyeballs" with "happy eyeballs" on
Dialer{DualStack: true}.Dial("tcp", name), please open new issue.

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 5:

to be clear, what you are suggesting are;
1) make Dial APIs with a pair of wildcard address family+name arguments available to
deal with multiple locator addresses by default,
2) Dial APIs are Dialer.Dial, DialTimeout and Dial,
3) the wildcard address families must be "tcp" or "udp",
4) the name must be DNS/mDNS or other-resolvable name,
5) the short list; preferences for the list of addresses will be preserved when the net
package uses getaddrinfo or similar system services; in the case of builtin dns
resolver, perhaps we'll use another approach,
6) the dns/tcp racers; no tcp syn racers, using rfc3493 way.
is there any omission?

@pmarks-net
Copy link
Contributor Author

Comment 6:

Forked the Happy Eyeballs discussion into issue #8455.
I suggest the following summary for this bug:
net: Dial should follow getaddrinfo() address ordering.

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 8:

Labels changed: added repo-main.

Status changed to Accepted.

@pmarks-net
Copy link
Contributor Author

Comment 9:

Your list looks good, except for these notes:
3) I think "udp" should use the first getaddrinfo result, but it's not obvious to me how
the subsequent addresses could ever be useful, given that UDP has no connection state.
5) Ideally, the built-in resolver should behave similar to getaddrinfo() where feasible;
the address sorting is described by RFC 6724.
6) I'm not sure what you mean by "dns/tcp race".  getaddrinfo() might send A+AAAA
queries in parallel, but that's beyond the application's control.  It returns a single
list, so TCP must begin after DNS has completed.

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 10:

> 3) I think "udp"...
hm, let's sleep on it.
> 5) Ideally, the built-in resolver...
we also need to fix issue #6579 (now alex is working on it).
> 6) I'm not sure what you mean by "dns/tcp race"
it means that in the case of builtin resolver we can use query racers but it won't.

@mikioh
Copy link
Contributor

mikioh commented Jul 31, 2014

Comment 11:

> 3) I think "udp"...
some kernel tries to deal with a given 5-tuple even on udp connect operation
(inpcb_connect on bsd variants, for example), i think it's enough reason.

@mikioh
Copy link
Contributor

mikioh commented Aug 13, 2014

Comment 12:

Status changed to New.

@mikioh
Copy link
Contributor

mikioh commented Aug 29, 2014

Comment 13:

i personally want to see this fixed in go1.4 but probably too late for the tree close,
not sure.

@gopherbot
Copy link
Contributor

Comment 14 by google@barrera.io:

Much like I said on #8455: By always-prefering IPv4, all golang applications fail to
establish any network connections on IPv6-only hosts. I've seen this behaviour
consistenly across several golang applications: they can never connect to any remote
host.
Either #8455 or #8453 would fix this issue (which feels like a duplicate of #8124,
actually).

@mikioh
Copy link
Contributor

mikioh commented Oct 1, 2014

Comment 15:

Labels changed: added release-go1.5.

@mikioh
Copy link
Contributor

mikioh commented Oct 1, 2014

Comment 16:

Owner changed to @mikioh.

@mikioh
Copy link
Contributor

mikioh commented Mar 25, 2015

Now @pmarks-net is working on this issue.

mikioh pushed a commit that referenced this issue Apr 10, 2015
Remove the "netaddr" type, which ambiguously represented either one
address, or a list of addresses. Instead, use "addrList" wherever
multiple addresses are supported.

The "first" method returns the first address matching some condition
(e.g. "is it IPv4?"), primarily to support legacy code that can't handle
multiple addresses.

The "partition" method splits an addrList into two categories, as
defined by some strategy function. This is useful for implementing
Happy Eyeballs, and similar two-channel algorithms.

Finally, internetAddrList (formerly resolveInternetAddr) no longer
mangles the ordering defined by getaddrinfo. In the future, this may
be used by a sequential Dial implementation.

Updates #8453, #8455.

Change-Id: I7375f4c34481580ab40e31d33002a4073a0474f3
Reviewed-on: https://go-review.googlesource.com/8360
Reviewed-by: Mikio Hara <mikioh.mikioh@gmail.com>
Run-TryBot: Mikio Hara <mikioh.mikioh@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc rsc removed the repo-main label Apr 14, 2015
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/8768 mentions this issue.

@pmarks-net
Copy link
Contributor Author

Expletive! This needs to be reopened, because getaddrinfo was recently replaced by a Go-specific DNS resolver that does not follow RFC 6724:

4a0ba7a

The IPv4/IPv6 preference implemented by goLookupIPOrder is essentially random (it depends on whether the A or AAAA DNS response arrives first.)

@mikioh
Copy link
Contributor

mikioh commented Jun 25, 2015

Let's open a new issue for the builtin DNS resolver. We should also fix #11081 for Go 1.5. The stuff has been a substitute, and now that it should play as a starter on the Go 1.5 pitch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants