diff --git a/shared/relay/client/client.go b/shared/relay/client/client.go index c851ab329..a8d627538 100644 --- a/shared/relay/client/client.go +++ b/shared/relay/client/client.go @@ -175,15 +175,16 @@ type Client struct { mtu uint16 - // transportFallback, when set, records QUIC datagram failures so the relay - // is dialed over WebSocket on subsequent connects. Shared via the manager. + // transportFallback, when set, records datagram-too-large failures so a + // datagram-sized transport is avoided on subsequent connects. Shared via + // the manager. transportFallback *transportFallback // datagramFallbackTriggered guards a single fallback per connection so a // burst of oversized datagrams triggers one reconnect, not many. datagramFallbackTriggered atomic.Bool } -// SetTransportFallback wires the shared QUIC-to-WebSocket fallback tracker. +// SetTransportFallback wires the shared datagram-transport fallback tracker. func (c *Client) SetTransportFallback(tf *transportFallback) { c.transportFallback = tf } @@ -665,21 +666,24 @@ func (c *Client) writeTo(containerRef *connContainer, dstID messages.PeerID, pay return len(payload), err } -// onDatagramTooLarge reacts to a QUIC datagram rejected as too large for the -// path. Unless the transport is pinned to QUIC, it records a WebSocket fallback -// for this server and closes the connection so the reconnect dials WebSocket. -// A single fallback is triggered per connection regardless of how many -// oversized datagrams arrive. cause carries the datagram size and path budget. +// onDatagramTooLarge reacts to a datagram rejected as too large for the path. +// When a non-datagram transport is available, it records a fallback for this +// server and closes the connection so the reconnect avoids datagram-sized +// transports. A single fallback is triggered per connection regardless of how +// many oversized datagrams arrive. cause carries the datagram size and budget. func (c *Client) onDatagramTooLarge(conn net.Conn, cause error) { - if transportModeFromEnv() == TransportModeQUIC { + // If the selected mode offers no non-datagram transport (e.g. pinned to a + // datagram-sized transport), reconnecting would just re-fail, so leave the + // connection up rather than loop. + if len(nonDatagramSized(c.baseDialers(transportModeFromEnv()))) == 0 { if c.datagramFallbackTriggered.CompareAndSwap(false, true) { - c.log.Warnf("%s, but %s=quic pins QUIC, not falling back", cause, EnvRelayTransport) + c.log.Warnf("%s, but no non-datagram transport is available, not falling back", cause) } return } - // Without the shared tracker a reconnect would just race QUIC again and - // re-fail, so leave the connection up rather than loop. + // Without the shared tracker a reconnect would just select the same + // transport again and re-fail, so leave the connection up rather than loop. if c.transportFallback == nil { if c.datagramFallbackTriggered.CompareAndSwap(false, true) { c.log.Debugf("%s, but no transport fallback configured, leaving connection up", cause) @@ -692,7 +696,7 @@ func (c *Client) onDatagramTooLarge(conn net.Conn, cause error) { } window := c.transportFallback.recordFailure(c.connectionURL) - c.log.Warnf("%s, falling back to WebSocket for %s", cause, window) + c.log.Warnf("%s, avoiding datagram-sized transport for %s", cause, window) if err := conn.Close(); err != nil { c.log.Debugf("close relay connection for transport fallback: %s", err) diff --git a/shared/relay/client/dialer/capability.go b/shared/relay/client/dialer/capability.go new file mode 100644 index 000000000..511cb2ac7 --- /dev/null +++ b/shared/relay/client/dialer/capability.go @@ -0,0 +1,18 @@ +package dialer + +// DatagramSized is implemented by dialers whose connections carry each write in +// a single datagram, so a write can be rejected when it exceeds the path's +// datagram budget (e.g. QUIC). Transports without this capability (e.g. +// WebSocket over TCP) impose no per-write size limit, so the relay client can +// fall back to them when a datagram-sized transport rejects a write as too +// large. The capability is advertised per dialer rather than hardcoded, so a +// new transport only needs to declare whether it is datagram-sized. +type DatagramSized interface { + DatagramSized() +} + +// IsDatagramSized reports whether d produces datagram-sized connections. +func IsDatagramSized(d DialeFn) bool { + _, ok := d.(DatagramSized) + return ok +} diff --git a/shared/relay/client/dialer/quic/quic.go b/shared/relay/client/dialer/quic/quic.go index f490e62bf..80f771173 100644 --- a/shared/relay/client/dialer/quic/quic.go +++ b/shared/relay/client/dialer/quic/quic.go @@ -24,6 +24,10 @@ func (d Dialer) Protocol() string { return Network } +// DatagramSized marks QUIC as a datagram-sized transport: relay traffic is +// carried in QUIC DATAGRAM frames, which must fit a single packet. +func (d Dialer) DatagramSized() {} + func (d Dialer) Dial(ctx context.Context, address, serverName string) (net.Conn, error) { quicURL, err := prepareURL(address) if err != nil { diff --git a/shared/relay/client/dialers_generic.go b/shared/relay/client/dialers_generic.go index 3495fb412..95e319338 100644 --- a/shared/relay/client/dialers_generic.go +++ b/shared/relay/client/dialers_generic.go @@ -9,10 +9,25 @@ import ( "github.com/netbirdio/netbird/shared/relay/client/dialer/ws" ) -// getDialers returns the ordered list of dialers for connecting to the relay -// server. For racing modes (auto) the order is irrelevant; for prefer modes the -// first entry is tried before falling back to the second. +// getDialers returns the ordered dialers for connecting to the relay server. It +// applies the datagram fallback generically: if this server recently rejected a +// datagram-sized transport, those dialers are dropped, leaving the rest. func (c *Client) getDialers(mode TransportMode) []dialer.DialeFn { + dialers := c.baseDialers(mode) + + if c.transportFallback != nil && c.transportFallback.avoidDatagramSized(c.connectionURL) { + if filtered := nonDatagramSized(dialers); len(filtered) > 0 { + c.log.Infof("relay recently rejected a datagram-sized transport, avoiding it") + return filtered + } + } + return dialers +} + +// baseDialers returns the ordered dialers for the mode, before any datagram +// fallback filtering. For racing modes (auto) the order is irrelevant; for +// prefer modes the first entry is tried before falling back to the second. +func (c *Client) baseDialers(mode TransportMode) []dialer.DialeFn { switch mode { case TransportModeWS: c.log.Infof("%s=ws, using WebSocket transport", EnvRelayTransport) @@ -22,18 +37,14 @@ func (c *Client) getDialers(mode TransportMode) []dialer.DialeFn { return []dialer.DialeFn{quic.Dialer{}} } - if c.mtu > 0 && c.mtu > iface.DefaultMTU { - c.log.Infof("MTU %d exceeds default (%d), forcing WebSocket transport to avoid DATAGRAM frame size issues", c.mtu, iface.DefaultMTU) - return []dialer.DialeFn{ws.Dialer{}} - } - - if c.transportFallback != nil && c.transportFallback.preferWS(c.connectionURL) { - c.log.Infof("relay recently rejected QUIC datagrams, using WebSocket transport") - return []dialer.DialeFn{ws.Dialer{}} - } - + all := []dialer.DialeFn{quic.Dialer{}, ws.Dialer{}} if mode == TransportModePreferWS { - return []dialer.DialeFn{ws.Dialer{}, quic.Dialer{}} + all = []dialer.DialeFn{ws.Dialer{}, quic.Dialer{}} } - return []dialer.DialeFn{quic.Dialer{}, ws.Dialer{}} + + if c.mtu > 0 && c.mtu > iface.DefaultMTU { + c.log.Infof("MTU %d exceeds default (%d), avoiding datagram-sized transports", c.mtu, iface.DefaultMTU) + return nonDatagramSized(all) + } + return all } diff --git a/shared/relay/client/dialers_generic_test.go b/shared/relay/client/dialers_generic_test.go index bc94e08e4..c4ef9cc59 100644 --- a/shared/relay/client/dialers_generic_test.go +++ b/shared/relay/client/dialers_generic_test.go @@ -12,8 +12,17 @@ import ( "github.com/netbirdio/netbird/client/iface" "github.com/netbirdio/netbird/shared/relay/client/dialer" netErr "github.com/netbirdio/netbird/shared/relay/client/dialer/net" + "github.com/netbirdio/netbird/shared/relay/client/dialer/quic" + "github.com/netbirdio/netbird/shared/relay/client/dialer/ws" ) +// TestDatagramSizedCapability locks the capability the generic fallback relies +// on: QUIC is datagram-sized, WebSocket is not. +func TestDatagramSizedCapability(t *testing.T) { + assert.True(t, dialer.IsDatagramSized(quic.Dialer{}), "QUIC must advertise datagram-sized") + assert.False(t, dialer.IsDatagramSized(ws.Dialer{}), "WebSocket must not advertise datagram-sized") +} + func protocols(dialers []dialer.DialeFn) []string { out := make([]string, len(dialers)) for i, d := range dialers { diff --git a/shared/relay/client/dialers_js.go b/shared/relay/client/dialers_js.go index 5347644d8..c93787729 100644 --- a/shared/relay/client/dialers_js.go +++ b/shared/relay/client/dialers_js.go @@ -11,3 +11,7 @@ func (c *Client) getDialers(_ TransportMode) []dialer.DialeFn { // JS/WASM build only uses WebSocket transport return []dialer.DialeFn{ws.Dialer{}} } + +func (c *Client) baseDialers(_ TransportMode) []dialer.DialeFn { + return []dialer.DialeFn{ws.Dialer{}} +} diff --git a/shared/relay/client/manager.go b/shared/relay/client/manager.go index b33f1018b..f87da15de 100644 --- a/shared/relay/client/manager.go +++ b/shared/relay/client/manager.go @@ -81,7 +81,7 @@ type Manager struct { keepUnusedServerTime time.Duration // transportFallback is shared across home and foreign relay clients so a - // QUIC datagram failure pins WebSocket for that server across reconnects. + // datagram-too-large failure makes that server avoid datagram-sized transports across reconnects. transportFallback *transportFallback } diff --git a/shared/relay/client/transport.go b/shared/relay/client/transport.go index 70b4910ea..baf76b31c 100644 --- a/shared/relay/client/transport.go +++ b/shared/relay/client/transport.go @@ -7,6 +7,8 @@ import ( "time" log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/shared/relay/client/dialer" ) // EnvRelayTransport pins the relay transport. Valid values: "auto" (default, @@ -18,8 +20,8 @@ import ( const EnvRelayTransport = "NB_RELAY_TRANSPORT" const ( - // transportFallbackBase is the initial window a relay server is pinned to - // WebSocket after a QUIC datagram is rejected as too large. + // transportFallbackBase is the initial window a relay server avoids + // datagram-sized transports after a datagram is rejected as too large. transportFallbackBase = 10 * time.Minute // transportFallbackMax caps the pinned window when failures repeat. transportFallbackMax = 60 * time.Minute @@ -62,11 +64,12 @@ func (m TransportMode) sequential() bool { return m == TransportModePreferQUIC || m == TransportModePreferWS } -// transportFallback tracks relay servers that have failed to carry QUIC -// datagrams and should temporarily use WebSocket instead. It is shared across -// the relay manager so the preference survives client recreation (foreign relay -// clients are evicted and rebuilt on disconnect). Entries are keyed by server -// URL and expire after a window that grows on repeated failures. +// transportFallback tracks relay servers that have rejected a datagram-sized +// transport (a write too large for the path) and should temporarily avoid such +// transports. It is shared across the relay manager so the preference survives +// client recreation (foreign relay clients are evicted and rebuilt on +// disconnect). Entries are keyed by server URL and expire after a window that +// grows on repeated failures. type transportFallback struct { mu sync.Mutex entries map[string]*fallbackEntry @@ -81,18 +84,31 @@ func newTransportFallback() *transportFallback { return &transportFallback{entries: make(map[string]*fallbackEntry)} } -// preferWS reports whether serverURL is currently within a WebSocket fallback -// window. -func (f *transportFallback) preferWS(serverURL string) bool { +// nonDatagramSized returns the dialers from in that are not datagram-sized, +// preserving order. +func nonDatagramSized(in []dialer.DialeFn) []dialer.DialeFn { + out := make([]dialer.DialeFn, 0, len(in)) + for _, d := range in { + if !dialer.IsDatagramSized(d) { + out = append(out, d) + } + } + return out +} + +// avoidDatagramSized reports whether serverURL is currently within a window +// where datagram-sized transports should be avoided. +func (f *transportFallback) avoidDatagramSized(serverURL string) bool { f.mu.Lock() defer f.mu.Unlock() e := f.entries[serverURL] return e != nil && time.Now().Before(e.until) } -// recordFailure pins serverURL to WebSocket for a window: transportFallbackBase -// on the first failure, doubling up to transportFallbackMax when QUIC fails -// again after a previous window expired. It returns the active window duration. +// recordFailure makes serverURL avoid datagram-sized transports for a window: +// transportFallbackBase on the first failure, doubling up to transportFallbackMax +// when a datagram transport fails again after a previous window expired. It +// returns the active window duration. func (f *transportFallback) recordFailure(serverURL string) time.Duration { f.mu.Lock() defer f.mu.Unlock() diff --git a/shared/relay/client/transport_test.go b/shared/relay/client/transport_test.go index ff3adf2c7..8e10c8d42 100644 --- a/shared/relay/client/transport_test.go +++ b/shared/relay/client/transport_test.go @@ -55,11 +55,11 @@ func TestTransportFallbackRecordAndExpiry(t *testing.T) { const url = "rels://relay.example:443" f := newTransportFallback() - assert.False(t, f.preferWS(url), "no fallback recorded yet") + assert.False(t, f.avoidDatagramSized(url), "no fallback recorded yet") d := f.recordFailure(url) assert.Equal(t, transportFallbackBase, d, "first failure pins for the base window") - assert.True(t, f.preferWS(url), "WebSocket preferred within the window") + assert.True(t, f.avoidDatagramSized(url), "datagram-sized transport avoided within the window") // A second failure while still inside the window must not grow the window. d = f.recordFailure(url) @@ -67,9 +67,9 @@ func TestTransportFallbackRecordAndExpiry(t *testing.T) { require.NotNil(t, f.entries[url]) assert.Equal(t, transportFallbackBase, f.entries[url].duration, "duration unchanged inside window") - // Expire the window: WebSocket no longer preferred. + // Expire the window: datagram-sized transport allowed again. f.entries[url].until = time.Now().Add(-time.Second) - assert.False(t, f.preferWS(url), "window expired") + assert.False(t, f.avoidDatagramSized(url), "window expired, datagram-sized transport allowed") } func TestTransportFallbackGrowsOnRepeat(t *testing.T) { @@ -105,7 +105,7 @@ func TestOnDatagramTooLargeAuto(t *testing.T) { c.onDatagramTooLarge(conn, netErr.ErrDatagramTooLarge) assert.True(t, conn.closed, "connection closed to force reconnect") - assert.True(t, tf.preferWS(url), "fallback recorded for the server") + assert.True(t, tf.avoidDatagramSized(url), "fallback recorded for the server") // A second oversized datagram on the same connection must not re-close. conn.closed = false @@ -128,13 +128,13 @@ func TestOnDatagramTooLargeQUICPinned(t *testing.T) { c.onDatagramTooLarge(conn, netErr.ErrDatagramTooLarge) assert.False(t, conn.closed, "QUIC pin keeps the connection, no fallback redial") - assert.False(t, tf.preferWS(url), "QUIC pin records no fallback") + assert.False(t, tf.avoidDatagramSized(url), "QUIC pin records no fallback") } func TestTransportFallbackPerServer(t *testing.T) { f := newTransportFallback() f.recordFailure("rels://a.example:443") - assert.True(t, f.preferWS("rels://a.example:443")) - assert.False(t, f.preferWS("rels://b.example:443"), "fallback is scoped to one server") + assert.True(t, f.avoidDatagramSized("rels://a.example:443")) + assert.False(t, f.avoidDatagramSized("rels://b.example:443"), "fallback is scoped to one server") }