[client] Name the netevents fields after their types

Review feedback on #7254: the fields kept their netstate-era names after the
type changed to *netevents.Manager or a NetworkWatcher interface.

Rename the Manager-typed fields to netMgr/NetMgr and the interface-typed guard
fields to netWatcher, document that the Manager write methods panic on a nil
receiver while the read methods stay nil-safe, and fix the waiteBeforeRetry
typo.

The relay client keeps its netEvents fields: those hold the NetEvents
interface, so the name already matches the type.
This commit is contained in:
Zoltán Papp
2026-08-25 15:09:00 +02:00
parent 1f1bccb674
commit 0a03211903
8 changed files with 64 additions and 57 deletions

View File

@@ -72,9 +72,9 @@ type ConnectClient struct {
persistSyncResponse bool
// netEvents gates every reconnection loop on OS-reported network
// netMgr gates every reconnection loop on OS-reported network
// availability and sweeps connections on network change.
netEvents *netevents.Manager
netMgr *netevents.Manager
}
// ConnectClientOption configures optional ConnectClient behavior.
@@ -82,7 +82,7 @@ type ConnectClientOption func(*ConnectClient)
// WithNetEvents injects the OS network event handling.
func WithNetEvents(events *netevents.Manager) ConnectClientOption {
return func(c *ConnectClient) { c.netEvents = events }
return func(c *ConnectClient) { c.netMgr = events }
}
func NewConnectClient(
@@ -293,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.netEvents.Wait(c.ctx); err != nil {
if waited, err := c.netMgr.Wait(c.ctx); err != nil {
return nil
} else if waited {
backOff.Reset()
@@ -311,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.WithNetEvents(c.netEvents))
mgm.WithNetEvents(c.netMgr))
if err != nil {
// On daemon shutdown / Down() the parent context is cancelled
// and the dial fails with "context canceled". Wrapping that
@@ -386,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.netEvents)
signalClient, err := connectToSignal(engineCtx, loginResp.GetNetbirdConfig(), myPrivateKey, c.netMgr)
if err != nil {
log.Error(err)
return wrapErr(err)
@@ -423,7 +423,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan
}
relayManager := relayClient.NewManager(engineCtx, relayURLs, myPrivateKey.PublicKey().String(), engineConfig.MTU,
relayClient.WithNetEvents(c.netEvents))
relayClient.WithNetEvents(c.netMgr))
c.statusRecorder.SetRelayMgr(relayManager)
if len(relayURLs) > 0 {
if token != nil {
@@ -451,7 +451,7 @@ func (c *ConnectClient) run(mobileDependency MobileDependency, runningChan chan
UpdateManager: c.updateManager,
ClientMetrics: c.clientMetrics,
MetricsCtx: c.ctx,
NetState: c.netEvents,
NetMgr: c.netMgr,
}, mobileDependency)
engine.SetSyncResponsePersistence(c.persistSyncResponse)
c.engine = engine
@@ -711,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, netEvents *netevents.Manager) (*signal.GrpcClient, error) {
func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourPrivateKey wgtypes.Key, netMgr *netevents.Manager) (*signal.GrpcClient, error) {
var sigTLSEnabled bool
if wtConfig.Signal.Protocol == mgmProto.HostConfig_HTTPS {
sigTLSEnabled = true
@@ -720,7 +720,7 @@ func connectToSignal(ctx context.Context, wtConfig *mgmProto.NetbirdConfig, ourP
}
signalClient, err := signal.NewClient(ctx, wtConfig.Signal.Uri, ourPrivateKey, sigTLSEnabled,
signal.WithNetEvents(netEvents))
signal.WithNetEvents(netMgr))
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)

View File

@@ -182,9 +182,9 @@ type EngineServices struct {
UpdateManager *updater.Manager
ClientMetrics *metrics.ClientMetrics
MetricsCtx context.Context
// NetState gates the reconnection loops on OS-reported network
// NetMgr gates the reconnection loops on OS-reported network
// availability; nil disables gating.
NetState *netevents.Manager
NetMgr *netevents.Manager
}
// Engine is a mechanism responsible for reacting on Signal and Management stream events and managing connections to the remote peers.
@@ -208,9 +208,9 @@ type Engine struct {
config *EngineConfig
mobileDep MobileDependency
// netState gates the peer reconnection guards on OS-reported network
// netMgr gates the peer reconnection guards on OS-reported network
// availability; nil disables gating.
netState *netevents.Manager
netMgr *netevents.Manager
// STUNs is a list of STUN servers used by ICE
STUNs []*stun.URI
@@ -345,7 +345,7 @@ func NewEngine(
syncMsgMux: &sync.Mutex{},
config: config,
mobileDep: mobileDep,
netState: services.NetState,
netMgr: services.NetMgr,
STUNs: []*stun.URI{},
TURNs: []*stun.URI{},
networkSerial: 0,
@@ -1902,8 +1902,8 @@ func (e *Engine) createPeerConn(pubKey string, allowedIPs []netip.Prefix, agentV
Addr: e.getRosenpassAddr(),
PermissiveMode: e.config.RosenpassPermissive,
},
ICEConfig: e.createICEConfig(),
NetworkState: e.netState,
ICEConfig: e.createICEConfig(),
NetMgr: e.netMgr,
}
serviceDependencies := peer.ServiceDependencies{

View File

@@ -95,9 +95,9 @@ type ConnConfig struct {
// ICEConfig ICE protocol configuration
ICEConfig icemaker.Config
// NetworkState gates the reconnection guard on OS-reported network
// NetMgr gates the reconnection guard on OS-reported network
// availability; nil disables gating.
NetworkState *netevents.Manager
NetMgr *netevents.Manager
}
type Conn struct {
@@ -259,7 +259,7 @@ func (conn *Conn) open(engineCtx context.Context, firstPacket []byte) error {
conn.handshaker.AddICEListener(conn.workerICE.OnNewOffer)
}
conn.guard = guard.NewGuard(conn.Log, conn.isConnectedOnAllWay, conn.config.Timeout, conn.srWatcher, conn.config.NetworkState)
conn.guard = guard.NewGuard(conn.Log, conn.isConnectedOnAllWay, conn.config.Timeout, conn.srWatcher, conn.config.NetMgr)
conn.wg.Add(1)
go func() {

View File

@@ -41,22 +41,22 @@ type Guard struct {
isConnectedOnAllWay connStatusFunc
timeout time.Duration
srWatcher *SRWatcher
// netState gates reconnect attempts on OS-reported network availability;
// netWatcher gates reconnect attempts on OS-reported network availability;
// nil disables gating.
netState NetworkWatcher
netWatcher NetworkWatcher
relayedConnDisconnected chan struct{}
iCEConnDisconnected chan struct{}
}
// NewGuard creates a reconnection guard for a peer connection. A nil netState
// NewGuard creates a reconnection guard for a peer connection. A nil netWatcher
// disables network availability gating.
func NewGuard(log *log.Entry, isConnectedFn connStatusFunc, timeout time.Duration, srWatcher *SRWatcher, netState NetworkWatcher) *Guard {
func NewGuard(log *log.Entry, isConnectedFn connStatusFunc, timeout time.Duration, srWatcher *SRWatcher, netWatcher NetworkWatcher) *Guard {
return &Guard{
log: log,
isConnectedOnAllWay: isConnectedFn,
timeout: timeout,
srWatcher: srWatcher,
netState: netState,
netWatcher: netWatcher,
relayedConnDisconnected: make(chan struct{}, 1),
iCEConnDisconnected: make(chan struct{}, 1),
}
@@ -109,8 +109,8 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
defer iceState.reset()
var netChanged <-chan struct{}
if g.netState != nil {
netChanged = g.netState.Changed()
if g.netWatcher != nil {
netChanged = g.netWatcher.Changed()
}
for {
@@ -118,7 +118,7 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
case <-tickerChannel:
// skip attempts while the OS reports no usable network; the
// netChanged case below resumes the loop once it returns
if g.netState != nil && !g.netState.IsOnline() {
if g.netWatcher != nil && !g.netWatcher.IsOnline() {
continue
}
switch g.isConnectedOnAllWay() {
@@ -159,8 +159,8 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
case <-netChanged:
// Re-arm for the next transition before acting on this one.
netChanged = g.netState.Changed()
if !g.netState.IsOnline() {
netChanged = g.netWatcher.Changed()
if !g.netWatcher.IsOnline() {
continue
}
// Ticks skipped while offline drove the backoff towards its

View File

@@ -23,8 +23,9 @@ type Recorder interface {
// 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.
// the valid no-events value for consumers: the read methods report
// always-online and never sweep. Only the event sources hold a real Manager,
// so the write methods do not tolerate a nil receiver.
type Manager struct {
// mu serializes availability transitions: the IsOnline check and the
// state update must be atomic, or a racing offline flip can skip the sweep
@@ -52,6 +53,9 @@ func NewManager(recorder Recorder) *Manager {
// 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.
//
// Panics on a nil receiver: only the mobile bindings that own a Manager
// report availability.
func (m *Manager) SetNetworkAvailable(available bool) {
m.mu.Lock()
defer m.mu.Unlock()
@@ -67,6 +71,9 @@ func (m *Manager) SetNetworkAvailable(available bool) {
// 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.
//
// Panics on a nil receiver: only the mobile bindings that own a Manager
// report network changes.
func (m *Manager) NotifyNetworkChange() {
m.sweeper.MarkNetworkChange()
log.Infof("network change: connections marked stale")

View File

@@ -63,9 +63,9 @@ type GrpcClient struct {
connStateCallbackLock sync.RWMutex
serverURL string
// netEvents gates the stream retry loop on OS-reported network
// netMgr gates the stream retry loop on OS-reported network
// availability and sweeps the transport on network change.
netEvents *netevents.Manager
netMgr *netevents.Manager
// syncStreamErr holds the last Sync stream error, or nil while the stream
// is established and healthy. GetServerKey succeeds even when the peer
@@ -121,7 +121,7 @@ type Option func(*GrpcClient)
// WithNetEvents injects the OS network event handling.
func WithNetEvents(events *netevents.Manager) Option {
return func(c *GrpcClient) { c.netEvents = events }
return func(c *GrpcClient) { c.netMgr = events }
}
// NewClient creates a new client to Management service
@@ -142,7 +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)
}
extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netEvents))
extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netMgr))
var conn *grpc.ClientConn
operation := func() error {
@@ -223,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.netEvents.QuickRetryBackoff(ctx, defaultBackoff(ctx))
backOff := c.netMgr.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.netEvents.Wait(ctx); err != nil {
if waited, err := c.netMgr.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 {
@@ -264,7 +264,7 @@ func (c *GrpcClient) withMgmtStream(
return handler(ctx, *serverPubKey, backOff)
}
err := nbgrpc.Retry(ctx, operation, backOff, c.netEvents)
err := nbgrpc.Retry(ctx, operation, backOff, c.netMgr)
if err != nil {
log.Warnf("exiting the Management service connection retry loop due to the unrecoverable error: %s", err)
}

View File

@@ -40,8 +40,8 @@ type Guard struct {
// attempts.
maxBackoffInterval time.Duration
// netState gates reconnect attempts on OS-reported network availability.
netState NetworkWatcher
// netWatcher gates reconnect attempts on OS-reported network availability.
netWatcher NetworkWatcher
// lastErr is the error from the most recent failed reconnect attempt,
// surfaced as the home relay status while disconnected.
@@ -50,7 +50,7 @@ type Guard struct {
// NewGuard creates a new guard for the relay client. A non-positive
// maxBackoffInterval falls back to defaultMaxBackoffInterval.
func NewGuard(sp *ServerPicker, maxBackoffInterval time.Duration, netState NetworkWatcher) *Guard {
func NewGuard(sp *ServerPicker, maxBackoffInterval time.Duration, netWatcher NetworkWatcher) *Guard {
if maxBackoffInterval <= 0 {
maxBackoffInterval = defaultMaxBackoffInterval
}
@@ -59,7 +59,7 @@ func NewGuard(sp *ServerPicker, maxBackoffInterval time.Duration, netState Netwo
OnReconnected: make(chan struct{}, 1),
serverPicker: sp,
maxBackoffInterval: maxBackoffInterval,
netState: netState,
netWatcher: netWatcher,
}
return g
}
@@ -100,8 +100,8 @@ func (g *Guard) StartReconnectTrys(ctx context.Context, relayClient *Client) {
select {
case <-ticker.C:
// suspend reconnect attempts while the OS reports no usable network
if g.netState != nil {
if waited, err := g.netState.Wait(ctx); err != nil {
if g.netWatcher != nil {
if waited, err := g.netWatcher.Wait(ctx); err != nil {
return
} else if waited {
ticker.Stop()
@@ -134,16 +134,16 @@ func (g *Guard) tryToQuickReconnect(parentCtx context.Context, rc *Client) bool
return false
}
if g.netState != nil {
if ok := g.netState.WaitSettled(parentCtx, quickReconnectBudget, verdictSettleWindow); !ok {
if g.netWatcher != nil {
if ok := g.netWatcher.WaitSettled(parentCtx, quickReconnectBudget, verdictSettleWindow); !ok {
return false
}
// Still offline after the budget: leave the retry to the ticker.
if !g.netState.IsOnline() {
if !g.netWatcher.IsOnline() {
return false
}
} else {
if cancelled := waiteBeforeRetry(parentCtx); !cancelled {
if cancelled := waitBeforeRetry(parentCtx); !cancelled {
return false
}
}
@@ -210,7 +210,7 @@ func (g *Guard) exponentTicker(ctx context.Context) *backoff.Ticker {
return backoff.NewTicker(bo)
}
func waiteBeforeRetry(ctx context.Context) bool {
func waitBeforeRetry(ctx context.Context) bool {
timer := time.NewTimer(quickReconnectBudget)
defer timer.Stop()

View File

@@ -66,9 +66,9 @@ type GrpcClient struct {
connStateCallback ConnStateNotifier
connStateCallbackLock sync.RWMutex
// netEvents gates the Receive retry loop on OS-reported network
// netMgr gates the Receive retry loop on OS-reported network
// availability and sweeps the transport on network change.
netEvents *netevents.Manager
netMgr *netevents.Manager
onReconnectedListenerFn func()
@@ -98,7 +98,7 @@ type Option func(*GrpcClient)
// WithNetEvents injects the OS network event handling.
func WithNetEvents(events *netevents.Manager) Option {
return func(c *GrpcClient) { c.netEvents = events }
return func(c *GrpcClient) { c.netMgr = events }
}
// NewClient creates a new Signal client
@@ -116,7 +116,7 @@ func NewClient(ctx context.Context, addr string, key wgtypes.Key, tlsEnabled boo
}
var extraOpts []grpc.DialOption
extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netEvents))
extraOpts = append(extraOpts, nbgrpc.WithSweeper(c.netMgr))
var conn *grpc.ClientConn
operation := func() error {
@@ -186,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.netEvents.QuickRetryBackoff(ctx, defaultBackoff(ctx))
backOff := c.netMgr.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.netEvents.Wait(ctx); err != nil {
if waited, err := c.netMgr.Wait(ctx); err != nil {
log.Debugf("signal connection context has been canceled while offline, this usually indicates shutdown")
return nil
} else if waited {
@@ -272,7 +272,7 @@ func (c *GrpcClient) Receive(ctx context.Context, msgHandler func(msg *proto.Mes
return nil
}
err := nbgrpc.Retry(ctx, operation, backOff, c.netEvents)
err := nbgrpc.Retry(ctx, operation, backOff, c.netMgr)
if err != nil {
log.Errorf("exiting the Signal service connection retry loop due to the unrecoverable error: %v", err)
return err