mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-08 14:39:09 +02:00
* [client] Pass the home relay client into isForeignServer isForeignServer read m.relayClient directly, so it was only safe while the caller held relayClientMu, and that dependency was invisible at the call site. Taking the client as a parameter makes the requirement explicit and lets a caller narrow the window it holds the lock for. Pure refactor, no behavior change: the single caller passes m.relayClient while still holding the read lock. * [client] Release the relay client lock before dialing a foreign server OpenConn held relayClientMu for its whole body, dial included. Dialing a foreign relay takes as long as that server's connect timeout, and onServerDisconnected needs the same mutex in write mode; a pending writer also blocks later readers, so a single unreachable relay stalled every relay operation on the node for the duration of the dial. On a routing peer this stalled the disconnect handling of an unrelated relay for a full dial timeout. By the time the eviction ran, the track had already been rebuilt, so it dropped a working client and the next OpenConn built a duplicate connection to a server the peer was already connected to. Read the home client under the lock, then release it and dial unlocked. * [client] Keep a reconnected foreign relay client on a late disconnect notice evictForeignRelay deleted the track by server address without checking which client the entry held. The disconnect notice is delivered on its own goroutine and carries only the address, so it can arrive after the track has been rebuilt: the eviction then dropped a live client, and the next OpenConn built a second connection to a server the peer was already connected to. The relay answers a duplicate peer ID by closing the existing connection, which tears down every relayed channel on it, including the peers whose active path it is. Skip the eviction when the stored client is still connected. * [client] Drop the comments added with the previous commits Comment-only change, no behavior change. * [client] Cover the foreign relay stall and the late disconnect notice Two regression tests, both failing on the unfixed code: TestOpenConn_DoesNotHoldRelayClientLockAcrossDial dials a listener that accepts and never answers, then calls onServerDisconnected and fails if it does not return. On the unfixed code the writer waits out the whole dial. TestEvictForeignRelay_KeepsConnectedClient opens a real connection to a second relay and then replays a disconnect notice for that address. On the unfixed code the live client is dropped from the map. Both reuse the existing stallingRelayListener and relay server helpers. * [client] Keep a foreign relay track whose dial is still in progress openConnVia publishes the track before dialing and fills relayClient only once the dial finishes, so the readiness guard did not cover the dial-in-progress window: a disconnect notice arriving there deleted a track whose client was about to connect. That client then became unreachable through the map, so cleanUpUnusedRelays could not close it either, and the next OpenConn dialed a duplicate the relay answers by closing the first - the churn this branch is meant to remove. Skip the eviction while relayClient is still nil, the same signal cleanUpUnusedRelays already uses to leave an in-flight dial alone. Reported by cubic on PR #8098.
514 lines
15 KiB
Go
514 lines
15 KiB
Go
package client
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"net/netip"
|
|
"sync"
|
|
"time"
|
|
|
|
log "github.com/sirupsen/logrus"
|
|
|
|
relayAuth "github.com/netbirdio/netbird/shared/relay/auth/hmac"
|
|
)
|
|
|
|
var (
|
|
relayCleanupInterval = 60 * time.Second
|
|
keepUnusedServerTime = 5 * time.Second
|
|
|
|
ErrRelayClientNotConnected = fmt.Errorf("relay client not connected")
|
|
)
|
|
|
|
// RelayTrack hold the relay clients for the foreign relay servers.
|
|
// With the mutex can ensure we can open new connection in case the relay connection has been established with
|
|
// the relay server.
|
|
type RelayTrack struct {
|
|
sync.RWMutex
|
|
relayClient *Client
|
|
err error
|
|
created time.Time
|
|
// ready is closed once the dial started by openConnVia finishes (relayClient
|
|
// or err is set). Callers reusing a track wait on this instead of the track
|
|
// lock, so the dial never runs under rt.Lock.
|
|
ready chan struct{}
|
|
}
|
|
|
|
func NewRelayTrack() *RelayTrack {
|
|
return &RelayTrack{
|
|
created: time.Now(),
|
|
ready: make(chan struct{}),
|
|
}
|
|
}
|
|
|
|
// ManagerOption configures a Manager at construction time.
|
|
type ManagerOption func(*Manager)
|
|
|
|
// RelayConnState is the connection state of a single relay server.
|
|
type RelayConnState struct {
|
|
// URL is the server's instance address when connected, otherwise the
|
|
// configured server URL.
|
|
URL string
|
|
// Transport is the negotiated transport, empty if not connected.
|
|
Transport string
|
|
// Err is set when the relay is not connected.
|
|
Err error
|
|
}
|
|
|
|
// WithMaxBackoffInterval caps the exponential backoff between reconnect
|
|
// attempts to the home relay. A non-positive value keeps the default.
|
|
func WithMaxBackoffInterval(d time.Duration) ManagerOption {
|
|
return func(m *Manager) { m.maxBackoffInterval = d }
|
|
}
|
|
|
|
// 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
|
|
// and automatically reconnect to them in case disconnection.
|
|
// The manager also manage temporary relay connection. If a client wants to communicate with a client on a
|
|
// different relay server, the manager will establish a new connection to the relay server. The connection with these
|
|
// relay servers will be closed if there is no active connection. Periodically the manager will check if there is any
|
|
// unused relay connection and close it.
|
|
type Manager struct {
|
|
ctx context.Context
|
|
peerID string
|
|
running bool
|
|
tokenStore *relayAuth.TokenStore
|
|
serverPicker *ServerPicker
|
|
|
|
relayClient *Client
|
|
// the guard logic can overwrite the relayClient variable, this mutex protect the usage of the variable
|
|
relayClientMu sync.RWMutex
|
|
reconnectGuard *Guard
|
|
|
|
relayClients map[string]*RelayTrack
|
|
relayClientsMutex sync.RWMutex
|
|
|
|
onReconnectedListenerFn func()
|
|
listenerLock sync.Mutex
|
|
|
|
mtu uint16
|
|
maxBackoffInterval time.Duration
|
|
netEvents NetEvents
|
|
|
|
cleanupInterval time.Duration
|
|
keepUnusedServerTime time.Duration
|
|
|
|
// transportFallback is shared across home and foreign relay clients so a
|
|
// datagram-too-large failure makes that server avoid datagram-sized transports across reconnects.
|
|
transportFallback *transportFallback
|
|
}
|
|
|
|
// NewManager creates a new manager instance.
|
|
// The serverURL address can be empty. In this case, the manager will not serve.
|
|
func NewManager(ctx context.Context, serverURLs []string, peerID string, mtu uint16, opts ...ManagerOption) *Manager {
|
|
tokenStore := &relayAuth.TokenStore{}
|
|
tf := newTransportFallback()
|
|
|
|
m := &Manager{
|
|
ctx: ctx,
|
|
peerID: peerID,
|
|
tokenStore: tokenStore,
|
|
mtu: mtu,
|
|
transportFallback: tf,
|
|
serverPicker: &ServerPicker{
|
|
TokenStore: tokenStore,
|
|
PeerID: peerID,
|
|
MTU: mtu,
|
|
ConnectionTimeout: defaultConnectionTimeout,
|
|
TransportFallback: tf,
|
|
},
|
|
relayClients: make(map[string]*RelayTrack),
|
|
cleanupInterval: relayCleanupInterval,
|
|
keepUnusedServerTime: keepUnusedServerTime,
|
|
}
|
|
for _, opt := range opts {
|
|
opt(m)
|
|
}
|
|
m.serverPicker.NetEvents = m.netEvents
|
|
m.serverPicker.ServerURLs.Store(serverURLs)
|
|
m.reconnectGuard = NewGuard(m.serverPicker, m.maxBackoffInterval, m.netEvents)
|
|
return m
|
|
}
|
|
|
|
// Serve starts the manager, attempting to establish a connection with the relay server.
|
|
// If the connection fails, it will keep trying to reconnect in the background.
|
|
// Additionally, it starts a cleanup loop to remove unused relay connections.
|
|
// The manager will automatically reconnect to the relay server in case of disconnection.
|
|
func (m *Manager) Serve() error {
|
|
if m.running {
|
|
return fmt.Errorf("manager already serving")
|
|
}
|
|
m.running = true
|
|
log.Debugf("starting relay client manager with %v relay servers", m.serverPicker.ServerURLs.Load())
|
|
|
|
client, err := m.serverPicker.PickServer(m.ctx)
|
|
if err != nil {
|
|
// record the initial failure so status shows the real reason before
|
|
// the guard's first retry tick
|
|
m.reconnectGuard.setLastError(err)
|
|
go m.reconnectGuard.StartReconnectTrys(m.ctx, nil)
|
|
} else {
|
|
m.storeClient(client)
|
|
}
|
|
|
|
go m.listenGuardEvent(m.ctx)
|
|
go m.startCleanupLoop()
|
|
return err
|
|
}
|
|
|
|
// OpenConn opens a connection to the given peer key. If the peer is on the same relay server, the connection will be
|
|
// established via the relay server. If the peer is on a different relay server, the manager will establish a new
|
|
// connection to the relay server. It returns the relayed connection to the remote peer.
|
|
//
|
|
// serverIP, when valid and serverAddress is foreign, is used as a dial target if the FQDN-based dial fails.
|
|
// Ignored for the local home-server path. TLS verification still uses the FQDN via SNI.
|
|
func (m *Manager) OpenConn(ctx context.Context, serverAddress, peerKey string, serverIP netip.Addr) (*Conn, error) {
|
|
m.relayClientMu.RLock()
|
|
homeClient := m.relayClient
|
|
m.relayClientMu.RUnlock()
|
|
|
|
if homeClient == nil {
|
|
return nil, ErrRelayClientNotConnected
|
|
}
|
|
|
|
foreign, err := m.isForeignServer(homeClient, serverAddress)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
var netConn *Conn
|
|
if !foreign {
|
|
log.Debugf("open peer connection via permanent server: %s", peerKey)
|
|
netConn, err = homeClient.OpenConn(ctx, peerKey)
|
|
} else {
|
|
log.Debugf("open peer connection via foreign server: %s", serverAddress)
|
|
netConn, err = m.openConnVia(ctx, serverAddress, peerKey, serverIP)
|
|
}
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
return netConn, err
|
|
}
|
|
|
|
// Ready returns true if the home Relay client is connected to the relay server.
|
|
func (m *Manager) Ready() bool {
|
|
m.relayClientMu.RLock()
|
|
defer m.relayClientMu.RUnlock()
|
|
|
|
if m.relayClient == nil {
|
|
return false
|
|
}
|
|
return m.relayClient.Ready()
|
|
}
|
|
|
|
func (m *Manager) SetOnReconnectedListener(f func()) {
|
|
m.listenerLock.Lock()
|
|
defer m.listenerLock.Unlock()
|
|
|
|
m.onReconnectedListenerFn = f
|
|
}
|
|
|
|
// RelayInstanceAddress returns the address and resolved IP of the permanent relay server. It could change if the
|
|
// network connection is lost. The address is sent to the target peer to choose the common relay server for the
|
|
// communication; the IP is sent alongside so remote peers can dial directly without their own DNS lookup. Both
|
|
// values are read under the same lock so they cannot diverge across a reconnection.
|
|
func (m *Manager) RelayInstanceAddress() (string, netip.Addr, error) {
|
|
m.relayClientMu.RLock()
|
|
defer m.relayClientMu.RUnlock()
|
|
|
|
if m.relayClient == nil {
|
|
return "", netip.Addr{}, ErrRelayClientNotConnected
|
|
}
|
|
return m.relayClient.serverInstanceAddress()
|
|
}
|
|
|
|
// ServerURLs returns the addresses of the relay servers.
|
|
func (m *Manager) ServerURLs() []string {
|
|
return m.serverPicker.ServerURLs.Load().([]string)
|
|
}
|
|
|
|
// RelayConnectError returns the error from the most recent failed home relay
|
|
// reconnect attempt, or nil if the relay last connected successfully.
|
|
func (m *Manager) RelayConnectError() error {
|
|
return m.reconnectGuard.LastError()
|
|
}
|
|
|
|
// RelayStates returns the connection state of the home relay and every foreign
|
|
// relay the manager currently tracks.
|
|
func (m *Manager) RelayStates() []RelayConnState {
|
|
var states []RelayConnState
|
|
|
|
m.relayClientMu.RLock()
|
|
home := m.relayClient
|
|
m.relayClientMu.RUnlock()
|
|
if home != nil {
|
|
st := relayConnState(home)
|
|
// The home relay reconnects through the guard, so the real failure
|
|
// reason lives there rather than on the (stale) client.
|
|
if st.Err != nil {
|
|
if gErr := m.reconnectGuard.LastError(); gErr != nil {
|
|
st.Err = gErr
|
|
}
|
|
}
|
|
states = append(states, st)
|
|
}
|
|
|
|
// Snapshot the tracks, then query each outside the map lock: a track can be
|
|
// held by an in-progress Connect, and blocking on it must not stall other
|
|
// relay operations.
|
|
m.relayClientsMutex.RLock()
|
|
tracks := make([]*RelayTrack, 0, len(m.relayClients))
|
|
for _, rt := range m.relayClients {
|
|
tracks = append(tracks, rt)
|
|
}
|
|
m.relayClientsMutex.RUnlock()
|
|
|
|
// Only connected foreign relays carry state; a failed connect is evicted
|
|
// immediately (openConnVia), so there is no error state to surface.
|
|
for _, rt := range tracks {
|
|
rt.RLock()
|
|
rc := rt.relayClient
|
|
rt.RUnlock()
|
|
if rc != nil {
|
|
states = append(states, relayConnState(rc))
|
|
}
|
|
}
|
|
|
|
return states
|
|
}
|
|
|
|
// HasRelayAddress returns true if the manager is serving. With this method can check if the peer can communicate with
|
|
// Relay service.
|
|
func (m *Manager) HasRelayAddress() bool {
|
|
return len(m.serverPicker.ServerURLs.Load().([]string)) > 0
|
|
}
|
|
|
|
func (m *Manager) UpdateServerURLs(serverURLs []string) {
|
|
log.Infof("update relay server URLs: %v", serverURLs)
|
|
m.serverPicker.ServerURLs.Store(serverURLs)
|
|
}
|
|
|
|
// UpdateToken updates the token in the token store.
|
|
func (m *Manager) UpdateToken(token *relayAuth.Token) error {
|
|
return m.tokenStore.UpdateToken(token)
|
|
}
|
|
|
|
func (m *Manager) openConnVia(ctx context.Context, serverAddress, peerKey string, serverIP netip.Addr) (*Conn, error) {
|
|
// check if already has a connection to the desired relay server
|
|
m.relayClientsMutex.RLock()
|
|
rt, ok := m.relayClients[serverAddress]
|
|
m.relayClientsMutex.RUnlock()
|
|
if ok {
|
|
return m.openConnOnTrack(ctx, rt, peerKey)
|
|
}
|
|
|
|
// if not, establish a new connection but check it again (because changed the lock type) before starting the
|
|
// connection
|
|
m.relayClientsMutex.Lock()
|
|
rt, ok = m.relayClients[serverAddress]
|
|
if ok {
|
|
m.relayClientsMutex.Unlock()
|
|
return m.openConnOnTrack(ctx, rt, peerKey)
|
|
}
|
|
|
|
// Publish the track and release the map lock BEFORE dialing, so the dial does
|
|
// not run under rt.Lock (which would block RelayStates and the cleanup loop
|
|
// for the full dial). Concurrent callers find this track and wait on rt.ready.
|
|
rt = NewRelayTrack()
|
|
m.relayClients[serverAddress] = rt
|
|
m.relayClientsMutex.Unlock()
|
|
|
|
relayClient := NewClientWithServerIP(serverAddress, serverIP, m.tokenStore, m.peerID, m.mtu)
|
|
relayClient.SetTransportFallback(m.transportFallback)
|
|
relayClient.netEvents = m.netEvents
|
|
err := relayClient.Connect(m.ctx)
|
|
if err != nil {
|
|
rt.Lock()
|
|
rt.err = err
|
|
rt.Unlock()
|
|
close(rt.ready)
|
|
m.relayClientsMutex.Lock()
|
|
delete(m.relayClients, serverAddress)
|
|
m.relayClientsMutex.Unlock()
|
|
return nil, err
|
|
}
|
|
// if connection closed then delete the relay client from the list
|
|
relayClient.SetOnDisconnectListener(m.onServerDisconnected)
|
|
rt.Lock()
|
|
rt.relayClient = relayClient
|
|
rt.Unlock()
|
|
close(rt.ready)
|
|
|
|
return relayClient.OpenConn(ctx, peerKey)
|
|
}
|
|
|
|
// openConnOnTrack opens a peer connection through an existing relay track,
|
|
// waiting for the dial started by another openConnVia call to finish. It waits
|
|
// on rt.ready rather than the track lock, so it neither holds nor contends the
|
|
// track lock across the dial.
|
|
func (m *Manager) openConnOnTrack(ctx context.Context, rt *RelayTrack, peerKey string) (*Conn, error) {
|
|
select {
|
|
case <-rt.ready:
|
|
case <-ctx.Done():
|
|
return nil, ctx.Err()
|
|
}
|
|
|
|
rt.RLock()
|
|
defer rt.RUnlock()
|
|
if rt.err != nil {
|
|
return nil, rt.err
|
|
}
|
|
if rt.relayClient == nil {
|
|
return nil, ErrRelayClientNotConnected
|
|
}
|
|
return rt.relayClient.OpenConn(ctx, peerKey)
|
|
}
|
|
|
|
func (m *Manager) onServerConnected() {
|
|
m.listenerLock.Lock()
|
|
defer m.listenerLock.Unlock()
|
|
|
|
if m.onReconnectedListenerFn == nil {
|
|
return
|
|
}
|
|
go m.onReconnectedListenerFn()
|
|
}
|
|
|
|
// onServerDisconnected handles relay disconnect events. For the home server it
|
|
// starts the reconnect guard. For foreign servers it evicts the now-dead client
|
|
// from the cache so the next OpenConn builds a fresh one instead of reusing a
|
|
// closed client.
|
|
func (m *Manager) onServerDisconnected(serverAddress string) {
|
|
m.relayClientMu.Lock()
|
|
isHome := m.relayClient != nil && serverAddress == m.relayClient.connectionURL
|
|
if isHome {
|
|
go func(client *Client) {
|
|
m.reconnectGuard.StartReconnectTrys(m.ctx, client)
|
|
}(m.relayClient)
|
|
}
|
|
m.relayClientMu.Unlock()
|
|
|
|
if !isHome {
|
|
m.evictForeignRelay(serverAddress)
|
|
}
|
|
}
|
|
|
|
func (m *Manager) evictForeignRelay(serverAddress string) {
|
|
m.relayClientsMutex.Lock()
|
|
defer m.relayClientsMutex.Unlock()
|
|
|
|
rt, ok := m.relayClients[serverAddress]
|
|
if !ok {
|
|
return
|
|
}
|
|
|
|
rt.RLock()
|
|
client := rt.relayClient
|
|
rt.RUnlock()
|
|
if client == nil {
|
|
log.Debugf("keeping foreign relay track with a dial in progress: %s", serverAddress)
|
|
return
|
|
}
|
|
if client.Ready() {
|
|
log.Debugf("keeping reconnected foreign relay client: %s", serverAddress)
|
|
return
|
|
}
|
|
|
|
delete(m.relayClients, serverAddress)
|
|
log.Debugf("evicted disconnected foreign relay client: %s", serverAddress)
|
|
}
|
|
|
|
func (m *Manager) listenGuardEvent(ctx context.Context) {
|
|
for {
|
|
select {
|
|
case <-m.reconnectGuard.OnReconnected:
|
|
m.onServerConnected()
|
|
case rc := <-m.reconnectGuard.OnNewRelayClient:
|
|
m.storeClient(rc)
|
|
m.onServerConnected()
|
|
case <-ctx.Done():
|
|
return
|
|
}
|
|
}
|
|
}
|
|
|
|
func (m *Manager) storeClient(client *Client) {
|
|
m.relayClientMu.Lock()
|
|
defer m.relayClientMu.Unlock()
|
|
|
|
m.relayClient = client
|
|
m.relayClient.SetOnDisconnectListener(m.onServerDisconnected)
|
|
}
|
|
|
|
func (m *Manager) isForeignServer(homeClient *Client, address string) (bool, error) {
|
|
rAddr, err := homeClient.ServerInstanceURL()
|
|
if err != nil {
|
|
return false, fmt.Errorf("relay client not connected")
|
|
}
|
|
return rAddr != address, nil
|
|
}
|
|
|
|
func (m *Manager) startCleanupLoop() {
|
|
ticker := time.NewTicker(m.cleanupInterval)
|
|
defer ticker.Stop()
|
|
for {
|
|
select {
|
|
case <-m.ctx.Done():
|
|
return
|
|
case <-ticker.C:
|
|
m.cleanUpUnusedRelays()
|
|
}
|
|
}
|
|
}
|
|
|
|
func (m *Manager) cleanUpUnusedRelays() {
|
|
m.relayClientsMutex.Lock()
|
|
defer m.relayClientsMutex.Unlock()
|
|
|
|
for addr, rt := range m.relayClients {
|
|
rt.Lock()
|
|
// if the connection failed to the server the relay client will be nil
|
|
// but the instance will be kept in the relayClients until the next locking
|
|
if rt.err != nil {
|
|
rt.Unlock()
|
|
continue
|
|
}
|
|
|
|
// dial still in progress (openConnVia publishes the track before Connect
|
|
// completes and no longer holds rt.Lock during it), nothing to clean up.
|
|
if rt.relayClient == nil {
|
|
rt.Unlock()
|
|
continue
|
|
}
|
|
|
|
if time.Since(rt.created) <= m.keepUnusedServerTime {
|
|
rt.Unlock()
|
|
continue
|
|
}
|
|
|
|
if rt.relayClient.HasConns() {
|
|
rt.Unlock()
|
|
continue
|
|
}
|
|
rt.relayClient.SetOnDisconnectListener(nil)
|
|
go func() {
|
|
_ = rt.relayClient.Close()
|
|
}()
|
|
log.Debugf("clean up unused relay server connection: %s", addr)
|
|
delete(m.relayClients, addr)
|
|
rt.Unlock()
|
|
}
|
|
}
|
|
|
|
func relayConnState(c *Client) RelayConnState {
|
|
addr, err := c.ServerInstanceURL()
|
|
if err != nil {
|
|
return RelayConnState{URL: c.connectionURL, Err: err}
|
|
}
|
|
return RelayConnState{URL: addr, Transport: c.Transport()}
|
|
}
|