control/controlknobs,tailcfg,wgengine/magicsock: remove DRPO shutoff switch

The DERP Return Path Optimization (DRPO) is over four years old (and
on by default for over two) and we haven't had problems, so time to
remove the emergency shutoff code (controlknob) which we've never
used. The controlknobs are only meant for new features, to mitigate
risk. But we don't want to keep them forever, as they kinda pollute
the code.

Updates #150

Change-Id: If021bc8fd1b51006d8bddd1ffab639bb1abb0ad1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
pull/12725/head
Brad Fitzpatrick 5 months ago committed by Brad Fitzpatrick
parent 9df107f4f0
commit d2fef01206

@ -19,10 +19,6 @@ type Knobs struct {
// DisableUPnP indicates whether to attempt UPnP mapping. // DisableUPnP indicates whether to attempt UPnP mapping.
DisableUPnP atomic.Bool DisableUPnP atomic.Bool
// DisableDRPO is whether control says to disable the
// DERP route optimization (Issue 150).
DisableDRPO atomic.Bool
// KeepFullWGConfig is whether we should disable the lazy wireguard // KeepFullWGConfig is whether we should disable the lazy wireguard
// programming and instead give WireGuard the full netmap always, even for // programming and instead give WireGuard the full netmap always, even for
// idle peers. // idle peers.
@ -110,7 +106,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) {
has := capMap.Contains has := capMap.Contains
var ( var (
keepFullWG = has(tailcfg.NodeAttrDebugDisableWGTrim) keepFullWG = has(tailcfg.NodeAttrDebugDisableWGTrim)
disableDRPO = has(tailcfg.NodeAttrDebugDisableDRPO)
disableUPnP = has(tailcfg.NodeAttrDisableUPnP) disableUPnP = has(tailcfg.NodeAttrDisableUPnP)
randomizeClientPort = has(tailcfg.NodeAttrRandomizeClientPort) randomizeClientPort = has(tailcfg.NodeAttrRandomizeClientPort)
disableDeltaUpdates = has(tailcfg.NodeAttrDisableDeltaUpdates) disableDeltaUpdates = has(tailcfg.NodeAttrDisableDeltaUpdates)
@ -136,7 +131,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) {
} }
k.KeepFullWGConfig.Store(keepFullWG) k.KeepFullWGConfig.Store(keepFullWG)
k.DisableDRPO.Store(disableDRPO)
k.DisableUPnP.Store(disableUPnP) k.DisableUPnP.Store(disableUPnP)
k.RandomizeClientPort.Store(randomizeClientPort) k.RandomizeClientPort.Store(randomizeClientPort)
k.OneCGNAT.Store(oneCGNAT) k.OneCGNAT.Store(oneCGNAT)
@ -163,7 +157,6 @@ func (k *Knobs) AsDebugJSON() map[string]any {
} }
return map[string]any{ return map[string]any{
"DisableUPnP": k.DisableUPnP.Load(), "DisableUPnP": k.DisableUPnP.Load(),
"DisableDRPO": k.DisableDRPO.Load(),
"KeepFullWGConfig": k.KeepFullWGConfig.Load(), "KeepFullWGConfig": k.KeepFullWGConfig.Load(),
"RandomizeClientPort": k.RandomizeClientPort.Load(), "RandomizeClientPort": k.RandomizeClientPort.Load(),
"OneCGNAT": k.OneCGNAT.Load(), "OneCGNAT": k.OneCGNAT.Load(),

@ -2203,10 +2203,6 @@ const (
// always giving WireGuard the full netmap, even for idle peers. // always giving WireGuard the full netmap, even for idle peers.
NodeAttrDebugDisableWGTrim NodeCapability = "debug-no-wg-trim" NodeAttrDebugDisableWGTrim NodeCapability = "debug-no-wg-trim"
// NodeAttrDebugDisableDRPO disables the DERP Return Path Optimization.
// See Issue 150.
NodeAttrDebugDisableDRPO NodeCapability = "debug-disable-drpo"
// NodeAttrDisableSubnetsIfPAC controls whether subnet routers should be // NodeAttrDisableSubnetsIfPAC controls whether subnet routers should be
// disabled if WPAD is present on the network. // disabled if WPAD is present on the network.
NodeAttrDisableSubnetsIfPAC NodeCapability = "debug-disable-subnets-if-pac" NodeAttrDisableSubnetsIfPAC NodeCapability = "debug-disable-subnets-if-pac"

@ -20,9 +20,6 @@ var (
// debugOmitLocalAddresses removes all local interface addresses // debugOmitLocalAddresses removes all local interface addresses
// from magicsock's discovered local endpoints. Used in some tests. // from magicsock's discovered local endpoints. Used in some tests.
debugOmitLocalAddresses = envknob.RegisterBool("TS_DEBUG_OMIT_LOCAL_ADDRS") debugOmitLocalAddresses = envknob.RegisterBool("TS_DEBUG_OMIT_LOCAL_ADDRS")
// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
// reverse routing is enabled (Issue 150).
debugUseDerpRoute = envknob.RegisterOptBool("TS_DEBUG_ENABLE_DERP_ROUTE")
// logDerpVerbose logs all received DERP packets, including their // logDerpVerbose logs all received DERP packets, including their
// full payload. // full payload.
logDerpVerbose = envknob.RegisterBool("TS_DEBUG_DERP") logDerpVerbose = envknob.RegisterBool("TS_DEBUG_DERP")

@ -22,8 +22,6 @@ func debugEnableSilentDisco() bool { return false }
func debugSendCallMeUnknownPeer() bool { return false } func debugSendCallMeUnknownPeer() bool { return false }
func debugPMTUD() bool { return false } func debugPMTUD() bool { return false }
func debugUseDERPAddr() string { return "" } func debugUseDERPAddr() string { return "" }
func debugUseDerpRouteEnv() string { return "" }
func debugUseDerpRoute() opt.Bool { return "" }
func debugEnablePMTUD() opt.Bool { return "" } func debugEnablePMTUD() opt.Bool { return "" }
func debugRingBufferMaxSizeBytes() int { return 0 } func debugRingBufferMaxSizeBytes() int { return 0 }
func inTest() bool { return false } func inTest() bool { return false }

@ -41,32 +41,20 @@ import (
// preferredDERPFrameTime, so update with care. // preferredDERPFrameTime, so update with care.
const frameReceiveRecordRate = 5 * time.Second const frameReceiveRecordRate = 5 * time.Second
// useDerpRoute reports whether magicsock should enable the DERP
// return path optimization (Issue 150).
//
// By default it's enabled, unless an environment variable
// or control says to disable it.
func (c *Conn) useDerpRoute() bool {
if b, ok := debugUseDerpRoute().Get(); ok {
return b
}
return c.controlKnobs == nil || !c.controlKnobs.DisableDRPO.Load()
}
// derpRoute is a route entry for a public key, saying that a certain // derpRoute is a route entry for a public key, saying that a certain
// peer should be available at DERP node derpID, as long as the // peer should be available at DERP regionID, as long as the
// current connection for that derpID is dc. (but dc should not be // current connection for that regionID is dc. (but dc should not be
// used to write directly; it's owned by the read/write loops) // used to write directly; it's owned by the read/write loops)
type derpRoute struct { type derpRoute struct {
derpID int regionID int
dc *derphttp.Client // don't use directly; see comment above dc *derphttp.Client // don't use directly; see comment above
} }
// removeDerpPeerRoute removes a DERP route entry previously added by addDerpPeerRoute. // removeDerpPeerRoute removes a DERP route entry previously added by addDerpPeerRoute.
func (c *Conn) removeDerpPeerRoute(peer key.NodePublic, derpID int, dc *derphttp.Client) { func (c *Conn) removeDerpPeerRoute(peer key.NodePublic, regionID int, dc *derphttp.Client) {
c.mu.Lock() c.mu.Lock()
defer c.mu.Unlock() defer c.mu.Unlock()
r2 := derpRoute{derpID, dc} r2 := derpRoute{regionID, dc}
if r, ok := c.derpRoute[peer]; ok && r == r2 { if r, ok := c.derpRoute[peer]; ok && r == r2 {
delete(c.derpRoute, peer) delete(c.derpRoute, peer)
} }
@ -365,10 +353,10 @@ func (c *Conn) derpWriteChanForRegion(regionID int, peer key.NodePublic) chan<-
// perhaps peer's home is Frankfurt, but they dialed our home DERP // perhaps peer's home is Frankfurt, but they dialed our home DERP
// node in SF to reach us, so we can reply to them using our // node in SF to reach us, so we can reply to them using our
// SF connection rather than dialing Frankfurt. (Issue 150) // SF connection rather than dialing Frankfurt. (Issue 150)
if !peer.IsZero() && c.useDerpRoute() { if !peer.IsZero() {
if r, ok := c.derpRoute[peer]; ok { if r, ok := c.derpRoute[peer]; ok {
if ad, ok := c.activeDerp[r.derpID]; ok && ad.c == r.dc { if ad, ok := c.activeDerp[r.regionID]; ok && ad.c == r.dc {
c.setPeerLastDerpLocked(peer, r.derpID, regionID) c.setPeerLastDerpLocked(peer, r.regionID, regionID)
*ad.lastWrite = time.Now() *ad.lastWrite = time.Now()
return ad.writeCh return ad.writeCh
} }

Loading…
Cancel
Save