Use Netroute (#25)

Use OS routing table as a hint when dialing

p2p/net/reuseport/multidialer.go

@@ -5,12 +5,15 @@ import (
 	"fmt"
 	"math/rand"
 	"net"
+
+	"github.com/libp2p/go-netroute"
 )
 
 type multiDialer struct {
-	loopback    []*net.TCPAddr
-	unspecified []*net.TCPAddr
-	global      *net.TCPAddr
+	listeningAddresses []*net.TCPAddr
+	loopback           []*net.TCPAddr
+	unspecified        []*net.TCPAddr
+	fallback           net.TCPAddr
 }
 
 func (d *multiDialer) Dial(network, addr string) (net.Conn, error) {
@@ -24,87 +27,64 @@ func randAddr(addrs []*net.TCPAddr) *net.TCPAddr {
 	return nil
 }
 
+// DialContext dials a target addr.
+// Dialing preference is
+// * If there is a listener on the local interface the OS expects to use to route towards addr, use that.
+// * If there is a listener on a loopback address, addr is loopback, use that.
+// * If there is a listener on an undefined address (0.0.0.0 or ::), use that.
+// * Use the fallback IP specified during construction, with a port that's already being listened on, if one exists.
 func (d *multiDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) {
 	tcpAddr, err := net.ResolveTCPAddr(network, addr)
 	if err != nil {
 		return nil, err
 	}
-
-	// We pick the source *port* based on the following algorithm.
-	//
-	// 1. If we're dialing loopback, choose a source-port in order of
-	//    preference:
-	//   1. A port in-use by an explicit loopback listener.
-	//   2. A port in-use by a listener on an unspecified address (must
-	//      also be listening on localhost).
-	//   3. A port in-use by a listener on a global address. We don't have
-	//      any other better options (other than picking a random port).
-	// 2. If we're dialing a global address, choose a source-port in order
-	//    of preference:
-	//   1. A port in-use by a listener on an unspecified address (the most
-	//      general case).
-	//   2. A port in-use by a listener on the global address.
-	// 3. Fail on link-local dials (go-ipfs currently forbids this and I
-	//    figured we could try lifting this restriction later).
-	//
-	//
-	// Note: We *always* dial from the unspecified address (regardless of
-	// the port we pick). In the future, we could use netlink (on Linux) to
-	// figure out the right source address but we're going to punt on that.
-
 	ip := tcpAddr.IP
-	source := d.global
-	switch {
-	case ip.IsLoopback():
-		switch {
-		case len(d.loopback) > 0:
-			source = randAddr(d.loopback)
-		case len(d.unspecified) > 0:
-			source = randAddr(d.unspecified)
-		}
-	case ip.IsGlobalUnicast():
-		switch {
-		case len(d.unspecified) > 0:
-			source = randAddr(d.unspecified)
+	if !ip.IsLoopback() && !ip.IsGlobalUnicast() {
+		return nil, fmt.Errorf("undialable IP: %s", ip)
+	}
+
+	if router, err := netroute.New(); err == nil {
+		if _, _, preferredSrc, err := router.Route(ip); err == nil {
+			for _, optAddr := range d.listeningAddresses {
+				if optAddr.IP.Equal(preferredSrc) {
+					return reuseDial(ctx, optAddr, network, addr)
+				}
+			}
 		}
-	default:
-		return nil, fmt.Errorf("undialable IP: %s", tcpAddr.IP)
 	}
-	return reuseDial(ctx, source, network, addr)
+
+	if ip.IsLoopback() && len(d.loopback) > 0 {
+		return reuseDial(ctx, randAddr(d.loopback), network, addr)
+	}
+	if len(d.unspecified) == 0 {
+		return reuseDial(ctx, &d.fallback, network, addr)
+	}
+
+	return reuseDial(ctx, randAddr(d.unspecified), network, addr)
 }
 
-func newMultiDialer(unspec net.IP, listeners map[*listener]struct{}) dialer {
-	m := new(multiDialer)
+func newMultiDialer(unspec net.IP, listeners map[*listener]struct{}) (m dialer) {
+	addrs := make([]*net.TCPAddr, 0)
+	loopback := make([]*net.TCPAddr, 0)
+	unspecified := make([]*net.TCPAddr, 0)
+	existingPort := 0
+
 	for l := range listeners {
-		laddr := l.Addr().(*net.TCPAddr)
-		switch {
-		case laddr.IP.IsLoopback():
-			m.loopback = append(m.loopback, laddr)
-		case laddr.IP.IsGlobalUnicast():
-			// Different global ports? Crap.
-			//
-			// The *proper* way to deal with this is to, e.g., use
-			// netlink to figure out which source address we would
-			// normally use to dial a destination address and then
-			// pick one of the ports we're listening on on that
-			// source address. However, this is a pain in the ass.
-			//
-			// Instead, we're just going to always dial from the
-			// unspecified address with the first global port we
-			// find.
-			//
-			// TODO: Port priority? Addr priority?
-			if m.global == nil {
-				m.global = &net.TCPAddr{
-					IP:   unspec,
-					Port: laddr.Port,
-				}
-			} else {
-				log.Warning("listening on external interfaces on multiple ports, will dial from %d, not %s", m.global, laddr)
-			}
-		case laddr.IP.IsUnspecified():
-			m.unspecified = append(m.unspecified, laddr)
+		addr := l.Addr().(*net.TCPAddr)
+		addrs = append(addrs, addr)
+		if addr.IP.IsLoopback() {
+			loopback = append(loopback, addr)
+		} else if addr.IP.IsGlobalUnicast() && existingPort == 0 {
+			existingPort = addr.Port
+		} else if addr.IP.IsUnspecified() {
+			unspecified = append(unspecified, addr)
 		}
 	}
-	return m
+	m = &multiDialer{
+		listeningAddresses: addrs,
+		loopback:           loopback,
+		unspecified:        unspecified,
+		fallback:           net.TCPAddr{IP: unspec, Port: existingPort},
+	}
+	return
 }

p2p/net/reuseport/transport_test.go

@@ -38,18 +38,20 @@ func init() {
 	}
 }
 
-func acceptOne(t *testing.T, listener manet.Listener) <-chan struct{} {
+func acceptOne(t *testing.T, listener manet.Listener) <-chan interface{} {
 	t.Helper()
-	done := make(chan struct{})
+	done := make(chan interface{}, 1)
 	go func() {
 		defer close(done)
+		done <- nil
 		c, err := listener.Accept()
 		if err != nil {
 			t.Error(err)
 			return
 		}
 		c.Close()
 	}()
+	<-done
 	return done
 }
 
@@ -72,7 +74,7 @@ func dialOne(t *testing.T, tr *Transport, listener manet.Listener, expected ...i
 			return port
 		}
 	}
-	t.Errorf("dialed from %d, expected to dial from one of %v", port, expected)
+	t.Errorf("dialed %s from %v. expected to dial from port %v", listener.Multiaddr(), c.LocalAddr(), expected)
 	return 0
 }
 
@@ -127,10 +129,12 @@ func TestGlobalPreferenceV4(t *testing.T) {
 		t.Skip("no global IPv4 addresses configured")
 		return
 	}
+	t.Logf("when listening on %v, should prefer %v over %v", loopbackV4, loopbackV4, globalV4)
 	testPrefer(t, loopbackV4, loopbackV4, globalV4)
+	t.Logf("when listening on %v, should prefer %v over %v", loopbackV4, unspecV4, globalV4)
 	testPrefer(t, loopbackV4, unspecV4, globalV4)
 
-	testPrefer(t, globalV4, unspecV4, globalV4)
+	t.Logf("when listening on %v, should prefer %v over %v", globalV4, unspecV4, loopbackV4)
 	testPrefer(t, globalV4, unspecV4, loopbackV4)
 }
 
@@ -142,7 +146,6 @@ func TestGlobalPreferenceV6(t *testing.T) {
 	testPrefer(t, loopbackV6, loopbackV6, globalV6)
 	testPrefer(t, loopbackV6, unspecV6, globalV6)
 
-	testPrefer(t, globalV6, unspecV6, globalV6)
 	testPrefer(t, globalV6, unspecV6, loopbackV6)
 }