From b702dfc362d16e11d96debc39a2dcdcc72619277 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Thu, 13 Feb 2025 17:03:35 +0100 Subject: [PATCH] bgp: T7161: fix IPv4/IPv6 unicast AFI "redistribute table" command Re-use existing XML constraint added via commit 8f6246da6 ("xml: T7161: provide re-usable building block for alternative routing tables") and add handy CLI completion helper. FRRouting supports redistribution of multiple non-main tables, thus make this a multi node in addition, too. --- data/templates/frr/bgpd.frr.j2 | 4 +- .../afi-redistribute-common-protocols.xml.i | 5 + smoketest/scripts/cli/test_protocols_bgp.py | 154 +++++++++++++++--- 3 files changed, 142 insertions(+), 21 deletions(-) diff --git a/data/templates/frr/bgpd.frr.j2 b/data/templates/frr/bgpd.frr.j2 index 51a3f25640..2f3719fdf3 100644 --- a/data/templates/frr/bgpd.frr.j2 +++ b/data/templates/frr/bgpd.frr.j2 @@ -310,7 +310,9 @@ router bgp {{ system_as }} {{ 'vrf ' ~ vrf if vrf is vyos_defined }} {% if afi_config.redistribute is vyos_defined %} {% for protocol, protocol_config in afi_config.redistribute.items() %} {% if protocol == 'table' %} - redistribute table {{ protocol_config.table }} +{% for table in protocol_config %} + redistribute table-direct {{ table }} +{% endfor %} {% else %} {% set redistribution_protocol = protocol %} {% if protocol == 'ospfv3' %} diff --git a/interface-definitions/include/bgp/afi-redistribute-common-protocols.xml.i b/interface-definitions/include/bgp/afi-redistribute-common-protocols.xml.i index b5ad0ea772..f4d18bc700 100644 --- a/interface-definitions/include/bgp/afi-redistribute-common-protocols.xml.i +++ b/interface-definitions/include/bgp/afi-redistribute-common-protocols.xml.i @@ -42,6 +42,11 @@ Redistribute non-main Kernel Routing Table + + protocols static table + + #include + diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py index 761eb8bfe5..0eda52ff63 100755 --- a/smoketest/scripts/cli/test_protocols_bgp.py +++ b/smoketest/scripts/cli/test_protocols_bgp.py @@ -655,10 +655,51 @@ def test_bgp_04_afi_ipv4(self): } # We want to redistribute ... - redistributes = ['connected', 'isis', 'kernel', 'ospf', 'rip', 'static'] - for redistribute in redistributes: - self.cli_set(base_path + ['address-family', 'ipv4-unicast', - 'redistribute', redistribute]) + redistributes = { + 'babel' : { + 'metric' : '100', + 'route_map' : 'redistr-ipv4-babel', + }, + 'connected' : { + 'metric' : '200', + 'route_map' : 'redistr-ipv4-connected', + }, + 'isis' : { + 'metric' : '300', + 'route_map' : 'redistr-ipv4-isis', + }, + 'kernel' : { + 'metric' : '400', + 'route_map' : 'redistr-ipv4-kernel', + }, + 'ospf' : { + 'metric' : '500', + 'route_map' : 'redistr-ipv4-ospf', + }, + 'rip' : { + 'metric' : '600', + 'route_map' : 'redistr-ipv4-rip', + }, + 'static' : { + 'metric' : '700', + 'route_map' : 'redistr-ipv4-static', + }, + 'table' : { + 'number' : ['10', '20', '30', '40'], + }, + } + for proto, proto_config in redistributes.items(): + proto_path = base_path + ['address-family', 'ipv4-unicast', 'redistribute', proto] + if proto == 'table' and 'number' in proto_config: + for number in proto_config['number']: + self.cli_set(proto_path, value=number) + else: + self.cli_set(proto_path) + if 'metric' in proto_config: + self.cli_set(proto_path + ['metric', proto_config['metric']]) + if 'route_map' in proto_config: + self.cli_set(['policy', 'route-map', proto_config['route_map'], 'rule', '10', 'action', 'permit']) + self.cli_set(proto_path + ['route-map', proto_config['route_map']]) for network, network_config in networks.items(): self.cli_set(base_path + ['address-family', 'ipv4-unicast', @@ -679,10 +720,22 @@ def test_bgp_04_afi_ipv4(self): # Verify FRR bgpd configuration frrconfig = self.getFRRconfig(f'router bgp {ASN}', endsection='^exit') self.assertIn(f'router bgp {ASN}', frrconfig) - self.assertIn(f' address-family ipv4 unicast', frrconfig) - - for redistribute in redistributes: - self.assertIn(f' redistribute {redistribute}', frrconfig) + self.assertIn(' address-family ipv4 unicast', frrconfig) + + for proto, proto_config in redistributes.items(): + if proto == 'table' and 'number' in proto_config: + for number in proto_config['number']: + self.assertIn(f' redistribute table-direct {number}', frrconfig) + else: + tmp = f' redistribute {proto}' + if 'metric' in proto_config: + metric = proto_config['metric'] + tmp += f' metric {metric}' + if 'route_map' in proto_config: + route_map = proto_config['route_map'] + tmp += f' route-map {route_map}' + + self.assertIn(tmp, frrconfig) for network, network_config in networks.items(): self.assertIn(f' network {network}', frrconfig) @@ -695,6 +748,10 @@ def test_bgp_04_afi_ipv4(self): command = f'{command} route-map {network_config["route_map"]}' self.assertIn(command, frrconfig) + for proto, proto_config in redistributes.items(): + if 'route_map' in proto_config: + self.cli_delete(['policy', 'route-map', proto_config['route_map']]) + def test_bgp_05_afi_ipv6(self): networks = { '2001:db8:100::/48' : { @@ -707,10 +764,51 @@ def test_bgp_05_afi_ipv6(self): } # We want to redistribute ... - redistributes = ['connected', 'kernel', 'ospfv3', 'ripng', 'static'] - for redistribute in redistributes: - self.cli_set(base_path + ['address-family', 'ipv6-unicast', - 'redistribute', redistribute]) + redistributes = { + 'babel' : { + 'metric' : '100', + 'route_map' : 'redistr-ipv6-babel', + }, + 'connected' : { + 'metric' : '200', + 'route_map' : 'redistr-ipv6-connected', + }, + 'isis' : { + 'metric' : '300', + 'route_map' : 'redistr-ipv6-isis', + }, + 'kernel' : { + 'metric' : '400', + 'route_map' : 'redistr-ipv6-kernel', + }, + 'ospfv3' : { + 'metric' : '500', + 'route_map' : 'redistr-ipv6-ospfv3', + }, + 'ripng' : { + 'metric' : '600', + 'route_map' : 'redistr-ipv6-ripng', + }, + 'static' : { + 'metric' : '700', + 'route_map' : 'redistr-ipv6-static', + }, + 'table' : { + 'number' : ['100', '120', '130', '140'], + }, + } + for proto, proto_config in redistributes.items(): + proto_path = base_path + ['address-family', 'ipv6-unicast', 'redistribute', proto] + if proto == 'table' and 'number' in proto_config: + for number in proto_config['number']: + self.cli_set(proto_path, value=number) + else: + self.cli_set(proto_path) + if 'metric' in proto_config: + self.cli_set(proto_path + ['metric', proto_config['metric']]) + if 'route_map' in proto_config: + self.cli_set(['policy', 'route-map', proto_config['route_map'], 'rule', '20', 'action', 'permit']) + self.cli_set(proto_path + ['route-map', proto_config['route_map']]) for network, network_config in networks.items(): self.cli_set(base_path + ['address-family', 'ipv6-unicast', @@ -725,22 +823,38 @@ def test_bgp_05_afi_ipv6(self): # Verify FRR bgpd configuration frrconfig = self.getFRRconfig(f'router bgp {ASN}', endsection='^exit') self.assertIn(f'router bgp {ASN}', frrconfig) - self.assertIn(f' address-family ipv6 unicast', frrconfig) + self.assertIn(' address-family ipv6 unicast', frrconfig) # T2100: By default ebgp-requires-policy is disabled to keep VyOS # 1.3 and 1.2 backwards compatibility - self.assertIn(f' no bgp ebgp-requires-policy', frrconfig) - - for redistribute in redistributes: - # FRR calls this OSPF6 - if redistribute == 'ospfv3': - redistribute = 'ospf6' - self.assertIn(f' redistribute {redistribute}', frrconfig) + self.assertIn(' no bgp ebgp-requires-policy', frrconfig) + + for proto, proto_config in redistributes.items(): + if proto == 'table' and 'number' in proto_config: + for number in proto_config['number']: + self.assertIn(f' redistribute table-direct {number}', frrconfig) + else: + # FRR calls this OSPF6 + if proto == 'ospfv3': + proto = 'ospf6' + tmp = f' redistribute {proto}' + if 'metric' in proto_config: + metric = proto_config['metric'] + tmp += f' metric {metric}' + if 'route_map' in proto_config: + route_map = proto_config['route_map'] + tmp += f' route-map {route_map}' + + self.assertIn(tmp, frrconfig) for network, network_config in networks.items(): self.assertIn(f' network {network}', frrconfig) if 'as_set' in network_config: self.assertIn(f' aggregate-address {network} summary-only', frrconfig) + for proto, proto_config in redistributes.items(): + if 'route_map' in proto_config: + self.cli_delete(['policy', 'route-map', proto_config['route_map']]) + def test_bgp_06_listen_range(self): # Implemented via T1875 limit = '64'