From bd99d89bac9ce60ff5b7ba71728175c07350fe13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Wed, 19 Aug 2026 17:15:57 +0200 Subject: [PATCH] [client] Sweep connections on network loss via a shared netevents manager Losing the last network only flipped the availability state: the dead management, signal and relay sockets stayed silently connected until their own timeouts, so the client kept reporting Connected with no network at all. Introduce client/netevents with a Manager that ties the availability state, the connection sweeper and the status recorder together, and move the netstate and netsweep packages under it (netsweep renamed to sweep). SetNetworkAvailable(false) now also sweeps the registered connections so their owners redial and the listener reaches the NoNetwork state. The Android and iOS bindings own a Manager instance and inject it through the constructors; consumers hold the concrete *Manager whose nil zero value reports always-online and never sweeps, with interfaces kept only as parameter contracts. The relay guard settle wait moved into the Manager as WaitSettled, removing the netevents import from the relay package. --- client/android/client.go | 35 ++-- client/grpc/dialer_generic.go | 9 +- client/grpc/dialer_js.go | 11 +- client/grpc/retry.go | 17 +- client/grpc/retry_test.go | 2 +- client/internal/connect.go | 40 ++--- client/internal/engine.go | 6 +- client/internal/peer/conn.go | 4 +- client/internal/peer/guard/guard.go | 19 ++- .../peer/guard/guard_netstate_test.go | 2 +- client/ios/NetBirdSDK/client.go | 31 ++-- client/netevents/netevents.go | 158 ++++++++++++++++++ client/netevents/netevents_test.go | 34 ++++ client/{ => netevents}/netstate/netstate.go | 0 .../{ => netevents}/netstate/netstate_test.go | 0 .../sweep}/quick_retry.go | 4 +- .../sweep}/quick_retry_test.go | 2 +- .../netsweep.go => netevents/sweep/sweep.go} | 8 +- .../sweep/sweep_test.go} | 2 +- shared/management/client/grpc.go | 34 ++-- shared/relay/client/client.go | 24 ++- shared/relay/client/guard.go | 101 +++++------ shared/relay/client/guard_test.go | 30 ---- shared/relay/client/manager.go | 23 +-- shared/relay/client/picker.go | 5 +- shared/signal/client/grpc.go | 34 ++-- 26 files changed, 382 insertions(+), 253 deletions(-) create mode 100644 client/netevents/netevents.go create mode 100644 client/netevents/netevents_test.go rename client/{ => netevents}/netstate/netstate.go (100%) rename client/{ => netevents}/netstate/netstate_test.go (100%) rename client/{netsweep => netevents/sweep}/quick_retry.go (90%) rename client/{netsweep => netevents/sweep}/quick_retry_test.go (99%) rename client/{netsweep/netsweep.go => netevents/sweep/sweep.go} (96%) rename client/{netsweep/netsweep_test.go => netevents/sweep/sweep_test.go} (99%) delete mode 100644 shared/relay/client/guard_test.go diff --git a/client/android/client.go b/client/android/client.go index 7eea83dc0..5bd0d1e10 100644 --- a/client/android/client.go +++ b/client/android/client.go @@ -26,8 +26,7 @@ import ( "github.com/netbirdio/netbird/client/internal/routemanager" "github.com/netbirdio/netbird/client/internal/stdnet" "github.com/netbirdio/netbird/client/net" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents" "github.com/netbirdio/netbird/client/system" "github.com/netbirdio/netbird/formatter" "github.com/netbirdio/netbird/route" @@ -82,13 +81,10 @@ type Client struct { deviceName string uiVersion string networkChangeListener listener.NetworkChangeListener - // netState outlives engine restarts: it mirrors the OS connectivity, not - // the engine lifecycle. Run and RunWithoutLogin inject it into each new - // ConnectClient, which distributes it to every reconnection loop. - netState *netstate.State - - // sweeper also outlives engine restarts; NotifyNetworkChange sweeps it. - sweeper *netsweep.Sweeper + // netMgr outlives engine restarts: it mirrors the OS connectivity, not + // the engine lifecycle. Run and RunWithoutLogin inject its state and + // sweeper into each new ConnectClient. + netMgr *netevents.Manager stateMu sync.RWMutex connectClient *internal.ConnectClient @@ -153,16 +149,16 @@ func NewClient(androidSDKVersion int, deviceName string, uiVersion string, tunAd net.SetAndroidProtectSocketFn(tunAdapter.ProtectSocket) system.SetIFaceDiscover(iFaceDiscover) + recorder := peer.NewRecorder("") return &Client{ deviceName: deviceName, uiVersion: uiVersion, tunAdapter: tunAdapter, iFaceDiscover: iFaceDiscover, - recorder: peer.NewRecorder(""), + recorder: recorder, ctxCancelLock: &sync.Mutex{}, networkChangeListener: networkChangeListener, - netState: netstate.New(), - sweeper: netsweep.New(), + netMgr: netevents.NewManager(recorder), } } @@ -203,8 +199,9 @@ func (c *Client) Run(platformFiles PlatformFiles, urlOpener URLOpener, isAndroid } // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) + connectClient := internal.NewConnectClient(ctx, cfg, c.recorder, - internal.WithNetworkState(c.netState), internal.WithSweeper(c.sweeper)) + internal.WithNetEvents(c.netMgr)) c.setState(cfg, cacheDir, cfgFile, connectClient) // This path runs the interactive SSO flow, so reaching here means the peer // is authenticated again — release the latch Status() reports from. Clear @@ -246,7 +243,7 @@ func (c *Client) RunWithoutLogin(platformFiles PlatformFiles, dns *DNSList, dnsR // todo do not throw error in case of cancelled context ctx = internal.CtxInitState(ctx) connectClient := internal.NewConnectClient(ctx, cfg, c.recorder, - internal.WithNetworkState(c.netState), internal.WithSweeper(c.sweeper)) + internal.WithNetEvents(c.netMgr)) c.setState(cfg, cacheDir, cfgFile, connectClient) return connectClient.RunOnAndroid(c.tunAdapter, c.iFaceDiscover, c.networkChangeListener, slices.Clone(dns.items), dnsReadyListener, stateFile, cacheDir) } @@ -298,9 +295,12 @@ func (c *Client) GetTunSettings() (*TunSettings, error) { // While unavailable, the internal reconnect loops suspend their attempts and // the connection listener reports NoNetwork instead of Connecting; when // availability returns, the loops resume immediately with a fresh backoff. +// Losing the last network also sweeps the registered connections: nothing can +// redial while offline, so the stale sockets would otherwise stay silently +// "connected" until their own timeouts and the client would keep reporting +// Connected with no network at all. func (c *Client) SetNetworkAvailable(available bool) { - c.netState.Set(available) - c.recorder.SetNetworkAvailable(available) + c.netMgr.SetNetworkAvailable(available) } // NotifyNetworkChange marks the management, signal and relay connections @@ -308,8 +308,7 @@ func (c *Client) SetNetworkAvailable(available bool) { // whatever has not redialed on the new network by then. The engine and the // TUN device stay untouched. func (c *Client) NotifyNetworkChange() { - c.sweeper.MarkNetworkChange() - log.Infof("network change: connections marked stale") + c.netMgr.NotifyNetworkChange() } // DebugBundle generates a debug bundle, uploads it, and returns the upload key. diff --git a/client/grpc/dialer_generic.go b/client/grpc/dialer_generic.go index 8a80525e9..737787223 100644 --- a/client/grpc/dialer_generic.go +++ b/client/grpc/dialer_generic.go @@ -16,9 +16,14 @@ import ( "google.golang.org/grpc" nbnet "github.com/netbirdio/netbird/client/net" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents/sweep" ) +// Sweeper registers in-flight dials for the network change sweep. +type Sweeper interface { + StartDial(ctx context.Context) *sweep.Dial +} + func WithCustomDialer(_ bool, _ string) grpc.DialOption { return grpc.WithContextDialer(dialContext) } @@ -26,7 +31,7 @@ func WithCustomDialer(_ bool, _ string) grpc.DialOption { // WithSweeper dials like WithCustomDialer but registers connections and // dials with the sweeper. Append it after WithCustomDialer: gRPC applies // dial options in order, so the later context dialer wins. -func WithSweeper(sweeper *netsweep.Sweeper) grpc.DialOption { +func WithSweeper(sweeper Sweeper) grpc.DialOption { return grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { dial := sweeper.StartDial(ctx) defer dial.Release() diff --git a/client/grpc/dialer_js.go b/client/grpc/dialer_js.go index 8863756d7..4ff4ceb20 100644 --- a/client/grpc/dialer_js.go +++ b/client/grpc/dialer_js.go @@ -1,12 +1,19 @@ package grpc import ( + "context" + "google.golang.org/grpc" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents/sweep" "github.com/netbirdio/netbird/util/wsproxy/client" ) +// Sweeper registers in-flight dials for the network change sweep. +type Sweeper interface { + StartDial(ctx context.Context) *sweep.Dial +} + // WithCustomDialer returns a gRPC dial option that uses WebSocket transport for WASM/JS environments. // The component parameter specifies the WebSocket proxy component path (e.g., "/management", "/signal"). func WithCustomDialer(tlsEnabled bool, component string) grpc.DialOption { @@ -14,6 +21,6 @@ func WithCustomDialer(tlsEnabled bool, component string) grpc.DialOption { } // WithSweeper is a no-op on WASM/JS: there is no network change signal. -func WithSweeper(_ *netsweep.Sweeper) grpc.DialOption { +func WithSweeper(_ Sweeper) grpc.DialOption { return grpc.EmptyDialOption{} } diff --git a/client/grpc/retry.go b/client/grpc/retry.go index 754ffa341..0bb6037bf 100644 --- a/client/grpc/retry.go +++ b/client/grpc/retry.go @@ -6,16 +6,19 @@ import ( "time" "github.com/cenkalti/backoff/v4" - - "github.com/netbirdio/netbird/client/netstate" ) +// ChangeWatcher exposes OS network availability transitions. +type ChangeWatcher interface { + Changed() <-chan struct{} +} + // Retry mirrors backoff.Retry, but the sleep between attempts also wakes on // OS network availability transitions: an operation cut down by a network // change retries the moment the network settles instead of sleeping through -// the recovery. A nil netState never fires, leaving plain backoff.Retry +// the recovery. A nil watcher never fires, leaving plain backoff.Retry // behavior. -func Retry(ctx context.Context, operation backoff.Operation, bo backoff.BackOff, netState *netstate.State) error { +func Retry(ctx context.Context, operation backoff.Operation, bo backoff.BackOff, watcher ChangeWatcher) error { bo.Reset() for { err := operation() @@ -36,10 +39,14 @@ func Retry(ctx context.Context, operation backoff.Operation, bo backoff.BackOff, return err } + var changed <-chan struct{} + if watcher != nil { + changed = watcher.Changed() + } timer := time.NewTimer(next) select { case <-timer.C: - case <-netState.Changed(): + case <-changed: timer.Stop() case <-ctx.Done(): timer.Stop() diff --git a/client/grpc/retry_test.go b/client/grpc/retry_test.go index 4edca47b6..266bb93e5 100644 --- a/client/grpc/retry_test.go +++ b/client/grpc/retry_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents/netstate" ) func TestRetryWakesOnNetworkChange(t *testing.T) { diff --git a/client/internal/connect.go b/client/internal/connect.go index e45ecca44..161c76079 100644 --- a/client/internal/connect.go +++ b/client/internal/connect.go @@ -38,8 +38,7 @@ import ( "github.com/netbirdio/netbird/client/internal/updater" "github.com/netbirdio/netbird/client/internal/updater/installer" nbnet "github.com/netbirdio/netbird/client/net" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents" cProto "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/ssh" sshconfig "github.com/netbirdio/netbird/client/ssh/config" @@ -73,28 +72,17 @@ type ConnectClient struct { persistSyncResponse bool - // netState gates every reconnection loop on OS-reported network - // availability. Nil (the default) disables gating; mobile platforms - // inject it via WithNetworkState. - netState *netstate.State - - // sweeper cuts the management, signal and relay connections on network - // change; nil disables it. - sweeper *netsweep.Sweeper + // netEvents gates every reconnection loop on OS-reported network + // availability and sweeps connections on network change. + netEvents *netevents.Manager } // ConnectClientOption configures optional ConnectClient behavior. type ConnectClientOption func(*ConnectClient) -// WithNetworkState injects the OS network availability state that gates every -// reconnection loop; without it gating is disabled. -func WithNetworkState(netState *netstate.State) ConnectClientOption { - return func(c *ConnectClient) { c.netState = netState } -} - -// WithSweeper injects the network change sweeper. -func WithSweeper(sweeper *netsweep.Sweeper) ConnectClientOption { - return func(c *ConnectClient) { c.sweeper = sweeper } +// WithNetEvents injects the OS network event handling. +func WithNetEvents(events *netevents.Manager) ConnectClientOption { + return func(c *ConnectClient) { c.netEvents = events } } func NewConnectClient( @@ -305,7 +293,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan } // suspend connection attempts while the OS reports no usable network - if waited, err := c.netState.Wait(c.ctx); err != nil { + if waited, err := c.netEvents.Wait(c.ctx); err != nil { return nil } else if waited { backOff.Reset() @@ -323,7 +311,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan log.Debugf("connecting to the Management service %s", c.config.ManagementURL.Host) mgmClient, err := mgm.NewClient(engineCtx, c.config.ManagementURL.Host, myPrivateKey, mgmTlsEnabled, - mgm.WithNetworkState(c.netState), mgm.WithSweeper(c.sweeper)) + mgm.WithNetEvents(c.netEvents)) if err != nil { // On daemon shutdown / Down() the parent context is cancelled // and the dial fails with "context canceled". Wrapping that @@ -398,7 +386,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan }() // with the global Netbird config in hand connect (just a connection, no stream yet) Signal - signalClient, err := connectToSignal(engineCtx, loginResp.GetNetbirdConfig(), myPrivateKey, c.netState, c.sweeper) + signalClient, err := connectToSignal(engineCtx, loginResp.GetNetbirdConfig(), myPrivateKey, c.netEvents) if err != nil { log.Error(err) return wrapErr(err) @@ -435,7 +423,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan } relayManager := relayClient.NewManager(engineCtx, relayURLs, myPrivateKey.PublicKey().String(), engineConfig.MTU, - relayClient.WithNetworkState(c.netState), relayClient.WithSweeper(c.sweeper)) + relayClient.WithNetEvents(c.netEvents)) c.statusRecorder.SetRelayMgr(relayManager) if len(relayURLs) > 0 { if token != nil { @@ -463,7 +451,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan UpdateManager: c.updateManager, ClientMetrics: c.clientMetrics, MetricsCtx: c.ctx, - NetState: c.netState, + NetState: c.netEvents, }, mobileDependency) engine.SetSyncResponsePersistence(c.persistSyncResponse) c.engine = engine @@ -723,7 +711,7 @@ func selectMTU(localMTU uint16, peerMTU int32) uint16 { } // connectToSignal creates Signal Service client and established a connection -func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourPrivateKey wgtypes.Key, netState *netstate.State, sweeper *netsweep.Sweeper) (*signal.GrpcClient, error) { +func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourPrivateKey wgtypes.Key, netEvents *netevents.Manager) (*signal.GrpcClient, error) { var sigTLSEnabled bool if wtConfig.Signal.Protocol == mgmProto.HostConfig_HTTPS { sigTLSEnabled = true @@ -732,7 +720,7 @@ func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourP } signalClient, err := signal.NewClient(ctx, wtConfig.Signal.Uri, ourPrivateKey, sigTLSEnabled, - signal.WithNetworkState(netState), signal.WithSweeper(sweeper)) + signal.WithNetEvents(netEvents)) if err != nil { log.Errorf("error while connecting to the Signal Exchange Service %s: %s", wtConfig.Signal.Uri, err) return nil, gstatus.Errorf(codes.FailedPrecondition, "failed connecting to Signal Service : %s", err) diff --git a/client/internal/engine.go b/client/internal/engine.go index 5380651a5..d9b8af3fd 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -59,7 +59,7 @@ import ( "github.com/netbirdio/netbird/client/internal/syncstore" "github.com/netbirdio/netbird/client/internal/updater" "github.com/netbirdio/netbird/client/jobexec" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents" cProto "github.com/netbirdio/netbird/client/proto" "github.com/netbirdio/netbird/client/system" nbdns "github.com/netbirdio/netbird/dns" @@ -184,7 +184,7 @@ type EngineServices struct { MetricsCtx context.Context // NetState gates the reconnection loops on OS-reported network // availability; nil disables gating. - NetState *netstate.State + NetState *netevents.Manager } // Engine is a mechanism responsible for reacting on Signal and Management stream events and managing connections to the remote peers. @@ -210,7 +210,7 @@ type Engine struct { // netState gates the peer reconnection guards on OS-reported network // availability; nil disables gating. - netState *netstate.State + netState *netevents.Manager // STUNs is a list of STUN servers used by ICE STUNs []*stun.URI diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index a3c320027..2a43c887c 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -26,7 +26,7 @@ import ( "github.com/netbirdio/netbird/client/internal/portforward" "github.com/netbirdio/netbird/client/internal/rosenpass" "github.com/netbirdio/netbird/client/internal/stdnet" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents" "github.com/netbirdio/netbird/route" relayClient "github.com/netbirdio/netbird/shared/relay/client" ) @@ -97,7 +97,7 @@ type ConnConfig struct { // NetworkState gates the reconnection guard on OS-reported network // availability; nil disables gating. - NetworkState *netstate.State + NetworkState *netevents.Manager } type Conn struct { diff --git a/client/internal/peer/guard/guard.go b/client/internal/peer/guard/guard.go index 68d77d318..e2ad32a73 100644 --- a/client/internal/peer/guard/guard.go +++ b/client/internal/peer/guard/guard.go @@ -6,8 +6,6 @@ import ( "github.com/cenkalti/backoff/v4" log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/client/netstate" ) // ConnStatus represents the connection state as seen by the guard. @@ -24,6 +22,12 @@ const ( type connStatusFunc func() ConnStatus +// NetworkWatcher is the availability view the guard gates reconnects on. +type NetworkWatcher interface { + IsOnline() bool + Changed() <-chan struct{} +} + // Guard is responsible for the reconnection logic. // It will trigger to send an offer to the peer then has connection issues. // Watch these events: @@ -39,14 +43,14 @@ type Guard struct { srWatcher *SRWatcher // netState gates reconnect attempts on OS-reported network availability; // nil disables gating. - netState *netstate.State + netState NetworkWatcher relayedConnDisconnected chan struct{} iCEConnDisconnected chan struct{} } // NewGuard creates a reconnection guard for a peer connection. A nil netState // disables network availability gating. -func NewGuard(log *log.Entry, isConnectedFn connStatusFunc, timeout time.Duration, srWatcher *SRWatcher, netState *netstate.State) *Guard { +func NewGuard(log *log.Entry, isConnectedFn connStatusFunc, timeout time.Duration, srWatcher *SRWatcher, netState NetworkWatcher) *Guard { return &Guard{ log: log, isConnectedOnAllWay: isConnectedFn, @@ -104,14 +108,17 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) { iceState := &iceRetryState{log: g.log} defer iceState.reset() - netChanged := g.netState.Changed() + var netChanged <-chan struct{} + if g.netState != nil { + netChanged = g.netState.Changed() + } for { select { case <-tickerChannel: // skip attempts while the OS reports no usable network; the // netChanged case below resumes the loop once it returns - if !g.netState.IsOnline() { + if g.netState != nil && !g.netState.IsOnline() { continue } switch g.isConnectedOnAllWay() { diff --git a/client/internal/peer/guard/guard_netstate_test.go b/client/internal/peer/guard/guard_netstate_test.go index 2ab736428..44999cae1 100644 --- a/client/internal/peer/guard/guard_netstate_test.go +++ b/client/internal/peer/guard/guard_netstate_test.go @@ -9,7 +9,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/netbirdio/netbird/client/internal/peer/ice" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents/netstate" ) // newTestGuardWithNetState builds a guard with a realistic MaxInterval: the diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index f92f085ab..8373e498a 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -22,8 +22,7 @@ import ( "github.com/netbirdio/netbird/client/internal/listener" "github.com/netbirdio/netbird/client/internal/peer" "github.com/netbirdio/netbird/client/internal/profilemanager" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents" "github.com/netbirdio/netbird/client/system" "github.com/netbirdio/netbird/formatter" "github.com/netbirdio/netbird/route" @@ -84,12 +83,10 @@ type Client struct { onHostDnsFn func([]string) dnsManager dns.IosDnsManager loginComplete bool - // netState outlives engine restarts: it mirrors the OS connectivity, not - // the engine lifecycle. Run injects it into each new ConnectClient, which - // distributes it to every reconnection loop. - netState *netstate.State - // sweeper also outlives engine restarts; NotifyNetworkChange sweeps it. - sweeper *netsweep.Sweeper + // netMgr outlives engine restarts: it mirrors the OS connectivity, not + // the engine lifecycle. Run injects its state and sweeper into each new + // ConnectClient. + netMgr *netevents.Manager // preloadedConfig holds config loaded from JSON (used on tvOS where file writes are blocked) preloadedConfig *profilemanager.Config @@ -100,6 +97,7 @@ type Client struct { // NewClient instantiate a new Client func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osVersion string, osName string, networkChangeListener NetworkChangeListener, dnsManager DnsManager) *Client { + recorder := peer.NewRecorder("") return &Client{ cfgFile: cfgFile, stateFile: stateFile, @@ -108,12 +106,11 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV deviceName: deviceName, osName: osName, osVersion: osVersion, - recorder: peer.NewRecorder(""), + recorder: recorder, ctxCancelLock: &sync.Mutex{}, networkChangeListener: networkChangeListener, dnsManager: dnsManager, - netState: netstate.New(), - sweeper: netsweep.New(), + netMgr: netevents.NewManager(recorder), } } @@ -190,7 +187,7 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error { cfg.WgIface = interfaceName connectClient := internal.NewConnectClient(ctx, cfg, c.recorder, - internal.WithNetworkState(c.netState), internal.WithSweeper(c.sweeper)) + internal.WithNetEvents(c.netMgr)) c.setState(cfg, connectClient) // Persist the latest sync response so DebugBundle can include the network // map. On iOS this is backed by disk to keep it out of the constrained @@ -203,10 +200,11 @@ func (c *Client) Run(fd int32, interfaceName string, envList *EnvList) error { // (e.g. from NWPathMonitor). While unavailable, the internal reconnect loops // suspend their attempts and the connection listener reports NoNetwork // instead of Connecting; when availability returns, the loops resume -// immediately with a fresh backoff. +// immediately with a fresh backoff. Losing the last network also sweeps the +// registered connections, so the client does not keep reporting Connected +// over stale sockets with no network at all. func (c *Client) SetNetworkAvailable(available bool) { - c.netState.Set(available) - c.recorder.SetNetworkAvailable(available) + c.netMgr.SetNetworkAvailable(available) } // NotifyNetworkChange marks the management, signal and relay connections @@ -214,8 +212,7 @@ func (c *Client) SetNetworkAvailable(available bool) { // whatever has not redialed on the new network by then. The engine and the // TUN device stay untouched. func (c *Client) NotifyNetworkChange() { - c.sweeper.MarkNetworkChange() - log.Infof("network change: connections marked stale") + c.netMgr.NotifyNetworkChange() } // Stop the internal client and free the resources diff --git a/client/netevents/netevents.go b/client/netevents/netevents.go new file mode 100644 index 000000000..f5b00a67a --- /dev/null +++ b/client/netevents/netevents.go @@ -0,0 +1,158 @@ +// Package netevents owns the OS network event handling shared by the mobile +// bindings: availability changes park or wake the reconnection loops and drive +// the NoNetwork listener state, and both losing the last network and switching +// networks sweep the stale connections so their owners redial immediately. +package netevents + +import ( + "context" + "time" + + "github.com/cenkalti/backoff/v4" + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/client/netevents/netstate" + "github.com/netbirdio/netbird/client/netevents/sweep" +) + +// Recorder receives the availability changes for listener state reporting. +type Recorder interface { + SetNetworkAvailable(available bool) +} + +// Manager ties the network availability state, the connection sweeper and the +// status recorder together; it outlives engine restarts. A nil *Manager is +// the valid no-events value: the read methods report always-online and never +// sweep. +type Manager struct { + netState *netstate.State + sweeper *sweep.Sweeper + recorder Recorder +} + +// NewManager creates a Manager reporting into recorder, starting online. +func NewManager(recorder Recorder) *Manager { + return &Manager{ + netState: netstate.New(), + sweeper: sweep.New(), + recorder: recorder, + } +} + +// SetNetworkAvailable records OS-reported network availability. While +// unavailable, the reconnection loops suspend their attempts and the +// connection listener reports NoNetwork instead of Connecting; when +// availability returns, the loops resume immediately with a fresh backoff. +// Losing the last network also sweeps the registered connections: nothing can +// redial while offline, so the stale sockets would otherwise stay silently +// "connected" until their own timeouts and the client would keep reporting +// Connected with no network at all. +func (m *Manager) SetNetworkAvailable(available bool) { + if !available && m.netState.IsOnline() { + m.sweeper.MarkNetworkChange() + } + m.netState.Set(available) + m.recorder.SetNetworkAvailable(available) +} + +// NotifyNetworkChange marks the management, signal and relay connections +// stale after the OS switched networks and schedules a sweep that cuts +// whatever has not redialed on the new network by then. The engine and the +// TUN device stay untouched. +func (m *Manager) NotifyNetworkChange() { + m.sweeper.MarkNetworkChange() + log.Infof("network change: connections marked stale") +} + +// IsOnline reports whether the OS reports at least one usable network. +func (m *Manager) IsOnline() bool { + if m == nil { + return true + } + return m.netState.IsOnline() +} + +// Changed returns a channel closed on the next availability transition. +func (m *Manager) Changed() <-chan struct{} { + if m == nil { + return nil + } + return m.netState.Changed() +} + +// Wait blocks while the network is offline; see netstate.State.Wait. +func (m *Manager) Wait(ctx context.Context) (bool, error) { + if m == nil { + return false, nil + } + return m.netState.Wait(ctx) +} + +// WaitSettled waits until an online verdict holds for a full settleWindow, or +// while offline until the budget runs out. Returns false when ctx is +// cancelled. The settle window exists because a disconnect often precedes the +// OS offline flag by a few milliseconds, so a fresh online verdict cannot be +// trusted immediately. A nil Manager has no events to watch: it degrades to a +// fixed budget-long sleep. +func (m *Manager) WaitSettled(ctx context.Context, budget, settleWindow time.Duration) bool { + if m == nil { + select { + case <-time.After(budget): + return true + case <-ctx.Done(): + return false + } + } + + budgetTimer := time.NewTimer(budget) + defer budgetTimer.Stop() + + settle := time.NewTimer(settleWindow) + defer settle.Stop() + + for { + // Channel first, flag second: a flip in between still fires the channel. + changedCh := m.netState.Changed() + if m.netState.IsOnline() { + select { + case <-settle.C: + return true + case <-changedCh: + case <-ctx.Done(): + return false + } + } else { + select { + case <-budgetTimer.C: + return true + case <-changedCh: + case <-ctx.Done(): + return false + } + } + if !settle.Stop() { + select { + case <-settle.C: + default: + } + } + settle.Reset(settleWindow) + } +} + +// StartDial registers an in-flight dial with the sweeper; see sweep.Sweeper.StartDial. +func (m *Manager) StartDial(ctx context.Context) *sweep.Dial { + if m == nil { + return (*sweep.Sweeper)(nil).StartDial(ctx) + } + return m.sweeper.StartDial(ctx) +} + +// QuickRetryBackoff wraps bo for a quick retry after a network change; see +// sweep.Sweeper.QuickRetryBackoff. +func (m *Manager) QuickRetryBackoff(ctx context.Context, bo backoff.BackOff) backoff.BackOff { + if m == nil { + return bo + } + return m.sweeper.QuickRetryBackoff(ctx, bo, m.netState) +} diff --git a/client/netevents/netevents_test.go b/client/netevents/netevents_test.go new file mode 100644 index 000000000..a62ddc270 --- /dev/null +++ b/client/netevents/netevents_test.go @@ -0,0 +1,34 @@ +package netevents + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +type recorderStub struct{} + +func (recorderStub) SetNetworkAvailable(bool) {} + +func TestWaitSettledAfterOutage(t *testing.T) { + const budget = 1500 * time.Millisecond + const settleWindow = 200 * time.Millisecond + const outage = 2 * settleWindow + + m := NewManager(recorderStub{}) + m.SetNetworkAvailable(false) + + start := time.Now() + go func() { + time.Sleep(outage) + m.SetNetworkAvailable(true) + }() + + ok := m.WaitSettled(context.Background(), budget, settleWindow) + elapsed := time.Since(start) + + assert.True(t, ok, "recovered network must let the caller proceed") + assert.GreaterOrEqual(t, elapsed, outage+settleWindow, "an online verdict must hold a full settle window before it is trusted") +} diff --git a/client/netstate/netstate.go b/client/netevents/netstate/netstate.go similarity index 100% rename from client/netstate/netstate.go rename to client/netevents/netstate/netstate.go diff --git a/client/netstate/netstate_test.go b/client/netevents/netstate/netstate_test.go similarity index 100% rename from client/netstate/netstate_test.go rename to client/netevents/netstate/netstate_test.go diff --git a/client/netsweep/quick_retry.go b/client/netevents/sweep/quick_retry.go similarity index 90% rename from client/netsweep/quick_retry.go rename to client/netevents/sweep/quick_retry.go index 524a5c50c..1e174b20a 100644 --- a/client/netsweep/quick_retry.go +++ b/client/netevents/sweep/quick_retry.go @@ -1,11 +1,11 @@ -package netsweep +package sweep import ( "time" "github.com/cenkalti/backoff/v4" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents/netstate" ) const quickRetryDelay = 200 * time.Millisecond diff --git a/client/netsweep/quick_retry_test.go b/client/netevents/sweep/quick_retry_test.go similarity index 99% rename from client/netsweep/quick_retry_test.go rename to client/netevents/sweep/quick_retry_test.go index 5505862c5..3dadd951c 100644 --- a/client/netsweep/quick_retry_test.go +++ b/client/netevents/sweep/quick_retry_test.go @@ -1,4 +1,4 @@ -package netsweep +package sweep import ( "context" diff --git a/client/netsweep/netsweep.go b/client/netevents/sweep/sweep.go similarity index 96% rename from client/netsweep/netsweep.go rename to client/netevents/sweep/sweep.go index 46bc0a709..52dce92be 100644 --- a/client/netsweep/netsweep.go +++ b/client/netevents/sweep/sweep.go @@ -1,10 +1,10 @@ -// Package netsweep cuts network-bound activity when the OS switches networks: +// Package sweep cuts network-bound activity when the OS switches networks: // a sweep closes the registered connections and aborts the in-flight dials, so // their owners redial immediately instead of waiting for the old sockets to // time out. // // A nil *Sweeper disables everything: all methods are nil-safe no-ops. -package netsweep +package sweep import ( "context" @@ -16,7 +16,7 @@ import ( "github.com/cenkalti/backoff/v4" log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/client/netstate" + "github.com/netbirdio/netbird/client/netevents/netstate" ) // DefaultSweepDelay absorbs network flapping while the OS settles on a @@ -34,7 +34,7 @@ type Config struct { // ErrSwept reports that a dial finished after a network change swept its // registration. The connection is already closed; the caller must treat it // as a failed dial and redial on the new network. -var ErrSwept = errors.New("netsweep: connection swept by network change") +var ErrSwept = errors.New("sweep: connection swept by network change") // sweepID identifies one registration in a sweeper. Connections and dials // draw from the same counter, so an id is unique across both registries. diff --git a/client/netsweep/netsweep_test.go b/client/netevents/sweep/sweep_test.go similarity index 99% rename from client/netsweep/netsweep_test.go rename to client/netevents/sweep/sweep_test.go index 88d660c2d..c162d4c0f 100644 --- a/client/netsweep/netsweep_test.go +++ b/client/netevents/sweep/sweep_test.go @@ -1,4 +1,4 @@ -package netsweep +package sweep import ( "context" diff --git a/shared/management/client/grpc.go b/shared/management/client/grpc.go index cd250b5f7..7ce79b418 100644 --- a/shared/management/client/grpc.go +++ b/shared/management/client/grpc.go @@ -21,8 +21,7 @@ import ( "google.golang.org/grpc/connectivity" nbgrpc "github.com/netbirdio/netbird/client/grpc" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents" "github.com/netbirdio/netbird/client/system" "github.com/netbirdio/netbird/encryption" "github.com/netbirdio/netbird/shared/management/domain" @@ -64,12 +63,9 @@ type GrpcClient struct { connStateCallbackLock sync.RWMutex serverURL string - // netState gates the stream retry loop on OS-reported network - // availability; nil (the default) disables gating. - netState *netstate.State - - // sweeper cuts the transport connections on network change; nil disables it. - sweeper *netsweep.Sweeper + // netEvents gates the stream retry loop on OS-reported network + // availability and sweeps the transport on network change. + netEvents *netevents.Manager // syncStreamErr holds the last Sync stream error, or nil while the stream // is established and healthy. GetServerKey succeeds even when the peer @@ -123,15 +119,9 @@ func MaxRecvMsgSize() int { // Option configures optional GrpcClient behavior. type Option func(*GrpcClient) -// WithNetworkState injects the OS network availability state that gates the -// stream retry loop; without it gating is disabled. -func WithNetworkState(netState *netstate.State) Option { - return func(c *GrpcClient) { c.netState = netState } -} - -// WithSweeper injects the network change sweeper. -func WithSweeper(sweeper *netsweep.Sweeper) Option { - return func(c *GrpcClient) { c.sweeper = sweeper } +// WithNetEvents injects the OS network event handling. +func WithNetEvents(events *netevents.Manager) Option { + return func(c *GrpcClient) { c.netEvents = events } } // NewClient creates a new client to Management service @@ -152,9 +142,7 @@ func NewClient(ctx context.Context, addr string, ourPrivateKey wgtypes.Key, tlsE extraOpts = append(extraOpts, grpc.WithDefaultCallOptions(grpc.MaxCallRecvMsgSize(maxSize))) log.Infof("management gRPC max receive message size set to %d bytes", maxSize) } - if c.sweeper != nil { - extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.sweeper)) - } + extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netEvents)) var conn *grpc.ClientConn operation := func() error { @@ -235,12 +223,12 @@ func (c *GrpcClient) withMgmtStream( ctx context.Context, handler func(ctx context.Context, serverPubKey wgtypes.Key, backOff backoff.BackOff) error, ) error { - backOff := c.sweeper.QuickRetryBackoff(ctx, defaultBackoff(ctx), c.netState) + backOff := c.netEvents.QuickRetryBackoff(ctx, defaultBackoff(ctx)) operation := func() error { // suspend reconnect attempts while the OS reports no usable network. // Wait only errors on a cancelled context, which means shutdown, so // stop the loop without reporting a failure. - if waited, err := c.netState.Wait(ctx); err != nil { + if waited, err := c.netEvents.Wait(ctx); err != nil { log.Debugf("management connection context has been canceled while offline, this usually indicates shutdown") return nil //nolint:nilerr // a cancelled context means shutdown, not a retryable failure } else if waited { @@ -273,7 +261,7 @@ func (c *GrpcClient) withMgmtStream( return handler(ctx, *serverPubKey, backOff) } - err := nbgrpc.Retry(ctx, operation, backOff, c.netState) + err := nbgrpc.Retry(ctx, operation, backOff, c.netEvents) if err != nil { log.Warnf("exiting the Management service connection retry loop due to the unrecoverable error: %s", err) } diff --git a/shared/relay/client/client.go b/shared/relay/client/client.go index 4fb30b8d9..38c9c7375 100644 --- a/shared/relay/client/client.go +++ b/shared/relay/client/client.go @@ -14,7 +14,7 @@ import ( log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents/sweep" auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" "github.com/netbirdio/netbird/shared/relay/client/dialer" netErr "github.com/netbirdio/netbird/shared/relay/client/dialer/net" @@ -151,6 +151,14 @@ type transportConn interface { Protocol() string } +// NetEvents is the OS network event view the relay consumes: availability +// gating for the reconnect guard and dial registration for the network change +// sweep. +type NetEvents interface { + NetworkWatcher + StartDial(ctx context.Context) *sweep.Dial +} + // Client is a client for the relay server. It is responsible for establishing a connection to the relay server and // managing connections to other peers. All exported functions are safe to call concurrently. After close the connection, // the client can be reused by calling Connect again. When the client is closed, all connections are closed too. @@ -186,9 +194,10 @@ type Client struct { // the manager. transportFallback *transportFallback - // sweeper cuts the relay connection on network change; the read loop - // reports the disconnect and the guard reconnects. Shared via the manager. - sweeper *netsweep.Sweeper + // netEvents registers the relay dial for the network change sweep; the + // read loop reports the disconnect and the guard reconnects. Shared via + // the manager. + netEvents NetEvents // datagramFallbackTriggered guards a single fallback per connection so a // burst of oversized datagrams triggers one reconnect, not many. datagramFallbackTriggered atomic.Bool @@ -400,7 +409,12 @@ func (c *Client) Close() error { func (c *Client) connect(ctx context.Context) (*RelayAddr, error) { // A sweep cancels this context, so a dial started on the old network // aborts instead of waiting out its handshake timeout. - dial := c.sweeper.StartDial(ctx) + var dial *sweep.Dial + if c.netEvents != nil { + dial = c.netEvents.StartDial(ctx) + } else { + dial = (*sweep.Sweeper)(nil).StartDial(ctx) + } defer dial.Release() ctx = dial.Ctx() diff --git a/shared/relay/client/guard.go b/shared/relay/client/guard.go index a62f8772d..c83939d6c 100644 --- a/shared/relay/client/guard.go +++ b/shared/relay/client/guard.go @@ -7,8 +7,6 @@ import ( "github.com/cenkalti/backoff/v4" log "github.com/sirupsen/logrus" - - "github.com/netbirdio/netbird/client/netstate" ) const ( @@ -24,6 +22,13 @@ const ( verdictSettleWindow = 200 * time.Millisecond ) +// NetworkWatcher is the availability view the guard gates reconnects on. +type NetworkWatcher interface { + Wait(ctx context.Context) (bool, error) + IsOnline() bool + WaitSettled(ctx context.Context, budget, settleWindow time.Duration) bool +} + // Guard manage the reconnection tries to the Relay server in case of disconnection event. type Guard struct { // OnNewRelayClient is a channel that is used to notify the relay manager about a new relay client instance. @@ -35,9 +40,8 @@ type Guard struct { // attempts. maxBackoffInterval time.Duration - // netState gates reconnect attempts on OS-reported network availability; - // nil disables gating. - netState *netstate.State + // netState gates reconnect attempts on OS-reported network availability. + netState NetworkWatcher // lastErr is the error from the most recent failed reconnect attempt, // surfaced as the home relay status while disconnected. @@ -45,9 +49,8 @@ type Guard struct { } // NewGuard creates a new guard for the relay client. A non-positive -// maxBackoffInterval falls back to defaultMaxBackoffInterval. A nil netState -// disables network availability gating. -func NewGuard(sp *ServerPicker, maxBackoffInterval time.Duration, netState *netstate.State) *Guard { +// maxBackoffInterval falls back to defaultMaxBackoffInterval. +func NewGuard(sp *ServerPicker, maxBackoffInterval time.Duration, netState NetworkWatcher) *Guard { if maxBackoffInterval <= 0 { maxBackoffInterval = defaultMaxBackoffInterval } @@ -97,12 +100,14 @@ func (g *Guard) StartReconnectTrys(ctx context.Context, relayClient *Client) { select { case <-ticker.C: // suspend reconnect attempts while the OS reports no usable network - if waited, err := g.netState.Wait(ctx); err != nil { - return - } else if waited { - ticker.Stop() - ticker = g.exponentTicker(ctx) - continue + if g.netState != nil { + if waited, err := g.netState.Wait(ctx); err != nil { + return + } else if waited { + ticker.Stop() + ticker = g.exponentTicker(ctx) + continue + } } if err := g.retry(ctx); err != nil { log.Errorf("failed to pick new Relay server: %s", err) @@ -129,13 +134,18 @@ func (g *Guard) tryToQuickReconnect(parentCtx context.Context, rc *Client) bool return false } - if ok := g.waitForNetwork(parentCtx); !ok { - return false - } - - // Still offline after the budget: leave the retry to the ticker. - if !g.netState.IsOnline() { - return false + if g.netState != nil { + if ok := g.netState.WaitSettled(parentCtx, quickReconnectBudget, verdictSettleWindow); !ok { + return false + } + // Still offline after the budget: leave the retry to the ticker. + if !g.netState.IsOnline() { + return false + } + } else { + if cancelled := waiteBeforeRetry(parentCtx); !cancelled { + return false + } } log.Infof("try to reconnect to Relay server: %s", rc.connectionURL) @@ -200,47 +210,14 @@ func (g *Guard) exponentTicker(ctx context.Context) *backoff.Ticker { return backoff.NewTicker(bo) } -// waitForNetwork waits out the settle window while online, or waits for the -// network to return while offline, within the budget. Returns false when ctx -// is cancelled. Without an injected netState it degrades to a fixed -// budget-long sleep, the pre-netstate behavior. -func (g *Guard) waitForNetwork(ctx context.Context) bool { - budget := time.NewTimer(quickReconnectBudget) - defer budget.Stop() +func waiteBeforeRetry(ctx context.Context) bool { + timer := time.NewTimer(quickReconnectBudget) + defer timer.Stop() - settleWindow := verdictSettleWindow - if g.netState == nil { - settleWindow = quickReconnectBudget - } - settle := time.NewTimer(settleWindow) - defer settle.Stop() - - for { - // Channel first, flag second: a flip in between still fires the channel. - changedCh := g.netState.Changed() - if g.netState.IsOnline() { - select { - case <-settle.C: - return true - case <-changedCh: - case <-ctx.Done(): - return false - } - } else { - select { - case <-budget.C: - return true - case <-changedCh: - case <-ctx.Done(): - return false - } - } - if !settle.Stop() { - select { - case <-settle.C: - default: - } - } - settle.Reset(settleWindow) + select { + case <-timer.C: + return true + case <-ctx.Done(): + return false } } diff --git a/shared/relay/client/guard_test.go b/shared/relay/client/guard_test.go deleted file mode 100644 index 0e05783e0..000000000 --- a/shared/relay/client/guard_test.go +++ /dev/null @@ -1,30 +0,0 @@ -package client - -import ( - "context" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/netbirdio/netbird/client/netstate" -) - -func TestWaitForNetworkSettlesAfterOutage(t *testing.T) { - ns := netstate.New() - ns.Set(false) - g := NewGuard(nil, 0, ns) - - const outage = 2 * verdictSettleWindow - start := time.Now() - go func() { - time.Sleep(outage) - ns.Set(true) - }() - - ok := g.waitForNetwork(context.Background()) - elapsed := time.Since(start) - - assert.True(t, ok, "recovered network must let the quick reconnect proceed") - assert.GreaterOrEqual(t, elapsed, outage+verdictSettleWindow, "reconnect must wait a full settle window after the network returns") -} diff --git a/shared/relay/client/manager.go b/shared/relay/client/manager.go index 80e38ae2d..50fcc0b8f 100644 --- a/shared/relay/client/manager.go +++ b/shared/relay/client/manager.go @@ -12,8 +12,6 @@ import ( log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" relayAuth "github.com/netbirdio/netbird/shared/relay/auth/hmac" ) @@ -67,15 +65,9 @@ func WithMaxBackoffInterval(d time.Duration) ManagerOption { return func(m *Manager) { m.maxBackoffInterval = d } } -// WithNetworkState injects the OS network availability state that gates the -// reconnect guard; without it reconnect attempts are not gated. -func WithNetworkState(netState *netstate.State) ManagerOption { - return func(m *Manager) { m.netState = netState } -} - -// WithSweeper injects the network change sweeper. -func WithSweeper(sweeper *netsweep.Sweeper) ManagerOption { - return func(m *Manager) { m.sweeper = sweeper } +// WithNetEvents injects the OS network event handling. +func WithNetEvents(events NetEvents) ManagerOption { + return func(m *Manager) { m.netEvents = events } } // Manager is a manager for the relay client instances. It establishes one persistent connection to the given relay URL @@ -105,8 +97,7 @@ type Manager struct { mtu uint16 maxBackoffInterval time.Duration - netState *netstate.State - sweeper *netsweep.Sweeper + netEvents NetEvents cleanupInterval time.Duration keepUnusedServerTime time.Duration @@ -143,9 +134,9 @@ func NewManager(ctx context.Context, serverURLs []string, peerID string, mtu uin for _, opt := range opts { opt(m) } - m.serverPicker.Sweeper = m.sweeper + m.serverPicker.NetEvents = m.netEvents m.serverPicker.ServerURLs.Store(serverURLs) - m.reconnectGuard = NewGuard(m.serverPicker, m.maxBackoffInterval, m.netState) + m.reconnectGuard = NewGuard(m.serverPicker, m.maxBackoffInterval, m.netEvents) return m } @@ -370,7 +361,7 @@ func (m *Manager) openConnVia(ctx context.Context, serverAddress, peerKey string relayClient := NewClientWithServerIP(serverAddress, serverIP, m.tokenStore, m.peerID, m.mtu) relayClient.SetTransportFallback(m.transportFallback) - relayClient.sweeper = m.sweeper + relayClient.netEvents = m.netEvents err := relayClient.Connect(m.ctx) if err != nil { rt.Lock() diff --git a/shared/relay/client/picker.go b/shared/relay/client/picker.go index 72789fadc..17b1390b1 100644 --- a/shared/relay/client/picker.go +++ b/shared/relay/client/picker.go @@ -9,7 +9,6 @@ import ( log "github.com/sirupsen/logrus" - "github.com/netbirdio/netbird/client/netsweep" auth "github.com/netbirdio/netbird/shared/relay/auth/hmac" ) @@ -31,7 +30,7 @@ type ServerPicker struct { MTU uint16 ConnectionTimeout time.Duration TransportFallback *transportFallback - Sweeper *netsweep.Sweeper + NetEvents NetEvents } func (sp *ServerPicker) PickServer(parentCtx context.Context) (*Client, error) { @@ -75,7 +74,7 @@ func (sp *ServerPicker) startConnection(ctx context.Context, resultChan chan con log.Infof("try to connecting to relay server: %s", url) relayClient := NewClient(url, sp.TokenStore, sp.PeerID, sp.MTU) relayClient.SetTransportFallback(sp.TransportFallback) - relayClient.sweeper = sp.Sweeper + relayClient.netEvents = sp.NetEvents err := relayClient.Connect(ctx) resultChan <- connResult{ RelayClient: relayClient, diff --git a/shared/signal/client/grpc.go b/shared/signal/client/grpc.go index 73c482e8f..0af4dc4da 100644 --- a/shared/signal/client/grpc.go +++ b/shared/signal/client/grpc.go @@ -19,8 +19,7 @@ import ( "google.golang.org/grpc/status" nbgrpc "github.com/netbirdio/netbird/client/grpc" - "github.com/netbirdio/netbird/client/netstate" - "github.com/netbirdio/netbird/client/netsweep" + "github.com/netbirdio/netbird/client/netevents" "github.com/netbirdio/netbird/encryption" "github.com/netbirdio/netbird/shared/management/client" "github.com/netbirdio/netbird/shared/signal/proto" @@ -67,12 +66,9 @@ type GrpcClient struct { connStateCallback ConnStateNotifier connStateCallbackLock sync.RWMutex - // netState gates the Receive retry loop on OS-reported network - // availability; nil (the default) disables gating. - netState *netstate.State - - // sweeper cuts the transport connections on network change; nil disables it. - sweeper *netsweep.Sweeper + // netEvents gates the Receive retry loop on OS-reported network + // availability and sweeps the transport on network change. + netEvents *netevents.Manager onReconnectedListenerFn func() @@ -100,15 +96,9 @@ type GrpcClient struct { // Option configures optional GrpcClient behavior. type Option func(*GrpcClient) -// WithNetworkState injects the OS network availability state that gates the -// Receive retry loop; without it gating is disabled. -func WithNetworkState(netState *netstate.State) Option { - return func(c *GrpcClient) { c.netState = netState } -} - -// WithSweeper injects the network change sweeper. -func WithSweeper(sweeper *netsweep.Sweeper) Option { - return func(c *GrpcClient) { c.sweeper = sweeper } +// WithNetEvents injects the OS network event handling. +func WithNetEvents(events *netevents.Manager) Option { + return func(c *GrpcClient) { c.netEvents = events } } // NewClient creates a new Signal client @@ -126,9 +116,7 @@ func NewClient(ctx context.Context, addr string, key wgtypes.Key, tlsEnabled boo } var extraOpts []grpc.DialOption - if c.sweeper != nil { - extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.sweeper)) - } + extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netEvents)) var conn *grpc.ClientConn operation := func() error { @@ -198,13 +186,13 @@ func defaultBackoff(ctx context.Context) backoff.BackOff { // The connection retry logic will try to reconnect for 30 min and if wasn't successful will propagate the error to the function caller. func (c *GrpcClient) Receive(ctx context.Context, msgHandler func(msg *proto.Message) error) error { - backOff := c.sweeper.QuickRetryBackoff(ctx, defaultBackoff(ctx), c.netState) + backOff := c.netEvents.QuickRetryBackoff(ctx, defaultBackoff(ctx)) operation := func() error { // suspend reconnect attempts while the OS reports no usable network. // Wait only errors on a cancelled context, which means shutdown, so // stop the loop without reporting a failure. - if waited, err := c.netState.Wait(ctx); err != nil { + if waited, err := c.netEvents.Wait(ctx); err != nil { log.Debugf("signal connection context has been canceled while offline, this usually indicates shutdown") return nil } else if waited { @@ -281,7 +269,7 @@ func (c *GrpcClient) Receive(ctx context.Context, msgHandler func(msg *proto.Mes return nil } - err := nbgrpc.Retry(ctx, operation, backOff, c.netState) + err := nbgrpc.Retry(ctx, operation, backOff, c.netEvents) if err != nil { log.Errorf("exiting the Signal service connection retry loop due to the unrecoverable error: %v", err) return err