From 60aabdc9587700a326c8e9df06f09ba0c60c5435 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Thu, 8 Oct 2026 22:27:36 +0700 Subject: ospf: T9441: migrate "parameters opaque-lsa" to "capability opaque" "protocols ospf parameters opaque-lsa" and "protocols ospf capability opaque" are two CLI nodes for one FRR setting, FRR's "capability opaque". Keep "capability opaque" only. The migration script ospf/2-to-3 sets "capability opaque" where "parameters opaque-lsa" exists in the default VRF, and deletes the old node (and the "parameters" node when it is left empty). FRR refuses opaque LSAs outside the default VRF, so in a VRF instance the option never took effect: for a VRF the migration drops both "parameters opaque-lsa" and a plain "capability opaque" instead of moving the value into a node FRR would refuse at the next commit. The template renders the line from "capability opaque" only; it rendered the alias form "ospf opaque-lsa" for the old node, which frr-reload found missing in the running config on every reload: while only the old node was set, every commit that reloads FRR reset all adjacencies of that OSPF instance. Tests: a migration unit test (global instance, VRF instance, a VRF with "capability opaque" set directly, both nodes set together in a VRF, and global+VRF independence), the vrf-ospf config test (now carries "parameters opaque-lsa" in both the default instance and a VRF instance; the VRF instance is expected to lose it with no replacement), test_ospf_02 uses the new node, and test_ospf_21 checks the generated FRR config. --- smoketest/scripts/cli/test_protocols_ospf.py | 31 +++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) (limited to 'smoketest/scripts/cli/test_protocols_ospf.py') diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py index 7879a7379..ef20bb2a2 100755 --- a/smoketest/scripts/cli/test_protocols_ospf.py +++ b/smoketest/scripts/cli/test_protocols_ospf.py @@ -85,7 +85,7 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase): self.cli_set(base_path + ['auto-cost', 'reference-bandwidth', bandwidth]) self.cli_set(base_path + ['parameters', 'router-id', router_id]) self.cli_set(base_path + ['parameters', 'abr-type', abr_type]) - self.cli_set(base_path + ['parameters', 'opaque-lsa']) + self.cli_set(base_path + ['capability', 'opaque']) self.cli_set(base_path + ['parameters', 'rfc1583-compatibility']) self.cli_set(base_path + ['log-adjacency-changes', 'detail']) self.cli_set(base_path + ['default-metric', metric]) @@ -712,5 +712,34 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase): self.cli_commit() verify('') + def test_ospf_21_capability_opaque(self): + # "capability opaque" is the only CLI node for FRR's "capability opaque" + # (T9441: "parameters opaque-lsa" was migrated to it). The generated + # FRR config file must hold the line once, and no alias form of it. + # + # This reads the generated FRR config file: FRR accepts the alias + # "ospf opaque-lsa", so vtysh cannot tell which form was rendered. + frr_conf = '/run/frr/config/vyos.frr.conf' + capability = base_path + ['capability', 'opaque'] + + # the router-id keeps "router ospf" configured when the node is + # deleted below, so the delete is tested against a running instance + self.cli_set(base_path + ['parameters', 'router-id', '192.0.2.1']) + self.cli_set(capability) + self.cli_commit() + + frrconfig = read_file(frr_conf) + self.assertEqual(frrconfig.count('\n capability opaque\n'), 1) + self.assertNotRegex(frrconfig, r'(?m)^ ospf opaque-lsa') + frrconfig = self.getFRRconfig('router ospf', stop_section='^exit') + self.assertIn(' capability opaque', frrconfig) + + self.cli_delete(capability) + self.cli_commit() + self.assertNotIn('capability opaque', read_file(frr_conf)) + frrconfig = self.getFRRconfig('router ospf', stop_section='^exit') + self.assertNotIn(' capability opaque', frrconfig) + self.assertIn(' ospf router-id 192.0.2.1', frrconfig) + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) -- cgit v1.2.3 From a57ab8277f5b35d6b3f5b7873d3107ab527887e3 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Thu, 8 Oct 2026 22:27:56 +0700 Subject: ospf: T9441: render "compatible rfc1583" as FRR prints it "protocols ospf parameters rfc1583-compatibility" was rendered with the alias command "ospf rfc1583compatibility". FRR accepts the alias but stores the setting under "compatible rfc1583" and prints it in that form. frr-reload compares text, so on every reload it found the rendered line missing in the running config, ran "no compatible rfc1583" and added the alias line again, in every commit that reloads FRR, also one that does not touch OSPF. "compatible rfc1583" schedules an SPF in both directions, which costs one extra SPF run per reload. Render the canonical line. --- data/templates/frr/ospfd.frr.j2 | 2 +- smoketest/scripts/cli/test_protocols_ospf.py | 51 ++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) (limited to 'smoketest/scripts/cli/test_protocols_ospf.py') diff --git a/data/templates/frr/ospfd.frr.j2 b/data/templates/frr/ospfd.frr.j2 index 0e1f079c4..77bd901b3 100644 --- a/data/templates/frr/ospfd.frr.j2 +++ b/data/templates/frr/ospfd.frr.j2 @@ -212,7 +212,7 @@ router ospf {{ 'vrf ' ~ vrf if vrf is vyos_defined }} ospf abr-type {{ parameters.abr_type }} {% endif %} {% if parameters.rfc1583_compatibility is vyos_defined %} - ospf rfc1583compatibility + compatible rfc1583 {% endif %} {% if parameters.router_id is vyos_defined %} ospf router-id {{ parameters.router_id }} diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py index ef20bb2a2..da7ead42b 100755 --- a/smoketest/scripts/cli/test_protocols_ospf.py +++ b/smoketest/scripts/cli/test_protocols_ospf.py @@ -741,5 +741,56 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase): self.assertNotIn(' capability opaque', frrconfig) self.assertIn(' ospf router-id 192.0.2.1', frrconfig) + def test_ospf_22_rfc1583_canonical_form(self): + # FRR prints "compatible rfc1583". The CLI option "parameters + # rfc1583-compatibility" must be rendered in this form: if the alias + # form "ospf rfc1583compatibility" is rendered, frr-reload finds the + # line missing on every reload, removes the setting and adds it again, + # which runs an extra SPF on each FRR reload, whatever it changed. + # + # This reads the generated FRR config file: FRR accepts both forms, so + # vtysh cannot tell which one we rendered. + frr_conf = '/run/frr/config/vyos.frr.conf' + alias_form = r'(?m)^ ospf rfc1583compatibility' + rfc1583 = base_path + ['parameters', 'rfc1583-compatibility'] + + # the router-id keeps "router ospf" configured when the node is + # deleted below, so the delete is tested against a running instance + self.cli_set(base_path + ['parameters', 'router-id', '192.0.2.1']) + self.cli_set(rfc1583) + self.cli_commit() + + frrconfig = read_file(frr_conf) + self.assertEqual(frrconfig.count('\n compatible rfc1583\n'), 1) + self.assertNotRegex(frrconfig, alias_form) + frrconfig = self.getFRRconfig('router ospf', stop_section='^exit') + self.assertIn(' compatible rfc1583', frrconfig) + + self.cli_delete(rfc1583) + self.cli_commit() + self.assertNotIn('rfc1583', read_file(frr_conf)) + frrconfig = self.getFRRconfig('router ospf', stop_section='^exit') + self.assertNotIn(' compatible rfc1583', frrconfig) + self.assertIn(' ospf router-id 192.0.2.1', frrconfig) + + # a VRF instance renders the same form, in the VRF block only + vrf = 'blue' + vrf_base = ['vrf', 'name', vrf] + vrf_header = f'router ospf vrf {vrf}' + self.cli_set(vrf_base + ['table', '1000']) + vrf_params = vrf_base + ['protocols', 'ospf', 'parameters'] + self.cli_set(vrf_params + ['router-id', '1.1.1.1']) + self.cli_set(vrf_params + ['rfc1583-compatibility']) + self.cli_commit() + + frrconfig = read_file(frr_conf) + self.assertNotRegex(frrconfig, alias_form) + _, _, vrf_block = frrconfig.partition(vrf_header) + self.assertIn(' compatible rfc1583', vrf_block) + frrconfig = self.getFRRconfig(vrf_header, stop_section='^exit') + self.assertIn(' compatible rfc1583', frrconfig) + + self.cli_delete(vrf_base) + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) -- cgit v1.2.3 From 20a57113117b5db6d54c8da3eb08fa29b9398947 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:01:27 +0700 Subject: ospf: T9441: reject "capability opaque" in a VRF instance FRR refuses opaque LSAs outside the default VRF ("OSPF Opaque LSA is only supported in default VRF"). Before this, a "set protocols ospf capability opaque" under "vrf name protocols ospf" was accepted by the CLI and only failed later, at FRR's own reload, with the render already written and every other change in the commit already applied. Reject it in verify() before the commit reaches FRR, with a message that names the node and the VRF; the default VRF is unaffected. Test: test_ospf_23_vrf_capability_opaque sets "capability opaque" in a VRF instance and expects the commit to fail. --- smoketest/scripts/cli/test_protocols_ospf.py | 24 ++++++++++++++++++++++++ src/conf_mode/protocols_ospf.py | 7 +++++++ 2 files changed, 31 insertions(+) (limited to 'smoketest/scripts/cli/test_protocols_ospf.py') diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py index da7ead42b..28568ecbb 100755 --- a/smoketest/scripts/cli/test_protocols_ospf.py +++ b/smoketest/scripts/cli/test_protocols_ospf.py @@ -792,5 +792,29 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase): self.cli_delete(vrf_base) + def test_ospf_23_vrf_capability_opaque(self): + # FRR refuses opaque LSAs outside the default VRF (T9441's migration + # drops a saved "capability opaque" there for the same reason); a new + # "set" must be rejected before FRR is reloaded, not left to FRR's + # own refusal + vrf = 'blue' + vrf_base = ['vrf', 'name', vrf] + vrf_ospf = vrf_base + ['protocols', 'ospf'] + self.cli_set(vrf_base + ['table', '1000']) + self.cli_set(vrf_ospf + ['parameters', 'router-id', '1.1.1.1']) + self.cli_set(vrf_ospf + ['capability', 'opaque']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(vrf_ospf + ['capability']) + self.cli_commit() + + vrf_header = f'router ospf vrf {vrf}' + frrconfig = self.getFRRconfig(vrf_header, stop_section='^exit') + self.assertIn(' ospf router-id 1.1.1.1', frrconfig) + self.assertNotIn('capability opaque', frrconfig) + + self.cli_delete(vrf_base) + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py index 47e1596a6..0f73a6c8a 100755 --- a/src/conf_mode/protocols_ospf.py +++ b/src/conf_mode/protocols_ospf.py @@ -52,6 +52,13 @@ def verify(config_dict): config_dict) or config_dict['ospf'] ospf['policy'] = config_dict['policy'] + # FRR supports opaque LSAs only in the default VRF + if vrf and 'opaque' in ospf.get('capability', {}): + raise ConfigError( + 'OSPF "capability opaque" is only supported in the ' + f'default VRF, not in VRF "{vrf}"!' + ) + verify_common_route_maps(ospf) # As we can have a default-information route-map, we need to validate it! -- cgit v1.2.3