cmd/cloner, tailcfg: fix nil vs len 0 issues, add tests, use for Hostinfo

Also use go:generate and https://golang.org/s/generatedcode header style.

cmd/tailscaled/tailscaled.go

@@ -169,7 +169,7 @@ func run() error {
 		SurviveDisconnects: true,
 		DebugMux:           debugMux,
 	}
-	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), ipnserver.FixedEngine(e), opts)
+	err = ipnserver.Run(ctx, logf, pol.PublicID.String(), opts, e)
 	// Cancelation is not an error: it is the only way to stop ipnserver.
 	if err != nil && err != context.Canceled {
 		logf("ipnserver.Run: %v", err)

control/controlclient/auto.go

@@ -517,7 +517,7 @@ func (c *Client) SetHostinfo(hi *tailcfg.Hostinfo) {
 		panic("nil Hostinfo")
 	}
 	if !c.direct.SetHostinfo(hi) {
-		// No changes. Don't log.
+		c.logf("[unexpected] duplicate Hostinfo: %v", hi)
 		return
 	}
 	c.logf("Hostinfo: %v", hi)

control/controlclient/controlclient_test.go

@@ -70,10 +70,3 @@ func TestStatusEqual(t *testing.T) {
 		}
 	}
 }
-
-func TestOSVersion(t *testing.T) {
-	if osVersion == nil {
-		t.Skip("not available for OS")
-	}
-	t.Logf("Got: %#q", osVersion())
-}

control/controlclient/direct.go

@@ -4,8 +4,6 @@
 
 package controlclient
 
-//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=direct_clone.go
-
 import (
 	"bytes"
 	"context"
@@ -21,7 +19,6 @@ import (
 	"net/url"
 	"os"
 	"reflect"
-	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -168,20 +165,12 @@ func NewDirect(opts Options) (*Direct, error) {
 	return c, nil
 }
 
-var osVersion func() string // non-nil on some platforms
-
 func NewHostinfo() *tailcfg.Hostinfo {
 	hostname, _ := os.Hostname()
-	var osv string
-	if osVersion != nil {
-		osv = osVersion()
-	}
 	return &tailcfg.Hostinfo{
 		IPNVersion: version.LONG,
 		Hostname:   hostname,
 		OS:         version.OS(),
-		OSVersion:  osv,
-		GoArch:     runtime.GOARCH,
 	}
 }
 
@@ -630,7 +619,6 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 			NodeKey:      tailcfg.NodeKey(persist.PrivateNodeKey.Public()),
 			PrivateKey:   persist.PrivateNodeKey,
 			Expiry:       resp.Node.KeyExpiry,
-			Name:         resp.Node.Name,
 			Addresses:    resp.Node.Addresses,
 			Peers:        resp.Peers,
 			LocalPort:    localPort,

control/controlclient/direct_clone.go

@@ -1,20 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Code generated by tailscale.com/cmd/cloner -type Persist; DO NOT EDIT.
-
-package controlclient
-
-import ()
-
-// Clone makes a deep copy of Persist.
-// The result aliases no memory with the original.
-func (src *Persist) Clone() *Persist {
-	if src == nil {
-		return nil
-	}
-	dst := new(Persist)
-	*dst = *src
-	return dst
-}

control/controlclient/hostinfo_linux.go

@@ -1,87 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// +build linux,!android
-
-package controlclient
-
-import (
-	"bytes"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"strings"
-	"syscall"
-
-	"go4.org/mem"
-	"tailscale.com/util/lineread"
-)
-
-func init() {
-	osVersion = osVersionLinux
-}
-
-func osVersionLinux() string {
-	m := map[string]string{}
-	lineread.File("/etc/os-release", func(line []byte) error {
-		eq := bytes.IndexByte(line, '=')
-		if eq == -1 {
-			return nil
-		}
-		k, v := string(line[:eq]), strings.Trim(string(line[eq+1:]), `"`)
-		m[k] = v
-		return nil
-	})
-
-	var un syscall.Utsname
-	syscall.Uname(&un)
-
-	var attrBuf strings.Builder
-	attrBuf.WriteString("; kernel=")
-	for _, b := range un.Release {
-		if b == 0 {
-			break
-		}
-		attrBuf.WriteByte(byte(b))
-	}
-	if inContainer() {
-		attrBuf.WriteString("; container")
-	}
-	attr := attrBuf.String()
-
-	id := m["ID"]
-
-	switch id {
-	case "debian":
-		slurp, _ := ioutil.ReadFile("/etc/debian_version")
-		return fmt.Sprintf("Debian %s (%s)%s", bytes.TrimSpace(slurp), m["VERSION_CODENAME"], attr)
-	case "ubuntu":
-		return fmt.Sprintf("Ubuntu %s%s", m["VERSION"], attr)
-	case "", "centos": // CentOS 6 has no /etc/os-release, so its id is ""
-		if cr, _ := ioutil.ReadFile("/etc/centos-release"); len(cr) > 0 { // "CentOS release 6.10 (Final)
-			return fmt.Sprintf("%s%s", bytes.TrimSpace(cr), attr)
-		}
-		fallthrough
-	case "fedora", "rhel", "alpine":
-		// Their PRETTY_NAME is fine as-is for all versions I tested.
-		fallthrough
-	default:
-		if v := m["PRETTY_NAME"]; v != "" {
-			return fmt.Sprintf("%s%s", v, attr)
-		}
-	}
-	return fmt.Sprintf("Other%s", attr)
-}
-
-func inContainer() (ret bool) {
-	lineread.File("/proc/1/cgroup", func(line []byte) error {
-		if mem.Contains(mem.B(line), mem.S("/docker/")) ||
-			mem.Contains(mem.B(line), mem.S("/lxc/")) {
-			ret = true
-			return io.EOF // arbitrary non-nil error to stop loop
-		}
-		return nil
-	})
-	return
-}

control/controlclient/hostinfo_windows.go

@@ -1,26 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package controlclient
-
-import (
-	"os/exec"
-	"strings"
-	"syscall"
-)
-
-func init() {
-	osVersion = osVersionWindows
-}
-
-func osVersionWindows() string {
-	cmd := exec.Command("cmd", "/c", "ver")
-	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
-	out, _ := cmd.Output() // "\nMicrosoft Windows [Version 10.0.19041.388]\n\n"
-	s := strings.TrimSpace(string(out))
-	s = strings.TrimPrefix(s, "Microsoft Windows [")
-	s = strings.TrimSuffix(s, "]")
-	s = strings.TrimPrefix(s, "Version ") // is this localized? do it last in case.
-	return s                              // "10.0.19041.388", ideally
-}

control/controlclient/netmap.go

@@ -23,11 +23,9 @@ import (
 type NetworkMap struct {
 	// Core networking
 
-	NodeKey    tailcfg.NodeKey
-	PrivateKey wgcfg.PrivateKey
-	Expiry     time.Time
-	// Name is the DNS name assigned to this node.
-	Name          string
+	NodeKey       tailcfg.NodeKey
+	PrivateKey    wgcfg.PrivateKey
+	Expiry        time.Time
 	Addresses     []wgcfg.CIDR
 	LocalPort     uint16 // used for debugging
 	MachineStatus tailcfg.MachineStatus

derp/derp_server.go

@@ -26,6 +26,7 @@ import (
 
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/sync/errgroup"
+	"tailscale.com/disco"
 	"tailscale.com/metrics"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -51,6 +52,13 @@ type Server struct {
 	// before failing when writing to a client.
 	WriteTimeout time.Duration
 
+	// OnlyDisco controls whether, for tests, non-discovery packets
+	// are dropped. This is used by magicsock tests to verify that
+	// NAT traversal works (using DERP for out-of-band messaging)
+	// but the packets themselves aren't going via DERP.
+	OnlyDisco bool
+	_         [pad32bit]byte
+
 	privateKey key.Private
 	publicKey  key.Public
 	logf       logger.Logf
@@ -551,6 +559,11 @@ func (c *sclient) handleFrameSendPacket(ft frameType, fl uint32) error {
 		return fmt.Errorf("client %x: recvPacket: %v", c.key, err)
 	}
 
+	if s.OnlyDisco && !disco.LooksLikeDiscoWrapper(contents) {
+		s.packetsDropped.Add(1)
+		return nil
+	}
+
 	var fwd PacketForwarder
 	s.mu.Lock()
 	dst := s.clients[dstKey]

go.sum

@@ -86,8 +86,6 @@ github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJy
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
 github.com/tailscale/winipcfg-go v0.0.0-20200413171540-609dcf2df55f h1:uFj5bslHsMzxIM8UTjAhq4VXeo6GfNW91rpoh/WMJaY=
 github.com/tailscale/winipcfg-go v0.0.0-20200413171540-609dcf2df55f/go.mod h1:x880GWw5fvrl2DVTQ04ttXQD4DuppTt1Yz6wLibbjNE=
-github.com/tailscale/wireguard-go v0.0.0-20200724155040-d554a2a5e7e1 h1:Ga895WFYzI9VOXyps7t9ax9P5zSKsP5Yqpiv2euuyeU=
-github.com/tailscale/wireguard-go v0.0.0-20200724155040-d554a2a5e7e1/go.mod h1:JPm5cTfu1K+qDFRbiHy0sOlHUylYQbpl356sdYFD8V4=
 github.com/tcnksm/go-httpstat v0.2.0 h1:rP7T5e5U2HfmOBmZzGgGZjBQ5/GluWUylujl0tJ04I0=
 github.com/tcnksm/go-httpstat v0.2.0/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8=
 github.com/toqueteos/webbrowser v1.2.0 h1:tVP/gpK69Fx+qMJKsLE7TD8LuGWPnEV71wBN9rrstGQ=

internal/deepprint/deepprint.go

@@ -18,23 +18,13 @@ import (
 	"reflect"
 )
 
-func Hash(v ...interface{}) string {
+func Hash(v interface{}) string {
 	h := sha256.New()
 	Print(h, v)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-// UpdateHash sets last to the hash of v and reports whether its value changed.
-func UpdateHash(last *string, v ...interface{}) (changed bool) {
-	sig := Hash(v)
-	if *last != sig {
-		*last = sig
-		return true
-	}
-	return false
-}
-
-func Print(w io.Writer, v ...interface{}) {
+func Print(w io.Writer, v interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 

ipn/ipnserver/server.go

@@ -69,6 +69,10 @@ type Options struct {
 	// DebugMux, if non-nil, specifies an HTTP ServeMux in which
 	// to register a debug handler.
 	DebugMux *http.ServeMux
+
+	// ErrorMessage, if not empty, signals that the server will exist
+	// only to relay the provided critical error message to the user.
+	ErrorMessage string
 }
 
 // server is an IPN backend and its set of 0 or more active connections
@@ -148,9 +152,7 @@ func (s *server) writeToClients(b []byte) {
 	}
 }
 
-// Run runs a Tailscale backend service.
-// The getEngine func is called repeatedly, once per connection, until it returns an engine successfully.
-func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (wgengine.Engine, error), opts Options) error {
+func Run(ctx context.Context, logf logger.Logf, logid string, opts Options, e wgengine.Engine) error {
 	runDone := make(chan struct{})
 	defer close(runDone)
 
@@ -177,38 +179,25 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 
 	bo := backoff.NewBackoff("ipnserver", logf)
 
-	var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet
-
-	eng, err := getEngine()
-	if err != nil {
-		logf("Initial getEngine call: %v", err)
+	if opts.ErrorMessage != "" {
 		for i := 1; ctx.Err() == nil; i++ {
-			c, err := listen.Accept()
+			s, err := listen.Accept()
 			if err != nil {
 				logf("%d: Accept: %v", i, err)
 				bo.BackOff(ctx, err)
 				continue
 			}
-			logf("%d: trying getEngine again...", i)
-			eng, err = getEngine()
-			if err == nil {
-				logf("%d: GetEngine worked; exiting failure loop", i)
-				unservedConn = c
-				break
+			serverToClient := func(b []byte) {
+				ipn.WriteMsg(s, b)
 			}
-			logf("%d: getEngine failed again: %v", i, err)
-			errMsg := err.Error()
 			go func() {
-				defer c.Close()
-				serverToClient := func(b []byte) { ipn.WriteMsg(c, b) }
+				defer s.Close()
 				bs := ipn.NewBackendServer(logf, nil, serverToClient)
-				bs.SendErrorMessage(errMsg)
-				time.Sleep(time.Second)
+				bs.SendErrorMessage(opts.ErrorMessage)
+				s.Read(make([]byte, 1))
 			}()
 		}
-		if err := ctx.Err(); err != nil {
-			return err
-		}
+		return ctx.Err()
 	}
 
 	var store ipn.StateStore
@@ -221,7 +210,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 		store = &ipn.MemoryStore{}
 	}
 
-	b, err := ipn.NewLocalBackend(logf, logid, store, eng)
+	b, err := ipn.NewLocalBackend(logf, logid, store, e)
 	if err != nil {
 		return fmt.Errorf("NewLocalBackend: %v", err)
 	}
@@ -254,14 +243,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() (
 	}
 
 	for i := 1; ctx.Err() == nil; i++ {
-		var c net.Conn
-		var err error
-		if unservedConn != nil {
-			c = unservedConn
-			unservedConn = nil
-		} else {
-			c, err = listen.Accept()
-		}
+		c, err := listen.Accept()
 		if err != nil {
 			if ctx.Err() == nil {
 				logf("ipnserver: Accept: %v", err)
@@ -389,8 +371,3 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		}
 	}
 }
-
-// FixedEngine returns a func that returns eng and a nil error.
-func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, error) {
-	return func() (wgengine.Engine, error) { return eng, nil }
-}

ipn/ipnserver/server_test.go

@@ -72,6 +72,6 @@ func TestRunMultipleAccepts(t *testing.T) {
 		SocketPath: socketPath,
 	}
 	t.Logf("pre-Run")
-	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", ipnserver.FixedEngine(eng), opts)
+	err = ipnserver.Run(ctx, logTriggerTestf, "dummy_logid", opts, eng)
 	t.Logf("ipnserver.Run = %v", err)
 }

ipn/ipnstate/ipnstate.go

@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 )
@@ -26,7 +25,6 @@ import (
 // Status represents the entire state of the IPN network.
 type Status struct {
 	BackendState string
-	TailscaleIPs []netaddr.IP // Tailscale IP(s) assigned to this node
 	Peer         map[key.Public]*PeerStatus
 	User         map[tailcfg.UserID]tailcfg.UserProfile
 }
@@ -111,18 +109,6 @@ func (sb *StatusBuilder) AddUser(id tailcfg.UserID, up tailcfg.UserProfile) {
 	sb.st.User[id] = up
 }
 
-// AddIP adds a Tailscale IP address to the status.
-func (sb *StatusBuilder) AddTailscaleIP(ip netaddr.IP) {
-	sb.mu.Lock()
-	defer sb.mu.Unlock()
-	if sb.locked {
-		log.Printf("[unexpected] ipnstate: AddIP after Locked")
-		return
-	}
-
-	sb.st.TailscaleIPs = append(sb.st.TailscaleIPs, ip)
-}
-
 // AddPeer adds a peer node to the status.
 //
 // Its PeerStatus is mixed with any previous status already added.
@@ -232,12 +218,6 @@ table tbody tr:nth-child(even) td { background-color: #f5f5f5; }
 	//f("<p><b>logid:</b> %s</p>\n", logid)
 	//f("<p><b>opts:</b> <code>%s</code></p>\n", html.EscapeString(fmt.Sprintf("%+v", opts)))
 
-	ips := make([]string, 0, len(st.TailscaleIPs))
-	for _, ip := range st.TailscaleIPs {
-		ips = append(ips, ip.String())
-	}
-	f("<p>Tailscale IP: %s", strings.Join(ips, ", "))
-
 	f("<table>\n<thead>\n")
 	f("<tr><th>Peer</th><th>Node</th><th>Owner</th><th>Rx</th><th>Tx</th><th>Activity</th><th>Endpoints</th></tr>\n")
 	f("</thead>\n<tbody>\n")

ipn/local.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/oauth2"
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
-	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
 	"tailscale.com/portlist"
@@ -52,10 +51,12 @@ type LocalBackend struct {
 	backendLogID    string
 	portpoll        *portlist.Poller // may be nil
 	portpollOnce    sync.Once
-	serverURL       string // tailcontrol URL
 	newDecompressor func() (controlclient.Decompressor, error)
 
-	filterHash string
+	// TODO: these fields are accessed unsafely by concurrent
+	// goroutines. They need to be protected.
+	serverURL       string // tailcontrol URL
+	lastFilterPrint time.Time
 
 	// The mutex protects the following elements.
 	mu       sync.Mutex
@@ -185,56 +186,21 @@ func (b *LocalBackend) SetDecompressor(fn func() (controlclient.Decompressor, er
 // setClientStatus is the callback invoked by the control client whenever it posts a new status.
 // Among other things, this is where we update the netmap, packet filters, DNS and DERP maps.
 func (b *LocalBackend) setClientStatus(st controlclient.Status) {
-	// The following do not depend on any data for which we need to lock b.
-	if st.Err != "" {
-		// TODO(crawshaw): display in the UI.
-		b.logf("Received error: %v", st.Err)
-		return
-	}
 	if st.LoginFinished != nil {
 		// Auth completed, unblock the engine
 		b.blockEngineUpdates(false)
 		b.authReconfig()
 		b.send(Notify{LoginFinished: &empty.Message{}})
 	}
-
-	prefsChanged := false
-
-	// Lock b once and do only the things that require locking.
-	b.mu.Lock()
-
-	prefs := b.prefs
-	stateKey := b.stateKey
-	netMap := b.netMap
-	interact := b.interact
-
 	if st.Persist != nil {
-		if b.prefs.Persist.Equals(st.Persist) {
-			prefsChanged = true
-			b.prefs.Persist = st.Persist.Clone()
-		}
-	}
-	if st.NetMap != nil {
-		b.netMap = st.NetMap
-	}
-	if st.URL != "" {
-		b.authURL = st.URL
-	}
-	if b.state == NeedsLogin {
-		if !b.prefs.WantRunning {
-			prefsChanged = true
-		}
-		b.prefs.WantRunning = true
-	}
-	// Prefs will be written out; this is not safe unless locked or cloned.
-	if prefsChanged {
-		prefs = b.prefs.Clone()
-	}
+		persist := *st.Persist // copy
 
-	b.mu.Unlock()
+		b.mu.Lock()
+		b.prefs.Persist = &persist
+		prefs := b.prefs.Clone()
+		stateKey := b.stateKey
+		b.mu.Unlock()
 
-	// Now complete the lock-free parts of what we started while locked.
-	if prefsChanged {
 		if stateKey != "" {
 			if err := b.store.WriteState(stateKey, prefs.ToBytes()); err != nil {
 				b.logf("Failed to save new controlclient state: %v", err)
@@ -243,41 +209,63 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.send(Notify{Prefs: prefs})
 	}
 	if st.NetMap != nil {
-		if netMap != nil {
-			diff := st.NetMap.ConciseDiffFrom(netMap)
+		// Netmap is unchanged only when the diff is empty.
+		changed := true
+		b.mu.Lock()
+		if b.netMap != nil {
+			diff := st.NetMap.ConciseDiffFrom(b.netMap)
 			if strings.TrimSpace(diff) == "" {
+				changed = false
 				b.logf("netmap diff: (none)")
 			} else {
 				b.logf("netmap diff:\n%v", diff)
 			}
 		}
+		disableDERP := b.prefs != nil && b.prefs.DisableDERP
+		b.netMap = st.NetMap
+		b.mu.Unlock()
 
-		b.updateFilter(st.NetMap, prefs)
-		b.e.SetNetworkMap(st.NetMap)
-
-		if !dnsMapsEqual(st.NetMap, netMap) {
+		b.send(Notify{NetMap: st.NetMap})
+		// There is nothing to update if the map hasn't changed.
+		if changed {
+			b.updateFilter(st.NetMap)
 			b.updateDNSMap(st.NetMap)
+			b.e.SetNetworkMap(st.NetMap)
 		}
-
-		disableDERP := prefs != nil && prefs.DisableDERP
 		if disableDERP {
 			b.e.SetDERPMap(nil)
 		} else {
 			b.e.SetDERPMap(st.NetMap.DERPMap)
 		}
-
-		b.send(Notify{NetMap: st.NetMap})
 	}
 	if st.URL != "" {
 		b.logf("Received auth URL: %.20v...", st.URL)
+
+		b.mu.Lock()
+		interact := b.interact
+		b.authURL = st.URL
+		b.mu.Unlock()
+
 		if interact > 0 {
 			b.popBrowserAuthNow()
 		}
 	}
+	if st.Err != "" {
+		// TODO(crawshaw): display in the UI.
+		b.logf("Received error: %v", st.Err)
+		return
+	}
+	if st.NetMap != nil {
+		b.mu.Lock()
+		if b.state == NeedsLogin {
+			b.prefs.WantRunning = true
+		}
+		prefs := b.prefs
+		b.mu.Unlock()
+
+		b.SetPrefs(prefs)
+	}
 	b.stateMachine()
-	// This is currently (2020-07-28) necessary; conditionally disabling it is fragile!
-	// This is where netmap information gets propagated to router and magicsock.
-	b.authReconfig()
 }
 
 // setWgengineStatus is the callback by the wireguard engine whenever it posts a new status.
@@ -372,7 +360,7 @@ func (b *LocalBackend) Start(opts Options) error {
 	persist := b.prefs.Persist
 	b.mu.Unlock()
 
-	b.updateFilter(nil, nil)
+	b.updateFilter(nil)
 
 	var discoPublic tailcfg.DiscoKey
 	if controlclient.Debug.Disco {
@@ -436,30 +424,20 @@ func (b *LocalBackend) Start(opts Options) error {
 
 // updateFilter updates the packet filter in wgengine based on the
 // given netMap and user preferences.
-func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Prefs) {
+func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
 	if netMap == nil {
 		// Not configured yet, block everything
 		b.logf("netmap packet filter: (not ready yet)")
 		b.e.SetFilter(filter.NewAllowNone(b.logf))
 		return
 	}
-	packetFilter := netMap.PacketFilter
-
-	var advRoutes []wgcfg.CIDR
-	if prefs != nil {
-		advRoutes = prefs.AdvertiseRoutes
-	}
-	// Be conservative while not ready.
-	shieldsUp := prefs == nil || prefs.ShieldsUp
-
-	changed := deepprint.UpdateHash(&b.filterHash, packetFilter, advRoutes, shieldsUp)
-	if !changed {
-		return
-	}
 
+	b.mu.Lock()
+	advRoutes := b.prefs.AdvertiseRoutes
+	b.mu.Unlock()
 	localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes)
 
-	if shieldsUp {
+	if b.shieldsAreUp() {
 		// Shields up, block everything
 		b.logf("netmap packet filter: (shields up)")
 		var prevFilter *filter.Filter // don't reuse old filter state
@@ -467,81 +445,44 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Pre
 		return
 	}
 
-	b.logf("netmap packet filter: %v", packetFilter)
-	b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
-}
-
-// dnsCIDRsEqual determines whether two CIDR lists are equal
-// for DNS map construction purposes (that is, only the first entry counts).
-func dnsCIDRsEqual(newAddr, oldAddr []wgcfg.CIDR) bool {
-	if len(newAddr) != len(oldAddr) {
-		return false
+	// TODO(apenwarr): don't replace filter at all if unchanged.
+	// TODO(apenwarr): print a diff instead of full filter.
+	now := time.Now()
+	if now.Sub(b.lastFilterPrint) > 1*time.Minute {
+		b.logf("netmap packet filter: %v", netMap.PacketFilter)
+		b.lastFilterPrint = now
+	} else {
+		b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter))
 	}
-	if len(newAddr) == 0 || newAddr[0] == oldAddr[0] {
-		return true
-	}
-	return false
-}
-
-// dnsMapsEqual determines whether the new and the old network map
-// induce the same DNS map. It does so without allocating memory,
-// at the expense of giving false negatives if peers are reordered.
-func dnsMapsEqual(new, old *controlclient.NetworkMap) bool {
-	if (old == nil) != (new == nil) {
-		return false
-	}
-	if old == nil && new == nil {
-		return true
-	}
-
-	if len(new.Peers) != len(old.Peers) {
-		return false
-	}
-
-	if new.Name != old.Name {
-		return false
-	}
-	if !dnsCIDRsEqual(new.Addresses, old.Addresses) {
-		return false
-	}
-
-	for i, newPeer := range new.Peers {
-		oldPeer := old.Peers[i]
-		if newPeer.Name != oldPeer.Name {
-			return false
-		}
-		if !dnsCIDRsEqual(newPeer.Addresses, oldPeer.Addresses) {
-			return false
-		}
-	}
-
-	return true
+	b.e.SetFilter(filter.New(netMap.PacketFilter, localNets, b.e.GetFilter(), b.logf))
 }
 
 // updateDNSMap updates the domain map in the DNS resolver in wgengine
 // based on the given netMap and user preferences.
 func (b *LocalBackend) updateDNSMap(netMap *controlclient.NetworkMap) {
 	if netMap == nil {
-		b.logf("dns map: (not ready)")
 		return
 	}
 
-	nameToIP := make(map[string]netaddr.IP)
-	set := func(name string, addrs []wgcfg.CIDR) {
+	domainToIP := make(map[string]netaddr.IP)
+	set := func(hostname string, addrs []wgcfg.CIDR) {
 		if len(addrs) == 0 {
 			return
 		}
-		nameToIP[name] = netaddr.IPFrom16(addrs[0].IP.Addr)
+		domain := hostname
+		// Like PeerStatus.SimpleHostName()
+		domain = strings.TrimSuffix(domain, ".local")
+		domain = strings.TrimSuffix(domain, ".localdomain")
+		domain = domain + ".b.tailscale.net"
+		domainToIP[domain] = netaddr.IPFrom16(addrs[0].IP.Addr)
 	}
 
 	for _, peer := range netMap.Peers {
-		set(peer.Name, peer.Addresses)
+		set(peer.Hostinfo.Hostname, peer.Addresses)
 	}
-	set(netMap.Name, netMap.Addresses)
+	set(netMap.Hostinfo.Hostname, netMap.Addresses)
 
-	dnsMap := tsdns.NewMap(nameToIP)
-	// map diff will be logged in tsdns.Resolver.SetMap.
-	b.e.SetDNSMap(dnsMap)
+	b.e.SetDNSMap(tsdns.NewMap(domainToIP))
 }
 
 // readPoller is a goroutine that receives service lists from
@@ -780,45 +721,37 @@ func (b *LocalBackend) SetPrefs(new *Prefs) {
 	}
 
 	b.mu.Lock()
-
-	netMap := b.netMap
-	stateKey := b.stateKey
-
 	old := b.prefs
 	new.Persist = old.Persist // caller isn't allowed to override this
 	b.prefs = new
-	// We do this to avoid holding the lock while doing everything else.
-	new = b.prefs.Clone()
-
+	if b.stateKey != "" {
+		if err := b.store.WriteState(b.stateKey, b.prefs.ToBytes()); err != nil {
+			b.logf("Failed to save new controlclient state: %v", err)
+		}
+	}
 	oldHi := b.hostinfo
 	newHi := oldHi.Clone()
 	newHi.RoutableIPs = append([]wgcfg.CIDR(nil), b.prefs.AdvertiseRoutes...)
 	applyPrefsToHostinfo(newHi, new)
 	b.hostinfo = newHi
 	hostInfoChanged := !oldHi.Equal(newHi)
-
 	b.mu.Unlock()
 
-	if stateKey != "" {
-		if err := b.store.WriteState(stateKey, new.ToBytes()); err != nil {
-			b.logf("Failed to save new controlclient state: %v", err)
-		}
-	}
-
 	b.logf("SetPrefs: %v", new.Pretty())
 
 	if old.ShieldsUp != new.ShieldsUp || hostInfoChanged {
 		b.doSetHostinfoFilterServices(newHi)
 	}
 
-	b.updateFilter(netMap, new)
+	b.updateFilter(b.netMap)
+	// TODO(dmytro): when Prefs gain an EnableTailscaleDNS toggle, updateDNSMap here.
 
 	turnDERPOff := new.DisableDERP && !old.DisableDERP
 	turnDERPOn := !new.DisableDERP && old.DisableDERP
 	if turnDERPOff {
 		b.e.SetDERPMap(nil)
-	} else if turnDERPOn && netMap != nil {
-		b.e.SetDERPMap(netMap.DERPMap)
+	} else if turnDERPOn && b.netMap != nil {
+		b.e.SetDERPMap(b.netMap.DERPMap)
 	}
 
 	if old.WantRunning != new.WantRunning {

logtail/logtail.go

@@ -462,6 +462,5 @@ func (l *logger) Write(buf []byte) (int, error) {
 		}
 	}
 	b := l.encode(buf)
-	_, err := l.send(b)
-	return len(buf), err
+	return l.send(b)
 }

logtail/logtail_test.go

@@ -32,18 +32,3 @@ func TestLoggerEncodeTextAllocs(t *testing.T) {
 		t.Logf("allocs = %d; want 1", int(n))
 	}
 }
-
-func TestLoggerWriteLength(t *testing.T) {
-	lg := &logger{
-		timeNow: time.Now,
-		buffer:  NewMemoryBuffer(1024),
-	}
-	inBuf := []byte("some text to encode")
-	n, err := lg.Write(inBuf)
-	if err != nil {
-		t.Error(err)
-	}
-	if n != len(inBuf) {
-		t.Errorf("logger.Write wrote %d bytes, expected %d", n, len(inBuf))
-	}
-}

net/interfaces/interfaces_linux.go

@@ -5,14 +5,8 @@
 package interfaces
 
 import (
-	"bytes"
-	"log"
-	"os/exec"
-	"runtime"
-
 	"go4.org/mem"
 	"inet.af/netaddr"
-	"tailscale.com/syncs"
 	"tailscale.com/util/lineread"
 )
 
@@ -20,8 +14,6 @@ func init() {
 	likelyHomeRouterIP = likelyHomeRouterIPLinux
 }
 
-var procNetRouteErr syncs.AtomicBool
-
 /*
 Parse 10.0.0.1 out of:
 
@@ -31,17 +23,9 @@ ens18   00000000        0100000A        0003    0       0       0       00000000
 ens18   0000000A        00000000        0001    0       0       0       0000FFFF        0       0       0
 */
 func likelyHomeRouterIPLinux() (ret netaddr.IP, ok bool) {
-	if procNetRouteErr.Get() {
-		// If we failed to read /proc/net/route previously, don't keep trying.
-		// But if we're on Android, go into the Android path.
-		if runtime.GOOS == "android" {
-			return likelyHomeRouterIPAndroid()
-		}
-		return ret, false
-	}
 	lineNum := 0
 	var f []mem.RO
-	err := lineread.File("/proc/net/route", func(line []byte) error {
+	lineread.File("/proc/net/route", func(line []byte) error {
 		lineNum++
 		if lineNum == 1 {
 			// Skip header line.
@@ -71,47 +55,5 @@ func likelyHomeRouterIPLinux() (ret netaddr.IP, ok bool) {
 		}
 		return nil
 	})
-	if err != nil {
-		procNetRouteErr.Set(true)
-		if runtime.GOOS == "android" {
-			return likelyHomeRouterIPAndroid()
-		}
-		log.Printf("interfaces: failed to read /proc/net/route: %v", err)
-	}
-	return ret, !ret.IsZero()
-}
-
-// Android apps don't have permission to read /proc/net/route, at
-// least on Google devices and the Android emulator.
-func likelyHomeRouterIPAndroid() (ret netaddr.IP, ok bool) {
-	cmd := exec.Command("/system/bin/ip", "route", "show", "table", "0")
-	out, err := cmd.StdoutPipe()
-	if err != nil {
-		return
-	}
-	if err := cmd.Start(); err != nil {
-		log.Printf("interfaces: running /system/bin/ip: %v", err)
-		return
-	}
-	// Search for line like "default via 10.0.2.2 dev radio0 table 1016 proto static mtu 1500 "
-	lineread.Reader(out, func(line []byte) error {
-		const pfx = "default via "
-		if !mem.HasPrefix(mem.B(line), mem.S(pfx)) {
-			return nil
-		}
-		line = line[len(pfx):]
-		sp := bytes.IndexByte(line, ' ')
-		if sp == -1 {
-			return nil
-		}
-		ipb := line[:sp]
-		if ip, err := netaddr.ParseIP(string(ipb)); err == nil && ip.Is4() {
-			ret = ip
-			log.Printf("interfaces: found Android default route %v", ip)
-		}
-		return nil
-	})
-	cmd.Process.Kill()
-	cmd.Wait()
 	return ret, !ret.IsZero()
 }

net/netcheck/netcheck.go

@@ -18,9 +18,7 @@ import (
 	"log"
 	"net"
 	"net/http"
-	"os"
 	"sort"
-	"strconv"
 	"sync"
 	"time"
 
@@ -38,45 +36,6 @@ import (
 	"tailscale.com/types/opt"
 )
 
-// Debugging and experimentation tweakables.
-var (
-	debugNetcheck, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_NETCHECK"))
-)
-
-// The various default timeouts for things.
-const (
-	// overallProbeTimeout is the maximum amount of time netcheck will
-	// spend gathering a single report.
-	overallProbeTimeout = 5 * time.Second
-	// stunTimeout is the maximum amount of time netcheck will spend
-	// probing with STUN packets without getting a reply before
-	// switching to HTTP probing, on the assumption that outbound UDP
-	// is blocked.
-	stunProbeTimeout = 3 * time.Second
-	// hairpinCheckTimeout is the amount of time we wait for a
-	// hairpinned packet to come back.
-	hairpinCheckTimeout = 100 * time.Millisecond
-	// defaultActiveRetransmitTime is the retransmit interval we use
-	// for STUN probes when we're in steady state (not in start-up),
-	// but don't have previous latency information for a DERP
-	// node. This is a somewhat conservative guess because if we have
-	// no data, likely the DERP node is very far away and we have no
-	// data because we timed out the last time we probed it.
-	defaultActiveRetransmitTime = 200 * time.Millisecond
-	// defaultInitialRetransmitTime is the retransmit interval used
-	// when netcheck first runs. We have no past context to work with,
-	// and we want answers relatively quickly, so it's biased slightly
-	// more aggressive than defaultActiveRetransmitTime. A few extra
-	// packets at startup is fine.
-	defaultInitialRetransmitTime = 100 * time.Millisecond
-	// portMapServiceProbeTimeout is the time we wait for port mapping
-	// services (UPnP, NAT-PMP, PCP) to respond before we give up and
-	// decide that they're not there. Since these services are on the
-	// same LAN as this machine and a single L3 hop away, we don't
-	// give them much time to respond.
-	portMapServiceProbeTimeout = 100 * time.Millisecond
-)
-
 type Report struct {
 	UDP                   bool     // UDP works
 	IPv6                  bool     // IPv6 works
@@ -180,7 +139,7 @@ func (c *Client) logf(format string, a ...interface{}) {
 }
 
 func (c *Client) vlogf(format string, a ...interface{}) {
-	if c.Verbose || debugNetcheck {
+	if c.Verbose {
 		c.logf(format, a...)
 	}
 }
@@ -211,8 +170,6 @@ func (c *Client) MakeNextReportFull() {
 }
 
 func (c *Client) ReceiveSTUNPacket(pkt []byte, src netaddr.IPPort) {
-	c.vlogf("received STUN packet from %s", src)
-
 	c.mu.Lock()
 	if c.handleHairSTUNLocked(pkt, src) {
 		c.mu.Unlock()
@@ -373,7 +330,7 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *interfaces.State, last *Report)
 			n := reg.Nodes[try%len(reg.Nodes)]
 			prevLatency := last.RegionLatency[reg.RegionID] * 120 / 100
 			if prevLatency == 0 {
-				prevLatency = defaultActiveRetransmitTime
+				prevLatency = 200 * time.Millisecond
 			}
 			delay := time.Duration(try) * prevLatency
 			if do4 {
@@ -396,12 +353,16 @@ func makeProbePlan(dm *tailcfg.DERPMap, ifState *interfaces.State, last *Report)
 func makeProbePlanInitial(dm *tailcfg.DERPMap, ifState *interfaces.State) (plan probePlan) {
 	plan = make(probePlan)
 
+	// initialSTUNTimeout is only 100ms because some extra retransmits
+	// when starting up is tolerable.
+	const initialSTUNTimeout = 100 * time.Millisecond
+
 	for _, reg := range dm.Regions {
 		var p4 []probe
 		var p6 []probe
 		for try := 0; try < 3; try++ {
 			n := reg.Nodes[try%len(reg.Nodes)]
-			delay := time.Duration(try) * defaultInitialRetransmitTime
+			delay := time.Duration(try) * initialSTUNTimeout
 			if ifState.HaveV4 && nodeMight4(n) {
 				p4 = append(p4, probe{delay: delay, node: n.Name, proto: probeIPv4})
 			}
@@ -557,7 +518,7 @@ func (rs *reportState) startHairCheckLocked(dst netaddr.IPPort) {
 	ua := dst.UDPAddr()
 	rs.pc4Hair.WriteTo(stun.Request(rs.hairTX), ua)
 	rs.c.vlogf("sent haircheck to %v", ua)
-	time.AfterFunc(hairpinCheckTimeout, func() { close(rs.hairTimeout) })
+	time.AfterFunc(500*time.Millisecond, func() { close(rs.hairTimeout) })
 }
 
 func (rs *reportState) waitHairCheck(ctx context.Context) {
@@ -578,7 +539,6 @@ func (rs *reportState) waitHairCheck(ctx context.Context) {
 	case <-rs.gotHairSTUN:
 		ret.HairPinning.Set(true)
 	case <-rs.hairTimeout:
-		rs.c.vlogf("hairCheck timeout")
 		ret.HairPinning.Set(false)
 	default:
 		select {
@@ -689,7 +649,7 @@ func (rs *reportState) probePortMapServices() {
 	}
 	defer uc.Close()
 	tempPort := uc.LocalAddr().(*net.UDPAddr).Port
-	uc.SetReadDeadline(time.Now().Add(portMapServiceProbeTimeout))
+	uc.SetReadDeadline(time.Now().Add(100 * time.Millisecond))
 
 	// Send request packets for all three protocols.
 	uc.WriteTo(uPnPPacket, port1900)
@@ -767,10 +727,15 @@ func newReport() *Report {
 //
 // It may not be called concurrently with itself.
 func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, error) {
+	// Wait for STUN for 3 seconds, but then give HTTP probing
+	// another 2 seconds if all UDP failed.
+	const overallTimeout = 5 * time.Second
+	const stunTimeout = 3 * time.Second
+
 	// Mask user context with ours that we guarantee to cancel so
 	// we can depend on it being closed in goroutines later.
 	// (User ctx might be context.Background, etc)
-	ctx, cancel := context.WithTimeout(ctx, overallProbeTimeout)
+	ctx, cancel := context.WithTimeout(ctx, overallTimeout)
 	defer cancel()
 
 	if dm == nil {
@@ -879,7 +844,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 		}(probeSet)
 	}
 
-	stunTimer := time.NewTimer(stunProbeTimeout)
+	stunTimer := time.NewTimer(stunTimeout)
 	defer stunTimer.Stop()
 
 	select {
@@ -892,9 +857,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 	}
 
 	rs.waitHairCheck(ctx)
-	c.vlogf("hairCheck done")
 	rs.waitPortMap.Wait()
-	c.vlogf("portMap done")
 	rs.stopTimers()
 
 	// Try HTTPS latency check if all STUN probes failed due to UDP presumably being blocked.
@@ -949,7 +912,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (*Report, e
 
 func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegion) (time.Duration, netaddr.IP, error) {
 	var result httpstat.Result
-	ctx, cancel := context.WithTimeout(httpstat.WithHTTPStat(ctx, &result), overallProbeTimeout)
+	ctx, cancel := context.WithTimeout(httpstat.WithHTTPStat(ctx, &result), 5*time.Second)
 	defer cancel()
 
 	var ip netaddr.IP

portlist/portlist_linux.go

@@ -10,15 +10,12 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"runtime"
 	"sort"
 	"strconv"
 	"strings"
-	"syscall"
 	"time"
 
 	"golang.org/x/sys/unix"
-	"tailscale.com/syncs"
 )
 
 // Reading the sockfiles on Linux is very fast, so we can do it often.
@@ -29,30 +26,13 @@ const pollInterval = 1 * time.Second
 var sockfiles = []string{"/proc/net/tcp", "/proc/net/udp"}
 var protos = []string{"tcp", "udp"}
 
-var sawProcNetPermissionErr syncs.AtomicBool
-
 func listPorts() (List, error) {
-	if sawProcNetPermissionErr.Get() {
-		return nil, nil
-	}
 	l := []Port{}
 
 	for pi, fname := range sockfiles {
 		proto := protos[pi]
 
-		// Android 10+ doesn't allow access to this anymore.
-		// https://developer.android.com/about/versions/10/privacy/changes#proc-net-filesystem
-		// Ignore it rather than have the system log about our violation.
-		if runtime.GOOS == "android" && syscall.Access(fname, unix.R_OK) != nil {
-			sawProcNetPermissionErr.Set(true)
-			return nil, nil
-		}
-
 		f, err := os.Open(fname)
-		if os.IsPermission(err) {
-			sawProcNetPermissionErr.Set(true)
-			return nil, nil
-		}
 		if err != nil {
 			return nil, fmt.Errorf("%s: %s", fname, err)
 		}
@@ -116,18 +96,7 @@ func addProcesses(pl []Port) ([]Port, error) {
 	}
 
 	err := foreachPID(func(pid string) error {
-		fdPath := fmt.Sprintf("/proc/%s/fd", pid)
-
-		// Android logs a bunch of audit violations in logcat
-		// if we try to open things we don't have access
-		// to. So on Android only, ask if we have permission
-		// rather than just trying it to determine whether we
-		// have permission.
-		if runtime.GOOS == "android" && syscall.Access(fdPath, unix.R_OK) != nil {
-			return nil
-		}
-
-		fdDir, err := os.Open(fdPath)
+		fdDir, err := os.Open(fmt.Sprintf("/proc/%s/fd", pid))
 		if err != nil {
 			// Can't open fd list for this pid. Maybe
 			// don't have access. Ignore it.

tailcfg/tailcfg.go

@@ -260,7 +260,6 @@ type Hostinfo struct {
 	OSVersion     string       // operating system version, with optional distro prefix ("Debian 10.4", "Windows 10 Pro 10.0.19041")
 	DeviceModel   string       // mobile phone model ("Pixel 3a", "iPhone 11 Pro")
 	Hostname      string       // name of the host the client runs on
-	GoArch        string       // the host's GOARCH value (of the running binary)
 	RoutableIPs   []wgcfg.CIDR `json:",omitempty"` // set of IP ranges this client can route
 	RequestTags   []string     `json:",omitempty"` // set of ACL tags this node wants to claim
 	Services      []Service    `json:",omitempty"` // services advertised by this machine

tailcfg/tailcfg_test.go

@@ -24,7 +24,7 @@ func fieldsOf(t reflect.Type) (fields []string) {
 func TestHostinfoEqual(t *testing.T) {
 	hiHandles := []string{
 		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "OSVersion",
-		"DeviceModel", "Hostname", "GoArch", "RoutableIPs", "RequestTags", "Services",
+		"DeviceModel", "Hostname", "RoutableIPs", "RequestTags", "Services",
 		"NetInfo",
 	}
 	if have := fieldsOf(reflect.TypeOf(Hostinfo{})); !reflect.DeepEqual(have, hiHandles) {

tstest/natlab/natlab.go

@@ -93,10 +93,9 @@ func mustPrefix(s string) netaddr.IPPrefix {
 // NewInternet returns a network that simulates the internet.
 func NewInternet() *Network {
 	return &Network{
-		Name: "internet",
-		// easily recognizable internett-y addresses
-		Prefix4: mustPrefix("1.0.0.0/24"),
-		Prefix6: mustPrefix("1111::/64"),
+		Name:    "internet",
+		Prefix4: mustPrefix("203.0.113.0/24"), // documentation netblock that looks Internet-y
+		Prefix6: mustPrefix("fc00:52::/64"),
 	}
 }
 
@@ -185,17 +184,6 @@ func (n *Network) write(p *Packet) (num int, err error) {
 	defer n.mu.Unlock()
 	iface, ok := n.machine[p.Dst.IP]
 	if !ok {
-		// If the destination is within the network's authoritative
-		// range, no route to host.
-		if p.Dst.IP.Is4() && n.Prefix4.Contains(p.Dst.IP) {
-			p.Trace("no route to %v", p.Dst.IP)
-			return len(p.Payload), nil
-		}
-		if p.Dst.IP.Is6() && n.Prefix6.Contains(p.Dst.IP) {
-			p.Trace("no route to %v", p.Dst.IP)
-			return len(p.Payload), nil
-		}
-
 		if n.defaultGW == nil {
 			p.Trace("no route to %v", p.Dst.IP)
 			return len(p.Payload), nil

wgengine/filter/filter.go

@@ -136,13 +136,9 @@ func maybeHexdump(flag RunFlags, b []byte) string {
 var acceptBucket = rate.NewLimiter(rate.Every(10*time.Second), 3)
 var dropBucket = rate.NewLimiter(rate.Every(5*time.Second), 10)
 
-func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, dir direction, r Response, why string) {
+func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, r Response, why string) {
 	var verdict string
 
-	if r == Drop && f.omitDropLogging(q, dir) {
-		return
-	}
-
 	if r == Drop && (runflags&LogDrops) != 0 && dropBucket.Allow() {
 		verdict = "Drop"
 		runflags &= HexdumpDrops
@@ -161,28 +157,26 @@ func (f *Filter) logRateLimit(runflags RunFlags, q *packet.ParsedPacket, dir dir
 
 // RunIn determines whether this node is allowed to receive q from a Tailscale peer.
 func (f *Filter) RunIn(q *packet.ParsedPacket, rf RunFlags) Response {
-	dir := in
-	r := f.pre(q, rf, dir)
+	r := f.pre(q, rf)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
 	}
 
 	r, why := f.runIn(q)
-	f.logRateLimit(rf, q, dir, r, why)
+	f.logRateLimit(rf, q, r, why)
 	return r
 }
 
 // RunOut determines whether this node is allowed to send q to a Tailscale peer.
 func (f *Filter) RunOut(q *packet.ParsedPacket, rf RunFlags) Response {
-	dir := out
-	r := f.pre(q, rf, dir)
+	r := f.pre(q, rf)
 	if r == Drop || r == Accept {
 		// already logged
 		return r
 	}
 	r, why := f.runOut(q)
-	f.logRateLimit(rf, q, dir, r, why)
+	f.logRateLimit(rf, q, r, why)
 	return r
 }
 
@@ -194,11 +188,6 @@ func (f *Filter) runIn(q *packet.ParsedPacket) (r Response, why string) {
 		return Drop, "destination not allowed"
 	}
 
-	if q.IPVersion == 6 {
-		// TODO: support IPv6.
-		return Drop, "no rules matched"
-	}
-
 	switch q.IPProto {
 	case packet.ICMP:
 		if q.IsEchoResponse() || q.IsError() {
@@ -258,78 +247,30 @@ func (f *Filter) runOut(q *packet.ParsedPacket) (r Response, why string) {
 	return Accept, "ok out"
 }
 
-// direction is whether a packet was flowing in to this machine, or flowing out.
-type direction int
-
-const (
-	in direction = iota
-	out
-)
-
-func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags, dir direction) Response {
+func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags) Response {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
 		return Accept
 	}
 	if len(q.Buffer()) < 20 {
-		f.logRateLimit(rf, q, dir, Drop, "too short")
+		f.logRateLimit(rf, q, Drop, "too short")
 		return Drop
 	}
 
-	if q.IPVersion == 6 {
-		f.logRateLimit(rf, q, dir, Drop, "ipv6")
-		return Drop
-	}
 	switch q.IPProto {
 	case packet.Unknown:
 		// Unknown packets are dangerous; always drop them.
-		f.logRateLimit(rf, q, dir, Drop, "unknown")
+		f.logRateLimit(rf, q, Drop, "unknown")
+		return Drop
+	case packet.IPv6:
+		f.logRateLimit(rf, q, Drop, "ipv6")
 		return Drop
 	case packet.Fragment:
 		// Fragments after the first always need to be passed through.
 		// Very small fragments are considered Junk by ParsedPacket.
-		f.logRateLimit(rf, q, dir, Accept, "fragment")
+		f.logRateLimit(rf, q, Accept, "fragment")
 		return Accept
 	}
 
 	return noVerdict
 }
-
-// ipv6AllRoutersLinkLocal is ff02::2 (All link-local routers).
-const ipv6AllRoutersLinkLocal = "\xff\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"
-
-// omitDropLogging reports whether packet p, which has already been
-// deemded a packet to Drop, should bypass the [rate-limited] logging.
-// We don't want to log scary & spammy reject warnings for packets that
-// are totally normal, like IPv6 route announcements.
-func (f *Filter) omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
-	switch dir {
-	case out:
-		switch p.IPVersion {
-		case 4:
-			// Omit logging about outgoing IGMP queries being dropped.
-			if p.IPProto == packet.IGMP {
-				return true
-			}
-		case 6:
-			b := p.Buffer()
-			if len(b) < 40 {
-				return false
-			}
-			src, dst := b[8:8+16], b[24:24+16]
-			// Omit logging for outgoing IPv6 ICMP-v6 queries to ff02::2,
-			// as sent by the OS, looking for routers.
-			if p.IPProto == packet.ICMPv6 {
-				if isLinkLocalV6(src) && string(dst) == ipv6AllRoutersLinkLocal {
-					return true
-				}
-			}
-		}
-	}
-	return false
-}
-
-// isLinkLocalV6 reports whether src is in fe80::/10.
-func isLinkLocalV6(src []byte) bool {
-	return len(src) == 16 && src[0] == 0xfe && src[1]>>6 == 0x80>>6
-}

wgengine/filter/filter_test.go

@@ -219,7 +219,7 @@ func TestPreFilter(t *testing.T) {
 	for _, testPacket := range packets {
 		p := &ParsedPacket{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts, in)
+		got := f.pre(p, LogDrops|LogAccepts)
 		if got != testPacket.want {
 			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
 		}

wgengine/magicsock/magicsock.go

@@ -55,43 +55,6 @@ import (
 	"tailscale.com/version"
 )
 
-// Various debugging and experimental tweakables, set by environment
-// variable.
-var (
-	// logPacketDests prints the known addresses for a peer every time
-	// they change, in the legacy (non-discovery) endpoint code only.
-	logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
-	// debugDisco prints verbose logs of active discovery events as
-	// they happen.
-	debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
-	// debugOmitLocalAddresses removes all local interface addresses
-	// from magicsock's discovered local endpoints. Used in some tests.
-	debugOmitLocalAddresses, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_OMIT_LOCAL_ADDRS"))
-	// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
-	// reverse routing is enabled (Issue 150). It will become always true
-	// later.
-	debugUseDerpRoute, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE"))
-	// logDerpVerbose logs all received DERP packets, including their
-	// full payload.
-	logDerpVerbose, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DERP"))
-	// debugReSTUNStopOnIdle unconditionally enables the "shut down
-	// STUN if magicsock is idle" behavior that normally only triggers
-	// on mobile devices, lowers the shutdown interval, and logs more
-	// verbosely about idle measurements.
-	debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
-)
-
-// inTest reports whether the running program is a test that set the
-// IN_TS_TEST environment variable.
-//
-// Unlike the other debug tweakables above, this one needs to be
-// checked every time at runtime, because tests set this after program
-// startup.
-func inTest() bool {
-	inTest, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST"))
-	return inTest
-}
-
 // A Conn routes UDP packets and actively manages a list of its endpoints.
 // It implements wireguard/conn.Bind.
 type Conn struct {
@@ -175,9 +138,8 @@ type Conn struct {
 	derpMap     *tailcfg.DERPMap // nil (or zero regions/nodes) means DERP is disabled
 	netMap      *controlclient.NetworkMap
 	privateKey  key.Private
-	everHadKey  bool               // whether we ever had a non-zero private key
 	myDerp      int                // nearest DERP region ID; 0 means none/unknown
-	derpStarted chan struct{}      // closed on first connection to DERP; for tests & cleaner Close
+	derpStarted chan struct{}      // closed on first connection to DERP; for tests
 	activeDerp  map[int]activeDerp // DERP regionID -> connection to a node in that region
 	prevDerp    map[int]*syncs.WaitGroupChan
 
@@ -640,6 +602,8 @@ func (c *Conn) goDerpConnect(node int) {
 	go c.derpWriteChanOfAddr(netaddr.IPPort{IP: derpMagicIPAddr, Port: uint16(node)}, key.Public{})
 }
 
+var debugOmitLocalAddresses, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_OMIT_LOCAL_ADDRS"))
+
 // determineEndpoints returns the machine's endpoint addresses. It
 // does a STUN lookup (via netcheck) to determine its public address.
 //
@@ -741,6 +705,10 @@ func shouldSprayPacket(b []byte) bool {
 	return false
 }
 
+var logPacketDests, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_LOG_PACKET_DESTS"))
+
+var debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
+
 const sprayPeriod = 3 * time.Second
 
 // appendDests appends to dsts the destinations that b should be
@@ -965,6 +933,11 @@ func (c *Conn) sendAddr(addr netaddr.IPPort, pubKey key.Public, b []byte) (sent
 // TODO: this is currently arbitrary. Figure out something better?
 const bufferedDerpWritesBeforeDrop = 32
 
+// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
+// reverse routing is enabled (Issue 150). It will become always true
+// later.
+var debugUseDerpRoute, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE"))
+
 // derpWriteChanOfAddr returns a DERP client for fake UDP addresses that
 // represent DERP servers, creating them as necessary. For real UDP
 // addresses, it returns nil.
@@ -1033,11 +1006,6 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.Public) chan<-
 	// Note that derphttp.NewClient does not dial the server
 	// so it is safe to do under the mu lock.
 	dc := derphttp.NewRegionClient(c.privateKey, c.logf, func() *tailcfg.DERPRegion {
-		if c.connCtx.Err() != nil {
-			// If we're closing, don't try to acquire the lock.
-			// We might already be in Conn.Close and the Lock would deadlock.
-			return nil
-		}
 		c.mu.Lock()
 		defer c.mu.Unlock()
 		if c.derpMap == nil {
@@ -1140,6 +1108,8 @@ type derpReadResult struct {
 	copyBuf func(dst []byte) int
 }
 
+var logDerpVerbose, _ = strconv.ParseBool(os.Getenv("DEBUG_DERP_VERBOSE"))
+
 // runDerpReader runs in a goroutine for the life of a DERP
 // connection, handling received packets.
 func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, dc *derphttp.Client, wg *syncs.WaitGroupChan, startGate <-chan struct{}) {
@@ -1262,11 +1232,8 @@ func (c *Conn) runDerpWriter(ctx context.Context, dc *derphttp.Client, ch <-chan
 // The provided addr and ipp must match.
 //
 // TODO(bradfitz): add a fast path that returns nil here for normal
-// wireguard-go transport packets; wireguard-go only uses this
-// Endpoint for the relatively rare non-data packets; but we need the
-// Endpoint to find the UDPAddr to return to wireguard anyway, so no
-// benefit unless we can, say, always return the same fake UDPAddr for
-// all packets.
+// wireguard-go transport packets; IIRC wireguard-go only uses this
+// Endpoint for the relatively rare non-data packets.
 func (c *Conn) findEndpoint(ipp netaddr.IPPort, addr *net.UDPAddr) conn.Endpoint {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -1819,7 +1786,6 @@ func (c *Conn) SetPrivateKey(privateKey wgcfg.PrivateKey) error {
 	c.privateKey = newKey
 
 	if oldKey.IsZero() {
-		c.everHadKey = true
 		c.logf("magicsock: SetPrivateKey called (init)")
 		go c.ReSTUN("set-private-key")
 	} else if newKey.IsZero() {
@@ -2099,6 +2065,8 @@ func (c *Conn) Close() error {
 	return err
 }
 
+var debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
+
 func maxIdleBeforeSTUNShutdown() time.Duration {
 	if debugReSTUNStopOnIdle {
 		return time.Minute
@@ -2189,19 +2157,8 @@ func (c *Conn) ReSTUN(why string) {
 		// raced with a shutdown.
 		return
 	}
-
-	// If the user stopped the app, stop doing work. (When the
-	// user stops Tailscale via the GUI apps, ipn/local.go
-	// reconfigures the engine with a zero private key.)
-	//
-	// This used to just check c.privateKey.IsZero, but that broke
-	// some end-to-end tests tests that didn't ever set a private
-	// key somehow. So for now, only stop doing work if we ever
-	// had a key, which helps real users, but appeases tests for
-	// now. TODO: rewrite those tests to be less brittle or more
-	// realistic.
-	if c.privateKey.IsZero() && c.everHadKey {
-		c.logf("magicsock: ReSTUN(%q) ignored; stopped, no private key", why)
+	if c.privateKey.IsZero() {
+		c.logf("magicsock: ReSTUN(%q) ignored; no private key", why)
 		return
 	}
 
@@ -2235,7 +2192,7 @@ func (c *Conn) listenPacket(ctx context.Context, network, addr string) (net.Pack
 
 func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 	host := ""
-	if inTest() {
+	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
 		host = "127.0.0.1"
 	}
 	var pc net.PacketConn
@@ -2265,7 +2222,7 @@ func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 // It should be followed by a call to ReSTUN.
 func (c *Conn) Rebind() {
 	host := ""
-	if inTest() {
+	if v, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST")); v {
 		host = "127.0.0.1"
 	}
 	listenCtx := context.Background() // unused without DNS name to resolve
@@ -2875,17 +2832,6 @@ func (c *Conn) UpdateStatus(sb *ipnstate.StatusBuilder) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if c.netMap != nil {
-		for _, addr := range c.netMap.Addresses {
-			if (addr.IP.Is4() && addr.Mask != 32) || (addr.IP.Is6() && addr.Mask != 128) {
-				continue
-			}
-			if ip, ok := netaddr.FromStdIP(addr.IP.IP()); ok {
-				sb.AddTailscaleIP(ip)
-			}
-		}
-	}
-
 	for dk, n := range c.nodeOfDisco {
 		ps := &ipnstate.PeerStatus{InMagicSock: true}
 		ps.Addrs = append(ps.Addrs, n.Endpoints...)

wgengine/magicsock/magicsock_test.go

@@ -6,7 +6,6 @@ package magicsock
 
 import (
 	"bytes"
-	"context"
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
@@ -27,11 +26,9 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"golang.org/x/crypto/nacl/box"
 	"inet.af/netaddr"
-	"tailscale.com/control/controlclient"
 	"tailscale.com/derp"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/derp/derpmap"
-	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/stun/stuntest"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
@@ -69,6 +66,11 @@ func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, st
 		t.Fatal(err)
 	}
 	d := derp.NewServer(serverPrivateKey, logf)
+	if l != (nettype.Std{}) {
+		// When using virtual networking, only allow DERP to forward
+		// discovery traffic, not actual packets.
+		d.OnlyDisco = true
+	}
 
 	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(d))
 	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
@@ -170,7 +172,7 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	// Wait for first endpoint update to be available
 	deadline := time.Now().Add(2 * time.Second)
 	for len(epCh) == 0 && time.Now().Before(deadline) {
-		time.Sleep(100 * time.Millisecond)
+		time.Sleep(10 * time.Millisecond)
 	}
 
 	return &magicStack{
@@ -183,136 +185,11 @@ func newMagicStack(t *testing.T, logf logger.Logf, l nettype.PacketListener, der
 	}
 }
 
-func (s *magicStack) String() string {
-	pub := s.Public()
-	return pub.ShortString()
-}
-
 func (s *magicStack) Close() {
 	s.dev.Close()
 	s.conn.Close()
 }
 
-func (s *magicStack) Public() key.Public {
-	return key.Public(s.privateKey.Public())
-}
-
-func (s *magicStack) Status() *ipnstate.Status {
-	var sb ipnstate.StatusBuilder
-	s.conn.UpdateStatus(&sb)
-	return sb.Status()
-}
-
-// IP returns the Tailscale IP address assigned to this magicStack.
-//
-// Something external needs to provide a NetworkMap and WireGuard
-// configs to the magicStack in order for it to acquire an IP
-// address. See meshStacks for one possible source of netmaps and IPs.
-func (s *magicStack) IP(t *testing.T) netaddr.IP {
-	for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) {
-		st := s.Status()
-		if len(st.TailscaleIPs) > 0 {
-			return st.TailscaleIPs[0]
-		}
-	}
-	t.Fatal("timed out waiting for magicstack to get an IP assigned")
-	panic("unreachable") // compiler doesn't know t.Fatal panics
-}
-
-// meshStacks monitors epCh on all given ms, and plumbs network maps
-// and WireGuard configs into everyone to form a full mesh that has up
-// to date endpoint info. Think of it as an extremely stripped down
-// and purpose-built Tailscale control plane.
-//
-// meshStacks only supports disco connections, not legacy logic.
-func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
-	ctx, cancel := context.WithCancel(context.Background())
-
-	// Serialize all reconfigurations globally, just to keep things
-	// simpler.
-	var (
-		mu  sync.Mutex
-		eps = make([][]string, len(ms))
-	)
-
-	buildNetmapLocked := func(myIdx int) *controlclient.NetworkMap {
-		me := ms[myIdx]
-		nm := &controlclient.NetworkMap{
-			PrivateKey: me.privateKey,
-			NodeKey:    tailcfg.NodeKey(me.privateKey.Public()),
-			Addresses:  []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(myIdx+1)), Mask: 32}},
-		}
-		for i, peer := range ms {
-			if i == myIdx {
-				continue
-			}
-			addrs := []wgcfg.CIDR{{IP: wgcfg.IPv4(1, 0, 0, byte(i+1)), Mask: 32}}
-			peer := &tailcfg.Node{
-				ID:         tailcfg.NodeID(i + 1),
-				Name:       fmt.Sprintf("node%d", i+1),
-				Key:        tailcfg.NodeKey(peer.privateKey.Public()),
-				DiscoKey:   peer.conn.DiscoPublicKey(),
-				Addresses:  addrs,
-				AllowedIPs: addrs,
-				Endpoints:  eps[i],
-				DERP:       "127.3.3.40:1",
-			}
-			nm.Peers = append(nm.Peers, peer)
-		}
-
-		return nm
-	}
-
-	updateEps := func(idx int, newEps []string) {
-		mu.Lock()
-		defer mu.Unlock()
-
-		eps[idx] = newEps
-
-		for i, m := range ms {
-			netmap := buildNetmapLocked(i)
-			m.conn.SetNetworkMap(netmap)
-			peerSet := make(map[key.Public]struct{}, len(netmap.Peers))
-			for _, peer := range netmap.Peers {
-				peerSet[key.Public(peer.Key)] = struct{}{}
-			}
-			m.conn.UpdatePeers(peerSet)
-			wg, err := netmap.WGCfg(logf, controlclient.AllowSingleHosts, nil)
-			if err != nil {
-				// We're too far from the *testing.T to be graceful,
-				// blow up. Shouldn't happen anyway.
-				panic(fmt.Sprintf("failed to construct wgcfg from netmap: %v", err))
-			}
-			if err := m.dev.Reconfig(wg); err != nil {
-				panic(fmt.Sprintf("device reconfig failed: %v", err))
-			}
-		}
-	}
-
-	var wg sync.WaitGroup
-	wg.Add(len(ms))
-	for i := range ms {
-		go func(myIdx int) {
-			defer wg.Done()
-
-			for {
-				select {
-				case <-ctx.Done():
-					return
-				case eps := <-ms[myIdx].epCh:
-					logf("conn%d endpoints update", myIdx+1)
-					updateEps(myIdx, eps)
-				}
-			}
-		}(i)
-	}
-
-	return func() {
-		cancel()
-		wg.Wait()
-	}
-}
-
 func TestNewConn(t *testing.T) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()
@@ -556,136 +433,65 @@ func makeNestable(t *testing.T) (logf logger.Logf, setT func(t *testing.T)) {
 }
 
 func TestTwoDevicePing(t *testing.T) {
-	l, ip := nettype.Std{}, netaddr.IPv4(127, 0, 0, 1)
-	n := &devices{
-		m1:     l,
-		m1IP:   ip,
-		m2:     l,
-		m2IP:   ip,
-		stun:   l,
-		stunIP: ip,
-	}
-	testTwoDevicePing(t, n)
-}
-
-func TestActiveDiscovery(t *testing.T) {
-	t.Run("simple_internet", func(t *testing.T) {
-		t.Parallel()
-		mstun := &natlab.Machine{Name: "stun"}
-		m1 := &natlab.Machine{Name: "m1"}
-		m2 := &natlab.Machine{Name: "m2"}
-		inet := natlab.NewInternet()
-		sif := mstun.Attach("eth0", inet)
-		m1if := m1.Attach("eth0", inet)
-		m2if := m2.Attach("eth0", inet)
-
+	t.Run("real", func(t *testing.T) {
+		l, ip := nettype.Std{}, netaddr.IPv4(127, 0, 0, 1)
 		n := &devices{
-			m1:     m1,
-			m1IP:   m1if.V4(),
-			m2:     m2,
-			m2IP:   m2if.V4(),
-			stun:   mstun,
-			stunIP: sif.V4(),
+			m1:     l,
+			m1IP:   ip,
+			m2:     l,
+			m2IP:   ip,
+			stun:   l,
+			stunIP: ip,
 		}
-		testActiveDiscovery(t, n)
+		testTwoDevicePing(t, n)
 	})
+	t.Run("natlab", func(t *testing.T) {
+		t.Run("simple internet", func(t *testing.T) {
+			mstun := &natlab.Machine{Name: "stun"}
+			m1 := &natlab.Machine{Name: "m1"}
+			m2 := &natlab.Machine{Name: "m2"}
+			inet := natlab.NewInternet()
+			sif := mstun.Attach("eth0", inet)
+			m1if := m1.Attach("eth0", inet)
+			m2if := m2.Attach("eth0", inet)
 
-	t.Run("facing_easy_firewalls", func(t *testing.T) {
-		mstun := &natlab.Machine{Name: "stun"}
-		m1 := &natlab.Machine{
-			Name:          "m1",
-			PacketHandler: &natlab.Firewall{},
-		}
-		m2 := &natlab.Machine{
-			Name:          "m2",
-			PacketHandler: &natlab.Firewall{},
-		}
-		inet := natlab.NewInternet()
-		sif := mstun.Attach("eth0", inet)
-		m1if := m1.Attach("eth0", inet)
-		m2if := m2.Attach("eth0", inet)
+			n := &devices{
+				m1:     m1,
+				m1IP:   m1if.V4(),
+				m2:     m2,
+				m2IP:   m2if.V4(),
+				stun:   mstun,
+				stunIP: sif.V4(),
+			}
+			testTwoDevicePing(t, n)
+		})
 
-		n := &devices{
-			m1:     m1,
-			m1IP:   m1if.V4(),
-			m2:     m2,
-			m2IP:   m2if.V4(),
-			stun:   mstun,
-			stunIP: sif.V4(),
-		}
-		testActiveDiscovery(t, n)
+		t.Run("facing firewalls", func(t *testing.T) {
+			mstun := &natlab.Machine{Name: "stun"}
+			m1 := &natlab.Machine{
+				Name:          "m1",
+				PacketHandler: &natlab.Firewall{},
+			}
+			m2 := &natlab.Machine{
+				Name:          "m2",
+				PacketHandler: &natlab.Firewall{},
+			}
+			inet := natlab.NewInternet()
+			sif := mstun.Attach("eth0", inet)
+			m1if := m1.Attach("eth0", inet)
+			m2if := m2.Attach("eth0", inet)
+
+			n := &devices{
+				m1:     m1,
+				m1IP:   m1if.V4(),
+				m2:     m2,
+				m2IP:   m2if.V4(),
+				stun:   mstun,
+				stunIP: sif.V4(),
+			}
+			testTwoDevicePing(t, n)
+		})
 	})
-
-	t.Run("facing_nats", func(t *testing.T) {
-		mstun := &natlab.Machine{Name: "stun"}
-		m1 := &natlab.Machine{
-			Name:          "m1",
-			PacketHandler: &natlab.Firewall{},
-		}
-		nat1 := &natlab.Machine{
-			Name: "nat1",
-		}
-		m2 := &natlab.Machine{
-			Name:          "m2",
-			PacketHandler: &natlab.Firewall{},
-		}
-		nat2 := &natlab.Machine{
-			Name: "nat2",
-		}
-
-		inet := natlab.NewInternet()
-		lan1 := &natlab.Network{
-			Name:    "lan1",
-			Prefix4: mustPrefix("192.168.0.0/24"),
-		}
-		lan2 := &natlab.Network{
-			Name:    "lan2",
-			Prefix4: mustPrefix("192.168.1.0/24"),
-		}
-
-		sif := mstun.Attach("eth0", inet)
-		nat1WAN := nat1.Attach("wan", inet)
-		nat1LAN := nat1.Attach("lan1", lan1)
-		nat2WAN := nat2.Attach("wan", inet)
-		nat2LAN := nat2.Attach("lan2", lan2)
-		m1if := m1.Attach("eth0", lan1)
-		m2if := m2.Attach("eth0", lan2)
-		lan1.SetDefaultGateway(nat1LAN)
-		lan2.SetDefaultGateway(nat2LAN)
-
-		nat1.PacketHandler = &natlab.SNAT44{
-			Machine:           nat1,
-			ExternalInterface: nat1WAN,
-			Firewall: &natlab.Firewall{
-				TrustedInterface: nat1LAN,
-			},
-		}
-		nat2.PacketHandler = &natlab.SNAT44{
-			Machine:           nat2,
-			ExternalInterface: nat2WAN,
-			Firewall: &natlab.Firewall{
-				TrustedInterface: nat2LAN,
-			},
-		}
-
-		n := &devices{
-			m1:     m1,
-			m1IP:   m1if.V4(),
-			m2:     m2,
-			m2IP:   m2if.V4(),
-			stun:   mstun,
-			stunIP: sif.V4(),
-		}
-		testActiveDiscovery(t, n)
-	})
-}
-
-func mustPrefix(s string) netaddr.IPPrefix {
-	pfx, err := netaddr.ParseIPPrefix(s)
-	if err != nil {
-		panic(err)
-	}
-	return pfx
 }
 
 type devices struct {
@@ -699,131 +505,6 @@ type devices struct {
 	stunIP netaddr.IP
 }
 
-// newPinger starts continuously sending test packets from srcM to
-// dstM, until cleanup is invoked to stop it. Each ping has 1 second
-// to transit the network. It is a test failure to lose a ping.
-func newPinger(t *testing.T, logf logger.Logf, src, dst *magicStack) (cleanup func()) {
-	ctx, cancel := context.WithCancel(context.Background())
-	done := make(chan struct{})
-	one := func() bool {
-		// TODO(danderson): requiring exactly zero packet loss
-		// will probably be too strict for some tests we'd like to
-		// run (e.g. discovery switching to a new path on
-		// failure). Figure out what kind of thing would be
-		// acceptable to test instead of "every ping must
-		// transit".
-		pkt := tuntest.Ping(dst.IP(t).IPAddr().IP, src.IP(t).IPAddr().IP)
-		select {
-		case src.tun.Outbound <- pkt:
-		case <-ctx.Done():
-			return false
-		}
-		select {
-		case <-dst.tun.Inbound:
-			return true
-		case <-time.After(10 * time.Second):
-			// Very generous timeout here because depending on
-			// magicsock setup races, the first handshake might get
-			// eaten by the receiving end (if wireguard-go hasn't been
-			// configured quite yet), so we have to wait for at least
-			// the first retransmit from wireguard before we declare
-			// failure.
-			t.Errorf("timed out waiting for ping to transit")
-			return true
-		case <-ctx.Done():
-			// Try a little bit longer to consume the packet we're
-			// waiting for. This is to deal with shutdown races, where
-			// natlab may still be delivering a packet to us from a
-			// goroutine.
-			select {
-			case <-dst.tun.Inbound:
-			case <-time.After(time.Second):
-			}
-			return false
-		}
-	}
-
-	cleanup = func() {
-		cancel()
-		<-done
-	}
-
-	// Synchronously transit one ping to get things started. This is
-	// nice because it means that newPinger returning means we've
-	// worked through initial connectivity.
-	if !one() {
-		cleanup()
-		return
-	}
-
-	go func() {
-		logf("sending ping stream from %s (%s) to %s (%s)", src, src.IP(t), dst, dst.IP(t))
-		defer close(done)
-		for one() {
-		}
-	}()
-
-	return cleanup
-}
-
-// testActiveDiscovery verifies that two magicStacks tied to the given
-// devices can establish a direct p2p connection with each other. See
-// TestActiveDiscovery for the various configurations of devices that
-// get exercised.
-func testActiveDiscovery(t *testing.T, d *devices) {
-	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
-
-	tlogf, setT := makeNestable(t)
-	setT(t)
-
-	start := time.Now()
-	logf := func(msg string, args ...interface{}) {
-		msg = fmt.Sprintf("%s: %s", time.Since(start), msg)
-		tlogf(msg, args...)
-	}
-
-	derpMap, cleanup := runDERPAndStun(t, logf, d.stun, d.stunIP)
-	defer cleanup()
-
-	m1 := newMagicStack(t, logger.WithPrefix(logf, "conn1: "), d.m1, derpMap)
-	defer m1.Close()
-	m2 := newMagicStack(t, logger.WithPrefix(logf, "conn2: "), d.m2, derpMap)
-	defer m2.Close()
-
-	cleanup = meshStacks(logf, []*magicStack{m1, m2})
-	defer cleanup()
-
-	m1IP := m1.IP(t)
-	m2IP := m2.IP(t)
-	logf("IPs: %s %s", m1IP, m2IP)
-
-	cleanup = newPinger(t, logf, m1, m2)
-	defer cleanup()
-
-	// Everything is now up and running, active discovery should find
-	// a direct path between our peers. Wait for it to switch away
-	// from DERP.
-
-	mustDirect := func(m1, m2 *magicStack) {
-		for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) {
-			pst := m1.Status().Peer[m2.Public()]
-			if pst.CurAddr != "" {
-				logf("direct link %s->%s found with addr %s", m1, m2, pst.CurAddr)
-				return
-			}
-			logf("no direct path %s->%s yet, addrs %v", m1, m2, pst.Addrs)
-		}
-		t.Errorf("magicsock did not find a direct path from %s to %s", m1, m2)
-	}
-
-	mustDirect(m1, m2)
-	mustDirect(m2, m1)
-
-	logf("starting cleanup")
-}
-
 func testTwoDevicePing(t *testing.T, d *devices) {
 	tstest.PanicOnLog()
 	rc := tstest.NewResourceCheck()

wgengine/packet/ip.go

@@ -47,13 +47,12 @@ const (
 	// Unknown represents an unknown or unsupported protocol; it's deliberately the zero value.
 	Unknown IPProto = 0x00
 	ICMP    IPProto = 0x01
-	IGMP    IPProto = 0x02
-	ICMPv6  IPProto = 0x3a
 	TCP     IPProto = 0x06
 	UDP     IPProto = 0x11
-	// Fragment is a special value. It's not really an IPProto value
-	// so we're using the unassigned 0xFF value.
+	// IPv6 and Fragment are special values. They're not really IPProto values
+	// so we're using the unassigned 0xFE and 0xFF values for them.
 	// TODO(dmytro): special values should be taken out of here.
+	IPv6     IPProto = 0xFE
 	Fragment IPProto = 0xFF
 )
 
@@ -67,6 +66,8 @@ func (p IPProto) String() string {
 		return "UDP"
 	case TCP:
 		return "TCP"
+	case IPv6:
+		return "IPv6"
 	default:
 		return "Unknown"
 	}

wgengine/packet/packet.go

@@ -30,8 +30,6 @@ var (
 )
 
 // ParsedPacket is a minimal decoding of a packet suitable for use in filters.
-//
-// In general, it only supports IPv4. The IPv6 parsing is very minimal.
 type ParsedPacket struct {
 	// b is the byte buffer that this decodes.
 	b []byte
@@ -43,32 +41,27 @@ type ParsedPacket struct {
 	// This is not the same as len(b) because b can have trailing zeros.
 	length int
 
-	IPVersion uint8   // 4, 6, or 0
-	IPProto   IPProto // IP subprotocol (UDP, TCP, etc); the NextHeader field for IPv6
-	SrcIP     IP      // IP source address (not used for IPv6)
-	DstIP     IP      // IP destination address (not used for IPv6)
-	SrcPort   uint16  // TCP/UDP source port
-	DstPort   uint16  // TCP/UDP destination port
-	TCPFlags  uint8   // TCP flags (SYN, ACK, etc)
+	IPProto  IPProto // IP subprotocol (UDP, TCP, etc)
+	SrcIP    IP      // IP source address
+	DstIP    IP      // IP destination address
+	SrcPort  uint16  // TCP/UDP source port
+	DstPort  uint16  // TCP/UDP destination port
+	TCPFlags uint8   // TCP flags (SYN, ACK, etc)
 }
 
-// NextHeader
-type NextHeader uint8
-
-func (p *ParsedPacket) String() string {
-	if p.IPVersion == 6 {
+func (q *ParsedPacket) String() string {
+	switch q.IPProto {
+	case IPv6:
 		return "IPv6{???}"
-	}
-	switch p.IPProto {
 	case Unknown:
 		return "Unknown{???}"
 	}
 	sb := strbuilder.Get()
-	sb.WriteString(p.IPProto.String())
+	sb.WriteString(q.IPProto.String())
 	sb.WriteByte('{')
-	writeIPPort(sb, p.SrcIP, p.SrcPort)
+	writeIPPort(sb, q.SrcIP, q.SrcPort)
 	sb.WriteString(" > ")
-	writeIPPort(sb, p.DstIP, p.DstPort)
+	writeIPPort(sb, q.DstIP, q.DstPort)
 	sb.WriteByte('}')
 	return sb.String()
 }
@@ -112,22 +105,20 @@ func (q *ParsedPacket) Decode(b []byte) {
 	q.b = b
 
 	if len(b) < ipHeaderLength {
-		q.IPVersion = 0
 		q.IPProto = Unknown
 		return
 	}
 
 	// Check that it's IPv4.
 	// TODO(apenwarr): consider IPv6 support
-	q.IPVersion = (b[0] & 0xF0) >> 4
-	switch q.IPVersion {
+	switch (b[0] & 0xF0) >> 4 {
 	case 4:
 		q.IPProto = IPProto(b[9])
+		// continue
 	case 6:
-		q.IPProto = IPProto(b[6]) // "Next Header" field
+		q.IPProto = IPv6
 		return
 	default:
-		q.IPVersion = 0
 		q.IPProto = Unknown
 		return
 	}

wgengine/packet/packet_test.go

@@ -47,12 +47,11 @@ var icmpRequestDecode = ParsedPacket{
 	dataofs: 24,
 	length:  len(icmpRequestBuffer),
 
-	IPVersion: 4,
-	IPProto:   ICMP,
-	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort:   0,
-	DstPort:   0,
+	IPProto: ICMP,
+	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort: 0,
+	DstPort: 0,
 }
 
 var icmpReplyBuffer = []byte{
@@ -73,12 +72,11 @@ var icmpReplyDecode = ParsedPacket{
 	dataofs: 24,
 	length:  len(icmpReplyBuffer),
 
-	IPVersion: 4,
-	IPProto:   ICMP,
-	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort:   0,
-	DstPort:   0,
+	IPProto: ICMP,
+	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort: 0,
+	DstPort: 0,
 }
 
 // IPv6 Router Solicitation
@@ -92,9 +90,8 @@ var ipv6PacketBuffer = []byte{
 }
 
 var ipv6PacketDecode = ParsedPacket{
-	b:         ipv6PacketBuffer,
-	IPVersion: 6,
-	IPProto:   ICMPv6,
+	b:       ipv6PacketBuffer,
+	IPProto: IPv6,
 }
 
 // This is a malformed IPv4 packet.
@@ -104,9 +101,8 @@ var unknownPacketBuffer = []byte{
 }
 
 var unknownPacketDecode = ParsedPacket{
-	b:         unknownPacketBuffer,
-	IPVersion: 0,
-	IPProto:   Unknown,
+	b:       unknownPacketBuffer,
+	IPProto: Unknown,
 }
 
 var tcpPacketBuffer = []byte{
@@ -129,13 +125,12 @@ var tcpPacketDecode = ParsedPacket{
 	dataofs: 40,
 	length:  len(tcpPacketBuffer),
 
-	IPVersion: 4,
-	IPProto:   TCP,
-	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort:   123,
-	DstPort:   567,
-	TCPFlags:  TCPSynAck,
+	IPProto:  TCP,
+	SrcIP:    NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:    NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort:  123,
+	DstPort:  567,
+	TCPFlags: TCPSynAck,
 }
 
 var udpRequestBuffer = []byte{
@@ -157,12 +152,11 @@ var udpRequestDecode = ParsedPacket{
 	dataofs: 28,
 	length:  len(udpRequestBuffer),
 
-	IPVersion: 4,
-	IPProto:   UDP,
-	SrcIP:     NewIP(net.ParseIP("1.2.3.4")),
-	DstIP:     NewIP(net.ParseIP("5.6.7.8")),
-	SrcPort:   123,
-	DstPort:   567,
+	IPProto: UDP,
+	SrcIP:   NewIP(net.ParseIP("1.2.3.4")),
+	DstIP:   NewIP(net.ParseIP("5.6.7.8")),
+	SrcPort: 123,
+	DstPort: 567,
 }
 
 var udpReplyBuffer = []byte{
@@ -240,7 +234,7 @@ func TestDecode(t *testing.T) {
 			var got ParsedPacket
 			got.Decode(tt.buf)
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("mismatch\n got: %#v\nwant: %#v", got, tt.want)
+				t.Errorf("got %v; want %v", got, tt.want)
 			}
 		})
 	}

wgengine/router/dns_direct.go

@@ -13,8 +13,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"os/exec"
-	"runtime"
 	"strings"
 
 	"inet.af/netaddr"
@@ -81,25 +79,6 @@ func dnsReadConfig() (DNSConfig, error) {
 	return config, nil
 }
 
-// isResolvedRunning reports whether systemd-resolved is running on the system,
-// even if it is not managing the system DNS settings.
-func isResolvedRunning() bool {
-	if runtime.GOOS != "linux" {
-		return false
-	}
-
-	// systemd-resolved is never installed without systemd.
-	_, err := exec.LookPath("systemctl")
-	if err != nil {
-		return false
-	}
-
-	// is-active exits with code 3 if the service is not active.
-	err = exec.Command("systemctl", "is-active", "systemd-resolved.service").Run()
-
-	return err == nil
-}
-
 // dnsDirectUp replaces /etc/resolv.conf with a file generated
 // from the given configuration, creating a backup of its old state.
 //
@@ -145,10 +124,6 @@ func dnsDirectUp(config DNSConfig) error {
 		return err
 	}
 
-	if isResolvedRunning() {
-		exec.Command("systemctl", "restart", "systemd-resolved.service").Run() // Best-effort.
-	}
-
 	return nil
 }
 
@@ -172,10 +147,5 @@ func dnsDirectDown() error {
 		return err
 	}
 	os.Remove(tsConf)
-
-	if isResolvedRunning() {
-		exec.Command("systemctl", "restart", "systemd-resolved.service").Run() // Best-effort.
-	}
-
 	return nil
 }

wgengine/router/router_userspace_bsd.go

@@ -153,10 +153,7 @@ func (r *userspaceBSDRouter) Set(cfg *Config) error {
 }
 
 func (r *userspaceBSDRouter) Close() error {
-	if err := downDNS(r.tunname); err != nil {
-		r.logf("dns down: %v", err)
-	}
-	// No interface cleanup is necessary during normal shutdown.
+	cleanup(r.logf, r.tunname)
 	return nil
 }
 
@@ -164,12 +161,9 @@ func cleanup(logf logger.Logf, interfaceName string) {
 	if err := downDNS(interfaceName); err != nil {
 		logf("dns down: %v", err)
 	}
-	// If the interface was left behind, ifconfig down will not remove it.
-	// In fact, this will leave a system in a tainted state where starting tailscaled
-	// will result in "interface tailscale0 already exists"
-	// until the defunct interface is ifconfig-destroyed.
-	ifup := []string{"ifconfig", interfaceName, "destroy"}
+
+	ifup := []string{"ifconfig", interfaceName, "down"}
 	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
-		logf("ifconfig destroy: %v\n%s", err, out)
+		logf("ifconfig down: %v\n%s", err, out)
 	}
 }

wgengine/tsdns/map.go

@@ -1,118 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package tsdns
-
-import (
-	"fmt"
-	"sort"
-	"strings"
-
-	"inet.af/netaddr"
-)
-
-// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
-type Map struct {
-	// nameToIP is a mapping of Tailscale domain names to their IP addresses.
-	// For example, monitoring.tailscale.us -> 100.64.0.1.
-	nameToIP map[string]netaddr.IP
-	// names are the keys of nameToIP in sorted order.
-	names []string
-}
-
-// NewMap returns a new Map with name to address mapping given by nameToIP.
-func NewMap(nameToIP map[string]netaddr.IP) *Map {
-	names := make([]string, 0, len(nameToIP))
-	for name := range nameToIP {
-		names = append(names, name)
-	}
-	sort.Strings(names)
-
-	return &Map{
-		nameToIP: nameToIP,
-		names:    names,
-	}
-}
-
-func printSingleNameIP(buf *strings.Builder, name string, ip netaddr.IP) {
-	// Output width is exactly 80 columns.
-	fmt.Fprintf(buf, "%s\t%s\n", name, ip)
-}
-
-func (m *Map) Pretty() string {
-	buf := new(strings.Builder)
-	for _, name := range m.names {
-		printSingleNameIP(buf, name, m.nameToIP[name])
-	}
-	return buf.String()
-}
-
-func (m *Map) PrettyDiffFrom(old *Map) string {
-	var (
-		oldNameToIP map[string]netaddr.IP
-		newNameToIP map[string]netaddr.IP
-		oldNames    []string
-		newNames    []string
-	)
-	if old != nil {
-		oldNameToIP = old.nameToIP
-		oldNames = old.names
-	}
-	if m != nil {
-		newNameToIP = m.nameToIP
-		newNames = m.names
-	}
-
-	buf := new(strings.Builder)
-
-	for len(oldNames) > 0 && len(newNames) > 0 {
-		var name string
-
-		newName, oldName := newNames[0], oldNames[0]
-		switch {
-		case oldName < newName:
-			name = oldName
-			oldNames = oldNames[1:]
-		case oldName > newName:
-			name = newName
-			newNames = newNames[1:]
-		case oldNames[0] == newNames[0]:
-			name = oldNames[0]
-			oldNames = oldNames[1:]
-			newNames = newNames[1:]
-		}
-
-		ipOld, inOld := oldNameToIP[name]
-		ipNew, inNew := newNameToIP[name]
-		switch {
-		case !inOld:
-			buf.WriteByte('+')
-			printSingleNameIP(buf, name, ipNew)
-		case !inNew:
-			buf.WriteByte('-')
-			printSingleNameIP(buf, name, ipOld)
-		case ipOld != ipNew:
-			buf.WriteByte('-')
-			printSingleNameIP(buf, name, ipOld)
-			buf.WriteByte('+')
-			printSingleNameIP(buf, name, ipNew)
-		}
-	}
-
-	for _, name := range oldNames {
-		if _, ok := newNameToIP[name]; !ok {
-			buf.WriteByte('-')
-			printSingleNameIP(buf, name, oldNameToIP[name])
-		}
-	}
-
-	for _, name := range newNames {
-		if _, ok := oldNameToIP[name]; !ok {
-			buf.WriteByte('+')
-			printSingleNameIP(buf, name, newNameToIP[name])
-		}
-	}
-
-	return buf.String()
-}

wgengine/tsdns/map_test.go

@@ -1,138 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package tsdns
-
-import (
-	"testing"
-
-	"inet.af/netaddr"
-)
-
-func TestPretty(t *testing.T) {
-	tests := []struct {
-		name string
-		dmap *Map
-		want string
-	}{
-		{"empty", NewMap(nil), ""},
-		{
-			"single",
-			NewMap(map[string]netaddr.IP{
-				"hello.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-			}),
-			"hello.ipn.dev\t100.101.102.103\n",
-		},
-		{
-			"multiple",
-			NewMap(map[string]netaddr.IP{
-				"test1.domain":     netaddr.IPv4(100, 101, 102, 103),
-				"test2.sub.domain": netaddr.IPv4(100, 99, 9, 1),
-			}),
-			"test1.domain\t100.101.102.103\ntest2.sub.domain\t100.99.9.1\n",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := tt.dmap.Pretty()
-			if tt.want != got {
-				t.Errorf("want %v; got %v", tt.want, got)
-			}
-		})
-	}
-}
-
-func TestPrettyDiffFrom(t *testing.T) {
-	tests := []struct {
-		name string
-		map1 *Map
-		map2 *Map
-		want string
-	}{
-		{
-			"from_empty",
-			nil,
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			"+test1.ipn.dev\t100.101.102.103\n+test2.ipn.dev\t100.103.102.101\n",
-		},
-		{
-			"equal",
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			NewMap(map[string]netaddr.IP{
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-			}),
-			"",
-		},
-		{
-			"changed_ip",
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			NewMap(map[string]netaddr.IP{
-				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-			}),
-			"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n",
-		},
-		{
-			"new_domain",
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			NewMap(map[string]netaddr.IP{
-				"test3.ipn.dev": netaddr.IPv4(100, 105, 106, 107),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-			}),
-			"+test3.ipn.dev\t100.105.106.107\n",
-		},
-		{
-			"gone_domain",
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-			}),
-			"-test2.ipn.dev\t100.103.102.101\n",
-		},
-		{
-			"mixed",
-			NewMap(map[string]netaddr.IP{
-				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
-				"test4.ipn.dev": netaddr.IPv4(100, 107, 106, 105),
-				"test5.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
-				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
-			}),
-			NewMap(map[string]netaddr.IP{
-				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
-				"test1.ipn.dev": netaddr.IPv4(100, 100, 101, 102),
-				"test3.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
-			}),
-			"-test1.ipn.dev\t100.101.102.103\n+test1.ipn.dev\t100.100.101.102\n" +
-				"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n" +
-				"+test3.ipn.dev\t100.64.1.1\n-test4.ipn.dev\t100.107.106.105\n-test5.ipn.dev\t100.64.1.1\n",
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := tt.map2.PrettyDiffFrom(tt.map1)
-			if tt.want != got {
-				t.Errorf("want %v; got %v", tt.want, got)
-			}
-		})
-	}
-}

wgengine/tsdns/tsdns.go

@@ -45,6 +45,18 @@ var (
 	errNotQuery       = errors.New("not a DNS query")
 )
 
+// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
+type Map struct {
+	// domainToIP is a mapping of Tailscale domains to their IP addresses.
+	// For example, monitoring.tailscale.us -> 100.64.0.1.
+	domainToIP map[string]netaddr.IP
+}
+
+// NewMap returns a new Map with domain to address mapping given by domainToIP.
+func NewMap(domainToIP map[string]netaddr.IP) *Map {
+	return &Map{domainToIP: domainToIP}
+}
+
 // Packet represents a DNS payload together with the address of its origin.
 type Packet struct {
 	// Payload is the application layer DNS payload.
@@ -130,10 +142,8 @@ func (r *Resolver) Close() {
 // SetMap sets the resolver's DNS map, taking ownership of it.
 func (r *Resolver) SetMap(m *Map) {
 	r.mu.Lock()
-	oldMap := r.dnsMap
 	r.dnsMap = m
 	r.mu.Unlock()
-	r.logf("map diff:\n%s", m.PrettyDiffFrom(oldMap))
 }
 
 // SetUpstreamNameservers sets the addresses of the resolver's
@@ -179,7 +189,7 @@ func (r *Resolver) Resolve(domain string) (netaddr.IP, dns.RCode, error) {
 		r.mu.RUnlock()
 		return netaddr.IP{}, dns.RCodeServerFailure, errMapNotSet
 	}
-	addr, found := r.dnsMap.nameToIP[domain]
+	addr, found := r.dnsMap.domainToIP[domain]
 	r.mu.RUnlock()
 
 	if !found {

wgengine/tsdns/tsdns_test.go

@@ -23,7 +23,7 @@ var testipv6 = netaddr.IPv6Raw([16]byte{
 })
 
 var dnsMap = &Map{
-	nameToIP: map[string]netaddr.IP{
+	domainToIP: map[string]netaddr.IP{
 		"test1.ipn.dev": testipv4,
 		"test2.ipn.dev": testipv6,
 	},

wgengine/userspace.go

@@ -16,7 +16,6 @@ import (
 	"os"
 	"os/exec"
 	"runtime"
-	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -34,7 +33,6 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
-	"tailscale.com/version"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/magicsock"
 	"tailscale.com/wgengine/monitor"
@@ -562,27 +560,13 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	p.run(ctx, peerKey, ips, srcIP)
 }
 
-var debugTrimWireguard, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_TRIM_WIREGUARD"))
-
-// forceFullWireguardConfig reports whether we should give wireguard
-// our full network map, even for inactive peers
-//
-// TODO(bradfitz): remove this after our 1.0 launch; we don't want to
-// enable wireguard config trimming quite yet because it just landed
-// and we haven't got enough time testing it.
-func forceFullWireguardConfig(numPeers int) bool {
-	// Did the user explicitly enable trimmming via the environment variable knob?
-	if debugTrimWireguard {
-		return false
+func updateSig(last *string, v interface{}) (changed bool) {
+	sig := deepprint.Hash(v)
+	if *last != sig {
+		*last = sig
+		return true
 	}
-	// On iOS with large networks, it's critical, so turn on trimming.
-	// Otherwise we run out of memory from wireguard-go goroutine stacks+buffers.
-	// This will be the default later for all platforms and network sizes.
-	iOS := runtime.GOOS == "darwin" && version.IsMobile()
-	if iOS && numPeers > 50 {
-		return false
-	}
-	return true
+	return false
 }
 
 // isTrimmablePeer reports whether p is a peer that we can trim out of the
@@ -594,10 +578,7 @@ func forceFullWireguardConfig(numPeers int) bool {
 // simplicity, have only one IP address (an IPv4 /32), which is the
 // common case for most peers. Subnet router nodes will just always be
 // created in the wireguard-go config.
-func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
-	if forceFullWireguardConfig(numPeers) {
-		return false
-	}
+func isTrimmablePeer(p *wgcfg.Peer) bool {
 	if len(p.AllowedIPs) != 1 || len(p.Endpoints) != 1 {
 		return false
 	}
@@ -699,7 +680,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 
 	for i := range full.Peers {
 		p := &full.Peers[i]
-		if !isTrimmablePeer(p, len(full.Peers)) {
+		if !isTrimmablePeer(p) {
 			min.Peers = append(min.Peers, *p)
 			continue
 		}
@@ -712,7 +693,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 		}
 	}
 
-	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min) {
+	if !updateSig(&e.lastEngineSigTrim, min) {
 		// No changes
 		return nil
 	}
@@ -814,8 +795,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
 	}
 
-	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
-	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
+	engineChanged := updateSig(&e.lastEngineSigFull, cfg)
+	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}