envknob,net/tstun,wgengine: use TS_DEBUG_MTU consistently

Noted on #5915 TS_DEBUG_MTU was not used consistently everywhere.
Extract the default into a function that can apply this centrally and
use it everywhere.

Added envknob.Lookup{Int,Uint}Sized to make it easier to keep CodeQL
happy when using converted values.

Updates #5915

Signed-off-by: James Tucker <james@tailscale.com>

envknob/envknob.go

@@ -330,6 +330,46 @@ func LookupInt(envVar string) (v int, ok bool) {
 	panic("unreachable")
 }
 
+// LookupIntSized returns the integer value of the named environment value
+// parsed in base and with a maximum bit size bitSize.
+// The ok result is whether a value was set.
+// If the value isn't a valid int, it exits the program with a failure.
+func LookupIntSized(envVar string, base, bitSize int) (v int, ok bool) {
+	assertNotInInit()
+	val := os.Getenv(envVar)
+	if val == "" {
+		return 0, false
+	}
+	i, err := strconv.ParseInt(val, base, bitSize)
+	if err == nil {
+		v = int(i)
+		noteEnv(envVar, val)
+		return v, true
+	}
+	log.Fatalf("invalid integer environment variable %s: %v", envVar, val)
+	panic("unreachable")
+}
+
+// LookupUintSized returns the unsigned integer value of the named environment
+// value parsed in base and with a maximum bit size bitSize.
+// The ok result is whether a value was set.
+// If the value isn't a valid int, it exits the program with a failure.
+func LookupUintSized(envVar string, base, bitSize int) (v uint, ok bool) {
+	assertNotInInit()
+	val := os.Getenv(envVar)
+	if val == "" {
+		return 0, false
+	}
+	i, err := strconv.ParseUint(val, base, bitSize)
+	if err == nil {
+		v = uint(i)
+		noteEnv(envVar, val)
+		return v, true
+	}
+	log.Fatalf("invalid unsigned integer environment variable %s: %v", envVar, val)
+	panic("unreachable")
+}
+
 // UseWIPCode is whether TAILSCALE_USE_WIP_CODE is set to permit use
 // of Work-In-Progress code.
 func UseWIPCode() bool { return Bool("TAILSCALE_USE_WIP_CODE") }

net/tstun/constants.go

@@ -1,15 +0,0 @@
-// Copyright (c) Tailscale Inc & AUTHORS
-// SPDX-License-Identifier: BSD-3-Clause
-
-package tstun
-
-// DefaultMTU is the Tailscale default MTU for now.
-//
-// wireguard-go defaults to 1420 bytes, which only works if the
-// "outer" MTU is 1500 bytes. This breaks on DSL connections
-// (typically 1492 MTU) and on GCE (1460 MTU?!).
-//
-// 1280 is the smallest MTU allowed for IPv6, which is a sensible
-// "probably works everywhere" setting until we develop proper PMTU
-// discovery.
-const DefaultMTU = 1280

net/tstun/mtu.go

@@ -0,0 +1,33 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+package tstun
+
+import "tailscale.com/envknob"
+
+const (
+	maxMTU     uint32 = 65536
+	defaultMTU uint32 = 1280
+)
+
+// DefaultMTU returns either the constant default MTU of 1280, or the value set
+// in TS_DEBUG_MTU clamped to a maximum of 65536.
+func DefaultMTU() uint32 {
+	// DefaultMTU is the Tailscale default MTU for now.
+	//
+	// wireguard-go defaults to 1420 bytes, which only works if the
+	// "outer" MTU is 1500 bytes. This breaks on DSL connections
+	// (typically 1492 MTU) and on GCE (1460 MTU?!).
+	//
+	// 1280 is the smallest MTU allowed for IPv6, which is a sensible
+	// "probably works everywhere" setting until we develop proper PMTU
+	// discovery.
+	tunMTU := defaultMTU
+	if mtu, ok := envknob.LookupUintSized("TS_DEBUG_MTU", 10, 32); ok {
+		mtu := uint32(mtu)
+		if mtu > maxMTU {
+			mtu = maxMTU
+		}
+		tunMTU = mtu
+	}
+	return tunMTU
+}

net/tstun/mtu_test.go

@@ -0,0 +1,28 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+package tstun
+
+import (
+	"os"
+	"testing"
+)
+
+func TestDefaultMTU(t *testing.T) {
+	orig := os.Getenv("TS_DEBUG_MTU")
+	defer os.Setenv("TS_DEBUG_MTU", orig)
+
+	os.Setenv("TS_DEBUG_MTU", "")
+	if DefaultMTU() != 1280 {
+		t.Errorf("DefaultMTU() = %d, want 1280", DefaultMTU())
+	}
+
+	os.Setenv("TS_DEBUG_MTU", "9000")
+	if DefaultMTU() != 9000 {
+		t.Errorf("DefaultMTU() = %d, want 9000", DefaultMTU())
+	}
+
+	os.Setenv("TS_DEBUG_MTU", "123456789")
+	if DefaultMTU() != maxMTU {
+		t.Errorf("DefaultMTU() = %d, want %d", DefaultMTU(), maxMTU)
+	}
+}

net/tstun/tun.go

@@ -14,7 +14,6 @@
 	"time"
 
 	"github.com/tailscale/wireguard-go/tun"
-	"tailscale.com/envknob"
 	"tailscale.com/types/logger"
 )
 
@@ -45,11 +44,7 @@ func New(logf logger.Logf, tunName string) (tun.Device, string, error) {
 		}
 		dev, err = createTAP(tapName, bridgeName)
 	} else {
-		tunMTU := DefaultMTU
-		if mtu, ok := envknob.LookupInt("TS_DEBUG_MTU"); ok {
-			tunMTU = mtu
-		}
-		dev, err = tun.CreateTUN(tunName, tunMTU)
+		dev, err = tun.CreateTUN(tunName, int(DefaultMTU()))
 	}
 	if err != nil {
 		return nil, "", err

wgengine/netstack/netstack.go

@@ -146,7 +146,6 @@ type Impl struct {
 }
 
 const nicID = 1
-const mtu = tstun.DefaultMTU
 
 // maxUDPPacketSize is the maximum size of a UDP packet we copy in startPacketCopy
 // when relaying UDP packets. We don't use the 'mtu' const in anticipation of
@@ -179,7 +178,7 @@ func Create(logf logger.Logf, tundev *tstun.Wrapper, e wgengine.Engine, mc *magi
 	if tcpipErr != nil {
 		return nil, fmt.Errorf("could not enable TCP SACK: %v", tcpipErr)
 	}
-	linkEP := channel.New(512, mtu, "")
+	linkEP := channel.New(512, tstun.DefaultMTU(), "")
 	if tcpipProblem := ipstack.CreateNIC(nicID, linkEP); tcpipProblem != nil {
 		return nil, fmt.Errorf("could not create netstack NIC: %v", tcpipProblem)
 	}
@@ -1078,7 +1077,7 @@ func (ns *Impl) acceptUDP(r *udp.ForwarderRequest) {
 func (ns *Impl) handleMagicDNSUDP(srcAddr netip.AddrPort, c *gonet.UDPConn) {
 	// In practice, implementations are advised not to exceed 512 bytes
 	// due to fragmenting. Just to be sure, we bump all the way to the MTU.
-	const maxUDPReqSize = mtu
+	var maxUDPReqSize = tstun.DefaultMTU()
 	// Packets are being generated by the local host, so there should be
 	// very, very little latency. 150ms was chosen as something of an upper
 	// bound on resource usage, while hopefully still being long enough for

wgengine/router/ifconfig_windows.go

@@ -241,7 +241,7 @@ func interfaceFromLUID(luid winipcfg.LUID, flags winipcfg.GAAFlags) (*winipcfg.I
 var networkCategoryWarning = health.NewWarnable(health.WithMapDebugFlag("warn-network-category-unhealthy"))
 
 func configureInterface(cfg *Config, tun *tun.NativeTun) (retErr error) {
-	const mtu = tstun.DefaultMTU
+	var mtu = tstun.DefaultMTU()
 	luid := winipcfg.LUID(tun.LUID())
 	iface, err := interfaceFromLUID(luid,
 		// Issue 474: on early boot, when the network is still