[PR #6825] [CLOSED] [client] lazyconn: preserve routed prefixes on idle→wake to fix subnet black-hole #29069

Open
opened 2026-08-05 08:07:24 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6825
Author: @MichaelUray
Created: 7/19/2026
Status: Closed

Base: mainHead: fix/lazyconn-routed-prefix-idle-wake


📝 Commits (1)

  • c44101e [client] lazyconn: preserve routed prefixes on idle→wake to fix subnet black-hole

📊 Changes

2 files changed (+400 additions, -7 deletions)

View changed files

📝 client/internal/lazyconn/manager/manager.go (+119 -7)
client/internal/lazyconn/manager/manager_routewake_test.go (+281 -0)

📄 Description

Describe your changes

When a routing peer goes idle under lazy connections, its WireGuard peer is torn
down, which also drops the routed subnet prefixes the route watcher had installed.
The activity listener then re-arms the peer's wake endpoint with only the overlay
/32 (PeerConfig.AllowedIPs), so traffic destined for the routed subnet no longer
matches the peer's AllowedIPs and is black-holed until the peer is woken by other
means (e.g. a ping to its overlay IP).

The asynchronous route-watcher re-add via AddAllowedIP uses UpdateOnly=true;
when it races into the window where the peer does not exist, wireguard-go treats
update-only on a missing peer as a silent no-op, so no self-heal occurs and the
subnet stays out of AllowedIPs.

Fix

Capture each routing peer's static routed prefixes in UpdateRouteHAMap and merge
them into the peer's AllowedIPs at activity-listener arm time, so the wake endpoint
covers both the overlay /32 and the routed subnets. The merge is applied to a copy
while the stored PeerConfig keeps its pristine overlay-only base; computing the
arm-time set fresh from the current route sync prevents stale prefixes from
accumulating across route changes. This matters because WireGuard AllowedIPs are
peer-exclusive: a stale prefix would let an idle peer re-claim a subnet that has
since moved to another HA peer, breaking failover.

Prefixes are gathered for every route, including single-peer (non-HA) routes,
covering both the cold-start and route-change paths. Dynamic (domain) routes are
skipped as they have no fixed subnet to pre-install.

Testing

Adds manager_routewake_test.go (unit): verifies routed prefixes are merged into
the wake endpoint at arm time for HA and single-peer routes, that the stored
PeerConfig stays overlay-only, and that stale prefixes do not persist across route
changes. go test ./client/internal/lazyconn/... -race green.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes

    • Improved wake-up behavior for routed connections by including the appropriate routed subnet information.
    • Updated wake settings correctly when routes change, preventing stale routes from being used.
    • Ensured each connection receives only its own routed networks.
    • Excluded dynamic or domain-based routes from wake-up configuration.
    • Improved cleanup when connections are removed or the manager shuts down.
  • Tests

    • Added coverage for route changes, multiple routed connections, dynamic routes, and cleanup scenarios.

🔄 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/6825 **Author:** [@MichaelUray](https://github.com/MichaelUray) **Created:** 7/19/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/lazyconn-routed-prefix-idle-wake` --- ### 📝 Commits (1) - [`c44101e`](https://github.com/netbirdio/netbird/commit/c44101e76866242b1214b58c473881165ab879af) [client] lazyconn: preserve routed prefixes on idle→wake to fix subnet black-hole ### 📊 Changes **2 files changed** (+400 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `client/internal/lazyconn/manager/manager.go` (+119 -7) ➕ `client/internal/lazyconn/manager/manager_routewake_test.go` (+281 -0) </details> ### 📄 Description ## Describe your changes When a routing peer goes idle under lazy connections, its WireGuard peer is torn down, which also drops the routed subnet prefixes the route watcher had installed. The activity listener then re-arms the peer's wake endpoint with only the overlay /32 (`PeerConfig.AllowedIPs`), so traffic destined for the routed subnet no longer matches the peer's AllowedIPs and is black-holed until the peer is woken by other means (e.g. a ping to its overlay IP). The asynchronous route-watcher re-add via `AddAllowedIP` uses `UpdateOnly=true`; when it races into the window where the peer does not exist, wireguard-go treats update-only on a missing peer as a silent no-op, so no self-heal occurs and the subnet stays out of AllowedIPs. ## Fix Capture each routing peer's static routed prefixes in `UpdateRouteHAMap` and merge them into the peer's AllowedIPs at activity-listener arm time, so the wake endpoint covers both the overlay /32 and the routed subnets. The merge is applied to a copy while the stored `PeerConfig` keeps its pristine overlay-only base; computing the arm-time set fresh from the current route sync prevents stale prefixes from accumulating across route changes. This matters because WireGuard AllowedIPs are peer-exclusive: a stale prefix would let an idle peer re-claim a subnet that has since moved to another HA peer, breaking failover. Prefixes are gathered for every route, including single-peer (non-HA) routes, covering both the cold-start and route-change paths. Dynamic (domain) routes are skipped as they have no fixed subnet to pre-install. ## Testing Adds `manager_routewake_test.go` (unit): verifies routed prefixes are merged into the wake endpoint at arm time for HA and single-peer routes, that the stored PeerConfig stays overlay-only, and that stale prefixes do not persist across route changes. `go test ./client/internal/lazyconn/... -race` green. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6825"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787054567&installation_id=146802194&pr_number=6825&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6825&signature=40d0c57db5294410ed644c096386fab78b8576c308fda3dbaf8adb707cb442ad"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved wake-up behavior for routed connections by including the appropriate routed subnet information. * Updated wake settings correctly when routes change, preventing stale routes from being used. * Ensured each connection receives only its own routed networks. * Excluded dynamic or domain-based routes from wake-up configuration. * Improved cleanup when connections are removed or the manager shuts down. * **Tests** * Added coverage for route changes, multiple routed connections, dynamic routes, and cleanup scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <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 08:07:24 -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#29069