derp/derphttp, wgengine/magicsock: prefer IPv6 to DERPs when IPv6 works

Fixes #3838

Change-Id: Ie47a2a30c7e8e431512824798d2355006d72fb6a
Signed-off-by: Brad Fitzpatrick <[email protected]>

derp/derphttp/derphttp_client.go

@@ -26,6 +26,7 @@ import (
 	"runtime"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"go4.org/mem"
@@ -64,6 +65,12 @@ type Client struct {
 	ctx       context.Context // closed via cancelCtx in Client.Close
 	cancelCtx context.CancelFunc
 
+	// addrFamSelAtomic is the last AddressFamilySelector set
+	// by SetAddressFamilySelector. It's an atomic because it needs
+	// to be accessed by multiple racing routines started while
+	// Client.conn holds mu.
+	addrFamSelAtomic atomic.Value // of AddressFamilySelector
+
 	mu           sync.Mutex
 	preferred    bool
 	canAckPings  bool
@@ -193,6 +200,32 @@ func (c *Client) urlString(node *tailcfg.DERPNode) string {
 	return fmt.Sprintf("https://%s/derp", node.HostName)
 }
 
+// AddressFamilySelector decides whethers IPv6 is preferred for
+// outbound dials.
+type AddressFamilySelector interface {
+	// PreferIPv6 reports whether IPv4 dials should be slightly
+	// delayed to give IPv6 a better chance of winning dial races.
+	// Implementations should only return true if IPv6 is expected
+	// to succeed. (otherwise delaying IPv4 will delay the
+	// connection overall)
+	PreferIPv6() bool
+}
+
+// SetAddressFamilySelector sets the AddressFamilySelector that this
+// connection will use. It should be called before any dials.
+// The value must not be nil. If called more than once, s must
+// be the same concrete type as any prior calls.
+func (c *Client) SetAddressFamilySelector(s AddressFamilySelector) {
+	c.addrFamSelAtomic.Store(s)
+}
+
+func (c *Client) preferIPv6() bool {
+	if s, ok := c.addrFamSelAtomic.Load().(AddressFamilySelector); ok {
+		return s.PreferIPv6()
+	}
+	return false
+}
+
 // dialWebsocketFunc is non-nil (set by websocket.go's init) when compiled in.
 var dialWebsocketFunc func(ctx context.Context, urlStr string) (net.Conn, error)
 
@@ -583,6 +616,18 @@ func (c *Client) dialNode(ctx context.Context, n *tailcfg.DERPNode) (net.Conn, e
 	startDial := func(dstPrimary, proto string) {
 		nwait++
 		go func() {
+			if proto == "tcp4" && c.preferIPv6() {
+				t := time.NewTimer(200 * time.Millisecond)
+				select {
+				case <-ctx.Done():
+					// Either user canceled original context,
+					// it timed out, or the v6 dial succeeded.
+					t.Stop()
+					return
+				case <-t.C:
+					// Start v4 dial
+				}
+			}
 			dst := dstPrimary
 			if dst == "" {
 				dst = n.HostName

wgengine/magicsock/magicsock.go

@@ -305,6 +305,8 @@ type Conn struct {
 	// lock ordering deadlocks. See issue 3726 and mu field docs.
 	derpMapAtomic atomic.Value // of *tailcfg.DERPMap
 
+	lastNetCheckReport atomic.Value // of *netcheck.Report
+
 	// port is the preferred port from opts.Port; 0 means auto.
 	port syncs.AtomicUint32
 
@@ -741,6 +743,7 @@ func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) {
 		return nil, err
 	}
 
+	c.lastNetCheckReport.Store(report)
 	c.noV4.Set(!report.IPv4)
 	c.noV6.Set(!report.IPv6)
 	c.noV4Send.Set(!report.IPv4CanSend)
@@ -1380,6 +1383,7 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.NodePublic) cha
 
 	dc.SetCanAckPings(true)
 	dc.NotePreferred(c.myDerp == regionID)
+	dc.SetAddressFamilySelector(derpAddrFamSelector{c})
 	dc.DNSCache = dnscache.Get()
 
 	ctx, cancel := context.WithCancel(c.connCtx)
@@ -4125,6 +4129,22 @@ func (di *discoInfo) setNodeKey(nk key.NodePublic) {
 	di.lastNodeKeyTime = time.Now()
 }
 
+// derpAddrFamSelector is the derphttp.AddressFamilySelector we pass
+// to derphttp.Client.SetAddressFamilySelector.
+//
+// It provides the hint as to whether in an IPv4-vs-IPv6 race that
+// IPv4 should be held back a bit to give IPv6 a better-than-50/50
+// chance of winning. We only return true when we believe IPv6 will
+// work anyway, so we don't artificially delay the connection speed.
+type derpAddrFamSelector struct{ c *Conn }
+
+func (s derpAddrFamSelector) PreferIPv6() bool {
+	if r, ok := s.c.lastNetCheckReport.Load().(*netcheck.Report); ok {
+		return r.IPv6
+	}
+	return false
+}
+
 var (
 	metricNumPeers     = clientmetric.NewGauge("magicsock_netmap_num_peers")
 	metricNumDERPConns = clientmetric.NewGauge("magicsock_num_derp_conns")