-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[client, management] Support per-peer lazy connection state and default proxy peers to lazy #6762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lixmal
wants to merge
5
commits into
main
Choose a base branch
from
lazy-conn-per-peer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
82ea626
Support per-peer lazy connection state and default proxy peers to lazy
lixmal 7602afe
Classify forward targets from incoming config in lazy exclusion
lixmal 9d89757
Set IsUserspaceBind mock so lazy manager starts in engine test
lixmal 99ceb9b
Skip lazy exclude reconciliation when the set is unchanged
lixmal 75fb373
Keep cached lazy flag when a sync carries no peer config
lixmal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package internal | |
|
|
||
| import ( | ||
| "context" | ||
| "maps" | ||
| "os" | ||
| "strconv" | ||
| "sync" | ||
|
|
@@ -14,6 +15,7 @@ import ( | |
| "github.com/netbirdio/netbird/client/internal/peer" | ||
| "github.com/netbirdio/netbird/client/internal/peerstore" | ||
| "github.com/netbirdio/netbird/route" | ||
| mgmProto "github.com/netbirdio/netbird/shared/management/proto" | ||
| ) | ||
|
|
||
| // lazyForce is the resolved local decision for lazy connections, layered above the | ||
|
|
@@ -40,8 +42,14 @@ type ConnMgr struct { | |
| iface lazyconn.WGIface | ||
| force lazyForce | ||
| rosenpassEnabled bool | ||
| // remoteLazyEnabled caches the account-wide lazy feature flag from management. | ||
| // It is the default for peers that do not carry a per-peer lazy hint. | ||
| remoteLazyEnabled bool | ||
|
|
||
| lazyConnMgr *manager.Manager | ||
| // appliedExcludeList is the exclude set last handed to the lazy manager, kept so an | ||
| // unchanged set on the next sync skips the O(n) reconciliation. | ||
| appliedExcludeList map[string]bool | ||
|
|
||
| wg sync.WaitGroup | ||
| lazyCtx context.Context | ||
|
|
@@ -59,69 +67,56 @@ func NewConnMgr(engineConfig *EngineConfig, statusRecorder *peer.Status, peerSto | |
| 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. | ||
| // Start initializes the connection manager. The lazy connection manager always runs so that | ||
| // per-peer lazy defaults (e.g. proxy peers) work even when the account flag is off; the | ||
| // account flag and the local override decide the default lazy state per peer (see | ||
| // PeerLazyDefault). Rosenpass is the only condition that disables it. | ||
| 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") | ||
| log.Warnf("rosenpass is enabled, lazy connection manager will not be started") | ||
| e.statusRecorder.UpdateLazyConnection(false) | ||
| return | ||
| } | ||
|
|
||
| e.initLazyManager(ctx) | ||
| e.statusRecorder.UpdateLazyConnection(true) | ||
| e.statusRecorder.UpdateLazyConnection(e.PeerLazyDefault(mgmProto.LazyState_LazyStateDefault)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this capturing the fact that proxy peers are lazy when account flag is not set?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope, right now it reflects the account wide setting |
||
| } | ||
|
|
||
| // 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 | ||
| // UpdatedRemoteFeatureFlag caches the account-wide lazy feature flag. The manager itself is | ||
| // not started or stopped here; the per-sync exclude-list reconciliation moves normal peers | ||
| // between the lazy and always-active sets when the flag flips. | ||
| func (e *ConnMgr) UpdatedRemoteFeatureFlag(_ context.Context, enabled bool) error { | ||
| e.remoteLazyEnabled = enabled | ||
| if e.isStartedWithLazyMgr() { | ||
| e.statusRecorder.UpdateLazyConnection(e.PeerLazyDefault(mgmProto.LazyState_LazyStateDefault)) | ||
| } | ||
| 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 | ||
| } | ||
| // PeerLazyDefault reports whether a peer should be lazy. The local override | ||
| // (NB_LAZY_CONN/MDM) wins over everything; without a local override the | ||
| // management per-peer state applies (LazyStateLazy/Eager force the decision), | ||
| // and LazyStateDefault follows the account-wide flag. | ||
| func (e *ConnMgr) PeerLazyDefault(state mgmProto.LazyState) bool { | ||
| switch e.force { | ||
| case lazyForceOn: | ||
| return true | ||
| case lazyForceOff: | ||
| return false | ||
| } | ||
|
|
||
| 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 | ||
| switch state { | ||
| case mgmProto.LazyState_LazyStateLazy: | ||
| return true | ||
| case mgmProto.LazyState_LazyStateEager: | ||
| return false | ||
| default: | ||
| return e.remoteLazyEnabled | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -141,6 +136,13 @@ func (e *ConnMgr) SetExcludeList(ctx context.Context, peerIDs map[string]bool) { | |
| return | ||
| } | ||
|
|
||
| // The exclude set is recomputed every sync but rarely changes; skip the O(n) | ||
| // store lookups and reconciliation when it matches what was already applied. | ||
| if maps.Equal(peerIDs, e.appliedExcludeList) { | ||
| return | ||
| } | ||
| e.appliedExcludeList = maps.Clone(peerIDs) | ||
|
|
||
| excludedPeers := make([]lazyconn.PeerConfig, 0, len(peerIDs)) | ||
|
|
||
| for peerID := range peerIDs { | ||
|
|
@@ -176,12 +178,16 @@ func (e *ConnMgr) SetExcludeList(ctx context.Context, peerIDs map[string]bool) { | |
| } | ||
| } | ||
|
|
||
| func (e *ConnMgr) AddPeerConn(ctx context.Context, peerKey string, conn *peer.Conn) (exists bool) { | ||
| // AddPeerConn registers a peer connection. permanent requests an always-active connection | ||
| // (the peer belongs to the exclude set: a forwarder, or a peer that is not lazy by policy). | ||
| // Non-permanent peers are handed to the lazy manager. The subsequent SetExcludeList call | ||
| // reconciles membership for existing peers across flag flips. | ||
| func (e *ConnMgr) AddPeerConn(ctx context.Context, peerKey string, conn *peer.Conn, permanent bool) (exists bool) { | ||
| if success := e.peerStore.AddPeerConn(peerKey, conn); !success { | ||
| return true | ||
| } | ||
|
|
||
| if !e.isStartedWithLazyMgr() { | ||
| if !e.isStartedWithLazyMgr() || permanent { | ||
| if err := conn.Open(ctx); err != nil { | ||
| conn.Log.Errorf("failed to open connection: %v", err) | ||
| } | ||
|
|
@@ -269,13 +275,15 @@ func (e *ConnMgr) Close() { | |
| e.lazyCtxCancel() | ||
| e.wg.Wait() | ||
| e.lazyConnMgr = nil | ||
| e.appliedExcludeList = nil | ||
| } | ||
|
|
||
| func (e *ConnMgr) initLazyManager(engineCtx context.Context) { | ||
| cfg := manager.Config{ | ||
| InactivityThreshold: inactivityThresholdEnv(), | ||
| } | ||
| e.lazyConnMgr = manager.NewManager(cfg, engineCtx, e.peerStore, e.iface) | ||
| e.appliedExcludeList = nil | ||
|
|
||
| e.lazyCtx, e.lazyCtxCancel = context.WithCancel(engineCtx) | ||
|
|
||
|
|
@@ -286,43 +294,6 @@ func (e *ConnMgr) initLazyManager(engineCtx context.Context) { | |
| }() | ||
| } | ||
|
|
||
| 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.lazyConnMgr = nil | ||
|
|
||
| for _, peerID := range e.peerStore.PeersPubKey() { | ||
| e.peerStore.PeerConnOpen(ctx, peerID) | ||
| } | ||
| } | ||
|
|
||
| func (e *ConnMgr) isStartedWithLazyMgr() bool { | ||
| return e.lazyConnMgr != nil && e.lazyCtxCancel != nil | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are starting the lazy manager even when account and local lazy flag say to not use lazy. (majority of customers today?)
By doing so, we are running
toExcludedLazyPeerson every sync update potentially on a long slice ofRemotePeerConfigs, even though the result will exactly the entire map of all peers.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a check to avoid running the expensive part when nothing has changed