[PR #6082] [client] Phase 3.5 + 3.7d-h of #5989: network-change ICE recovery + GUI mode tab (stack 2/4) #24788

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/6082
Author: @MichaelUray
Created: 5/5/2026
Status: 🔄 Open

Base: mainHead: pr/b-phase3.5-network-change


📝 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

51 files changed (+8427 additions, -2464 deletions)

View changed files

📝 client/android/client.go (+60 -0)
📝 client/android/preferences.go (+85 -0)
📝 client/cmd/root.go (+17 -0)
📝 client/cmd/service.go (+18 -0)
📝 client/cmd/service_installer.go (+57 -0)
📝 client/cmd/up.go (+39 -0)
📝 client/internal/conn_mgr.go (+432 -42)
client/internal/conn_mgr_test.go (+221 -0)
📝 client/internal/connect.go (+19 -0)
📝 client/internal/debug/debug.go (+6 -0)
📝 client/internal/engine.go (+38 -4)
📝 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 (+296 -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/guard/guard.go (+15 -0)

...and 31 more files

📄 Description

[client] Phase 3.5 + 3.7d-h of #5989: network-change ICE recovery + GUI mode tab

Branch: pr/b-phase3.5-network-change → base pr/a-p2p-dynamic-foundation (stacked PR — depends on PR-A landing first)
Compare: https://github.com/netbirdio/netbird/compare/MichaelUray:netbird:pr/a-p2p-dynamic-foundation...MichaelUray:netbird:pr/b-phase3.5-network-change?expand=1


Summary

This PR adds network-change handling, signal-trigger ICE re-attach, a short-grace post-network-change ICE-failure delay (so we don't burn the exponential backoff on the well-known "first ICE pair-check after WiFi/cellular handover fails on stale NAT mappings"), the Guard-Loop fix from the closed PR #5805, and the GUI surface for Connection Mode + timeouts in the client UI.

This is the second of four stacked PRs implementing #5989. It builds on PR-A.

Why

The Phase-1+2+3 foundation (PR-A) handles steady-state operation well, but real devices do not stay in steady state:

  • Laptops roam between WiFi and cellular.
  • Mobile devices wake from Doze/StandBy with stale NAT mappings.
  • LTE modems briefly drop and re-attach.

After such an event the existing ICE agent is bound to a departing local interface and pion's pair-checks fail. The exponential backoff (PR-A) then kicks in and the peer stays on relay for minutes.

Two related fixes are needed:

  1. Re-attach ICE after a network change — recreate the ICE worker on a fresh socket, send a new offer. (Phases 3.5, 3.7d, 3.7e.)
  2. Short retry delay during the post-network-change grace window — the first pair-check often fails on stale mappings even when subsequent ones succeed. Falling back to the normal 1-minute initial backoff after that single failure leaves the peer on relay far longer than the underlying connectivity warrants. A 60-second grace window with 2-second retries lets ~4-5 attempts run while the new mapping primes. (Phases 3.7f, 3.7h.)

Plus: previously closed PR #5805 ("Fix ICE reconnection loop and buffer candidates before agent initialization") had a known reconnect-loop where ICE worker rebuild on every signal-state change racked up offers faster than pion could complete pair-checks. The clean fix is to skip new offers while the ICE agent is still in Connecting.

What's in this PR

Phase 3.5 — Reset on network change

  • client/peer/conn: reset ICE backoff + recreate workerICE when the signal/relay layer reconnects after a network change.
  • client/peer/conn: refactor onNetworkChange to use the in-place agent recreate path; drop SendOffer from onNetworkChange to fix offer-storm; revert and replace an interim worker_ice candidate-buffering attempt with a clean approach.

Phase 3.7d — Signal-trigger re-attach

  • Re-attach ICE on every signal trigger so a peer that lost ICE while the daemon was Disconnected from signaling resumes correctly when signaling is back.

Phase 3.7e — Listener inside onNetworkChange

  • Re-attach the ICE listener inside onNetworkChange itself so a network change fires a fresh listener registration without waiting for the next activity event.

Phase 3.7f + 3.7h — Grace window

  • iceBackoff.markFailure: while inside networkChangeGracePeriod (60s) of the most recent Reset(), cap the suspend delay at networkChangeRetryDelay (2s) and do NOT advance the long-term exponential schedule. Outside the window the normal 1-minute initial backoff applies.
  • Window widened from initial 30s to 60s and delay reduced from 5s to 2s after observing real-world LTE-bounce behaviour: cold NAT mappings often need 3-4 ICE attempts to prime, and the previous 30s window only fit ~2 attempts.

Phase 3.7g — Skip ICE-Close on NC when still connected

  • If ICE is still in Connected state when the network-change fires, don't call workerICE.Close() — the existing pair is fine, just refresh the listener.

#5805 cleanup — Guard-Loop fix

  • worker_ice: skip new offers while the ICE agent is in Connecting. Drops the candidate-buffer approach the closed PR proposed in favour of a much simpler offer-skip during connecting.

GUI surface (Phase 3.7h GUI)

  • client/cmd/service: persist connection-mode + timeouts on install/reconfigure.
  • client/ui: Connection Mode + timeouts in Network tab.
  • client: surface server-pushed connection-mode/timeouts via daemon-RPC.
  • client/ui: "Follow-Server (currently: …)" display + remove the now-redundant Lazy menu.
  • client/android: gomobile getters for ConnectionMode + ServerPushed values.

Tests

  • go test ./client/internal/peer ./client/internal/peer/guard — pass (full ICE-backoff suite including the new grace-window cases).
  • go build ./client/... — pass on linux/amd64, linux/arm64, windows/amd64.
  • Hardware-validated against three OpenWrt routers (LAN host candidate, internet srflx, LTE srflx) plus a Windows 11 daemon. After a forced LTE-modem reset, the LTE peer recovers to P2P srflx within ~50 s instead of the 2-3 minutes the pre-3.7f code took.

Test plan

  • Grace window: failure inside 60s → 2s retry; failure after 60s → exponential restarts at ~1m.
  • No regression for the non-network-change failure path (still uses the exponential schedule).
  • Signal/relay reconnect drives Reset() → grace window engages.
  • Maintainer to verify: GUI changes on macOS / Linux ARM (I tested Windows + Linux/amd64 only).

Use case

A laptop that switches between docked Ethernet, office WiFi, and tethered LTE during the day. Without this PR every transition burns the exponential backoff and the user sees "stuck on relay" for 5-15 minutes. With this PR the post-handover recovery is bounded at ~50s.

Linked work

  • Stacks on PR-A (Phase 1+2+3 foundation).
  • Picks up the offer-skip-during-connecting fix from closed #5805 (the original PR's candidate-buffer approach is replaced with the simpler pattern).
  • Closes part of #5989.

Maintainers are welcome to push directly to this branch.

Summary by CodeRabbit

  • New Features

    • Configurable Connection Modes (relay-forced, p2p, p2p-lazy, p2p-dynamic) with per-mode timeouts, local overrides, and server-pushed defaults surfaced to clients.
    • ICE failure backoff with visible status (suspended, failures, next-retry) and per-peer retry cap; settings persist via profile/install.
  • UI

    • Network/Settings UI updated to manage connection mode, timeout fields, and show server-provided hints.
  • Tests

    • Expanded coverage for mode resolution, inactivity timers, ICE backoff, and related flows.

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/6082 **Author:** [@MichaelUray](https://github.com/MichaelUray) **Created:** 5/5/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `pr/b-phase3.5-network-change` --- ### 📝 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 **51 files changed** (+8427 additions, -2464 deletions) <details> <summary>View changed files</summary> 📝 `client/android/client.go` (+60 -0) 📝 `client/android/preferences.go` (+85 -0) 📝 `client/cmd/root.go` (+17 -0) 📝 `client/cmd/service.go` (+18 -0) 📝 `client/cmd/service_installer.go` (+57 -0) 📝 `client/cmd/up.go` (+39 -0) 📝 `client/internal/conn_mgr.go` (+432 -42) ➕ `client/internal/conn_mgr_test.go` (+221 -0) 📝 `client/internal/connect.go` (+19 -0) 📝 `client/internal/debug/debug.go` (+6 -0) 📝 `client/internal/engine.go` (+38 -4) 📝 `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` (+296 -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/guard/guard.go` (+15 -0) _...and 31 more files_ </details> ### 📄 Description # [client] Phase 3.5 + 3.7d-h of #5989: network-change ICE recovery + GUI mode tab **Branch:** `pr/b-phase3.5-network-change` → base **`pr/a-p2p-dynamic-foundation`** (stacked PR — depends on PR-A landing first) **Compare:** https://github.com/netbirdio/netbird/compare/MichaelUray:netbird:pr/a-p2p-dynamic-foundation...MichaelUray:netbird:pr/b-phase3.5-network-change?expand=1 --- ## Summary This PR adds network-change handling, signal-trigger ICE re-attach, a short-grace post-network-change ICE-failure delay (so we don't burn the exponential backoff on the well-known "first ICE pair-check after WiFi/cellular handover fails on stale NAT mappings"), the Guard-Loop fix from the closed PR #5805, and the GUI surface for Connection Mode + timeouts in the client UI. This is the second of four stacked PRs implementing #5989. It builds on PR-A. ## Why The Phase-1+2+3 foundation (PR-A) handles steady-state operation well, but real devices do not stay in steady state: - Laptops roam between WiFi and cellular. - Mobile devices wake from Doze/StandBy with stale NAT mappings. - LTE modems briefly drop and re-attach. After such an event the existing ICE agent is bound to a departing local interface and pion's pair-checks fail. The exponential backoff (PR-A) then kicks in and the peer stays on relay for minutes. Two related fixes are needed: 1. **Re-attach ICE after a network change** — recreate the ICE worker on a fresh socket, send a new offer. (Phases 3.5, 3.7d, 3.7e.) 2. **Short retry delay during the post-network-change grace window** — the first pair-check often fails on stale mappings even when subsequent ones succeed. Falling back to the normal 1-minute initial backoff after that single failure leaves the peer on relay far longer than the underlying connectivity warrants. A 60-second grace window with 2-second retries lets ~4-5 attempts run while the new mapping primes. (Phases 3.7f, 3.7h.) Plus: previously closed PR #5805 ("Fix ICE reconnection loop and buffer candidates before agent initialization") had a known reconnect-loop where ICE worker rebuild on every signal-state change racked up offers faster than pion could complete pair-checks. The clean fix is to skip new offers while the ICE agent is still in `Connecting`. ## What's in this PR ### Phase 3.5 — Reset on network change - `client/peer/conn`: reset ICE backoff + recreate `workerICE` when the signal/relay layer reconnects after a network change. - `client/peer/conn`: refactor `onNetworkChange` to use the in-place agent recreate path; drop `SendOffer` from `onNetworkChange` to fix offer-storm; revert and replace an interim `worker_ice` candidate-buffering attempt with a clean approach. ### Phase 3.7d — Signal-trigger re-attach - Re-attach ICE on every signal trigger so a peer that lost ICE while the daemon was Disconnected from signaling resumes correctly when signaling is back. ### Phase 3.7e — Listener inside onNetworkChange - Re-attach the ICE listener inside `onNetworkChange` itself so a network change fires a fresh listener registration without waiting for the next activity event. ### Phase 3.7f + 3.7h — Grace window - `iceBackoff.markFailure`: while inside `networkChangeGracePeriod` (60s) of the most recent `Reset()`, cap the suspend delay at `networkChangeRetryDelay` (2s) and do NOT advance the long-term exponential schedule. Outside the window the normal 1-minute initial backoff applies. - Window widened from initial 30s to 60s and delay reduced from 5s to 2s after observing real-world LTE-bounce behaviour: cold NAT mappings often need 3-4 ICE attempts to prime, and the previous 30s window only fit ~2 attempts. ### Phase 3.7g — Skip ICE-Close on NC when still connected - If ICE is still in `Connected` state when the network-change fires, don't call `workerICE.Close()` — the existing pair is fine, just refresh the listener. ### #5805 cleanup — Guard-Loop fix - `worker_ice`: skip new offers while the ICE agent is in `Connecting`. Drops the candidate-buffer approach the closed PR proposed in favour of a much simpler offer-skip during connecting. ### GUI surface (Phase 3.7h GUI) - `client/cmd/service`: persist connection-mode + timeouts on install/reconfigure. - `client/ui`: Connection Mode + timeouts in Network tab. - `client`: surface server-pushed connection-mode/timeouts via daemon-RPC. - `client/ui`: "Follow-Server (currently: …)" display + remove the now-redundant Lazy menu. - `client/android`: gomobile getters for ConnectionMode + ServerPushed values. ## Tests - `go test ./client/internal/peer ./client/internal/peer/guard` — pass (full ICE-backoff suite including the new grace-window cases). - `go build ./client/...` — pass on linux/amd64, linux/arm64, windows/amd64. - Hardware-validated against three OpenWrt routers (LAN host candidate, internet srflx, LTE srflx) plus a Windows 11 daemon. After a forced LTE-modem reset, the LTE peer recovers to P2P srflx within ~50 s instead of the 2-3 minutes the pre-3.7f code took. ## Test plan - [x] Grace window: failure inside 60s → 2s retry; failure after 60s → exponential restarts at ~1m. - [x] No regression for the non-network-change failure path (still uses the exponential schedule). - [x] Signal/relay reconnect drives `Reset()` → grace window engages. - [ ] Maintainer to verify: GUI changes on macOS / Linux ARM (I tested Windows + Linux/amd64 only). ## Use case A laptop that switches between docked Ethernet, office WiFi, and tethered LTE during the day. Without this PR every transition burns the exponential backoff and the user sees "stuck on relay" for 5-15 minutes. With this PR the post-handover recovery is bounded at ~50s. ## Linked work - Stacks on PR-A (Phase 1+2+3 foundation). - Picks up the offer-skip-during-connecting fix from closed #5805 (the original PR's candidate-buffer approach is replaced with the simpler pattern). - Closes part of #5989. **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 Connection Modes (relay-forced, p2p, p2p-lazy, p2p-dynamic) with per-mode timeouts, local overrides, and server-pushed defaults surfaced to clients. * ICE failure backoff with visible status (suspended, failures, next-retry) and per-peer retry cap; settings persist via profile/install. * **UI** * Network/Settings UI updated to manage connection mode, timeout fields, and show server-provided hints. * **Tests** * Expanded coverage for mode resolution, inactivity timers, ICE backoff, and related flows. <!-- 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#24788