From ab2380a6312858ccf95451787f33acc9c9fca066 Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Tue, 29 Sep 2026 15:14:59 -0700 Subject: [PATCH] net/dns,util/coalescedop: serialize and coalesce DNS flush calls on Windows Previously, ipconfig /flushdns and ipconfig /registerdns were called from multiple sites without coordination: the DNS manager's SetDNS (in a fire-and-forget goroutine), the router's winRouter.Set (synchronously on every route change), the link change handler, and session unlock. These could fire concurrently or in rapid succession, spawning multiple ipconfig processes and putting excessive pressure on the Dnscache service. In this PR, we introduce a new util/coalescedop.CoalescedOp type that provides "one in-flight, at most one pending" semantics: Do() is non-blocking, at most one execution runs at a time, and concurrent/rapid successive requests coalesce into a single follow-up execution. We then use CoalescedOp for both ipconfig /flushdns and ipconfig /registerdns, replacing the fire-and-forget goroutine in windowsManager.SetDNS and in the session unlock handler in tailscaled. We also remove the redundant dns.Flush() call from winRouter.Set, since every meaningful router.Set is followed by dns.Set in Reconfig, which already flushes. Updates #21001 Signed-off-by: Nick Khyl --- cmd/derper/depaware.txt | 2 +- cmd/k8s-operator/depaware.txt | 23 +++--- cmd/stund/depaware.txt | 2 +- cmd/tailscale/depaware.txt | 2 +- cmd/tailscaled/depaware.txt | 3 +- cmd/tailscaled/tailscaled_windows.go | 7 +- cmd/tsidp/depaware.txt | 1 + net/dns/flush_windows.go | 66 +++++++++++++--- net/dns/manager_windows.go | 39 ++-------- tsnet/depaware.txt | 1 + util/coalescedop/coalescedop.go | 66 ++++++++++++++++ util/coalescedop/coalescedop_test.go | 91 ++++++++++++++++++++++ wgengine/router/osrouter/router_windows.go | 6 -- 13 files changed, 239 insertions(+), 70 deletions(-) create mode 100644 util/coalescedop/coalescedop.go create mode 100644 util/coalescedop/coalescedop_test.go diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 89204c890..3b7aa26fc 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -371,7 +371,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa internal/strconv from internal/poll+ internal/stringslite from embed+ internal/sync from sync+ - internal/synctest from sync+ + internal/synctest from net/http/internal/http2+ internal/syscall/execenv from os+ LD internal/syscall/unix from crypto/internal/sysrand+ W internal/syscall/windows from crypto/internal/fips140deps/time+ diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index edeeae018..ab1c8c5c4 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -889,6 +889,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/util/cloudenv from tailscale.com/hostinfo+ tailscale.com/util/cloudinfo from tailscale.com/wgengine/magicsock LW tailscale.com/util/cmpver from tailscale.com/net/dns+ + W tailscale.com/util/coalescedop from tailscale.com/net/dns tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/util/syspolicy/setting tailscale.com/util/def from tailscale.com/ipn/localapi @@ -1039,23 +1040,23 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ crypto/internal/boring/bbig from crypto/ecdsa+ crypto/internal/boring/sig from crypto/internal/boring crypto/internal/constanttime from crypto/internal/fips140/edwards25519+ - crypto/internal/fips140 from crypto/internal/fips140/aes+ + crypto/internal/fips140 from crypto/fips140+ crypto/internal/fips140/aes from crypto/aes+ crypto/internal/fips140/aes/gcm from crypto/cipher+ crypto/internal/fips140/alias from crypto/cipher+ crypto/internal/fips140/bigmod from crypto/internal/fips140/ecdsa+ - crypto/internal/fips140/check from crypto/internal/fips140/aes+ - crypto/internal/fips140/drbg from crypto/internal/fips140/aes/gcm+ + crypto/internal/fips140/check from crypto/fips140+ + crypto/internal/fips140/drbg from crypto/hpke+ crypto/internal/fips140/ecdh from crypto/ecdh crypto/internal/fips140/ecdsa from crypto/ecdsa crypto/internal/fips140/ed25519 from crypto/ed25519 crypto/internal/fips140/edwards25519 from crypto/internal/fips140/ed25519 crypto/internal/fips140/edwards25519/field from crypto/ecdh+ - crypto/internal/fips140/hkdf from crypto/internal/fips140/tls13+ + crypto/internal/fips140/hkdf from crypto/hkdf+ crypto/internal/fips140/hmac from crypto/hmac+ crypto/internal/fips140/mldsa from crypto/mldsa crypto/internal/fips140/mlkem from crypto/mlkem - crypto/internal/fips140/nistec from crypto/elliptic+ + crypto/internal/fips140/nistec from crypto/ecdsa+ crypto/internal/fips140/nistec/fiat from crypto/internal/fips140/nistec crypto/internal/fips140/rsa from crypto/rsa crypto/internal/fips140/sha256 from crypto/internal/fips140/check+ @@ -1142,14 +1143,14 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ internal/filepathlite from os+ internal/fmtsort from fmt+ internal/goarch from crypto/internal/fips140deps/cpu+ - internal/godebug from crypto/internal/fips140deps/godebug+ + internal/godebug from crypto/ed25519+ internal/godebugs from internal/godebug+ internal/goexperiment from internal/runtime/maps+ internal/goos from crypto/x509+ internal/lazyregexp from go/doc internal/msan from internal/runtime/maps+ internal/nettrace from net+ - internal/oserror from io/fs+ + internal/oserror from internal/syscall/windows+ internal/poll from net+ internal/profile from net/http/pprof internal/profilerecord from runtime+ @@ -1159,9 +1160,9 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ internal/runtime/atomic from internal/runtime/exithook+ L internal/runtime/cgroup from runtime internal/runtime/exithook from runtime - internal/runtime/gc from runtime+ + internal/runtime/gc from internal/runtime/gc/scan+ internal/runtime/gc/scan from runtime - internal/runtime/maps from reflect+ + internal/runtime/maps from hash/maphash+ internal/runtime/math from internal/runtime/maps+ internal/runtime/pprof/label from runtime+ internal/runtime/sys from crypto/subtle+ @@ -1172,10 +1173,10 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ internal/strconv from internal/poll+ internal/stringslite from embed+ internal/sync from sync+ - internal/synctest from sync+ + internal/synctest from net/http/internal/http2+ internal/syscall/execenv from os+ LD internal/syscall/unix from crypto/internal/sysrand+ - W internal/syscall/windows from crypto/internal/sysrand+ + W internal/syscall/windows from crypto/internal/fips140deps/time+ W internal/syscall/windows/registry from mime+ W internal/syscall/windows/sysdll from internal/syscall/windows+ internal/testlog from os diff --git a/cmd/stund/depaware.txt b/cmd/stund/depaware.txt index 1c280c825..8fab9523f 100644 --- a/cmd/stund/depaware.txt +++ b/cmd/stund/depaware.txt @@ -258,7 +258,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar internal/strconv from internal/poll+ internal/stringslite from embed+ internal/sync from sync+ - internal/synctest from sync+ + internal/synctest from net/http/internal/http2+ internal/syscall/execenv from os LD internal/syscall/unix from crypto/internal/sysrand+ W internal/syscall/windows from crypto/internal/fips140deps/time+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 4be4bfcd0..c775daa1b 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -569,7 +569,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep internal/strconv from internal/poll+ internal/stringslite from embed+ internal/sync from sync+ - internal/synctest from sync+ + internal/synctest from net/http/internal/http2+ internal/syscall/execenv from os+ LD internal/syscall/unix from crypto/internal/sysrand+ W internal/syscall/windows from crypto/internal/fips140deps/time+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 5eccd04cc..8407e07bb 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -475,6 +475,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/util/cloudenv from tailscale.com/net/dns/resolver+ tailscale.com/util/cloudinfo from tailscale.com/wgengine/magicsock+ tailscale.com/util/cmpver from tailscale.com/net/dns+ + W tailscale.com/util/coalescedop from tailscale.com/net/dns tailscale.com/util/ctxkey from tailscale.com/ipn/ipnlocal+ 💣 tailscale.com/util/deephash from tailscale.com/util/syspolicy/setting tailscale.com/util/def from tailscale.com/feature/debugportmapper+ @@ -757,7 +758,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de internal/strconv from internal/poll+ internal/stringslite from embed+ internal/sync from sync+ - internal/synctest from sync+ + internal/synctest from net/http/internal/http2+ internal/syscall/execenv from os+ LD internal/syscall/unix from crypto/internal/sysrand+ W internal/syscall/windows from crypto/internal/fips140deps/time+ diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index 95a643559..cc95577cc 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -410,12 +410,7 @@ func handleSessionChange(chgRequest svc.ChangeRequest) { } if flushDNSOnSessionUnlock, _ := policyclient.Get().GetBoolean(pkey.FlushDNSOnSessionUnlock, false); flushDNSOnSessionUnlock { log.Printf("Received WTS_SESSION_UNLOCK event, initiating DNS flush.") - go func() { - err := dns.Flush() - if err != nil { - log.Printf("Error flushing DNS on session unlock: %v", err) - } - }() + dns.Flush() // non-blocking; coalesced with other flush calls } } diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index da2a4b867..d256b57f6 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -275,6 +275,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/util/cloudenv from tailscale.com/hostinfo+ tailscale.com/util/cloudinfo from tailscale.com/wgengine/magicsock LW tailscale.com/util/cmpver from tailscale.com/net/dns+ + W tailscale.com/util/coalescedop from tailscale.com/net/dns tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/util/syspolicy/setting tailscale.com/util/def from tailscale.com/ipn/localapi diff --git a/net/dns/flush_windows.go b/net/dns/flush_windows.go index a3b1f6b31..5f27a0a33 100644 --- a/net/dns/flush_windows.go +++ b/net/dns/flush_windows.go @@ -4,31 +4,77 @@ package dns import ( - "fmt" + "log" "os/exec" "syscall" + "time" "golang.org/x/sys/windows" + "tailscale.com/util/coalescedop" ) -func flushCaches() error { +var ( + flushDNSOp = coalescedop.New(doFlushDNS) + registerDNSOp = coalescedop.New(doRegisterDNS) +) + +func doFlushDNS() { + t0 := time.Now() cmd := exec.Command("ipconfig", "/flushdns") cmd.SysProcAttr = &syscall.SysProcAttr{ CreationFlags: windows.DETACHED_PROCESS, } - out, err := cmd.CombinedOutput() + err := cmd.Run() + d := time.Since(t0).Round(time.Millisecond) if err != nil { - return fmt.Errorf("%v (output: %s)", err, out) + log.Printf("error running ipconfig /flushdns after %v: %v", d, err) + } else { + log.Printf("ran ipconfig /flushdns in %v", d) } +} + +func doRegisterDNS() { + t0 := time.Now() + cmd := exec.Command("ipconfig", "/registerdns") + cmd.SysProcAttr = &syscall.SysProcAttr{ + CreationFlags: windows.DETACHED_PROCESS, + } + err := cmd.Run() + d := time.Since(t0).Round(time.Millisecond) + if err != nil { + log.Printf("error running ipconfig /registerdns after %v: %v", d, err) + } else { + log.Printf("ran ipconfig /registerdns in %v", d) + } +} + +func flushCaches() error { + flushDNSOp.Do() return nil } // Flush clears the local resolver cache. // -// Only Windows has a public dns.Flush, needed in router_windows.go. Other -// platforms like Linux need a different flush implementation depending on -// the DNS manager. There is a FlushCaches method on the manager which -// can be used on all platforms. -func Flush() error { - return flushCaches() +// Only Windows has a public dns.Flush, needed in tailscaled_windows.go. +// Other platforms like Linux need a different flush implementation +// depending on the DNS manager. There is a FlushCaches method on the manager +// which can be used on all platforms. +// +// Flush is non-blocking; it coalesces concurrent and rapid successive +// calls so that at most one ipconfig /flushdns is in flight at a time, +// with at most one additional execution queued. +func Flush() { + flushDNSOp.Do() +} + +// registerDNS forces DNS re-registration in Active Directory. This +// invokes an undocumented hidden function that forces Windows to +// notice that adapter settings have changed, which makes the DNS +// settings actually take effect. +// +// registerDNS is non-blocking; it coalesces concurrent and rapid +// successive calls so that at most one ipconfig /registerdns is in +// flight at a time, with at most one additional execution queued. +func registerDNS() { + registerDNSOp.Do() } diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index dddb2ada5..f379739b4 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -12,12 +12,10 @@ import ( "maps" "net/netip" "os" - "os/exec" "path/filepath" "slices" "sort" "strings" - "syscall" "time" "golang.org/x/sys/windows" @@ -493,40 +491,15 @@ func (m *windowsManager) SetDNS(cfg OSConfig) error { // have changed, which makes the DNS settings actually take // effect. // - // This command can take a few seconds to run, so run it async, best effort. - // // After re-registering DNS, also flush the DNS cache to clear out // any cached split-horizon queries that are no longer the correct // answer. - go func() { - t0 := time.Now() - m.logf("running ipconfig /registerdns ...") - cmd := exec.Command("ipconfig", "/registerdns") - cmd.SysProcAttr = &syscall.SysProcAttr{ - CreationFlags: windows.DETACHED_PROCESS, - } - err := cmd.Run() - d := time.Since(t0).Round(time.Millisecond) - if err != nil { - m.logf("error running ipconfig /registerdns after %v: %v", d, err) - } else { - m.logf("ran ipconfig /registerdns in %v", d) - } - - t0 = time.Now() - m.logf("running ipconfig /flushdns ...") - cmd = exec.Command("ipconfig", "/flushdns") - cmd.SysProcAttr = &syscall.SysProcAttr{ - CreationFlags: windows.DETACHED_PROCESS, - } - err = cmd.Run() - d = time.Since(t0).Round(time.Millisecond) - if err != nil { - m.logf("error running ipconfig /flushdns after %v: %v", d, err) - } else { - m.logf("ran ipconfig /flushdns in %v", d) - } - }() + // + // Both operations are non-blocking and coalesced: at most one of + // each is in flight at a time, with at most one more queued. + m.logf("Re-registering DNS and flushing cache") + registerDNS() + Flush() // On initial setup of WSL, the restart caused by --shutdown is slow, // so we do it out-of-line. diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index ec962ab08..8879ff77d 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -270,6 +270,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/util/cloudenv from tailscale.com/hostinfo+ tailscale.com/util/cloudinfo from tailscale.com/wgengine/magicsock LW tailscale.com/util/cmpver from tailscale.com/net/dns+ + W tailscale.com/util/coalescedop from tailscale.com/net/dns tailscale.com/util/ctxkey from tailscale.com/client/tailscale/apitype+ 💣 tailscale.com/util/deephash from tailscale.com/util/syspolicy/setting tailscale.com/util/def from tailscale.com/ipn/localapi diff --git a/util/coalescedop/coalescedop.go b/util/coalescedop/coalescedop.go new file mode 100644 index 000000000..3454fb88c --- /dev/null +++ b/util/coalescedop/coalescedop.go @@ -0,0 +1,66 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package coalescedop provides a mechanism to serialize and coalesce +// calls to a function. At most one call is in flight at a time, and +// if additional calls are requested while one is running, exactly one +// more execution is scheduled after the current one completes. +// +// Unlike [singleflight.Group], which deduplicates concurrent callers +// onto a single in-flight call, [CoalescedOp] ensures that a new +// execution occurs after the current one finishes, so changes that +// happened during the in-flight call are picked up by the next run. +package coalescedop + +import "sync" + +// CoalescedOp serializes and coalesces calls to a function. +// At most one call is in flight at a time. If additional calls +// are requested while one is running, exactly one more execution +// is scheduled (not one per request). +// +// The zero value is not valid; use [New]. +type CoalescedOp struct { + mu sync.Mutex + fn func() + running bool + pending bool +} + +// New returns a CoalescedOp that calls fn. +func New(fn func()) *CoalescedOp { + return &CoalescedOp{fn: fn} +} + +// Do requests that the operation be run. If no execution is in +// progress, a new goroutine is started to run the function. If an +// execution is already in progress, the function will be run exactly +// once more after the current execution completes. +// +// Do never blocks. +func (c *CoalescedOp) Do() { + c.mu.Lock() + if c.running { + c.pending = true + c.mu.Unlock() + return + } + c.running = true + c.mu.Unlock() + go c.run() +} + +func (c *CoalescedOp) run() { + for { + c.fn() + + c.mu.Lock() + if !c.pending { + c.running = false + c.mu.Unlock() + return + } + c.pending = false + c.mu.Unlock() + } +} diff --git a/util/coalescedop/coalescedop_test.go b/util/coalescedop/coalescedop_test.go new file mode 100644 index 000000000..8eee8226c --- /dev/null +++ b/util/coalescedop/coalescedop_test.go @@ -0,0 +1,91 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package coalescedop_test + +import ( + "sync/atomic" + "testing" + "testing/synctest" + "time" + + "tailscale.com/util/coalescedop" +) + +func TestSingleCall(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var n atomic.Int32 + done := make(chan struct{}) + op := coalescedop.New(func() { + n.Add(1) + close(done) + }) + op.Do() + <-done + if got := n.Load(); got != 1 { + t.Errorf("n=%d; want 1", got) + } + }) +} + +func TestCoalescesWhileRunning(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var n atomic.Int32 + release := make(chan struct{}) + op := coalescedop.New(func() { + n.Add(1) + <-release + }) + + // Start the first execution; it blocks on release. + op.Do() + // synctest.Test advances the fake clock only once all + // goroutines in the bubble are blocked, so this Sleep + // reliably waits for the goroutine to be parked on <-release. + time.Sleep(time.Millisecond) + + // Request several more while the first is running. + op.Do() + op.Do() + op.Do() + + // Release the first execution. + release <- struct{}{} + time.Sleep(time.Millisecond) // wait for coalesced run to block on <-release + + // Release the second (coalesced) execution. + release <- struct{}{} + time.Sleep(time.Millisecond) // wait for run() to finish + + if got := n.Load(); got != 2 { + t.Errorf("n=%d; want 2 (one original + one coalesced)", got) + } + }) +} + +func TestNoPendingIfNotRequested(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + var n atomic.Int32 + done := make(chan struct{}, 1) + op := coalescedop.New(func() { + n.Add(1) + done <- struct{}{} + }) + + op.Do() + <-done + time.Sleep(time.Millisecond) // wait for run() to finish + + // No more executions should happen. + if got := n.Load(); got != 1 { + t.Errorf("n=%d; want 1", got) + } + + // A new Do after completion should run again. + op.Do() + <-done + if got := n.Load(); got != 2 { + t.Errorf("n=%d; want 2", got) + } + }) +} diff --git a/wgengine/router/osrouter/router_windows.go b/wgengine/router/osrouter/router_windows.go index ef9eb04a1..3b32d5e01 100644 --- a/wgengine/router/osrouter/router_windows.go +++ b/wgengine/router/osrouter/router_windows.go @@ -23,7 +23,6 @@ import ( "golang.org/x/sys/windows" "golang.zx2c4.com/wireguard/windows/tunnel/winipcfg" "tailscale.com/health" - "tailscale.com/net/dns" "tailscale.com/net/netmon" "tailscale.com/types/logger" "tailscale.com/util/backoff" @@ -97,11 +96,6 @@ func (r *winRouter) Set(cfg *router.Config) error { return err } - // Flush DNS on router config change to clear cached DNS entries (solves #1430) - if err := dns.Flush(); err != nil { - r.logf("flushdns error: %v", err) - } - return nil }