Skip to content

[client] Eliminate packet loss during lazy connections.#6355

Merged
pappz merged 4 commits into
mainfrom
feature/immediate-handshake-on-endpoint-change
Jun 28, 2026
Merged

[client] Eliminate packet loss during lazy connections.#6355
pappz merged 4 commits into
mainfrom
feature/immediate-handshake-on-endpoint-change

Conversation

@pappz

@pappz pappz commented Jun 7, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

Updated WireGuard dependency with a patch and removed the RemovePeer call on lazy activity detection to force a new handshake initiation to the updated endpoint. This also flushed the staged queue, dropping the first packet.

Since UpdatePeer (called after ICE/relay negotiation) triggers SendStagedPackets via IpcSet/handlePostConfig, the peer removal is no longer necessary. The staged packet survives and the handshake is initiated on the real endpoint automatically.

This also eliminates the transient state where the peer's endpoint and routes were absent between the lazy idle and connected states.

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Improved connection cleanup during disconnects to make shutdowns more reliable and avoid unnecessary error handling.
    • Updated the bundled WireGuard dependency to a newer revision for compatibility and stability.

@pappz
[client] Remove peer deletion on lazy activity detection
af3e9d1 
Updated WireGuard dependency with a patch and removed the RemovePeer
call on lazy activity detection to force a new handshake initiation
to the updated endpoint. This also flushed the staged queue, dropping
the first packet.

Since UpdatePeer (called after ICE/relay negotiation) triggers
SendStagedPackets via IpcSet/handlePostConfig, the peer removal is
no longer necessary. The staged packet survives and the handshake
is initiated on the real endpoint automatically.

This also eliminates the transient state where the peer's endpoint
and routes were absent between the lazy idle and connected states.
@coderabbitai

coderabbitai Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2b982546-63cb-4670-a5bd-c6af9e725283

📥 Commits

Reviewing files that changed from the base of the PR and between f7c7248 and 808c371.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • go.mod
🚧 Files skipped from review as they are similar to previous changes (1)
  • go.mod
📝 Walkthrough

Walkthrough

Removes the wgIface.RemovePeer call (and its error log) from the BindListener.ReadPackets shutdown path, leaving only lazyConn close, bind endpoint removal, and done.Done(). Also bumps the wireguard-go replace directive in go.mod to a newer pseudo-version.

Lazy-conn shutdown and dependency update

Layer / File(s) Summary
Remove wgIface.RemovePeer from BindListener shutdown and bump wireguard-go
client/internal/lazyconn/activity/listener_bind.go, go.mod		 Drops wgIface.RemovePeer and its error-log branch from the post-select cleanup in BindListener.ReadPackets; updates the golang.zx2c4.com/wireguard replace directive to a newer wireguard-go pseudo-version.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • netbirdio/netbird#6028: Modifies the same BindListener flow in client/internal/lazyconn/activity, changing createListener and tests to use BindListener.

Poem

🐇 Hop, hop, the peer removal's gone,
No more RemovePeer to linger on!
The lazy conn just shuts the door,
Cleans the bind and nothing more.
A fresher wireguard tag in tow—
Watch that rabbit code-yard grow! 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description omits required sections like the change details, issue link, and stack, so it is largely incomplete. Add the missing template sections with a clear change summary, issue ticket/link, stack info, and any relevant notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: removing packet loss in lazy connections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/immediate-handshake-on-endpoint-change

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown

Release artifacts

Built for PR head 808c371 in workflow run #16238.

Artifact Link
All release artifacts Download
Linux packages Download
Windows packages Download
macOS packages Download
UI artifacts Download
UI macOS artifacts Download

GHCR images (amd64)

No GHCR images were pushed.

This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy.

@lixmal lixmal marked this pull request as ready for review June 23, 2026 16:32
lixmal
lixmal previously approved these changes Jun 23, 2026
View reviewed changes
@pappz pappz merged commit 62f5467 into main Jun 28, 2026
47 checks passed
@pappz pappz deleted the feature/immediate-handshake-on-endpoint-change branch June 28, 2026 12:22
@sonarqubecloud

sonarqubecloud Bot commented Jun 28, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lixmal lixmal lixmal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pappz @lixmal