mirror of
https://github.com/tailscale/tailscale.git
synced 2026-07-15 01:53:08 -04:00
wgengine,ipn/ipnlocal,tsnet,cmd/tailscaled: remove PeerForIP from the Engine interface
Engine.PeerForIP was pure delegation to the callback that LocalBackend installs via SetPeerForIPFunc, so external callers going through the engine were taking a pointless round trip: LocalBackend called b.e.PeerForIP, which called right back into LocalBackend, and the netstack UseNetstackForIP hooks in tsnet and tailscaled did the same dance one layer removed. Export LocalBackend.PeerForIP and make those callers use it directly. The engine-internal cold paths (Ping, TSMP disco advertisements, pendopen diagnostics) still need the lookup and have no netmap of their own, so SetPeerForIPFunc stays on the interface, but the lookup method itself is now unexported and gone from the Engine interface. In tailscaled the UseNetstackForIP hook moves from netstack setup to just after the LocalBackend is created, since the backend doesn't exist yet when netstack is wired up. Updates #12542 Change-Id: Ib1e1a4fa5c84ee0dcb9ce5d1910047f2bab9453c Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
committed by
Brad Fitzpatrick
parent
c436dec43c
commit
9cb1147805
@@ -44,11 +44,6 @@ func newNetstack(logf logger.Logf, sys *tsd.System, onlyNetstack bool) (tsd.Nets
|
||||
dialer := sys.Dialer.Get() // must be set by caller already
|
||||
|
||||
if onlyNetstack {
|
||||
e := sys.Engine.Get()
|
||||
dialer.UseNetstackForIP = func(ip netip.Addr) bool {
|
||||
_, ok := e.PeerForIP(ip)
|
||||
return ok
|
||||
}
|
||||
dialer.NetstackDialTCP = func(ctx context.Context, dst netip.AddrPort) (net.Conn, error) {
|
||||
// Note: don't just return ns.DialContextTCP or we'll return
|
||||
// *gonet.TCPConn(nil) instead of a nil interface which trips up
|
||||
|
||||
@@ -18,6 +18,7 @@
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"os"
|
||||
"os/signal"
|
||||
"path/filepath"
|
||||
@@ -694,6 +695,12 @@ func getLocalBackend(ctx context.Context, logf logger.Logf, logID logid.PublicID
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("ipnlocal.NewLocalBackend: %w", err)
|
||||
}
|
||||
if onlyNetstack {
|
||||
dialer.UseNetstackForIP = func(ip netip.Addr) bool {
|
||||
_, ok := lb.PeerForIP(ip)
|
||||
return ok
|
||||
}
|
||||
}
|
||||
lb.SetVarRoot(opts.VarRoot)
|
||||
if logPol != nil {
|
||||
lb.SetLogFlusher(logPol.Logtail.StartFlush)
|
||||
|
||||
@@ -634,7 +634,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo
|
||||
|
||||
e.SetPeerByIPPacketFunc(b.lookupPeerByIP)
|
||||
e.SetPeerConfigFunc(b.peerAllowedIPs)
|
||||
e.SetPeerForIPFunc(b.peerForIP)
|
||||
e.SetPeerForIPFunc(b.PeerForIP)
|
||||
e.SetPeerSessionStateFunc(b.onPeerWireGuardState)
|
||||
e.SetNetLogSource(netLogNodeSource{b})
|
||||
e.SetWGPeerLookup(b.lookupPeerWireGuardString)
|
||||
@@ -8492,7 +8492,7 @@ func (b *LocalBackend) ResetAuth() error {
|
||||
}
|
||||
|
||||
func (b *LocalBackend) GetPeerEndpointChanges(ctx context.Context, ip netip.Addr) ([]magicsock.EndpointChange, error) {
|
||||
pip, ok := b.e.PeerForIP(ip)
|
||||
pip, ok := b.PeerForIP(ip)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("no matching peer")
|
||||
}
|
||||
|
||||
@@ -9488,14 +9488,13 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) {
|
||||
|
||||
nm := buildNetmapWithPeers(selfNode, exitA, exitB, subnetBig, subnetSmall)
|
||||
|
||||
var eng wgengine.Engine
|
||||
var curLB *LocalBackend
|
||||
var curT *testing.T // active subtest, for test helpers
|
||||
|
||||
wantPeer := func(ip string, n tailcfg.NodeView) {
|
||||
t := curT
|
||||
t.Helper()
|
||||
pip, ok := eng.PeerForIP(netip.MustParseAddr(ip))
|
||||
pip, ok := curLB.PeerForIP(netip.MustParseAddr(ip))
|
||||
if !ok {
|
||||
t.Fatalf("PeerForIP(%s): ok=false, want true", ip)
|
||||
}
|
||||
@@ -9509,7 +9508,7 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) {
|
||||
wantNotPeer := func(ip string) {
|
||||
t := curT
|
||||
t.Helper()
|
||||
if _, ok := eng.PeerForIP(netip.MustParseAddr(ip)); ok {
|
||||
if _, ok := curLB.PeerForIP(netip.MustParseAddr(ip)); ok {
|
||||
t.Fatalf("PeerForIP(%s): ok=true, want false", ip)
|
||||
}
|
||||
}
|
||||
@@ -9534,7 +9533,7 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) {
|
||||
wantSelf := func(ip string) {
|
||||
t := curT
|
||||
t.Helper()
|
||||
pip, ok := eng.PeerForIP(netip.MustParseAddr(ip))
|
||||
pip, ok := curLB.PeerForIP(netip.MustParseAddr(ip))
|
||||
if !ok {
|
||||
t.Fatalf("PeerForIP(%s): ok=false, want true", ip)
|
||||
}
|
||||
@@ -9663,7 +9662,6 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) {
|
||||
LoggedIn: true,
|
||||
})
|
||||
|
||||
eng = lb.sys.Engine.Get()
|
||||
curLB = lb
|
||||
curT = t
|
||||
tt.check()
|
||||
|
||||
@@ -118,13 +118,15 @@ func addrFamilyMatch(ip netip.Addr, network string) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// peerForIP returns which peer is responsible for a given IP address.
|
||||
// PeerForIP returns which peer is responsible for a given IP address.
|
||||
// Despite the name, it can also return the self node (with IsSelf set).
|
||||
// It handles both Tailscale IPs (returning the owning peer or self) and
|
||||
// non-Tailscale addresses like subnet-routed IPs or exit-node global
|
||||
// internet IPs (returning whichever peer would route that traffic).
|
||||
// It is installed as the [wgengine.Engine.SetPeerForIPFunc] callback.
|
||||
func (b *LocalBackend) peerForIP(ip netip.Addr) (_ wgengine.PeerForIP, ok bool) {
|
||||
// It is installed as the [wgengine.Engine.SetPeerForIPFunc] callback,
|
||||
// serving the engine's internal cold-path lookups (Ping, TSMP, pendopen
|
||||
// diagnostics).
|
||||
func (b *LocalBackend) PeerForIP(ip netip.Addr) (_ wgengine.PeerForIP, ok bool) {
|
||||
nb := b.currentNode()
|
||||
|
||||
if tsaddr.IsTailscaleIP(ip) {
|
||||
|
||||
@@ -1949,10 +1949,6 @@ func (e *mockEngine) DNSConfig() *dns.Config {
|
||||
return e.dnsCfg
|
||||
}
|
||||
|
||||
func (e *mockEngine) PeerForIP(netip.Addr) (_ wgengine.PeerForIP, ok bool) {
|
||||
return wgengine.PeerForIP{}, false
|
||||
}
|
||||
|
||||
func (e *mockEngine) GetFilter() *filter.Filter {
|
||||
e.mu.Lock()
|
||||
defer e.mu.Unlock()
|
||||
|
||||
@@ -886,7 +886,8 @@ func (s *Server) start() (reterr error) {
|
||||
ns.GetUDPHandlerForFlow = s.getUDPHandlerForFlow
|
||||
s.netstack = ns
|
||||
s.dialer.UseNetstackForIP = func(ip netip.Addr) bool {
|
||||
_, ok := eng.PeerForIP(ip)
|
||||
// s.lb is assigned below, before any dials can happen.
|
||||
_, ok := s.lb.PeerForIP(ip)
|
||||
return ok
|
||||
}
|
||||
s.dialer.NetstackDialTCP = func(ctx context.Context, dst netip.AddrPort) (net.Conn, error) {
|
||||
|
||||
@@ -138,14 +138,14 @@ func (e *userspaceEngine) isOSNetworkProbe(dst netip.AddrPort) bool {
|
||||
// iOS had log spam like:
|
||||
// open-conn-track: timeout opening (100.115.73.60:52501 => 17.125.252.5:443); no associated peer node
|
||||
if runtime.GOOS == "ios" && dst.Port() == 443 && appleIPRange.Contains(dst.Addr()) {
|
||||
if _, ok := e.PeerForIP(dst.Addr()); !ok {
|
||||
if _, ok := e.peerForIP(dst.Addr()); !ok {
|
||||
return true
|
||||
}
|
||||
}
|
||||
// NetworkManager; https://github.com/tailscale/tailscale/issues/13687
|
||||
// open-conn-track: timeout opening (TCP 100.96.229.119:42798 => 185.125.190.49:80); no associated peer node
|
||||
if runtime.GOOS == "linux" && dst.Port() == 80 && canonicalIPs()(dst.Addr()) {
|
||||
if _, ok := e.PeerForIP(dst.Addr()); !ok {
|
||||
if _, ok := e.peerForIP(dst.Addr()); !ok {
|
||||
return true
|
||||
}
|
||||
}
|
||||
@@ -199,7 +199,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
|
||||
}
|
||||
|
||||
// Diagnose why it might've timed out.
|
||||
pip, ok := e.PeerForIP(flow.DstAddr())
|
||||
pip, ok := e.peerForIP(flow.DstAddr())
|
||||
if !ok {
|
||||
e.logf("open-conn-track: timeout opening %v; no associated peer node", flow)
|
||||
return
|
||||
|
||||
@@ -103,10 +103,10 @@ type userspaceEngine struct {
|
||||
|
||||
wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below
|
||||
|
||||
// peerForIP, if non-nil, is the callback installed via
|
||||
// [userspaceEngine.SetPeerForIPFunc]. PeerForIP delegates to it
|
||||
// peerForIPFn, if non-nil, is the callback installed via
|
||||
// [userspaceEngine.SetPeerForIPFunc]. peerForIP delegates to it
|
||||
// for the cold-path control lookups (Ping, TSMP, pendopen, etc).
|
||||
peerForIP atomic.Pointer[func(netip.Addr) (_ PeerForIP, ok bool)]
|
||||
peerForIPFn atomic.Pointer[func(netip.Addr) (_ PeerForIP, ok bool)]
|
||||
|
||||
// peerConfigFn, if non-nil, is the live per-peer allowed-IPs
|
||||
// source installed via [userspaceEngine.SetPeerConfigFunc]. When
|
||||
@@ -605,7 +605,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
|
||||
pkt := packet.TSMPDiscoKeyAdvertisement{
|
||||
Key: update.Key,
|
||||
}
|
||||
peer, ok := e.PeerForIP(update.Src)
|
||||
peer, ok := e.peerForIP(update.Src)
|
||||
if !ok {
|
||||
e.logf("wgengine: no peer found for %v", update.Src)
|
||||
return
|
||||
@@ -1388,7 +1388,7 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) {
|
||||
|
||||
func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) {
|
||||
res := &ipnstate.PingResult{IP: ip.String()}
|
||||
pip, ok := e.PeerForIP(ip)
|
||||
pip, ok := e.peerForIP(ip)
|
||||
if !ok {
|
||||
e.logf("ping(%v): no matching peer", ip)
|
||||
res.Err = "no matching peer"
|
||||
@@ -1585,24 +1585,24 @@ func (e *userspaceEngine) ProbeLocks() {
|
||||
e.wgLock.Unlock()
|
||||
}
|
||||
|
||||
// SetPeerForIPFunc installs the callback used by [userspaceEngine.PeerForIP].
|
||||
// SetPeerForIPFunc installs the callback used by [userspaceEngine.peerForIP].
|
||||
// See [Engine.SetPeerForIPFunc].
|
||||
func (e *userspaceEngine) SetPeerForIPFunc(fn func(netip.Addr) (PeerForIP, bool)) {
|
||||
if fn == nil {
|
||||
e.peerForIP.Store(nil)
|
||||
e.peerForIPFn.Store(nil)
|
||||
return
|
||||
}
|
||||
e.peerForIP.Store(&fn)
|
||||
e.peerForIPFn.Store(&fn)
|
||||
}
|
||||
|
||||
// PeerForIP returns the node responsible for handling the given IP.
|
||||
// It delegates to the callback installed via [SetPeerForIPFunc]; engines
|
||||
// without an installed callback return (zero, false).
|
||||
func (e *userspaceEngine) PeerForIP(ip netip.Addr) (ret PeerForIP, ok bool) {
|
||||
// peerForIP returns the node responsible for handling the given IP.
|
||||
// It delegates to the callback installed via [Engine.SetPeerForIPFunc];
|
||||
// engines without an installed callback return (zero, false).
|
||||
func (e *userspaceEngine) peerForIP(ip netip.Addr) (ret PeerForIP, ok bool) {
|
||||
if !ip.IsValid() {
|
||||
return ret, false
|
||||
}
|
||||
if fn := e.peerForIP.Load(); fn != nil {
|
||||
if fn := e.peerForIPFn.Load(); fn != nil {
|
||||
return (*fn)(ip)
|
||||
}
|
||||
return ret, false
|
||||
|
||||
@@ -98,36 +98,18 @@ type Engine interface {
|
||||
// Unlike Reconfig, it does not return ErrNoChanges.
|
||||
ResetAndStop() (*Status, error)
|
||||
|
||||
// PeerForIP returns the node to which the provided IP routes,
|
||||
// if any. If none is found, (zero, false) is returned.
|
||||
// SetPeerForIPFunc installs the IP-to-node lookup used by the
|
||||
// engine's internal cold paths (Ping, TSMP, pendopen diagnostics).
|
||||
// It parallels [Engine.SetPeerByIPPacketFunc] but returns richer
|
||||
// data (a full NodeView, the matched route prefix, and the IsSelf
|
||||
// flag).
|
||||
//
|
||||
// Despite the name, it can return the self node (with
|
||||
// PeerForIP.IsSelf set). It handles Tailscale IPs, subnet-routed
|
||||
// IPs, and exit-node global internet IPs, returning whichever
|
||||
// node would handle that traffic.
|
||||
//
|
||||
// This is the cold path used by Ping, TSMP, pendopen diagnostics,
|
||||
// and debug endpoints. It uses the same underlying data structures
|
||||
// as the wireguard-go outbound packet path
|
||||
// ([Engine.SetPeerByIPPacketFunc]), but is slower because it
|
||||
// returns richer data (a full NodeView, the matched route prefix,
|
||||
// and the IsSelf flag) requiring extra lookups.
|
||||
//
|
||||
// In production, the lookup is implemented by LocalBackend and
|
||||
// plumbed in via [Engine.SetPeerForIPFunc]; the engine itself holds
|
||||
// no peer-lookup state on this path.
|
||||
PeerForIP(netip.Addr) (_ PeerForIP, ok bool)
|
||||
|
||||
// SetPeerForIPFunc installs a callback used by [Engine.PeerForIP].
|
||||
// It parallels [Engine.SetPeerByIPPacketFunc] but serves the
|
||||
// cold-path control lookups (Ping, TSMP, pendopen diagnostics,
|
||||
// [tsdial.Dialer.UseNetstackForIP], debug endpoints).
|
||||
//
|
||||
// If fn is nil, PeerForIP returns (zero, false) for every IP.
|
||||
// If fn is nil, those lookups fail for every IP.
|
||||
//
|
||||
// LocalBackend installs a func backed by the live nodeBackend for
|
||||
// exact-match and self addresses, with the RouteManager's outbound
|
||||
// table supplying the subnet-route / exit-node fallback.
|
||||
// table supplying the subnet-route / exit-node fallback; the engine
|
||||
// itself holds no peer-lookup state on this path.
|
||||
SetPeerForIPFunc(fn func(netip.Addr) (_ PeerForIP, ok bool))
|
||||
|
||||
// GetFilter returns the current packet filter, if any.
|
||||
|
||||
Reference in New Issue
Block a user