Repository navigation
feat(wireguard): support configurable server interface MTU - #90
Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe change adds optional WireGuard MTU configuration. Values are validated between 576 and 9000. The manager applies MTU values during interface initialization and configuration restart, while tracking the kernel default for restoration. ChangesWireGuard MTU management
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant WireGuard
participant Manager
participant netlink
WireGuard->>Manager: initializeWithPeers(..., mtu)
Manager->>netlink: Apply interface MTU
netlink-->>Manager: Return MTU result
Manager-->>WireGuard: Return initialization result
WireGuard->>Manager: applyConfig(config, mtu)
Manager->>netlink: Apply interface MTU
netlink-->>Manager: Return MTU result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. A rabbit sets the MTU with care Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
The release lead holds upstream 52d5aab (grpc 1.84.0, PasarGuard#91) out of 0.6.21, so the node stays on grpc 1.83.2. The Go vulnerability database still lists released v1.84.0 as affected by GO-2026-6443 (golang/vulndb #6511, #6580), although the fix is verified in v1.84.0 http2_server.go, so the bump would make govulncheck fail. It is waived as pending until the database entry is corrected. docs/upstream-parity.md said 0.6.21 would port it; that row now records the hold. So that pending waivers cannot be forgotten, scripts/upstream_drift.sh now ends every run with a PENDING WAIVERS (N) block. The block lists each waived item whose category is pending, across node, xray, sing-box, sing-quic and mtg, with its component, id and reason. Pending items stay waived; the exit code rules are unchanged. The state section names the node heads checked on 2026-09-24. Upstream dev moved past be2c445 to 4a9d0cf, which adds PasarGuard#90 (configurable WireGuard server interface MTU). That commit is new, not assessed, and flagged by the check. In the run with this change, the block lists 52 pending items, and 3 items remain UNPORTED: be2c445 and 4a9d0cf on node, and the new sing-box release v1.14.2 (af6e64c3b69e).
Tests for the port of upstream PasarGuard#90. An optional mtu config key accepts 576 to 9000 and nothing else. The MTU is set at New after the interface is configured and before it comes up, and again at Restart and UpdateUsersAndRestart when the kernel drifted from it; a full sync does not touch it. The manager captures the kernel default only at the first override and restores it when the override is cleared. An out-of-range MTU fails New before any manager or interface exists; an MTU the kernel refuses fails New with the interface removed; both leave the accounting hand-over for the next backend. An MTU set again at Restart, or refused there, keeps every per-user and interface counter, and the interleaving test also runs with an MTU and a drifting kernel value. A config without the key must behave as before: the fake link now logs every operation, and TestWireGuardConfigWithoutMTUTouchesTheLinkAsBefore pins the exact sequence of New, Restart, full sync, UpdateUsersAndRestart, a tick, a node-level read and Shutdown. It passed on a2c63c2 before the fake manager was given the MTU hook. RED on a2c63c2: the package does not build (Manager has no setLinkMTU, Config has no MTU, no initializeWithPeers or applyConfig).
…d#90) Port of upstream PasarGuard/node dev 4a9d0cf. The WireGuard core config takes an optional mtu between 576 and 9000. New sets it on the created interface before the interface comes up, Restart sets it again when the kernel value drifted, and a value equal to the kernel value makes no call. The manager captures the kernel default only when an override is first applied and restores it when the override is cleared. Without the key, or with it null, the node makes no MTU call at all. config.go and manager.go are identical to upstream, including its two comment lines on applyMTULocked, kept as upstream text. Adaptations to the fork's wireguard.go: - restartLocked: the fork samples every peer before the kernel replaces the peer list and restarts the counters only after a successful apply. Upstream's hunk swaps ApplyConfig for applyConfig on a line that has no flush in front of it; here the flush stays first and applyConfig(config, cfg.MTU) replaces ApplyConfig, so the MTU is set after the flush and before the peer replacement. A refused MTU returns before the replacement and restarts no counter, and the flush stays an ordinary sample. - newWithManagerFactory: no change beyond upstream's. Its MTU check sits before the manager exists, and a kernel that refuses the MTU fails initializeWithPeers, which removes the interface; both return before the fork takes the accounting hand-over, so the hand-over stays for the next backend (FR-009b). - The fork's version probe timeout, accounting hand-over and serialised Shutdown are untouched. Upstream's co-author trailer is not carried; fork commits carry none. A config that has an mtu outside 576-9000, which the node ignored before, is now refused by NewConfig. Upstream: 4a9d0cf
…ift rules - PasarGuard#93 (be2c445) is ported by 40006fe, and PasarGuard#90 (4a9d0cf) by f9c4f05, merged into release/v0.6.21 at 01eed71. Both rows and the node state now say so, and the upstream dev head in the branch table is 4a9d0cf. - The drift-check section describes the two new item kinds: fork commits with an upstream parent, which must be allowlisted as allowed-merge (the ours-merge hole), and upstream merges whose combined diff is not empty, which need a port or a waiver. - The waiver section describes the allowed-merge category and the until:/issue: requirement for pending waivers, what happens after the date, and DRIFT_TODAY.
Summary
WireGuard core configs currently ignore
mtu, so changing it in the panel does not change the server interface. Accept an optional integer MTU (576–9000) and apply it before the interface is brought up and during in-place restarts.Keep the kernel default when MTU is omitted. Capture the interface MTU before the first override so clearing the setting restores it. Reject invalid values before interface initialization and propagate netlink failures through the existing permission-error handling.
Related to PasarGuard/panel#906. Companion panel PR: PasarGuard/panel#917.
Validation
Verified on Linux against this PR's production code using a separate fork branch: https://github.com/dr-hoseyn/node/actions/runs/35507877164
go test -race ./backend/wireguard.go build ./cmd/nodeandgit diff --check.make testwas also run but is not green: the unchanged controller tests callkeepAliveStalewith three arguments while the dev implementation accepts two; Xray and REST/RPC tests require the Xray binary and TLS fixtures absent from this runner. The verification workflow allows that full-suite step to fail so the independent WireGuard checks can complete.Only three production Go files are included. Verification-only tests and workflow are in a separate branch; no test or documentation files are part of this PR.
Summary by CodeRabbit
New Features
Bug Fixes