Merge branch 'master' of github.com:tailscale/tailscale into HEAD

* 'master' of github.com:tailscale/tailscale:
  netcheck: fix data races for staggler STUN packets arriving after GetReport
  wgengine/magicsock: add a pointer value for logging
  netcheck: ignore IPv4 STUN failures if we saw at least one reply
  netcheck: ignore IPv6 STUN failures
  derp: add clients_replaced counter
  version: bump OSS version datestamp.

derp/derp_server.go

@@ -50,6 +50,7 @@ type Server struct {
 	accepts                expvar.Int
 	curClients             expvar.Int
 	curHomeClients         expvar.Int // ones with preferred
+	clientsReplaced        expvar.Int
 	unknownFrames          expvar.Int
 	homeMovesIn            expvar.Int // established clients announce home server moves in
 	homeMovesOut           expvar.Int // established clients announce home server moves out
@@ -145,6 +146,7 @@ func (s *Server) registerClient(c *sclient) {
 	if old == nil {
 		c.logf("adding connection")
 	} else {
+		s.clientsReplaced.Add(1)
 		old.nc.Close()
 		c.logf("adding connection, replacing %s", old.nc.RemoteAddr())
 	}
@@ -539,6 +541,7 @@ func (s *Server) ExpVar() expvar.Var {
 	m.Set("gauge_current_connnections", &s.curClients)
 	m.Set("gauge_current_home_connnections", &s.curHomeClients)
 	m.Set("accepts", &s.accepts)
+	m.Set("clients_replaced", &s.clientsReplaced)
 	m.Set("bytes_received", &s.bytesRecv)
 	m.Set("bytes_sent", &s.bytesSent)
 	m.Set("packets_dropped", &s.packetsDropped)

netcheck/netcheck.go

@@ -68,10 +68,11 @@ type Client struct {
 	GetSTUNConn4 func() STUNConn
 	GetSTUNConn6 func() STUNConn
 
+	mu          sync.Mutex // guards following
 	s4          *stunner.Stunner
 	s6          *stunner.Stunner
 	hairTX      stun.TxID
-	gotHairSTUN chan *net.UDPAddr
+	gotHairSTUN chan *net.UDPAddr // non-nil if we're in GetReport
 }
 
 // STUNConn is the interface required by the netcheck Client when
@@ -92,6 +93,12 @@ func (c *Client) logf(format string, a ...interface{}) {
 // handleHairSTUN reports whether pkt (from src) was our magic hairpin
 // probe packet that we sent to ourselves.
 func (c *Client) handleHairSTUN(pkt []byte, src *net.UDPAddr) bool {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	return c.handleHairSTUNLocked(pkt, src)
+}
+
+func (c *Client) handleHairSTUNLocked(pkt []byte, src *net.UDPAddr) bool {
 	if tx, err := stun.ParseBindingRequest(pkt); err == nil && tx == c.hairTX {
 		select {
 		case c.gotHairSTUN <- src:
@@ -103,18 +110,26 @@ func (c *Client) handleHairSTUN(pkt []byte, src *net.UDPAddr) bool {
 }
 
 func (c *Client) ReceiveSTUNPacket(pkt []byte, src *net.UDPAddr) {
-	var st *stunner.Stunner
 	if src == nil || src.IP == nil {
 		panic("bogus src")
 	}
-	if c.handleHairSTUN(pkt, src) {
+
+	c.mu.Lock()
+
+	if c.handleHairSTUNLocked(pkt, src) {
+		c.mu.Unlock()
 		return
 	}
+
+	var st *stunner.Stunner
 	if src.IP.To4() != nil {
 		st = c.s4
 	} else {
 		st = c.s6
 	}
+
+	c.mu.Unlock()
+
 	if st != nil {
 		st.Receive(pkt, src)
 	}
@@ -129,16 +144,30 @@ func (c *Client) GetReport(ctx context.Context) (*Report, error) {
 	// (User ctx might be context.Background, etc)
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
+
+	if c.DERP == nil {
+		return nil, errors.New("netcheck: GetReport: Client.DERP is nil")
+	}
+
+	c.mu.Lock()
+	if c.gotHairSTUN != nil {
+		c.mu.Unlock()
+		return nil, errors.New("invalid concurrent call to GetReport")
+	}
+	hairTX := stun.NewTxID() // random payload
+	c.hairTX = hairTX
+	gotHairSTUN := make(chan *net.UDPAddr, 1)
+	c.gotHairSTUN = gotHairSTUN
+	c.mu.Unlock()
+
 	defer func() {
+		c.mu.Lock()
+		defer c.mu.Unlock()
 		c.s4 = nil
 		c.s6 = nil
+		c.gotHairSTUN = nil
 	}()
-	c.hairTX = stun.NewTxID() // random payload
-	c.gotHairSTUN = make(chan *net.UDPAddr, 1)
 
-	if c.DERP == nil {
-		return nil, errors.New("netcheck: GetReport: Client.DERP is nil")
-	}
 	stuns4 := c.DERP.STUN4()
 	stuns6 := c.DERP.STUN6()
 	if len(stuns4) == 0 {
@@ -179,7 +208,7 @@ func (c *Client) GetReport(ctx context.Context) (*Report, error) {
 	hairTimeout := make(chan bool, 1)
 	startHairCheck := func(dstEP string) {
 		if dst, err := net.ResolveUDPAddr("udp4", dstEP); err == nil {
-			pc4Hair.WriteTo(stun.Request(c.hairTX), dst)
+			pc4Hair.WriteTo(stun.Request(hairTX), dst)
 			time.AfterFunc(500*time.Millisecond, func() { hairTimeout <- true })
 		}
 	}
@@ -295,8 +324,25 @@ func (c *Client) GetReport(ctx context.Context) (*Report, error) {
 		Logf:     c.logf,
 		DNSCache: dnscache.Get(),
 	}
+
+	c.mu.Lock()
 	c.s4 = s4
-	grp.Go(func() error { return s4.Run(ctx) })
+	c.mu.Unlock()
+
+	grp.Go(func() error {
+		err := s4.Run(ctx)
+		if err == nil {
+			return nil
+		}
+		mu.Lock()
+		defer mu.Unlock()
+		// If we got at least one IPv4 endpoint, treat that as
+		// good enough.
+		if gotEP4 != "" {
+			return nil
+		}
+		return err
+	})
 	if c.GetSTUNConn4 == nil {
 		go reader(s4, pc4)
 	}
@@ -310,8 +356,19 @@ func (c *Client) GetReport(ctx context.Context) (*Report, error) {
 			OnlyIPv6: true,
 			DNSCache: dnscache.Get(),
 		}
+
+		c.mu.Lock()
 		c.s6 = s6
-		grp.Go(func() error { return s6.Run(ctx) })
+		c.mu.Unlock()
+
+		grp.Go(func() error {
+			if err := s6.Run(ctx); err != nil {
+				// IPv6 seemed like it was configured, but actually failed.
+				// Just log and return a nil error.
+				c.logf("netcheck: ignoring IPv6 failure: %v", err)
+			}
+			return nil
+		})
 		if c.GetSTUNConn6 == nil {
 			go reader(s6, pc6)
 		}
@@ -328,7 +385,7 @@ func (c *Client) GetReport(ctx context.Context) (*Report, error) {
 	// Check hairpinning.
 	if ret.MappingVariesByDestIP == "false" && gotEP4 != "" {
 		select {
-		case <-c.gotHairSTUN:
+		case <-gotHairSTUN:
 			ret.HairPinning.Set(true)
 		case <-hairTimeout:
 			ret.HairPinning.Set(false)

version/version.go

@@ -7,5 +7,5 @@
 // Package version provides the version that the binary was built at.
 package version
 
-const LONG = "date.20200306"
+const LONG = "date.20200311"
 const SHORT = LONG // TODO: unused; remove SHORT? Make it a func?

wgengine/magicsock/magicsock.go

@@ -779,7 +779,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr *net.UDPAddr, dc
 				return
 			default:
 			}
-			c.logf("derp.Recv(derp%d): %v", derpFakeAddr.Port, err)
+			c.logf("[%p] derp.Recv(derp%d): %v", dc, derpFakeAddr.Port, err)
 			time.Sleep(250 * time.Millisecond)
 			continue
 		}