ipn/ipnext, ipn/ipnlocal, feature/conn25: pass peer seq to AllowedIPs hook
The ExtraWireGuardAllowedIPs hook was called once per peer on every authReconfig, so each netmap delta paid an O(n) scan over all peers even when conn25 (the only implementer) wasn't configured and every call returned nothing. Invert the API: the hook now receives an iter.Seq2 of the current peers and returns the extra prefixes keyed by node ID. An idle extension returns nil without iterating, so the unconfigured case does no per-peer work at all. With this, the runtime.DidRange analysis (see the ts_rangehook test) no longer reports the updateRouteManagerExtras peer scan on netmap deltas. Updates #12542 Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com> Change-Id: I9181e77416fa22f4c904620d42e9bcb934165216
This commit is contained in:
committed by
Brad Fitzpatrick
parent
f68e4d93fd
commit
3515b009c2
@@ -14,6 +14,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"iter"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"slices"
|
||||
@@ -267,11 +268,17 @@ func (e *extension) installHooks(dph *datapathHandler) error {
|
||||
})
|
||||
|
||||
// Tell WireGuard what Transit IPs belong to which connector peers.
|
||||
e.host.Hooks().ExtraWireGuardAllowedIPs.Set(func(k key.NodePublic) views.Slice[netip.Prefix] {
|
||||
e.host.Hooks().ExtraWireGuardAllowedIPs.Set(func(peers iter.Seq2[tailcfg.NodeID, key.NodePublic]) map[tailcfg.NodeID][]netip.Prefix {
|
||||
if !e.conn25.isConfigured() {
|
||||
return views.Slice[netip.Prefix]{}
|
||||
return nil
|
||||
}
|
||||
return e.extraWireGuardAllowedIPs(k)
|
||||
var extras map[tailcfg.NodeID][]netip.Prefix
|
||||
for id, k := range peers {
|
||||
if pfxs := e.extraWireGuardAllowedIPs(k); pfxs.Len() > 0 {
|
||||
mak.Set(&extras, id, pfxs.AsSlice())
|
||||
}
|
||||
}
|
||||
return extras
|
||||
})
|
||||
|
||||
return nil
|
||||
|
||||
+25
-13
@@ -430,29 +430,41 @@ type Hooks struct {
|
||||
// See [filter.Filter] for details on how these hooks are invoked.
|
||||
Filter FilterHooks
|
||||
|
||||
// ExtraWireGuardAllowedIPs is called with each peer's public key
|
||||
// from the initial [wgcfg.Config], and returns a view of prefixes to
|
||||
// append to each peer's AllowedIPs.
|
||||
// ExtraWireGuardAllowedIPs is called with a sequence of peers whose
|
||||
// extra AllowedIPs the caller wants (re)computed, and returns
|
||||
// prefixes to append to those peers' AllowedIPs, keyed by node ID.
|
||||
//
|
||||
// The extra AllowedIPs are added after the [router.Config] is generated, but
|
||||
// before the WireGuard config is sent to the engine, so the extra IPs are
|
||||
// given to WireGuard, but not the OS routing table.
|
||||
// The sequence is not necessarily all peers: callers may pass any
|
||||
// subset (such as only the peers changed by a netmap delta), and
|
||||
// the returned map's meaning is scoped to the peers presented. A
|
||||
// peer absent from the returned map has no extra AllowedIPs. As of
|
||||
// 2026-07-15 the only caller passes all current peers on each
|
||||
// reconfig, but extensions must not rely on that.
|
||||
//
|
||||
// The prefixes returned from the hook should not contain duplicates, either
|
||||
// internally, or with netmap peer prefixes. Returned prefixes should only
|
||||
// An extension with nothing to add should return nil without
|
||||
// iterating peers; that keeps steady-state netmap deltas free of
|
||||
// per-peer work when the extension is idle. The peers sequence is
|
||||
// only valid during the call.
|
||||
//
|
||||
// The extra AllowedIPs are given to WireGuard, but not the OS
|
||||
// routing table.
|
||||
//
|
||||
// The returned prefixes should not contain duplicates, either
|
||||
// internally, or with netmap peer prefixes. They should only
|
||||
// contain host routes, and not contain default or subnet routes.
|
||||
// Subsequent calls that return an unchanged set of prefixes for a given peer,
|
||||
// should return the prefixes in the same order for that peer,
|
||||
// to prevent configuration churn.
|
||||
// Subsequent calls that return an unchanged set of prefixes for a
|
||||
// given peer should return the prefixes in the same order for that
|
||||
// peer, to prevent configuration churn.
|
||||
//
|
||||
// The returned slice should not be mutated by the extension after it is returned.
|
||||
// The returned map and slices should not be mutated by the
|
||||
// extension after they are returned.
|
||||
//
|
||||
// The hook is called with LocalBackend's mutex locked.
|
||||
//
|
||||
// TODO(#17858): This hook may not be needed and can possibly be replaced by
|
||||
// new hooks that fit into the new architecture that make use of new
|
||||
// WireGuard APIs.
|
||||
ExtraWireGuardAllowedIPs feature.Hook[func(key.NodePublic) views.Slice[netip.Prefix]]
|
||||
ExtraWireGuardAllowedIPs feature.Hook[func(peers iter.Seq2[tailcfg.NodeID, key.NodePublic]) map[tailcfg.NodeID][]netip.Prefix]
|
||||
|
||||
// ExtraRouterConfigRoutes returns a view of prefixes to append to [router.Config.Routes].
|
||||
//
|
||||
|
||||
@@ -6,6 +6,7 @@ package ipnlocal
|
||||
import (
|
||||
"cmp"
|
||||
"context"
|
||||
"iter"
|
||||
"maps"
|
||||
"net/netip"
|
||||
"slices"
|
||||
@@ -943,22 +944,23 @@ func (nb *nodeBackend) updateRouteManagerPrefs(p routePrefs) routemanager.PeersW
|
||||
return res.AllowedIPs
|
||||
}
|
||||
|
||||
// updateRouteManagerExtras pushes each peer's extra WireGuard-only
|
||||
// allowed IPs into the route manager, obtained by calling fn (the
|
||||
// [ipnext.Hooks.ExtraWireGuardAllowedIPs] hook) with each peer's
|
||||
// public key.
|
||||
// updateRouteManagerExtras pushes extra WireGuard-only allowed IPs
|
||||
// into the route manager, obtained by calling fn (the
|
||||
// [ipnext.Hooks.ExtraWireGuardAllowedIPs] hook) with a sequence of
|
||||
// the current peers.
|
||||
//
|
||||
// It returns the peers whose allowed source prefixes changed as a
|
||||
// result.
|
||||
func (nb *nodeBackend) updateRouteManagerExtras(fn func(key.NodePublic) views.Slice[netip.Prefix]) routemanager.PeersWithRouteChanges {
|
||||
func (nb *nodeBackend) updateRouteManagerExtras(fn func(peers iter.Seq2[tailcfg.NodeID, key.NodePublic]) map[tailcfg.NodeID][]netip.Prefix) routemanager.PeersWithRouteChanges {
|
||||
nb.mu.Lock()
|
||||
defer nb.mu.Unlock()
|
||||
var extras map[tailcfg.NodeID][]netip.Prefix
|
||||
for id, p := range nb.peers {
|
||||
if pfxs := fn(p.Key()); pfxs.Len() > 0 {
|
||||
mak.Set(&extras, id, pfxs.AsSlice())
|
||||
extras := fn(func(yield func(tailcfg.NodeID, key.NodePublic) bool) {
|
||||
for id, p := range nb.peers {
|
||||
if !yield(id, p.Key()) {
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
rt := nb.routeMgr.Begin()
|
||||
rt.SetExtraAllowedIPs(extras)
|
||||
res := rt.Commit()
|
||||
|
||||
@@ -6,6 +6,7 @@ package ipnlocal
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"iter"
|
||||
"maps"
|
||||
"net/netip"
|
||||
"slices"
|
||||
@@ -17,7 +18,6 @@ import (
|
||||
"tailscale.com/tstest"
|
||||
"tailscale.com/types/key"
|
||||
"tailscale.com/types/netmap"
|
||||
"tailscale.com/types/views"
|
||||
"tailscale.com/util/dnsname"
|
||||
"tailscale.com/util/eventbus"
|
||||
"tailscale.com/util/mak"
|
||||
@@ -524,11 +524,14 @@ func TestNodeBackendRouteManagerExtras(t *testing.T) {
|
||||
nb.SetNetMap(&netmap.NetworkMap{Peers: []tailcfg.NodeView{p1}})
|
||||
|
||||
transit := netip.MustParsePrefix("fe80::1234/128")
|
||||
extrasFor := func(k key.NodePublic) views.Slice[netip.Prefix] {
|
||||
if k == p1.Key() {
|
||||
return views.SliceOf([]netip.Prefix{transit})
|
||||
extrasFor := func(peers iter.Seq2[tailcfg.NodeID, key.NodePublic]) map[tailcfg.NodeID][]netip.Prefix {
|
||||
var extras map[tailcfg.NodeID][]netip.Prefix
|
||||
for id, k := range peers {
|
||||
if k == p1.Key() {
|
||||
mak.Set(&extras, id, []netip.Prefix{transit})
|
||||
}
|
||||
}
|
||||
return views.Slice[netip.Prefix]{}
|
||||
return extras
|
||||
}
|
||||
|
||||
// Installing extras reports the peer's allowed prefixes as
|
||||
@@ -550,8 +553,8 @@ func TestNodeBackendRouteManagerExtras(t *testing.T) {
|
||||
}
|
||||
|
||||
// A hook that no longer returns extras removes them.
|
||||
changed = nb.updateRouteManagerExtras(func(key.NodePublic) views.Slice[netip.Prefix] {
|
||||
return views.Slice[netip.Prefix]{}
|
||||
changed = nb.updateRouteManagerExtras(func(iter.Seq2[tailcfg.NodeID, key.NodePublic]) map[tailcfg.NodeID][]netip.Prefix {
|
||||
return nil
|
||||
})
|
||||
if len(changed) != 1 {
|
||||
t.Errorf("clearing extras changed = %v; want just %v", changed, p1.Key())
|
||||
|
||||
Reference in New Issue
Block a user