optimize: calc size, use values

note that this splits a mutex access in two

cmd/tailscale/cli/admin.go

@@ -1,155 +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 cli
-
-import (
-	"context"
-	"errors"
-	"flag"
-	"fmt"
-	"io"
-	"net/http"
-	"os"
-	"strings"
-
-	"github.com/dsnet/golib/jsonfmt"
-	"github.com/peterbourgon/ff/v2/ffcli"
-)
-
-const tailscaleAPIURL = "https://api.tailscale.com/api"
-
-var adminCmd = &ffcli.Command{
-	Name:       "admin",
-	ShortUsage: "admin <subcommand> [command flags]",
-	ShortHelp:  "Administrate a tailnet",
-	LongHelp: strings.TrimSpace(`
-The "tailscale admin" command administrates a tailnet through the CLI.
-It is a wrapper over the RESTful API served at ` + tailscaleAPIURL + `.
-See https://github.com/tailscale/tailscale/blob/main/api.md for more information
-about the API itself.
-
-In order for the "admin" command to call the API, it needs an API key,
-which is specified by setting the TAILSCALE_API_KEY environment variable.
-Also, to easy usage, the tailnet to administrate can be specified through the
-TAILSCALE_NET_NAME environment variable, or specified with the -tailnet flag.
-
-Visit https://login.tailscale.com/admin/settings/authkeys in order to obtain
-an API key.
-`),
-	FlagSet: (func() *flag.FlagSet {
-		fs := flag.NewFlagSet("status", flag.ExitOnError)
-		// TODO(dsnet): Can we determine the default tailnet from what this
-		// device is currently part of? Alternatively, when add specific logic
-		// to handle auth keys, we can always associate a given key with a
-		// specific tailnet.
-		fs.StringVar(&adminArgs.tailnet, "tailnet", os.Getenv("TAILSCALE_NET_NAME"), "which tailnet to administrate")
-		return fs
-	})(),
-	// TODO(dsnet): Handle users, groups, dns.
-	Subcommands: []*ffcli.Command{{
-		Name:       "acl",
-		ShortUsage: "acl <subcommand> [command flags]",
-		ShortHelp:  "Manage the ACL for a tailnet",
-		// TODO(dsnet): Handle preview.
-		Subcommands: []*ffcli.Command{{
-			Name:       "get",
-			ShortUsage: "get",
-			ShortHelp:  "Downloads the HuJSON ACL file to stdout",
-			Exec:       checkAdminKey(runAdminACLGet),
-		}, {
-			Name:       "set",
-			ShortUsage: "set",
-			ShortHelp:  "Uploads the HuJSON ACL file from stdin",
-			Exec:       checkAdminKey(runAdminACLSet),
-		}},
-		Exec: runHelp,
-	}, {
-		Name:       "devices",
-		ShortUsage: "devices <subcommand> [command flags]",
-		ShortHelp:  "Manage devices in a tailnet",
-		Subcommands: []*ffcli.Command{{
-			Name:       "list",
-			ShortUsage: "list",
-			ShortHelp:  "List all devices in a tailnet",
-			Exec:       checkAdminKey(runAdminDevicesList),
-		}, {
-			Name:       "get",
-			ShortUsage: "get <id>",
-			ShortHelp:  "Get information about a specific device",
-			Exec:       checkAdminKey(runAdminDevicesGet),
-		}},
-		Exec: runHelp,
-	}},
-	Exec: runHelp,
-}
-
-var adminArgs struct {
-	tailnet string // which tailnet to operate upon
-}
-
-func checkAdminKey(f func(context.Context, string, []string) error) func(context.Context, []string) error {
-	return func(ctx context.Context, args []string) error {
-		// TODO(dsnet): We should have a subcommand or flag to manage keys.
-		// Use of an environment variable is a temporary hack.
-		key := os.Getenv("TAILSCALE_API_KEY")
-		if !strings.HasPrefix(key, "tskey-") {
-			return errors.New("no API key specified")
-		}
-		return f(ctx, key, args)
-	}
-}
-
-func runAdminACLGet(ctx context.Context, key string, args []string) error {
-	if len(args) > 0 {
-		return flag.ErrHelp
-	}
-	return adminCallAPI(ctx, key, http.MethodGet, "/v2/tailnet/"+adminArgs.tailnet+"/acl", nil, os.Stdout)
-}
-
-func runAdminACLSet(ctx context.Context, key string, args []string) error {
-	if len(args) > 0 {
-		return flag.ErrHelp
-	}
-	return adminCallAPI(ctx, key, http.MethodPost, "/v2/tailnet/"+adminArgs.tailnet+"/acl", os.Stdin, os.Stdout)
-}
-
-func runAdminDevicesList(ctx context.Context, key string, args []string) error {
-	if len(args) > 0 {
-		return flag.ErrHelp
-	}
-	return adminCallAPI(ctx, key, http.MethodGet, "/v2/tailnet/"+adminArgs.tailnet+"/devices", nil, os.Stdout)
-}
-
-func runAdminDevicesGet(ctx context.Context, key string, args []string) error {
-	if len(args) != 1 {
-		return flag.ErrHelp
-	}
-	return adminCallAPI(ctx, key, http.MethodGet, "/v2/device/"+args[0], nil, os.Stdout)
-}
-
-func adminCallAPI(ctx context.Context, key, method, path string, in io.Reader, out io.Writer) error {
-	req, err := http.NewRequestWithContext(ctx, method, tailscaleAPIURL+path, in)
-	req.SetBasicAuth(key, "")
-	if err != nil {
-		return fmt.Errorf("failed to create request: %w", err)
-	}
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return fmt.Errorf("failed to send HTTP request: %w", err)
-	}
-	defer resp.Body.Close()
-
-	b, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return fmt.Errorf("failed to receive HTTP response: %w", err)
-	}
-	b, err = jsonfmt.Format(b)
-	if err != nil {
-		return fmt.Errorf("failed to format JSON response: %w", err)
-	}
-	_, err = out.Write(b)
-	return err
-
-}

cmd/tailscale/cli/cli.go

@@ -76,10 +76,6 @@ func ActLikeCLI() bool {
 	return false
 }
 
-func runHelp(context.Context, []string) error {
-	return flag.ErrHelp
-}
-
 // Run runs the CLI. The args do not include the binary name.
 func Run(args []string) error {
 	if len(args) == 1 && (args[0] == "-V" || args[0] == "--version") {
@@ -103,7 +99,6 @@ change in the future.
 			upCmd,
 			downCmd,
 			logoutCmd,
-			adminCmd,
 			netcheckCmd,
 			ipCmd,
 			statusCmd,
@@ -114,7 +109,7 @@ change in the future.
 			bugReportCmd,
 		},
 		FlagSet:   rootfs,
-		Exec:      runHelp,
+		Exec:      func(context.Context, []string) error { return flag.ErrHelp },
 		UsageFunc: usageFunc,
 	}
 	for _, c := range rootCmd.Subcommands {

cmd/tailscale/depaware.txt

@@ -3,7 +3,6 @@ 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/internal/common                 from github.com/alexbrainman/sspi/negotiate
    W 💣 github.com/alexbrainman/sspi/negotiate                       from tailscale.com/net/tshttpproxy
-        github.com/dsnet/golib/jsonfmt                               from tailscale.com/cmd/tailscale/cli
         github.com/kballard/go-shellquote                            from tailscale.com/cmd/tailscale/cli
      💣 github.com/mitchellh/go-ps                                   from tailscale.com/cmd/tailscale/cli
         github.com/peterbourgon/ff/v2                                from github.com/peterbourgon/ff/v2/ffcli

go.mod

@@ -9,7 +9,6 @@ require (
 	github.com/coreos/go-iptables v0.6.0
 	github.com/creack/pty v1.1.9
 	github.com/dave/jennifer v1.4.1
-	github.com/dsnet/golib/jsonfmt v1.0.0
 	github.com/frankban/quicktest v1.13.0
 	github.com/gliderlabs/ssh v0.3.2
 	github.com/go-multierror/multierror v1.0.2

go.sum

@@ -96,8 +96,6 @@ github.com/denis-tingajkin/go-header v0.3.1 h1:ymEpSiFjeItCy1FOP+x0M2KdCELdEAHUs
 github.com/denis-tingajkin/go-header v0.3.1/go.mod h1:sq/2IxMhaZX+RRcgHfCRx/m0M5na0fBt4/CRe7Lrji0=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/dgryski/go-sip13 v0.0.0-20181026042036-e10d5fee7954/go.mod h1:vAd38F8PWV+bWy6jNmig1y/TA+kYO4g3RSRF0IAv0no=
-github.com/dsnet/golib/jsonfmt v1.0.0 h1:qrfqvbua2pQvj+dt3BcxEwwqy86F7ri2NdLQLm6g2TQ=
-github.com/dsnet/golib/jsonfmt v1.0.0/go.mod h1:C0/DCakJBCSVJ3mWBjDVzym2Wf7w5hpvwgHCwI/M7/w=
 github.com/dvyukov/go-fuzz v0.0.0-20210103155950-6a8e9d1f2415/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
 github.com/emirpasic/gods v1.12.0 h1:QAUIPSaCu4G+POclxeqb3F+WPpdKqFGlw36+yOzGlrg=
 github.com/emirpasic/gods v1.12.0/go.mod h1:YfzfFFoVP/catgzJb4IKIqXjX78Ha8FMSDh3ymbK86o=

net/dns/resolver/forwarder.go

@@ -10,7 +10,6 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
-	"hash/crc32"
 	"io"
 	"io/ioutil"
 	"math/rand"
@@ -65,44 +64,13 @@ func getTxID(packet []byte) txid {
 	}
 
 	dnsid := binary.BigEndian.Uint16(packet[0:2])
-	qcount := binary.BigEndian.Uint16(packet[4:6])
-	if qcount == 0 {
-		return txid(dnsid)
-	}
-
-	offset := headerBytes
-	for i := uint16(0); i < qcount; i++ {
-		// Note: this relies on the fact that names are not compressed in questions,
-		// so they are guaranteed to end with a NUL byte.
-		//
-		// Justification:
-		// RFC 1035 doesn't seem to explicitly prohibit compressing names in questions,
-		// but this is exceedingly unlikely to be done in practice. A DNS request
-		// with multiple questions is ill-defined (which questions do the header flags apply to?)
-		// and a single question would have to contain a pointer to an *answer*,
-		// which would be excessively smart, pointless (an answer can just as well refer to the question)
-		// and perhaps even prohibited: a draft RFC (draft-ietf-dnsind-local-compression-05) states:
-		//
-		// > It is important that these pointers always point backwards.
-		//
-		// This is said in summarizing RFC 1035, although that phrase does not appear in the original RFC.
-		// Additionally, (https://cr.yp.to/djbdns/notes.html) states:
-		//
-		// > The precise rule is that a name can be compressed if it is a response owner name,
-		// > the name in NS data, the name in CNAME data, the name in PTR data, the name in MX data,
-		// > or one of the names in SOA data.
-		namebytes := bytes.IndexByte(packet[offset:], 0)
-		// ... | name | NUL | type | class
-		//        ??     1      2      2
-		offset = offset + namebytes + 5
-		if len(packet) < offset {
-			// Corrupt packet; don't crash.
-			return txid(dnsid)
-		}
-	}
-
-	hash := crc32.ChecksumIEEE(packet[headerBytes:offset])
-	return (txid(hash) << 32) | txid(dnsid)
+	// Previously, we hashed the question and combined it with the original txid
+	// which was useful when concurrent queries were multiplexed on a single
+	// local source port. We encountered some situations where the DNS server
+	// canonicalizes the question in the response (uppercase converted to
+	// lowercase in this case), which resulted in responses that we couldn't
+	// match to the original request due to hash mismatches.
+	return txid(dnsid)
 }
 
 // clampEDNSSize attempts to limit the maximum EDNS response size. This is not

net/dns/resolver/tsdns_server_test.go

@@ -6,6 +6,7 @@ package resolver
 
 import (
 	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/miekg/dns"
@@ -66,6 +67,58 @@ func resolveToIP(ipv4, ipv6 netaddr.IP, ns string) dns.HandlerFunc {
 	}
 }
 
+// resolveToIPLowercase returns a handler function which canonicalizes responses
+// by lowercasing the question and answer names, and responds
+// to queries of type A it receives with an A record containing ipv4,
+// to queries of type AAAA with an AAAA record containing ipv6,
+// to queries of type NS with an NS record containg name.
+func resolveToIPLowercase(ipv4, ipv6 netaddr.IP, ns string) dns.HandlerFunc {
+	return func(w dns.ResponseWriter, req *dns.Msg) {
+		m := new(dns.Msg)
+		m.SetReply(req)
+
+		if len(req.Question) != 1 {
+			panic("not a single-question request")
+		}
+		m.Question[0].Name = strings.ToLower(m.Question[0].Name)
+		question := req.Question[0]
+
+		var ans dns.RR
+		switch question.Qtype {
+		case dns.TypeA:
+			ans = &dns.A{
+				Hdr: dns.RR_Header{
+					Name:   question.Name,
+					Rrtype: dns.TypeA,
+					Class:  dns.ClassINET,
+				},
+				A: ipv4.IPAddr().IP,
+			}
+		case dns.TypeAAAA:
+			ans = &dns.AAAA{
+				Hdr: dns.RR_Header{
+					Name:   question.Name,
+					Rrtype: dns.TypeAAAA,
+					Class:  dns.ClassINET,
+				},
+				AAAA: ipv6.IPAddr().IP,
+			}
+		case dns.TypeNS:
+			ans = &dns.NS{
+				Hdr: dns.RR_Header{
+					Name:   question.Name,
+					Rrtype: dns.TypeNS,
+					Class:  dns.ClassINET,
+				},
+				Ns: ns,
+			}
+		}
+
+		m.Answer = append(m.Answer, ans)
+		w.WriteMsg(m)
+	}
+}
+
 // resolveToTXT returns a handler function which responds to queries of type TXT
 // it receives with the strings in txts.
 func resolveToTXT(txts []string, ednsMaxSize uint16) dns.HandlerFunc {

net/dns/resolver/tsdns_test.go

@@ -440,6 +440,8 @@ func TestDelegate(t *testing.T) {
 	records := []interface{}{
 		"test.site.",
 		resolveToIP(testipv4, testipv6, "dns.test.site."),
+		"LCtesT.SiTe.",
+		resolveToIPLowercase(testipv4, testipv6, "dns.test.site."),
 		"nxdomain.site.", resolveToNXDOMAIN,
 		"small.txt.", resolveToTXT(smallTXT, noEdns),
 		"smalledns.txt.", resolveToTXT(smallTXT, 512),
@@ -485,6 +487,21 @@ func TestDelegate(t *testing.T) {
 			dnspacket("test.site.", dns.TypeNS, noEdns),
 			dnsResponse{name: "dns.test.site.", rcode: dns.RCodeSuccess},
 		},
+		{
+			"ipv4",
+			dnspacket("LCtesT.SiTe.", dns.TypeA, noEdns),
+			dnsResponse{ip: testipv4, rcode: dns.RCodeSuccess},
+		},
+		{
+			"ipv6",
+			dnspacket("LCtesT.SiTe.", dns.TypeAAAA, noEdns),
+			dnsResponse{ip: testipv6, rcode: dns.RCodeSuccess},
+		},
+		{
+			"ns",
+			dnspacket("LCtesT.SiTe.", dns.TypeNS, noEdns),
+			dnsResponse{name: "dns.test.site.", rcode: dns.RCodeSuccess},
+		},
 		{
 			"nxdomain",
 			dnspacket("nxdomain.site.", dns.TypeA, noEdns),

wgengine/userspace.go

@@ -921,11 +921,6 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 		errc <- err
 	}()
 
-	pp := make(map[wgkey.Key]*ipnstate.PeerStatusLite)
-	p := &ipnstate.PeerStatusLite{}
-
-	var hst1, hst2, n int64
-
 	br := e.statusBufioReader
 	if br != nil {
 		br.Reset(pr)
@@ -933,6 +928,29 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 		br = bufio.NewReaderSize(pr, 1<<10)
 		e.statusBufioReader = br
 	}
+
+	peers, err := e.getPeerStatusLite(br)
+	if err != nil {
+		return nil, err
+	}
+	if err := <-errc; err != nil {
+		return nil, fmt.Errorf("IpcGetOperation: %v", err)
+	}
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	return &Status{
+		LocalAddrs: append([]tailcfg.Endpoint(nil), e.endpoints...),
+		Peers:      peers,
+		DERPs:      derpConns,
+	}, nil
+}
+
+func (e *userspaceEngine) getPeerStatusLite(br *bufio.Reader) ([]ipnstate.PeerStatusLite, error) {
+	pp := make(map[wgkey.Key]ipnstate.PeerStatusLite)
+	var p ipnstate.PeerStatusLite
+
+	var hst1, hst2, n int64
+
 	for {
 		line, err := br.ReadSlice('\n')
 		if err == io.EOF {
@@ -954,11 +972,10 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 			if err != nil {
 				return nil, fmt.Errorf("IpcGetOperation: invalid key in line %q", line)
 			}
-			p = &ipnstate.PeerStatusLite{}
-			pp[wgkey.Key(pk)] = p
-
-			key := tailcfg.NodeKey(pk)
-			p.NodeKey = key
+			if !p.NodeKey.IsZero() {
+				pp[wgkey.Key(p.NodeKey)] = p
+			}
+			p = ipnstate.PeerStatusLite{NodeKey: tailcfg.NodeKey(pk)}
 		case "rx_bytes":
 			n, err = mem.ParseInt(v, 10, 64)
 			p.RxBytes = n
@@ -986,25 +1003,29 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 			} // else leave at time.IsZero()
 		}
 	}
-	if err := <-errc; err != nil {
-		return nil, fmt.Errorf("IpcGetOperation: %v", err)
+	if !p.NodeKey.IsZero() {
+		pp[wgkey.Key(p.NodeKey)] = p
 	}
 
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
-	var peers []ipnstate.PeerStatusLite
+	// Do two passes, one to calculate size and the other to populate.
+	// This code is sensitive to allocations.
+	npeers := 0
 	for _, pk := range e.peerSequence {
-		if p, ok := pp[pk]; ok { // ignore idle ones not in wireguard-go's config
-			peers = append(peers, *p)
+		if _, ok := pp[pk]; ok { // ignore idle ones not in wireguard-go's config
+			npeers++
 		}
 	}
 
-	return &Status{
-		LocalAddrs: append([]tailcfg.Endpoint(nil), e.endpoints...),
-		Peers:      peers,
-		DERPs:      derpConns,
-	}, nil
+	peers := make([]ipnstate.PeerStatusLite, 0, npeers)
+	for _, pk := range e.peerSequence {
+		if p, ok := pp[pk]; ok { // ignore idle ones not in wireguard-go's config
+			peers = append(peers, p)
+		}
+	}
+	return peers, nil
 }
 
 func (e *userspaceEngine) RequestStatus() {

wgengine/userspace_test.go

@@ -5,9 +5,11 @@
 package wgengine
 
 import (
+	"bufio"
 	"bytes"
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 
 	"go4.org/mem"
@@ -17,6 +19,7 @@ import (
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstime/mono"
 	"tailscale.com/types/key"
+	"tailscale.com/types/wgkey"
 	"tailscale.com/wgengine/router"
 	"tailscale.com/wgengine/wgcfg"
 )
@@ -252,3 +255,67 @@ func BenchmarkGenLocalAddrFunc(b *testing.B) {
 	})
 	b.Logf("x = %v", x)
 }
+
+func BenchmarkGetPeerStatusLite(b *testing.B) {
+	e := new(userspaceEngine)
+	br := bufio.NewReaderSize(nil, 1<<10)
+	parseKey := func(s string) wgkey.Key {
+		k, err := wgkey.ParseHex(s)
+		if err != nil {
+			b.Fatal(err)
+		}
+		return k
+	}
+	e.peerSequence = []wgkey.Key{
+		parseKey("0000000000000000000000000000000000000000000000000000000000000002"),
+		parseKey("0000000000000000000000000000000000000000000000000000000000000003"),
+		parseKey("0000000000000000000000000000000000000000000000000000000000000004"),
+	}
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		br.Reset(strings.NewReader(sampleStatus))
+		_, err := e.getPeerStatusLite(br)
+		if err != nil {
+			b.Fatal(err)
+		}
+	}
+}
+
+const sampleStatus = `
+private_key=0000000000000000000000000000000000000000000000000000000000000001
+listen_port=50818
+public_key=0000000000000000000000000000000000000000000000000000000000000002
+preshared_key=0000000000000000000000000000000000000000000000000000000000000000
+protocol_version=1
+endpoint={"pk":"0000000000000000000000000000000000000000000000000000000000000002","dk":"discokey:0000000000000000000000000000000000000000000000000000000000000000","ipp":["139.162.81.125:41641"]}
+last_handshake_time_sec=1628288511
+last_handshake_time_nsec=993469000
+tx_bytes=272
+rx_bytes=272
+persistent_keepalive_interval=25
+allowed_ip=100.100.100.101/32
+allowed_ip=::2/128
+public_key=0000000000000000000000000000000000000000000000000000000000000003
+preshared_key=0000000000000000000000000000000000000000000000000000000000000000
+protocol_version=1
+endpoint={"pk":"0000000000000000000000000000000000000000000000000000000000000003","dk":"discokey:c6c0a1488ac6cb9e0103d056ca51387e33513d577b985e77c6ea075f76b27d35","ipp":null}
+last_handshake_time_sec=1628288511
+last_handshake_time_nsec=615842000
+tx_bytes=1300
+rx_bytes=92
+persistent_keepalive_interval=0
+allowed_ip=100.100.100.102/32
+allowed_ip=::3/128
+public_key=0000000000000000000000000000000000000000000000000000000000000004
+preshared_key=0000000000000000000000000000000000000000000000000000000000000000
+protocol_version=1
+endpoint={"pk":"0000000000000000000000000000000000000000000000000000000000000004","dk":"discokey:c6c0a1488ac6cb9e0103d056ca51387e33513d577b985e77c6ea075f76b27d35","ipp":null}
+last_handshake_time_sec=1628288511
+last_handshake_time_nsec=615842000
+tx_bytes=1300
+rx_bytes=92
+persistent_keepalive_interval=0
+allowed_ip=100.100.100.103/32
+allowed_ip=::4/128
+`