From 938140bfe512e8464a5880ae5e7f5c93700765ec Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 17 Oct 2026 21:52:37 +0200 Subject: nat: T9475: load-balance weights are relative to their sum The hash for load-balanced NAT rules was always taken modulo 100, while backend weights may add up to any value. With a sum below 100 some hash values had no backend in the map; commit only warned about it. The modulus is now the sum of all weights, so every hash value maps to a backend and weights act as relative shares. Rules whose weights add up to 100 render exactly as before, and the warning is dropped. --- src/conf_mode/nat.py | 7 ------- 1 file changed, 7 deletions(-) (limited to 'src') diff --git a/src/conf_mode/nat.py b/src/conf_mode/nat.py index fe3359446..bd50c26d9 100755 --- a/src/conf_mode/nat.py +++ b/src/conf_mode/nat.py @@ -133,13 +133,6 @@ def verify_rule(config, err_msg, groups_dict): for item in ['source-port', 'destination-port']: if item in config['load_balance']['hash'] and config['protocol'] not in ['tcp', 'udp']: raise ConfigError('Protocol must be tcp or udp when specifying hash ports') - count = 0 - if 'backend' in config['load_balance']: - for member in config['load_balance']['backend']: - weight = config['load_balance']['backend'][member]['weight'] - count = count + int(weight) - if count != 100: - Warning(f'Sum of weight for nat load balance rule is not 100. You may get unexpected behaviour') def verify(nat): if not nat or 'deleted' in nat: -- cgit v1.2.3 From 9c68803064f96dae1afea21d9201a9422bea86fb Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 17 Oct 2026 21:52:43 +0200 Subject: nat: T9475: validate mac-group references A rule referencing a mac-group that does not exist was not caught by verify. The rendered ruleset then failed the nft dry run and commit only reported "Configuration file errors encountered!". mac-group is now checked like the other group types and reports the missing group by name. --- src/conf_mode/nat.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/conf_mode/nat.py b/src/conf_mode/nat.py index bd50c26d9..0bd38420c 100755 --- a/src/conf_mode/nat.py +++ b/src/conf_mode/nat.py @@ -47,7 +47,8 @@ valid_groups = [ 'address_group', 'domain_group', 'network_group', - 'port_group' + 'port_group', + 'mac_group', ] def get_config(config=None): -- cgit v1.2.3 From e3e7587d7ac4f6fa3854528d91798637019c0b57 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 17 Oct 2026 21:52:50 +0200 Subject: nat: T9475: no missing interface warning for wildcards Interface wildcards such as "eth*" were looked up as literal interface names and always produced a "does not exist" warning. They are no longer looked up. The check for "any" was a substring test and now compares the name. --- src/conf_mode/nat.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/conf_mode/nat.py b/src/conf_mode/nat.py index 0bd38420c..dc3308983 100755 --- a/src/conf_mode/nat.py +++ b/src/conf_mode/nat.py @@ -149,10 +149,10 @@ def verify(nat): raise ConfigError(f'{err_msg} cannot specify both interface group and interface name for nat source rule "{rule}"') elif 'name' in config['outbound_interface']: interface_name = config['outbound_interface']['name'] - if interface_name not in 'any': + if interface_name != 'any': if interface_name.startswith('!'): interface_name = interface_name[1:] - if not interface_exists(interface_name): + if '*' not in interface_name and not interface_exists(interface_name): Warning(f'Interface "{interface_name}" for source NAT rule "{rule}" does not exist!') else: group_name = config['outbound_interface']['group'] @@ -180,10 +180,10 @@ def verify(nat): raise ConfigError(f'{err_msg} cannot specify both interface group and interface name for destination nat rule "{rule}"') elif 'name' in config['inbound_interface']: interface_name = config['inbound_interface']['name'] - if interface_name not in 'any': + if interface_name != 'any': if interface_name.startswith('!'): interface_name = interface_name[1:] - if not interface_exists(interface_name): + if '*' not in interface_name and not interface_exists(interface_name): Warning(f'Interface "{interface_name}" for destination NAT rule "{rule}" does not exist!') else: group_name = config['inbound_interface']['group'] -- cgit v1.2.3 From 7e7839dc4c76e53d22b5557f022fa87fb152c35a Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 17 Oct 2026 21:52:56 +0200 Subject: nat: T9475: recover from stale nftables rule files Whether the NAT tables already exist was derived from the rendered rule files in /run. A failed commit could leave such a file behind without the table, every later NAT commit then tried to delete a missing table and failed with "Configuration file errors encountered!" until reboot. The NAT and static NAT tables are now declared before they are deleted, which succeeds whether or not they exist. --- data/templates/firewall/nftables-nat.j2 | 4 ++-- data/templates/firewall/nftables-static-nat.j2 | 4 ++-- src/conf_mode/nat.py | 3 --- 3 files changed, 4 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/data/templates/firewall/nftables-nat.j2 b/data/templates/firewall/nftables-nat.j2 index 8c8dd3a8b..a1acbf741 100644 --- a/data/templates/firewall/nftables-nat.j2 +++ b/data/templates/firewall/nftables-nat.j2 @@ -2,9 +2,9 @@ {% import 'firewall/nftables-defines.j2' as group_tmpl %} -{% if first_install is not vyos_defined %} +# declaring the table first makes the delete succeed whether or not it exists +table ip vyos_nat delete table ip vyos_nat -{% endif %} {% if deleted is not vyos_defined %} table ip vyos_nat { # diff --git a/data/templates/firewall/nftables-static-nat.j2 b/data/templates/firewall/nftables-static-nat.j2 index e5e3da867..4f97270a5 100644 --- a/data/templates/firewall/nftables-static-nat.j2 +++ b/data/templates/firewall/nftables-static-nat.j2 @@ -1,8 +1,8 @@ #!/usr/sbin/nft -f -{% if first_install is not vyos_defined %} +# declaring the table first makes the delete succeed whether or not it exists +table ip vyos_static_nat delete table ip vyos_static_nat -{% endif %} {% if deleted is not vyos_defined %} table ip vyos_static_nat { # diff --git a/src/conf_mode/nat.py b/src/conf_mode/nat.py index dc3308983..1ae0ae9b6 100755 --- a/src/conf_mode/nat.py +++ b/src/conf_mode/nat.py @@ -215,9 +215,6 @@ def verify(nat): return None def generate(nat): - if not os.path.exists(nftables_nat_config): - nat['first_install'] = True - render(nftables_nat_config, 'firewall/nftables-nat.j2', nat) render(nftables_static_nat_conf, 'firewall/nftables-static-nat.j2', nat) -- cgit v1.2.3 From c01cf102163c93891b17a663790db6c2b8433301 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 17 Oct 2026 21:53:22 +0200 Subject: nat66: T9475: rebuild NAT66 table when rule file and table drift apart The NAT66 ruleset only deleted the existing table when its rule file in /run was present. A table removed while the file stayed made the delete, and with it every following commit, fail. The table is now declared right before it is deleted, so each commit rebuilds it regardless of what is currently loaded. --- data/templates/firewall/nftables-nat66.j2 | 4 ++-- smoketest/scripts/cli/test_nat66.py | 22 ++++++++++++++++++++++ src/conf_mode/nat66.py | 3 --- 3 files changed, 24 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/data/templates/firewall/nftables-nat66.j2 b/data/templates/firewall/nftables-nat66.j2 index 09b5b6ac2..3f206ca4e 100644 --- a/data/templates/firewall/nftables-nat66.j2 +++ b/data/templates/firewall/nftables-nat66.j2 @@ -2,9 +2,9 @@ {% import 'firewall/nftables-defines.j2' as group_tmpl %} -{% if first_install is not vyos_defined %} +# declaring the table first makes the delete succeed whether or not it exists +table ip6 vyos_nat delete table ip6 vyos_nat -{% endif %} {% if deleted is not vyos_defined %} table ip6 vyos_nat { # diff --git a/smoketest/scripts/cli/test_nat66.py b/smoketest/scripts/cli/test_nat66.py index 9072e78dd..ee25edbc5 100755 --- a/smoketest/scripts/cli/test_nat66.py +++ b/smoketest/scripts/cli/test_nat66.py @@ -19,6 +19,7 @@ import unittest from base_vyostest_shim import VyOSUnitTestSHIM from vyos.configsession import ConfigSessionError +from vyos.utils.process import cmdl base_path = ['nat66'] src_path = base_path + ['source'] @@ -319,5 +320,26 @@ class TestNAT66(VyOSUnitTestSHIM.TestCase): self.cli_set(dst_path) self.cli_commit() + def test_nat66_table_recreated(self): + # the rule file in /run and the nftables table can drift apart, the + # next commit must rebuild the table instead of failing on it + self.cli_set(src_path + ['rule', '1', 'outbound-interface', 'name', 'eth1']) + self.cli_set(src_path + ['rule', '1', 'source', 'prefix', 'fc00::/64']) + self.cli_set(src_path + ['rule', '1', 'translation', 'address', 'masquerade']) + self.cli_commit() + + cmdl(['nft', 'delete', 'table', 'ip6', 'vyos_nat'], sudo=True) + + self.cli_set(src_path + ['rule', '2', 'outbound-interface', 'name', 'eth1']) + self.cli_set(src_path + ['rule', '2', 'source', 'prefix', 'fc02::/64']) + self.cli_set(src_path + ['rule', '2', 'exclude']) + self.cli_commit() + + nftables_search = [ + ['ip6 saddr fc00::/64', 'masquerade', 'comment "SRC-NAT66-1"'], + ['ip6 saddr fc02::/64', 'return', 'comment "SRC-NAT66-2"'], + ] + self.verify_nftables(nftables_search, 'ip6 vyos_nat') + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) diff --git a/src/conf_mode/nat66.py b/src/conf_mode/nat66.py index 999e320eb..420f9143d 100755 --- a/src/conf_mode/nat66.py +++ b/src/conf_mode/nat66.py @@ -127,9 +127,6 @@ def verify(nat): return None def generate(nat): - if not os.path.exists(nftables_nat66_config): - nat['first_install'] = True - render(nftables_nat66_config, 'firewall/nftables-nat66.j2', nat) # dry-run newly generated configuration -- cgit v1.2.3