net/isoping: add isoping package

Signed-off-by: Simeng He <simeng@tailscale.com>

.github/workflows/xe-experimental-vm-test.yml

@@ -21,7 +21,7 @@ jobs:
         uses: actions/checkout@v1
 
       - name: Download VM Images
-        run: go test ./tstest/integration/vms -run-vm-tests -run=Download -timeout=60m
+        run: go test ./tstest/integration/vms -run-vm-tests -run=Download -timeout=60m -no-s3
         env:
           XDG_CACHE_HOME: "/var/lib/ghrunner/cache"
 

build_docker.sh

@@ -25,7 +25,7 @@
 
 set -eu
 
-eval $(./version/version.sh)
+eval $(./build_dist.sh shellvars)
 
 docker build \
   --build-arg VERSION_LONG=$VERSION_LONG \

cmd/tailscale/cli/web.go

@@ -60,6 +60,14 @@ var webCmd = &ffcli.Command{
 	ShortUsage: "web [flags]",
 	ShortHelp:  "Run a web server for controlling Tailscale",
 
+	LongHelp: strings.TrimSpace(`
+"tailscale web" runs a webserver for controlling the Tailscale daemon.
+
+It's primarily intended for use on Synology, QNAP, and other
+NAS devices where a web interface is the natural place to control
+Tailscale, as opposed to a CLI or a native app.
+`),
+
 	FlagSet: (func() *flag.FlagSet {
 		webf := flag.NewFlagSet("web", flag.ExitOnError)
 		webf.StringVar(&webArgs.listen, "listen", "localhost:8088", "listen address; use port 0 for automatic")

net/isoping/constants.go

@@ -0,0 +1,24 @@
+// 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 isoping
+
+const (
+	MAGIC                           = 0x424c4950
+	DEFAULT_PORT                    = ":4948"
+	DEFAULT_PACKETS_PER_SEC float64 = 10.0
+	USEC_PER_CYCLE                  = (10 * 1000 * 1000)
+)
+
+// DIV takes two int64 divides the two and returns a float64
+func DIV(x, y int64) float64 {
+	if y == 0 {
+		return 0
+	}
+	return float64(x) / float64(y)
+}
+
+// DIFF takes the difference between two uint32s and returns int32
+func DIFF(x, y uint32) int32 {
+	return int32(int64(x) - int64(y))
+}

net/isoping/isoping.go

@@ -0,0 +1,254 @@
+// 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 isoping implements isoping in Go.
+package isoping
+
+import (
+	"bytes"
+	"encoding/binary"
+	"log"
+	"math"
+	"net"
+	"time"
+)
+
+type Packet struct {
+	Magic      uint32 // Magic number to reject bogus packets
+	Id         uint32 // Id is a sequential packet id number
+	Txtime     uint32 // Txtime is the transmitter's monotonic time when pkt was sent
+	Clockdiff  uint32 // Clockdiff is an estimate of (transmitter's clk) - (receiver's clk)
+	UsecPerPkt uint32 // Usec_per_pkt microseconds of delay between packets
+	NumLost    uint32 // Num_lost is the number of pkts transmitter expected to get but didn't
+	FirstAck   uint32 // First_ack is the starting index in acks[] circular buffer
+	Acks       [64]struct {
+		// txtime==0 for empty elements in this array.
+		Id     uint32 // Id field from a received packet
+		Rxtime uint32 // Rxtime is a receiver's monotonic time when pkt arrived
+	}
+}
+
+type Isoping struct {
+	ClockStartTime time.Time    // ClockStartTime is the time the program starts
+	IsServer       bool         // IsServer distinguishes if we are a server or client
+	Conn           *net.UDPConn // Conn is either the server or client's connection
+	Tx             Packet       // Tx is a Packet that will be sent
+	Rx             Packet       // Rx is a Packet that will be received
+	LastAckInfo    string       // LastAckInfo human readable format of latest ack
+	ListenAddr     *net.UDPAddr // ListenAddr is the address of the listener
+	RemoteAddr     *net.UDPAddr // RemtoteAddr remote UDP address we send to.
+	RxAddr         *net.UDPAddr // RxAddr keeps track of what address we are sending to
+	LastRxAddr     *net.UDPAddr // LastRxAddr keeps track of what we last used
+	Quiet          bool         // Option to show output or not
+
+	printsPerSec   float64
+	packetsPerSec  float64
+	usecPerPkt     int32
+	usecPerPrint   int32
+	nextTxId       uint32
+	nextRxId       uint32
+	nextRxackId    uint32
+	startRtxtime   uint32 // remote's txtime at startup
+	startRxtime    uint32 // local rxtime at startup
+	lastRxtime     uint32 // local rxtime of last received packet
+	minCycleRxdiff int32  // smallest packet delay seen this cycle
+	nextCycle      uint32 // time when next cycle begins
+	now            uint32 // current time
+	nextSend       uint32 // time when we'll send next pkt
+	numLost        uint32 // number of rx packets not received
+	nextTxackIndex int    // next array item to fill in tx.acks
+	lastPrint      uint32 // time of last packet printout
+	latTx          int64
+	latTxMin       int64
+	latTxMax       int64
+	latTxCount     int64
+	latTxSum       int64
+	latTxVarSum    int64
+
+	latRx       int64
+	latRxMin    int64
+	latRxMax    int64
+	latRxCount  int64
+	latRxSum    int64
+	latRxVarSum int64
+}
+
+// Incremental standard deviation calculation, without needing to know the
+// mean in advance.  See:
+// http://mathcentral.uregina.ca/QQ/database/QQ.09.02/carlos1.html
+func onePassStddev(sumsq, sum, count int64) float64 {
+	numer := (count * sumsq) - (sum * sum)
+	denom := count * (count - 1)
+	return math.Sqrt(DIV(numer, denom))
+}
+
+// UsecMonoTimeNow returns the monotonic number of microseconds since the program started.
+func (srv *Isoping) UsecMonoTimeNow() uint64 {
+	tn := time.Since(srv.ClockStartTime)
+	return uint64(tn.Microseconds())
+}
+
+// UsecMonoTime returns the monotonic number of microseconds since the program started, as a uint32.
+func (srv *Isoping) UsecMonoTime() uint32 {
+	return uint32(srv.UsecMonoTimeNow())
+}
+
+// initClock keeps track of when the server/client starts.
+// keeps the exact time and we can subtract from the time
+// to get monotonicClock values
+func (srv *Isoping) initClock() {
+	srv.ClockStartTime = time.Now()
+}
+
+// initClient sets the Isoping.Conn, to the address string otherwise
+// uses [::]:4948 as the default
+func (srv *Isoping) initClient(address string) {
+	srv.initClock()
+	srv.IsServer = false
+	udpaddr, err := net.ResolveUDPAddr("udp", address)
+	if err != nil {
+		log.Println(err)
+		addr := DEFAULT_PORT
+		udpaddr, err = net.ResolveUDPAddr("udp", addr)
+		if err != nil {
+			log.Println(err)
+			return
+		}
+		log.Printf("Address %v failed to resolve\n", address)
+	}
+
+	conn, err := net.DialUDP("udp", nil, udpaddr)
+	if err != nil {
+		log.Println(err)
+		return
+	}
+
+	srv.RemoteAddr = udpaddr
+	srv.Conn = conn
+}
+
+// initServer sets the Conn field of Isoping, for the listener side.
+func (srv *Isoping) initServer(port string) {
+	srv.initClock()
+	srv.IsServer = true
+	addr, err := net.ResolveUDPAddr("udp", port)
+	if err != nil {
+		log.Println(err)
+		return
+	}
+
+	srv.ListenAddr = addr
+	srv.Conn, err = net.ListenUDP("udp", addr)
+	if err != nil {
+		log.Printf("%v\n", err)
+		return
+	}
+}
+
+func NewInstance() *Isoping {
+	clockStartTime := time.Now()
+
+	packetsPerSec := DEFAULT_PACKETS_PER_SEC
+	printsPerSec := -1
+
+	usecPerPkt := int32(1e6 / packetsPerSec)
+	usecPerPrint := int32(0)
+	if usecPerPrint > 0 {
+		usecPerPrint = int32(1e6 / printsPerSec)
+	}
+	log.Println("UsecPerPkt : ", usecPerPkt)
+	log.Println("UsecPerPrint : ", usecPerPrint)
+
+	nextTxId := 1
+	nextRxId := 0
+
+	nextRxackId := 0
+	startRtxtime := 0
+	startRxtime := 0
+	lastRxtime := 0
+
+	minCycleRxdiff := 0
+	nextCycle := 0
+	nextSend := 0
+	nextTxackIndex := 0
+
+	LastAckInfo := ""
+	inst := &Isoping{
+		packetsPerSec:  packetsPerSec,
+		printsPerSec:   float64(printsPerSec),
+		usecPerPkt:     int32(1e6 / DEFAULT_PACKETS_PER_SEC),
+		usecPerPrint:   usecPerPrint,
+		nextTxId:       uint32(nextTxId),
+		nextRxId:       uint32(nextRxId),
+		nextRxackId:    uint32(nextRxackId),
+		startRtxtime:   uint32(startRtxtime),
+		startRxtime:    uint32(startRxtime),
+		lastRxtime:     uint32(lastRxtime),
+		minCycleRxdiff: int32(minCycleRxdiff),
+		nextCycle:      uint32(nextCycle),
+		nextSend:       uint32(nextSend),
+		nextTxackIndex: nextTxackIndex,
+		Tx:             Packet{},
+		Rx:             Packet{},
+		LastAckInfo:    LastAckInfo,
+		ClockStartTime: clockStartTime,
+
+		latTx:       0,
+		latTxMin:    0x7fffffff,
+		latTxMax:    0,
+		latTxCount:  0,
+		latTxSum:    0,
+		latTxVarSum: 0,
+		latRx:       0,
+		latRxMin:    0x7fffffff,
+		latRxMax:    0,
+		latRxCount:  0,
+		latRxSum:    0,
+		latRxVarSum: 0,
+	}
+
+	// Setup the clock functions after creating the fields
+	inst.now = inst.UsecMonoTime()
+	inst.lastPrint = inst.now - uint32(inst.usecPerPkt)
+	return inst
+}
+
+// generateInitialPacket generates the inital packet Tx
+func (srv *Isoping) generateInitialPacket() (*bytes.Buffer, error) {
+	srv.Tx.Magic = MAGIC
+	srv.Tx.Id = srv.nextTxId
+	srv.nextTxId++
+	srv.Tx.Txtime = srv.nextSend
+	srv.Tx.UsecPerPkt = uint32(srv.usecPerPkt)
+	srv.Tx.Clockdiff = 0
+	if srv.startRtxtime > 0 {
+		srv.Rx.Clockdiff = srv.startRtxtime - srv.startRxtime
+	}
+	srv.Tx.NumLost = srv.numLost
+	srv.Tx.FirstAck = uint32(srv.nextTxackIndex)
+
+	// Setup the Tx to be sent from either server of client
+	buf := new(bytes.Buffer)
+	return buf, binary.Write(buf, binary.BigEndian, srv.Tx)
+}
+
+// StartServer starts the Isoping Server with port
+// If no port is given, then starts with DEFAULT_PORT
+func (srv *Isoping) StartServer(port string) {
+	if port != "" {
+		srv.initServer(port)
+	} else {
+		srv.initServer(DEFAULT_PORT)
+	}
+}
+
+// StartServer starts the Isoping Client with port
+// If no port is given, then starts with DEFAULT_PORT
+func (srv *Isoping) StartClient(port string) {
+	if port != "" {
+		srv.initClient(port)
+	} else {
+		srv.initClient(DEFAULT_PORT)
+	}
+}

net/isoping/isoping_test.go

@@ -0,0 +1,107 @@
+// 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 isoping
+
+import (
+	"bytes"
+	"encoding/binary"
+	"math"
+	"net"
+	"strconv"
+	"testing"
+)
+
+// Tests if our stddev calculation is within reason
+// Must do some rounding to a certain significant digit
+// Currently only need 6 digits for the testing.
+func sigDigs(x float64, digs int) float64 {
+	return math.Round(x*math.Pow10(digs)) / math.Pow10(digs)
+}
+
+// TestOnepass_stddev tests if the function receives the same answer as in
+// the C implementation of this function.
+func TestOnepass_stddev(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name  string
+		input []int64
+		out   float64
+	}{
+
+		{
+			name:  "basic1",
+			input: []int64{12, 2, 3},
+			out:   2.309401,
+		},
+		{
+
+			name:  "basic2",
+			input: []int64{12023232232, 212, 321},
+			out:   6129.649279,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ttAns := sigDigs(onePassStddev(tt.input[0], tt.input[1], tt.input[2]), 6)
+			if ttAns != tt.out {
+				t.Errorf("got %v, expected %v", ttAns, tt.out)
+			}
+		})
+	}
+
+}
+
+// TestUstimeCast tests if casting was correct
+func TestUstimeCast(t *testing.T) {
+	t.Parallel()
+
+	var num uint64 = 11471851221
+	var expected uint32 = 2881916629
+	if uint32(num) != expected {
+		t.Errorf("expected %v, got : %v", expected, uint32(num))
+	}
+}
+
+// TestValidInitialPacket will send a packet via UDP, and check if it matches
+// The size and the Magic number field that needs to be equal.
+// This mocks the initial packet sent in Isoping.
+func TestValidInitialPacket(t *testing.T) {
+	t.Parallel()
+
+	server := NewInstance()
+	server.StartServer(":0")
+	defer server.Conn.Close()
+	serverPort := server.Conn.LocalAddr().(*net.UDPAddr).Port
+
+	client := NewInstance()
+	client.StartClient(":" + strconv.Itoa(serverPort))
+
+	buf, err := client.generateInitialPacket()
+	if err != nil {
+		t.Error(err)
+	}
+
+	// Client writes to the server, server tries to read it.
+	p := make([]byte, binary.Size(server.Rx))
+	if _, err := client.Conn.Write(buf.Bytes()); err != nil {
+		t.Error(err)
+	}
+
+	got, _, err := server.Conn.ReadFromUDP(p)
+	if err != nil {
+		t.Error(err)
+	}
+
+	buffer := bytes.NewBuffer(p)
+	defer buffer.Reset()
+
+	err = binary.Read(buffer, binary.BigEndian, &server.Rx)
+	if err != nil {
+		t.Error(err)
+	}
+
+	if got != binary.Size(server.Rx) || server.Rx.Magic != MAGIC {
+		t.Error("received Rx is not proper")
+	}
+}

net/tstun/wrap.go

@@ -71,6 +71,9 @@ type Wrapper struct {
 	// buffer stores the oldest unconsumed packet from tdev.
 	// It is made a static buffer in order to avoid allocations.
 	buffer [maxBufferSize]byte
+	// bufferConsumedMu protects bufferConsumed from concurrent sends and closes.
+	// It does not prevent send-after-close, only data races.
+	bufferConsumedMu sync.Mutex
 	// bufferConsumed synchronizes access to buffer (shared by Read and poll).
 	//
 	// Close closes bufferConsumed. There may be outstanding sends to bufferConsumed
@@ -80,6 +83,9 @@ type Wrapper struct {
 
 	// closed signals poll (by closing) when the device is closed.
 	closed chan struct{}
+	// outboundMu protects outbound from concurrent sends and closes.
+	// It does not prevent send-after-close, only data races.
+	outboundMu sync.Mutex
 	// outbound is the queue by which packets leave the TUN device.
 	//
 	// The directions are relative to the network, not the device:
@@ -174,8 +180,12 @@ func (t *Wrapper) Close() error {
 	var err error
 	t.closeOnce.Do(func() {
 		close(t.closed)
+		t.bufferConsumedMu.Lock()
 		close(t.bufferConsumed)
+		t.bufferConsumedMu.Unlock()
+		t.outboundMu.Lock()
 		close(t.outbound)
+		t.outboundMu.Unlock()
 		err = t.tdev.Close()
 	})
 	return err
@@ -275,7 +285,6 @@ func allowSendOnClosedChannel() {
 // This is needed because t.tdev.Read in general may block (it does on Windows),
 // so packets may be stuck in t.outbound if t.Read called t.tdev.Read directly.
 func (t *Wrapper) poll() {
-	defer allowSendOnClosedChannel() // for send to t.outbound
 	for range t.bufferConsumed {
 		var n int
 		var err error
@@ -293,10 +302,28 @@ func (t *Wrapper) poll() {
 			}
 			n, err = t.tdev.Read(t.buffer[:], PacketStartOffset)
 		}
-		t.outbound <- tunReadResult{data: t.buffer[PacketStartOffset : PacketStartOffset+n], err: err}
+		t.sendOutbound(tunReadResult{data: t.buffer[PacketStartOffset : PacketStartOffset+n], err: err})
 	}
 }
 
+// sendBufferConsumed does t.bufferConsumed <- struct{}{}.
+// It protects against any panics or data races that that send could cause.
+func (t *Wrapper) sendBufferConsumed() {
+	defer allowSendOnClosedChannel()
+	t.bufferConsumedMu.Lock()
+	defer t.bufferConsumedMu.Unlock()
+	t.bufferConsumed <- struct{}{}
+}
+
+// sendOutbound does t.outboundMu <- r.
+// It protects against any panics or data races that that send could cause.
+func (t *Wrapper) sendOutbound(r tunReadResult) {
+	defer allowSendOnClosedChannel()
+	t.outboundMu.Lock()
+	defer t.outboundMu.Unlock()
+	t.outbound <- r
+}
+
 var magicDNSIPPort = netaddr.MustParseIPPort("100.100.100.100:0")
 
 func (t *Wrapper) filterOut(p *packet.Parsed) filter.Response {
@@ -357,7 +384,6 @@ func (t *Wrapper) Read(buf []byte, offset int) (int, error) {
 	if res.err != nil {
 		return 0, res.err
 	}
-	defer allowSendOnClosedChannel() // for send to t.bufferConsumed
 	pkt := res.data
 	n := copy(buf[offset:], pkt)
 	// t.buffer has a fixed location in memory.
@@ -366,7 +392,7 @@ func (t *Wrapper) Read(buf []byte, offset int) (int, error) {
 	isInjectedPacket := &pkt[0] != &t.buffer[PacketStartOffset]
 	if !isInjectedPacket {
 		// We are done with t.buffer. Let poll re-use it.
-		t.bufferConsumed <- struct{}{}
+		t.sendBufferConsumed()
 	}
 
 	p := parsedPacketPool.Get().(*packet.Parsed)
@@ -583,8 +609,7 @@ func (t *Wrapper) InjectOutbound(packet []byte) error {
 	if len(packet) == 0 {
 		return nil
 	}
-	defer allowSendOnClosedChannel() // for send to t.outbound
-	t.outbound <- tunReadResult{data: packet}
+	t.sendOutbound(tunReadResult{data: packet})
 	return nil
 }
 

tailcfg/derpmap.go

@@ -48,6 +48,9 @@ type DERPRegion struct {
 	//
 	// RegionIDs must be non-zero, positive, and guaranteed to fit
 	// in a JavaScript number.
+	//
+	// RegionIDs in range 900-999 are reserved for end users to run their
+	// own DERP nodes.
 	RegionID int
 
 	// RegionCode is a short name for the region. It's usually a popular

tstest/integration/gen_deps.go

@@ -0,0 +1,53 @@
+// 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 ignore
+
+package main
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"log"
+	"os/exec"
+)
+
+func main() {
+	var x struct {
+		Imports []string
+	}
+	j, err := exec.Command("go", "list", "-json", "tailscale.com/cmd/tailscaled").Output()
+	if err != nil {
+		log.Fatal(err)
+	}
+	if err := json.Unmarshal(j, &x); err != nil {
+		log.Fatal(err)
+	}
+	var out bytes.Buffer
+	out.WriteString(`// 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.
+
+// Code generated by gen_deps.go; DO NOT EDIT.
+
+package integration
+
+import (
+	// And depend on a bunch of tailscaled innards, for Go's test caching.
+	// Otherwise cmd/go never sees that we depend on these packages'
+	// transitive deps when we run "go install tailscaled" in a child
+	// process and can cache a prior success when a dependency changes.
+`)
+	for _, dep := range x.Imports {
+		fmt.Fprintf(&out, "\t_ %q\n", dep)
+	}
+	fmt.Fprintf(&out, ")\n")
+
+	err = ioutil.WriteFile("tailscaled_deps_test.go", out.Bytes(), 0644)
+	if err != nil {
+		log.Fatal(err)
+	}
+}

tstest/integration/integration_test.go

@@ -4,6 +4,8 @@
 
 package integration
 
+//go:generate go run gen_deps.go
+
 import (
 	"bytes"
 	"context"

tstest/integration/tailscaled_deps_test.go

@@ -0,0 +1,58 @@
+// 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.
+
+// Code generated by gen_deps.go; DO NOT EDIT.
+
+package integration
+
+import (
+	// And depend on a bunch of tailscaled innards, for Go's test caching.
+	// Otherwise cmd/go never sees that we depend on these packages'
+	// transitive deps when we run "go install tailscaled" in a child
+	// process and can cache a prior success when a dependency changes.
+	_ "context"
+	_ "crypto/tls"
+	_ "encoding/json"
+	_ "errors"
+	_ "flag"
+	_ "fmt"
+	_ "github.com/go-multierror/multierror"
+	_ "io"
+	_ "io/ioutil"
+	_ "log"
+	_ "net"
+	_ "net/http"
+	_ "net/http/httptrace"
+	_ "net/http/pprof"
+	_ "net/url"
+	_ "os"
+	_ "os/signal"
+	_ "runtime"
+	_ "runtime/debug"
+	_ "strconv"
+	_ "strings"
+	_ "syscall"
+	_ "tailscale.com/derp/derphttp"
+	_ "tailscale.com/ipn"
+	_ "tailscale.com/ipn/ipnserver"
+	_ "tailscale.com/logpolicy"
+	_ "tailscale.com/net/dns"
+	_ "tailscale.com/net/interfaces"
+	_ "tailscale.com/net/socks5/tssocks"
+	_ "tailscale.com/net/tshttpproxy"
+	_ "tailscale.com/net/tstun"
+	_ "tailscale.com/paths"
+	_ "tailscale.com/tailcfg"
+	_ "tailscale.com/types/flagtype"
+	_ "tailscale.com/types/key"
+	_ "tailscale.com/types/logger"
+	_ "tailscale.com/util/osshare"
+	_ "tailscale.com/version"
+	_ "tailscale.com/version/distro"
+	_ "tailscale.com/wgengine"
+	_ "tailscale.com/wgengine/monitor"
+	_ "tailscale.com/wgengine/netstack"
+	_ "tailscale.com/wgengine/router"
+	_ "time"
+)

tstest/integration/vms/vms_test.go

@@ -159,6 +159,8 @@ var distros = []Distro{
 	{"opensuse-leap-15-2", "https://download.opensuse.org/repositories/Cloud:/Images:/Leap_15.2/images/openSUSE-Leap-15.2-OpenStack.x86_64.qcow2", "4df9cee9281d1f57d20f79dc65d76e255592b904760e73c0dd44ac753a54330f", 512, "zypper", "systemd"},
 	{"opensuse-leap-15-3", "http://mirror.its.dal.ca/opensuse/distribution/leap/15.3/appliances/openSUSE-Leap-15.3-JeOS.x86_64-OpenStack-Cloud.qcow2", "22e0392e4d0becb523d1bc5f709366140b7ee20d6faf26de3d0f9046d1ee15d5", 512, "zypper", "systemd"},
 	{"opensuse-tumbleweed", "https://download.opensuse.org/tumbleweed/appliances/openSUSE-Tumbleweed-JeOS.x86_64-OpenStack-Cloud.qcow2", "79e610bba3ed116556608f031c06e4b9260e3be2b193ce1727914ba213afac3f", 512, "zypper", "systemd"},
+	{"oracle-linux-7", "https://yum.oracle.com/templates/OracleLinux/OL7/u9/x86_64/OL7U9_x86_64-olvm-b86.qcow2", "2ef4c10c0f6a0b17844742adc9ede7eb64a2c326e374068b7175f2ecbb1956fb", 512, "yum", "systemd"},
+	{"oracle-linux-8", "https://yum.oracle.com/templates/OracleLinux/OL8/u4/x86_64/OL8U4_x86_64-olvm-b85.qcow2", "b86e1f1ea8fc904ed763a85ba12e9f12f4291c019c8435d0e4e6133392182b0b", 768, "dnf", "systemd"},
 	{"ubuntu-16-04", "https://cloud-images.ubuntu.com/xenial/20210429/xenial-server-cloudimg-amd64-disk1.img", "50a21bc067c05e0c73bf5d8727ab61152340d93073b3dc32eff18b626f7d813b", 512, "apt", "systemd"},
 	{"ubuntu-18-04", "https://cloud-images.ubuntu.com/bionic/20210526/bionic-server-cloudimg-amd64.img", "389ffd5d36bbc7a11bf384fd217cda9388ccae20e5b0cb7d4516733623c96022", 512, "apt", "systemd"},
 	{"ubuntu-20-04", "https://cloud-images.ubuntu.com/focal/20210603/focal-server-cloudimg-amd64.img", "1c0969323b058ba8b91fec245527069c2f0502fc119b9138b213b6bfebd965cb", 512, "apt", "systemd"},
@@ -276,11 +278,16 @@ func fetchDistro(t *testing.T, resultDistro Distro, bins *integration.Binaries)
 			}
 
 			_, err = io.Copy(fout, resp.Body)
-			resp.Body.Close()
 			if err != nil {
 				t.Fatalf("download of %s failed: %v", resultDistro.url, err)
 			}
 
+			resp.Body.Close()
+			err = fout.Close()
+			if err != nil {
+				t.Fatalf("can't close fout: %v", err)
+			}
+
 			hash := checkCachedImageHash(t, resultDistro, cdir)
 
 			if hash != resultDistro.sha256sum {

util/deephash/deephash.go

@@ -12,6 +12,7 @@ package deephash
 import (
 	"bufio"
 	"crypto/sha256"
+	"encoding/binary"
 	"encoding/hex"
 	"fmt"
 	"hash"
@@ -21,29 +22,50 @@ import (
 	"sync"
 )
 
+const scratchSize = 128
+
 // hasher is reusable state for hashing a value.
 // Get one via hasherPool.
 type hasher struct {
 	h       hash.Hash
 	bw      *bufio.Writer
-	scratch [128]byte
+	scratch [scratchSize]byte
+	visited map[uintptr]bool
 }
 
 // newHasher initializes a new hasher, for use by hasherPool.
 func newHasher() *hasher {
-	h := &hasher{h: sha256.New()}
+	h := &hasher{
+		h:       sha256.New(),
+		visited: map[uintptr]bool{},
+	}
 	h.bw = bufio.NewWriterSize(h.h, h.h.BlockSize())
 	return h
 }
 
+// setBufioWriter switches the bufio writer to w after flushing
+// any output to the old one. It then also returns the old one, so
+// the caller can switch back to it.
+func (h *hasher) setBufioWriter(w *bufio.Writer) (old *bufio.Writer) {
+	old = h.bw
+	old.Flush()
+	h.bw = w
+	return old
+}
+
 // Hash returns the raw SHA-256 (not hex) of v.
 func (h *hasher) Hash(v interface{}) (hash [sha256.Size]byte) {
 	h.bw.Flush()
 	h.h.Reset()
-	printTo(h.bw, v, h.scratch[:])
+	h.print(reflect.ValueOf(v))
 	h.bw.Flush()
-	h.h.Sum(hash[:0])
-	return hash
+	// Sum into scratch & copy out, as hash.Hash is an interface
+	// so the slice necessarily escapes, and there's no sha256
+	// concrete type exported and we don't want the 'hash' result
+	// parameter to escape to the heap:
+	h.h.Sum(h.scratch[:0])
+	copy(hash[:], h.scratch[:])
+	return
 }
 
 var hasherPool = &sync.Pool{
@@ -52,9 +74,12 @@ var hasherPool = &sync.Pool{
 
 // Hash returns the raw SHA-256 hash of v.
 func Hash(v interface{}) [sha256.Size]byte {
-	hasher := hasherPool.Get().(*hasher)
-	defer hasherPool.Put(hasher)
-	return hasher.Hash(v)
+	h := hasherPool.Get().(*hasher)
+	defer hasherPool.Put(h)
+	for k := range h.visited {
+		delete(h.visited, k)
+	}
+	return h.Hash(v)
 }
 
 // UpdateHash sets last to the hex-encoded hash of v and reports whether its value changed.
@@ -84,28 +109,39 @@ func sha256EqualHex(sum [sha256.Size]byte, hx string) bool {
 	return true
 }
 
-func printTo(w *bufio.Writer, v interface{}, scratch []byte) {
-	print(w, reflect.ValueOf(v), make(map[uintptr]bool), scratch)
-}
-
 var appenderToType = reflect.TypeOf((*appenderTo)(nil)).Elem()
 
 type appenderTo interface {
 	AppendTo([]byte) []byte
 }
 
+func (h *hasher) uint(i uint64) {
+	binary.BigEndian.PutUint64(h.scratch[:8], i)
+	h.bw.Write(h.scratch[:8])
+}
+
+func (h *hasher) int(i int) {
+	binary.BigEndian.PutUint64(h.scratch[:8], uint64(i))
+	h.bw.Write(h.scratch[:8])
+}
+
+var uint8Type = reflect.TypeOf(byte(0))
+
 // print hashes v into w.
 // It reports whether it was able to do so without hitting a cycle.
-func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool, scratch []byte) (acyclic bool) {
+func (h *hasher) print(v reflect.Value) (acyclic bool) {
 	if !v.IsValid() {
 		return true
 	}
 
+	w := h.bw
+	visited := h.visited
+
 	if v.CanInterface() {
 		// Use AppendTo methods, if available and cheap.
 		if v.CanAddr() && v.Type().Implements(appenderToType) {
 			a := v.Addr().Interface().(appenderTo)
-			scratch = a.AppendTo(scratch[:0])
+			scratch := a.AppendTo(h.scratch[:0])
 			w.Write(scratch)
 			return true
 		}
@@ -121,54 +157,84 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool, scratch [
 			return false
 		}
 		visited[ptr] = true
-		return print(w, v.Elem(), visited, scratch)
+		return h.print(v.Elem())
 	case reflect.Struct:
 		acyclic = true
-		w.WriteString("struct{\n")
+		w.WriteString("struct")
+		h.int(v.NumField())
 		for i, n := 0, v.NumField(); i < n; i++ {
-			fmt.Fprintf(w, " [%d]: ", i)
-			if !print(w, v.Field(i), visited, scratch) {
+			h.int(i)
+			if !h.print(v.Field(i)) {
 				acyclic = false
 			}
-			w.WriteString("\n")
 		}
-		w.WriteString("}\n")
 		return acyclic
 	case reflect.Slice, reflect.Array:
-		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
-			fmt.Fprintf(w, "%q", v.Interface())
+		vLen := v.Len()
+		if v.Kind() == reflect.Slice {
+			h.int(vLen)
+		}
+		if v.Type().Elem() == uint8Type && v.CanInterface() {
+			if vLen > 0 && vLen <= scratchSize {
+				// If it fits in scratch, avoid the Interface allocation.
+				// It seems tempting to do this for all sizes, doing
+				// scratchSize bytes at a time, but reflect.Slice seems
+				// to allocate, so it's not a win.
+				n := reflect.Copy(reflect.ValueOf(&h.scratch).Elem(), v)
+				w.Write(h.scratch[:n])
+				return true
+			}
+			fmt.Fprintf(w, "%s", v.Interface())
 			return true
 		}
-		fmt.Fprintf(w, "[%d]{\n", v.Len())
 		acyclic = true
-		for i, ln := 0, v.Len(); i < ln; i++ {
-			fmt.Fprintf(w, " [%d]: ", i)
-			if !print(w, v.Index(i), visited, scratch) {
+		for i := 0; i < vLen; i++ {
+			h.int(i)
+			if !h.print(v.Index(i)) {
 				acyclic = false
 			}
-			w.WriteString("\n")
 		}
-		w.WriteString("}\n")
 		return acyclic
 	case reflect.Interface:
-		return print(w, v.Elem(), visited, scratch)
+		return h.print(v.Elem())
 	case reflect.Map:
-		if hashMapAcyclic(w, v, visited, scratch) {
+		// TODO(bradfitz): ideally we'd avoid these map
+		// operations to detect cycles if we knew from the map
+		// element type that there no way to form a cycle,
+		// which is the common case. Notably, we don't care
+		// about hashing the same map+contents twice in
+		// different parts of the tree. In fact, we should
+		// ideally. (And this prevents it) We should only stop
+		// hashing when there's a cycle.  What we should
+		// probably do is make sure we enumerate the data
+		// structure tree is a fixed order and then give each
+		// pointer an increasing number, and when we hit a
+		// dup, rather than emitting nothing, we should emit a
+		// "value #12" reference. Which implies that all things
+		// emit to the bufio.Writer should be type-tagged so
+		// we can distinguish loop references without risk of
+		// collisions.
+		ptr := v.Pointer()
+		if visited[ptr] {
+			return false
+		}
+		visited[ptr] = true
+
+		if h.hashMapAcyclic(v) {
 			return true
 		}
-		return hashMapFallback(w, v, visited, scratch)
+		return h.hashMapFallback(v)
 	case reflect.String:
+		h.int(v.Len())
 		w.WriteString(v.String())
 	case reflect.Bool:
-		w.Write(strconv.AppendBool(scratch[:0], v.Bool()))
+		w.Write(strconv.AppendBool(h.scratch[:0], v.Bool()))
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		w.Write(strconv.AppendInt(scratch[:0], v.Int(), 10))
+		w.Write(strconv.AppendInt(h.scratch[:0], v.Int(), 10))
 	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
-		scratch = strconv.AppendUint(scratch[:0], v.Uint(), 10)
-		w.Write(scratch)
+		h.uint(v.Uint())
 	case reflect.Float32, reflect.Float64:
-		scratch = strconv.AppendUint(scratch[:0], math.Float64bits(v.Float()), 10)
-		w.Write(scratch)
+		w.Write(strconv.AppendUint(h.scratch[:0], math.Float64bits(v.Float()), 10))
 	case reflect.Complex64, reflect.Complex128:
 		fmt.Fprintf(w, "%v", v.Complex())
 	}
@@ -230,40 +296,46 @@ func (c valueCache) get(t reflect.Type) reflect.Value {
 // hashMapAcyclic is the faster sort-free version of map hashing. If
 // it detects a cycle it returns false and guarantees that nothing was
 // written to w.
-func hashMapAcyclic(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool, scratch []byte) (acyclic bool) {
+func (h *hasher) hashMapAcyclic(v reflect.Value) (acyclic bool) {
 	mh := mapHasherPool.Get().(*mapHasher)
 	defer mapHasherPool.Put(mh)
 	mh.Reset()
 	iter := mapIter(mh.iter, v)
 	defer mapIter(mh.iter, reflect.Value{}) // avoid pinning v from mh.iter when we return
+
+	// Temporarily switch to the map hasher's bufio.Writer.
+	oldw := h.setBufioWriter(mh.bw)
+	defer h.setBufioWriter(oldw)
+
 	k := mh.val.get(v.Type().Key())
 	e := mh.val.get(v.Type().Elem())
 	for iter.Next() {
 		key := iterKey(iter, k)
 		val := iterVal(iter, e)
 		mh.startEntry()
-		if !print(mh.bw, key, visited, scratch) {
+		if !h.print(key) {
 			return false
 		}
-		if !print(mh.bw, val, visited, scratch) {
+		if !h.print(val) {
 			return false
 		}
 		mh.endEntry()
 	}
-	w.Write(mh.xbuf[:])
+	oldw.Write(mh.xbuf[:])
 	return true
 }
 
-func hashMapFallback(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool, scratch []byte) (acyclic bool) {
+func (h *hasher) hashMapFallback(v reflect.Value) (acyclic bool) {
 	acyclic = true
 	sm := newSortedMap(v)
+	w := h.bw
 	fmt.Fprintf(w, "map[%d]{\n", len(sm.Key))
 	for i, k := range sm.Key {
-		if !print(w, k, visited, scratch) {
+		if !h.print(k) {
 			acyclic = false
 		}
 		w.WriteString(": ")
-		if !print(w, sm.Value[i], visited, scratch) {
+		if !h.print(sm.Value[i]) {
 			acyclic = false
 		}
 		w.WriteString("\n")

util/deephash/deephash_test.go

@@ -15,7 +15,10 @@ import (
 
 	"inet.af/netaddr"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/ipproto"
 	"tailscale.com/util/dnsname"
+	"tailscale.com/version"
+	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/router"
 	"tailscale.com/wgengine/wgcfg"
 )
@@ -125,6 +128,9 @@ func getVal() []interface{} {
 				{ID: 2, LoginName: "bar@foo.com"},
 			},
 		},
+		filter.Match{
+			IPProto: []ipproto.Proto{1, 2, 3},
+		},
 	}
 }
 
@@ -149,12 +155,14 @@ func TestHashMapAcyclic(t *testing.T) {
 	bw := bufio.NewWriter(&buf)
 
 	for i := 0; i < 20; i++ {
-		visited := map[uintptr]bool{}
-		scratch := make([]byte, 0, 64)
 		v := reflect.ValueOf(m)
 		buf.Reset()
 		bw.Reset(&buf)
-		if !hashMapAcyclic(bw, v, visited, scratch) {
+		h := &hasher{
+			bw:      bw,
+			visited: map[uintptr]bool{},
+		}
+		if !h.hashMapAcyclic(v) {
 			t.Fatal("returned false")
 		}
 		if got[string(buf.Bytes())] {
@@ -167,6 +175,29 @@ func TestHashMapAcyclic(t *testing.T) {
 	}
 }
 
+func TestPrintArray(t *testing.T) {
+	type T struct {
+		X [32]byte
+	}
+	x := &T{X: [32]byte{1: 1, 31: 31}}
+	var got bytes.Buffer
+	bw := bufio.NewWriter(&got)
+	h := &hasher{
+		bw:      bw,
+		visited: map[uintptr]bool{},
+	}
+	h.print(reflect.ValueOf(x))
+	bw.Flush()
+	const want = "struct" +
+		"\x00\x00\x00\x00\x00\x00\x00\x01" + // 1 field
+		"\x00\x00\x00\x00\x00\x00\x00\x00" + // 0th field
+		// the 32 bytes:
+		"\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1f"
+	if got := got.Bytes(); string(got) != want {
+		t.Errorf("wrong:\n got: %q\nwant: %q\n", got, want)
+	}
+}
+
 func BenchmarkHashMapAcyclic(b *testing.B) {
 	b.ReportAllocs()
 	m := map[int]string{}
@@ -176,14 +207,17 @@ func BenchmarkHashMapAcyclic(b *testing.B) {
 
 	var buf bytes.Buffer
 	bw := bufio.NewWriter(&buf)
-	visited := map[uintptr]bool{}
-	scratch := make([]byte, 0, 64)
 	v := reflect.ValueOf(m)
 
+	h := &hasher{
+		bw:      bw,
+		visited: map[uintptr]bool{},
+	}
+
 	for i := 0; i < b.N; i++ {
 		buf.Reset()
 		bw.Reset(&buf)
-		if !hashMapAcyclic(bw, v, visited, scratch) {
+		if !h.hashMapAcyclic(v) {
 			b.Fatal("returned false")
 		}
 	}
@@ -221,3 +255,43 @@ func TestSHA256EqualHex(t *testing.T) {
 		}
 	}
 }
+
+// verify this doesn't loop forever, as it used to (Issue 2340)
+func TestMapCyclicFallback(t *testing.T) {
+	type T struct {
+		M map[string]interface{}
+	}
+	v := &T{
+		M: map[string]interface{}{},
+	}
+	v.M["m"] = v.M
+	Hash(v)
+}
+
+func TestArrayAllocs(t *testing.T) {
+	if version.IsRace() {
+		t.Skip("skipping test under race detector")
+	}
+	type T struct {
+		X [32]byte
+	}
+	x := &T{X: [32]byte{1: 1, 2: 2, 3: 3, 4: 4}}
+	n := int(testing.AllocsPerRun(1000, func() {
+		sink = Hash(x)
+	}))
+	if n > 0 {
+		t.Errorf("allocs = %v; want 0", n)
+	}
+}
+
+func BenchmarkHashArray(b *testing.B) {
+	b.ReportAllocs()
+	type T struct {
+		X [32]byte
+	}
+	x := &T{X: [32]byte{1: 1, 2: 2, 3: 3, 4: 4}}
+
+	for i := 0; i < b.N; i++ {
+		sink = Hash(x)
+	}
+}