[client] Address review comments on DNS lazy-connection warm-up

- Use netip.Addr instead of string IPs in the local resolver's
  PeerConnectivity and PeerActivator interfaces; extract record
  addresses as netip.Addr (v4-mapped forms unmapped) and convert to
  string only at the peer.Status boundary.
- Scope warm-up to match-only (non-authoritative) zones so the
  synthesized private-service records keep warming while plain
  peer-name lookups in the authoritative peer zone no longer wake
  every idle peer.
- Parse NB_DNS_LAZY_WARMUP_TIMEOUT once in the resolver constructor
  instead of on every request, and log invalid values.
- Stop reusing engine.syncMsgMux on the DNS path: ConnMgr.ActivatePeer
  is now safe for concurrent use (lazy manager pointer guarded by a
  dedicated RWMutex; the manager itself is internally synchronized),
  so network-map processing no longer blocks DNS resolution.
- Add SetPeerActivator to the dns.Server interface (no-op on the mock)
  and drop the *dns.DefaultServer type assertion in the engine.
- Drop WireGuard wording from dns/local comments; the layer is
  transport-agnostic.

Tests: authoritative zones never trigger warm-up; warm-up timeout env
parsing (valid/invalid/non-positive); extractRecordAddr unmaps
v4-mapped record data; dnsPeerActivator skips connected/unknown/
conn-less peers, returns on connect, and releases at the budget when
the peer stays idle; ConnMgr.ActivatePeer races the manager lifecycle
cleanly under -race.
This commit is contained in:
Claude
2026-07-22 03:22:22 +00:00
parent 7825036fae
commit a4d0f242b4
10 changed files with 419 additions and 87 deletions

View File

@@ -34,6 +34,8 @@ const (
// - Handling connection establishment based on peer signaling
//
// The implementation is not thread-safe; it is protected by engine.syncMsgMux.
// The only exception is ActivatePeer, which is safe for concurrent use so the
// DNS warm-up path can call it without contending on the engine mutex.
type ConnMgr struct {
peerStore *peerstore.Store
statusRecorder *peer.Status
@@ -42,6 +44,10 @@ type ConnMgr struct {
rosenpassEnabled bool
lazyConnMgr *manager.Manager
// lazyConnMgrMu guards the lazyConnMgr pointer for readers outside the
// engine loop (ActivatePeer). Writers hold it in addition to
// engine.syncMsgMux; all other reads stay under engine.syncMsgMux only.
lazyConnMgrMu sync.RWMutex
wg sync.WaitGroup
lazyCtx context.Context
@@ -238,12 +244,20 @@ func (e *ConnMgr) RemovePeerConn(peerKey string) {
conn.Log.Infof("removed peer from lazy conn manager")
}
// ActivatePeer wakes an idle lazy connection. Unlike the rest of ConnMgr it is
// safe for concurrent use: the lazy manager pointer is read under lazyConnMgrMu
// and the manager itself is internally synchronized, so callers outside the
// engine loop (DNS warm-up) do not need engine.syncMsgMux.
func (e *ConnMgr) ActivatePeer(ctx context.Context, conn *peer.Conn) {
if !e.isStartedWithLazyMgr() {
e.lazyConnMgrMu.RLock()
lazyConnMgr := e.lazyConnMgr
started := lazyConnMgr != nil && e.lazyCtxCancel != nil
e.lazyConnMgrMu.RUnlock()
if !started {
return
}
if found := e.lazyConnMgr.ActivatePeer(conn.GetKey()); found {
if found := lazyConnMgr.ActivatePeer(conn.GetKey()); found {
if err := conn.Open(ctx); err != nil {
conn.Log.Errorf("failed to open connection: %v", err)
}
@@ -268,16 +282,21 @@ func (e *ConnMgr) Close() {
e.lazyCtxCancel()
e.wg.Wait()
e.lazyConnMgrMu.Lock()
e.lazyConnMgr = nil
e.lazyConnMgrMu.Unlock()
}
func (e *ConnMgr) initLazyManager(engineCtx context.Context) {
cfg := manager.Config{
InactivityThreshold: inactivityThresholdEnv(),
}
e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface)
e.lazyConnMgrMu.Lock()
e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface)
e.lazyCtx, e.lazyCtxCancel = context.WithCancel(engineCtx)
e.lazyConnMgrMu.Unlock()
e.wg.Add(1)
go func() {
@@ -316,7 +335,10 @@ func (e *ConnMgr) closeManager(ctx context.Context) {
e.lazyCtxCancel()
e.wg.Wait()
e.lazyConnMgrMu.Lock()
e.lazyConnMgr = nil
e.lazyConnMgrMu.Unlock()
for _, peerID := range e.peerStore.PeersPubKey() {
e.peerStore.PeerConnOpen(ctx, peerID)

View File

@@ -1,10 +1,21 @@
package internal
import (
"context"
"net"
"net/netip"
"os"
"sync"
"testing"
"time"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
"github.com/netbirdio/netbird/client/iface/wgaddr"
"github.com/netbirdio/netbird/client/internal/lazyconn"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/internal/peerstore"
"github.com/netbirdio/netbird/monotime"
)
func TestResolveLazyForce(t *testing.T) {
@@ -38,3 +49,58 @@ func TestResolveLazyForce(t *testing.T) {
})
}
}
type mockLazyWGIface struct{}
func (mockLazyWGIface) RemovePeer(string) error { return nil }
func (mockLazyWGIface) UpdatePeer(string, []netip.Prefix, time.Duration, *net.UDPAddr, *wgtypes.Key) error {
return nil
}
func (mockLazyWGIface) IsUserspaceBind() bool { return false }
func (mockLazyWGIface) Address() wgaddr.Address { return wgaddr.Address{} }
func (mockLazyWGIface) LastActivities() map[string]monotime.Time { return nil }
func (mockLazyWGIface) MTU() uint16 { return 1280 }
// TestConnMgr_ActivatePeerConcurrentWithLifecycle exercises ActivatePeer from
// non-engine goroutines (the DNS warm-up path) racing the manager lifecycle,
// which stays on the engine loop. Run with -race: it fails if ActivatePeer
// still requires engine.syncMsgMux for safety.
func TestConnMgr_ActivatePeerConcurrentWithLifecycle(t *testing.T) {
t.Setenv(lazyconn.EnvLazyConn, "on")
status := peer.NewRecorder("https://mgm")
store := peerstore.NewConnStore()
connMgr := NewConnMgr(&EngineConfig{}, status, store, mockLazyWGIface{})
conn := newTestPeerConn(t, "peerA")
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
connMgr.Start(ctx)
done := make(chan struct{})
var wg sync.WaitGroup
for range 4 {
wg.Add(1)
go func() {
defer wg.Done()
for {
select {
case <-done:
return
default:
connMgr.ActivatePeer(ctx, conn)
}
}
}()
}
// Let the activators spin against the started manager, then tear it down
// underneath them and let them spin against the stopped manager.
time.Sleep(100 * time.Millisecond)
connMgr.Close()
time.Sleep(50 * time.Millisecond)
close(done)
wg.Wait()
}

View File

@@ -37,31 +37,43 @@ type resolver interface {
// record is left alone (it points at something outside our mesh, e.g.
// a non-peer upstream).
type PeerConnectivity interface {
IsConnectedByIP(ip string) (known, connected bool)
IsConnectedByIP(ip netip.Addr) (known, connected bool)
}
// PeerActivator wakes lazy-connection peers on demand. The local resolver calls
// it with the tunnel IPs an answer points at, so a peer that is idle (lazily
// disconnected) begins its WireGuard handshake at DNS-resolution time rather
// than racing the client's first request packet. nil disables warm-up.
// disconnected) starts connecting at DNS-resolution time rather than racing the
// client's first request packet. nil disables warm-up.
type PeerActivator interface {
// ActivatePeersByIP triggers wake-up for the peer(s) owning ips and blocks
// ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and blocks
// until one is connected or ctx (a short per-query budget) expires. It is a
// fast no-op for unknown or already-connected IPs.
ActivatePeersByIP(ctx context.Context, ips []string)
// fast no-op for unknown or already-connected addresses.
ActivatePeersByIP(ctx context.Context, addrs []netip.Addr)
}
const defaultLazyWarmupTimeout = 2 * time.Second
const (
defaultLazyWarmupTimeout = 2 * time.Second
envLazyWarmupTimeout = "NB_DNS_LAZY_WARMUP_TIMEOUT"
)
// lazyWarmupTimeout is the per-query budget for waking a lazy-connection peer a
// DNS answer points at. Tunable via NB_DNS_LAZY_WARMUP_TIMEOUT (a Go duration).
func lazyWarmupTimeout() time.Duration {
if v := os.Getenv("NB_DNS_LAZY_WARMUP_TIMEOUT"); v != "" {
if d, err := time.ParseDuration(v); err == nil && d > 0 {
return d
}
// lazyWarmupTimeoutFromEnv returns the per-query budget for waking a
// lazy-connection peer a DNS answer points at. Tunable via
// NB_DNS_LAZY_WARMUP_TIMEOUT (a Go duration). Parsed once at construction time.
func lazyWarmupTimeoutFromEnv() time.Duration {
v := os.Getenv(envLazyWarmupTimeout)
if v == "" {
return defaultLazyWarmupTimeout
}
return defaultLazyWarmupTimeout
d, err := time.ParseDuration(v)
if err != nil {
log.Warnf("invalid %s value %q, using default %s: %v", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout, err)
return defaultLazyWarmupTimeout
}
if d <= 0 {
log.Warnf("non-positive %s value %q, using default %s", envLazyWarmupTimeout, v, defaultLazyWarmupTimeout)
return defaultLazyWarmupTimeout
}
return d
}
type Resolver struct {
@@ -79,6 +91,9 @@ type Resolver struct {
// peerActivator, when non-nil, is called at resolution time to warm the
// lazy connection to the peer(s) an answer points at. nil disables warm-up.
peerActivator PeerActivator
// warmupTimeout is the per-query budget for the lazy-connection warm-up
// wait, resolved from the environment once at construction time.
warmupTimeout time.Duration
ctx context.Context
cancel context.CancelFunc
@@ -87,11 +102,12 @@ type Resolver struct {
func NewResolver() *Resolver {
ctx, cancel := context.WithCancel(context.Background())
return &Resolver{
records: make(map[dns.Question][]dns.RR),
domains: make(map[domain.Domain]struct{}),
zones: make(map[domain.Domain]bool),
ctx: ctx,
cancel: cancel,
records: make(map[dns.Question][]dns.RR),
domains: make(map[domain.Domain]struct{}),
zones: make(map[domain.Domain]bool),
warmupTimeout: lazyWarmupTimeoutFromEnv(),
ctx: ctx,
cancel: cancel,
}
}
@@ -160,7 +176,7 @@ func (d *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
result := d.lookupRecords(logger, question)
// Warm before filtering: activation flips a lazily-idle target to connected,
// which then lets it survive the disconnected-peer filter below.
d.warmLazyPeers(result.records)
d.warmLazyPeers(question, result.records)
result.records = d.filterDisconnectedPeerAnswers(logger, question, result.records)
replyMessage.Authoritative = !result.hasExternalData
replyMessage.Answer = result.records
@@ -534,8 +550,8 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns
kept := make([]dns.RR, 0, len(records))
var dropped int
for _, rr := range records {
ip := extractRecordIP(rr)
if ip == "" {
ip, ok := extractRecordAddr(rr)
if !ok {
kept = append(kept, rr)
continue
}
@@ -559,47 +575,52 @@ func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns
// warmLazyPeers triggers lazy-connection wake-up for the peers a resolved
// answer points at and waits briefly for one to connect, so the caller's first
// request doesn't race the WireGuard handshake. No-op when no activator is
// wired (lazy connections disabled) or the answer carries no peer IPs.
func (d *Resolver) warmLazyPeers(records []dns.RR) {
// request doesn't race the connection establishment. Warm-up is scoped to
// match-only (non-authoritative) zones — the synthesized private-service zones
// and user-created zones whose records point at specific peers. The account's
// peer zone is authoritative, so plain peer-name lookups never trigger warm-up;
// otherwise resolving any peer's name would wake its idle connection, defeating
// laziness mesh-wide. No-op when no activator is wired (lazy connections
// disabled) or the answer carries no peer IPs.
func (d *Resolver) warmLazyPeers(question dns.Question, records []dns.RR) {
d.mu.RLock()
activator := d.peerActivator
var nonAuth, found bool
if activator != nil {
nonAuth, found = d.findZone(question.Name)
}
d.mu.RUnlock()
if activator == nil {
if activator == nil || !found || !nonAuth {
return
}
var ips []string
var addrs []netip.Addr
for _, rr := range records {
if ip := extractRecordIP(rr); ip != "" {
ips = append(ips, ip)
if addr, ok := extractRecordAddr(rr); ok {
addrs = append(addrs, addr)
}
}
if len(ips) == 0 {
if len(addrs) == 0 {
return
}
ctx, cancel := context.WithTimeout(d.ctx, lazyWarmupTimeout())
ctx, cancel := context.WithTimeout(d.ctx, d.warmupTimeout)
defer cancel()
activator.ActivatePeersByIP(ctx, ips)
activator.ActivatePeersByIP(ctx, addrs)
}
// extractRecordIP returns the dotted-decimal / colon-hex IP carried by
// an A or AAAA record, or "" for any other record type.
func extractRecordIP(rr dns.RR) string {
// extractRecordAddr returns the IP address carried by an A or AAAA record.
// ok is false for any other record type or a record with no address.
func extractRecordAddr(rr dns.RR) (netip.Addr, bool) {
switch r := rr.(type) {
case *dns.A:
if r.A == nil {
return ""
}
return r.A.String()
addr, ok := netip.AddrFromSlice(r.A)
return addr.Unmap(), ok
case *dns.AAAA:
if r.AAAA == nil {
return ""
}
return r.AAAA.String()
addr, ok := netip.AddrFromSlice(r.AAAA)
return addr.Unmap(), ok
}
return ""
return netip.Addr{}, false
}
// Update replaces all zones and their records

View File

@@ -37,8 +37,8 @@ type mockPeerConnectivity struct {
byIP map[string]struct{ known, connected bool }
}
func (m mockPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) {
v, ok := m.byIP[ip]
func (m mockPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) {
v, ok := m.byIP[ip.String()]
if !ok {
return false, false
}

View File

@@ -2,8 +2,11 @@ package local
import (
"context"
"net"
"net/netip"
"sync"
"testing"
"time"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
@@ -13,19 +16,19 @@ import (
nbdns "github.com/netbirdio/netbird/dns"
)
// recordingActivator records the IPs it was asked to warm and returns
// recordingActivator records the addresses it was asked to warm and returns
// immediately, so ServeDNS is not blocked by the test.
type recordingActivator struct {
mu sync.Mutex
called bool
ips []string
addrs []netip.Addr
}
func (r *recordingActivator) ActivatePeersByIP(_ context.Context, ips []string) {
func (r *recordingActivator) ActivatePeersByIP(_ context.Context, addrs []netip.Addr) {
r.mu.Lock()
defer r.mu.Unlock()
r.called = true
r.ips = append(r.ips, ips...)
r.addrs = append(r.addrs, addrs...)
}
func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg {
@@ -36,10 +39,21 @@ func serveA(t *testing.T, resolver *Resolver, name string) *dns.Msg {
return resp
}
// serviceZone registers rec in a match-only (non-authoritative) zone, the shape
// the synthesized private-service zones arrive in.
func serviceZone(t *testing.T, resolver *Resolver, zone string, records ...nbdns.SimpleRecord) {
t.Helper()
resolver.Update([]nbdns.CustomZone{{
Domain: zone,
Records: records,
NonAuthoritative: true,
}})
}
func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) {
rec := nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
resolver := NewResolver()
require.NoError(t, resolver.RegisterRecord(rec))
serviceZone(t, resolver, "proxy.netbird.cloud", rec)
act := &recordingActivator{}
resolver.SetPeerActivator(act)
@@ -50,15 +64,15 @@ func TestLocalResolver_WarmsLazyPeerOnResolve(t *testing.T) {
act.mu.Lock()
defer act.mu.Unlock()
assert.True(t, act.called, "activator must be invoked for an overlay A answer")
assert.Contains(t, act.ips, "100.64.0.7", "activator must receive the answer's peer IP")
assert.True(t, act.called, "activator must be invoked for a service-zone A answer")
assert.Contains(t, act.addrs, netip.MustParseAddr("100.64.0.7"), "activator must receive the answer's peer IP")
}
func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) {
// With no activator wired the resolver behaves exactly as before.
rec := nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
rec := nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}
resolver := NewResolver()
require.NoError(t, resolver.RegisterRecord(rec))
serviceZone(t, resolver, "proxy.netbird.cloud", rec)
resp := serveA(t, resolver, rec.Name)
require.NotNil(t, resp, "resolver must still answer without an activator")
@@ -68,14 +82,91 @@ func TestLocalResolver_NoActivatorNoWarmup(t *testing.T) {
func TestLocalResolver_NoWarmupForMissingRecord(t *testing.T) {
// A query that resolves to nothing must not invoke the activator (no IPs).
resolver := NewResolver()
require.NoError(t, resolver.RegisterRecord(nbdns.SimpleRecord{Name: "svc.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"}))
serviceZone(t, resolver, "proxy.netbird.cloud",
nbdns.SimpleRecord{Name: "svc.proxy.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.7"})
act := &recordingActivator{}
resolver.SetPeerActivator(act)
serveA(t, resolver, "absent.netbird.cloud.")
serveA(t, resolver, "absent.proxy.netbird.cloud.")
act.mu.Lock()
defer act.mu.Unlock()
assert.False(t, act.called, "activator must not be invoked when there is no answer")
}
func TestLocalResolver_NoWarmupInAuthoritativeZone(t *testing.T) {
// The account's peer zone is authoritative; resolving a peer's name there
// must not wake its lazy connection — warm-up is scoped to match-only
// (non-authoritative) zones such as the synthesized private-service zones.
rec := nbdns.SimpleRecord{Name: "peer.netbird.cloud.", Type: 1, Class: nbdns.DefaultClass, TTL: 300, RData: "100.64.0.9"}
resolver := NewResolver()
resolver.Update([]nbdns.CustomZone{{
Domain: "netbird.cloud",
Records: []nbdns.SimpleRecord{rec},
}})
act := &recordingActivator{}
resolver.SetPeerActivator(act)
resp := serveA(t, resolver, rec.Name)
require.NotNil(t, resp, "resolver must answer")
require.NotEmpty(t, resp.Answer, "answer must carry the A record")
act.mu.Lock()
defer act.mu.Unlock()
assert.False(t, act.called, "activator must not be invoked for authoritative-zone answers")
}
func TestLazyWarmupTimeoutFromEnv(t *testing.T) {
tests := []struct {
name string
value string
envSet bool
want time.Duration
}{
{name: "unset uses default", want: defaultLazyWarmupTimeout},
{name: "valid overrides", value: "5s", envSet: true, want: 5 * time.Second},
{name: "invalid falls back", value: "not-a-duration", envSet: true, want: defaultLazyWarmupTimeout},
{name: "negative falls back", value: "-1s", envSet: true, want: defaultLazyWarmupTimeout},
{name: "zero falls back", value: "0s", envSet: true, want: defaultLazyWarmupTimeout},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.envSet {
t.Setenv(envLazyWarmupTimeout, tt.value)
}
assert.Equal(t, tt.want, lazyWarmupTimeoutFromEnv())
assert.Equal(t, tt.want, NewResolver().warmupTimeout, "constructor must resolve the timeout once")
})
}
}
func TestExtractRecordAddr(t *testing.T) {
t.Run("A record yields unmapped v4", func(t *testing.T) {
// net.ParseIP returns the 16-byte v4-in-v6 form, the same shape
// miekg/dns stores after parsing an A record; the extracted address
// must compare equal to a plain v4 netip.Addr.
addr, ok := extractRecordAddr(&dns.A{A: net.ParseIP("100.64.0.7")})
require.True(t, ok)
assert.True(t, addr.Is4())
assert.Equal(t, netip.MustParseAddr("100.64.0.7"), addr)
})
t.Run("AAAA record yields v6", func(t *testing.T) {
addr, ok := extractRecordAddr(&dns.AAAA{AAAA: net.ParseIP("fd00::1")})
require.True(t, ok)
assert.Equal(t, netip.MustParseAddr("fd00::1"), addr)
})
t.Run("A record without address", func(t *testing.T) {
_, ok := extractRecordAddr(&dns.A{})
assert.False(t, ok)
})
t.Run("non-address record", func(t *testing.T) {
_, ok := extractRecordAddr(&dns.CNAME{Target: "target.netbird.cloud."})
assert.False(t, ok)
})
}

View File

@@ -8,6 +8,7 @@ import (
"github.com/miekg/dns"
dnsconfig "github.com/netbirdio/netbird/client/internal/dns/config"
"github.com/netbirdio/netbird/client/internal/dns/local"
nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/route"
"github.com/netbirdio/netbird/shared/management/domain"
@@ -92,6 +93,11 @@ func (m *MockServer) SetFirewall(Firewall) {
// Mock implementation - no-op
}
// SetPeerActivator mock implementation of SetPeerActivator from Server interface
func (m *MockServer) SetPeerActivator(local.PeerActivator) {
// Mock implementation - no-op
}
// BeginBatch mock implementation of BeginBatch from Server interface
func (m *MockServer) BeginBatch() {
// Mock implementation - no-op

View File

@@ -82,6 +82,7 @@ type Server interface {
PopulateManagementDomain(mgmtURL *url.URL) error
SetRouteSources(selected, active func() route.HAMap)
SetFirewall(Firewall)
SetPeerActivator(local.PeerActivator)
}
type nsGroupsByDomain struct {
@@ -1442,11 +1443,11 @@ type localPeerConnectivity struct {
// IsConnectedByIP looks the IP up in the peerstore and surfaces both
// the known and connected bits. Used by Resolver.filterDisconnectedPeerAnswers.
func (l localPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) {
func (l localPeerConnectivity) IsConnectedByIP(ip netip.Addr) (known, connected bool) {
if l.status == nil {
return false, false
}
state, ok := l.status.PeerStateByIP(ip)
state, ok := l.status.PeerStateByIP(ip.String())
if !ok {
return false, false
}

View File

@@ -2,7 +2,7 @@ package internal
import (
"context"
"sync"
"net/netip"
"time"
"github.com/netbirdio/netbird/client/internal/peer"
@@ -12,33 +12,33 @@ import (
const dnsActivationPollInterval = 50 * time.Millisecond
// dnsPeerActivator wakes lazy-connection peers from the DNS resolution path. It
// implements dns/local.PeerActivator. ConnMgr is not thread-safe (guarded by
// the engine's syncMsgMux) while DNS queries run on their own goroutines, so
// activation runs under that mutex; the connection wait runs without it.
// implements dns/local.PeerActivator. DNS queries run on their own goroutines,
// so it only touches state that is safe for concurrent use — ConnMgr.ActivatePeer,
// peerstore.Store and peer.Status — and never takes the engine's syncMsgMux,
// keeping DNS resolution from contending with network-map processing.
type dnsPeerActivator struct {
connMgr *ConnMgr
peerStore *peerstore.Store
status *peer.Status
mu *sync.Mutex
// ctx is the engine's long-lived context. The connection dial is tied to it
// (not the per-query DNS wait budget) so a handshake that outlasts the wait
// still completes in the background rather than being cancelled at the deadline.
ctx context.Context
}
// ActivatePeersByIP triggers wake-up for the peer(s) owning ips and waits until
// one is connected or ctx (the per-query DNS wait budget) expires. Activation
// itself is tied to the engine's long-lived context so the dial survives a wait
// that times out. Unknown or already-connected IPs are skipped, so the
// steady-state (warm) path adds no latency.
func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, ips []string) {
// ActivatePeersByIP triggers wake-up for the peer(s) owning addrs and waits
// until one is connected or ctx (the per-query DNS wait budget) expires.
// Activation itself is tied to the engine's long-lived context so the dial
// survives a wait that times out. Unknown or already-connected addresses are
// skipped, so the steady-state (warm) path adds no latency.
func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, addrs []netip.Addr) {
if a == nil || a.connMgr == nil {
return
}
var pending []string
a.mu.Lock()
for _, ip := range ips {
for _, addr := range addrs {
ip := addr.String()
st, ok := a.status.PeerStateByIP(ip)
if !ok || st.ConnStatus == peer.StatusConnected {
continue
@@ -50,7 +50,6 @@ func (a *dnsPeerActivator) ActivatePeersByIP(ctx context.Context, ips []string)
a.connMgr.ActivatePeer(a.ctx, conn)
pending = append(pending, ip)
}
a.mu.Unlock()
if len(pending) == 0 {
return

View File

@@ -0,0 +1,129 @@
package internal
import (
"context"
"net/netip"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/client/internal/peer"
"github.com/netbirdio/netbird/client/internal/peerstore"
)
func newTestPeerConn(t *testing.T, key string) *peer.Conn {
t.Helper()
conn, err := peer.NewConn(peer.ConnConfig{
Key: key,
LocalKey: "local",
WgConfig: peer.WgConfig{
AllowedIps: []netip.Prefix{netip.MustParsePrefix("100.64.0.1/32")},
},
}, peer.ServiceDependencies{})
require.NoError(t, err)
return conn
}
func newTestDNSPeerActivator(t *testing.T) (*dnsPeerActivator, *peer.Status, *peerstore.Store) {
t.Helper()
status := peer.NewRecorder("https://mgm")
store := peerstore.NewConnStore()
// ConnMgr without Start: the lazy manager is nil, so ActivatePeer is a
// no-op — these tests exercise the activator's skip/wait logic.
connMgr := NewConnMgr(&EngineConfig{}, status, store, nil)
return &dnsPeerActivator{
connMgr: connMgr,
peerStore: store,
status: status,
ctx: context.Background(),
}, status, store
}
func TestDNSPeerActivator_NilSafe(t *testing.T) {
var a *dnsPeerActivator
a.ActivatePeersByIP(context.Background(), []netip.Addr{netip.MustParseAddr("100.64.0.1")})
}
// TestDNSPeerActivator_SkipsUnknownAndConnectedPeers verifies the steady-state
// (warm) path adds no latency: already-connected and unknown addresses never
// enter the wait loop.
func TestDNSPeerActivator_SkipsUnknownAndConnectedPeers(t *testing.T) {
a, status, store := newTestDNSPeerActivator(t)
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", "fd00::1"))
require.NoError(t, status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected}))
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
start := time.Now()
a.ActivatePeersByIP(ctx, []netip.Addr{
netip.MustParseAddr("100.64.0.1"), // known, connected -> skipped
netip.MustParseAddr("fd00::1"), // known via IPv6, connected -> skipped
netip.MustParseAddr("100.64.0.99"), // unknown -> skipped
})
require.Less(t, time.Since(start), time.Second, "no pending peer must mean no wait")
}
// TestDNSPeerActivator_WaitsForPendingPeerToConnect verifies the wait loop
// returns as soon as a pending peer reports connected, well before the
// per-query budget expires.
func TestDNSPeerActivator_WaitsForPendingPeerToConnect(t *testing.T) {
a, status, store := newTestDNSPeerActivator(t)
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
go func() {
time.Sleep(150 * time.Millisecond)
_ = status.UpdatePeerState(peer.State{PubKey: "peerA", ConnStatus: peer.StatusConnected})
}()
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
start := time.Now()
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
elapsed := time.Since(start)
require.GreaterOrEqual(t, elapsed, 100*time.Millisecond, "must wait for the pending peer")
require.Less(t, elapsed, 5*time.Second, "must return on connect, not at the deadline")
}
// TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle verifies a peer that
// never connects releases the DNS response at the per-query budget instead of
// blocking it indefinitely.
func TestDNSPeerActivator_ReturnsAtBudgetWhenPeerStaysIdle(t *testing.T) {
a, status, store := newTestDNSPeerActivator(t)
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
store.AddPeerConn("peerA", newTestPeerConn(t, "peerA"))
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
defer cancel()
start := time.Now()
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
elapsed := time.Since(start)
require.GreaterOrEqual(t, elapsed, 250*time.Millisecond, "must wait out the budget for a pending peer")
require.Less(t, elapsed, 5*time.Second, "must not block past the budget")
}
// TestDNSPeerActivator_NoWaitWithoutPeerConn verifies a known-but-idle peer
// with no connection object in the store is not waited on: there is nothing to
// activate, so waiting could only ever time out.
func TestDNSPeerActivator_NoWaitWithoutPeerConn(t *testing.T) {
a, status, _ := newTestDNSPeerActivator(t)
require.NoError(t, status.AddPeer("peerA", "a.netbird.cloud", "100.64.0.1", ""))
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
start := time.Now()
a.ActivatePeersByIP(ctx, []netip.Addr{netip.MustParseAddr("100.64.0.1")})
require.Less(t, time.Since(start), time.Second, "peer without a conn must not be waited on")
}

View File

@@ -657,15 +657,12 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL)
// Wire DNS-time lazy-connection warm-up now that the connection manager
// exists (it does not at DNS-server construction time). A DNS answer that
// points at an idle peer then wakes it before the client's first request.
if ds, ok := e.dnsServer.(*dns.DefaultServer); ok {
ds.SetPeerActivator(&dnsPeerActivator{
connMgr: e.connMgr,
peerStore: e.peerStore,
status: e.statusRecorder,
mu: e.syncMsgMux,
ctx: e.ctx,
})
}
e.dnsServer.SetPeerActivator(&dnsPeerActivator{
connMgr: e.connMgr,
peerStore: e.peerStore,
status: e.statusRecorder,
ctx: e.ctx,
})
e.srWatcher = guard.NewSRWatcher(e.signal, e.relayManager, e.mobileDep.IFaceDiscover, iceCfg)
e.srWatcher.Start(peer.IsForceRelayed())