[PR #6081] [client, management] Phase 1+2+3 of #5989: ConnectionMode foundation, two-timer lifecycle, iceBackoff (stack 1/4) #24787

Open
opened 2026-08-05 06:09:08 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6081
Author: @MichaelUray
Created: 5/5/2026
Status: 🔄 Open

Base: mainHead: pr/a-p2p-dynamic-foundation


📝 Commits (10+)

  • c4844cc proto: add ConnectionMode enum and p2p/relay timeout fields to PeerConfig
  • e0ed831 client: add connectionmode package with Mode type and proto bridge
  • c71c951 client/peer: ResolveModeFromEnv with NB_CONNECTION_MODE and deprecation warns
  • 7d90a5b client: add --connection-mode, --relay-timeout, --p2p-timeout CLI flags
  • cc10c9f client/conn_mgr: replace asymmetric Lazy/ForceRelay precedence with Mode
  • dfd48e9 client/peer: connection mode drives skip-ICE branch in Open()
  • 82877f0 client/engine: forward resolved Mode to per-peer ConnConfig
  • cd0abe8 mgmt/types: add ConnectionMode + p2p/relay timeout to Settings
  • 0022145 openapi: add connection_mode + p2p/relay timeout fields to AccountSettings
  • b22128e mgmt/handlers/accounts: accept connection_mode + timeout settings on PUT

📊 Changes

42 files changed (+7449 additions, -2407 deletions)

View changed files

📝 client/cmd/root.go (+17 -0)
📝 client/cmd/up.go (+39 -0)
📝 client/internal/conn_mgr.go (+348 -42)
client/internal/conn_mgr_test.go (+204 -0)
📝 client/internal/connect.go (+19 -0)
📝 client/internal/debug/debug.go (+6 -0)
📝 client/internal/engine.go (+26 -3)
📝 client/internal/lazyconn/env.go (+5 -0)
📝 client/internal/lazyconn/inactivity/manager.go (+120 -21)
📝 client/internal/lazyconn/inactivity/manager_test.go (+257 -0)
📝 client/internal/lazyconn/manager/manager.go (+43 -1)
📝 client/internal/peer/conn.go (+169 -4)
📝 client/internal/peer/conn_test.go (+133 -0)
📝 client/internal/peer/env.go (+77 -0)
client/internal/peer/env_test.go (+58 -0)
📝 client/internal/peer/handshaker.go (+25 -4)
client/internal/peer/handshaker_test.go (+50 -0)
client/internal/peer/ice_backoff.go (+123 -0)
client/internal/peer/ice_backoff_test.go (+140 -0)
📝 client/internal/peer/status.go (+24 -0)

...and 22 more files

📄 Description

[client+management] Phase 1+2+3 of #5989: ConnectionMode foundation, two-timer lifecycle, iceBackoff

Branch: pr/a-p2p-dynamic-foundation → base main
Compare: https://github.com/netbirdio/netbird/compare/main...MichaelUray:netbird:pr/a-p2p-dynamic-foundation?expand=1


Summary

This PR is the foundation for the work proposed in issue #5989 ("Consolidate connection-mode flags; add p2p-dynamic and p2p-dynamic-lazy modes"). It introduces a single first-class ConnectionMode enum on the wire and across Settings, replaces the asymmetric LazyConnectionEnabled / IsForceRelayed precedence with that enum, adds the per-peer two-timer lifecycle (P2pTimeoutSeconds for ICE-inactivity detach, RelayTimeoutSeconds for full close), and lands the per-peer iceBackoffState so failed pair-checks don't loop forever (P2pRetryMaxSeconds controls the cap).

It supersedes the older PR #6047 (which only landed Phase 1 and is no longer mergeable).

Why

NetBird currently has two flags fighting for the same job:

  • LazyConnectionEnabled (account-wide bool) — eager vs. lazy initial connect.
  • IsForceRelayed (per-peer hint) — skip ICE entirely.

A user who wants "all my peers should start lazy, but on demand try P2P, and if P2P never comes up, fall back to relay" can't express that with two booleans. The proposed ConnectionMode is one of p2p, p2p-lazy, p2p-dynamic, p2p-dynamic-lazy, relay-forced. Each mode unambiguously selects one combination of (eager-vs-lazy, skip-ICE-yes-no, ICE-fallback-to-relay-yes-no).

The p2p-dynamic variants additionally need two timers (one for ICE-only inactivity, one for full conn close) and an exponential ICE-failure backoff so a chronically-broken NAT path can't pin a peer to relay-with-stalled-retries forever.

What's in this PR

Phase 1 — Wire + Settings + CLI plumbing

  • New proto field PeerConfig.ConnectionMode (enum) + P2pTimeoutSeconds, RelayTimeoutSeconds.
  • New OpenAPI AccountSettings.connection_mode + the two timeouts; mgmt PUT handler accepts and audit-logs them.
  • Client: --connection-mode, --p2p-timeout, --relay-timeout CLI flags + env-var bridge with deprecation warnings for the old flags.
  • connectionmode package with Mode type, proto bridge, ResolveModeFromEnv.

Phase 2 — Lifecycle mechanism

  • Conn.AttachICE() / DetachICE() — register/remove the ICE listener on the handshaker on demand instead of at Open().
  • client/peer/conn.Open() defers ICE-listener registration in p2p-dynamic; the ICE worker is created but the dispatch is held back until the activity listener fires.
  • Two-timer client/lazyconn/inactivity (per-peer ICE-inactivity vs. relay-timeout, separate channels).
  • conn_mgr.ActivatePeer/DeactivatePeer per-mode plumbing for ICE attach/detach.

Phase 3 — Backoff

  • client/peer.iceBackoffState truncated exponential schedule; markFailure, markSuccess, Reset API.
  • Conn.AttachICE no-ops while suspended; pion ICE state changes drive markFailure/markSuccess.
  • conn_mgr resolves P2pRetryMaxSeconds from server-pushed PeerConfig and propagates updates to active conns.
  • netbird status -d shows the current backoff state (Failures, NextRetry, Suspended) per peer; line is suppressed when nextRetry has passed.

Tests

  • go test ./client/internal/peer ./client/internal/peer/guard ./client/internal/lazyconn/... — pass.
  • go build ./client/... ./management/... — pass on linux/amd64, linux/arm64, windows/amd64.
  • Hardware-validated end-to-end on a 4-peer testbed (Windows 11 daemon + 3 OpenWrt routers) running the full p2p-dynamic lifecycle (cold-start → P2P → idle-detach to Relay → idle-close to "Idle" → wake → P2P recovery). The follow-up PRs (B, C, D) build on this foundation and the full lifecycle test goes through them.

Test plan

  • Unit tests for iceBackoffState (initial, exponential, reset, suspended-expired, max-cap, disabled-when-zero, grace-after-reset, no-grace-without-reset).
  • mgmt/grpc tests cover toPeerConfig mode resolution paths.
  • All proto regenerated cleanly (bash client/proto/generate.sh, bash shared/management/proto/generate.sh).
  • Maintainer review of the proto schema for backwards compatibility on legacy clients (no behavioural change for unset / unknown enum values; existing LazyConnectionEnabled still honoured by older daemons).

Use case

Real-world deployment: a fleet of OpenWrt routers + a small number of always-on Linux/Windows boxes. For roaming/laptop peers we want strict relay (relay-forced) for predictable latency; for the routers we want p2p-dynamic so the daemon attempts P2P on demand but doesn't hold an ICE pair when the link is quiet. With the current two-flag approach this configuration is impossible from the dashboard.

Linked work

  • Closes part of #5989.
  • Supersedes #6047 (Phase 1 only, no longer mergeable due to drift).
  • Independent of #5807 / #5806 / #5994 (Android NetworkAddresses, posture-check, lazy-toggle UI — those remain stand-alone).

What's not here (follow-up PRs)

  • PR-B: Phase 3.5 + 3.7d-h network-change handling, signal-trigger re-attach, ICE-grace-window for stale NAT mappings, GUI mode tab.
  • PR-C: Phase 3.7i peer-status visibility (per-peer remote meta, conn-state pusher with adaptive heartbeat, Peers tab in client UI).
  • PR-D: Activity-trigger fast-path (relay-state activity → re-attach ICE), Guard activity-driven retry-budget reset, hardening (legacy-fallback, Codex review fixes), Windows ICE filter case-insensitive (subset, branched separately as PR-Q2), keepWgPeer routed-subnet fix.

The PRs are stacked: B's base = A's branch; C's base = B; D's base = C. Reviewing A first lets the foundation land standalone.

Maintainers are welcome to push directly to this branch.

Summary by CodeRabbit

  • New Features
    • Configurable peer connection modes (relay, p2p, p2p-lazy, p2p-dynamic); account/client timeout and retry settings; new CLI flags; per-peer ICE attach/detach with exponential backoff; status shows ICE backoff state.
  • Documentation
    • OpenAPI and HTTP API updated to expose experimental connection-mode and timeout fields; debug output includes new config fields.
  • Tests
    • Extensive unit tests for mode resolution, inactivity managers, ICE lifecycle/backoff, and related flows.
  • Chores
    • Audit events added for connection-mode and timeout changes.

Documentation

  • Documentation is not needed

These changes are internal lifecycle / behavioural improvements; no user-visible API or CLI flag added that warrants new public docs. Existing flags/Settings already documented at netbirdio/docs cover the surface area.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6081 **Author:** [@MichaelUray](https://github.com/MichaelUray) **Created:** 5/5/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `pr/a-p2p-dynamic-foundation` --- ### 📝 Commits (10+) - [`c4844cc`](https://github.com/netbirdio/netbird/commit/c4844ccfd1361ad48194731423a7f873a73a24f7) proto: add ConnectionMode enum and p2p/relay timeout fields to PeerConfig - [`e0ed831`](https://github.com/netbirdio/netbird/commit/e0ed8313697835bafa34556aec487bc782e95307) client: add connectionmode package with Mode type and proto bridge - [`c71c951`](https://github.com/netbirdio/netbird/commit/c71c951d88b9ce5fb2d224684e50e6dd31d77072) client/peer: ResolveModeFromEnv with NB_CONNECTION_MODE and deprecation warns - [`7d90a5b`](https://github.com/netbirdio/netbird/commit/7d90a5bdd5cc598293640d1c8bcfa23ee866e6c6) client: add --connection-mode, --relay-timeout, --p2p-timeout CLI flags - [`cc10c9f`](https://github.com/netbirdio/netbird/commit/cc10c9f108fce4b90edf7d4f4bf399e53057e038) client/conn_mgr: replace asymmetric Lazy/ForceRelay precedence with Mode - [`dfd48e9`](https://github.com/netbirdio/netbird/commit/dfd48e920bf889a20ef699b5aaff1a0a4b64d491) client/peer: connection mode drives skip-ICE branch in Open() - [`82877f0`](https://github.com/netbirdio/netbird/commit/82877f0ce8cb23ba722f393d47b50a508dc692bd) client/engine: forward resolved Mode to per-peer ConnConfig - [`cd0abe8`](https://github.com/netbirdio/netbird/commit/cd0abe890d579c0dfbbb2251b4451d62c5b9e3bf) mgmt/types: add ConnectionMode + p2p/relay timeout to Settings - [`0022145`](https://github.com/netbirdio/netbird/commit/0022145a414e794f7701250460b10badba7741b5) openapi: add connection_mode + p2p/relay timeout fields to AccountSettings - [`b22128e`](https://github.com/netbirdio/netbird/commit/b22128ed14f35d56d75e640f986d2e934414401d) mgmt/handlers/accounts: accept connection_mode + timeout settings on PUT ### 📊 Changes **42 files changed** (+7449 additions, -2407 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/root.go` (+17 -0) 📝 `client/cmd/up.go` (+39 -0) 📝 `client/internal/conn_mgr.go` (+348 -42) ➕ `client/internal/conn_mgr_test.go` (+204 -0) 📝 `client/internal/connect.go` (+19 -0) 📝 `client/internal/debug/debug.go` (+6 -0) 📝 `client/internal/engine.go` (+26 -3) 📝 `client/internal/lazyconn/env.go` (+5 -0) 📝 `client/internal/lazyconn/inactivity/manager.go` (+120 -21) 📝 `client/internal/lazyconn/inactivity/manager_test.go` (+257 -0) 📝 `client/internal/lazyconn/manager/manager.go` (+43 -1) 📝 `client/internal/peer/conn.go` (+169 -4) 📝 `client/internal/peer/conn_test.go` (+133 -0) 📝 `client/internal/peer/env.go` (+77 -0) ➕ `client/internal/peer/env_test.go` (+58 -0) 📝 `client/internal/peer/handshaker.go` (+25 -4) ➕ `client/internal/peer/handshaker_test.go` (+50 -0) ➕ `client/internal/peer/ice_backoff.go` (+123 -0) ➕ `client/internal/peer/ice_backoff_test.go` (+140 -0) 📝 `client/internal/peer/status.go` (+24 -0) _...and 22 more files_ </details> ### 📄 Description # [client+management] Phase 1+2+3 of #5989: ConnectionMode foundation, two-timer lifecycle, iceBackoff **Branch:** `pr/a-p2p-dynamic-foundation` → base `main` **Compare:** https://github.com/netbirdio/netbird/compare/main...MichaelUray:netbird:pr/a-p2p-dynamic-foundation?expand=1 --- ## Summary This PR is the foundation for the work proposed in issue #5989 ("Consolidate connection-mode flags; add p2p-dynamic and p2p-dynamic-lazy modes"). It introduces a single first-class `ConnectionMode` enum on the wire and across Settings, replaces the asymmetric `LazyConnectionEnabled` / `IsForceRelayed` precedence with that enum, adds the per-peer two-timer lifecycle (`P2pTimeoutSeconds` for ICE-inactivity detach, `RelayTimeoutSeconds` for full close), and lands the per-peer `iceBackoffState` so failed pair-checks don't loop forever (`P2pRetryMaxSeconds` controls the cap). It supersedes the older PR #6047 (which only landed Phase 1 and is no longer mergeable). ## Why NetBird currently has two flags fighting for the same job: - `LazyConnectionEnabled` (account-wide bool) — eager vs. lazy initial connect. - `IsForceRelayed` (per-peer hint) — skip ICE entirely. A user who wants "all my peers should start lazy, but on demand try P2P, and if P2P never comes up, fall back to relay" can't express that with two booleans. The proposed `ConnectionMode` is one of `p2p`, `p2p-lazy`, `p2p-dynamic`, `p2p-dynamic-lazy`, `relay-forced`. Each mode unambiguously selects one combination of (eager-vs-lazy, skip-ICE-yes-no, ICE-fallback-to-relay-yes-no). The `p2p-dynamic` variants additionally need two timers (one for ICE-only inactivity, one for full conn close) and an exponential ICE-failure backoff so a chronically-broken NAT path can't pin a peer to relay-with-stalled-retries forever. ## What's in this PR ### Phase 1 — Wire + Settings + CLI plumbing - New proto field `PeerConfig.ConnectionMode` (enum) + `P2pTimeoutSeconds`, `RelayTimeoutSeconds`. - New OpenAPI `AccountSettings.connection_mode` + the two timeouts; mgmt PUT handler accepts and audit-logs them. - Client: `--connection-mode`, `--p2p-timeout`, `--relay-timeout` CLI flags + env-var bridge with deprecation warnings for the old flags. - `connectionmode` package with `Mode` type, proto bridge, `ResolveModeFromEnv`. ### Phase 2 — Lifecycle mechanism - `Conn.AttachICE()` / `DetachICE()` — register/remove the ICE listener on the handshaker on demand instead of at `Open()`. - `client/peer/conn.Open()` defers ICE-listener registration in p2p-dynamic; the ICE worker is created but the dispatch is held back until the activity listener fires. - Two-timer `client/lazyconn/inactivity` (per-peer ICE-inactivity vs. relay-timeout, separate channels). - `conn_mgr.ActivatePeer/DeactivatePeer` per-mode plumbing for ICE attach/detach. ### Phase 3 — Backoff - `client/peer.iceBackoffState` truncated exponential schedule; `markFailure`, `markSuccess`, `Reset` API. - `Conn.AttachICE` no-ops while suspended; pion ICE state changes drive `markFailure`/`markSuccess`. - `conn_mgr` resolves `P2pRetryMaxSeconds` from server-pushed `PeerConfig` and propagates updates to active conns. - `netbird status -d` shows the current backoff state (Failures, NextRetry, Suspended) per peer; line is suppressed when `nextRetry` has passed. ## Tests - `go test ./client/internal/peer ./client/internal/peer/guard ./client/internal/lazyconn/...` — pass. - `go build ./client/... ./management/...` — pass on linux/amd64, linux/arm64, windows/amd64. - Hardware-validated end-to-end on a 4-peer testbed (Windows 11 daemon + 3 OpenWrt routers) running the full p2p-dynamic lifecycle (cold-start → P2P → idle-detach to Relay → idle-close to "Idle" → wake → P2P recovery). The follow-up PRs (B, C, D) build on this foundation and the full lifecycle test goes through them. ## Test plan - [x] Unit tests for `iceBackoffState` (initial, exponential, reset, suspended-expired, max-cap, disabled-when-zero, grace-after-reset, no-grace-without-reset). - [x] `mgmt/grpc` tests cover `toPeerConfig` mode resolution paths. - [x] All proto regenerated cleanly (`bash client/proto/generate.sh`, `bash shared/management/proto/generate.sh`). - [ ] Maintainer review of the proto schema for backwards compatibility on legacy clients (no behavioural change for unset / unknown enum values; existing `LazyConnectionEnabled` still honoured by older daemons). ## Use case Real-world deployment: a fleet of OpenWrt routers + a small number of always-on Linux/Windows boxes. For roaming/laptop peers we want strict relay (`relay-forced`) for predictable latency; for the routers we want `p2p-dynamic` so the daemon attempts P2P on demand but doesn't hold an ICE pair when the link is quiet. With the current two-flag approach this configuration is impossible from the dashboard. ## Linked work - Closes part of #5989. - Supersedes #6047 (Phase 1 only, no longer mergeable due to drift). - Independent of #5807 / #5806 / #5994 (Android NetworkAddresses, posture-check, lazy-toggle UI — those remain stand-alone). ## What's *not* here (follow-up PRs) - **PR-B**: Phase 3.5 + 3.7d-h network-change handling, signal-trigger re-attach, ICE-grace-window for stale NAT mappings, GUI mode tab. - **PR-C**: Phase 3.7i peer-status visibility (per-peer remote meta, conn-state pusher with adaptive heartbeat, Peers tab in client UI). - **PR-D**: Activity-trigger fast-path (relay-state activity → re-attach ICE), Guard activity-driven retry-budget reset, hardening (legacy-fallback, Codex review fixes), Windows ICE filter case-insensitive (subset, branched separately as PR-Q2), keepWgPeer routed-subnet fix. The PRs are stacked: B's base = A's branch; C's base = B; D's base = C. Reviewing A first lets the foundation land standalone. **Maintainers are welcome to push directly to this branch.** <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Configurable peer connection modes (relay, p2p, p2p-lazy, p2p-dynamic); account/client timeout and retry settings; new CLI flags; per-peer ICE attach/detach with exponential backoff; status shows ICE backoff state. * **Documentation** * OpenAPI and HTTP API updated to expose experimental connection-mode and timeout fields; debug output includes new config fields. * **Tests** * Extensive unit tests for mode resolution, inactivity managers, ICE lifecycle/backoff, and related flows. * **Chores** * Audit events added for connection-mode and timeout changes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Documentation - [x] Documentation is **not needed** These changes are internal lifecycle / behavioural improvements; no user-visible API or CLI flag added that warrants new public docs. Existing flags/Settings already documented at netbirdio/docs cover the surface area. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 06:09:08 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#24787