Fix WireGuard startup error handling - #2377
Conversation
PR Summary by QodoFix WireGuard startup during temporary DNS outages
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo
1.
|
openipc-ai
left a comment
There was a problem hiding this comment.
Thanks — the shape is right. A hostname endpoint costs wg setconf about 2.5 minutes of retries at boot and then fails for good, so waiting for the name outside the boot path is the correct fix, and the literal-IPv4 bypass is necessary: busybox nslookup 1.2.3.4 is a reverse lookup and returns 1 even with DNS working. I checked the regex and the nslookup exit codes under the same busybox 1.36.1/musl the image ships. The shell parse and strip tests pass on this head.
One thing to fix before merge (inline): a bracketed IPv6 endpoint never starts. The rest is take-or-leave.
Two points that are not code:
- Title: the tree uses
area: lowercase imperative summary, and maintainers squash with the PR title.wireguard: wait for the endpoint to resolve instead of failing at bootsays what changed; "error handling" does not. - Evidence: the before-log in #2319 is good. A pasted
wg show wg0from a camera that booted with DNS unreachable and then recovered would close the loop.
On Qodo's review: the digit-prefixed-hostname half of its first finding was fixed by your second commit (the regex sends 3cam.example.com to the resolver correctly). A DNS relapse between the probe and wg setconf is covered by wg's own 15 retries. stop doing nothing, and a second start failing at ip link add, were both true before this change.
For the record, WG_ENDPOINT_RESOLUTION_RETRIES=infinity on wg setconf (the shipped wireguard-tools supports it) was considered: it gives up immediately on NXDOMAIN and REFUSED, which a router's resolver can return while its WAN is down, so your loop is the more robust shape.
CI has not run yet because the fork's workflow runs are waiting for maintainer approval.
Co-authored-by: OpenIPC AI team <ai@openipc.org>
|
Thanks for applying the fast-fail on an empty Two items from the review are still open on 0622cd3:
Once the first lands I will re-review. |
openipc-ai
left a comment
There was a problem hiding this comment.
Thanks, the IPv6 case is fixed. Measured with the new pattern under musl grep -E: [2001:db8::1] and [::1] skip the wait, 1.2.3.4 skips, vpn.example.com and 1abc.example.com wait. Both scripts still parse under busybox sh -n, before and after comment stripping.
The start-stop-daemon change in 0c8e7fe is the new blocker; details and a suggestion to revert it are inline on S98wireguard.
Still open from the first review: a sentence in the description that wg0 now comes up asynchronously, so rc.local and anything else at boot can no longer assume it exists.
CI on this fork is held for maintainer approval; it will be run once the init script is back to its previous form.
openipc-ai
left a comment
There was a problem hiding this comment.
Everything from both reviews is in: the IPv6 guard, the empty-endpoint fast-fail, S98wireguard back to wireguard & with the empty stop, and the description now says wg0 comes up asynchronously. Both scripts parse under the busybox sh this tree builds, before and after comment stripping.
I brought the branch up to date with master (the protected branch requires it) and released the fork's CI runs. This merges once the board matrix is green.
|
I don't think |
Problem
WireGuard can fail to start after a reboot when the peer endpoint is specified as a hostname and DNS is temporarily unavailable. In that case, the camera can end up without a working WireGuard connection.
Solution
The WireGuard script now waits for DNS to become available when the endpoint is specified as a hostname, while IPv4 and IPv6 literal endpoints skip the DNS wait. An empty
wg_endpointfails immediately with an error.WireGuard is started asynchronously so that a DNS delay does not block the boot process.
rc.localand other boot-time scripts should not assume thatwg0is immediately available.Hardware tested on
The fix has been tested for more than a week on these platforms.
Evidence
See OpenIPC/firmware#2319 for the problem reproduction, testing results, and discussion.
Scope
general/package/all-patches/linux/(those go to OpenIPC/linux)general/overlay/or in a sharedload_<vendor>script hardcodes a value specific to my boardLD_PRELOAD, and no binaries that cannot be rebuilt from source