pbr[1.2.3]: tighten is_netifd_interface() to require env.netifd_mark#105
Merged
stangri merged 1 commit intoMay 20, 2026
Merged
Conversation
The helper classified by UCI alone (any iface with `ip4table` or `ip6table` set), which over-included user-manual two-uplink setups that pbr did not put in place. In dynamic-routing-tables mode this caused `interface_routing 'create'` to early-return for such ifaces, leaving the per-mark chain and `ip rule fwmark` uninstalled while policy processing still emitted `goto pbr_mark_${mark}` — the resulting ruleset failed `nft -c -f` and pbr aborted on start.
Narrow the helper to ifaces present in `env.netifd_mark`, populated from `pkg.nft_netifd_file` — i.e. ifaces pbr itself set up via its netifd extensions. The check now mirrors `is_mwan4_interface()`.
Side effect: user-manual `ip4table` ifaces are now added to `ifaces_triggers` so pbr reloads on their dev up/down — the intended behaviour once pbr does manage them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: per_terra <53376589+Per-Terra@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR refines how pbr detects “netifd-managed” interfaces by narrowing is_netifd_interface() so it only matches interfaces that pbr itself set up via netifd extensions (those present in env.netifd_mark). This prevents manual ip4table/ip6table configurations from being misclassified as netifd interfaces, which (per the PR description) can lead to missing mark chains / fwmark rules and an invalid generated nft ruleset in dynamic-routing-tables mode.
Changes:
- Change
is_netifd_interface()from UCI (ip4table/ip6table) based detection toenv.netifd_mark[iface]based detection.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
133
to
136
| function is_netifd_table(name) { let c = readfile('/etc/config/network') || ''; return index(c, name) >= 0 && !!match(c, regexp('ip.table.*' + name)); } | ||
| function is_netifd_interface(iface) { | ||
| let ctx = config.uci_ctx('network'); | ||
| let ip4t = ctx.get('network', iface, 'ip4table'); | ||
| let ip6t = ctx.get('network', iface, 'ip6table'); | ||
| return !!(ip4t || ip6t); | ||
| return !!(iface && env.netifd_mark[iface]); | ||
| } |
Collaborator
|
@Per-Terra apologies for the delay in merging. This is a very elegant fix, kudos! |
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
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.
Re-take of #104 along the direction @stangri proposed in #104 (comment) — instead of adding a netifd-specific branch to
interface_routing.create, tightenis_netifd_interface()itself.is_netifd_interface()previously classified by UCI alone (any iface withip4tableorip6tableset), which over-included user-manual two-uplink setups that pbr did not put in place. In dynamic-routing-tables mode this causedinterface_routing 'create'to early-return for such ifaces, leaving the per-mark chain andip rule fwmarkuninstalled while policy processing still emittedgoto pbr_mark_${mark}— the resulting ruleset failednft -c -fand pbr aborted on start.The helper is now narrowed to ifaces present in
env.netifd_mark, populated frompkg.nft_netifd_file— i.e. ifaces pbr itself set up via its netifd extensions. The check now mirrorsis_mwan4_interface()'senv.mwan4_mark[iface].One subtle side effect: user-manual
ip4tableifaces are now added toifaces_triggers, so pbr reloads on their dev up/down — the intended behaviour once pbr does manage them.