diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 9ff12c76f..bdbe5bdd2 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -19,10 +19,6 @@ type Knobs struct { // DisableUPnP indicates whether to attempt UPnP mapping. 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 // programming and instead give WireGuard the full netmap always, even for // idle peers. @@ -110,7 +106,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { has := capMap.Contains var ( keepFullWG = has(tailcfg.NodeAttrDebugDisableWGTrim) - disableDRPO = has(tailcfg.NodeAttrDebugDisableDRPO) disableUPnP = has(tailcfg.NodeAttrDisableUPnP) randomizeClientPort = has(tailcfg.NodeAttrRandomizeClientPort) disableDeltaUpdates = has(tailcfg.NodeAttrDisableDeltaUpdates) @@ -136,7 +131,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { } k.KeepFullWGConfig.Store(keepFullWG) - k.DisableDRPO.Store(disableDRPO) k.DisableUPnP.Store(disableUPnP) k.RandomizeClientPort.Store(randomizeClientPort) k.OneCGNAT.Store(oneCGNAT) @@ -163,7 +157,6 @@ func (k *Knobs) AsDebugJSON() map[string]any { } return map[string]any{ "DisableUPnP": k.DisableUPnP.Load(), - "DisableDRPO": k.DisableDRPO.Load(), "KeepFullWGConfig": k.KeepFullWGConfig.Load(), "RandomizeClientPort": k.RandomizeClientPort.Load(), "OneCGNAT": k.OneCGNAT.Load(), diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 18076914c..9276cebc2 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2203,10 +2203,6 @@ const ( // always giving WireGuard the full netmap, even for idle peers. 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 // disabled if WPAD is present on the network. NodeAttrDisableSubnetsIfPAC NodeCapability = "debug-disable-subnets-if-pac" diff --git a/wgengine/magicsock/debugknobs.go b/wgengine/magicsock/debugknobs.go index a8bbc61ff..867109a42 100644 --- a/wgengine/magicsock/debugknobs.go +++ b/wgengine/magicsock/debugknobs.go @@ -20,9 +20,6 @@ var ( // debugOmitLocalAddresses removes all local interface addresses // from magicsock's discovered local endpoints. Used in some tests. 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 // full payload. logDerpVerbose = envknob.RegisterBool("TS_DEBUG_DERP") diff --git a/wgengine/magicsock/debugknobs_stubs.go b/wgengine/magicsock/debugknobs_stubs.go index de49865bf..7ee316e0f 100644 --- a/wgengine/magicsock/debugknobs_stubs.go +++ b/wgengine/magicsock/debugknobs_stubs.go @@ -22,8 +22,6 @@ func debugEnableSilentDisco() bool { return false } func debugSendCallMeUnknownPeer() 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 } diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 2a9bf051c..1735a71d8 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -41,32 +41,20 @@ import ( // preferredDERPFrameTime, so update with care. 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 -// peer should be available at DERP node derpID, as long as the -// current connection for that derpID is dc. (but dc should not be +// peer should be available at DERP regionID, as long as the +// current connection for that regionID is dc. (but dc should not be // used to write directly; it's owned by the read/write loops) type derpRoute struct { - derpID int - dc *derphttp.Client // don't use directly; see comment above + regionID int + dc *derphttp.Client // don't use directly; see comment above } // 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() defer c.mu.Unlock() - r2 := derpRoute{derpID, dc} + r2 := derpRoute{regionID, dc} if r, ok := c.derpRoute[peer]; ok && r == r2 { 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 // node in SF to reach us, so we can reply to them using our // SF connection rather than dialing Frankfurt. (Issue 150) - if !peer.IsZero() && c.useDerpRoute() { + if !peer.IsZero() { if r, ok := c.derpRoute[peer]; ok { - if ad, ok := c.activeDerp[r.derpID]; ok && ad.c == r.dc { - c.setPeerLastDerpLocked(peer, r.derpID, regionID) + if ad, ok := c.activeDerp[r.regionID]; ok && ad.c == r.dc { + c.setPeerLastDerpLocked(peer, r.regionID, regionID) *ad.lastWrite = time.Now() return ad.writeCh }