Merge pull request #1848 from ipfs/feat/dial-sorting

order addresses to give certain address types priority

p2p/net/swarm/dial_test.go

@@ -2,6 +2,7 @@ package swarm
 
 import (
 	"net"
+	"sort"
 	"sync"
 	"testing"
 	"time"
@@ -438,3 +439,38 @@ func TestDialBackoffClears(t *testing.T) {
 		t.Log("correctly cleared backoff")
 	}
 }
+
+func mkAddr(t *testing.T, s string) ma.Multiaddr {
+	a, err := ma.NewMultiaddr(s)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return a
+}
+
+func TestAddressSorting(t *testing.T) {
+	u1 := mkAddr(t, "/ip4/152.12.23.53/udp/1234/utp")
+	u2l := mkAddr(t, "/ip4/127.0.0.1/udp/1234/utp")
+	local := mkAddr(t, "/ip4/127.0.0.1/tcp/1234")
+	norm := mkAddr(t, "/ip4/6.5.4.3/tcp/1234")
+
+	l := AddrList{local, u1, u2l, norm}
+	sort.Sort(l)
+
+	if !l[0].Equal(u2l) {
+		t.Fatal("expected utp local addr to be sorted first: ", l[0])
+	}
+
+	if !l[1].Equal(u1) {
+		t.Fatal("expected utp addr to be sorted second")
+	}
+
+	if !l[2].Equal(local) {
+		t.Fatal("expected tcp localhost addr thid")
+	}
+
+	if !l[3].Equal(norm) {
+		t.Fatal("expected normal addr last")
+	}
+}

p2p/net/swarm/swarm_dial.go

@@ -1,10 +1,11 @@
 package swarm
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
-	"math/rand"
 	"net"
+	"sort"
 	"sync"
 	"time"
 
@@ -358,6 +359,9 @@ func (s *Swarm) dial(ctx context.Context, p peer.ID) (*Conn, error) {
 
 func (s *Swarm) dialAddrs(ctx context.Context, d *conn.Dialer, p peer.ID, remoteAddrs []ma.Multiaddr) (conn.Conn, error) {
 
+	// sort addresses so preferred addresses are dialed sooner
+	sort.Sort(AddrList(remoteAddrs))
+
 	// try to connect to one of the peer's known addresses.
 	// we dial concurrently to each of the addresses, which:
 	// * makes the process faster overall
@@ -404,10 +408,7 @@ func (s *Swarm) dialAddrs(ctx context.Context, d *conn.Dialer, p peer.ID, remote
 	// to end early.
 	go func() {
 		limiter := make(chan struct{}, 8)
-		// permute addrs so we try different sets first each time.
-		for _, i := range rand.Perm(len(remoteAddrs)) {
-
-			addr := remoteAddrs[i]
+		for _, addr := range remoteAddrs {
 			// returns whatever ratelimiting is acceptable for workerAddr.
 			// may not rate limit at all.
 			rl := s.addrDialRateLimit(addr)
@@ -526,3 +527,52 @@ func isTCPMultiaddr(a ma.Multiaddr) bool {
 	p := a.Protocols()
 	return len(p) == 2 && (p[0].Name == "ip4" || p[0].Name == "ip6") && p[1].Name == "tcp"
 }
+
+type AddrList []ma.Multiaddr
+
+func (al AddrList) Len() int {
+	return len(al)
+}
+
+func (al AddrList) Swap(i, j int) {
+	al[i], al[j] = al[j], al[i]
+}
+
+func (al AddrList) Less(i, j int) bool {
+	a := al[i]
+	b := al[j]
+
+	// dial localhost addresses next, they should fail immediately
+	lba := manet.IsIPLoopback(a)
+	lbb := manet.IsIPLoopback(b)
+	if lba {
+		if !lbb {
+			return true
+		}
+	}
+
+	// dial utp and similar 'non-fd-consuming' addresses first
+	fda := isFDCostlyTransport(a)
+	fdb := isFDCostlyTransport(b)
+	if !fda {
+		if fdb {
+			return true
+		}
+
+		// if neither consume fd's, assume equal ordering
+		return false
+	}
+
+	// if 'b' doesnt take a file descriptor
+	if !fdb {
+		return false
+	}
+
+	// if 'b' is loopback and both take file descriptors
+	if lbb {
+		return false
+	}
+
+	// for the rest, just sort by bytes
+	return bytes.Compare(a.Bytes(), b.Bytes()) > 0
+}