Skip to content

mwan3: strip dead flag from copied routes - #30390

Open
dormancygrace wants to merge 1 commit into
openwrt:masterfrom
dormancygrace:codex/mwan3-strip-dead-route-flag
Open

mwan3: strip dead flag from copied routes#30390
dormancygrace wants to merge 1 commit into
openwrt:masterfrom
dormancygrace:codex/mwan3-strip-dead-route-flag

Conversation

@dormancygrace

Copy link
Copy Markdown

Description

Routes for devices without carrier may be reported as:

192.0.2.0/24 dev eth0 scope link src 192.0.2.1 dead linkdown

mwan3 removes linkdown before copying the route to its policy tables,
but retains dead. ip route replace table ... rejects that token with:

Error: either "to" is duplicate, or "dead" is garbage.

The connected route is then missing from the WAN policy tables. Marked reply
traffic follows the table's default route back to the WAN instead of reaching
the LAN client.

Strip both flags, whether followed by whitespace or present at the end of the
line. Bump the package release because an installed support file changes.

Testing

  • Added a runtime test for dead and linkdown, individually and in both
    orders.
  • Ran the test successfully on OpenWrt 25.12.5 with BusyBox 1.37.0.
  • Reproduced missing policy-table routes before the patch and confirmed that
    routes for link-down devices populate tables 1, 2, and 3 after the patch.
  • Confirmed affected client TCP sessions can establish after restarting
    mwan3 with the patch applied.
  • Built package/mwan3/compile successfully with the OpenWrt 25.12.5 SDK.

@dormancygrace
dormancygrace force-pushed the codex/mwan3-strip-dead-route-flag branch from 18f1e6c to 9546df4 Compare August 27, 2026 11:54
@BKPepe
BKPepe requested a review from feckert August 27, 2026 15:11

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 new commit. The diagnosis is right — ip route replace does reject dead, and stripping it at end-of-line as well as mid-line is the correct scope. Two findings plus one optional nit.

The only one I would treat as blocking is the unanchored s/dead //: dead is valid hex, so the substitution also fires inside IPv6 address tokens and silently rewrites via fe80::dead dev eth0 into via fe80::dev eth0. The suggested anchored form keeps every assertion in the new test.sh passing unchanged, so it should be a drop-in.

The set -e placement in test.sh is a robustness issue rather than a defect on the current CI configs — all 10 Feeds Package Test Build jobs are green on this head, so ip6tables is evidently present there.

PKG_RELEASE 1 → 2 is right per the feed rules: an installed support file changed with no PKG_VERSION change. test-version.sh is already in place, so the new test.sh does not need one added.


Generated by Claude Code

Comment thread net/mwan3/files/lib/mwan3/common.sh Outdated
Comment thread net/mwan3/test.sh Outdated
Comment thread net/mwan3/files/lib/mwan3/common.sh Outdated
The kernel marks routes using no-carrier devices as dead linkdown. mwan3
strips linkdown but passes dead to ip route, which rejects the copied route
and leaves policy tables incomplete.

Strip both flags, including at end of line, and add a runtime test.

Signed-off-by: Gleb Pesin <dormancygrace@gmail.com>
@dormancygrace
dormancygrace force-pushed the codex/mwan3-strip-dead-route-flag branch from 9546df4 to cc0231e Compare August 28, 2026 08:58

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 new commit; no new issues found.

All three findings from the previous round are resolved in cc0231ece:

  • Both flags now require a leading token boundary (s/ dead / /, s/ dead$/ /), so the IPv6 next-hop mangling is gone. I ran the resulting sed program against the four flag orderings plus realistic kernel output and it behaves: fd00::/64 dev br-lan proto kernel metric 256 dead linkdown pref mediumfd00::/64 dev br-lan proto kernel metric 256 pref medium, and fd00:1::/64 via fe80::dead dev eth0 … passes through untouched. The added regression assertion covers exactly that case.
  • set -e now sits after the . /lib/functions.sh / . /lib/mwan3/mwan3.sh sourcing, so the unguarded command -v ip6tables probe in common.sh no longer aborts the test on a build without ip6tables. The remaining [ $source_routing -eq 1 ] && unset source_routing inside mwan3_init is exempt from set -e (non-final command of an AND-OR list), confirmed against ash, dash and bash, so running mwan3_init under set -e is safe.
  • MWAN3_ROUTE_LINE_EXP is assembled in a single assignment, with no embedded tabs in the sed program.

The end-of-line substitutions leave a trailing space on the sanitized line, but that matches the pre-existing behaviour of s/offload// and s/expires [0-9]\+sec//, and both consumers (mwan3_get_routes | grep -qxF in mwan3rtmon, and the unquoted $IP route add … $route_line) compare or split post-sed output on both sides, so it is consistent.

PKG_RELEASE 1 → 2 remains correct, and test-version.sh is already present, so the new test.sh needs no companion. Only the three FormalityCheck runs have reported on this head (all green); the package build/test jobs have not run yet, so there is nothing CI-side to look at.


Generated by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants