use net.JoinHostPort

Signed-off-by: Naman Sood <mail@nsood.in>

cmd/tailscale/cli/cli_test.go

@@ -40,7 +40,6 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 		name     string
 		flagSet  map[string]bool
 		curPrefs *ipn.Prefs
-		curUser  string // os.Getenv("USER") on the client side
 		mp       *ipn.MaskedPrefs
 		want     string
 	}{
@@ -79,7 +78,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 				WantRunningSet: true,
 				CorpDNSSet:     true,
 			},
-			want: accidentalUpPrefix + " --accept-dns --hostname=foo",
+			want: `'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --hostname is not specified but its default value of "" differs from current value "foo"`,
 		},
 		{
 			name:    "hostname_changing_explicitly",
@@ -149,213 +148,14 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			},
 			want: "",
 		},
-		{
-			name:    "implicit_operator_change",
-			flagSet: f("hostname"),
-			curPrefs: &ipn.Prefs{
-				ControlURL:   ipn.DefaultControlURL,
-				OperatorUser: "alice",
-			},
-			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-				},
-				ControlURLSet: true,
-			},
-			want: accidentalUpPrefix + " --hostname= --operator=alice",
-		},
-		{
-			name:    "implicit_operator_matches_shell_user",
-			flagSet: f("hostname"),
-			curPrefs: &ipn.Prefs{
-				ControlURL:   ipn.DefaultControlURL,
-				OperatorUser: "alice",
-			},
-			curUser: "alice",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-				},
-				ControlURLSet: true,
-			},
-			want: "",
-		},
-		{
-			name:    "error_advertised_routes_exit_node_removed",
-			flagSet: f("advertise-routes"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					netaddr.MustParseIPPrefix("::/0"),
-				},
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
-			want: accidentalUpPrefix + " --advertise-routes=10.0.42.0/24 --advertise-exit-node",
-		},
-		{
-			name:    "advertised_routes_exit_node_removed",
-			flagSet: f("advertise-routes", "advertise-exit-node"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					netaddr.MustParseIPPrefix("::/0"),
-				},
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
-			want: "",
-		},
-		{
-			name:    "advertised_routes_includes_the_0_routes", // but no --advertise-exit-node
-			flagSet: f("advertise-routes"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					netaddr.MustParseIPPrefix("::/0"),
-				},
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("11.1.43.0/24"),
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-						netaddr.MustParseIPPrefix("::/0"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
-			want: "",
-		},
-		{
-			name:    "advertised_routes_includes_only_one_0_route", // and no --advertise-exit-node
-			flagSet: f("advertise-routes"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.42.0/24"),
-					netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					netaddr.MustParseIPPrefix("::/0"),
-				},
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					AdvertiseRoutes: []netaddr.IPPrefix{
-						netaddr.MustParseIPPrefix("11.1.43.0/24"),
-						netaddr.MustParseIPPrefix("0.0.0.0/0"),
-					},
-				},
-				AdvertiseRoutesSet: true,
-			},
-			want: accidentalUpPrefix + " --advertise-routes=11.1.43.0/24,0.0.0.0/0 --advertise-exit-node",
-		},
-		{
-			name:    "exit_node_clearing", // Issue 1777
-			flagSet: f("exit-node"),
-			curPrefs: &ipn.Prefs{
-				ControlURL: ipn.DefaultControlURL,
-				ExitNodeID: "fooID",
-			},
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL: ipn.DefaultControlURL,
-					ExitNodeIP: netaddr.IP{},
-				},
-				ExitNodeIPSet: true,
-			},
-			want: "",
-		},
-		{
-			name:    "remove_all_implicit",
-			flagSet: f("force-reauth"),
-			curPrefs: &ipn.Prefs{
-				WantRunning:      true,
-				ControlURL:       ipn.DefaultControlURL,
-				RouteAll:         true,
-				AllowSingleHosts: false,
-				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
-				CorpDNS:          true,
-				ShieldsUp:        true,
-				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
-				Hostname:         "myhostname",
-				ForceDaemon:      true,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.0.0/16"),
-				},
-				NetfilterMode: preftype.NetfilterNoDivert,
-				OperatorUser:  "alice",
-			},
-			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-				},
-			},
-			want: accidentalUpPrefix + " --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --hostname=myhostname --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
-		},
-		{
-			name:    "remove_all_implicit_except_hostname",
-			flagSet: f("hostname"),
-			curPrefs: &ipn.Prefs{
-				WantRunning:      true,
-				ControlURL:       ipn.DefaultControlURL,
-				RouteAll:         true,
-				AllowSingleHosts: false,
-				ExitNodeIP:       netaddr.MustParseIP("100.64.5.6"),
-				CorpDNS:          true,
-				ShieldsUp:        true,
-				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
-				Hostname:         "myhostname",
-				ForceDaemon:      true,
-				AdvertiseRoutes: []netaddr.IPPrefix{
-					netaddr.MustParseIPPrefix("10.0.0.0/16"),
-				},
-				NetfilterMode: preftype.NetfilterNoDivert,
-				OperatorUser:  "alice",
-			},
-			curUser: "eve",
-			mp: &ipn.MaskedPrefs{
-				Prefs: ipn.Prefs{
-					ControlURL:  ipn.DefaultControlURL,
-					WantRunning: true,
-					Hostname:    "newhostname",
-				},
-				HostnameSet: true,
-			},
-			want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			var got string
-			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, tt.curUser); err != nil {
+			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp); err != nil {
 				got = err.Error()
 			}
-			if strings.TrimSpace(got) != tt.want {
+			if got != tt.want {
 				t.Errorf("unexpected result\n got: %s\nwant: %s\n", got, tt.want)
 			}
 		})

cmd/tailscale/cli/push.go

@@ -16,7 +16,6 @@ import (
 	"net/http"
 	"net/url"
 	"os"
-	"path/filepath"
 	"strconv"
 	"time"
 	"unicode/utf8"
@@ -62,13 +61,12 @@ func runPush(ctx context.Context, args []string) error {
 		return err
 	}
 
-	peerAPIBase, lastSeen, isOffline, err := discoverPeerAPIBase(ctx, ip)
+	peerAPIBase, lastSeen, err := discoverPeerAPIBase(ctx, ip)
 	if err != nil {
 		return err
 	}
-	if isOffline {
-		fmt.Fprintf(os.Stderr, "# warning: %s is offline\n", hostOrIP)
-	} else if !lastSeen.IsZero() && time.Since(lastSeen) > lastSeenOld {
+
+	if !lastSeen.IsZero() && time.Since(lastSeen) > lastSeenOld {
 		fmt.Fprintf(os.Stderr, "# warning: %s last seen %v ago\n", hostOrIP, time.Since(lastSeen).Round(time.Minute))
 	}
 
@@ -99,7 +97,7 @@ func runPush(ctx context.Context, args []string) error {
 		contentLength = fi.Size()
 		fileContents = io.LimitReader(f, contentLength)
 		if name == "" {
-			name = filepath.Base(fileArg)
+			name = fileArg
 		}
 
 		if slow, _ := strconv.ParseBool(os.Getenv("TS_DEBUG_SLOW_PUSH")); slow {
@@ -128,14 +126,14 @@ func runPush(ctx context.Context, args []string) error {
 	return errors.New(res.Status)
 }
 
-func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSeen time.Time, isOffline bool, err error) {
+func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSeen time.Time, err error) {
 	ip, err := netaddr.ParseIP(ipStr)
 	if err != nil {
-		return "", time.Time{}, false, err
+		return "", time.Time{}, err
 	}
 	fts, err := tailscale.FileTargets(ctx)
 	if err != nil {
-		return "", time.Time{}, false, err
+		return "", time.Time{}, err
 	}
 	for _, ft := range fts {
 		n := ft.Node
@@ -146,11 +144,10 @@ func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSe
 			if n.LastSeen != nil {
 				lastSeen = *n.LastSeen
 			}
-			isOffline = n.Online != nil && !*n.Online
-			return ft.PeerAPIURL, lastSeen, isOffline, nil
+			return ft.PeerAPIURL, lastSeen, nil
 		}
 	}
-	return "", time.Time{}, false, errors.New("target seems to be running an old Tailscale version")
+	return "", time.Time{}, errors.New("target seems to be running an old Tailscale version")
 }
 
 const maxSniff = 4 << 20
@@ -207,22 +204,15 @@ func runPushTargets(ctx context.Context, args []string) error {
 	}
 	for _, ft := range fts {
 		n := ft.Node
-		var detail string
-		if n.Online != nil {
-			if !*n.Online {
-				detail = "offline"
-			}
+		var ago string
+		if n.LastSeen == nil {
+			ago = "\tnode never seen"
 		} else {
-			detail = "unknown-status"
+			if d := time.Since(*n.LastSeen); d > lastSeenOld {
+				ago = fmt.Sprintf("\tlast seen %v ago", d.Round(time.Minute))
+			}
 		}
-		if detail != "" && n.LastSeen != nil {
-			d := time.Since(*n.LastSeen)
-			detail += fmt.Sprintf("; last seen %v ago", d.Round(time.Minute))
-		}
-		if detail != "" {
-			detail = "\t" + detail
-		}
-		fmt.Printf("%s\t%s%s\n", n.Addresses[0].IP, n.ComputedName, detail)
+		fmt.Printf("%s\t%s%s\n", n.Addresses[0].IP, n.ComputedName, ago)
 	}
 	return nil
 }

cmd/tailscale/cli/up.go

@@ -17,7 +17,7 @@ import (
 	"strings"
 	"sync"
 
-	shellquote "github.com/kballard/go-shellquote"
+	"github.com/go-multierror/multierror"
 	"github.com/peterbourgon/ff/v2/ffcli"
 	"inet.af/netaddr"
 	"tailscale.com/client/tailscale"
@@ -245,23 +245,6 @@ func runUp(ctx context.Context, args []string) error {
 	if err != nil {
 		fatalf("can't fetch status from tailscaled: %v", err)
 	}
-	origAuthURL := st.AuthURL
-
-	// printAuthURL reports whether we should print out the
-	// provided auth URL from an IPN notify.
-	printAuthURL := func(url string) bool {
-		if upArgs.authKey != "" {
-			// Issue 1755: when using an authkey, don't
-			// show an authURL that might still be pending
-			// from a previous non-completed interactive
-			// login.
-			return false
-		}
-		if upArgs.forceReauth && url == origAuthURL {
-			return false
-		}
-		return true
-	}
 
 	if distro.Get() == distro.Synology {
 		notSupported := "not yet supported on Synology; see https://github.com/tailscale/tailscale/issues/451"
@@ -302,7 +285,7 @@ func runUp(ctx context.Context, args []string) error {
 	})
 
 	if !upArgs.reset {
-		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, os.Getenv("USER")); err != nil {
+		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp); err != nil {
 			fatalf("%s", err)
 		}
 	}
@@ -329,10 +312,7 @@ func runUp(ctx context.Context, args []string) error {
 	// simpleUp is whether we're running a simple "tailscale up"
 	// to transition to running from a previously-logged-in but
 	// down state, without changing any settings.
-	simpleUp := len(flagSet) == 0 &&
-		curPrefs.Persist != nil &&
-		curPrefs.Persist.LoginName != "" &&
-		st.BackendState != ipn.NeedsLogin.String()
+	simpleUp := len(flagSet) == 0 && curPrefs.Persist != nil && curPrefs.Persist.LoginName != ""
 
 	// At this point we need to subscribe to the IPN bus to watch
 	// for state transitions and possible need to authenticate.
@@ -387,7 +367,7 @@ func runUp(ctx context.Context, args []string) error {
 				cancel()
 			}
 		}
-		if url := n.BrowseToURL; url != nil && printAuthURL(*url) {
+		if url := n.BrowseToURL; url != nil {
 			printed = true
 			fmt.Fprintf(os.Stderr, "\nTo authenticate, visit:\n\n\t%s\n\n", *url)
 		}
@@ -438,9 +418,7 @@ func runUp(ctx context.Context, args []string) error {
 		}
 
 		bc.Start(opts)
-		if upArgs.forceReauth {
-			startLoginInteractive()
-		}
+		startLoginInteractive()
 	}
 
 	select {
@@ -472,7 +450,7 @@ func init() {
 	addPrefFlagMapping("netfilter-mode", "NetfilterMode")
 	addPrefFlagMapping("shields-up", "ShieldsUp")
 	addPrefFlagMapping("snat-subnet-routes", "NoSNAT")
-	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeID")
+	addPrefFlagMapping("exit-node", "ExitNodeIP", "ExitNodeIP")
 	addPrefFlagMapping("exit-node-allow-lan-access", "ExitNodeAllowLANAccess")
 	addPrefFlagMapping("unattended", "ForceDaemon")
 	addPrefFlagMapping("operator", "OperatorUser")
@@ -508,12 +486,6 @@ func updateMaskedPrefsFromUpFlag(mp *ipn.MaskedPrefs, flagName string) {
 	}
 }
 
-const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires mentioning all\n" +
-	"non-default flags. To proceed, either re-run your command with --reset or\n" +
-	"specify use the command below to explicitly mention the current value of\n" +
-	"all non-default settings:\n\n" +
-	"\ttailscale up"
-
 // checkForAccidentalSettingReverts checks for people running
 // "tailscale up" with a subset of the flags they originally ran it
 // with.
@@ -528,7 +500,7 @@ const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires
 //
 // mp is the mask of settings actually set, where mp.Prefs is the new
 // preferences to set, including any values set from implicit flags.
-func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs, curUser string) error {
+func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs) error {
 	if len(flagSet) == 0 {
 		// A bare "tailscale up" is a special case to just
 		// mean bringing the network up without any changes.
@@ -547,29 +519,15 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 	ev := reflect.ValueOf(curWithExplicitEdits).Elem()
 	// Implicit values (what we'd get if we replaced everything with flag defaults):
 	iv := reflect.ValueOf(&mp.Prefs).Elem()
-
-	var missing []string
-	flagExplicitValue := map[string]interface{}{} // e.g. "accept-dns" => true (from flagSet)
+	var errs []error
+	var didExitNodeErr bool
 	for i := 0; i < prefType.NumField(); i++ {
 		prefName := prefType.Field(i).Name
 		if prefName == "Persist" {
 			continue
 		}
 		flagName, hasFlag := flagForPref[prefName]
-
-		// Special case for advertise-exit-node; which is a
-		// flag but doesn't have a corresponding pref.  The
-		// flag augments advertise-routes, so we have to infer
-		// the imaginary pref's current value from the routes.
-		if prefName == "AdvertiseRoutes" &&
-			hasExitNodeRoutes(curPrefs.AdvertiseRoutes) &&
-			!hasExitNodeRoutes(curWithExplicitEdits.AdvertiseRoutes) &&
-			!flagSet["advertise-exit-node"] {
-			missing = append(missing, "--advertise-exit-node")
-		}
-
 		if hasFlag && flagSet[flagName] {
-			flagExplicitValue[flagName] = ev.Field(i).Interface()
 			continue
 		}
 		// Get explicit value and implicit value
@@ -582,93 +540,33 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 			}
 		}
 		exi, imi := ex.Interface(), im.Interface()
-
 		if reflect.DeepEqual(exi, imi) {
 			continue
 		}
-		if flagName == "operator" && imi == "" && exi == curUser {
-			// Don't require setting operator if the current user matches
-			// the configured operator.
-			continue
-		}
 		switch flagName {
 		case "":
-			return fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; this command would change the value of flagless pref %q", prefName)
+			errs = append(errs, fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; this command would change the value of flagless pref %q", prefName))
 		case "exit-node":
-			if prefName == "ExitNodeIP" {
-				missing = append(missing, fmtFlagValueArg("exit-node", fmtSettingVal(exi)))
+			if !didExitNodeErr {
+				didExitNodeErr = true
+				errs = append(errs, errors.New("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --exit-node is not specified but an exit node is currently configured"))
 			}
 		default:
-			missing = append(missing, fmtFlagValueArg(flagName, fmtSettingVal(exi)))
+			errs = append(errs, fmt.Errorf("'tailscale up' without --reset requires all preferences with changing values to be explicitly mentioned; --%s is not specified but its default value of %v differs from current value %v",
+				flagName, fmtSettingVal(imi), fmtSettingVal(exi)))
 		}
 	}
-	if len(missing) == 0 {
-		return nil
-	}
-	var sb strings.Builder
-	sb.WriteString(accidentalUpPrefix)
-
-	var flagSetSorted []string
-	for f := range flagSet {
-		flagSetSorted = append(flagSetSorted, f)
-	}
-	sort.Strings(flagSetSorted)
-	for _, flagName := range flagSetSorted {
-		if ev, ok := flagExplicitValue[flagName]; ok {
-			fmt.Fprintf(&sb, " %s", fmtFlagValueArg(flagName, fmtSettingVal(ev)))
-		}
-	}
-	for _, a := range missing {
-		fmt.Fprintf(&sb, " %s", a)
-	}
-	sb.WriteString("\n\n")
-	return errors.New(sb.String())
-}
-
-func fmtFlagValueArg(flagName, val string) string {
-	if val == "true" {
-		// TODO: check flagName's type to see if its Pref is of type bool
-		return "--" + flagName
-	}
-	if val == "" {
-		return "--" + flagName + "="
-	}
-	return fmt.Sprintf("--%s=%v", flagName, shellquote.Join(val))
+	return multierror.New(errs)
 }
 
 func fmtSettingVal(v interface{}) string {
 	switch v := v.(type) {
 	case bool:
 		return strconv.FormatBool(v)
-	case string:
-		return v
-	case preftype.NetfilterMode:
-		return v.String()
+	case string, preftype.NetfilterMode:
+		return fmt.Sprintf("%q", v)
 	case []string:
 		return strings.Join(v, ",")
-	case []netaddr.IPPrefix:
-		var sb strings.Builder
-		for i, r := range v {
-			if i > 0 {
-				sb.WriteByte(',')
-			}
-			sb.WriteString(r.String())
-		}
-		return sb.String()
 	}
 	return fmt.Sprint(v)
 }
-
-func hasExitNodeRoutes(rr []netaddr.IPPrefix) bool {
-	var v4, v6 bool
-	for _, r := range rr {
-		if r.Bits == 0 {
-			if r.IP.Is4() {
-				v4 = true
-			} else if r.IP.Is6() {
-				v6 = true
-			}
-		}
-	}
-	return v4 && v6
-}

cmd/tailscale/depaware.txt

@@ -2,7 +2,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
 
    W 💣 github.com/alexbrainman/sspi                                 from github.com/alexbrainman/sspi/negotiate
    W 💣 github.com/alexbrainman/sspi/negotiate                       from tailscale.com/net/tshttpproxy
-        github.com/kballard/go-shellquote                            from tailscale.com/cmd/tailscale/cli
+        github.com/go-multierror/multierror                          from tailscale.com/cmd/tailscale/cli
         github.com/peterbourgon/ff/v2                                from github.com/peterbourgon/ff/v2/ffcli
         github.com/peterbourgon/ff/v2/ffcli                          from tailscale.com/cmd/tailscale/cli
         github.com/tcnksm/go-httpstat                                from tailscale.com/net/netcheck

cmd/tailscaled/depaware.txt

@@ -131,11 +131,9 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/strbuilder                               from tailscale.com/net/packet
         tailscale.com/types/structs                                  from tailscale.com/control/controlclient+
         tailscale.com/types/wgkey                                    from tailscale.com/control/controlclient+
-   L    tailscale.com/util/cmpver                                    from tailscale.com/net/dns
         tailscale.com/util/dnsname                                   from tailscale.com/ipn/ipnstate+
   LW    tailscale.com/util/endian                                    from tailscale.com/net/netns+
    L    tailscale.com/util/lineread                                  from tailscale.com/control/controlclient+
-        tailscale.com/util/osshare                                   from tailscale.com/cmd/tailscaled+
         tailscale.com/util/pidowner                                  from tailscale.com/ipn/ipnserver
         tailscale.com/util/racebuild                                 from tailscale.com/logpolicy
         tailscale.com/util/systemd                                   from tailscale.com/control/controlclient+

cmd/tailscaled/install_windows.go

@@ -16,7 +16,6 @@ import (
 	"golang.org/x/sys/windows/svc/mgr"
 	"tailscale.com/logtail/backoff"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/osshare"
 )
 
 func init() {
@@ -80,9 +79,6 @@ func installSystemDaemonWindows(args []string) (err error) {
 }
 
 func uninstallSystemDaemonWindows(args []string) (ret error) {
-	// Remove file sharing from Windows shell (noop in non-windows)
-	osshare.SetFileSharingEnabled(false, logger.Discard)
-
 	m, err := mgr.Connect()
 	if err != nil {
 		return fmt.Errorf("failed to connect to Windows service manager: %v", err)

cmd/tailscaled/tailscaled.go

@@ -40,7 +40,6 @@ import (
 	"tailscale.com/types/flagtype"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
-	"tailscale.com/util/osshare"
 	"tailscale.com/version"
 	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
@@ -161,12 +160,7 @@ func main() {
 		log.Fatalf("--socket is required")
 	}
 
-	err := run()
-
-	// Remove file sharing from Windows shell (noop in non-windows)
-	osshare.SetFileSharingEnabled(false, logger.Discard)
-
-	if err != nil {
+	if err := run(); err != nil {
 		// No need to log; the func already did
 		os.Exit(1)
 	}

control/controlclient/auto.go

@@ -597,9 +597,6 @@ func (c *Client) mapRoutine() {
 }
 
 func (c *Client) AuthCantContinue() bool {
-	if c == nil {
-		return true
-	}
 	c.mu.Lock()
 	defer c.mu.Unlock()
 

control/controlclient/direct.go

@@ -413,7 +413,7 @@ func (c *Direct) doLogin(ctx context.Context, opt loginOpt) (mustRegen bool, new
 
 		// Don't log the common error types. Signatures are not usually enabled,
 		// so these are expected.
-		if !errors.Is(err, errCertificateNotConfigured) && !errors.Is(err, errNoCertStore) {
+		if err != errCertificateNotConfigured && err != errNoCertStore {
 			c.logf("RegisterReq sign error: %v", err)
 		}
 	}
@@ -570,11 +570,6 @@ func (c *Direct) SendLiteMapUpdate(ctx context.Context) error {
 	return c.sendMapRequest(ctx, 1, nil)
 }
 
-// If we go more than pollTimeout without hearing from the server,
-// end the long poll. We should be receiving a keep alive ping
-// every minute.
-const pollTimeout = 120 * time.Second
-
 // cb nil means to omit peers.
 func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netmap.NetworkMap)) error {
 	c.mu.Lock()
@@ -699,6 +694,10 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 		return nil
 	}
 
+	// If we go more than pollTimeout without hearing from the server,
+	// end the long poll. We should be receiving a keep alive ping
+	// every minute.
+	const pollTimeout = 120 * time.Second
 	timeout := time.NewTimer(pollTimeout)
 	timeoutReset := make(chan struct{})
 	pollDone := make(chan struct{})
@@ -796,11 +795,6 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*netm
 			}
 			setControlAtomic(&controlUseDERPRoute, resp.Debug.DERPRoute)
 			setControlAtomic(&controlTrimWGConfig, resp.Debug.TrimWGConfig)
-			if sleep := time.Duration(resp.Debug.SleepSeconds * float64(time.Second)); sleep > 0 {
-				if err := sleepAsRequested(ctx, c.logf, timeoutReset, sleep); err != nil {
-					return err
-				}
-			}
 		}
 
 		nm := sess.netmapForResponse(&resp)
@@ -1187,34 +1181,3 @@ func answerPing(logf logger.Logf, c *http.Client, pr *tailcfg.PingRequest) {
 		logf("answerPing complete to %v (after %v)", pr.URL, d)
 	}
 }
-
-func sleepAsRequested(ctx context.Context, logf logger.Logf, timeoutReset chan<- struct{}, d time.Duration) error {
-	const maxSleep = 5 * time.Minute
-	if d > maxSleep {
-		logf("sleeping for %v, capped from server-requested %v ...", maxSleep, d)
-		d = maxSleep
-	} else {
-		logf("sleeping for server-requested %v ...", d)
-	}
-
-	ticker := time.NewTicker(pollTimeout / 2)
-	defer ticker.Stop()
-	timer := time.NewTimer(d)
-	defer timer.Stop()
-	for {
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		case <-timer.C:
-			return nil
-		case <-ticker.C:
-			select {
-			case timeoutReset <- struct{}{}:
-			case <-timer.C:
-				return nil
-			case <-ctx.Done():
-				return ctx.Err()
-			}
-		}
-	}
-}

derp/derpmap/derpmap.go

@@ -83,9 +83,6 @@ func Prod() *tailcfg.DERPMap {
 			10: derpRegion(10, "sea", "Seattle",
 				derpNode("a", "137.220.36.168", "2001:19f0:8001:2d9:5400:2ff:feef:bbb1"),
 			),
-			11: derpRegion(11, "sao", "São Paulo",
-				derpNode("a", "18.230.97.74", "2600:1f1e:ee4:5611:ec5c:1736:d43b:a454"),
-			),
 		},
 	}
 }

go.mod

@@ -15,7 +15,6 @@ require (
 	github.com/google/go-cmp v0.5.4
 	github.com/goreleaser/nfpm v1.1.10
 	github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b
-	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
 	github.com/klauspost/compress v1.10.10
 	github.com/kr/pty v1.1.8
 	github.com/mdlayher/netlink v1.3.2

go.sum

@@ -61,8 +61,6 @@ github.com/jsimonetti/rtnetlink v0.0.0-20201220180245-69540ac93943/go.mod h1:z4c
 github.com/jsimonetti/rtnetlink v0.0.0-20210122163228-8d122574c736/go.mod h1:ZXpIyOK59ZnN7J0BV99cZUPmsqDRZ3eq5X+st7u/oSA=
 github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b h1:c3NTyLNozICy8B4mlMXemD3z/gXgQzVXZS/HqT+i3do=
 github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b/go.mod h1:8w9Rh8m+aHZIG69YPGGem1i5VzoyRC8nw2kA8B+ik5U=
-github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
-github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.10.10 h1:a/y8CglcM7gLGYmlbP/stPE5sR3hbhFRUjCBfd/0B3I=
 github.com/klauspost/compress v1.10.10/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=

health/health.go

@@ -36,10 +36,6 @@ var (
 	ipnState                string
 	ipnWantRunning          bool
 	anyInterfaceUp          = true // until told otherwise
-
-	ReceiveIPv4 = ReceiveFuncState{name: "IPv4"}
-	ReceiveIPv6 = ReceiveFuncState{name: "IPv6"}
-	ReceiveDERP = ReceiveFuncState{name: "DERP"}
 )
 
 // Subsystem is the name of a subsystem whose health can be monitored.
@@ -217,68 +213,6 @@ func SetAnyInterfaceUp(up bool) {
 	selfCheckLocked()
 }
 
-// ReceiveFuncState tracks the state of a wireguard-go conn.ReceiveFunc.
-type ReceiveFuncState struct {
-	// name is a mnemonic for the receive func, used in error messages.
-	name string
-	// started indicates whether magicsock.connBind.Open
-	// has requested that wireguard-go start its receive func
-	// goroutine (without a corresponding connBind.Close).
-	started bool
-	// running models whether wireguard-go's receive func
-	// goroutine is actually running. We cannot easily introspect that,
-	// so it is based on our knowledge of wireguard-go's internals.
-	running bool
-}
-
-// err returns the error state (if any) that s represents.
-func (s ReceiveFuncState) err() error {
-	// Possible states:
-	//   | started | running | notes
-	//   | ------- | ------- | -----
-	//   | true    | true    | normal operation
-	//   | true    | false   | we prematurely returned a permanent error from this receive func
-	//   | false   | true    | we have told package health that we're closing the bind, but the receive funcs haven't closed yet (transient)
-	//   | false   | false   | not running
-
-	// The problematic case is started && !running.
-	// If that happens, wireguard-go will no longer request packets,
-	// and we'll lose an entire communication channel.
-	if s.started && !s.running {
-		return fmt.Errorf("receive%s started but not running", s.name)
-	}
-	return nil
-}
-
-// Open tells r that connBind.Open has requested wireguard-go open a conn.Bind that includes r.
-func (r *ReceiveFuncState) Open() {
-	mu.Lock()
-	defer mu.Unlock()
-	r.started = true
-	r.running = true
-	selfCheckLocked()
-}
-
-// Stop tells r that we have returned a permanent error to wireguard-go.
-// wireguard-go's receive func goroutine for r will soon stop.
-func (r *ReceiveFuncState) Stop() {
-	mu.Lock()
-	defer mu.Unlock()
-	r.running = false
-	selfCheckLocked()
-}
-
-// Close tells r that connBind.Close has requested wireguard-go close the bind for r.
-// This will stop the corresponding receive func goroutine.
-// Close must be called before actually closing the underlying connection,
-// to avoid a small window of false positives.
-func (r *ReceiveFuncState) Close() {
-	mu.Lock()
-	defer mu.Unlock()
-	r.started = false
-	selfCheckLocked()
-}
-
 func timerSelfCheck() {
 	mu.Lock()
 	defer mu.Unlock()
@@ -329,11 +263,6 @@ func overallErrorLocked() error {
 	_ = lastMapRequestHeard
 
 	var errs []error
-	for _, recv := range []ReceiveFuncState{ReceiveIPv4, ReceiveIPv6, ReceiveDERP} {
-		if err := recv.err(); err != nil {
-			errs = append(errs, err)
-		}
-	}
 	for sys, err := range sysErr {
 		if err == nil || sys == SysOverall {
 			continue

ipn/backend.go

@@ -55,21 +55,17 @@ type EngineStatus struct {
 // that they have not changed.
 // They are JSON-encoded on the wire, despite the lack of struct tags.
 type Notify struct {
-	_       structs.Incomparable
-	Version string // version number of IPN backend
-
-	// ErrMessage, if non-nil, contains a critical error message.
-	// For State InUseOtherUser, ErrMessage is not critical and just contains the details.
-	ErrMessage *string
-
-	LoginFinished *empty.Message       // non-nil when/if the login process succeeded
-	State         *State               // if non-nil, the new or current IPN state
-	Prefs         *Prefs               // if non-nil, the new or current preferences
-	NetMap        *netmap.NetworkMap   // if non-nil, the new or current netmap
-	Engine        *EngineStatus        // if non-nil, the new or urrent wireguard stats
-	BrowseToURL   *string              // if non-nil, UI should open a browser right now
-	BackendLogID  *string              // if non-nil, the public logtail ID used by backend
-	PingResult    *ipnstate.PingResult // if non-nil, a ping response arrived
+	_             structs.Incomparable
+	Version       string             // version number of IPN backend
+	ErrMessage    *string            // critical error message, if any; for InUseOtherUser, the details
+	LoginFinished *empty.Message     // event: non-nil when login process succeeded
+	State         *State             // current IPN state has changed
+	Prefs         *Prefs             // preferences were changed
+	NetMap        *netmap.NetworkMap // new netmap received
+	Engine        *EngineStatus      // wireguard engine stats
+	BrowseToURL   *string            // UI should open a browser right now
+	BackendLogID  *string            // public logtail id used by backend
+	PingResult    *ipnstate.PingResult
 
 	// FilesWaiting if non-nil means that files are buffered in
 	// the Tailscale daemon and ready for local transfer to the

ipn/ipnlocal/local.go

@@ -46,7 +46,6 @@ import (
 	"tailscale.com/types/persist"
 	"tailscale.com/types/wgkey"
 	"tailscale.com/util/dnsname"
-	"tailscale.com/util/osshare"
 	"tailscale.com/util/systemd"
 	"tailscale.com/version"
 	"tailscale.com/wgengine"
@@ -106,7 +105,6 @@ type LocalBackend struct {
 	inServerMode   bool
 	machinePrivKey wgkey.Private
 	state          ipn.State
-	capFileSharing bool // whether netMap contains the file sharing capability
 	// hostinfo is mutated in-place while mu is held.
 	hostinfo *tailcfg.Hostinfo
 	// netMap is not mutated in-place once set.
@@ -116,8 +114,7 @@ type LocalBackend struct {
 	engineStatus     ipn.EngineStatus
 	endpoints        []tailcfg.Endpoint
 	blocked          bool
-	authURL          string // cleared on Notify
-	authURLSticky    string // not cleared on Notify
+	authURL          string
 	interact         bool
 	prevIfState      *interfaces.State
 	peerAPIServer    *peerAPIServer // or nil
@@ -147,8 +144,6 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, e wge
 		panic("ipn.NewLocalBackend: wgengine must not be nil")
 	}
 
-	osshare.SetFileSharingEnabled(false, logf)
-
 	// Default filter blocks everything and logs nothing, until Start() is called.
 	e.SetFilter(filter.NewAllowNone(logf, &netaddr.IPSet{}))
 
@@ -315,7 +310,7 @@ func (b *LocalBackend) updateStatus(sb *ipnstate.StatusBuilder, extraLocked func
 	sb.MutateStatus(func(s *ipnstate.Status) {
 		s.Version = version.Long
 		s.BackendState = b.state.String()
-		s.AuthURL = b.authURLSticky
+		s.AuthURL = b.authURL
 		if b.netMap != nil {
 			s.MagicDNSSuffix = b.netMap.MagicDNSSuffix()
 		}
@@ -465,7 +460,6 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 	}
 	if st.URL != "" {
 		b.authURL = st.URL
-		b.authURLSticky = st.URL
 	}
 	if b.state == ipn.NeedsLogin {
 		if !b.prefs.WantRunning {
@@ -594,24 +588,6 @@ func (b *LocalBackend) SetHTTPTestClient(c *http.Client) {
 	b.httpTestClient = c
 }
 
-// startIsNoopLocked reports whether a Start call on this LocalBackend
-// with the provided Start Options would be a useless no-op.
-//
-// b.mu must be held.
-func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool {
-	// Options has 4 fields; check all of them:
-	//   * FrontendLogID
-	//   * StateKey
-	//   * Prefs
-	//   * AuthKey
-	return b.state == ipn.Running &&
-		b.hostinfo != nil &&
-		b.hostinfo.FrontendLogID == opts.FrontendLogID &&
-		b.stateKey == opts.StateKey &&
-		opts.Prefs == nil &&
-		opts.AuthKey == ""
-}
-
 // Start applies the configuration specified in opts, and starts the
 // state machine.
 //
@@ -627,37 +603,18 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 		return errors.New("no state key or prefs provided")
 	}
 
-	defer b.stateMachine()
 	if opts.Prefs != nil {
 		b.logf("Start: %v", opts.Prefs.Pretty())
 	} else {
 		b.logf("Start")
 	}
 
-	b.mu.Lock()
-
-	// The iOS client sends a "Start" whenever its UI screen comes
-	// up, just because it wants a netmap. That should be fixed,
-	// but meanwhile we can make Start cheaper here for such a
-	// case and not restart the world (which takes a few seconds).
-	// Instead, just send a notify with the state that iOS needs.
-	if b.startIsNoopLocked(opts) {
-		b.logf("Start: already running; sending notify")
-		nm := b.netMap
-		state := b.state
-		b.mu.Unlock()
-		b.send(ipn.Notify{
-			State:         &state,
-			NetMap:        nm,
-			LoginFinished: new(empty.Message),
-		})
-		return nil
-	}
-
 	hostinfo := controlclient.NewHostinfo()
 	hostinfo.BackendLogID = b.backendLogID
 	hostinfo.FrontendLogID = opts.FrontendLogID
 
+	b.mu.Lock()
+
 	if b.cc != nil {
 		// TODO(apenwarr): avoid the need to reinit controlclient.
 		// This will trigger a full relogin/reconfigure cycle every
@@ -1073,7 +1030,7 @@ func (b *LocalBackend) popBrowserAuthNow() {
 	b.mu.Lock()
 	url := b.authURL
 	b.interact = false
-	b.authURL = "" // but NOT clearing authURLSticky
+	b.authURL = ""
 	b.mu.Unlock()
 
 	b.logf("popBrowserAuthNow: url=%v", url != "")
@@ -1690,21 +1647,15 @@ func (b *LocalBackend) authReconfig() {
 		switch {
 		case len(dcfg.DefaultResolvers) != 0:
 			// Default resolvers already set.
-		case !uc.ExitNodeID.IsZero():
-			// When using exit nodes, it's very likely the LAN
-			// resolvers will become unreachable. So, force use of the
-			// fallback resolvers until we implement DNS forwarding to
-			// exit nodes.
-			//
-			// This is especially important on Apple OSes, where
-			// adding the default route to the tunnel interface makes
-			// it "primary", and we MUST provide VPN-sourced DNS
-			// settings or we break all DNS resolution.
-			//
-			// https://github.com/tailscale/tailscale/issues/1713
-			addDefault(nm.DNS.FallbackResolvers)
 		case len(dcfg.Routes) == 0 && len(dcfg.Hosts) == 0 && len(dcfg.AuthoritativeSuffixes) == 0:
 			// No settings requiring split DNS, no problem.
+		case (version.OS() == "iOS" || version.OS() == "macOS") && !uc.ExitNodeID.IsZero():
+			// On Apple OSes, if your NetworkExtension provides a
+			// default route, underlying primary resolvers are
+			// automatically removed, so we MUST provide a set of
+			// resolvers capable of resolving the entire world.
+			// https://github.com/tailscale/tailscale/issues/1713
+			addDefault(nm.DNS.FallbackResolvers)
 		case version.OS() == "android":
 			// We don't support split DNS at all on Android yet.
 			addDefault(nm.DNS.FallbackResolvers)
@@ -1765,20 +1716,6 @@ func (b *LocalBackend) fileRootLocked(uid tailcfg.UserID) string {
 	return dir
 }
 
-// closePeerAPIListenersLocked closes any existing peer API listeners
-// and clears out the peer API server state.
-//
-// It does not kick off any Hostinfo update with new services.
-//
-// b.mu must be held.
-func (b *LocalBackend) closePeerAPIListenersLocked() {
-	b.peerAPIServer = nil
-	for _, pln := range b.peerAPIListeners {
-		pln.Close()
-	}
-	b.peerAPIListeners = nil
-}
-
 func (b *LocalBackend) initPeerAPIListener() {
 	b.mu.Lock()
 	defer b.mu.Unlock()
@@ -1797,7 +1734,11 @@ func (b *LocalBackend) initPeerAPIListener() {
 		}
 	}
 
-	b.closePeerAPIListenersLocked()
+	b.peerAPIServer = nil
+	for _, pln := range b.peerAPIListeners {
+		pln.Close()
+	}
+	b.peerAPIListeners = nil
 
 	selfNode := b.netMap.SelfNode
 	if len(b.netMap.Addresses) == 0 || selfNode == nil {
@@ -2016,27 +1957,20 @@ func applyPrefsToHostinfo(hi *tailcfg.Hostinfo, prefs *ipn.Prefs) {
 // happen".
 func (b *LocalBackend) enterState(newState ipn.State) {
 	b.mu.Lock()
-	oldState := b.state
+	state := b.state
 	b.state = newState
 	prefs := b.prefs
 	cc := b.cc
 	networkUp := b.prevIfState.AnyInterfaceUp()
 	activeLogin := b.activeLogin
 	authURL := b.authURL
-	if newState == ipn.Running {
-		b.authURL = ""
-		b.authURLSticky = ""
-	} else if oldState == ipn.Running {
-		// Transitioning away from running.
-		b.closePeerAPIListenersLocked()
-	}
 	b.mu.Unlock()
 
-	if oldState == newState {
+	if state == newState {
 		return
 	}
 	b.logf("Switching ipn state %v -> %v (WantRunning=%v)",
-		oldState, newState, prefs.WantRunning)
+		state, newState, prefs.WantRunning)
 	health.SetIPNState(newState.String(), prefs.WantRunning)
 	b.send(ipn.Notify{State: &newState})
 
@@ -2181,7 +2115,6 @@ func (b *LocalBackend) ResetForClientDisconnect() {
 	b.setNetMapLocked(nil)
 	b.prefs = new(ipn.Prefs)
 	b.authURL = ""
-	b.authURLSticky = ""
 	b.activeLogin = ""
 }
 
@@ -2260,17 +2193,6 @@ func (b *LocalBackend) setNetInfo(ni *tailcfg.NetInfo) {
 	cc.SetNetInfo(ni)
 }
 
-func hasCapability(nm *netmap.NetworkMap, cap string) bool {
-	if nm != nil && nm.SelfNode != nil {
-		for _, c := range nm.SelfNode.Capabilities {
-			if c == cap {
-				return true
-			}
-		}
-	}
-	return false
-}
-
 func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	var login string
 	if nm != nil {
@@ -2285,13 +2207,6 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 		b.activeLogin = login
 	}
 
-	// Determine if file sharing is enabled
-	fs := hasCapability(nm, tailcfg.CapabilityFileSharing)
-	if fs != b.capFileSharing {
-		osshare.SetFileSharingEnabled(fs, b.logf)
-	}
-	b.capFileSharing = fs
-
 	if nm == nil {
 		b.nodeByAddr = nil
 		return
@@ -2400,7 +2315,20 @@ func (b *LocalBackend) OpenFile(name string) (rc io.ReadCloser, size int64, err
 func (b *LocalBackend) hasCapFileSharing() bool {
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	return b.capFileSharing
+	return b.hasCapFileSharingLocked()
+}
+
+func (b *LocalBackend) hasCapFileSharingLocked() bool {
+	nm := b.netMap
+	if nm == nil || nm.SelfNode == nil {
+		return false
+	}
+	for _, c := range nm.SelfNode.Capabilities {
+		if c == tailcfg.CapabilityFileSharing {
+			return true
+		}
+	}
+	return false
 }
 
 // FileTargets lists nodes that the current node can send files to.
@@ -2409,7 +2337,7 @@ func (b *LocalBackend) FileTargets() ([]*apitype.FileTarget, error) {
 
 	b.mu.Lock()
 	defer b.mu.Unlock()
-	if !b.capFileSharing {
+	if !b.hasCapFileSharingLocked() {
 		return nil, errors.New("file sharing not enabled by Tailscale admin")
 	}
 	nm := b.netMap

ipn/ipnlocal/local_test.go

@@ -5,20 +5,14 @@
 package ipnlocal
 
 import (
-	"bytes"
-	"fmt"
-	"net/http"
 	"reflect"
-	"sync"
 	"testing"
 
 	"inet.af/netaddr"
-	"tailscale.com/ipn"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/netmap"
-	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/wgcfg"
 )
 
@@ -425,48 +419,3 @@ func TestPeerAPIBase(t *testing.T) {
 		})
 	}
 }
-
-type panicOnUseTransport struct{}
-
-func (panicOnUseTransport) RoundTrip(*http.Request) (*http.Response, error) {
-	panic("unexpected HTTP request")
-}
-
-var nl = []byte("\n")
-
-func TestStartsInNeedsLoginState(t *testing.T) {
-	var (
-		mu     sync.Mutex
-		logBuf bytes.Buffer
-	)
-	logf := func(format string, a ...interface{}) {
-		mu.Lock()
-		defer mu.Unlock()
-		fmt.Fprintf(&logBuf, format, a...)
-		if !bytes.HasSuffix(logBuf.Bytes(), nl) {
-			logBuf.Write(nl)
-		}
-	}
-	store := new(ipn.MemoryStore)
-	eng, err := wgengine.NewFakeUserspaceEngine(logf, 0)
-	if err != nil {
-		t.Fatalf("NewFakeUserspaceEngine: %v", err)
-	}
-	lb, err := NewLocalBackend(logf, "logid", store, eng)
-	if err != nil {
-		t.Fatalf("NewLocalBackend: %v", err)
-	}
-
-	lb.SetHTTPTestClient(&http.Client{
-		Transport: panicOnUseTransport{}, // validate we don't send HTTP requests
-	})
-
-	if err := lb.Start(ipn.Options{
-		StateKey: ipn.GlobalDaemonStateKey,
-	}); err != nil {
-		t.Fatalf("Start: %v", err)
-	}
-	if st := lb.State(); st != ipn.NeedsLogin {
-		t.Errorf("State = %v; want NeedsLogin", st)
-	}
-}

ipn/ipnlocal/peerapi.go

@@ -11,7 +11,6 @@ import (
 	"hash/crc32"
 	"html"
 	"io"
-	"io/fs"
 	"net"
 	"net/http"
 	"net/url"
@@ -19,7 +18,6 @@ import (
 	"path"
 	"path/filepath"
 	"runtime"
-	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -30,7 +28,6 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/ipn"
-	"tailscale.com/logtail/backoff"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/syncs"
 	"tailscale.com/tailcfg"
@@ -50,24 +47,10 @@ type peerAPIServer struct {
 	// download directory (as *.partial files), rather than making
 	// the frontend retrieve it over localapi HTTP and write it
 	// somewhere itself. This is used on GUI macOS version.
-	// In directFileMode, the peerapi doesn't do the final rename
-	// from "foo.jpg.partial" to "foo.jpg".
 	directFileMode bool
 }
 
-const (
-	// partialSuffix is the suffix appened to files while they're
-	// still in the process of being transferred.
-	partialSuffix = ".partial"
-
-	// deletedSuffix is the suffix for a deleted marker file
-	// that's placed next to a file (without the suffix) that we
-	// tried to delete, but Windows wouldn't let us. These are
-	// only written on Windows (and in tests), but they're not
-	// permitted to be uploaded directly on any platform, like
-	// partial files.
-	deletedSuffix = ".deleted"
-)
+const partialSuffix = ".partial"
 
 func validFilenameRune(r rune) bool {
 	switch r {
@@ -100,7 +83,6 @@ func (s *peerAPIServer) diskPath(baseName string) (fullPath string, ok bool) {
 	clean := path.Clean(baseName)
 	if clean != baseName ||
 		clean == "." || clean == ".." ||
-		strings.HasSuffix(clean, deletedSuffix) ||
 		strings.HasSuffix(clean, partialSuffix) {
 		return "", false
 	}
@@ -134,28 +116,11 @@ func (s *peerAPIServer) hasFilesWaiting() bool {
 	for {
 		des, err := f.ReadDir(10)
 		for _, de := range des {
-			name := de.Name()
-			if strings.HasSuffix(name, partialSuffix) {
-				continue
-			}
-			if strings.HasSuffix(name, deletedSuffix) { // for Windows + tests
-				// After we're done looping over files, then try
-				// to delete this file. Don't do it proactively,
-				// as the OS may return "foo.jpg.deleted" before "foo.jpg"
-				// and we don't want to delete the ".deleted" file before
-				// enumerating to the "foo.jpg" file.
-				defer tryDeleteAgain(filepath.Join(s.rootDir, strings.TrimSuffix(name, deletedSuffix)))
+			if strings.HasSuffix(de.Name(), partialSuffix) {
 				continue
 			}
 			if de.Type().IsRegular() {
-				_, err := os.Stat(filepath.Join(s.rootDir, name+deletedSuffix))
-				if os.IsNotExist(err) {
-					return true
-				}
-				if err == nil {
-					tryDeleteAgain(filepath.Join(s.rootDir, name))
-					continue
-				}
+				return true
 			}
 		}
 		if err == io.EOF {
@@ -168,12 +133,6 @@ func (s *peerAPIServer) hasFilesWaiting() bool {
 	return false
 }
 
-// WaitingFiles returns the list of files that have been sent by a
-// peer that are waiting in the buffered "pick up" directory owned by
-// the Tailscale daemon.
-//
-// As a side effect, it also does any lazy deletion of files as
-// required by Windows.
 func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 	if s.rootDir == "" {
 		return nil, errors.New("peerapi disabled; no storage configured")
@@ -186,7 +145,6 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 		return nil, err
 	}
 	defer f.Close()
-	var deleted map[string]bool // "foo.jpg" => true (if "foo.jpg.deleted" exists)
 	for {
 		des, err := f.ReadDir(10)
 		for _, de := range des {
@@ -194,13 +152,6 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 			if strings.HasSuffix(name, partialSuffix) {
 				continue
 			}
-			if strings.HasSuffix(name, deletedSuffix) { // for Windows + tests
-				if deleted == nil {
-					deleted = map[string]bool{}
-				}
-				deleted[strings.TrimSuffix(name, deletedSuffix)] = true
-				continue
-			}
 			if de.Type().IsRegular() {
 				fi, err := de.Info()
 				if err != nil {
@@ -219,41 +170,9 @@ func (s *peerAPIServer) WaitingFiles() (ret []apitype.WaitingFile, err error) {
 			return nil, err
 		}
 	}
-	if len(deleted) > 0 {
-		// Filter out any return values "foo.jpg" where a
-		// "foo.jpg.deleted" marker file exists on disk.
-		all := ret
-		ret = ret[:0]
-		for _, wf := range all {
-			if !deleted[wf.Name] {
-				ret = append(ret, wf)
-			}
-		}
-		// And do some opportunistic deleting while we're here.
-		// Maybe Windows is done virus scanning the file we tried
-		// to delete a long time ago and will let us delete it now.
-		for name := range deleted {
-			tryDeleteAgain(filepath.Join(s.rootDir, name))
-		}
-	}
-	sort.Slice(ret, func(i, j int) bool { return ret[i].Name < ret[j].Name })
 	return ret, nil
 }
 
-// tryDeleteAgain tries to delete path (and path+deletedSuffix) after
-// it failed earlier.  This happens on Windows when various anti-virus
-// tools hook into filesystem operations and have the file open still
-// while we're trying to delete it. In that case we instead mark it as
-// deleted (writing a "foo.jpg.deleted" marker file), but then we
-// later try to clean them up.
-//
-// fullPath is the full path to the file without the deleted suffix.
-func tryDeleteAgain(fullPath string) {
-	if err := os.Remove(fullPath); err == nil || os.IsNotExist(err) {
-		os.Remove(fullPath + deletedSuffix)
-	}
-}
-
 func (s *peerAPIServer) DeleteFile(baseName string) error {
 	if s.rootDir == "" {
 		return errors.New("peerapi disabled; no storage configured")
@@ -265,55 +184,11 @@ func (s *peerAPIServer) DeleteFile(baseName string) error {
 	if !ok {
 		return errors.New("bad filename")
 	}
-	var bo *backoff.Backoff
-	logf := s.b.logf
-	t0 := time.Now()
-	for {
-		err := os.Remove(path)
-		if err != nil && !os.IsNotExist(err) {
-			err = redactErr(err)
-			// Put a retry loop around deletes on Windows. Windows
-			// file descriptor closes are effectively asynchronous,
-			// as a bunch of hooks run on/after close, and we can't
-			// necessarily delete the file for a while after close,
-			// as we need to wait for everybody to be done with
-			// it. (on Windows, unlike Unix, a file can't be deleted
-			// if it's open anywhere)
-			// So try a few times but ultimately just leave a
-			// "foo.jpg.deleted" marker file to note that it's
-			// deleted and we clean it up later.
-			if runtime.GOOS == "windows" {
-				if bo == nil {
-					bo = backoff.NewBackoff("delete-retry", logf, 1*time.Second)
-				}
-				if time.Since(t0) < 5*time.Second {
-					bo.BackOff(context.Background(), err)
-					continue
-				}
-				if err := redactErr(touchFile(path + deletedSuffix)); err != nil {
-					logf("peerapi: failed to leave deleted marker: %v", err)
-				}
-			}
-			logf("peerapi: failed to DeleteFile: %v", err)
-			return err
-		}
-		return nil
-	}
-}
-
-func redactErr(err error) error {
-	if pe, ok := err.(*os.PathError); ok {
-		pe.Path = "redacted"
-	}
-	return err
-}
-
-func touchFile(path string) error {
-	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0666)
-	if err != nil {
+	err := os.Remove(path)
+	if err != nil && !os.IsNotExist(err) {
 		return err
 	}
-	return f.Close()
+	return nil
 }
 
 func (s *peerAPIServer) OpenFile(baseName string) (rc io.ReadCloser, size int64, err error) {
@@ -327,10 +202,6 @@ func (s *peerAPIServer) OpenFile(baseName string) (rc io.ReadCloser, size int64,
 	if !ok {
 		return nil, 0, errors.New("bad filename")
 	}
-	if fi, err := os.Stat(path + deletedSuffix); err == nil && fi.Mode().IsRegular() {
-		tryDeleteAgain(path)
-		return nil, 0, &fs.PathError{Op: "open", Path: path, Err: fs.ErrNotExist}
-	}
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, 0, err
@@ -496,10 +367,6 @@ func (h *peerAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		h.handlePeerPut(w, r)
 		return
 	}
-	if r.URL.Path == "/v0/goroutines" {
-		h.handleServeGoroutines(w, r)
-		return
-	}
 	who := h.peerUser.DisplayName
 	fmt.Fprintf(w, `<html>
 <meta name="viewport" content="width=device-width, initial-scale=1">
@@ -612,18 +479,19 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "bad filename", 400)
 		return
 	}
-	// TODO(bradfitz): prevent same filename being sent by two peers at once
-	partialFile := dstFile + partialSuffix
-	f, err := os.Create(partialFile)
+	if h.ps.directFileMode {
+		dstFile += partialSuffix
+	}
+	f, err := os.Create(dstFile)
 	if err != nil {
-		h.logf("put Create error: %v", redactErr(err))
+		h.logf("put Create error: %v", err)
 		http.Error(w, err.Error(), http.StatusInternalServerError)
 		return
 	}
 	var success bool
 	defer func() {
 		if !success {
-			os.Remove(partialFile)
+			os.Remove(dstFile)
 		}
 	}()
 	var finalSize int64
@@ -637,7 +505,7 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 			ph:      h,
 		}
 		if h.ps.directFileMode {
-			inFile.partialPath = partialFile
+			inFile.partialPath = dstFile
 		}
 		h.ps.b.registerIncomingFile(inFile, true)
 		defer h.ps.b.registerIncomingFile(inFile, false)
@@ -659,13 +527,6 @@ func (h *peerAPIHandler) handlePeerPut(w http.ResponseWriter, r *http.Request) {
 		if inFile != nil { // non-zero length; TODO: notify even for zero length
 			inFile.markAndNotifyDone()
 		}
-	} else {
-		if err := os.Rename(partialFile, dstFile); err != nil {
-			err = redactErr(err)
-			h.logf("put final rename: %v", err)
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-			return
-		}
 	}
 
 	h.logf("put of %s from %v/%v", approxSize(finalSize), h.remoteAddr.IP, h.peerNode.ComputedName)
@@ -685,21 +546,5 @@ func approxSize(n int64) string {
 	if n <= 1<<20 {
 		return "<=1MB"
 	}
-	return fmt.Sprintf("~%dMB", n>>20)
-}
-
-func (h *peerAPIHandler) handleServeGoroutines(w http.ResponseWriter, r *http.Request) {
-	if !h.isSelf {
-		http.Error(w, "not owner", http.StatusForbidden)
-		return
-	}
-	var buf []byte
-	for size := 4 << 10; size <= 2<<20; size *= 2 {
-		buf = make([]byte, size)
-		buf = buf[:runtime.Stack(buf, true)]
-		if len(buf) < size {
-			break
-		}
-	}
-	w.Write(buf)
+	return fmt.Sprintf("~%dMB", n/1<<20)
 }

ipn/ipnlocal/peerapi_test.go

@@ -10,16 +10,15 @@ import (
 	"io"
 	"io/fs"
 	"io/ioutil"
-	"math/rand"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"testing"
 
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/netmap"
 )
 
 type peerAPITestEnv struct {
@@ -103,7 +102,7 @@ func hexAll(v string) string {
 	return sb.String()
 }
 
-func TestHandlePeerAPI(t *testing.T) {
+func TestHandlePeerPut(t *testing.T) {
 	tests := []struct {
 		name       string
 		isSelf     bool // the peer sending the request is owned by us
@@ -134,21 +133,6 @@ func TestHandlePeerAPI(t *testing.T) {
 				bodyNotContains("You are the owner of this node."),
 			),
 		},
-		{
-			name:   "peer_api_goroutines_deny",
-			isSelf: false,
-			req:    httptest.NewRequest("GET", "/v0/goroutines", nil),
-			checks: checks(httpStatus(403)),
-		},
-		{
-			name:   "peer_api_goroutines",
-			isSelf: true,
-			req:    httptest.NewRequest("GET", "/v0/goroutines", nil),
-			checks: checks(
-				httpStatus(200),
-				bodyContains("ServeHTTP"),
-			),
-		},
 		{
 			name:       "reject_non_owner_put",
 			isSelf:     false,
@@ -236,16 +220,6 @@ func TestHandlePeerAPI(t *testing.T) {
 				bodyContains("bad filename"),
 			),
 		},
-		{
-			name:       "bad_filename_deleted",
-			isSelf:     true,
-			capSharing: true,
-			req:        httptest.NewRequest("PUT", "/v0/put/foo.deleted", nil),
-			checks: checks(
-				httpStatus(400),
-				bodyContains("bad filename"),
-			),
-		},
 		{
 			name:       "bad_filename_dot",
 			isSelf:     true,
@@ -401,10 +375,18 @@ func TestHandlePeerAPI(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			var caps []string
+			if tt.capSharing {
+				caps = append(caps, tailcfg.CapabilityFileSharing)
+			}
 			var e peerAPITestEnv
 			lb := &LocalBackend{
-				logf:           e.logf,
-				capFileSharing: tt.capSharing,
+				netMap: &netmap.NetworkMap{
+					SelfNode: &tailcfg.Node{
+						Capabilities: caps,
+					},
+				},
+				logf: e.logf,
 			}
 			e.ph = &peerAPIHandler{
 				isSelf: tt.isSelf,
@@ -440,134 +422,3 @@ func TestHandlePeerAPI(t *testing.T) {
 		})
 	}
 }
-
-// Windows likes to hold on to file descriptors for some indeterminate
-// amount of time after you close them and not let you delete them for
-// a bit. So test that we work around that sufficiently.
-func TestFileDeleteRace(t *testing.T) {
-	dir := t.TempDir()
-	ps := &peerAPIServer{
-		b: &LocalBackend{
-			logf:           t.Logf,
-			capFileSharing: true,
-		},
-		rootDir: dir,
-	}
-	ph := &peerAPIHandler{
-		isSelf: true,
-		peerNode: &tailcfg.Node{
-			ComputedName: "some-peer-name",
-		},
-		ps: ps,
-	}
-	buf := make([]byte, 2<<20)
-	for i := 0; i < 30; i++ {
-		rr := httptest.NewRecorder()
-		ph.ServeHTTP(rr, httptest.NewRequest("PUT", "/v0/put/foo.txt", bytes.NewReader(buf[:rand.Intn(len(buf))])))
-		if res := rr.Result(); res.StatusCode != 200 {
-			t.Fatal(res.Status)
-		}
-		wfs, err := ps.WaitingFiles()
-		if err != nil {
-			t.Fatal(err)
-		}
-		if len(wfs) != 1 {
-			t.Fatalf("waiting files = %d; want 1", len(wfs))
-		}
-
-		if err := ps.DeleteFile("foo.txt"); err != nil {
-			t.Fatal(err)
-		}
-		wfs, err = ps.WaitingFiles()
-		if err != nil {
-			t.Fatal(err)
-		}
-		if len(wfs) != 0 {
-			t.Fatalf("waiting files = %d; want 0", len(wfs))
-		}
-	}
-}
-
-// Tests "foo.jpg.deleted" marks (for Windows).
-func TestDeletedMarkers(t *testing.T) {
-	dir := t.TempDir()
-	ps := &peerAPIServer{
-		b: &LocalBackend{
-			logf:           t.Logf,
-			capFileSharing: true,
-		},
-		rootDir: dir,
-	}
-
-	nothingWaiting := func() {
-		t.Helper()
-		ps.knownEmpty.Set(false)
-		if ps.hasFilesWaiting() {
-			t.Fatal("unexpected files waiting")
-		}
-	}
-	touch := func(base string) {
-		t.Helper()
-		if err := touchFile(filepath.Join(dir, base)); err != nil {
-			t.Fatal(err)
-		}
-	}
-	wantEmptyTempDir := func() {
-		t.Helper()
-		if fis, err := ioutil.ReadDir(dir); err != nil {
-			t.Fatal(err)
-		} else if len(fis) > 0 && runtime.GOOS != "windows" {
-			for _, fi := range fis {
-				t.Errorf("unexpected file in tempdir: %q", fi.Name())
-			}
-		}
-	}
-
-	nothingWaiting()
-	wantEmptyTempDir()
-
-	touch("foo.jpg.deleted")
-	nothingWaiting()
-	wantEmptyTempDir()
-
-	touch("foo.jpg.deleted")
-	touch("foo.jpg")
-	nothingWaiting()
-	wantEmptyTempDir()
-
-	touch("foo.jpg.deleted")
-	touch("foo.jpg")
-	wf, err := ps.WaitingFiles()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(wf) != 0 {
-		t.Fatalf("WaitingFiles = %d; want 0", len(wf))
-	}
-	wantEmptyTempDir()
-
-	touch("foo.jpg.deleted")
-	touch("foo.jpg")
-	if rc, _, err := ps.OpenFile("foo.jpg"); err == nil {
-		rc.Close()
-		t.Fatal("unexpected foo.jpg open")
-	}
-	wantEmptyTempDir()
-
-	// And verify basics still work in non-deleted cases.
-	touch("foo.jpg")
-	touch("bar.jpg.deleted")
-	if wf, err := ps.WaitingFiles(); err != nil {
-		t.Error(err)
-	} else if len(wf) != 1 {
-		t.Errorf("WaitingFiles = %d; want 1", len(wf))
-	} else if wf[0].Name != "foo.jpg" {
-		t.Errorf("unexpected waiting file %+v", wf[0])
-	}
-	if rc, _, err := ps.OpenFile("foo.jpg"); err != nil {
-		t.Fatal(err)
-	} else {
-		rc.Close()
-	}
-
-}

ipn/message.go

@@ -93,27 +93,17 @@ type BackendServer struct {
 	GotQuit       bool                 // a Quit command was received
 }
 
-// NewBackendServer creates a new BackendServer using b.
-//
-// If sendNotifyMsg is non-nil, it additionally sets the Backend's
-// notification callback to call the func with ipn.Notify messages in
-// JSON form. If nil, it does not change the notification callback.
 func NewBackendServer(logf logger.Logf, b Backend, sendNotifyMsg func(b []byte)) *BackendServer {
 	bs := &BackendServer{
 		logf:          logf,
 		b:             b,
 		sendNotifyMsg: sendNotifyMsg,
 	}
-	if sendNotifyMsg != nil {
-		b.SetNotifyCallback(bs.send)
-	}
+	b.SetNotifyCallback(bs.send)
 	return bs
 }
 
 func (bs *BackendServer) send(n Notify) {
-	if bs.sendNotifyMsg == nil {
-		return
-	}
 	n.Version = version.Long
 	b, err := json.Marshal(n)
 	if err != nil {

ipn/message_test.go

@@ -87,8 +87,6 @@ func TestClientServer(t *testing.T) {
 		t.Logf("c: "+fmt, args...)
 	}
 	bs = NewBackendServer(slogf, b, serverToClient)
-	// Verify that this doesn't break bs's callback:
-	NewBackendServer(slogf, b, nil)
 	bc = NewBackendClient(clogf, clientToServer)
 
 	ch := make(chan Notify, 256)

net/dns/manager_linux.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/godbus/dbus/v5"
 	"tailscale.com/types/logger"
-	"tailscale.com/util/cmpver"
 )
 
 type kv struct {
@@ -65,54 +64,18 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 			return newResolvedManager(logf)
 		}
 		dbg("nm-resolved", "yes")
-
-		// Version of NetworkManager before 1.26.6 programmed resolved
-		// incorrectly, such that NM's settings would always take
-		// precedence over other settings set by other resolved
-		// clients.
-		//
-		// If we're dealing with such a version, we have to set our
-		// DNS settings through NM to have them take.
-		//
-		// However, versions 1.26.6 later both fixed the resolved
-		// programming issue _and_ started ignoring DNS settings for
-		// "unmanaged" interfaces - meaning NM 1.26.6 and later
-		// actively ignore DNS configuration we give it. So, for those
-		// NM versions, we can and must use resolved directly.
-		old, err := nmVersionOlderThan("1.26.6")
-		if err != nil {
-			// Failed to figure out NM's version, can't make a correct
-			// decision.
-			return nil, fmt.Errorf("checking NetworkManager version: %v", err)
-		}
-		if old {
-			dbg("nm-old", "yes")
-			return newNMManager(interfaceName)
-		}
-		dbg("nm-old", "no")
-		return newResolvedManager(logf)
+		return newNMManager(interfaceName)
 	case "resolvconf":
 		dbg("rc", "resolvconf")
 		if err := resolvconfSourceIsNM(bs); err == nil {
 			dbg("src-is-nm", "yes")
 			if err := dbusPing("org.freedesktop.NetworkManager", "/org/freedesktop/NetworkManager/DnsManager"); err == nil {
 				dbg("nm", "yes")
-				old, err := nmVersionOlderThan("1.26.6")
-				if err != nil {
-					return nil, fmt.Errorf("checking NetworkManager version: %v", err)
-				}
-				if old {
-					dbg("nm-old", "yes")
-					return newNMManager(interfaceName)
-				} else {
-					dbg("nm-old", "no")
-				}
-			} else {
-				dbg("nm", "no")
+				return newNMManager(interfaceName)
 			}
-		} else {
-			dbg("src-is-nm", "no")
+			dbg("nm", "no")
 		}
+		dbg("src-is-nm", "no")
 		if _, err := exec.LookPath("resolvconf"); err != nil {
 			dbg("resolvconf", "no")
 			return newDirectManager()
@@ -126,16 +89,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 			return newDirectManager()
 		}
 		dbg("nm", "yes")
-		old, err := nmVersionOlderThan("1.26.6")
-		if err != nil {
-			return nil, fmt.Errorf("checking NetworkManager version: %v", err)
-		}
-		if old {
-			dbg("nm-old", "yes")
-			return newNMManager(interfaceName)
-		}
-		dbg("nm-old", "no")
-		return newDirectManager()
+		return newNMManager(interfaceName)
 	default:
 		dbg("rc", "unknown")
 		return newDirectManager()
@@ -181,27 +135,6 @@ func resolvconfSourceIsNM(resolvDotConf []byte) error {
 	return nil
 }
 
-func nmVersionOlderThan(want string) (bool, error) {
-	conn, err := dbus.SystemBus()
-	if err != nil {
-		// DBus probably not running.
-		return false, err
-	}
-
-	nm := conn.Object("org.freedesktop.NetworkManager", dbus.ObjectPath("/org/freedesktop/NetworkManager"))
-	v, err := nm.GetProperty("org.freedesktop.NetworkManager.Version")
-	if err != nil {
-		return false, err
-	}
-
-	version, ok := v.Value().(string)
-	if !ok {
-		return false, fmt.Errorf("unexpected type %T for NM version", v.Value())
-	}
-
-	return cmpver.Compare(version, want) < 0, nil
-}
-
 func nmIsUsingResolved() error {
 	conn, err := dbus.SystemBus()
 	if err != nil {

net/dns/resolver/forwarder.go

@@ -13,6 +13,7 @@ import (
 	"hash/crc32"
 	"math/rand"
 	"net"
+	"os"
 	"sync"
 	"time"
 
@@ -38,6 +39,8 @@ const (
 
 var errNoUpstreams = errors.New("upstream nameservers not set")
 
+var aLongTimeAgo = time.Unix(0, 1)
+
 type forwardingRecord struct {
 	src       netaddr.IPPort
 	createdAt time.Time
@@ -300,6 +303,8 @@ type fwdConn struct {
 	// logf allows a fwdConn to log.
 	logf logger.Logf
 
+	// wg tracks the number of outstanding conn.Read and conn.Write calls.
+	wg sync.WaitGroup
 	// change allows calls to read to block until a the network connection has been replaced.
 	change *sync.Cond
 
@@ -347,12 +352,15 @@ func (c *fwdConn) send(packet []byte, dst netaddr.IPPort) {
 		}
 		c.mu.Unlock()
 
-		_, err := conn.WriteTo(packet, dst.UDPAddr())
+		a := dst.UDPAddr()
+		c.wg.Add(1)
+		_, err := conn.WriteTo(packet, a)
+		c.wg.Done()
 		if err == nil {
 			// Success
 			return
 		}
-		if errors.Is(err, net.ErrClosed) {
+		if errors.Is(err, os.ErrDeadlineExceeded) {
 			// We intentionally closed this connection.
 			// It has been replaced by a new connection. Try again.
 			continue
@@ -421,12 +429,14 @@ func (c *fwdConn) read(out []byte) int {
 		}
 		c.mu.Unlock()
 
+		c.wg.Add(1)
 		n, _, err := conn.ReadFrom(out)
+		c.wg.Done()
 		if err == nil {
 			// Success.
 			return n
 		}
-		if errors.Is(err, net.ErrClosed) {
+		if errors.Is(err, os.ErrDeadlineExceeded) {
 			// We intentionally closed this connection.
 			// It has been replaced by a new connection. Try again.
 			continue
@@ -458,7 +468,10 @@ func (c *fwdConn) closeConnLocked() {
 	if c.conn == nil {
 		return
 	}
-	c.conn.Close() // unblocks all readers/writers, they'll pick up the next connection.
+	// Unblock all readers/writers, wait for them, close the connection.
+	c.conn.SetDeadline(aLongTimeAgo)
+	c.wg.Wait()
+	c.conn.Close()
 	c.conn = nil
 }
 

net/interfaces/interfaces_darwin.go

@@ -10,7 +10,6 @@ import (
 	"log"
 	"net"
 	"syscall"
-	"time"
 
 	"golang.org/x/net/route"
 	"golang.org/x/sys/unix"
@@ -29,34 +28,6 @@ func DefaultRouteInterface() (string, error) {
 	return iface.Name, nil
 }
 
-// fetchRoutingTable is a retry loop around route.FetchRIB, fetching NET_RT_DUMP2.
-//
-// The retry loop is due to a bug in the BSDs (or Go?). See
-// https://github.com/tailscale/tailscale/issues/1345
-func fetchRoutingTable() (rib []byte, err error) {
-	fails := 0
-	for {
-		rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
-		if err == nil {
-			return rib, nil
-		}
-		fails++
-		if fails < 10 {
-			// Empirically, 1 retry is enough. In a long
-			// stress test while toggling wifi on & off, I
-			// only saw a few occurrences of 2 and one 3.
-			// So 10 should be more plenty.
-			if fails > 5 {
-				time.Sleep(5 * time.Millisecond)
-			}
-			continue
-		}
-		if err != nil {
-			return nil, fmt.Errorf("route.FetchRIB: %w", err)
-		}
-	}
-}
-
 func DefaultRouteInterfaceIndex() (int, error) {
 	// $ netstat -nr
 	// Routing tables
@@ -72,7 +43,7 @@ func DefaultRouteInterfaceIndex() (int, error) {
 	// c       RTF_PRCLONING    Protocol-specified generate new routes on use
 	// I       RTF_IFSCOPE      Route is associated with an interface scope
 
-	rib, err := fetchRoutingTable()
+	rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
 	if err != nil {
 		return 0, fmt.Errorf("route.FetchRIB: %w", err)
 	}
@@ -112,7 +83,7 @@ func init() {
 }
 
 func likelyHomeRouterIPDarwinFetchRIB() (ret netaddr.IP, ok bool) {
-	rib, err := fetchRoutingTable()
+	rib, err := route.FetchRIB(syscall.AF_UNSPEC, syscall.NET_RT_DUMP2, 0)
 	if err != nil {
 		log.Printf("routerIP/FetchRIB: %v", err)
 		return ret, false

net/interfaces/interfaces_darwin_test.go

@@ -83,14 +83,4 @@ func likelyHomeRouterIPDarwinExec() (ret netaddr.IP, ok bool) {
 	return ret, !ret.IsZero()
 }
 
-func TestFetchRoutingTable(t *testing.T) {
-	// Issue 1345: this used to be flaky on darwin.
-	for i := 0; i < 20; i++ {
-		_, err := fetchRoutingTable()
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-}
-
 var errStopReadingNetstatTable = errors.New("found private gateway")

net/tstun/wrap.go

@@ -90,7 +90,7 @@ type Wrapper struct {
 	// to discard an empty packet instead of sending it through t.outbound.
 	outbound chan []byte
 
-	// filter atomically stores the currently active packet filter
+	// fitler stores the currently active package filter
 	filter atomic.Value // of *filter.Filter
 	// filterFlags control the verbosity of logging packet drops/accepts.
 	filterFlags filter.RunFlags

tailcfg/tailcfg.go

@@ -41,8 +41,7 @@ import (
 //    16: 2021-04-15: client understands Node.Online, MapResponse.OnlineChange
 //    17: 2021-04-18: MapResponse.Domain empty means unchanged
 //    18: 2021-04-19: MapResponse.Node nil means unchanged (all fields now omitempty)
-//    19: 2021-04-21: MapResponse.Debug.SleepSeconds
-const CurrentMapRequestVersion = 19
+const CurrentMapRequestVersion = 18
 
 type StableID string
 
@@ -1013,12 +1012,6 @@ type Debug struct {
 	// GoroutineDumpURL, if non-empty, requests that the client do
 	// a one-time dump of its active goroutines to the given URL.
 	GoroutineDumpURL string `json:",omitempty"`
-
-	// SleepSeconds requests that the client sleep for the
-	// provided number of seconds.
-	// The client can (and should) limit the value (such as 5
-	// minutes).
-	SleepSeconds float64 `json:",omitempty"`
 }
 
 func (k MachineKey) String() string                   { return fmt.Sprintf("mkey:%x", k[:]) }

util/cmpver/version.go

@@ -1,96 +0,0 @@
-// Copyright (c) 2021 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 cmpver implements a variant of debian version number
-// comparison.
-//
-// A version is a string consisting of alternating non-numeric and
-// numeric fields. When comparing two versions, each one is broken
-// down into its respective fields, and the fields are compared
-// pairwise. The comparison is lexicographic for non-numeric fields,
-// numeric for numeric fields. The first non-equal field pair
-// determines the ordering of the two versions.
-//
-// This comparison scheme is a simplified version of Debian's version
-// number comparisons. Debian differs in a few details of
-// lexicographical field comparison, where certain characters have
-// special meaning and ordering. We don't need that, because Tailscale
-// version numbers don't need it.
-package cmpver
-
-import (
-	"fmt"
-	"strconv"
-	"strings"
-	"unicode"
-)
-
-// Compare returns an integer comparing two strings as version
-// numbers. The result will be 0 if v1==v2, -1 if v1 < v2, and +1 if
-// v1 > v2.
-func Compare(v1, v2 string) int {
-	notNumber := func(r rune) bool { return !unicode.IsNumber(r) }
-
-	var (
-		f1, f2 string
-		n1, n2 uint64
-		err    error
-	)
-	for v1 != "" || v2 != "" {
-		// Compare the non-numeric character run lexicographically.
-		f1, v1 = splitPrefixFunc(v1, notNumber)
-		f2, v2 = splitPrefixFunc(v2, notNumber)
-
-		if res := strings.Compare(f1, f2); res != 0 {
-			return res
-		}
-
-		// Compare the numeric character run numerically.
-		f1, v1 = splitPrefixFunc(v1, unicode.IsNumber)
-		f2, v2 = splitPrefixFunc(v2, unicode.IsNumber)
-
-		// ParseUint refuses to parse empty strings, which would only
-		// happen if we reached end-of-string. We follow the Debian
-		// convention that empty strings mean zero, because
-		// empirically that produces reasonable-feeling comparison
-		// behavior.
-		n1 = 0
-		if f1 != "" {
-			n1, err = strconv.ParseUint(f1, 10, 64)
-			if err != nil {
-				panic(fmt.Sprintf("all-number string %q didn't parse as string: %s", f1, err))
-			}
-		}
-
-		n2 = 0
-		if f2 != "" {
-			n2, err = strconv.ParseUint(f2, 10, 64)
-			if err != nil {
-				panic(fmt.Sprintf("all-number string %q didn't parse as string: %s", f2, err))
-			}
-		}
-
-		switch {
-		case n1 == n2:
-		case n1 < n2:
-			return -1
-		case n1 > n2:
-			return 1
-		}
-	}
-
-	// Only way to reach here is if v1 and v2 run out of fields
-	// simultaneously - i.e. exactly equal versions.
-	return 0
-}
-
-// splitPrefixFunc splits s at the first rune where f(rune) is false.
-func splitPrefixFunc(s string, f func(rune) bool) (string, string) {
-	for i, r := range s {
-		if !f(r) {
-			return s[:i], s[i:]
-		}
-	}
-	return s, s[:0]
-}

util/cmpver/version_test.go

@@ -1,106 +0,0 @@
-// Copyright (c) 2021 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 cmpver
-
-import "testing"
-
-func TestCompare(t *testing.T) {
-	tests := []struct {
-		name   string
-		v1, v2 string
-		want   int
-	}{
-		{
-			name: "both empty",
-			want: 0,
-		},
-		{
-			name: "v1 empty",
-			v2:   "1.2.3",
-			want: -1,
-		},
-		{
-			name: "v2 empty",
-			v1:   "1.2.3",
-			want: 1,
-		},
-
-		{
-			name: "semver major",
-			v1:   "2.0.0",
-			v2:   "1.9.9",
-			want: 1,
-		},
-		{
-			name: "semver major",
-			v1:   "2.0.0",
-			v2:   "1.9.9",
-			want: 1,
-		},
-		{
-			name: "semver minor",
-			v1:   "1.9.0",
-			v2:   "1.8.9",
-			want: 1,
-		},
-		{
-			name: "semver patch",
-			v1:   "1.9.9",
-			v2:   "1.9.8",
-			want: 1,
-		},
-		{
-			name: "semver equal",
-			v1:   "1.9.8",
-			v2:   "1.9.8",
-			want: 0,
-		},
-
-		{
-			name: "tailscale major",
-			v1:   "1.0-0",
-			v2:   "0.97-105",
-			want: 1,
-		},
-		{
-			name: "tailscale minor",
-			v1:   "0.98-0",
-			v2:   "0.97-105",
-			want: 1,
-		},
-		{
-			name: "tailscale patch",
-			v1:   "0.97-120",
-			v2:   "0.97-105",
-			want: 1,
-		},
-		{
-			name: "tailscale equal",
-			v1:   "0.97-105",
-			v2:   "0.97-105",
-			want: 0,
-		},
-		{
-			name: "tailscale weird extra field",
-			v1:   "0.96.1-0", // more fields == larger
-			v2:   "0.96-105",
-			want: 1,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			got := Compare(test.v1, test.v2)
-			if got != test.want {
-				t.Errorf("Compare(%v, %v) = %v, want %v", test.v1, test.v2, got, test.want)
-			}
-			// Reversing the comparison should reverse the outcome.
-			got2 := Compare(test.v2, test.v1)
-			if got2 != -test.want {
-				t.Errorf("Compare(%v, %v) = %v, want %v", test.v2, test.v1, got2, -test.want)
-			}
-		})
-	}
-}

util/osshare/filesharingstatus_noop.go

@@ -1,13 +0,0 @@
-// Copyright (c) 2021 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 !windows
-
-package osshare
-
-import (
-	"tailscale.com/types/logger"
-)
-
-func SetFileSharingEnabled(enabled bool, logf logger.Logf) {}

util/osshare/filesharingstatus_windows.go

@@ -1,107 +0,0 @@
-// Copyright (c) 2021 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 windows
-
-package osshare
-
-import (
-	"fmt"
-	"os"
-	"path/filepath"
-	"sync"
-
-	"golang.org/x/sys/windows/registry"
-	"tailscale.com/types/logger"
-)
-
-const (
-	sendFileShellKey = `*\shell\tailscale`
-)
-
-var ipnExePath struct {
-	sync.Mutex
-	cache string // absolute path of tailscale-ipn.exe, populated lazily on first use
-}
-
-func getIpnExePath(logf logger.Logf) string {
-	ipnExePath.Lock()
-	defer ipnExePath.Unlock()
-
-	if ipnExePath.cache != "" {
-		return ipnExePath.cache
-	}
-
-	// Find the absolute path of tailscale-ipn.exe assuming that it's in the same
-	// directory as this executable (tailscaled.exe).
-	p, err := os.Executable()
-	if err != nil {
-		logf("os.Executable error: %v", err)
-		return ""
-	}
-	if p, err = filepath.EvalSymlinks(p); err != nil {
-		logf("filepath.EvalSymlinks error: %v", err)
-		return ""
-	}
-	p = filepath.Join(filepath.Dir(p), "tailscale-ipn.exe")
-	if p, err = filepath.Abs(p); err != nil {
-		logf("filepath.Abs error: %v", err)
-		return ""
-	}
-	ipnExePath.cache = p
-
-	return p
-}
-
-// SetFileSharingEnabled adds/removes "Send with Tailscale" from the Windows shell menu.
-func SetFileSharingEnabled(enabled bool, logf logger.Logf) {
-	logf = logger.WithPrefix(logf, fmt.Sprintf("SetFileSharingEnabled(%v) error: ", enabled))
-	if enabled {
-		enableFileSharing(logf)
-	} else {
-		disableFileSharing(logf)
-	}
-}
-
-func enableFileSharing(logf logger.Logf) {
-	path := getIpnExePath(logf)
-	if path == "" {
-		return
-	}
-
-	k, _, err := registry.CreateKey(registry.CLASSES_ROOT, sendFileShellKey, registry.WRITE)
-	if err != nil {
-		logf("failed to create HKEY_CLASSES_ROOT\\%s reg key: %v", sendFileShellKey, err)
-		return
-	}
-	defer k.Close()
-	if err := k.SetStringValue("", "Send with Tailscale..."); err != nil {
-		logf("k.SetStringValue error: %v", err)
-		return
-	}
-	if err := k.SetStringValue("Icon", path+",0"); err != nil {
-		logf("k.SetStringValue error: %v", err)
-		return
-	}
-	c, _, err := registry.CreateKey(k, "command", registry.WRITE)
-	if err != nil {
-		logf("failed to create HKEY_CLASSES_ROOT\\%s\\command reg key: %v", sendFileShellKey, err)
-		return
-	}
-	defer c.Close()
-	if err := c.SetStringValue("", "\""+path+"\" /push \"%1\""); err != nil {
-		logf("c.SetStringValue error: %v", err)
-	}
-}
-
-func disableFileSharing(logf logger.Logf) {
-	if err := registry.DeleteKey(registry.CLASSES_ROOT, sendFileShellKey+"\\command"); err != nil &&
-		err != registry.ErrNotExist {
-		logf("registry.DeleteKey error: %v\n", err)
-		return
-	}
-	if err := registry.DeleteKey(registry.CLASSES_ROOT, sendFileShellKey); err != nil && err != registry.ErrNotExist {
-		logf("registry.DeleteKey error: %v\n", err)
-	}
-}

wgengine/bench/bench.go

@@ -1,398 +0,0 @@
-// Copyright (c) 2021 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.
-
-// Create two wgengine instances and pass data through them, measuring
-// throughput, latency, and packet loss.
-package main
-
-import (
-	"bufio"
-	"io"
-	"log"
-	"net"
-	"net/http"
-	"net/http/pprof"
-	"os"
-	"strconv"
-	"time"
-
-	"inet.af/netaddr"
-	"tailscale.com/types/logger"
-)
-
-const PayloadSize = 1000
-const ICMPMinSize = 24
-
-var Addr1 = netaddr.MustParseIPPrefix("100.64.1.1/32")
-var Addr2 = netaddr.MustParseIPPrefix("100.64.1.2/32")
-
-func main() {
-	var logf logger.Logf = log.Printf
-	log.SetFlags(0)
-
-	debugMux := newDebugMux()
-	go runDebugServer(debugMux, "0.0.0.0:8999")
-
-	mode, err := strconv.Atoi(os.Args[1])
-	if err != nil {
-		log.Fatalf("%q: %v", os.Args[1], err)
-	}
-
-	traf := NewTrafficGen(nil)
-
-	// Sample test results below are using GOMAXPROCS=2 (for some
-	// tests, including wireguard-go, higher GOMAXPROCS goes slower)
-	// on apenwarr's old Linux box:
-	//   Intel(R) Core(TM) i7-4785T CPU @ 2.20GHz
-	// My 2019 Mac Mini is about 20% faster on most tests.
-
-	switch mode {
-	// tx=8786325 rx=8786326 (0 = 0.00% loss) (70768.7 Mbits/sec)
-	case 1:
-		setupTrivialNoAllocTest(logf, traf)
-
-	// tx=6476293 rx=6476293 (0 = 0.00% loss) (52249.7 Mbits/sec)
-	case 2:
-		setupTrivialTest(logf, traf)
-
-	// tx=1957974 rx=1958379 (0 = 0.00% loss) (15939.8 Mbits/sec)
-	case 11:
-		setupBlockingChannelTest(logf, traf)
-
-	// tx=728621 rx=701825 (26620 = 3.65% loss) (5525.2 Mbits/sec)
-	// (much faster on macOS??)
-	case 12:
-		setupNonblockingChannelTest(logf, traf)
-
-	// tx=1024260 rx=941098 (83334 = 8.14% loss) (7516.6 Mbits/sec)
-	// (much faster on macOS??)
-	case 13:
-		setupDoubleChannelTest(logf, traf)
-
-	// tx=265468 rx=263189 (2279 = 0.86% loss) (2162.0 Mbits/sec)
-	case 21:
-		setupUDPTest(logf, traf)
-
-	// tx=1493580 rx=1493580 (0 = 0.00% loss) (12210.4 Mbits/sec)
-	case 31:
-		setupBatchTCPTest(logf, traf)
-
-	// tx=134236 rx=133166 (1070 = 0.80% loss) (1088.9 Mbits/sec)
-	case 101:
-		setupWGTest(logf, traf, Addr1, Addr2)
-
-	default:
-		log.Fatalf("provide a valid test number (0..n)")
-	}
-
-	logf("initialized ok.")
-	traf.Start(Addr1.IP, Addr2.IP, PayloadSize+ICMPMinSize, 0)
-
-	var cur, prev Snapshot
-	var pps int64
-	i := 0
-	for {
-		i += 1
-		time.Sleep(10 * time.Millisecond)
-
-		if (i % 100) == 0 {
-			prev = cur
-			cur = traf.Snap()
-			d := cur.Sub(prev)
-
-			if prev.WhenNsec == 0 {
-				logf("tx=%-6d rx=%-6d", d.TxPackets, d.RxPackets)
-			} else {
-				logf("%v @%7d pkt/s", d, pps)
-			}
-		}
-
-		pps = traf.Adjust()
-	}
-}
-
-func newDebugMux() *http.ServeMux {
-	mux := http.NewServeMux()
-	mux.HandleFunc("/debug/pprof/", pprof.Index)
-	mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
-	mux.HandleFunc("/debug/pprof/profile", pprof.Profile)
-	mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
-	mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
-	return mux
-}
-
-func runDebugServer(mux *http.ServeMux, addr string) {
-	srv := &http.Server{
-		Addr:    addr,
-		Handler: mux,
-	}
-	if err := srv.ListenAndServe(); err != nil {
-		log.Fatal(err)
-	}
-}
-
-// The absolute minimal test of the traffic generator: have it fill
-// a packet buffer, then absorb it again. Zero packet loss.
-func setupTrivialNoAllocTest(logf logger.Logf, traf *TrafficGen) {
-	go func() {
-		b := make([]byte, 1600)
-		for {
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				break
-			}
-			traf.GotPacket(b[0:n+16], 16)
-		}
-	}()
-}
-
-// Almost the same, but this time allocate a fresh buffer each time
-// through the loop. Still zero packet loss. Runs about 2/3 as fast for me.
-func setupTrivialTest(logf logger.Logf, traf *TrafficGen) {
-	go func() {
-		for {
-			b := make([]byte, 1600)
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				break
-			}
-			traf.GotPacket(b[0:n+16], 16)
-		}
-	}()
-}
-
-// Pass packets through a blocking channel between sender and receiver.
-// Still zero packet loss since the sender stops when the channel is full.
-// Max speed depends on channel length (I'm not sure why).
-func setupBlockingChannelTest(logf logger.Logf, traf *TrafficGen) {
-	ch := make(chan []byte, 1000)
-
-	go func() {
-		// transmitter
-		for {
-			b := make([]byte, 1600)
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				close(ch)
-				break
-			}
-			ch <- b[0 : n+16]
-		}
-	}()
-
-	go func() {
-		// receiver
-		for b := range ch {
-			traf.GotPacket(b, 16)
-		}
-	}()
-}
-
-// Same as setupBlockingChannelTest, but now we drop packets whenever the
-// channel is full. Max speed is about the same as the above test, but
-// now with nonzero packet loss.
-func setupNonblockingChannelTest(logf logger.Logf, traf *TrafficGen) {
-	ch := make(chan []byte, 1000)
-
-	go func() {
-		// transmitter
-		for {
-			b := make([]byte, 1600)
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				close(ch)
-				break
-			}
-			select {
-			case ch <- b[0 : n+16]:
-			default:
-			}
-		}
-	}()
-
-	go func() {
-		// receiver
-		for b := range ch {
-			traf.GotPacket(b, 16)
-		}
-	}()
-}
-
-// Same as above, but at an intermediate blocking channel and goroutine
-// to make things a little more like wireguard-go. Roughly 20% slower than
-// the single-channel verison.
-func setupDoubleChannelTest(logf logger.Logf, traf *TrafficGen) {
-	ch := make(chan []byte, 1000)
-	ch2 := make(chan []byte, 1000)
-
-	go func() {
-		// transmitter
-		for {
-			b := make([]byte, 1600)
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				close(ch)
-				break
-			}
-			select {
-			case ch <- b[0 : n+16]:
-			default:
-			}
-		}
-	}()
-
-	go func() {
-		// intermediary
-		for b := range ch {
-			ch2 <- b
-		}
-		close(ch2)
-	}()
-
-	go func() {
-		// receiver
-		for b := range ch2 {
-			traf.GotPacket(b, 16)
-		}
-	}()
-}
-
-// Instead of a channel, pass packets through a UDP socket.
-func setupUDPTest(logf logger.Logf, traf *TrafficGen) {
-	la, err := net.ResolveUDPAddr("udp", ":0")
-	if err != nil {
-		log.Fatalf("resolve: %v", err)
-	}
-
-	s1, err := net.ListenUDP("udp", la)
-	if err != nil {
-		log.Fatalf("listen1: %v", err)
-	}
-	s2, err := net.ListenUDP("udp", la)
-	if err != nil {
-		log.Fatalf("listen2: %v", err)
-	}
-
-	a2 := s2.LocalAddr()
-
-	// On macOS (but not Linux), you can't transmit to 0.0.0.0:port,
-	// which is what returns from .LocalAddr() above. We have to
-	// force it to localhost instead.
-	a2.(*net.UDPAddr).IP = net.ParseIP("127.0.0.1")
-
-	s1.SetWriteBuffer(1024 * 1024)
-	s2.SetReadBuffer(1024 * 1024)
-
-	go func() {
-		// transmitter
-		b := make([]byte, 1600)
-		for {
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				break
-			}
-			s1.WriteTo(b[16:n+16], a2)
-		}
-	}()
-
-	go func() {
-		// receiver
-		b := make([]byte, 1600)
-		for traf.Running() {
-			// Use ReadFrom instead of Read, to be more like
-			// how wireguard-go does it, even though we're not
-			// going to actually look at the address.
-			n, _, err := s2.ReadFrom(b)
-			if err != nil {
-				log.Fatalf("s2.Read: %v", err)
-			}
-			traf.GotPacket(b[:n], 0)
-		}
-	}()
-}
-
-// Instead of a channel, pass packets through a TCP socket.
-// TCP is a single stream, so we can amortize one syscall across
-// multiple packets. 10x amortization seems to make it go ~10x faster,
-// as expected, getting us close to the speed of the channel tests above.
-// There's also zero packet loss.
-func setupBatchTCPTest(logf logger.Logf, traf *TrafficGen) {
-	sl, err := net.Listen("tcp", ":0")
-	if err != nil {
-		log.Fatalf("listen: %v", err)
-	}
-
-	s1, err := net.Dial("tcp", sl.Addr().String())
-	if err != nil {
-		log.Fatalf("dial: %v", err)
-	}
-
-	s2, err := sl.Accept()
-	if err != nil {
-		log.Fatalf("accept: %v", err)
-	}
-
-	s1.(*net.TCPConn).SetWriteBuffer(1024 * 1024)
-	s2.(*net.TCPConn).SetReadBuffer(1024 * 1024)
-
-	ch := make(chan int)
-
-	go func() {
-		// transmitter
-
-		bs1 := bufio.NewWriterSize(s1, 1024*1024)
-
-		b := make([]byte, 1600)
-		i := 0
-		for {
-			i += 1
-			n := traf.Generate(b, 16)
-			if n == 0 {
-				break
-			}
-			if i == 1 {
-				ch <- n
-			}
-			bs1.Write(b[16 : n+16])
-
-			// TODO: this is a pretty half-baked batching
-			// function, which we'd never want to employ in
-			// a real-life program.
-			//
-			// In real life, we'd probably want to flush
-			// immediately when there are no more packets to
-			// generate, and queue up only if we fall behind.
-			//
-			// In our case however, we just want to see the
-			// technical benefits of batching 10 syscalls
-			// into 1, so a fixed ratio makes more sense.
-			if (i % 10) == 0 {
-				bs1.Flush()
-			}
-		}
-	}()
-
-	go func() {
-		// receiver
-
-		bs2 := bufio.NewReaderSize(s2, 1024*1024)
-
-		// Find out the packet size (we happen to know they're
-		// all the same size)
-		packetSize := <-ch
-
-		b := make([]byte, packetSize)
-		for traf.Running() {
-			// TODO: can't use ReadFrom() here, which is
-			// unfair compared to UDP. (ReadFrom for UDP
-			// apparently allocates memory per packet, which
-			// this test does not.)
-			n, err := io.ReadFull(bs2, b)
-			if err != nil {
-				log.Fatalf("s2.Read: %v", err)
-			}
-			traf.GotPacket(b[:n], 0)
-		}
-	}()
-}

wgengine/bench/bench_test.go

@@ -1,108 +0,0 @@
-// Copyright (c) 2021 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.
-
-// Create two wgengine instances and pass data through them, measuring
-// throughput, latency, and packet loss.
-package main
-
-import (
-	"fmt"
-	"testing"
-	"time"
-
-	"tailscale.com/types/logger"
-)
-
-func BenchmarkTrivialNoAlloc(b *testing.B) {
-	run(b, setupTrivialNoAllocTest)
-}
-func BenchmarkTrivial(b *testing.B) {
-	run(b, setupTrivialTest)
-}
-
-func BenchmarkBlockingChannel(b *testing.B) {
-	run(b, setupBlockingChannelTest)
-}
-
-func BenchmarkNonblockingChannel(b *testing.B) {
-	run(b, setupNonblockingChannelTest)
-}
-
-func BenchmarkDoubleChannel(b *testing.B) {
-	run(b, setupDoubleChannelTest)
-}
-
-func BenchmarkUDP(b *testing.B) {
-	run(b, setupUDPTest)
-}
-
-func BenchmarkBatchTCP(b *testing.B) {
-	run(b, setupBatchTCPTest)
-}
-
-func BenchmarkWireGuardTest(b *testing.B) {
-	run(b, func(logf logger.Logf, traf *TrafficGen) {
-		setupWGTest(logf, traf, Addr1, Addr2)
-	})
-}
-
-type SetupFunc func(logger.Logf, *TrafficGen)
-
-func run(b *testing.B, setup SetupFunc) {
-	sizes := []int{
-		ICMPMinSize + 8,
-		ICMPMinSize + 100,
-		ICMPMinSize + 1000,
-	}
-
-	for _, size := range sizes {
-		b.Run(fmt.Sprintf("%d", size), func(b *testing.B) {
-			runOnce(b, setup, size)
-		})
-	}
-}
-
-func runOnce(b *testing.B, setup SetupFunc, payload int) {
-	b.StopTimer()
-	b.ReportAllocs()
-
-	var logf logger.Logf = b.Logf
-	if !testing.Verbose() {
-		logf = logger.Discard
-	}
-
-	traf := NewTrafficGen(b.StartTimer)
-	setup(logf, traf)
-
-	logf("initialized. (n=%v)", b.N)
-	b.SetBytes(int64(payload))
-
-	traf.Start(Addr1.IP, Addr2.IP, payload, int64(b.N))
-
-	var cur, prev Snapshot
-	var pps int64
-	i := 0
-	for traf.Running() {
-		i += 1
-		time.Sleep(10 * time.Millisecond)
-
-		if (i % 100) == 0 {
-			prev = cur
-			cur = traf.Snap()
-			d := cur.Sub(prev)
-
-			if prev.WhenNsec != 0 {
-				logf("%v @%7d pkt/sec", d, pps)
-			}
-		}
-
-		pps = traf.Adjust()
-	}
-
-	cur = traf.Snap()
-	d := cur.Sub(prev)
-	loss := float64(d.LostPackets) / float64(d.RxPackets)
-
-	b.ReportMetric(loss*100, "%lost")
-}

wgengine/bench/trafficgen.go

@@ -1,262 +0,0 @@
-// Copyright (c) 2021 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 main
-
-import (
-	"encoding/binary"
-	"fmt"
-	"log"
-	"sync"
-	"time"
-
-	"inet.af/netaddr"
-	"tailscale.com/net/packet"
-	"tailscale.com/types/ipproto"
-)
-
-type Snapshot struct {
-	WhenNsec int64 // current time
-	timeAcc  int64 // accumulated time (+NSecPerTx per transmit)
-
-	LastSeqTx    int64 // last sequence number sent
-	LastSeqRx    int64 // last sequence number received
-	TotalLost    int64 // packets out-of-order or lost so far
-	TotalOOO     int64 // packets out-of-order so far
-	TotalBytesRx int64 // total bytes received so far
-}
-
-type Delta struct {
-	DurationNsec int64
-	TxPackets    int64
-	RxPackets    int64
-	LostPackets  int64
-	OOOPackets   int64
-	Bytes        int64
-}
-
-func (b Snapshot) Sub(a Snapshot) Delta {
-	return Delta{
-		DurationNsec: b.WhenNsec - a.WhenNsec,
-		TxPackets:    b.LastSeqTx - a.LastSeqTx,
-		RxPackets: (b.LastSeqRx - a.LastSeqRx) -
-			(b.TotalLost - a.TotalLost) +
-			(b.TotalOOO - a.TotalOOO),
-		LostPackets: b.TotalLost - a.TotalLost,
-		OOOPackets:  b.TotalOOO - a.TotalOOO,
-		Bytes:       b.TotalBytesRx - a.TotalBytesRx,
-	}
-}
-
-func (d Delta) String() string {
-	return fmt.Sprintf("tx=%-6d rx=%-4d (%6d = %.1f%% loss) (%d OOO) (%4.1f Mbit/s)",
-		d.TxPackets, d.RxPackets, d.LostPackets,
-		float64(d.LostPackets)*100/float64(d.TxPackets),
-		d.OOOPackets,
-		float64(d.Bytes)*8*1e9/float64(d.DurationNsec)/1e6)
-}
-
-type TrafficGen struct {
-	mu        sync.Mutex
-	cur, prev Snapshot // snapshots used for rate control
-	buf       []byte   // pre-generated packet buffer
-	done      bool     // true if the test has completed
-
-	onFirstPacket func() // function to call on first received packet
-
-	// maxPackets is the max packets to receive (not send) before
-	// ending the test. If it's zero, the test runs forever.
-	maxPackets int64
-
-	// nsPerPacket is the target average nanoseconds between packets.
-	// It's initially zero, which means transmit as fast as the
-	// caller wants to go.
-	nsPerPacket int64
-
-	// ppsHistory is the observed packets-per-second from recent
-	// samples.
-	ppsHistory [5]int64
-}
-
-// NewTrafficGen creates a new, initially locked, TrafficGen.
-// Until Start() is called, Generate() will block forever.
-func NewTrafficGen(onFirstPacket func()) *TrafficGen {
-	t := TrafficGen{
-		onFirstPacket: onFirstPacket,
-	}
-
-	// initially locked, until first Start()
-	t.mu.Lock()
-
-	return &t
-}
-
-// Start starts the traffic generator. It assumes mu is already locked,
-// and unlocks it.
-func (t *TrafficGen) Start(src, dst netaddr.IP, bytesPerPacket int, maxPackets int64) {
-	h12 := packet.ICMP4Header{
-		IP4Header: packet.IP4Header{
-			IPProto: ipproto.ICMPv4,
-			IPID:    0,
-			Src:     src,
-			Dst:     dst,
-		},
-		Type: packet.ICMP4EchoRequest,
-		Code: packet.ICMP4NoCode,
-	}
-
-	// ensure there's room for ICMP header plus sequence number
-	if bytesPerPacket < ICMPMinSize+8 {
-		log.Fatalf("bytesPerPacket must be > 24+8")
-	}
-
-	t.maxPackets = maxPackets
-
-	payload := make([]byte, bytesPerPacket-ICMPMinSize)
-	t.buf = packet.Generate(h12, payload)
-
-	t.mu.Unlock()
-}
-
-func (t *TrafficGen) Snap() Snapshot {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	t.cur.WhenNsec = time.Now().UnixNano()
-	return t.cur
-}
-
-func (t *TrafficGen) Running() bool {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	return !t.done
-}
-
-// Generate produces the next packet in the sequence. It sleeps if
-// it's too soon for the next packet to be sent.
-//
-// The generated packet is placed into buf at offset ofs, for compatibility
-// with the wireguard-go conventions.
-//
-// The return value is the number of bytes generated in the packet, or 0
-// if the test has finished running.
-func (t *TrafficGen) Generate(b []byte, ofs int) int {
-	t.mu.Lock()
-
-	now := time.Now().UnixNano()
-	if t.nsPerPacket == 0 || t.cur.timeAcc == 0 {
-		t.cur.timeAcc = now - 1
-	}
-	if t.cur.timeAcc >= now {
-		// too soon
-		t.mu.Unlock()
-		time.Sleep(time.Duration(t.cur.timeAcc-now) * time.Nanosecond)
-		t.mu.Lock()
-
-		now = t.cur.timeAcc
-	}
-	if t.done {
-		t.mu.Unlock()
-		return 0
-	}
-
-	t.cur.timeAcc += t.nsPerPacket
-	t.cur.LastSeqTx += 1
-	t.cur.WhenNsec = now
-	seq := t.cur.LastSeqTx
-
-	t.mu.Unlock()
-
-	copy(b[ofs:], t.buf)
-	binary.BigEndian.PutUint64(
-		b[ofs+ICMPMinSize:ofs+ICMPMinSize+8],
-		uint64(seq))
-
-	return len(t.buf)
-}
-
-// GotPacket processes a packet that came back on the receive side.
-func (t *TrafficGen) GotPacket(b []byte, ofs int) {
-	t.mu.Lock()
-
-	s := &t.cur
-	seq := int64(binary.BigEndian.Uint64(
-		b[ofs+ICMPMinSize : ofs+ICMPMinSize+8]))
-	if seq > s.LastSeqRx {
-		if s.LastSeqRx > 0 {
-			// only count lost packets after the very first
-			// successful one.
-			s.TotalLost += seq - s.LastSeqRx - 1
-		}
-		s.LastSeqRx = seq
-	} else {
-		s.TotalOOO += 1
-	}
-
-	// +1 packet since we only start counting after the first one
-	if t.maxPackets > 0 && s.LastSeqRx >= t.maxPackets+1 {
-		t.done = true
-	}
-	s.TotalBytesRx += int64(len(b) - ofs)
-
-	f := t.onFirstPacket
-	t.onFirstPacket = nil
-
-	t.mu.Unlock()
-
-	if f != nil {
-		f()
-	}
-}
-
-// Adjust tunes the transmit rate based on the received packets.
-// The goal is to converge on the fastest transmit rate that still has
-// minimal packet loss. Returns the new target rate in packets/sec.
-//
-// We need to play this guessing game in order to balance out tx and rx
-// rates when there's a lossy network between them. Otherwise we can end
-// up using 99% of the CPU to blast out transmitted packets and leaving only
-// 1% to receive them, leading to a misleading throughput calculation.
-//
-// Call this function multiple times per second.
-func (t *TrafficGen) Adjust() (pps int64) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-
-	d := t.cur.Sub(t.prev)
-
-	// don't adjust rate until the first full period *after* receiving
-	// the first packet. This skips any handshake time in the underlying
-	// transport.
-	if t.prev.LastSeqRx == 0 || d.DurationNsec == 0 {
-		t.prev = t.cur
-		return 0 // no estimate yet, continue at max speed
-	}
-
-	pps = int64(d.RxPackets) * 1e9 / int64(d.DurationNsec)
-
-	// We use a rate selection algorithm based loosely on TCP BBR.
-	// Basically, we set the transmit rate to be a bit higher than
-	// the best observed transmit rate in the last several time
-	// periods. This guarantees some packet loss, but should converge
-	// quickly on a rate near the sustainable maximum.
-	bestPPS := pps
-	for _, p := range t.ppsHistory {
-		if p > bestPPS {
-			bestPPS = p
-		}
-	}
-	if pps > 0 && t.prev.WhenNsec > 0 {
-		copy(t.ppsHistory[1:], t.ppsHistory[0:len(t.ppsHistory)-1])
-		t.ppsHistory[0] = pps
-	}
-	if bestPPS > 0 {
-		pps = bestPPS * 103 / 100
-		t.nsPerPacket = int64(1e9 / pps)
-	}
-	t.prev = t.cur
-
-	return pps
-}

wgengine/bench/wg.go

@@ -1,205 +0,0 @@
-// Copyright (c) 2021 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 main
-
-import (
-	"io"
-	"log"
-	"os"
-	"strings"
-	"sync"
-
-	"github.com/tailscale/wireguard-go/tun"
-	"inet.af/netaddr"
-
-	"tailscale.com/net/dns"
-	"tailscale.com/tailcfg"
-	"tailscale.com/types/logger"
-	"tailscale.com/types/netmap"
-	"tailscale.com/types/wgkey"
-	"tailscale.com/wgengine"
-	"tailscale.com/wgengine/filter"
-	"tailscale.com/wgengine/router"
-	"tailscale.com/wgengine/wgcfg"
-)
-
-func setupWGTest(logf logger.Logf, traf *TrafficGen, a1, a2 netaddr.IPPrefix) {
-	l1 := logger.WithPrefix(logf, "e1: ")
-	k1, err := wgcfg.NewPrivateKey()
-	if err != nil {
-		log.Fatalf("e1 NewPrivateKey: %v", err)
-	}
-	c1 := wgcfg.Config{
-		Name:       "e1",
-		PrivateKey: k1,
-		Addresses:  []netaddr.IPPrefix{a1},
-	}
-	t1 := &sourceTun{
-		logf: logger.WithPrefix(logf, "tun1: "),
-		traf: traf,
-	}
-	e1, err := wgengine.NewUserspaceEngine(l1, wgengine.Config{
-		Router:      router.NewFake(l1),
-		LinkMonitor: nil,
-		ListenPort:  0,
-		Tun:         t1,
-	})
-	if err != nil {
-		log.Fatalf("e1 init: %v", err)
-	}
-
-	l2 := logger.WithPrefix(logf, "e2: ")
-	k2, err := wgcfg.NewPrivateKey()
-	if err != nil {
-		log.Fatalf("e2 NewPrivateKey: %v", err)
-	}
-	c2 := wgcfg.Config{
-		Name:       "e2",
-		PrivateKey: k2,
-		Addresses:  []netaddr.IPPrefix{a2},
-	}
-	t2 := &sinkTun{
-		logf: logger.WithPrefix(logf, "tun2: "),
-		traf: traf,
-	}
-	e2, err := wgengine.NewUserspaceEngine(l2, wgengine.Config{
-		Router:      router.NewFake(l2),
-		LinkMonitor: nil,
-		ListenPort:  0,
-		Tun:         t2,
-	})
-	if err != nil {
-		log.Fatalf("e2 init: %v", err)
-	}
-
-	e1.SetFilter(filter.NewAllowAllForTest(l1))
-	e2.SetFilter(filter.NewAllowAllForTest(l2))
-
-	var wait sync.WaitGroup
-	wait.Add(2)
-
-	e1.SetStatusCallback(func(st *wgengine.Status, err error) {
-		if err != nil {
-			log.Fatalf("e1 status err: %v", err)
-		}
-		logf("e1 status: %v", *st)
-
-		var eps []string
-		for _, ep := range st.LocalAddrs {
-			eps = append(eps, ep.Addr.String())
-		}
-
-		n := tailcfg.Node{
-			ID:         tailcfg.NodeID(0),
-			Name:       "n1",
-			Addresses:  []netaddr.IPPrefix{a1},
-			AllowedIPs: []netaddr.IPPrefix{a1},
-			Endpoints:  eps,
-		}
-		e2.SetNetworkMap(&netmap.NetworkMap{
-			NodeKey:    tailcfg.NodeKey(k2),
-			PrivateKey: wgkey.Private(k2),
-			Peers:      []*tailcfg.Node{&n},
-		})
-
-		p := wgcfg.Peer{
-			PublicKey:  c1.PrivateKey.Public(),
-			AllowedIPs: []netaddr.IPPrefix{a1},
-			Endpoints:  strings.Join(eps, ","),
-		}
-		c2.Peers = []wgcfg.Peer{p}
-		e2.Reconfig(&c2, &router.Config{}, new(dns.Config))
-		wait.Done()
-	})
-
-	e2.SetStatusCallback(func(st *wgengine.Status, err error) {
-		if err != nil {
-			log.Fatalf("e2 status err: %v", err)
-		}
-		logf("e2 status: %v", *st)
-
-		var eps []string
-		for _, ep := range st.LocalAddrs {
-			eps = append(eps, ep.Addr.String())
-		}
-
-		n := tailcfg.Node{
-			ID:         tailcfg.NodeID(0),
-			Name:       "n2",
-			Addresses:  []netaddr.IPPrefix{a2},
-			AllowedIPs: []netaddr.IPPrefix{a2},
-			Endpoints:  eps,
-		}
-		e1.SetNetworkMap(&netmap.NetworkMap{
-			NodeKey:    tailcfg.NodeKey(k1),
-			PrivateKey: wgkey.Private(k1),
-			Peers:      []*tailcfg.Node{&n},
-		})
-
-		p := wgcfg.Peer{
-			PublicKey:  c2.PrivateKey.Public(),
-			AllowedIPs: []netaddr.IPPrefix{a2},
-			Endpoints:  strings.Join(eps, ","),
-		}
-		c1.Peers = []wgcfg.Peer{p}
-		e1.Reconfig(&c1, &router.Config{}, new(dns.Config))
-		wait.Done()
-	})
-
-	// Not using DERP in this test (for now?).
-	e1.SetDERPMap(&tailcfg.DERPMap{})
-	e2.SetDERPMap(&tailcfg.DERPMap{})
-
-	wait.Wait()
-}
-
-type sourceTun struct {
-	logf logger.Logf
-	traf *TrafficGen
-}
-
-func (t *sourceTun) Close() error           { return nil }
-func (t *sourceTun) Events() chan tun.Event { return nil }
-func (t *sourceTun) File() *os.File         { return nil }
-func (t *sourceTun) Flush() error           { return nil }
-func (t *sourceTun) MTU() (int, error)      { return 1500, nil }
-func (t *sourceTun) Name() (string, error)  { return "source", nil }
-
-func (t *sourceTun) Write(b []byte, ofs int) (int, error) {
-	// Discard all writes
-	return len(b) - ofs, nil
-}
-
-func (t *sourceTun) Read(b []byte, ofs int) (int, error) {
-	// Continually generate "input" packets
-	n := t.traf.Generate(b, ofs)
-	if n == 0 {
-		return 0, io.EOF
-	}
-	return n, nil
-}
-
-type sinkTun struct {
-	logf logger.Logf
-	traf *TrafficGen
-}
-
-func (t *sinkTun) Close() error           { return nil }
-func (t *sinkTun) Events() chan tun.Event { return nil }
-func (t *sinkTun) File() *os.File         { return nil }
-func (t *sinkTun) Flush() error           { return nil }
-func (t *sinkTun) MTU() (int, error)      { return 1500, nil }
-func (t *sinkTun) Name() (string, error)  { return "sink", nil }
-
-func (t *sinkTun) Read(b []byte, ofs int) (int, error) {
-	// Never returns
-	select {}
-}
-
-func (t *sinkTun) Write(b []byte, ofs int) (int, error) {
-	// Count packets, but discard them
-	t.traf.GotPacket(b, ofs)
-	return len(b) - ofs, nil
-}

wgengine/magicsock/magicsock.go

@@ -1584,9 +1584,6 @@ func (c *Conn) receiveIPv6(b []byte) (int, conn.Endpoint, error) {
 	for {
 		n, ipp, err := c.pconn6.ReadFromNetaddr(b)
 		if err != nil {
-			if isPermanentNetError(err) {
-				health.ReceiveIPv6.Stop()
-			}
 			return 0, nil, err
 		}
 		if ep, ok := c.receiveIP(b[:n], ipp, &c.ippEndpoint6); ok {
@@ -1600,9 +1597,6 @@ func (c *Conn) receiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
 	for {
 		n, ipp, err := c.pconn4.ReadFromNetaddr(b)
 		if err != nil {
-			if isPermanentNetError(err) {
-				health.ReceiveIPv4.Stop()
-			}
 			return 0, nil, err
 		}
 		if ep, ok := c.receiveIP(b[:n], ipp, &c.ippEndpoint4); ok {
@@ -1663,7 +1657,6 @@ func (c *connBind) receiveDERP(b []byte) (n int, ep conn.Endpoint, err error) {
 		}
 		return n, ep, nil
 	}
-	health.ReceiveDERP.Stop()
 	return 0, nil, net.ErrClosed
 }
 
@@ -1734,18 +1727,6 @@ func (c *Conn) processDERPReadResult(dm derpReadResult, b []byte) (n int, ep con
 	return n, ep
 }
 
-// isPermanentNetError reports whether err is permanent.
-// It matches an equivalent check in wireguard-go's RoutineReceiveIncoming.
-func isPermanentNetError(err error) bool {
-	// Once this module requires Go 1.17, the comparison to net.ErrClosed can be removed.
-	// See https://github.com/golang/go/issues/45357.
-	if errors.Is(err, net.ErrClosed) {
-		return true
-	}
-	neterr, ok := err.(net.Error)
-	return ok && !neterr.Temporary()
-}
-
 // discoLogLevel controls the verbosity of discovery log messages.
 type discoLogLevel int
 
@@ -2430,11 +2411,8 @@ func (c *connBind) Open(ignoredPort uint16) ([]conn.ReceiveFunc, uint16, error)
 	}
 	c.closed = false
 	fns := []conn.ReceiveFunc{c.receiveIPv4, c.receiveDERP}
-	health.ReceiveIPv4.Open()
-	health.ReceiveDERP.Open()
 	if c.pconn6 != nil {
 		fns = append(fns, c.receiveIPv6)
-		health.ReceiveIPv6.Open()
 	}
 	// TODO: Combine receiveIPv4 and receiveIPv6 and receiveIP into a single
 	// closure that closes over a *RebindingUDPConn?
@@ -2456,15 +2434,12 @@ func (c *connBind) Close() error {
 	}
 	c.closed = true
 	// Unblock all outstanding receives.
-	health.ReceiveIPv4.Close()
 	c.pconn4.Close()
 	if c.pconn6 != nil {
-		health.ReceiveIPv6.Close()
 		c.pconn6.Close()
 	}
 	// Send an empty read result to unblock receiveDERP,
 	// which will then check connBind.Closed.
-	health.ReceiveDERP.Close()
 	c.derpRecvCh <- derpReadResult{}
 	return nil
 }

wgengine/netstack/netstack.go

@@ -477,7 +477,7 @@ func (ns *Impl) acceptUDP(r *udp.ForwarderRequest) {
 	var wq waiter.Queue
 	ep, err := r.CreateEndpoint(&wq)
 	if err != nil {
-		ns.logf("acceptUDP: could not create endpoint: %v", err)
+		ns.logf("Could not create endpoint, exiting")
 		return
 	}
 	localAddr, err := ep.GetLocalAddress()

wgengine/pendopen.go

@@ -190,25 +190,6 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
 		return
 	}
 	if ps == nil {
-		onlyZeroRoute := true // whether peerForIP returned n only because its /0 route matched
-		for _, r := range n.AllowedIPs {
-			if r.Bits != 0 && r.Contains(flow.Dst.IP) {
-				onlyZeroRoute = false
-				break
-			}
-		}
-		if onlyZeroRoute {
-			// This node was returned by peerForIP because
-			// its exit node /0 route(s) matched, but this
-			// might not be the exit node that's currently
-			// selected.  Rather than log misleading
-			// errors, just don't log at all for now.
-			// TODO(bradfitz): update this code to be
-			// exit-node-aware and make peerForIP return
-			// the node of the currently selected exit
-			// node.
-			return
-		}
 		e.logf("open-conn-track: timeout opening %v; target node %v in netmap but unknown to wireguard", flow, n.Key.ShortString())
 		return
 	}

wgengine/router/ifconfig_windows.go

@@ -713,68 +713,22 @@ func getInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses, family winipcfg.Addres
 	return
 }
 
-func getAllInterfaceRoutes(ifc *winipcfg.IPAdapterAddresses) ([]*winipcfg.RouteData, error) {
-	routes4, err := getInterfaceRoutes(ifc, windows.AF_INET)
-	if err != nil {
-		return nil, err
+// isSingleRouteInPrefixes reports whether r is a single-address
+// prefix that appears in pfxs.
+func isSingleRouteInPrefixes(r net.IPNet, pfxs []netaddr.IPPrefix) bool {
+	rr, ok := netaddr.FromStdIPNet(&r)
+	if !ok {
+		return false
 	}
-
-	routes6, err := getInterfaceRoutes(ifc, windows.AF_INET6)
-	if err != nil {
-		// TODO: what if v6 unavailable?
-		return nil, err
+	if !rr.IsSingleIP() {
+		return false
 	}
-	rd := make([]*winipcfg.RouteData, 0, len(routes4)+len(routes6))
-	for _, r := range routes4 {
-		rd = append(rd, &winipcfg.RouteData{
-			Destination: r.DestinationPrefix.IPNet(),
-			NextHop:     r.NextHop.IP(),
-			Metric:      r.Metric,
-		})
-	}
-	for _, r := range routes6 {
-		rd = append(rd, &winipcfg.RouteData{
-			Destination: r.DestinationPrefix.IPNet(),
-			NextHop:     r.NextHop.IP(),
-			Metric:      r.Metric,
-		})
-	}
-	return rd, nil
-}
-
-// filterRoutes removes routes that have been added by Windows and should not
-// be managed by us.
-func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netaddr.IPPrefix) []*winipcfg.RouteData {
-	ddm := make(map[netaddr.IPPrefix]bool)
-	for _, dd := range dontDelete {
-		// See issue 1448: we don't want to touch the routes added
-		// by Windows for our interface addresses.
-		ddm[dd] = true
-	}
-	for _, r := range routes {
-		// We don't want to touch broadcast routes that Windows adds.
-		nr, ok := netaddr.FromStdIPNet(&r.Destination)
-		if !ok {
-			continue
+	for _, pfx := range pfxs {
+		if pfx == rr {
+			return true
 		}
-		if nr.IsSingleIP() {
-			continue
-		}
-		lastIP := nr.Range().To
-		ddm[netaddr.IPPrefix{
-			IP:   lastIP,
-			Bits: lastIP.BitLen(),
-		}] = true
 	}
-	filtered := make([]*winipcfg.RouteData, 0, len(routes))
-	for _, r := range routes {
-		rr, ok := netaddr.FromStdIPNet(&r.Destination)
-		if ok && ddm[rr] {
-			continue
-		}
-		filtered = append(filtered, r)
-	}
-	return filtered
+	return false
 }
 
 // syncRoutes incrementally sets multiples routes on an interface.
@@ -782,11 +736,42 @@ func filterRoutes(routes []*winipcfg.RouteData, dontDelete []netaddr.IPPrefix) [
 // dontDelete is a list of interface address routes that the
 // synchronization logic should never delete.
 func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, dontDelete []netaddr.IPPrefix) error {
-	existingRoutes, err := getAllInterfaceRoutes(ifc)
+	routes4, err := getInterfaceRoutes(ifc, windows.AF_INET)
 	if err != nil {
 		return err
 	}
-	got := filterRoutes(existingRoutes, dontDelete)
+
+	routes6, err := getInterfaceRoutes(ifc, windows.AF_INET6)
+	if err != nil {
+		// TODO: what if v6 unavailable?
+		return err
+	}
+
+	got := make([]*winipcfg.RouteData, 0, len(routes4))
+	for _, r := range routes4 {
+		if isSingleRouteInPrefixes(r.DestinationPrefix.IPNet(), dontDelete) {
+			// See issue 1448: we don't want to touch the routes added
+			// by Windows for our interface addresses.
+			continue
+		}
+		got = append(got, &winipcfg.RouteData{
+			Destination: r.DestinationPrefix.IPNet(),
+			NextHop:     r.NextHop.IP(),
+			Metric:      r.Metric,
+		})
+	}
+	for _, r := range routes6 {
+		if isSingleRouteInPrefixes(r.DestinationPrefix.IPNet(), dontDelete) {
+			// See issue 1448: we don't want to touch the routes added
+			// by Windows for our interface addresses.
+			continue
+		}
+		got = append(got, &winipcfg.RouteData{
+			Destination: r.DestinationPrefix.IPNet(),
+			NextHop:     r.NextHop.IP(),
+			Metric:      r.Metric,
+		})
+	}
 
 	add, del := deltaRouteData(got, want)
 
@@ -798,7 +783,6 @@ func syncRoutes(ifc *winipcfg.IPAdapterAddresses, want []*winipcfg.RouteData, do
 			if dstStr == "169.254.255.255/32" {
 				// Issue 785. Ignore these routes
 				// failing to delete. Harmless.
-				// TODO(maisem): do we still need this?
 				continue
 			}
 			errs = append(errs, fmt.Errorf("deleting route %v: %w", dstStr, err))

wgengine/router/ifconfig_windows_test.go

@@ -193,14 +193,6 @@ func TestDeltaNets(t *testing.T) {
 	}
 }
 
-func formatRouteData(rds []*winipcfg.RouteData) string {
-	var b strings.Builder
-	for _, rd := range rds {
-		b.WriteString(fmt.Sprintf("%+v", rd))
-	}
-	return b.String()
-}
-
 func equalRouteDatas(a, b []*winipcfg.RouteData) bool {
 	if len(a) != len(b) {
 		return false
@@ -213,43 +205,6 @@ func equalRouteDatas(a, b []*winipcfg.RouteData) bool {
 	return true
 }
 
-func TestFilterRoutes(t *testing.T) {
-	var h0 net.IP
-	in := []*winipcfg.RouteData{
-		// LinkLocal and Loopback routes.
-		{*ipnet4("169.254.0.0", 16), h0, 1},
-		{*ipnet4("169.254.255.255", 32), h0, 1},
-		{*ipnet4("127.0.0.0", 8), h0, 1},
-		{*ipnet4("127.255.255.255", 32), h0, 1},
-		// Local LAN routes.
-		{*ipnet4("192.168.0.0", 24), h0, 1},
-		{*ipnet4("192.168.0.255", 32), h0, 1},
-		{*ipnet4("192.168.1.0", 25), h0, 1},
-		{*ipnet4("192.168.1.127", 32), h0, 1},
-		// Some random other route.
-		{*ipnet4("192.168.2.23", 32), h0, 1},
-		// Our own tailscale address.
-		{*ipnet4("100.100.100.100", 32), h0, 1},
-		// Other tailscale addresses.
-		{*ipnet4("100.100.100.101", 32), h0, 1},
-		{*ipnet4("100.100.100.102", 32), h0, 1},
-	}
-	want := []*winipcfg.RouteData{
-		{*ipnet4("169.254.0.0", 16), h0, 1},
-		{*ipnet4("127.0.0.0", 8), h0, 1},
-		{*ipnet4("192.168.0.0", 24), h0, 1},
-		{*ipnet4("192.168.1.0", 25), h0, 1},
-		{*ipnet4("192.168.2.23", 32), h0, 1},
-		{*ipnet4("100.100.100.101", 32), h0, 1},
-		{*ipnet4("100.100.100.102", 32), h0, 1},
-	}
-
-	got := filterRoutes(in, mustCIDRs("100.100.100.100/32"))
-	if !equalRouteDatas(got, want) {
-		t.Errorf("\ngot:  %v\n  want: %v\n", formatRouteData(got), formatRouteData(want))
-	}
-}
-
 func TestDeltaRouteData(t *testing.T) {
 	var h0 net.IP
 	h1 := net.ParseIP("99.99.99.99")
@@ -277,9 +232,9 @@ func TestDeltaRouteData(t *testing.T) {
 	}
 
 	if !equalRouteDatas(add, wantAdd) {
-		t.Errorf("add:\n   got: %v\n  want: %v\n", formatRouteData(add), formatRouteData(wantAdd))
+		t.Errorf("add:\n   got: %v\n  want: %v\n", add, wantAdd)
 	}
 	if !equalRouteDatas(del, wantDel) {
-		t.Errorf("del:\n   got: %v\n  want: %v\n", formatRouteData(del), formatRouteData(wantDel))
+		t.Errorf("del:\n   got: %v\n  want: %v\n", del, wantDel)
 	}
 }

wgengine/router/router_linux_test.go

@@ -20,6 +20,22 @@ import (
 	"inet.af/netaddr"
 )
 
+func mustCIDR(s string) netaddr.IPPrefix {
+	pfx, err := netaddr.ParseIPPrefix(s)
+	if err != nil {
+		panic(err)
+	}
+	return pfx
+}
+
+func mustCIDRs(ss ...string) []netaddr.IPPrefix {
+	var ret []netaddr.IPPrefix
+	for _, s := range ss {
+		ret = append(ret, mustCIDR(s))
+	}
+	return ret
+}
+
 func TestRouterStates(t *testing.T) {
 	basic := `
 ip rule add -4 pref 5210 fwmark 0x80000 table main

wgengine/router/router_test.go

@@ -1,15 +0,0 @@
-// Copyright (c) 2021 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 router
-
-import "inet.af/netaddr"
-
-func mustCIDRs(ss ...string) []netaddr.IPPrefix {
-	var ret []netaddr.IPPrefix
-	for _, s := range ss {
-		ret = append(ret, netaddr.MustParseIPPrefix(s))
-	}
-	return ret
-}

wgengine/wgcfg/nmcfg/nmcfg.go

@@ -6,7 +6,6 @@
 package nmcfg
 
 import (
-	"bytes"
 	"fmt"
 	"net"
 	"strconv"
@@ -61,11 +60,6 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 		Peers:      make([]wgcfg.Peer, 0, len(nm.Peers)),
 	}
 
-	// Logging buffers
-	skippedUnselected := new(bytes.Buffer)
-	skippedIPs := new(bytes.Buffer)
-	skippedSubnets := new(bytes.Buffer)
-
 	for _, peer := range nm.Peers {
 		if controlclient.Debug.OnlyDisco && peer.DiscoKey.IsZero() {
 			continue
@@ -98,23 +92,16 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 					continue
 				}
 				didExitNodeWarn = true
-				if skippedUnselected.Len() > 0 {
-					skippedUnselected.WriteString(", ")
-				}
-				fmt.Fprintf(skippedUnselected, "%q (%v)", nodeDebugName(peer), peer.Key.ShortString())
+				logf("[v1] wgcfg: skipping unselected default route from %q (%v)", nodeDebugName(peer), peer.Key.ShortString())
 				continue
 			} else if allowedIP.IsSingleIP() && tsaddr.IsTailscaleIP(allowedIP.IP) && (flags&netmap.AllowSingleHosts) == 0 {
-				if skippedIPs.Len() > 0 {
-					skippedIPs.WriteString(", ")
-				}
-				fmt.Fprintf(skippedIPs, "%v from %q (%v)", allowedIP.IP, nodeDebugName(peer), peer.Key.ShortString())
+				logf("[v1] wgcfg: skipping node IP %v from %q (%v)",
+					allowedIP.IP, nodeDebugName(peer), peer.Key.ShortString())
 				continue
 			} else if cidrIsSubnet(peer, allowedIP) {
 				if (flags & netmap.AllowSubnetRoutes) == 0 {
-					if skippedSubnets.Len() > 0 {
-						skippedSubnets.WriteString(", ")
-					}
-					fmt.Fprintf(skippedSubnets, "%v from %q (%v)", allowedIP, nodeDebugName(peer), peer.Key.ShortString())
+					logf("[v1] wgcfg: not accepting subnet route %v from %q (%v)",
+						allowedIP, nodeDebugName(peer), peer.Key.ShortString())
 					continue
 				}
 			}
@@ -122,16 +109,6 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 		}
 	}
 
-	if skippedUnselected.Len() > 0 {
-		logf("[v1] wgcfg: skipped unselected default routes from: %s", skippedUnselected.Bytes())
-	}
-	if skippedIPs.Len() > 0 {
-		logf("[v1] wgcfg: skipped node IPs: %s", skippedIPs)
-	}
-	if skippedSubnets.Len() > 0 {
-		logf("[v1] wgcfg: did not accept subnet routes: %s", skippedSubnets)
-	}
-
 	return cfg, nil
 }
 

wgengine/wglog/wglog.go

@@ -31,7 +31,7 @@ func NewLogger(logf logger.Logf) *Logger {
 
 	wrapper := func(format string, args ...interface{}) {
 		msg := fmt.Sprintf(format, args...)
-		if strings.Contains(msg, "Routine:") && !strings.Contains(msg, "receive incoming") {
+		if strings.Contains(msg, "Routine:") {
 			// wireguard-go logs as it starts and stops routines.
 			// Drop those; there are a lot of them, and they're just noise.
 			return