optimize: calc size, use values

note that this splits a mutex access in two

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
+`