Skip to content

pbr[1.2.3]: refactor iface-priority to bring it in line with 1.2.2#91

Merged
stangri merged 1 commit intomossdef-org:1.2.3from
egc112:1.2.3
Mar 20, 2026
Merged

pbr[1.2.3]: refactor iface-priority to bring it in line with 1.2.2#91
stangri merged 1 commit intomossdef-org:1.2.3from
egc112:1.2.3

Conversation

@egc112
Copy link
Collaborator

@egc112 egc112 commented Mar 18, 2026

Apparently a commit from 1.2.2 to make sure sport rule and prefix_length rule are first (lowest priority) was not carried over.

This PR should correct that

Apparently a commit from 1.2.2 to make sure sport rule and  prefix_length rule are first (lowest priority)
was not carried over.

This PR should correct that

Signed-off-by: Erik Conijn <egc112@msn.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts how ip rule priorities are assigned for global (interface-independent) rules in pbr[1.2.3], aiming to match the behavior from 1.2.2 where WireGuard sport and suppress_prefixlength rules should be evaluated before other PBR rules.

Changes:

  • Introduces a prio cursor initialized from iface_priority when creating global rules.
  • Decrements prio as WireGuard server sport rules are added to avoid sharing the same priority.
  • Uses the resulting prio for the suppress_prefixlength rule and updates iface_priority accordingly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1011 to 1038
@@ -1017,29 +1018,29 @@ function create_pbr(fs_mod, uci_mod, ubus_mod) {
if (disabled != '1' && listen_port) {
if (cfg.uplink_interface4) {
let tbl = pkg.ip_table_prefix + '_' + cfg.uplink_interface4;
let prio = '' + iface_priority;
system(pkg.ip_full + ' -4 rule del sport ' + listen_port + ' table ' + tbl + ' priority ' + prio + ' 2>/dev/null');
sh.ip('-4', 'rule', 'add', 'sport', listen_port, 'table', tbl, 'priority', prio);
if (cfg.ipv6_enabled) {
system(pkg.ip_full + ' -6 rule del sport ' + listen_port + ' table ' + tbl + ' priority ' + prio + ' 2>/dev/null');
sh.ip('-6', 'rule', 'add', 'sport', listen_port, 'table', tbl, 'priority', prio);
}
prio = '' + (+prio - 1);
}
}
}
});
let spl_prio = '' + (int(cfg.uplink_ip_rules_priority) + 1);
system(pkg.ip_full + ' -4 rule del priority ' + spl_prio + ' 2>/dev/null');
system(pkg.ip_full + ' -4 rule del priority ' + prio + ' 2>/dev/null');
system(pkg.ip_full + ' -4 rule del lookup main suppress_prefixlength ' + cfg.prefixlength + ' 2>/dev/null');
sh.try_cmd(state.errors, pkg.ip_full, '-4', 'rule', 'add', 'lookup', 'main', 'suppress_prefixlength',
'' + cfg.prefixlength, 'pref', spl_prio);
'' + cfg.prefixlength, 'pref', prio);
if (cfg.ipv6_enabled) {
system(pkg.ip_full + ' -6 rule del priority ' + spl_prio + ' 2>/dev/null');
system(pkg.ip_full + ' -6 rule del priority ' + prio + ' 2>/dev/null');
system(pkg.ip_full + ' -6 rule del lookup main suppress_prefixlength ' + cfg.prefixlength + ' 2>/dev/null');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have tested it and all rules are cleared because ther is just one suppress_prefixlength rule.
There could be more then one WireGuard servers ,I have tested with two and that also works.
So keep it as is

Comment on lines 1021 to 1024
system(pkg.ip_full + ' -4 rule del sport ' + listen_port + ' table ' + tbl + ' priority ' + prio + ' 2>/dev/null');
sh.ip('-4', 'rule', 'add', 'sport', listen_port, 'table', tbl, 'priority', prio);
if (cfg.ipv6_enabled) {
system(pkg.ip_full + ' -6 rule del sport ' + listen_port + ' table ' + tbl + ' priority ' + prio + ' 2>/dev/null');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There could be other suppress prefix length rules and we do not want to delete those.
But thinking about it I think the whole rule is probably redundant anyway but I will check that later for now just keep it as is

@stangri stangri merged commit dc55e7a into mossdef-org:1.2.3 Mar 20, 2026
7 checks passed
@stangri
Copy link
Collaborator

stangri commented Mar 20, 2026

thanks Erik!

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.

3 participants