mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-19 23:11:29 +02:00
2d8b0310a4
Two related fixes for the embedded netbird client and the per-account
inbound listeners that ride on its gVisor netstack.
client/internal/engine.go — replace hasIPv6Changed with reconcileIPv6:
- First v6 assignment (current had no v6, conf carries one) is applied
in place via WGIface.UpdateAddr instead of returning ErrResetConnection.
Pre-fix, every embedded client whose account had IPv6 enabled would
reset on its first NetworkMap sync — boot config has no v6, the sync
introduces one, the engine tore itself down to "apply" it. That
teardown destroys the gVisor netstack and orphans every listener
bound on it, which is what made the proxy's per-account :80/:443
silently stop accepting traffic.
- v6 removed clears in place.
- v6 swapped to a different non-empty value still resets (gVisor
netstack can't safely swap its address at runtime).
- Mutates e.config.WgAddr to match the applied state so subsequent
PeerConfig comparisons are stable.
proxy/internal/tcp/accept.go (new) + proxy/inbound.go +
proxy/internal/tcp/router.go — harden the two Accept() loops on
netstack-backed listeners:
- IsClosedListenerErr recognises net.ErrClosed AND gVisor's
"endpoint is in invalid state" — the latter survives gonet's
*net.OpError wrapping in a way errors.Is(.., net.ErrClosed) does
not. Without this the loop spins CPU-hot after the underlying
netstack is destroyed (peer rekey, embedded-client reset, account
churn), emitting one log line per iteration.
- AcceptBackoff implements the exponential backoff that
net/http.Server.Serve uses on transient Accept errors: 5ms doubling
up to 1s. Defence-in-depth so an unknown sticky error cannot burn
a CPU core even if IsClosedListenerErr misses its signature.
proxy/internal/roundtrip/netbird.go — emit a single structured INFO
line summarising every embed.Options flag (account_id, service_id,
public_key, management_url, wg_port, block_inbound, block_lan_access,
disable_ipv6, no_userspace, presence of credentials) when each
per-account embedded client is created. Secrets reduced to a "present"
boolean — never logged verbatim. Diagnostic-only; no behavior change,
but it makes the "why is this embedded peer misbehaving" loop a single
log read instead of a code dive.
Tests (real listeners, scripted errors, no mocks of production code):
- engine_reconcileipv6_test.go: 8 cases for every transition (first
assignment, no change, removed, prefix-length changed, value
changed, invalid bytes, UpdateAddr error) plus a updateConfig
integration check that the fix actually fires on a v6-added
PeerConfig.
- accept_test.go: IsClosedListenerErr matrix + AcceptBackoff
progression / cap / reset / cancel-during-wait / cancel-before-call.
- router_test.go, inbound_test.go: scriptedAcceptListener +
TestRouter_Serve_ExitsOnGVisorInvalidEndpoint and
TestFeedRouterFromListener_ExitsOnGVisorInvalidEndpoint —
regression guards that fail in 2 s if the loop ever spins.
86 lines
2.7 KiB
Go
86 lines
2.7 KiB
Go
package tcp
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"net"
|
|
"strings"
|
|
"time"
|
|
)
|
|
|
|
// gvisorInvalidEndpointMsg is the canonical text gVisor netstack returns
|
|
// when Accept() is called on a listener whose underlying endpoint has
|
|
// been destroyed (peer rekey, embedded-client reset, account churn).
|
|
// There is no exported sentinel from gvisor.dev/gvisor/pkg/tcpip that
|
|
// survives gonet's *net.OpError wrapping in a way errors.Is can match,
|
|
// so we fall back to a string check. Stable across the gVisor versions
|
|
// netbird pins.
|
|
const gvisorInvalidEndpointMsg = "endpoint is in invalid state"
|
|
|
|
// IsClosedListenerErr reports whether err signals that an accept loop
|
|
// should exit because the underlying listener can no longer serve
|
|
// connections. It recognises:
|
|
//
|
|
// - net.ErrClosed for stdlib listeners (Listener.Close was called).
|
|
// - gVisor's "endpoint is in invalid state" for netstack-backed
|
|
// listeners whose endpoint was destroyed out from under them
|
|
// (typically when a per-account WireGuard netstack is reset without
|
|
// also tearing the listener entry down).
|
|
//
|
|
// Without the gVisor branch an accept loop on a netstack listener spins
|
|
// CPU-hot forever after the endpoint dies, because Accept never blocks
|
|
// again and the error neither matches net.ErrClosed nor cancels ctx.
|
|
func IsClosedListenerErr(err error) bool {
|
|
if err == nil {
|
|
return false
|
|
}
|
|
if errors.Is(err, net.ErrClosed) {
|
|
return true
|
|
}
|
|
return strings.Contains(err.Error(), gvisorInvalidEndpointMsg)
|
|
}
|
|
|
|
// AcceptBackoff implements the exponential backoff used by
|
|
// net/http.Server.Serve for transient Accept errors. Without it a loop
|
|
// hitting a sticky unknown error burns a full CPU core. The zero value
|
|
// is ready to use; call Reset after a successful Accept.
|
|
type AcceptBackoff struct {
|
|
delay time.Duration
|
|
}
|
|
|
|
// minAcceptDelay / maxAcceptDelay mirror the stdlib defaults
|
|
// (net/http.Server.Serve) and keep us well below 1 log line per second
|
|
// per orphaned listener.
|
|
const (
|
|
minAcceptDelay = 5 * time.Millisecond
|
|
maxAcceptDelay = time.Second
|
|
)
|
|
|
|
// Backoff waits the next exponential delay (5ms doubling up to 1s) and
|
|
// returns true when the wait completed. Returns false if ctx fired
|
|
// during the wait — callers should treat that as "exit the loop".
|
|
func (b *AcceptBackoff) Backoff(ctx context.Context) bool {
|
|
b.advance()
|
|
select {
|
|
case <-ctx.Done():
|
|
return false
|
|
case <-time.After(b.delay):
|
|
return true
|
|
}
|
|
}
|
|
|
|
// Reset clears the accumulated delay so the next failure starts at the
|
|
// minimum delay again. Call after a successful Accept.
|
|
func (b *AcceptBackoff) Reset() { b.delay = 0 }
|
|
|
|
func (b *AcceptBackoff) advance() {
|
|
if b.delay == 0 {
|
|
b.delay = minAcceptDelay
|
|
} else {
|
|
b.delay *= 2
|
|
}
|
|
if b.delay > maxAcceptDelay {
|
|
b.delay = maxAcceptDelay
|
|
}
|
|
}
|