mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-28 17:02:36 -04:00
## Describe your changes Under lazy connections, when a routing peer goes idle its WireGuard peer is torn down and re-created with a wake endpoint by the activity listener, carrying only the overlay /32 (`peerCfg.AllowedIPs`). The routed subnet prefixes are dropped from the device on the Connected→Idle transition. They are meant to be restored by the route watcher, which reacts to the peer's status change and calls `recalculateRoutes` → `AddAllowedIP`. Two things prevent that from healing the peer: - `AddAllowedIP` uses `update_only`, which is a silent no-op (no error) when the peer does not exist. While the peer is being torn down and re-armed with its wake endpoint, it is briefly absent, so a re-add that lands in that window is lost. - The allowed-IP refcounter only calls its add function on a prefix's 0→1 transition. The routed prefix stays referenced across the idle cycle, so once the device entry is gone the refcounter does not re-push it on its own, and nothing retries. As a result, traffic to the routed subnet is black-holed while the peer is idle. Because the wake endpoint only fires when a packet matches the peer's AllowedIPs, a packet to the subnet is dropped before reaching the wake endpoint, so it cannot wake the peer. The peer only recovers when woken by other means (e.g. a ping to its overlay IP). ## Approach This change keeps the existing Connected→Idle transition as-is and reconciles the AllowedIPs afterwards, avoiding any additional locking on the transition path. The peer is torn down and re-armed with its wake endpoint as today; the routed prefixes are then re-applied from the route manager's allowed-IP refcounter once the wake endpoint has been (re)armed. A single add-only method, `ReconcilePeerAllowedIPs(peerKey)`, re-applies every routed prefix currently tracked for the peer in the refcounter (the authoritative store; it already covers static, dynamic and dnsinterceptor routes). It runs whenever the peer's wake endpoint is (re)created in the lazy manager — every point where the activity listener builds it with the overlay /32 only: - **initial registration** (`AddPeer`, cold start): the route manager may have already pushed the peer's routes before the wake endpoint existed, so those `AddAllowedIP` calls no-op'd; the reconcile installs them on the freshly created wake endpoint. - **the two paths into idle** (`DeactivatePeer` on a remote GOAWAY, `onPeerInactivityTimedOut` on local inactivity): the peer is torn down and re-armed, so the routed prefixes must be re-applied. In every case the routed prefixes end up on the wake endpoint, so traffic to a routed subnet can wake the peer. Arming the wake endpoint and reconciling are wrapped in a single `armActivityListener` helper so the two always happen together. New helper: `refcounter.Counter.KeysMatching(pred)` to enumerate a peer's prefixes under the counter lock. Note on scope: the reconcile restores what the refcounter tracks. All routed AllowedIPs currently go through it, so this covers the routed-prefix case; it does not attempt to reconcile AllowedIPs installed outside the refcounter. The Idle→Connected (wake) path does not need this: the peer is not removed there (the listener close leaves it in place and only the endpoint is updated), so a concurrent `AddAllowedIP` lands normally. ## Testing Reproduced deterministically in a local dev setup (userspace client, `NB_WG_KERNEL_DISABLED=true`, `B_LAZY_CONN_INACTIVITY_THRESHOLD=1` inactivity threshold 1 min). A temporary 30s sleep in the tear-down → re-arm window widens the race so the route watcher's async `AddAllowedIP` reliably lands while the peer is absent and no-ops (the sleep is a test aid, not part of the change): - **without the reconcile:** after the peer goes idle, a ping to any routed IP — both a pre-existing route and one added during the window — black-holes; the peer never wakes. - **with the reconcile:** the same ping wakes the peer and passes. Added unit tests: `ReconcilePeerAllowedIPs` (re-applies all of a peer's tracked prefixes, scoped to that peer) and `refcounter.Counter.KeysMatching`. Note: `netbird status -d` is not a reliable signal for this — `AddPeerStateRoute` records the route regardless of whether the underlying `AddAllowedIP` no-op'd, so it reflects the route manager's intent rather than device state. The reliable signal is functional (ping the subnet from idle). ## Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (unit tests for the reconcile + `KeysMatching`) ## Documentation - [x] Documentation is **not needed** for this change (internal client behavior, no API / gRPC / CLI / flag change) <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6863"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787330960&installation_model_id=427504&pr_number=6863&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6863&signature=f3d6a97d7db82e92b3939fdd0f159c5ee74913ff88f4eb82e41e88fcb787aff4"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Routed IP assignments are automatically reconciled and restored whenever a peer’s lazy wake endpoint is armed or re-armed. * Routed allowed IPs are re-applied after inactivity transitions and monitoring re-initialization. * If reconciliation can’t be performed, the client safely skips it; if reconciliation encounters issues, failures are logged without stopping connection monitoring. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
396 lines
12 KiB
Go
396 lines
12 KiB
Go
package internal
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"strconv"
|
|
"sync"
|
|
"time"
|
|
|
|
log "github.com/sirupsen/logrus"
|
|
|
|
"github.com/netbirdio/netbird/client/internal/lazyconn"
|
|
"github.com/netbirdio/netbird/client/internal/lazyconn/manager"
|
|
"github.com/netbirdio/netbird/client/internal/peer"
|
|
"github.com/netbirdio/netbird/client/internal/peerstore"
|
|
"github.com/netbirdio/netbird/route"
|
|
)
|
|
|
|
// lazyForce is the resolved local decision for lazy connections, layered above the
|
|
// management feature flag. lazyForceNone defers to management.
|
|
type lazyForce int
|
|
|
|
const (
|
|
lazyForceNone lazyForce = iota
|
|
lazyForceOn
|
|
lazyForceOff
|
|
)
|
|
|
|
// ConnMgr coordinates both lazy connections (established on-demand) and permanent peer connections.
|
|
//
|
|
// The connection manager is responsible for:
|
|
// - Managing lazy connections via the lazyConnManager
|
|
// - Maintaining a list of excluded peers that should always have permanent connections
|
|
// - 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
|
|
iface lazyconn.WGIface
|
|
force lazyForce
|
|
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
|
|
|
|
// reconcileRoutedIPs re-applies a peer's routed allowed IPs after its lazy wake endpoint is
|
|
// (re)armed (Mode A at arm time). Injected by the engine; nil disables the reconcile.
|
|
reconcileRoutedIPs func(peerKey string) error
|
|
|
|
wg sync.WaitGroup
|
|
lazyCtx context.Context
|
|
lazyCtxCancel context.CancelFunc
|
|
}
|
|
|
|
// SetRoutedIPsReconciler injects the callback used to re-apply a peer's routed allowed IPs when
|
|
// its lazy wake endpoint is (re)armed. Must be called before the lazy manager starts.
|
|
func (e *ConnMgr) SetRoutedIPsReconciler(fn func(peerKey string) error) {
|
|
e.reconcileRoutedIPs = fn
|
|
}
|
|
|
|
func NewConnMgr(engineConfig *EngineConfig, statusRecorder *peer.Status, peerStore *peerstore.Store, iface lazyconn.WGIface) *ConnMgr {
|
|
e := &ConnMgr{
|
|
peerStore: peerStore,
|
|
statusRecorder: statusRecorder,
|
|
iface: iface,
|
|
force: resolveLazyForce(engineConfig.LazyConnection),
|
|
rosenpassEnabled: engineConfig.RosenpassEnabled,
|
|
}
|
|
return e
|
|
}
|
|
|
|
// Start initializes the connection manager. It starts the lazy connection manager when a
|
|
// local override forces it on; with no local override it waits for the management feature flag.
|
|
func (e *ConnMgr) Start(ctx context.Context) {
|
|
if e.lazyConnMgr != nil {
|
|
log.Errorf("lazy connection manager is already started")
|
|
return
|
|
}
|
|
|
|
switch e.force {
|
|
case lazyForceOff:
|
|
log.Infof("lazy connection manager is disabled by local override (%s or MDM policy)", lazyconn.EnvLazyConn)
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return
|
|
case lazyForceNone:
|
|
log.Infof("lazy connection manager is managed by the management feature flag")
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return
|
|
}
|
|
|
|
if e.rosenpassEnabled {
|
|
log.Warnf("rosenpass connection manager is enabled, lazy connection manager will not be started")
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return
|
|
}
|
|
|
|
e.initLazyManager(ctx)
|
|
e.statusRecorder.UpdateLazyConnection(true)
|
|
}
|
|
|
|
// UpdatedRemoteFeatureFlag is called when the remote feature flag is updated.
|
|
// If enabled, it initializes the lazy connection manager and start it. Do not need to call Start() again.
|
|
// If disabled, then it closes the lazy connection manager and open the connections to all peers.
|
|
func (e *ConnMgr) UpdatedRemoteFeatureFlag(ctx context.Context, enabled bool) error {
|
|
// a local override (NB_LAZY_CONN or local config) takes precedence over management
|
|
if e.force != lazyForceNone {
|
|
return nil
|
|
}
|
|
|
|
if enabled {
|
|
// if the lazy connection manager is already started, do not start it again
|
|
if e.lazyConnMgr != nil {
|
|
return nil
|
|
}
|
|
|
|
if e.rosenpassEnabled {
|
|
log.Infof("rosenpass connection manager is enabled, lazy connection manager will not be started")
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return nil
|
|
}
|
|
|
|
log.Infof("lazy connection manager is enabled by the management feature flag")
|
|
e.initLazyManager(ctx)
|
|
e.statusRecorder.UpdateLazyConnection(true)
|
|
return e.addPeersToLazyConnManager()
|
|
} else {
|
|
if e.lazyConnMgr == nil {
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return nil
|
|
}
|
|
log.Infof("lazy connection manager is disabled by management feature flag")
|
|
e.closeManager(ctx)
|
|
e.statusRecorder.UpdateLazyConnection(false)
|
|
return nil
|
|
}
|
|
}
|
|
|
|
// UpdateRouteHAMap updates the route HA mappings in the lazy connection manager
|
|
func (e *ConnMgr) UpdateRouteHAMap(haMap route.HAMap) {
|
|
if !e.isStartedWithLazyMgr() {
|
|
log.Debugf("lazy connection manager is not started, skipping UpdateRouteHAMap")
|
|
return
|
|
}
|
|
|
|
e.lazyConnMgr.UpdateRouteHAMap(haMap)
|
|
}
|
|
|
|
// SetExcludeList sets the list of peer IDs that should always have permanent connections.
|
|
func (e *ConnMgr) SetExcludeList(ctx context.Context, peerIDs map[string]bool) {
|
|
if e.lazyConnMgr == nil {
|
|
return
|
|
}
|
|
|
|
excludedPeers := make([]lazyconn.PeerConfig, 0, len(peerIDs))
|
|
|
|
for peerID := range peerIDs {
|
|
var peerConn *peer.Conn
|
|
var exists bool
|
|
if peerConn, exists = e.peerStore.PeerConn(peerID); !exists {
|
|
log.Warnf("failed to find peer conn for peerID: %s", peerID)
|
|
continue
|
|
}
|
|
|
|
lazyPeerCfg := lazyconn.PeerConfig{
|
|
PublicKey: peerID,
|
|
AllowedIPs: peerConn.WgConfig().AllowedIps,
|
|
PeerConnID: peerConn.ConnID(),
|
|
Log: peerConn.Log,
|
|
}
|
|
excludedPeers = append(excludedPeers, lazyPeerCfg)
|
|
}
|
|
|
|
added := e.lazyConnMgr.ExcludePeer(excludedPeers)
|
|
for _, peerID := range added {
|
|
var peerConn *peer.Conn
|
|
var exists bool
|
|
if peerConn, exists = e.peerStore.PeerConn(peerID); !exists {
|
|
// if the peer not exist in the store, it means that the engine will call the AddPeerConn in next step
|
|
continue
|
|
}
|
|
|
|
peerConn.Log.Infof("peer has been added to lazy connection exclude list, opening permanent connection")
|
|
if err := peerConn.Open(ctx); err != nil {
|
|
peerConn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
func (e *ConnMgr) AddPeerConn(ctx context.Context, peerKey string, conn *peer.Conn) (exists bool) {
|
|
if success := e.peerStore.AddPeerConn(peerKey, conn); !success {
|
|
return true
|
|
}
|
|
|
|
if !e.isStartedWithLazyMgr() {
|
|
if err := conn.Open(ctx); err != nil {
|
|
conn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
return
|
|
}
|
|
|
|
if !lazyconn.IsSupported(conn.AgentVersionString()) {
|
|
conn.Log.Warnf("peer does not support lazy connection (%s), open permanent connection", conn.AgentVersionString())
|
|
if err := conn.Open(ctx); err != nil {
|
|
conn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
return
|
|
}
|
|
|
|
lazyPeerCfg := lazyconn.PeerConfig{
|
|
PublicKey: peerKey,
|
|
AllowedIPs: conn.WgConfig().AllowedIps,
|
|
PeerConnID: conn.ConnID(),
|
|
Log: conn.Log,
|
|
}
|
|
excluded, err := e.lazyConnMgr.AddPeer(lazyPeerCfg)
|
|
if err != nil {
|
|
conn.Log.Errorf("failed to add peer to lazyconn manager: %v", err)
|
|
if err := conn.Open(ctx); err != nil {
|
|
conn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
return
|
|
}
|
|
|
|
if excluded {
|
|
conn.Log.Infof("peer is on lazy conn manager exclude list, opening connection")
|
|
if err := conn.Open(ctx); err != nil {
|
|
conn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
return
|
|
}
|
|
|
|
conn.Log.Infof("peer added to lazy conn manager")
|
|
return
|
|
}
|
|
|
|
func (e *ConnMgr) RemovePeerConn(peerKey string) {
|
|
conn, ok := e.peerStore.Remove(peerKey)
|
|
if !ok {
|
|
return
|
|
}
|
|
defer conn.Close(false)
|
|
|
|
if !e.isStartedWithLazyMgr() {
|
|
return
|
|
}
|
|
|
|
e.lazyConnMgr.RemovePeer(peerKey)
|
|
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) {
|
|
e.lazyConnMgrMu.RLock()
|
|
lazyConnMgr := e.lazyConnMgr
|
|
started := lazyConnMgr != nil && e.lazyCtxCancel != nil
|
|
e.lazyConnMgrMu.RUnlock()
|
|
if !started {
|
|
return
|
|
}
|
|
|
|
if found := lazyConnMgr.ActivatePeer(conn.GetKey()); found {
|
|
if err := conn.Open(ctx); err != nil {
|
|
conn.Log.Errorf("failed to open connection: %v", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
// DeactivatePeer deactivates a peer connection in the lazy connection manager.
|
|
// If locally the lazy connection is disabled, we force the peer connection open.
|
|
func (e *ConnMgr) DeactivatePeer(conn *peer.Conn) {
|
|
if !e.isStartedWithLazyMgr() {
|
|
return
|
|
}
|
|
|
|
conn.Log.Infof("closing peer connection: remote peer initiated inactive, idle lazy state and sent GOAWAY")
|
|
e.lazyConnMgr.DeactivatePeer(conn.ConnID())
|
|
}
|
|
|
|
func (e *ConnMgr) Close() {
|
|
if !e.isStartedWithLazyMgr() {
|
|
return
|
|
}
|
|
|
|
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(),
|
|
ReconcileAllowedIPs: e.reconcileRoutedIPs,
|
|
}
|
|
|
|
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() {
|
|
defer e.wg.Done()
|
|
e.lazyConnMgr.Start(e.lazyCtx)
|
|
}()
|
|
}
|
|
|
|
func (e *ConnMgr) addPeersToLazyConnManager() error {
|
|
peers := e.peerStore.PeersPubKey()
|
|
lazyPeerCfgs := make([]lazyconn.PeerConfig, 0, len(peers))
|
|
for _, peerID := range peers {
|
|
var peerConn *peer.Conn
|
|
var exists bool
|
|
if peerConn, exists = e.peerStore.PeerConn(peerID); !exists {
|
|
log.Warnf("failed to find peer conn for peerID: %s", peerID)
|
|
continue
|
|
}
|
|
|
|
lazyPeerCfg := lazyconn.PeerConfig{
|
|
PublicKey: peerID,
|
|
AllowedIPs: peerConn.WgConfig().AllowedIps,
|
|
PeerConnID: peerConn.ConnID(),
|
|
Log: peerConn.Log,
|
|
}
|
|
lazyPeerCfgs = append(lazyPeerCfgs, lazyPeerCfg)
|
|
}
|
|
|
|
return e.lazyConnMgr.AddActivePeers(lazyPeerCfgs)
|
|
}
|
|
|
|
func (e *ConnMgr) closeManager(ctx context.Context) {
|
|
if e.lazyConnMgr == nil {
|
|
return
|
|
}
|
|
|
|
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)
|
|
}
|
|
}
|
|
|
|
func (e *ConnMgr) isStartedWithLazyMgr() bool {
|
|
return e.lazyConnMgr != nil && e.lazyCtxCancel != nil
|
|
}
|
|
|
|
// resolveLazyForce determines the local override. NB_LAZY_CONN takes precedence; when it
|
|
// is unset the MDM policy override (mdmState) applies. Either wins in both directions over
|
|
// the management feature flag; StateUnset for both defers to management.
|
|
func resolveLazyForce(mdmState lazyconn.State) lazyForce {
|
|
state := lazyconn.EnvState()
|
|
if state == lazyconn.StateUnset {
|
|
state = mdmState
|
|
}
|
|
|
|
switch state {
|
|
case lazyconn.StateOn:
|
|
return lazyForceOn
|
|
case lazyconn.StateOff:
|
|
return lazyForceOff
|
|
default:
|
|
return lazyForceNone
|
|
}
|
|
}
|
|
|
|
func inactivityThresholdEnv() *time.Duration {
|
|
envValue := os.Getenv(lazyconn.EnvInactivityThreshold)
|
|
if envValue == "" {
|
|
return nil
|
|
}
|
|
|
|
parsedMinutes, err := strconv.Atoi(envValue)
|
|
if err != nil || parsedMinutes <= 0 {
|
|
return nil
|
|
}
|
|
|
|
d := time.Duration(parsedMinutes) * time.Minute
|
|
return &d
|
|
}
|