tstest/controll: add a trolling control server for stressing clients

Updates #1909
Updates #13390

Change-Id: Ia24b8b9b8d2f20985de1454cc2013bec99ca8f3f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/derper/derper.go

@@ -77,6 +77,8 @@ var (
 	tcpKeepAlive = flag.Duration("tcp-keepalive-time", 10*time.Minute, "TCP keepalive time")
 	// tcpUserTimeout is intentionally short, so that hung connections are cleaned up promptly. DERPs should be nearby users.
 	tcpUserTimeout = flag.Duration("tcp-user-timeout", 15*time.Second, "TCP user timeout")
+	// tcpWriteTimeout is the timeout for writing to client TCP connections. It does not apply to mesh connections.
+	tcpWriteTimeout = flag.Duration("tcp-write-timeout", derp.DefaultTCPWiteTimeout, "TCP write timeout; 0 results in no timeout being set on writes")
 )
 
 var (
@@ -173,6 +175,7 @@ func main() {
 	s.SetVerifyClient(*verifyClients)
 	s.SetVerifyClientURL(*verifyClientURL)
 	s.SetVerifyClientURLFailOpen(*verifyFailOpen)
+	s.SetTCPWriteTimeout(*tcpWriteTimeout)
 
 	if *meshPSKFile != "" {
 		b, err := os.ReadFile(*meshPSKFile)

cmd/tailscale/cli/cli_test.go

@@ -17,6 +17,7 @@ import (
 
 	qt "github.com/frankban/quicktest"
 	"github.com/google/go-cmp/cmp"
+	"github.com/peterbourgon/ff/v3/ffcli"
 	"tailscale.com/envknob"
 	"tailscale.com/health/healthmsg"
 	"tailscale.com/ipn"
@@ -1525,3 +1526,45 @@ func TestHelpAlias(t *testing.T) {
 		t.Fatalf("Run: %v", err)
 	}
 }
+
+func TestDocs(t *testing.T) {
+	root := newRootCmd()
+	check := func(t *testing.T, c *ffcli.Command) {
+		shortVerb, _, ok := strings.Cut(c.ShortHelp, " ")
+		if !ok || shortVerb == "" {
+			t.Errorf("couldn't find verb+space in ShortHelp")
+		} else {
+			if strings.HasSuffix(shortVerb, ".") {
+				t.Errorf("ShortHelp shouldn't end in period; got %q", c.ShortHelp)
+			}
+			if b := shortVerb[0]; b >= 'a' && b <= 'z' {
+				t.Errorf("ShortHelp should start with upper-case letter; got %q", c.ShortHelp)
+			}
+			if strings.HasSuffix(shortVerb, "s") && shortVerb != "Does" {
+				t.Errorf("verb %q ending in 's' is unexpected, from %q", shortVerb, c.ShortHelp)
+			}
+		}
+
+		name := t.Name()
+		wantPfx := strings.ReplaceAll(strings.TrimPrefix(name, "TestDocs/"), "/", " ")
+		switch name {
+		case "TestDocs/tailscale/completion/bash",
+			"TestDocs/tailscale/completion/zsh":
+			wantPfx = "" // special-case exceptions
+		}
+		if !strings.HasPrefix(c.ShortUsage, wantPfx) {
+			t.Errorf("ShortUsage should start with %q; got %q", wantPfx, c.ShortUsage)
+		}
+	}
+
+	var walk func(t *testing.T, c *ffcli.Command)
+	walk = func(t *testing.T, c *ffcli.Command) {
+		t.Run(c.Name, func(t *testing.T) {
+			check(t, c)
+			for _, sub := range c.Subcommands {
+				walk(t, sub)
+			}
+		})
+	}
+	walk(t, root)
+}

cmd/tailscale/cli/configure_apple.go

@@ -27,28 +27,28 @@ func sysExtCmd() *ffcli.Command {
 	return &ffcli.Command{
 		Name:       "sysext",
 		ShortUsage: "tailscale configure sysext [activate|deactivate|status]",
-		ShortHelp:  "Manages the system extension for macOS (Standalone variant)",
+		ShortHelp:  "Manage the system extension for macOS (Standalone variant)",
 		LongHelp: "The sysext set of commands provides a way to activate, deactivate, or manage the state of the Tailscale system extension on macOS. " +
 			"This is only relevant if you are running the Standalone variant of the Tailscale client for macOS. " +
 			"To access more detailed information about system extensions installed on this Mac, run 'systemextensionsctl list'.",
 		Subcommands: []*ffcli.Command{
 			{
 				Name:       "activate",
-				ShortUsage: "tailscale sysext activate",
+				ShortUsage: "tailscale configure sysext activate",
 				ShortHelp:  "Register the Tailscale system extension with macOS.",
 				LongHelp:   "This command registers the Tailscale system extension with macOS. To run Tailscale, you'll also need to install the VPN configuration separately (run `tailscale configure vpn-config install`). After running this command, you need to approve the extension in System Settings > Login Items and Extensions > Network Extensions.",
 				Exec:       requiresStandalone,
 			},
 			{
 				Name:       "deactivate",
-				ShortUsage: "tailscale sysext deactivate",
+				ShortUsage: "tailscale configure sysext deactivate",
 				ShortHelp:  "Deactivate the Tailscale system extension on macOS",
 				LongHelp:   "This command deactivates the Tailscale system extension on macOS. To completely remove Tailscale, you'll also need to delete the VPN configuration separately (use `tailscale configure vpn-config uninstall`).",
 				Exec:       requiresStandalone,
 			},
 			{
 				Name:       "status",
-				ShortUsage: "tailscale sysext status",
+				ShortUsage: "tailscale configure sysext status",
 				ShortHelp:  "Print the enablement status of the Tailscale system extension",
 				LongHelp:   "This command prints the enablement status of the Tailscale system extension. If the extension is not enabled, run `tailscale sysext activate` to enable it.",
 				Exec:       requiresStandalone,
@@ -69,14 +69,14 @@ func vpnConfigCmd() *ffcli.Command {
 		Subcommands: []*ffcli.Command{
 			{
 				Name:       "install",
-				ShortUsage: "tailscale mac-vpn install",
+				ShortUsage: "tailscale configure mac-vpn install",
 				ShortHelp:  "Write the Tailscale VPN configuration to the macOS settings",
 				LongHelp:   "This command writes the Tailscale VPN configuration to the macOS settings. This is the entry that appears in System Settings > VPN. If you are running the Standalone variant of the client, you'll also need to install the system extension separately (run `tailscale configure sysext activate`).",
 				Exec:       requiresGUI,
 			},
 			{
 				Name:       "uninstall",
-				ShortUsage: "tailscale mac-vpn uninstall",
+				ShortUsage: "tailscale configure mac-vpn uninstall",
 				ShortHelp:  "Delete the Tailscale VPN configuration from the macOS settings",
 				LongHelp:   "This command removes the Tailscale VPN configuration from the macOS settings. This is the entry that appears in System Settings > VPN. If you are running the Standalone variant of the client, you'll also need to deactivate the system extension separately (run `tailscale configure sysext deactivate`).",
 				Exec:       requiresGUI,

cmd/tailscale/cli/debug.go

@@ -289,7 +289,7 @@ var debugCmd = &ffcli.Command{
 			Name:       "capture",
 			ShortUsage: "tailscale debug capture",
 			Exec:       runCapture,
-			ShortHelp:  "Streams pcaps for debugging",
+			ShortHelp:  "Stream pcaps for debugging",
 			FlagSet: (func() *flag.FlagSet {
 				fs := newFlagSet("capture")
 				fs.StringVar(&captureArgs.outFile, "o", "", "path to stream the pcap (or - for stdout), leave empty to start wireshark")
@@ -315,13 +315,13 @@ var debugCmd = &ffcli.Command{
 			Name:       "peer-endpoint-changes",
 			ShortUsage: "tailscale debug peer-endpoint-changes <hostname-or-IP>",
 			Exec:       runPeerEndpointChanges,
-			ShortHelp:  "Prints debug information about a peer's endpoint changes",
+			ShortHelp:  "Print debug information about a peer's endpoint changes",
 		},
 		{
 			Name:       "dial-types",
 			ShortUsage: "tailscale debug dial-types <hostname-or-IP> <port>",
 			Exec:       runDebugDialTypes,
-			ShortHelp:  "Prints debug information about connecting to a given host or IP",
+			ShortHelp:  "Print debug information about connecting to a given host or IP",
 			FlagSet: (func() *flag.FlagSet {
 				fs := newFlagSet("dial-types")
 				fs.StringVar(&debugDialTypesArgs.network, "network", "tcp", `network type to dial ("tcp", "udp", etc.)`)
@@ -342,7 +342,7 @@ var debugCmd = &ffcli.Command{
 		{
 			Name:       "go-buildinfo",
 			ShortUsage: "tailscale debug go-buildinfo",
-			ShortHelp:  "Prints Go's runtime/debug.BuildInfo",
+			ShortHelp:  "Print Go's runtime/debug.BuildInfo",
 			Exec:       runGoBuildInfo,
 		},
 	},

cmd/tailscale/cli/dns.go

@@ -20,7 +20,7 @@ var dnsCmd = &ffcli.Command{
 			Name:       "status",
 			ShortUsage: "tailscale dns status [--all]",
 			Exec:       runDNSStatus,
-			ShortHelp:  "Prints the current DNS status and configuration",
+			ShortHelp:  "Print the current DNS status and configuration",
 			LongHelp:   dnsStatusLongHelp(),
 			FlagSet: (func() *flag.FlagSet {
 				fs := newFlagSet("status")

cmd/tailscale/cli/exitnode.go

@@ -41,7 +41,7 @@ func exitNodeCmd() *ffcli.Command {
 			{
 				Name:       "suggest",
 				ShortUsage: "tailscale exit-node suggest",
-				ShortHelp:  "Suggests the best available exit node",
+				ShortHelp:  "Suggest the best available exit node",
 				Exec:       runExitNodeSuggest,
 			}},
 			(func() []*ffcli.Command {

cmd/tailscale/cli/metrics.go

@@ -33,13 +33,13 @@ https://tailscale.com/s/client-metrics
 			Name:       "print",
 			ShortUsage: "tailscale metrics print",
 			Exec:       runMetricsPrint,
-			ShortHelp:  "Prints current metric values in the Prometheus text exposition format",
+			ShortHelp:  "Print current metric values in Prometheus text format",
 		},
 		{
 			Name:       "write",
 			ShortUsage: "tailscale metrics write <path>",
 			Exec:       runMetricsWrite,
-			ShortHelp:  "Writes metric values to a file",
+			ShortHelp:  "Write metric values to a file",
 			LongHelp: strings.TrimSpace(`
 
 The 'tailscale metrics write' command writes metric values to a text file provided as its

cmd/tailscale/cli/network-lock.go

@@ -191,8 +191,7 @@ var nlStatusArgs struct {
 var nlStatusCmd = &ffcli.Command{
 	Name:       "status",
 	ShortUsage: "tailscale lock status",
-	ShortHelp:  "Outputs the state of tailnet lock",
-	LongHelp:   "Outputs the state of tailnet lock",
+	ShortHelp:  "Output the state of tailnet lock",
 	Exec:       runNetworkLockStatus,
 	FlagSet: (func() *flag.FlagSet {
 		fs := newFlagSet("lock status")
@@ -293,8 +292,7 @@ func runNetworkLockStatus(ctx context.Context, args []string) error {
 var nlAddCmd = &ffcli.Command{
 	Name:       "add",
 	ShortUsage: "tailscale lock add <public-key>...",
-	ShortHelp:  "Adds one or more trusted signing keys to tailnet lock",
-	LongHelp:   "Adds one or more trusted signing keys to tailnet lock",
+	ShortHelp:  "Add one or more trusted signing keys to tailnet lock",
 	Exec: func(ctx context.Context, args []string) error {
 		return runNetworkLockModify(ctx, args, nil)
 	},
@@ -307,8 +305,7 @@ var nlRemoveArgs struct {
 var nlRemoveCmd = &ffcli.Command{
 	Name:       "remove",
 	ShortUsage: "tailscale lock remove [--re-sign=false] <public-key>...",
-	ShortHelp:  "Removes one or more trusted signing keys from tailnet lock",
-	LongHelp:   "Removes one or more trusted signing keys from tailnet lock",
+	ShortHelp:  "Remove one or more trusted signing keys from tailnet lock",
 	Exec:       runNetworkLockRemove,
 	FlagSet: (func() *flag.FlagSet {
 		fs := newFlagSet("lock remove")
@@ -448,7 +445,7 @@ func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) err
 var nlSignCmd = &ffcli.Command{
 	Name:       "sign",
 	ShortUsage: "tailscale lock sign <node-key> [<rotation-key>]\ntailscale lock sign <auth-key>",
-	ShortHelp:  "Signs a node or pre-approved auth key",
+	ShortHelp:  "Sign a node or pre-approved auth key",
 	LongHelp: `Either:
   - signs a node key and transmits the signature to the coordination
     server, or
@@ -510,7 +507,7 @@ func runNetworkLockSign(ctx context.Context, args []string) error {
 var nlDisableCmd = &ffcli.Command{
 	Name:       "disable",
 	ShortUsage: "tailscale lock disable <disablement-secret>",
-	ShortHelp:  "Consumes a disablement secret to shut down tailnet lock for the tailnet",
+	ShortHelp:  "Consume a disablement secret to shut down tailnet lock for the tailnet",
 	LongHelp: strings.TrimSpace(`
 
 The 'tailscale lock disable' command uses the specified disablement
@@ -539,7 +536,7 @@ func runNetworkLockDisable(ctx context.Context, args []string) error {
 var nlLocalDisableCmd = &ffcli.Command{
 	Name:       "local-disable",
 	ShortUsage: "tailscale lock local-disable",
-	ShortHelp:  "Disables tailnet lock for this node only",
+	ShortHelp:  "Disable tailnet lock for this node only",
 	LongHelp: strings.TrimSpace(`
 
 The 'tailscale lock local-disable' command disables tailnet lock for only
@@ -561,8 +558,8 @@ func runNetworkLockLocalDisable(ctx context.Context, args []string) error {
 var nlDisablementKDFCmd = &ffcli.Command{
 	Name:       "disablement-kdf",
 	ShortUsage: "tailscale lock disablement-kdf <hex-encoded-disablement-secret>",
-	ShortHelp:  "Computes a disablement value from a disablement secret (advanced users only)",
-	LongHelp:   "Computes a disablement value from a disablement secret (advanced users only)",
+	ShortHelp:  "Compute a disablement value from a disablement secret (advanced users only)",
+	LongHelp:   "Compute a disablement value from a disablement secret (advanced users only)",
 	Exec:       runNetworkLockDisablementKDF,
 }
 

cmd/tailscale/cli/switch.go

@@ -20,7 +20,7 @@ import (
 var switchCmd = &ffcli.Command{
 	Name:       "switch",
 	ShortUsage: "tailscale switch <id>",
-	ShortHelp:  "Switches to a different Tailscale account",
+	ShortHelp:  "Switch to a different Tailscale account",
 	LongHelp: `"tailscale switch" switches between logged in accounts. You can
 use the ID that's returned from 'tailnet switch -list'
 to pick which profile you want to switch to. Alternatively, you

cmd/tailscale/cli/syspolicy.go

@@ -31,7 +31,7 @@ var syspolicyCmd = &ffcli.Command{
 			Name:       "list",
 			ShortUsage: "tailscale syspolicy list",
 			Exec:       runSysPolicyList,
-			ShortHelp:  "Prints effective policy settings",
+			ShortHelp:  "Print effective policy settings",
 			LongHelp:   "The 'tailscale syspolicy list' subcommand displays the effective policy settings and their sources (e.g., MDM or environment variables).",
 			FlagSet: (func() *flag.FlagSet {
 				fs := newFlagSet("syspolicy list")
@@ -43,7 +43,7 @@ var syspolicyCmd = &ffcli.Command{
 			Name:       "reload",
 			ShortUsage: "tailscale syspolicy reload",
 			Exec:       runSysPolicyReload,
-			ShortHelp:  "Forces a reload of policy settings, even if no changes are detected, and prints the result",
+			ShortHelp:  "Force a reload of policy settings, even if no changes are detected, and prints the result",
 			LongHelp:   "The 'tailscale syspolicy reload' subcommand forces a reload of policy settings, even if no changes are detected, and prints the result.",
 			FlagSet: (func() *flag.FlagSet {
 				fs := newFlagSet("syspolicy reload")

derp/derp.go

@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"time"
 )
 
@@ -254,3 +255,14 @@ func writeFrame(bw *bufio.Writer, t frameType, b []byte) error {
 	}
 	return bw.Flush()
 }
+
+// Conn is the subset of the underlying net.Conn the DERP Server needs.
+// It is a defined type so that non-net connections can be used.
+type Conn interface {
+	io.WriteCloser
+	LocalAddr() net.Addr
+	// The *Deadline methods follow the semantics of net.Conn.
+	SetDeadline(time.Time) error
+	SetReadDeadline(time.Time) error
+	SetWriteDeadline(time.Time) error
+}

derp/derp_server.go

@@ -23,7 +23,6 @@ import (
 	"math"
 	"math/big"
 	"math/rand/v2"
-	"net"
 	"net/http"
 	"net/netip"
 	"os"
@@ -85,7 +84,7 @@ func init() {
 
 const (
 	defaultPerClientSendQueueDepth = 32 // default packets buffered for sending
-	writeTimeout                   = 2 * time.Second
+	DefaultTCPWiteTimeout          = 2 * time.Second
 	privilegedWriteTimeout         = 30 * time.Second // for clients with the mesh key
 )
 
@@ -202,6 +201,8 @@ type Server struct {
 	// Sets the client send queue depth for the server.
 	perClientSendQueueDepth int
 
+	tcpWriteTimeout time.Duration
+
 	clock tstime.Clock
 }
 
@@ -341,17 +342,6 @@ type PacketForwarder interface {
 	String() string
 }
 
-// Conn is the subset of the underlying net.Conn the DERP Server needs.
-// It is a defined type so that non-net connections can be used.
-type Conn interface {
-	io.WriteCloser
-	LocalAddr() net.Addr
-	// The *Deadline methods follow the semantics of net.Conn.
-	SetDeadline(time.Time) error
-	SetReadDeadline(time.Time) error
-	SetWriteDeadline(time.Time) error
-}
-
 var packetsDropped = metrics.NewMultiLabelMap[dropReasonKindLabels](
 	"derp_packets_dropped",
 	"counter",
@@ -389,6 +379,7 @@ func NewServer(privateKey key.NodePrivate, logf logger.Logf) *Server {
 		bufferedWriteFrames: metrics.NewHistogram([]float64{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 15, 20, 25, 50, 100}),
 		keyOfAddr:           map[netip.AddrPort]key.NodePublic{},
 		clock:               tstime.StdClock{},
+		tcpWriteTimeout:     DefaultTCPWiteTimeout,
 	}
 	s.initMetacert()
 	s.packetsRecvDisco = s.packetsRecvByKind.Get(string(packetKindDisco))
@@ -493,6 +484,13 @@ func (s *Server) SetVerifyClientURLFailOpen(v bool) {
 	s.verifyClientsURLFailOpen = v
 }
 
+// SetTCPWriteTimeout sets the timeout for writing to connected clients.
+// This timeout does not apply to mesh connections.
+// Defaults to 2 seconds.
+func (s *Server) SetTCPWriteTimeout(d time.Duration) {
+	s.tcpWriteTimeout = d
+}
+
 // HasMeshKey reports whether the server is configured with a mesh key.
 func (s *Server) HasMeshKey() bool { return s.meshKey != "" }
 
@@ -1817,7 +1815,7 @@ func (c *sclient) sendLoop(ctx context.Context) error {
 }
 
 func (c *sclient) setWriteDeadline() {
-	d := writeTimeout
+	d := c.s.tcpWriteTimeout
 	if c.canMesh {
 		// Trusted peers get more tolerance.
 		//
@@ -1829,7 +1827,10 @@ func (c *sclient) setWriteDeadline() {
 		// of connected peers.
 		d = privilegedWriteTimeout
 	}
-	c.nc.SetWriteDeadline(time.Now().Add(d))
+	// Ignore the error from setting the write deadline. In practice,
+	// setting the deadline will only fail if the connection is closed
+	// or closing, so the subsequent Write() will fail anyway.
+	_ = c.nc.SetWriteDeadline(time.Now().Add(d))
 }
 
 // sendKeepAlive sends a keep-alive frame, without flushing.

ipn/ipnlocal/cert.go

@@ -556,6 +556,7 @@ func (b *LocalBackend) getCertPEM(ctx context.Context, cs certStore, logf logger
 	}
 
 	logf("requesting cert...")
+	traceACME(csr)
 	der, _, err := ac.CreateOrderCert(ctx, order.FinalizeURL, csr, true)
 	if err != nil {
 		return nil, fmt.Errorf("CreateOrder: %v", err)
@@ -578,10 +579,10 @@ func (b *LocalBackend) getCertPEM(ctx context.Context, cs certStore, logf logger
 }
 
 // certRequest generates a CSR for the given common name cn and optional SANs.
-func certRequest(key crypto.Signer, cn string, ext []pkix.Extension, san ...string) ([]byte, error) {
+func certRequest(key crypto.Signer, name string, ext []pkix.Extension) ([]byte, error) {
 	req := &x509.CertificateRequest{
-		Subject:         pkix.Name{CommonName: cn},
-		DNSNames:        san,
+		Subject:         pkix.Name{CommonName: name},
+		DNSNames:        []string{name},
 		ExtraExtensions: ext,
 	}
 	return x509.CreateCertificateRequest(rand.Reader, req, key)

net/tstun/wrap.go

@@ -877,12 +877,13 @@ func (t *Wrapper) filterPacketOutboundToWireGuard(p *packet.Parsed, pc *peerConf
 		return filter.Drop, gro
 	}
 
-	if filt.RunOut(p, t.filterFlags) != filter.Accept {
+	if resp, reason := filt.RunOut(p, t.filterFlags); resp != filter.Accept {
 		metricPacketOutDropFilter.Add(1)
-		// TODO(#14280): increment a t.metrics.outboundDroppedPacketsTotal here
-		// once we figure out & document what labels to use for multicast,
-		// link-local-unicast, IP fragments, etc. But they're not
-		// usermetric.ReasonACL.
+		if reason != "" {
+			t.metrics.outboundDroppedPacketsTotal.Add(usermetric.DropLabels{
+				Reason: reason,
+			}, 1)
+		}
 		return filter.Drop, gro
 	}
 

tailcfg/tailcfg.go

@@ -27,6 +27,7 @@ import (
 	"tailscale.com/types/tkatype"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/vizerror"
 )
 
 // CapabilityVersion represents the client's capability level. That
@@ -891,14 +892,14 @@ type ServiceName string
 
 // Validate validates if the service name is formatted correctly.
 // We only allow valid DNS labels, since the expectation is that these will be
-// used as parts of domain names.
+// used as parts of domain names. All errors are [vizerror.Error].
 func (sn ServiceName) Validate() error {
 	bareName, ok := strings.CutPrefix(string(sn), "svc:")
 	if !ok {
-		return errors.New("services must start with 'svc:'")
+		return vizerror.Errorf("%q is not a valid service name: must start with 'svc:'", sn)
 	}
 	if bareName == "" {
-		return errors.New("service names must not be empty")
+		return vizerror.Errorf("%q is not a valid service name: must not be empty after the 'svc:' prefix", sn)
 	}
 	return dnsname.ValidLabel(bareName)
 }

tstest/controll/controll.go

@@ -0,0 +1,290 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// The controll program trolls tailscaleds, simulating huge and busy tailnets.
+package main
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"flag"
+	"fmt"
+	"log"
+	"math/rand/v2"
+	"net/http"
+	"net/netip"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"golang.org/x/crypto/acme/autocert"
+	"tailscale.com/net/tsaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/tstest/integration"
+	"tailscale.com/tstest/integration/testcontrol"
+	"tailscale.com/types/key"
+	"tailscale.com/types/logger"
+	"tailscale.com/types/ptr"
+	"tailscale.com/util/must"
+)
+
+var (
+	flagNFake = flag.Int("nfake", 0, "number of fake nodes to add to network")
+	certHost  = flag.String("certhost", "controll.fitz.dev", "hostname to use in TLS certificate")
+)
+
+type state struct {
+	Legacy  key.ControlPrivate
+	Machine key.MachinePrivate
+}
+
+func loadState() *state {
+	st := &state{}
+	path := filepath.Join(must.Get(os.UserCacheDir()), "controll.state")
+	f, _ := os.ReadFile(path)
+	f = bytes.TrimSpace(f)
+	if err := json.Unmarshal(f, st); err == nil {
+		return st
+	}
+	st.Legacy = key.NewControl()
+	st.Machine = key.NewMachine()
+	f = must.Get(json.Marshal(st))
+	must.Do(os.WriteFile(path, f, 0600))
+	return st
+}
+
+func main() {
+	flag.Parse()
+
+	var t fakeTB
+	derpMap := integration.RunDERPAndSTUN(t, logger.Discard, "127.0.0.1")
+
+	certManager := &autocert.Manager{
+		Prompt:     autocert.AcceptTOS,
+		HostPolicy: autocert.HostWhitelist(*certHost),
+		Cache:      autocert.DirCache(filepath.Join(must.Get(os.UserCacheDir()), "controll-cert")),
+	}
+
+	control := &testcontrol.Server{
+		DERPMap:              derpMap,
+		ExplicitBaseURL:      "http://127.0.0.1:9911",
+		TolerateUnknownPaths: true,
+		AltMapStream:         sendClientChaos,
+	}
+
+	st := loadState()
+	control.SetPrivateKeys(st.Machine, st.Legacy)
+	for range *flagNFake {
+		control.AddFakeNode()
+	}
+	mux := http.NewServeMux()
+	mux.HandleFunc("/{$}", func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte("<html><body>con<b>troll</b>"))
+	})
+	mux.Handle("/", control)
+
+	go func() {
+		addr := "127.0.0.1:9911"
+		log.Printf("listening on %s", addr)
+		err := http.ListenAndServe(addr, mux)
+		log.Fatal(err)
+	}()
+
+	if *certHost != "" {
+		go func() {
+			srv := &http.Server{
+				Addr:      ":https",
+				Handler:   mux,
+				TLSConfig: certManager.TLSConfig(),
+			}
+			log.Fatalf("TLS: %v", srv.ListenAndServeTLS("", ""))
+		}()
+	}
+
+	select {}
+}
+
+func node4(nid tailcfg.NodeID) netip.Prefix {
+	return netip.PrefixFrom(
+		netip.AddrFrom4([4]byte{100, 100 + byte(nid>>16), byte(nid >> 8), byte(nid)}),
+		32)
+}
+
+func node6(nid tailcfg.NodeID) netip.Prefix {
+	a := tsaddr.TailscaleULARange().Addr().As16()
+	a[13] = byte(nid >> 16)
+	a[14] = byte(nid >> 8)
+	a[15] = byte(nid)
+	v6 := netip.AddrFrom16(a)
+	return netip.PrefixFrom(v6, 128)
+}
+
+func sendClientChaos(ctx context.Context, w testcontrol.MapStreamWriter, r *tailcfg.MapRequest) {
+	selfPub := r.NodeKey
+
+	nodeID := tailcfg.NodeID(0)
+	newNodeID := func() tailcfg.NodeID {
+		nodeID++
+		return nodeID
+	}
+
+	selfNodeID := newNodeID()
+	selfIP4 := node4(nodeID)
+	selfIP6 := node6(nodeID)
+
+	selfUserID := tailcfg.UserID(1_000_000)
+
+	var peers []*tailcfg.Node
+	for range *flagNFake {
+		nid := newNodeID()
+		v4, v6 := node4(nid), node6(nid)
+		user := selfUserID
+		if rand.IntN(2) == 0 {
+			// Randomly assign a different user to the peer.
+			// ...
+		}
+		peers = append(peers, &tailcfg.Node{
+			ID:                nid,
+			StableID:          tailcfg.StableNodeID(fmt.Sprintf("peer-%d", nid)),
+			Name:              fmt.Sprintf("peer-%d.troll.ts.net.", nid),
+			Key:               key.NewNode().Public(),
+			MachineAuthorized: true,
+			DiscoKey:          key.NewDisco().Public(),
+			Addresses:         []netip.Prefix{v4, v6},
+			AllowedIPs:        []netip.Prefix{v4, v6},
+			User:              user,
+		})
+	}
+
+	w.SendMapMessage(&tailcfg.MapResponse{
+		Node: &tailcfg.Node{
+			ID:                selfNodeID,
+			StableID:          "self",
+			Name:              "test-mctestfast.troll.ts.net.",
+			User:              selfUserID,
+			Key:               selfPub,
+			KeyExpiry:         time.Now().Add(5000 * time.Hour),
+			Machine:           key.NewMachine().Public(), // fake; client shouldn't care
+			DiscoKey:          r.DiscoKey,
+			MachineAuthorized: true,
+			Addresses:         []netip.Prefix{selfIP4, selfIP6},
+			AllowedIPs:        []netip.Prefix{selfIP4, selfIP6},
+			Capabilities:      []tailcfg.NodeCapability{},
+			CapMap:            map[tailcfg.NodeCapability][]tailcfg.RawMessage{},
+		},
+		DERPMap: &tailcfg.DERPMap{
+			Regions: map[int]*tailcfg.DERPRegion{
+				1: {RegionID: 1,
+					Nodes: []*tailcfg.DERPNode{{
+						RegionID:  1,
+						Name:      "1i",
+						IPv4:      "199.38.181.103",
+						IPv6:      "2607:f740:f::e19",
+						HostName:  "derp1i.tailscale.com",
+						CanPort80: true,
+					}}},
+			},
+		},
+		Peers: peers,
+	})
+
+	sendChange := func() error {
+		const (
+			actionToggleOnline = iota
+			numActions
+		)
+		action := rand.IntN(numActions)
+		switch action {
+		case actionToggleOnline:
+			peer := peers[rand.IntN(len(peers))]
+			online := peer.Online != nil && *peer.Online
+			peer.Online = ptr.To(!online)
+			var lastSeen *time.Time
+			if !online {
+				lastSeen = ptr.To(time.Now().UTC().Round(time.Second))
+			}
+			w.SendMapMessage(&tailcfg.MapResponse{
+				PeersChangedPatch: []*tailcfg.PeerChange{
+					{
+						NodeID:   peer.ID,
+						Online:   peer.Online,
+						LastSeen: lastSeen,
+					},
+				},
+			})
+		}
+		return nil
+	}
+
+	ticker := time.NewTicker(250 * time.Millisecond)
+	defer ticker.Stop()
+	for {
+		select {
+		case <-ticker.C:
+			if err := sendChange(); err != nil {
+				log.Printf("sendChange: %v", err)
+				return
+			}
+		case <-ctx.Done():
+			return
+		}
+	}
+}
+
+type fakeTB struct {
+	*testing.T
+}
+
+func (t fakeTB) Cleanup(_ func()) {}
+func (t fakeTB) Error(args ...any) {
+	t.Fatal(args...)
+}
+func (t fakeTB) Errorf(format string, args ...any) {
+	t.Fatalf(format, args...)
+}
+func (t fakeTB) Fail() {
+	t.Fatal("failed")
+}
+func (t fakeTB) FailNow() {
+	t.Fatal("failed")
+}
+func (t fakeTB) Failed() bool {
+	return false
+}
+func (t fakeTB) Fatal(args ...any) {
+	log.Fatal(args...)
+}
+func (t fakeTB) Fatalf(format string, args ...any) {
+	log.Fatalf(format, args...)
+}
+func (t fakeTB) Helper() {}
+func (t fakeTB) Log(args ...any) {
+	log.Print(args...)
+}
+func (t fakeTB) Logf(format string, args ...any) {
+	log.Printf(format, args...)
+}
+func (t fakeTB) Name() string {
+	return "faketest"
+}
+func (t fakeTB) Setenv(key string, value string) {
+	panic("not implemented")
+}
+func (t fakeTB) Skip(args ...any) {
+	t.Fatal("skipped")
+}
+func (t fakeTB) SkipNow() {
+	t.Fatal("skipnow")
+}
+func (t fakeTB) Skipf(format string, args ...any) {
+	t.Logf(format, args...)
+	t.Fatal("skipped")
+}
+func (t fakeTB) Skipped() bool {
+	return false
+}
+func (t fakeTB) TempDir() string {
+	panic("not implemented")
+}

tstest/integration/testcontrol/testcontrol.go

@@ -46,19 +46,22 @@ const msgLimit = 1 << 20 // encrypted message length limit
 // Server is a control plane server. Its zero value is ready for use.
 // Everything is stored in-memory in one tailnet.
 type Server struct {
-	Logf           logger.Logf      // nil means to use the log package
-	DERPMap        *tailcfg.DERPMap // nil means to use prod DERP map
-	RequireAuth    bool
-	RequireAuthKey string // required authkey for all nodes
-	Verbose        bool
-	DNSConfig      *tailcfg.DNSConfig // nil means no DNS config
-	MagicDNSDomain string
-	HandleC2N      http.Handler // if non-nil, used for /some-c2n-path/ in tests
+	Logf                 logger.Logf      // nil means to use the log package
+	DERPMap              *tailcfg.DERPMap // nil means to use prod DERP map
+	RequireAuth          bool
+	RequireAuthKey       string // required authkey for all nodes
+	Verbose              bool
+	DNSConfig            *tailcfg.DNSConfig // nil means no DNS config
+	MagicDNSDomain       string
+	HandleC2N            http.Handler // if non-nil, used for /some-c2n-path/ in tests
+	TolerateUnknownPaths bool         // if true, serve 404 instead of panicking on unknown URLs paths
 
 	// ExplicitBaseURL or HTTPTestServer must be set.
 	ExplicitBaseURL string           // e.g. "http://127.0.0.1:1234" with no trailing URL
 	HTTPTestServer  *httptest.Server // if non-nil, used to get BaseURL
 
+	AltMapStream func(context.Context, MapStreamWriter, *tailcfg.MapRequest)
+
 	initMuxOnce sync.Once
 	mux         *http.ServeMux
 
@@ -268,10 +271,15 @@ func (s *Server) initMux() {
 
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	s.initMuxOnce.Do(s.initMux)
+	log.Printf("control: %s %s", r.Method, r.URL.Path)
 	s.mux.ServeHTTP(w, r)
 }
 
 func (s *Server) serveUnhandled(w http.ResponseWriter, r *http.Request) {
+	if s.TolerateUnknownPaths {
+		http.Error(w, "unknown control URL", http.StatusNotFound)
+		return
+	}
 	var got bytes.Buffer
 	r.Write(&got)
 	go panic(fmt.Sprintf("testcontrol.Server received unhandled request: %s", got.Bytes()))
@@ -324,6 +332,13 @@ func (s *Server) ensureKeyPairLocked() {
 	s.pubKey = s.privKey.Public()
 }
 
+func (s *Server) SetPrivateKeys(noise key.MachinePrivate, legacy key.ControlPrivate) {
+	s.noisePrivKey = noise
+	s.noisePubKey = noise.Public()
+	s.privKey = legacy
+	s.pubKey = legacy.Public()
+}
+
 func (s *Server) serveKey(w http.ResponseWriter, r *http.Request) {
 	noiseKey, legacyKey := s.publicKeys()
 	if r.FormValue("v") == "" {
@@ -460,19 +475,20 @@ func (s *Server) AddFakeNode() {
 	mk := key.NewMachine().Public()
 	dk := key.NewDisco().Public()
 	r := nk.Raw32()
-	id := int64(binary.LittleEndian.Uint64(r[:]))
+	id := int64(binary.LittleEndian.Uint64(r[:]) >> 11)
 	ip := netaddr.IPv4(r[0], r[1], r[2], r[3])
 	addr := netip.PrefixFrom(ip, 32)
 	s.nodes[nk] = &tailcfg.Node{
 		ID:                tailcfg.NodeID(id),
 		StableID:          tailcfg.StableNodeID(fmt.Sprintf("TESTCTRL%08x", id)),
-		User:              tailcfg.UserID(id),
+		User:              123,
 		Machine:           mk,
 		Key:               nk,
 		MachineAuthorized: true,
 		DiscoKey:          dk,
 		Addresses:         []netip.Prefix{addr},
 		AllowedIPs:        []netip.Prefix{addr},
+		Name:              fmt.Sprintf("node-%d.big-troll.ts.net.", id),
 	}
 	// TODO: send updates to other (non-fake?) nodes
 }
@@ -613,7 +629,7 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key.
 		log.Printf("Got %T: %s", req, j)
 	}
 	if s.RequireAuthKey != "" && (req.Auth == nil || req.Auth.AuthKey != s.RequireAuthKey) {
-		res := must.Get(s.encode(false, tailcfg.RegisterResponse{
+		res := must.Get(encode(false, tailcfg.RegisterResponse{
 			Error: "invalid authkey",
 		}))
 		w.WriteHeader(200)
@@ -687,7 +703,7 @@ func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey key.
 		authURL = s.BaseURL() + authPath
 	}
 
-	res, err := s.encode(false, tailcfg.RegisterResponse{
+	res, err := encode(false, tailcfg.RegisterResponse{
 		User:              *user,
 		Login:             *login,
 		NodeKeyExpired:    allExpired,
@@ -765,6 +781,37 @@ func (s *Server) InServeMap() int {
 	return s.inServeMap
 }
 
+type MapStreamWriter interface {
+	SendMapMessage(*tailcfg.MapResponse) error
+}
+
+type mapStreamSender struct {
+	w        io.Writer
+	compress bool
+}
+
+func (s mapStreamSender) SendMapMessage(msg *tailcfg.MapResponse) error {
+	resBytes, err := encode(s.compress, msg)
+	if err != nil {
+		return err
+	}
+	if len(resBytes) > 16<<20 {
+		return fmt.Errorf("map message too big: %d", len(resBytes))
+	}
+	var siz [4]byte
+	binary.LittleEndian.PutUint32(siz[:], uint32(len(resBytes)))
+	if _, err := s.w.Write(siz[:]); err != nil {
+		return err
+	}
+	if _, err := s.w.Write(resBytes); err != nil {
+		return err
+	}
+	if f, ok := s.w.(http.Flusher); ok {
+		f.Flush()
+	}
+	return nil
+}
+
 func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.MachinePublic) {
 	s.incrInServeMap(1)
 	defer s.incrInServeMap(-1)
@@ -783,6 +830,19 @@ func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.Machi
 		go panic(fmt.Sprintf("bad map request: %v", err))
 	}
 
+	// ReadOnly implies no streaming, as it doesn't
+	// register an updatesCh to get updates.
+	streaming := req.Stream && !req.ReadOnly
+	compress := req.Compress != ""
+
+	if s.AltMapStream != nil {
+		s.AltMapStream(ctx, mapStreamSender{
+			w:        w,
+			compress: compress,
+		}, req)
+		return
+	}
+
 	jitter := rand.N(8 * time.Second)
 	keepAlive := 50*time.Second + jitter
 
@@ -832,11 +892,6 @@ func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey key.Machi
 	s.condLocked().Broadcast()
 	s.mu.Unlock()
 
-	// ReadOnly implies no streaming, as it doesn't
-	// register an updatesCh to get updates.
-	streaming := req.Stream && !req.ReadOnly
-	compress := req.Compress != ""
-
 	w.WriteHeader(200)
 	for {
 		if resBytes, ok := s.takeRawMapMessage(req.NodeKey); ok {
@@ -1063,7 +1118,7 @@ func (s *Server) takeRawMapMessage(nk key.NodePublic) (mapResJSON []byte, ok boo
 }
 
 func (s *Server) sendMapMsg(w http.ResponseWriter, compress bool, msg any) error {
-	resBytes, err := s.encode(compress, msg)
+	resBytes, err := encode(compress, msg)
 	if err != nil {
 		return err
 	}
@@ -1093,7 +1148,7 @@ func (s *Server) decode(msg []byte, v any) error {
 	return json.Unmarshal(msg, v)
 }
 
-func (s *Server) encode(compress bool, v any) (b []byte, err error) {
+func encode(compress bool, v any) (b []byte, err error) {
 	var isBytes bool
 	if b, isBytes = v.([]byte); !isBytes {
 		b, err = json.Marshal(v)

util/clientmetric/clientmetric.go

@@ -270,7 +270,7 @@ func (c *AggregateCounter) UnregisterAll() {
 // a sum of expvar variables registered with it.
 func NewAggregateCounter(name string) *AggregateCounter {
 	c := &AggregateCounter{counters: set.Set[*expvar.Int]{}}
-	NewGaugeFunc(name, c.Value)
+	NewCounterFunc(name, c.Value)
 	return c
 }
 

util/dnsname/dnsname.go

@@ -94,7 +94,8 @@ func (f FQDN) Contains(other FQDN) bool {
 	return strings.HasSuffix(other.WithTrailingDot(), cmp)
 }
 
-// ValidLabel reports whether label is a valid DNS label.
+// ValidLabel reports whether label is a valid DNS label. All errors are
+// [vizerror.Error].
 func ValidLabel(label string) error {
 	if len(label) == 0 {
 		return vizerror.New("empty DNS label")

util/usermetric/metrics.go

@@ -28,6 +28,22 @@ const (
 	// ReasonACL means that the packet was not permitted by ACL.
 	ReasonACL DropReason = "acl"
 
+	// ReasonMulticast means that the packet was dropped because it was a multicast packet.
+	ReasonMulticast DropReason = "multicast"
+
+	// ReasonLinkLocalUnicast means that the packet was dropped because it was a link-local unicast packet.
+	ReasonLinkLocalUnicast DropReason = "link_local_unicast"
+
+	// ReasonTooShort means that the packet was dropped because it was a bad packet,
+	// this could be due to a short packet.
+	ReasonTooShort DropReason = "too_short"
+
+	// ReasonFragment means that the packet was dropped because it was an IP fragment.
+	ReasonFragment DropReason = "fragment"
+
+	// ReasonUnknownProtocol means that the packet was dropped because it was an unknown protocol.
+	ReasonUnknownProtocol DropReason = "unknown_protocol"
+
 	// ReasonError means that the packet was dropped because of an error.
 	ReasonError DropReason = "error"
 )

wgengine/filter/filter.go

@@ -24,6 +24,7 @@ import (
 	"tailscale.com/types/views"
 	"tailscale.com/util/mak"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/usermetric"
 	"tailscale.com/wgengine/filter/filtertype"
 )
 
@@ -410,7 +411,7 @@ func (f *Filter) ShieldsUp() bool { return f.shieldsUp }
 // Tailscale peer.
 func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response {
 	dir := in
-	r := f.pre(q, rf, dir)
+	r, _ := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
 		return r
@@ -431,16 +432,16 @@ func (f *Filter) RunIn(q *packet.Parsed, rf RunFlags) Response {
 
 // RunOut determines whether this node is allowed to send q to a
 // Tailscale peer.
-func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) Response {
+func (f *Filter) RunOut(q *packet.Parsed, rf RunFlags) (Response, usermetric.DropReason) {
 	dir := out
-	r := f.pre(q, rf, dir)
+	r, reason := f.pre(q, rf, dir)
 	if r == Accept || r == Drop {
 		// already logged
-		return r
+		return r, reason
 	}
 	r, why := f.runOut(q)
 	f.logRateLimit(rf, q, dir, r, why)
-	return r
+	return r, ""
 }
 
 var unknownProtoStringCache sync.Map // ipproto.Proto -> string
@@ -610,33 +611,38 @@ var gcpDNSAddr = netaddr.IPv4(169, 254, 169, 254)
 
 // pre runs the direction-agnostic filter logic. dir is only used for
 // logging.
-func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) Response {
+func (f *Filter) pre(q *packet.Parsed, rf RunFlags, dir direction) (Response, usermetric.DropReason) {
 	if len(q.Buffer()) == 0 {
 		// wireguard keepalive packet, always permit.
-		return Accept
+		return Accept, ""
 	}
 	if len(q.Buffer()) < 20 {
 		f.logRateLimit(rf, q, dir, Drop, "too short")
-		return Drop
+		return Drop, usermetric.ReasonTooShort
+	}
+
+	if q.IPProto == ipproto.Unknown {
+		f.logRateLimit(rf, q, dir, Drop, "unknown proto")
+		return Drop, usermetric.ReasonUnknownProtocol
 	}
 
 	if q.Dst.Addr().IsMulticast() {
 		f.logRateLimit(rf, q, dir, Drop, "multicast")
-		return Drop
+		return Drop, usermetric.ReasonMulticast
 	}
 	if q.Dst.Addr().IsLinkLocalUnicast() && q.Dst.Addr() != gcpDNSAddr {
 		f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
-		return Drop
+		return Drop, usermetric.ReasonLinkLocalUnicast
 	}
 
 	if q.IPProto == ipproto.Fragment {
 		// Fragments after the first always need to be passed through.
 		// Very small fragments are considered Junk by Parsed.
 		f.logRateLimit(rf, q, dir, Accept, "fragment")
-		return Accept
+		return Accept, ""
 	}
 
-	return noVerdict
+	return noVerdict, ""
 }
 
 // loggingAllowed reports whether p can appear in logs at all.

wgengine/filter/filter_test.go

@@ -30,6 +30,7 @@ import (
 	"tailscale.com/types/views"
 	"tailscale.com/util/must"
 	"tailscale.com/util/slicesx"
+	"tailscale.com/util/usermetric"
 	"tailscale.com/wgengine/filter/filtertype"
 )
 
@@ -211,7 +212,7 @@ func TestUDPState(t *testing.T) {
 		t.Fatalf("incoming initial packet not dropped, got=%v: %v", got, a4)
 	}
 	// We talk to that peer
-	if got := acl.RunOut(&b4, flags); got != Accept {
+	if got, _ := acl.RunOut(&b4, flags); got != Accept {
 		t.Fatalf("outbound packet didn't egress, got=%v: %v", got, b4)
 	}
 	// Now, the same packet as before is allowed back.
@@ -227,7 +228,7 @@ func TestUDPState(t *testing.T) {
 		t.Fatalf("incoming initial packet not dropped: %v", a4)
 	}
 	// We talk to that peer
-	if got := acl.RunOut(&b6, flags); got != Accept {
+	if got, _ := acl.RunOut(&b6, flags); got != Accept {
 		t.Fatalf("outbound packet didn't egress: %v", b4)
 	}
 	// Now, the same packet as before is allowed back.
@@ -382,25 +383,27 @@ func BenchmarkFilter(b *testing.B) {
 
 func TestPreFilter(t *testing.T) {
 	packets := []struct {
-		desc string
-		want Response
-		b    []byte
+		desc       string
+		want       Response
+		wantReason usermetric.DropReason
+		b          []byte
 	}{
-		{"empty", Accept, []byte{}},
-		{"short", Drop, []byte("short")},
-		{"junk", Drop, raw4default(ipproto.Unknown, 10)},
-		{"fragment", Accept, raw4default(ipproto.Fragment, 40)},
-		{"tcp", noVerdict, raw4default(ipproto.TCP, 0)},
-		{"udp", noVerdict, raw4default(ipproto.UDP, 0)},
-		{"icmp", noVerdict, raw4default(ipproto.ICMPv4, 0)},
+		{"empty", Accept, "", []byte{}},
+		{"short", Drop, usermetric.ReasonTooShort, []byte("short")},
+		{"short-junk", Drop, usermetric.ReasonTooShort, raw4default(ipproto.Unknown, 10)},
+		{"long-junk", Drop, usermetric.ReasonUnknownProtocol, raw4default(ipproto.Unknown, 21)},
+		{"fragment", Accept, "", raw4default(ipproto.Fragment, 40)},
+		{"tcp", noVerdict, "", raw4default(ipproto.TCP, 0)},
+		{"udp", noVerdict, "", raw4default(ipproto.UDP, 0)},
+		{"icmp", noVerdict, "", raw4default(ipproto.ICMPv4, 0)},
 	}
 	f := NewAllowNone(t.Logf, &netipx.IPSet{})
 	for _, testPacket := range packets {
 		p := &packet.Parsed{}
 		p.Decode(testPacket.b)
-		got := f.pre(p, LogDrops|LogAccepts, in)
-		if got != testPacket.want {
-			t.Errorf("%q got=%v want=%v packet:\n%s", testPacket.desc, got, testPacket.want, packet.Hexdump(testPacket.b))
+		got, gotReason := f.pre(p, LogDrops|LogAccepts, in)
+		if got != testPacket.want || gotReason != testPacket.wantReason {
+			t.Errorf("%q got=%v want=%v gotReason=%s wantReason=%s packet:\n%s", testPacket.desc, got, testPacket.want, gotReason, testPacket.wantReason, packet.Hexdump(testPacket.b))
 		}
 	}
 }