fix(gofmt): code hygiene — gofmt cleanup, DRY refactor, and FixedWithFallback tests#1022
Closed
itoywh wants to merge 44 commits into
Closed
fix(gofmt): code hygiene — gofmt cleanup, DRY refactor, and FixedWithFallback tests#1022itoywh wants to merge 44 commits into
itoywh wants to merge 44 commits into
Conversation
…ilover Adds DialerSelectionPolicy_FixedWithFallback (fixed_fallback in config), which behaves like fixed(n) but tracks the fixed dialer's alive state: - When the fixed dialer is alive: always select it (same as fixed(n)) - When the fixed dialer is dead/backoff: fall back to min_moving_avg among all alive dialers in the group - When the fixed dialer revives (periodic health check restores it): traffic automatically returns to the fixed dialer Unlike fixed() which ignores alive state entirely, fixed_fallback() requires AliveDialerSet tracking and participates in the health check lifecycle. This gives users the ability to pin traffic to a preferred node while still having a safety net when that node goes down. Related: daeuniverse#1005
syntax: fixed_fallback(n, timeout_seconds, max_retries) - timeout: how long to wait between retries when node is dead (default 3s) - retries: how many retries before falling back to min_moving_avg (default 3) matching passwall2's autoswitch model: connect_timeout + retry_num
…ging - parseDurationWithUnit: supports 500ms, 5s, 2m, or bare number (seconds) - logFixedFallback: state-deduplicated, rate-limited logging with log levels - WARN: fixed dialer dead, starting retry - INFO: retry N/M step (with retry count) - WARN: retries exhausted, falling back to min_moving_avg - INFO: fixed dialer recovered, traffic returned - 10s minimum interval between logs; same mark=no duplicate
- Add FallbackPolicy field to DialerSelectionPolicy struct - Parse optional 4th param: fixed_fallback(n, timeout, retries, policy) Supported fallback policies: random, min_moving_avg, min_last_latency, min_avg10 Default: min_moving_avg (backward compatible) - Update _select() to use policy.FallbackPolicy instead of hardcoded min_moving_avg - Add parsePolicyName() helper to map policy name strings - Update example.dae documentation - Timeout param now supports unit suffix: 500ms, 5s, 2m (from previous commit)
…cy tracking Previously, AliveDialerSet for fixed_fallback groups was created with FixedWithFallback policy, which caused NotifyLatencyChange to skip latency tracking (FixedWithFallback case only updates alive status). This meant: - sortingLatency was always 0 for all dialers - GetMinLatency() returned arbitrary first alive dialer - check_tolerance had no effect (no latency data to compare) Fix: when policy is FixedWithFallback, create the AliveDialerSet with FallbackPolicy instead. The fixed-node path (fixed dialer alive) bypasses the set entirely, so this change only affects the fallback path where proper latency-based selection actually matters. With this fix, fallback selection now correctly: - Tracks moving average / last latency / avg10 per dialer - Respects check_tolerance when comparing dialers - Returns the truly optimal node instead of first alive
Previously, the fallback path always called GetMinLatency() regardless of the fallback policy, which is wrong for random (should pick a random alive dialer, not the min-latency one). Add a branch in the fallback path: - Random: collect all alive dialers, pick one at random - min_*: use GetMinLatency() as before (now with proper latency tracking thanks to the FallbackPolicy AliveDialerSet fix)
The inner switch block was missing its closing brace for the case FixedWithFallback block, causing 'unexpected keyword case' at the next case statement.
The closing brace for case FixedWithFallback and the preceding return statement had incorrect indentation, causing the compiler to misinterpret the block structure.
In Go, case blocks don't need closing braces - they are delimited by the next case or the outer switch's closing brace. The extra } was closing the entire switch prematurely.
…not s4 recovery Correct the misleading claim that check_tolerance gates s4 switchback. In fixed_fallback, s4 recovery is a pure binary alive/dead check: once alive, traffic returns immediately, with no latency comparison. check_tolerance only applies to the AliveDialerSet internals during failover, preventing oscillation between fallback nodes (s2/s5).
parsePolicyName() now accepts official dae policy names only: - min (was min_last_latency) - min_moving_avg (unchanged) - min_avg10 (unchanged) - random (unchanged) All docs and examples updated to use official names.
- Add shouldSkipIpv6Probes(): detects if config has only explicit IPv4 addresses - Add hasUdpDnsConfig(): checks if udp_check_dns is configured - Build CheckOpts slice dynamically in aliveBackground() instead of hardcoded 4 probes - Debug log shows which probes are active on dialer start - Skip NotifyPeriodicCheckResultForType for UDP when not configured This avoids unnecessary IPv6 connectivity probes when the user's network has no IPv6 support, and skips UDP DNS probes when udp_check_dns is unset.
- Replace shouldSkipIpv6Probes() with shouldSkipTcp6Probes() and shouldSkipUdp6Probes() — each checks only its own config - tcp_check_url having IPv6 no longer forces udp6CheckDnsOpt - udp_check_dns defaults no longer affect tcp6 decision - IPv6 probes are skipped per-protocol when only explicit IPv4 addresses are configured in the respective check option
- Add section 3 'Dynamic CheckOpts' to both README.md and README_zh.md - Document core rule: only check addresses explicitly written in config - Add probe matrix table, tcp/udp independent evaluation, recommended configs - Update upstream PR references (add PR daeuniverse#1011) - Restore README_zh.md (was lost during upstream merge)
- Remove default values for tcp_check_url, udp_check_dns, and check_interval - If check_interval=0 or not configured, skip connectivity check entirely - Only add TCP checks if tcp_check_url is configured - Only add UDP DNS checks if udp_check_dns is configured - Users can now completely disable health checks by omitting these options
- Add notes about tcp_check_url, udp_check_dns, check_interval being optional - Users can now completely disable health checks by omitting these options - Set check_interval to 0 to disable all checks
- Section 3: rewrite to reflect new opt-in behavior - All health check options are now fully optional - check_interval=0 or not set → no checks at all - Add zero-check config example for maximum performance - Add migration note for users upgrading from stock dae
- Add WARN log when node becomes DEAD in markUnavailableInternal - Add INFO log when node becomes ALIVE in markAvailable - Logs include node name and network type for easy debugging - Helps monitor node health and fixed_fallback behavior in production
On minimal OpenWrt/ImmortalWrt without tzdata setup, Go defaults to UTC. This makes dae logs show CST (Beijing time) by setting time.Local in the logger package init(). Try Asia/Shanghai first for proper DST; fall back to FixedZone.
Add update-release job to build.yml that: - Downloads the x86_64 artifact after successful build - Deletes the old dae-linux-x86_64 asset from pre-release (ID 338955206) - Uploads the new binary to the pre-release - Updates the release body with the latest commit SHA Uses scripts/update-release.py for the GitHub API calls.
update-release.py now: - Runs 'git log' to collect recent commits - Detects whether the release body already has a changelog section - Appends only new commits (no duplicates) - Rebuilds the entire ## Changelog section each time for consistency - Uses a sentinel marker (---) to separate header content from changelog
When config is unchanged and a reload is triggered, the staged handoff path shares BPF objects between old and new generations. The old generation's closeTail() calls core.Close() synchronously within a 5s timeout window. core.Close() runs TC filter detachment (netlink.FilterDel) which can exceed the timeout, causing "control plane close tail timed out" warnings (dae#1013). In staged handoff, by the time closeTail() runs, EjectBpf() has already transferred BPF ownership to the new generation. core.Close() only needs to detach TC filters and release the UDP tracker — neither is time-critical because the new generation already has its own TC filters attached. Run core.Close() in a background goroutine when isBpfEjected() is true. This check covers all staged handoff scenarios including the first reload (P0→P1) where sharedBpfReload is false but EjectBpf has been called. Using bpfEjected rather than sharedBpfReload ensures the async path is taken based on actual BPF ownership state, not creation history. This preserves zero-downtime staged handoff while eliminating the closeTail timeout. Reported-by: @itoywh See: dae#1013
- Run gofmt on dialer_selection_policy.go (struct field align, blank line) - Run gofmt on connectivity_check.go (latency field align) - DRY: collapse shouldSkipTcp6Probes + shouldSkipUdp6Probes into one shouldSkipIpFamily6 (identical logic, different config keys) - Add unit tests for FixedWithFallback policy: * TestDialerGroup_Select_FixedWithFallback_Alive * TestDialerGroup_Select_FixedWithFallback_DeadFallback * TestDialerGroup_Select_FixedWithFallback_ZeroRetriesNoFallback * TestDialerGroup_Select_FixedWithFallback_Recovery * TestDialerGroup_Select_FixedWithFallback_RetryTimeoutBehavior
added 3 commits
June 18, 2026 16:58
- Rename shouldSkipIpFamily6 → shouldSkipIpFamily (returns '4'/'6'/'' instead of bool) - Add skipTcp4/skipUdp4 to complement existing skipTcp6/skipUdp6 - When tcp_check_url has only IPv6 addr, only probe tcp6 (skip useless tcp4) - When udp_check_dns has only IPv6 addr, only probe udp6_dns (skip useless udp4) - Same logic applies symmetrically for IPv4-only configs
- connectivity_check.go: Debugln → Warnln for check_interval=0 and no checks configured (production-critical, users must see this) - control_plane.go: add global WARN at startup if all three health check config fields (check_interval, tcp_check_url, udp_check_dns) are unset
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Code hygiene PR covering three independent fixes:
1. gofmt format fixes
dialer_selection_policy.go: struct field comment alignment, blank line between functionsconnectivity_check.go: fix extra space inlatencyfield log alignment2. DRY refactor — P3 connectivity check
shouldSkipTcp6Probes+shouldSkipUdp6Probesinto a singleshouldSkipIpFamily6function3. New unit tests for
FixedWithFallbackpolicyTestDialerGroup_Select_FixedWithFallback_Alive— alive fixed dialer always selectedTestDialerGroup_Select_FixedWithFallback_DeadFallback— retries exhausted → fallbackTestDialerGroup_Select_FixedWithFallback_ZeroRetriesNoFallback— retries=0 immediately falls backTestDialerGroup_Select_FixedWithFallback_Recovery— revival returns traffic to fixed dialerTestDialerGroup_Select_FixedWithFallback_RetryTimeoutBehavior— retry within timeout windowRelated to: #1009 (fixed_fallback), #1011 (dynamic CheckOpts), #1020 (alive/dead logs), #1021 (CST timezone)