From 4660a961ebd64c4fdafe93d8cef834d973b36387 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 15 Jul 2026 02:19:14 +0000 Subject: [PATCH] ipn/ipnlocal, wgengine/wgcfg/nmcfg: stop building peer lists on delta path Processing a peer add/remove delta still materialized the full netmap (an O(n) slicesx.MapValues plus sort over all peers, at 10k+ peers in a large tailnet) twice per delta: once in UpdateNetmapDelta purely to hand the self node to Engine.SetSelfNode, and once in authReconfigLocked. Neither needs peers anymore. SetSelfNode gets the self node from the existing nodeBackend.Self accessor. authReconfigLocked only reads self-node fields (SelfNode, NodeKey, GetAddresses, HasCap) now that WireGuard peers ride the incremental route manager and per-peer config source, so it can use the peers-free NetMap accessor. That also makes nmcfg.WGCfg vestigial: since wgcfg.Config lost its Peers field, its peer walk existed only to emit the [v1] skip logs (expired peers, unselected exit nodes, unaccepted subnet routes), duplicating filtering the route manager already does. Delete the package and construct the two-field wgcfg.Config inline. The skip logs go away; if they're missed, the route manager can log them incrementally at upsert time instead of rescanning every peer on every reconfig. With this, the runtime.DidRange analysis (see the ts_rangehook test) shows a delta netmap update performing no O(n) range loops except updateRouteManagerExtras, and the delta phase of that test drops from 1.09s to 0.14s for 400 deltas at n=10000 (from 4.79s at the start of this effort, before the incremental route manager work). Updates #12542 Signed-off-by: Brad Fitzpatrick Change-Id: Ia0e03ef9db0c988790b2c29de1f0505305e93f58 --- cmd/k8s-operator/depaware.txt | 1 - cmd/tailscaled/depaware-min.txt | 1 - cmd/tailscaled/depaware-minbox.txt | 1 - cmd/tailscaled/depaware.txt | 1 - cmd/tsidp/depaware.txt | 1 - ipn/ipnlocal/local.go | 22 ++--- net/routemanager/routemanager.go | 14 ++-- tsnet/depaware.txt | 1 - wgengine/magicsock/magicsock_test.go | 38 ++++----- wgengine/wgcfg/nmcfg/nmcfg.go | 117 --------------------------- 10 files changed, 36 insertions(+), 161 deletions(-) delete mode 100644 wgengine/wgcfg/nmcfg/nmcfg.go diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 603af4fb2..ce9833d6a 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -928,7 +928,6 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine golang.org/x/crypto/argon2 from tailscale.com/tka diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index 56eade3f0..5f599604d 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -200,7 +200,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine golang.org/x/crypto/blake2b from golang.org/x/crypto/nacl/box diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index 3a8756781..7cb7fc727 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/cmd/tailscaled/depaware-minbox.txt @@ -221,7 +221,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine golang.org/x/crypto/blake2b from golang.org/x/crypto/nacl/box diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 0ecc5bd7d..164f9f6dd 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -521,7 +521,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/router/osrouter from tailscale.com/feature/condregister tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine W 💣 tailscale.com/wgengine/winnet from tailscale.com/wgengine/router/osrouter diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index 6c1a9e2c2..e252f82fa 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -326,7 +326,6 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine golang.org/x/crypto/argon2 from tailscale.com/tka diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e95f3409a..d754f35b4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -107,7 +107,6 @@ import ( "tailscale.com/wgengine/magicsock" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" - "tailscale.com/wgengine/wgcfg/nmcfg" ) var controlDebugFlags = getControlDebugFlags() @@ -2502,8 +2501,8 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo needsAuthReconfig = needsAuthReconfig || peersUpsertedOrRemoved if needsAuthReconfig { if peersUpsertedOrRemoved { - if nm := cn.netMapWithPeers(); nm != nil { - b.e.SetSelfNode(nm.SelfNode) + if self := cn.Self(); self.Valid() { + b.e.SetSelfNode(self) } } b.authReconfigLocked() @@ -6054,7 +6053,10 @@ func (b *LocalBackend) authReconfigLocked() { cn := b.currentNode() - nm := cn.netMapWithPeers() + // Note this netmap does not have its Peers populated. Nothing + // below needs them; per-peer work rides the incremental route + // manager and engine paths instead. + nm := cn.NetMap() if nm == nil { b.logf("[v1] authReconfig: netmap not yet valid. Skipping.") return @@ -6100,10 +6102,12 @@ func (b *LocalBackend) authReconfigLocked() { priv = key.NodePrivate{} } - cfg, err := nmcfg.WGCfg(priv, nm, b.logf, flags, prefs.ExitNodeID()) - if err != nil { - b.logf("wgcfg: %v", err) - return + // The config carries no peers; wireguard-go gets those from the + // live per-peer config source installed via + // [wgengine.Engine.SetPeerConfigFunc], fed by the route manager. + cfg := &wgcfg.Config{ + PrivateKey: priv, + Addresses: nm.GetAddresses().AsSlice(), } // Note: b.goos (set only by tests) speaks runtime.GOOS while @@ -6146,7 +6150,7 @@ func (b *LocalBackend) authReconfigLocked() { // the new config never see a stale peer table. b.setDataPlanePeerRoutes() - err = b.e.Reconfig(cfg, rcfg, dcfg) + err := b.e.Reconfig(cfg, rcfg, dcfg) if err == wgengine.ErrNoChanges { return } diff --git a/net/routemanager/routemanager.go b/net/routemanager/routemanager.go index a6055c91f..b7afe741b 100644 --- a/net/routemanager/routemanager.go +++ b/net/routemanager/routemanager.go @@ -397,12 +397,11 @@ func (m *Mutation) UpsertPeer(n tailcfg.NodeView) { // peerViewOf reduces a tailcfg.NodeView to the routing-relevant // peerView. // -// It mirrors the peer and prefix filtering in nmcfg.WGCfg: peers we -// cannot communicate with (expired, or predating both DERP and disco) -// contribute no prefixes. They remain tracked by ID and key so that a -// later update can make them routable again. AllowedIPs is the sole -// source of prefixes; an address absent from AllowedIPs is not -// routable. The self-vs-route split mirrors nmcfg's cidrIsSubnet: +// Peers we cannot communicate with (expired, or predating both DERP +// and disco) contribute no prefixes. They remain tracked by ID and key +// so that a later update can make them routable again. AllowedIPs is +// the sole source of prefixes; an address absent from AllowedIPs is +// not routable. For the self-vs-route split, // single Tailscale IPs are never subnets, so a VIP service address // hosted by the peer lands in SelfAddrs and stays routable without // Prefs.RouteAll. @@ -662,8 +661,7 @@ func normalizePrefix(p netip.Prefix) netip.Prefix { // non-address bits set, mirroring the defensive check in // ipnlocal.peerRoutes. It includes the peer's extra allowed IPs, // except for peers that contribute no addresses or routes of their -// own (expired or otherwise non-communicable peers, which mirrors -// nmcfg.WGCfg dropping such peers entirely). +// own (expired or otherwise non-communicable peers). func (rm *RouteManager) contribs(p peerView) map[netip.Prefix]contribKind { c := make(map[netip.Prefix]contribKind, len(p.SelfAddrs)+len(p.Routes)) add := func(pfx netip.Prefix, kind contribKind) { diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 19bb12ba8..954557c3f 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -321,7 +321,6 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal 💣 tailscale.com/wgengine/wgint from tailscale.com/wgengine+ tailscale.com/wgengine/wglog from tailscale.com/wgengine golang.org/x/crypto/argon2 from tailscale.com/tka diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 63876bc51..64d69bcdc 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -81,7 +81,6 @@ import ( "tailscale.com/wgengine/filter" "tailscale.com/wgengine/filter/filtertype" "tailscale.com/wgengine/wgcfg" - "tailscale.com/wgengine/wgcfg/nmcfg" "tailscale.com/wgengine/wglog" ) @@ -246,6 +245,17 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, ln nettype.PacketListe } } +// wgCfgOf builds a test node's wgcfg.Config from its netmap the same +// way ipnlocal.authReconfigLocked does in production: just the private +// key and self addresses. Peers ride the per-peer config source +// instead; see [magicStack.Reconfig]. +func wgCfgOf(pk key.NodePrivate, nm *netmap.NetworkMap) *wgcfg.Config { + return &wgcfg.Config{ + PrivateKey: pk, + Addresses: nm.GetAddresses().AsSlice(), + } +} + // Reconfig applies cfg and peers to the stack's WireGuard device and // tun-layer data plane. In production these flow from LocalBackend // (via [tailscale.com/wgengine.Engine.Reconfig] and the live per-peer @@ -259,8 +269,8 @@ func (s *magicStack) Reconfig(cfg *wgcfg.Config, peers []tailcfg.NodeView) error rm := routemanager.New(nil) mut := rm.Begin() - // Mirror the netmap.AllowSubnetRoutes flag the tests pass to - // nmcfg.WGCfg. + // Tests want subnet routes accepted, matching what production + // gets from Prefs.RouteAll. mut.SetPrefs(routemanager.Prefs{RouteAll: true}) // idByKey stands in for nodeBackend's public-key-to-node-ID // index, which LocalBackend uses to serve the engine's per-peer @@ -412,12 +422,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM for i, m := range ms { nm := buildNetmapLocked(i) m.conn.SetNetworkMap(nm.SelfNode, nm.Peers) - wg, err := nmcfg.WGCfg(ms[i].privateKey, nm, logf, 0, "") - if err != nil { - // We're too far from the *testing.T to be graceful, - // blow up. Shouldn't happen anyway. - panic(fmt.Sprintf("failed to construct wgcfg from netmap: %v", err)) - } + wg := wgCfgOf(ms[i].privateKey, nm) if err := m.Reconfig(wg, nm.Peers); err != nil { if ctx.Err() != nil || errors.Is(err, errConnClosed) { // shutdown race, don't care. @@ -2568,10 +2573,7 @@ func TestIsWireGuardOnlyPeer(t *testing.T) { } m.conn.SetNetworkMap(nm.SelfNode, nm.Peers) - cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") - if err != nil { - t.Fatal(err) - } + cfg := wgCfgOf(m.privateKey, nm) m.Reconfig(cfg, nm.Peers) pbuf := tuntest.Ping(wgaip.Addr(), tsaip.Addr()) @@ -2629,10 +2631,7 @@ func TestIsWireGuardOnlyPeerWithMasquerade(t *testing.T) { } m.conn.SetNetworkMap(nm.SelfNode, nm.Peers) - cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") - if err != nil { - t.Fatal(err) - } + cfg := wgCfgOf(m.privateKey, nm) m.Reconfig(cfg, nm.Peers) pbuf := tuntest.Ping(wgaip.Addr(), tsaip.Addr()) @@ -2669,10 +2668,7 @@ func applyNetworkMap(t *testing.T, m *magicStack, nm *netmap.NetworkMap) { m.conn.noV6.Store(true) // Turn the network map into a wireguard config (for the tailscale internal wireguard device). - cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") - if err != nil { - t.Fatal(err) - } + cfg := wgCfgOf(m.privateKey, nm) // Apply the wireguard config to the tailscale internal wireguard device. if err := m.Reconfig(cfg, nm.Peers); err != nil { t.Fatal(err) diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go deleted file mode 100644 index da4d038db..000000000 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -// Package nmcfg converts a controlclient.NetMap into a wgcfg config. -package nmcfg - -import ( - "bufio" - "cmp" - "fmt" - "net/netip" - "strings" - - "tailscale.com/net/tsaddr" - "tailscale.com/tailcfg" - "tailscale.com/types/key" - "tailscale.com/types/logger" - "tailscale.com/types/netmap" - "tailscale.com/wgengine/wgcfg" -) - -func nodeDebugName(n tailcfg.NodeView) string { - name, _, _ := strings.Cut(cmp.Or(n.Name(), n.Hostinfo().Hostname()), ".") - return name -} - -// cidrIsSubnet reports whether cidr is a non-default-route subnet -// exported by node that is not one of its own self addresses. -func cidrIsSubnet(node tailcfg.NodeView, cidr netip.Prefix) bool { - if cidr.Bits() == 0 { - return false - } - if !cidr.IsSingleIP() { - return true - } - if tsaddr.IsTailscaleIP(cidr.Addr()) { - return false - } - for _, selfCIDR := range node.Addresses().All() { - if cidr == selfCIDR { - return false - } - } - return true -} - -// WGCfg returns the NetworkMaps's WireGuard configuration. -// -// The config does not include peers; wireguard-go gets those from the -// live per-peer config source installed via -// [tailscale.com/wgengine.Engine.SetPeerConfigFunc], fed by the route -// manager. WGCfg still walks the peers to log which ones are not -// routable and why, mirroring the route manager's filtering. -func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, exitNode tailcfg.StableNodeID) (*wgcfg.Config, error) { - cfg := &wgcfg.Config{ - PrivateKey: pk, - Addresses: nm.GetAddresses().AsSlice(), - } - - var skippedExitNode, skippedSubnetRouter, skippedExpired []tailcfg.NodeView - - for _, peer := range nm.Peers { - if peer.DiscoKey().IsZero() && peer.HomeDERP() == 0 && !peer.IsWireGuardOnly() { - // Peer predates both DERP and active discovery, we cannot - // communicate with it. - logf("[v1] wgcfg: skipped peer %s, doesn't offer DERP or disco", peer.Key().ShortString()) - continue - } - // Skip expired peers; we'll end up failing to connect to them - // anyway, since control intentionally breaks node keys for - // expired peers so that we can't discover endpoints via DERP. - if peer.Expired() { - skippedExpired = append(skippedExpired, peer) - continue - } - - didExitNodeLog := false - for _, allowedIP := range peer.AllowedIPs().All() { - if allowedIP.Bits() == 0 && peer.StableID() != exitNode { - if didExitNodeLog { - // Don't log about both the IPv4 /0 and IPv6 /0. - continue - } - didExitNodeLog = true - skippedExitNode = append(skippedExitNode, peer) - } else if cidrIsSubnet(peer, allowedIP) { - if (flags & netmap.AllowSubnetRoutes) == 0 { - skippedSubnetRouter = append(skippedSubnetRouter, peer) - } - } - } - } - - logList := func(title string, nodes []tailcfg.NodeView) { - if len(nodes) == 0 { - return - } - logf("[v1] wgcfg: %s from %d nodes: %s", title, len(nodes), logger.ArgWriter(func(bw *bufio.Writer) { - const max = 5 - for i, n := range nodes { - if i == max { - fmt.Fprintf(bw, "... +%d", len(nodes)-max) - return - } - if i > 0 { - bw.WriteString(", ") - } - fmt.Fprintf(bw, "%s (%s)", nodeDebugName(n), n.StableID()) - } - })) - } - logList("skipped unselected exit nodes", skippedExitNode) - logList("did not accept subnet routes", skippedSubnetRouter) - logList("skipped expired peers", skippedExpired) - - return cfg, nil -}