[PR #6419] [client] Recover from tun device read/write panics and restart the client #28187

Closed
opened 2026-08-05 08:05:56 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/6419

State: closed
Merged: Yes


Describe your changes

Recover from panics in the underlying tun device read/write calls and restart the client instead of crashing the daemon.

On Windows, third-party filter drivers can place zero-length packets in the wintun ring. The wintun binding panics on these (&packet[0] on an empty slice), which today takes down the whole daemon process. The wintun session cannot be repaired in place after this, since the unreleased ring slot permanently blocks the ring head, so the interface has to be recreated.

  • Recover panics from the underlying device Read/Write and convert them to errors so the wireguard-go read loop shuts down cleanly
  • Trigger a client restart on recovery so the interface is recreated with a fresh tun session
  • Keep the recover scoped to the device call only, so panics in the packet filter or capture code still propagate normally instead of being masked

The recover is a single open-coded defer around the device call. Measured overhead on the read hot path is under ~10 ns/op with zero extra allocations, negligible against the per-packet syscall and MTU-sized copy already on that path.

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 (internal resilience fix, no user-facing behavior change)

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 client stability by intercepting and recovering from underlying device panics instead of crashing; dropped packet counts are handled correctly and recovery can trigger a graceful restart when needed.
  • Tests
    • Added tests validating panic recovery for device read/write paths and confirming the recovery callback is invoked.
**Original Pull Request:** https://github.com/netbirdio/netbird/pull/6419 **State:** closed **Merged:** Yes --- ## Describe your changes Recover from panics in the underlying tun device read/write calls and restart the client instead of crashing the daemon. On Windows, third-party filter drivers can place zero-length packets in the wintun ring. The wintun binding panics on these (`&packet[0]` on an empty slice), which today takes down the whole daemon process. The wintun session cannot be repaired in place after this, since the unreleased ring slot permanently blocks the ring head, so the interface has to be recreated. - Recover panics from the underlying device Read/Write and convert them to errors so the wireguard-go read loop shuts down cleanly - Trigger a client restart on recovery so the interface is recreated with a fresh tun session - Keep the recover scoped to the device call only, so panics in the packet filter or capture code still propagate normally instead of being masked The recover is a single open-coded `defer` around the device call. Measured overhead on the read hot path is under ~10 ns/op with zero extra allocations, negligible against the per-packet syscall and MTU-sized copy already on that path. ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [x] 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](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (internal resilience fix, no user-facing behavior change) ### 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/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved client stability by intercepting and recovering from underlying device panics instead of crashing; dropped packet counts are handled correctly and recovery can trigger a graceful restart when needed. * **Tests** * Added tests validating panic recovery for device read/write paths and confirming the recovery callback is invoked. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
saavagebueno added the pull-request label 2026-08-05 08:05:56 -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#28187