wgengine/magicsock: replace CanPMTUD() with ShouldPMTUD()

Replace CanPMTUD() with ShouldPMTUD() to check if peer path MTU discovery should
be enabled, in preparation for adding support for enabling/disabling peer MTU
dynamically.

Updated #311

Signed-off-by: Val <valerie@tailscale.com>

wgengine/magicsock/debugknobs.go

@@ -53,9 +53,11 @@
 	// discovery on UDP connections between peers. Currently (2023-09-05)
 	// this only turns on the don't fragment bit for the magicsock UDP
 	// sockets.
-	debugEnablePMTUD = envknob.RegisterBool("TS_DEBUG_ENABLE_PMTUD")
-	// Hey you! Adding a new debugknob? Make sure to stub it out in the debugknob_stubs.go
-	// file too.
+	debugEnablePMTUD = envknob.RegisterOptBool("TS_DEBUG_ENABLE_PMTUD")
+	// debugPMTUD prints extra debugging about peer MTU path discovery.
+	debugPMTUD = envknob.RegisterBool("TS_DEBUG_PMTUD")
+	// Hey you! Adding a new debugknob? Make sure to stub it out in the
+	// debugknobs_stubs.go file too.
 )
 
 // inTest reports whether the running program is a test that set the

wgengine/magicsock/debugknobs_stubs.go

@@ -20,10 +20,11 @@ func debugAlwaysDERP() bool            { return false }
 func debugUseDERPHTTP() bool           { return false }
 func debugEnableSilentDisco() bool     { return false }
 func debugSendCallMeUnknownPeer() bool { return false }
-func debugEnablePMTUD() bool           { return false }
+func debugPMTUD() bool                 { return false }
 func debugUseDERPAddr() string         { return "" }
 func debugUseDerpRouteEnv() string     { return "" }
 func debugUseDerpRoute() opt.Bool      { return "" }
+func debugEnablePMTUD() opt.Bool       { return "" }
 func debugRingBufferMaxSizeBytes() int { return 0 }
 func inTest() bool                     { return false }
 func debugPeerMap() bool               { return false }

wgengine/magicsock/magicsock.go

@@ -165,6 +165,9 @@ type Conn struct {
 	// port is the preferred port from opts.Port; 0 means auto.
 	port atomic.Uint32
 
+	// peerMTUEnabled is whether path MTU discovery to peers is enabled.
+	peerMTUEnabled atomic.Bool
+
 	// stats maintains per-connection counters.
 	stats atomic.Pointer[connstats.Statistics]
 
@@ -2310,15 +2313,6 @@ func (c *Conn) bindSocket(ruc *RebindingUDPConn, network string, curPortFate cur
 		}
 		trySetSocketBuffer(pconn, c.logf)
 
-		if CanPMTUD() {
-			err = c.setDontFragment(network, true)
-			if err != nil {
-				c.logf("magicsock: set dontfragment failed for %v port %d: %v", network, port, err)
-				// TODO disable PMTUD in this case. We don't expect the setsockopt to fail on
-				// supported platforms, but we might as well be paranoid.
-			}
-		}
-
 		// Success.
 		if debugBindSocket() {
 			c.logf("magicsock: bindSocket: successfully listened %v port %d", network, port)
@@ -2358,6 +2352,7 @@ func (c *Conn) rebind(curPortFate currentPortFate) error {
 		return fmt.Errorf("magicsock: Rebind IPv4 failed: %w", err)
 	}
 	c.portMapper.SetLocalPort(c.LocalPort())
+	c.UpdatePMTUD()
 	return nil
 }
 

wgengine/magicsock/peermtu.go

@@ -0,0 +1,99 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build (darwin && !ios) || (linux && !android)
+
+package magicsock
+
+// Peer path MTU routines shared by platforms that implement it.
+
+// DontFragSetting returns true if at least one of the underlying sockets of
+// this connection is a UDP socket with the don't fragment bit set, otherwise it
+// returns false. It also returns an error if either connection returned an error
+// other than errUnsupportedConnType.
+func (c *Conn) DontFragSetting() (bool, error) {
+	df4, err4 := c.getDontFragment("udp4")
+	df6, err6 := c.getDontFragment("udp6")
+	df := df4 || df6
+	err := err4
+	if err4 != nil && err4 != errUnsupportedConnType {
+		err = err6
+	}
+	if err == errUnsupportedConnType {
+		err = nil
+	}
+	return df, err
+}
+
+// ShouldPMTUD returns true if this client should try to enable peer MTU
+// discovery, false otherwise.
+func (c *Conn) ShouldPMTUD() bool {
+	if v, ok := debugEnablePMTUD().Get(); ok {
+		if debugPMTUD() {
+			c.logf("magicsock: peermtu: peer path MTU discovery set via envknob to %v", v)
+		}
+		return v
+	}
+	if debugPMTUD() {
+		c.logf("magicsock: peermtu: peer path MTU discovery set by default to false")
+	}
+	return false // Until we feel confident PMTUD is solid.
+}
+
+// PeerMTUEnabled returns true if this Conn is has peer path MTU discovery enabled.
+func (c *Conn) PeerMTUEnabled() bool {
+	return c.peerMTUEnabled.Load()
+}
+
+// UpdatePMTUD configures underlying sockets of this Conn to enable or disable
+// peer path MTU discovery according to the current configuration.
+//
+// Enabling or disabling peer path MTU discovery requires setting the don't
+// fragment bit on its two underlying pconns. There are three distinct results
+// for this operation on each pconn:
+//
+// 1. Success
+// 2. Failure (not supported on this platform, or supported but failed)
+// 3. Not a UDP socket (most likely one of IPv4 or IPv6 couldn't be used)
+//
+// To simplify the fast path for the most common case, we set the PMTUD status
+// of the overall Conn according to the results of setting the sockopt on pconn
+// as follows:
+//
+// 1. Both setsockopts succeed: PMTUD status update succeeds
+// 2. One succeeds, one returns not a UDP socket: PMTUD status update succeeds
+// 4. Neither setsockopt succeeds: PMTUD disabled
+// 3. Either setsockopt fails: PMTUD disabled
+//
+// If the PMTUD settings changed, it resets the endpoint state so that it will
+// re-probe path MTUs to this peer.
+func (c *Conn) UpdatePMTUD() {
+	if debugPMTUD() {
+		df4, err4 := c.getDontFragment("udp4")
+		df6, err6 := c.getDontFragment("udp6")
+		c.logf("magicsock: peermtu: peer MTU status %v DF bit status: v4: %v (%v) v6: %v (%v)", c.peerMTUEnabled.Load(), df4, err4, df6, err6)
+	}
+
+	enable := c.ShouldPMTUD()
+	if c.peerMTUEnabled.Load() == enable {
+		c.logf("magicsock: peermtu: peer MTU status is %v", enable)
+		return
+	}
+
+	newStatus := enable
+	err4 := c.setDontFragment("udp4", enable)
+	err6 := c.setDontFragment("udp6", enable)
+	anySuccess := err4 == nil || err6 == nil
+	noFailures := (err4 == nil || err4 == errUnsupportedConnType) && (err6 == nil || err6 == errUnsupportedConnType)
+
+	if anySuccess && noFailures {
+		c.logf("magicsock: peermtu: peer MTU status updated to %v", newStatus)
+	} else {
+		c.logf("[unexpected] magicsock: peermtu: updating peer MTU status to %v failed (v4: %v, v6: %v), disabling", enable, err4, err6)
+		_ = c.setDontFragment("udp4", false)
+		_ = c.setDontFragment("udp6", false)
+		newStatus = false
+	}
+	c.peerMTUEnabled.Store(newStatus)
+	c.resetEndpointStates()
+}

wgengine/magicsock/peermtu_stubs.go

@@ -30,7 +30,17 @@ func (c *Conn) getDontFragment(network string) (bool, error) {
 	return false, nil
 }
 
-// CanPMTUD returns whether this platform supports performing peet path MTU discovery.
-func CanPMTUD() bool {
+func (c *Conn) DontFragSetting() (bool, error) {
+	return false, nil
+}
+
+func (c *Conn) ShouldPMTUD() bool {
 	return false
 }
+
+func (c *Conn) PeerMTUEnabled() bool {
+	return false
+}
+
+func (c *Conn) UpdatePMTUD() {
+}

wgengine/magicsock/peermtu_unix.go

@@ -40,7 +40,3 @@ func (c *Conn) connControl(network string, fn func(fd uintptr)) error {
 	}
 	return rc.Control(fn)
 }
-
-func CanPMTUD() bool {
-	return debugEnablePMTUD()
-}

wgengine/userspace.go

@@ -804,6 +804,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 		listenPort = 0
 	}
 
+	peerMTUEnable := e.magicConn.ShouldPMTUD()
+
 	isSubnetRouter := false
 	if e.birdClient != nil && nm != nil && nm.SelfNode.Valid() {
 		isSubnetRouter = hasOverlap(nm.SelfNode.PrimaryRoutes(), nm.SelfNode.Hostinfo().RoutableIPs())
@@ -818,7 +820,9 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 		RouterConfig *router.Config
 		DNSConfig    *dns.Config
 	}{routerCfg, dnsCfg})
-	if !engineChanged && !routerChanged && listenPort == e.magicConn.LocalPort() && !isSubnetRouterChanged {
+	listenPortChanged := listenPort != e.magicConn.LocalPort()
+	peerMTUChanged := peerMTUEnable != e.magicConn.PeerMTUEnabled()
+	if !engineChanged && !routerChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged {
 		return ErrNoChanges
 	}
 	newLogIDs := cfg.NetworkLogging
@@ -874,6 +878,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	}
 	e.magicConn.UpdatePeers(peerSet)
 	e.magicConn.SetPreferredPort(listenPort)
+	e.magicConn.UpdatePMTUD()
 
 	if err := e.maybeReconfigWireguardLocked(discoChanged); err != nil {
 		return err