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 does not try all resolved addresses #5267

Closed
dsymonds opened this issue Apr 11, 2013 · 23 comments

Comments

Projects
None yet
7 participants
@dsymonds
Copy link
Member

commented Apr 11, 2013

If a network name maps to multiple addresses (e.g. "localhost" often maps to
both "::1" and "127.0.0.1"), net.Dial does not try them all, but
only the first. This is a problem if a server is only listening on one of those
addresses.

Arguably the server is behaving incorrectly, but most networking libraries behave
reasonably here: they try them in sequence rather than just one.

I have a test case that demonstrates this in https://golang.org/cl/8644043.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 11, 2013

Comment 1:

I believe this is a dup of issue #3610. At least it's very related.
@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2013

Comment 2:

Yeah, it's related, but that one appears a lot more expansive and more
performance-focused, whereas this is relatively simple and about correctness.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 11, 2013

Comment 3:

They have almost identical implementations, though.
Actually, now that the dial code's been cleaned up a bunch, both are pretty easy.
@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2013

Comment 4:

Maybe. I don't know about how to implement either of these, but this one should require
no API changes, and thus I hope it could be in 1.1.1, whereas the other one sounds like
people are pushing it back to 1.2.
@mikioh

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2013

Comment 5:

Fast, Cheap, Good; pick any two.
F C G
- - v  DNS TTL lookup (for correctness)
v - v  Native, net-embedded DNS resolver (no more cgo threads)
v - v  DNS-related record caching (for future dialings)
- v -  Run multiple dial-racers at each dial
v - v  TCP connection pooling w/ scoreboards (for future dialings)
I guess the example algorithm in RFC 6555 (section 6) without
any preferences (TTL, scoreboards, addrsel policies, etc) is
enough at first.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 11, 2013

Comment 6:

Um. All this bug really needs is a for loop.
@remyoudompheng

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2013

Comment 7:

What does it mean to "try all resolved addresses" ?
@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2013

Comment 8:

It means that if a name resolves to addr1, addr2, addr3, net.Dial
should not give up if it can't connect addr1, but rather try addr2 and
addr3 before giving up.
@bradfitz

This comment has been minimized.

Copy link
Member

commented Apr 11, 2013

Comment 9:

David, for the purposes you care about (localhost resolving to both 127.0.0.1 and ::1),
the Dial code already tries to deal with that:
func firstFavoriteAddr(filter func(IP) IP, addrs []string) (addr IP) {
        if filter == nil {
                // We'll take any IP address, but since the dialing code                                             
                // does not yet try multiple addresses, prefer to use                                                
                // an IPv4 address if possible.  This is especially relevant                                         
                // if localhost resolves to [ipv6-localhost, ipv4-localhost].                                        
                // Too much code assumes localhost == ipv4-localhost.                                                
                addr = firstSupportedAddr(ipv4only, addrs)
                if addr == nil {
                        addr = firstSupportedAddr(anyaddr, addrs)
                }
        } else {
                addr = firstSupportedAddr(filter, addrs)
        }
        return
}
To be clear, in your case, is the problem that the listener is on [::1]:nnn and the Go
net package is dialing 127.0.0.1:nnn?
@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2013

Comment 10:

Yes, the listener in my scenario is only on [::1]:port, so always
picking 127.0.0.1:port is failing to find it.
@robpike

This comment has been minimized.

Copy link
Contributor

commented May 18, 2013

Comment 11:

Labels changed: added go1.2maybe, removed go1.1maybe.

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 12:

Labels changed: added feature.

@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Jul 30, 2013

Comment 13:

This is a bug, not a feature. The net package is not behaving in the correct way.

Labels changed: added go1.2, removed go1.2maybe.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2013

Comment 14:

I'm fine either way; fixing this by old dual stack style or by happy eyeball style.
old dual stack style:
for _, ip := lookupIP {
  dialTCP(...)
}
happy eyeball style:
ips := lookupIP
for _, ip := range ips {
  go func() { dialTCP; wire<-sig }
}
select {
case winner := <-wire:
...
}
@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2013

Comment 15:

See the key on the label entry. "Feature" means "Feature or feature-sized work; not okay
during release freeze."
@mikioh

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2013

Comment 16:

Owner changed to @mikioh.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2013

Comment 17:

Status changed to Started.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Aug 7, 2013

Comment 18:

Two CLs are CLs.
net: pick a pair of different address family IP addresses for DNS registered name
https://golang.org/cl/12414043/
makes eyes open on control plane,
net: implement Happy Eyeballs connection setup for TCP
https://golang.org/cl/12416043/
runs two dialers simply by using channel and goroutine.
The latter includes the former for convenience.
@dsymonds

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2013

Comment 19:

Labels changed: added priority-soon, removed priority-triage.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2013

Comment 20:

https://golang.org/cl/12414043/ was abandoned. smaller CLs coming. Probably not
in time for 1.2 but leaving open for now.
@mikioh

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2013

Comment 21:

The review of the last CL in progress.
https://golang.org/cl/12416043/
@rsc

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2013

Comment 22:

Labels changed: added releaseblocker.

@mikioh

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2013

Comment 23:

This issue was closed by revision 89b2676.

Status changed to Fixed.

@dsymonds dsymonds added fixed labels Sep 11, 2013

@rsc rsc added this to the Go1.2 milestone Apr 14, 2015

@rsc rsc removed the go1.2 label Apr 14, 2015

@golang golang locked and limited conversation to collaborators Jun 24, 2016

This issue was closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.