diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index a5b5ca203..a7d399daf 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -26,6 +26,7 @@ "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/tailscale/wireguard-go/wgcfg" @@ -37,6 +38,7 @@ "tailscale.com/net/tlsdial" "tailscale.com/tailcfg" "tailscale.com/types/logger" + "tailscale.com/types/opt" "tailscale.com/types/structs" "tailscale.com/version" ) @@ -621,8 +623,14 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM vlogf("netmap: new map contains DERP map") lastDERPMap = resp.DERPMap } - if resp.Debug != nil && resp.Debug.LogHeapPprof { - go logheap.LogHeap(resp.Debug.LogHeapURL) + if resp.Debug != nil { + if resp.Debug.LogHeapPprof { + go logheap.LogHeap(resp.Debug.LogHeapURL) + } + newv := resp.Debug.DERPRoute + if old, ok := controlUseDERPRoute.Load().(opt.Bool); !ok || old != newv { + controlUseDERPRoute.Store(newv) + } } // Temporarily (2020-06-29) support removing all but // discovery-supporting nodes during development, for @@ -953,3 +961,12 @@ func cloneNodes(v1 []*tailcfg.Node) []*tailcfg.Node { } return v2 } + +var controlUseDERPRoute atomic.Value + +// DERPRouteFlag reports the last reported value from control for whether +// DERP route optimization (Issue 150) should be enabled. +func DERPRouteFlag() opt.Bool { + v, _ := controlUseDERPRoute.Load().(opt.Bool) + return v +} diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 0d073a17d..21a6eb4e6 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -563,6 +563,13 @@ type Debug struct { // always do its background STUN queries (see magicsock's // periodicReSTUN), regardless of inactivity. ForceBackgroundSTUN bool `json:",omitempty"` + + // DERPRoute controls whether the DERP reverse path + // optimization (see Issue 150) should be enabled or + // disabled. The environment variable in magicsock is the + // highest priority (if set), then this (if set), then the + // binary default value. + DERPRoute opt.Bool `json:",omitempty"` } func (k MachineKey) String() string { return fmt.Sprintf("mkey:%x", k[:]) } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 67b2ae9b2..d20389a10 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -70,7 +70,8 @@ // debugUseDerpRoute temporarily (2020-03-22) controls whether DERP // reverse routing is enabled (Issue 150). It will become always true // later. - debugUseDerpRoute, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE")) + debugUseDerpRouteEnv = os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE") + debugUseDerpRoute, _ = strconv.ParseBool(debugUseDerpRouteEnv) // logDerpVerbose logs all received DERP packets, including their // full payload. logDerpVerbose, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DERP")) @@ -81,6 +82,19 @@ debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE")) ) +// useDerpRoute reports whether magicsock should enable the DERP +// return path optimization (Issue 150). +func useDerpRoute() bool { + if debugUseDerpRouteEnv != "" { + return debugUseDerpRoute + } + ob := controlclient.DERPRouteFlag() + if v, ok := ob.Get(); ok { + return v + } + return false +} + // inTest reports whether the running program is a test that set the // IN_TS_TEST environment variable. // @@ -1111,7 +1125,7 @@ func (c *Conn) derpWriteChanOfAddr(addr netaddr.IPPort, peer key.Public) 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() && debugUseDerpRoute { + if !peer.IsZero() && useDerpRoute() { 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)