diff --git a/ospfd/ospf_asbr.c b/ospfd/ospf_asbr.c index 5baad1754d5a..47419335d78b 100644 --- a/ospfd/ospf_asbr.c +++ b/ospfd/ospf_asbr.c @@ -167,6 +167,38 @@ void ospf_external_info_delete(struct ospf *ospf, uint8_t type, } } +/* + * ospf_external_info_delete_multi_instance + * + * Delete instances of the external route information for a given route type. + * The preserve_instance parameter may be used to prevent the current instance + * from being deleted. + */ +void ospf_external_info_delete_multi_instance(struct ospf *ospf, uint8_t type, struct prefix_ipv4 p, + unsigned long preserve_instance) +{ + struct route_node *rn; + struct ospf_external *ext; + struct list *ext_list; + struct listnode *node; + + ext_list = ospf->external[type]; + if (!ext_list) + return; + + for (ALL_LIST_ELEMENTS_RO(ext_list, node, ext)) { + if (ext->instance != preserve_instance) { + rn = route_node_lookup(EXTERNAL_INFO(ext), (struct prefix *)&p); + if (rn) { + ospf_external_info_free(rn->info); + rn->info = NULL; + route_unlock_node(rn); + route_unlock_node(rn); + } + } + } +} + struct external_info *ospf_external_info_lookup(struct ospf *ospf, uint8_t type, unsigned short instance, struct prefix_ipv4 *p) @@ -188,6 +220,44 @@ struct external_info *ospf_external_info_lookup(struct ospf *ospf, uint8_t type, return NULL; } +/* + * ospf_external_info_default_lookup + * + * For default information criteria, we really don't care about the + * source of the route and there only should be one. + */ +struct external_info *ospf_external_info_default_lookup(struct ospf *ospf) +{ + struct ospf_external *ext; + struct external_info *ei; + struct list *ext_list; + struct listnode *node; + struct route_node *rn; + struct prefix_ipv4 p = { + .family = AF_INET, + .prefixlen = 0, + .prefix.s_addr = INADDR_ANY, + }; + + ext_list = ospf->external[DEFAULT_ROUTE]; + if (!ext_list) + return (NULL); + + for (ALL_LIST_ELEMENTS_RO(ext_list, node, ext)) { + rn = route_node_lookup(EXTERNAL_INFO(ext), (struct prefix *)&p); + if (rn) { + route_unlock_node(rn); + if (rn->info) { + ei = rn->info; + if (ei->type != ZEBRA_ROUTE_OSPF || ei->instance != ospf->instance) + return ei; + } + } + } + + return NULL; +} + struct ospf_lsa *ospf_external_info_find_lsa(struct ospf *ospf, struct prefix_ipv4 *p) { diff --git a/ospfd/ospf_asbr.h b/ospfd/ospf_asbr.h index 6158d65f22e8..648a5a11aee2 100644 --- a/ospfd/ospf_asbr.h +++ b/ospfd/ospf_asbr.h @@ -109,6 +109,10 @@ ospf_external_info_add(struct ospf *, uint8_t, unsigned short, route_tag_t, uint32_t metric); extern void ospf_external_info_delete(struct ospf *, uint8_t, unsigned short, struct prefix_ipv4); +extern void ospf_external_info_delete_multi_instance(struct ospf *ospf, uint8_t type, + struct prefix_ipv4 p, + unsigned long preserve_instance); +#define OSPF_DELETE_ANY_INSTANCE 0xffffffff extern struct external_info *ospf_external_info_lookup(struct ospf *, uint8_t, unsigned short, struct prefix_ipv4 *); diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c index e47f83272892..b37608117313 100644 --- a/ospfd/ospf_lsa.c +++ b/ospfd/ospf_lsa.c @@ -2396,15 +2396,10 @@ struct ospf_lsa *ospf_nssa_lsa_refresh(struct ospf_area *area, static struct external_info *ospf_default_external_info(struct ospf *ospf) { int type; - struct prefix_ipv4 p; struct external_info *default_ei; int ret = 0; - p.family = AF_INET; - p.prefix.s_addr = 0; - p.prefixlen = 0; - - default_ei = ospf_external_info_lookup(ospf, DEFAULT_ROUTE, 0, &p); + default_ei = ospf_external_info_default_lookup(ospf); if (!default_ei) return NULL; diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 565527526a14..183cf89dfdaf 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -7525,6 +7525,9 @@ DEFPY (show_ip_ospf_database, struct in_addr *adv_router_p = NULL; json_object *json = NULL; + if (instance_id != ospf_instance) + return CMD_NOT_MY_INSTANCE; + if (uj) json = json_object_new_object(); if (lsid_str) diff --git a/ospfd/ospf_zebra.c b/ospfd/ospf_zebra.c index 1af703d88d6d..aa1bd3126097 100644 --- a/ospfd/ospf_zebra.c +++ b/ospfd/ospf_zebra.c @@ -1282,15 +1282,22 @@ static int ospf_zebra_read_route(ZAPI_CALLBACK_ARGS) * originate)ZEBRA_ROUTE_MAX is used to delete the ex-info. * Resolved this inconsistency by maintaining same route type. */ - if ((is_default_prefix(&pgen)) && (api.type != ZEBRA_ROUTE_OSPF)) + if ((is_default_prefix(&pgen)) && + ((api.type != ZEBRA_ROUTE_OSPF) || (api.instance != ospf->instance))) rt_type = DEFAULT_ROUTE; if (IS_DEBUG_OSPF(zebra, ZEBRA_REDISTRIBUTE)) +<<<<<<< HEAD zlog_debug( "%s: cmd %s from client %s: vrf_id %d, p %pFX, metric %d", __func__, zserv_command_string(cmd), zebra_route_string(api.type), vrf_id, &api.prefix, api.metric); +======= + zlog_debug("%s: cmd %s from client %s-%d: vrf %s(%u), p %pFX, metric %d", __func__, + zserv_command_string(cmd), zebra_route_string(api.type), api.instance, + ospf_vrf_id_to_name(vrf_id), vrf_id, &api.prefix, api.metric); +>>>>>>> 6afd56da96 (ospfd: OSPF multi-instance default origination fixes) if (cmd == ZEBRA_REDISTRIBUTE_ROUTE_ADD) { /* XXX|HACK|TODO|FIXME: @@ -1305,16 +1312,17 @@ static int ospf_zebra_read_route(ZAPI_CALLBACK_ARGS) api.tag = ospf->dtag[rt_type]; /* - * Given zebra sends update for a prefix via ADD message, it - * should - * be considered as an implicit DEL for that prefix with other - * source - * types. + * Given zebra sends an update for a prefix via an ADD message, it + * will be considered as an impilict DELETE for that prefix for other + * types and instances other than the type and instance associated with + * the prefix. */ - for (i = 0; i <= ZEBRA_ROUTE_MAX; i++) - if (i != rt_type) - ospf_external_info_delete(ospf, i, api.instance, - p); + for (i = 0; i <= ZEBRA_ROUTE_MAX; i++) { + unsigned long preserve_instance; + + preserve_instance = (i == rt_type) ? api.instance : OSPF_DELETE_ANY_INSTANCE; + ospf_external_info_delete_multi_instance(ospf, i, p, preserve_instance); + } ei = ospf_external_info_add(ospf, rt_type, api.instance, p, ifindex, nexthop, api.tag, diff --git a/ospfd/ospf_zebra.h b/ospfd/ospf_zebra.h index 86a5678fc4fa..b83524303fa7 100644 --- a/ospfd/ospf_zebra.h +++ b/ospfd/ospf_zebra.h @@ -47,6 +47,9 @@ extern uint8_t ospf_distance_apply(struct ospf *ospf, struct prefix_ipv4 *, struct ospf_route *); extern struct ospf_external *ospf_external_lookup(struct ospf *, uint8_t, unsigned short); + +extern struct external_info *ospf_external_info_default_lookup(struct ospf *ospf); + extern struct ospf_external *ospf_external_add(struct ospf *, uint8_t, unsigned short); diff --git a/tests/topotests/ospf_multi_instance/r1/frr.conf b/tests/topotests/ospf_multi_instance/r1/frr.conf new file mode 100644 index 000000000000..c341a7176a09 --- /dev/null +++ b/tests/topotests/ospf_multi_instance/r1/frr.conf @@ -0,0 +1,19 @@ +! +hostname r1 +password zebra +log file /tmp/r1-frr.log +ip forwarding +! +interface lo + ip address 1.1.1.1/32 + ip ospf area 0 +! +interface r1-eth0 + ip address 10.1.1.1/24 + ip ospf area 0 +! +! +router ospf + ospf router-id 1.1.1.1 + distance 20 +! diff --git a/tests/topotests/ospf_multi_instance/r2/frr.conf b/tests/topotests/ospf_multi_instance/r2/frr.conf new file mode 100644 index 000000000000..8501e0edc07d --- /dev/null +++ b/tests/topotests/ospf_multi_instance/r2/frr.conf @@ -0,0 +1,37 @@ +! +hostname r2 +password zebra +! debug ospf event +! debug ospf lsa +! debug ospf default-information +! debug ospf zebra redistribute + +ip forwarding +! +interface lo1 + ip address 2.2.2.1/32 + ip ospf 1 area 0 + no shut +! +interface lo2 + ip address 2.2.2.2/32 + ip ospf 2 area 0 + no shut +! +interface r2-eth0 + ip address 10.1.1.2/24 + ip ospf 1 area 0 +! +interface r2-eth1 + ip address 10.1.2.2/24 + ip ospf 2 area 0 +! +router ospf 1 + ospf router-id 2.2.2.1 + distance 20 +! +router ospf 2 + ospf router-id 2.2.2.2 + distance 20 +! + diff --git a/tests/topotests/ospf_multi_instance/r3/frr.conf b/tests/topotests/ospf_multi_instance/r3/frr.conf new file mode 100644 index 000000000000..97a3e19c9b10 --- /dev/null +++ b/tests/topotests/ospf_multi_instance/r3/frr.conf @@ -0,0 +1,19 @@ +! +hostname r3 +password zebra +log file /tmp/r3-frr.log +ip forwarding +! +interface lo + ip address 3.3.3.1/32 + ip ospf area 0 +! +interface r3-eth0 + ip address 10.1.2.3/24 + ip ospf area 0 +! +! +router ospf + ospf router-id 3.3.3.1 + distance 20 +! diff --git a/tests/topotests/ospf_multi_instance/test_ospf_multi_instance.py b/tests/topotests/ospf_multi_instance/test_ospf_multi_instance.py new file mode 100644 index 000000000000..de44140c09dc --- /dev/null +++ b/tests/topotests/ospf_multi_instance/test_ospf_multi_instance.py @@ -0,0 +1,403 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# +# test_ospf_multi_instance.py +# +# Copyright (c) 2024 LabN Consulting +# Acee Lindem +# + +import os +import sys +from functools import partial +import pytest + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib import topotest +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger + +from lib.common_config import ( + step, + create_interface_in_kernel, +) + + +""" +test_ospf_metric_propagation.py: Test OSPF/BGP metric propagation +""" + +TOPOLOGY = """ + + +---------+ +--------------------+ +---------+ + | r1 | | r2 | r2 | | r3 | + | | | ospf 1 | ospf 2 | | | + | 1.1.1.1 | eth0 eth0| 2.2.2.1 | 2.2.2.2 |eth1 eth0| 3.3.3.1 | + | +-------------+ | +-------------+ | + | | 10.1.1.0/24 | | | 10.1.2.0/24 | | + +---------+ +--------------------+ +---------+ + + +""" + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# Required to instantiate the topology builder class. + +pytestmark = [pytest.mark.ospfd, pytest.mark.bgpd] + + +def build_topo(tgen): + "Build function" + + # Create 3 routers + tgen.add_router("r1") + tgen.add_router("r2") + tgen.add_router("r3") + + # Interconect router 1, 2 (0) + switch = tgen.add_switch("s1-1-2") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r2"]) + + # Interconect router 2, 3 (1) + switch = tgen.add_switch("s2-2-3") + switch.add_link(tgen.gears["r2"]) + switch.add_link(tgen.gears["r3"]) + + # Add more loopbacks to r2 + create_interface_in_kernel( + tgen, "r2", "lo1", "2.2.2.1", netmask="255.255.255.255", create=True + ) + create_interface_in_kernel( + tgen, "r2", "lo2", "2.2.2.2", netmask="255.255.255.255", create=True + ) + + +def setup_module(mod): + logger.info("OSPF Multi-Instance:\n {}".format(TOPOLOGY)) + + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + # Starting Routers + router_list = tgen.routers() + + for rname, router in router_list.items(): + logger.info("Loading router %s" % rname) + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + # Initialize all routers. + tgen.start_router() + + +def teardown_module(): + "Teardown the pytest environment" + tgen = get_topogen() + tgen.stop_topology() + + +def test_multi_instance_default_origination(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip("Skipped because of router(s) failure") + + step("Configure a local default route") + r1 = tgen.gears["r1"] + r1.vtysh_cmd("conf t\nip route 0.0.0.0/0 Null0") + + step("Verify the R1 configuration and install of 'ip route 0.0.0.0/0 Null0'") + prefix_suppression_cfg = ( + tgen.net["r1"] + .cmd('vtysh -c "show running" | grep "^ip route 0.0.0.0/0 Null0"') + .rstrip() + ) + assertmsg = "'ip route 0.0.0.0/0 Null0' applied, but not present in configuration" + assert prefix_suppression_cfg == "ip route 0.0.0.0/0 Null0", assertmsg + + input_dict = { + "0.0.0.0/0": [ + { + "prefix": "0.0.0.0/0", + "prefixLen": 0, + "protocol": "static", + "nexthops": [ + { + "blackhole": True, + } + ], + } + ] + } + test_func = partial( + topotest.router_json_cmp, r1, "show ip route 0.0.0.0/0 json", input_dict + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "0.0.0.0/0 not installed on router r1" + assert result is None, assertmsg + + step( + "Verify the R1 configuration and advertisement of 'default-information originate'" + ) + r1.vtysh_cmd("conf t\nrouter ospf\n default-information originate") + + input_dict = { + "asExternalLinkStates": [ + { + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "1.1.1.1", + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ] + } + test_func = partial( + topotest.router_json_cmp, r1, "show ip ospf database json", input_dict + ) + + r2 = tgen.gears["r2"] + step("Verify the OSPF instance 1 installation of default route on router 2") + input_dict = { + "0.0.0.0/0": [ + { + "prefix": "0.0.0.0/0", + "prefixLen": 0, + "protocol": "ospf", + "instance": 1, + "nexthops": [ + { + "ip": "10.1.1.1", + "interfaceName": "r2-eth0", + } + ], + } + ] + } + test_func = partial( + topotest.router_json_cmp, r2, "show ip route 0.0.0.0/0 json", input_dict + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "0.0.0.0/0 not installed on router r2" + assert result is None, assertmsg + + step("Configure OSPF 'default-intformation originate' on router r2 instance 2") + r2.vtysh_cmd("conf t\nrouter ospf 2\n default-information originate") + + step("Verify r2 instance 2 AS-External default origination") + input_dict = { + "ospfInstance": 2, + "routerId": "2.2.2.2", + "asExternalLinkStates": [ + { + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "2.2.2.2", + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "tos": 0, + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ], + } + test_func = partial( + topotest.router_json_cmp, + r2, + "show ip ospf 2 database external json", + input_dict, + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "AS-External default not originated by router r2 OSPF instance 2" + assert result is None, assertmsg + + step("Update the OSPF instance 2 distance so it will be preferred over instance 1") + r2.vtysh_cmd("conf t\nrouter ospf 2\n distance 15") + + step("Generate a default route from OSPF on r3") + r3 = tgen.gears["r3"] + r3.vtysh_cmd("conf t\nrouter ospf\n default-information originate") + r3.vtysh_cmd("conf t\nip route 0.0.0.0/0 Null0") + + step("Verify r3 AS-External default origination on r2") + input_dict = { + "ospfInstance": 2, + "routerId": "2.2.2.2", + "asExternalLinkStates": [ + { + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "3.3.3.1", + "length": 36, + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "tos": 0, + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ], + } + test_func = partial( + topotest.router_json_cmp, + r2, + "show ip ospf 2 database external json", + input_dict, + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "AS-External default not originated by router r3 OSPF" + assert result is None, assertmsg + + step("Verify r3's default installed by OSPF instance 2 is preferred on r2") + input_dict = { + "0.0.0.0/0": [ + { + "prefix": "0.0.0.0/0", + "prefixLen": 0, + "protocol": "ospf", + "instance": 2, + "distance": 15, + "nexthops": [ + { + "ip": "10.1.2.3", + "interfaceName": "r2-eth1", + } + ], + } + ] + } + test_func = partial( + topotest.router_json_cmp, r2, "show ip route 0.0.0.0/0 json", input_dict + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "0.0.0.0/0 from r3 not installed on router r2" + assert result is None, assertmsg + + step( + "Verify that r2's OSPF instance 2 AS-External LSA default is flushed due to default from r3" + ) + input_dict = { + "ospfInstance": 2, + "routerId": "2.2.2.2", + "asExternalLinkStates": [ + { + "lsaAge": 3600, + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "2.2.2.2", + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "tos": 0, + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ], + } + test_func = partial( + topotest.router_json_cmp, + r2, + "show ip ospf 2 database external json", + input_dict, + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "AS-External default not flushed by router r2 OSPF instance 2" + assert result is None, assertmsg + + step("Remove r3's default route and verify that its advertisement is flushed") + r3.vtysh_cmd("conf t\nno ip route 0.0.0.0/0 Null0") + input_dict = { + "routerId": "3.3.3.1", + "asExternalLinkStates": [ + { + "lsaAge": 3600, + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "3.3.3.1", + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "tos": 0, + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ], + } + test_func = partial( + topotest.router_json_cmp, r3, "show ip ospf database external json", input_dict + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "AS-External default not flushed by router r3 OSPF" + assert result is None, assertmsg + + step( + "Verify that r2's OSPF instance 2 AS-External default is advertised and installed by r3" + ) + input_dict = { + "routerId": "3.3.3.1", + "asExternalLinkStates": [ + { + "lsaType": "AS-external-LSA", + "linkStateId": "0.0.0.0", + "advertisingRouter": "2.2.2.2", + "networkMask": 0, + "metricType": "E2 (Larger than any link state path)", + "tos": 0, + "metric": 10, + "forwardAddress": "0.0.0.0", + "externalRouteTag": 0, + } + ], + } + test_func = partial( + topotest.router_json_cmp, r3, "show ip ospf database external json", input_dict + ) + assertmsg = "AS-External default not originated by r2 OSPF instance 2" + assert result is None, assertmsg + + step("Verify r2's OSPF instance 2 is AS-External default is installed on r3") + input_dict = { + "0.0.0.0/0": [ + { + "prefix": "0.0.0.0/0", + "prefixLen": 0, + "protocol": "ospf", + "distance": 20, + "nexthops": [ + { + "ip": "10.1.2.2", + "interfaceName": "r3-eth0", + } + ], + } + ] + } + test_func = partial( + topotest.router_json_cmp, r3, "show ip route 0.0.0.0/0 json", input_dict + ) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assertmsg = "0.0.0.0/0 from router r2 not installed on r3" + assert result is None, assertmsg + + +def test_memory_leak(): + "Run the memory leak test and report results." + tgen = get_topogen() + if not tgen.is_memleak_enabled(): + pytest.skip("Memory leak test/report is disabled") + + tgen.report_memory_leaks() + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args))