From 35ee59fb57cfb36732a54c6514a0f5cb381b79fa Mon Sep 17 00:00:00 2001 From: ckishimo Date: Sat, 27 Nov 2021 00:14:07 +0100 Subject: [PATCH 1/3] ospf6d: stop refreshing type-5 from NSSA With the current code, in a topology like this: r1 ---- 0.0.0.0 ---- r2(ABR) ---- 1.1.1.1 -----r3(ASBR) NSSA where r3 is redistributing statics within the NSSA area, the ABR (r2) is translating type-7 lsa to type-5. Everytime the function ospf6_abr_nssa_task() is executed all translated type-5 are aged out and refreshed for no reason. So for instance having 3 lsas already advertised: r1# sh ipv6 os database AS Scoped Link State Database Type LSId AdvRouter Age SeqNum Payload ASE 0.0.0.1 2.2.2.2 39 80000001 3:3::3/128 ASE 0.0.0.2 2.2.2.2 39 80000001 4:4::4/128 ASE 0.0.0.3 2.2.2.2 39 80000001 5:5::5/128 Adversting a new route from r3: r3(config)# ipv6 route 6:6::6/128 Null0 r1# sh ipv6 os database AS Scoped Link State Database Type LSId AdvRouter Age SeqNum Payload ASE 0.0.0.1 2.2.2.2 124 80000001 3:3::3/128 ASE 0.0.0.2 2.2.2.2 124 80000001 4:4::4/128 ASE 0.0.0.3 2.2.2.2 124 80000001 5:5::5/128 ASE 0.0.0.4 2.2.2.2 8 80000001 6:6::6/128 That seems okay, however a few seconds later we see all prefixes refreshed r1# sh ipv6 os database AS Scoped Link State Database Type LSId AdvRouter Age SeqNum Payload ASE 0.0.0.1 2.2.2.2 3600 80000001 3:3::3/128 ASE 0.0.0.2 2.2.2.2 3600 80000001 4:4::4/128 ASE 0.0.0.3 2.2.2.2 3600 80000001 5:5::5/128 ASE 0.0.0.4 2.2.2.2 3600 80000001 6:6::6/128 ASE 0.0.0.5 2.2.2.2 3 80000001 3:3::3/128 ASE 0.0.0.6 2.2.2.2 3 80000001 4:4::4/128 ASE 0.0.0.7 2.2.2.2 3 80000001 5:5::5/128 ASE 0.0.0.8 2.2.2.2 3 80000001 6:6::6/128 This PR prevents the LSA of being refreshed by unsetting the OSPF6_LSA_UNAPPROVED flag so advertising the last prefix will not refresh all of them: r1# sh ipv6 os database AS Scoped Link State Database Type LSId AdvRouter Age SeqNum Payload ASE 0.0.0.1 2.2.2.2 90 80000001 3:3::3/128 ASE 0.0.0.2 2.2.2.2 47 80000001 4:4::4/128 ASE 0.0.0.3 2.2.2.2 35 80000001 5:5::5/128 ASE 0.0.0.4 2.2.2.2 7 80000001 6:6::6/128 Signed-off-by: ckishimo --- ospf6d/ospf6_nssa.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/ospf6d/ospf6_nssa.c b/ospf6d/ospf6_nssa.c index cd1be3a5b7..7d85b32986 100644 --- a/ospf6d/ospf6_nssa.c +++ b/ospf6d/ospf6_nssa.c @@ -625,7 +625,7 @@ static void ospf6_abr_translate_nssa(struct ospf6_area *area, struct ospf6_lsa * * Later, any Unapproved Translated Type-5's are flushed/discarded */ - struct ospf6_lsa *old = NULL, *new = NULL; + struct ospf6_lsa *old = NULL; struct ospf6_as_external_lsa *nssa_lsa; struct prefix prefix; struct ospf6_route *match; @@ -662,11 +662,11 @@ static void ospf6_abr_translate_nssa(struct ospf6_area *area, struct ospf6_lsa * } /* Find the existing AS-External LSA for this prefix */ - match = ospf6_route_lookup(&prefix, ospf6->external_table); + match = ospf6_route_lookup(&prefix, ospf6->route_table); if (match) { - old = ospf6_lsdb_lookup(OSPF6_LSTYPE_AS_EXTERNAL, - match->path.origin.id, ospf6->router_id, - ospf6->lsdb); + old = ospf6_lsdb_lookup(htons(OSPF6_LSTYPE_AS_EXTERNAL), + lsa->external_lsa_id, ospf6->router_id, + ospf6->lsdb); } if (OSPF6_LSA_IS_MAXAGE(lsa)) { @@ -675,20 +675,15 @@ static void ospf6_abr_translate_nssa(struct ospf6_area *area, struct ospf6_lsa * return; } - if (old) { + if (old && !OSPF6_LSA_IS_MAXAGE(old)) { if (IS_OSPF6_DEBUG_NSSA) zlog_debug( - "%s : found old translated LSA Id %pI4, refreshing", + "%s : found old translated LSA Id %pI4, skip", __func__, &old->header->id); - /* refresh */ - new = ospf6_translated_nssa_refresh(area, lsa, old); - if (!new) { - if (IS_OSPF6_DEBUG_NSSA) - zlog_debug( - "%s : could not refresh translated LSA Id %pI4", - __func__, &old->header->id); - } + UNSET_FLAG(old->flag, OSPF6_LSA_UNAPPROVED); + return; + } else { /* no existing external route for this LSA Id * originate translated LSA From 3cd5108d828b5c6a0c4b0f28c984846750d3734b Mon Sep 17 00:00:00 2001 From: ckishimo Date: Wed, 5 Jan 2022 20:27:55 +0100 Subject: [PATCH 2/3] ospf6d: fix NSSA area-range command When an area-range command is applied in an ABR, the more specific prefixes need to be removed. r2# sh ipv6 ospf database AS Scoped Link State Database Type LSId AdvRouter Age SeqNum Payload ASE 0.0.0.1 10.254.254.2 53 80000001 :: ASE 0.0.0.2 10.254.254.2 51 80000001 2001:db8:1::/64 ASE 0.0.0.3 10.254.254.2 51 80000001 2001:db8:3::/64 ASE 0.0.0.4 10.254.254.2 51 80000001 2001:db8:2::/64 ASE 0.0.0.5 10.254.254.2 46 80000001 2001:db8:1::/64 ASE 0.0.0.6 10.254.254.2 46 80000001 2001:db8:3::/64 ASE 0.0.0.7 10.254.254.2 46 80000001 2001:db8:2::/64 ASE 0.0.0.8 10.254.254.2 41 80000001 2001:db8:3::/64 ASE 0.0.0.9 10.254.254.2 41 80000001 2001:db8:1000::1/128 <-- ** ASE 0.0.0.10 10.254.254.2 41 80000001 2001:db8:1000::2/128 <-- ** ASE 0.0.0.12 10.254.254.2 24 80000001 2001:db8:1000::/64 ASE 0.0.0.1 10.254.254.3 52 80000001 2001:db8:2::/64 Signed-off-by: ckishimo --- ospf6d/ospf6_nssa.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/ospf6d/ospf6_nssa.c b/ospf6d/ospf6_nssa.c index 7d85b32986..5c3d31dda8 100644 --- a/ospf6d/ospf6_nssa.c +++ b/ospf6d/ospf6_nssa.c @@ -613,7 +613,8 @@ struct ospf6_lsa *ospf6_translated_nssa_refresh(struct ospf6_area *area, return new; } -static void ospf6_abr_translate_nssa(struct ospf6_area *area, struct ospf6_lsa *lsa) +static void ospf6_abr_translate_nssa(struct ospf6_area *area, + struct ospf6_lsa *lsa) { /* Incoming Type-7 or later aggregated Type-7 * @@ -661,12 +662,37 @@ static void ospf6_abr_translate_nssa(struct ospf6_area *area, struct ospf6_lsa * return; } + /* Find the type-5 LSA in the area-range table */ + match = ospf6_route_lookup_bestmatch(&prefix, area->nssa_range_table); + if (match && CHECK_FLAG(match->flag, OSPF6_ROUTE_NSSA_RANGE)) { + if (prefix_same(&prefix, &match->prefix)) { + /* The prefix range is being removed, + * no need to refresh + */ + if + CHECK_FLAG(match->flag, OSPF6_ROUTE_REMOVE) + return; + } else { + if (!CHECK_FLAG(match->flag, OSPF6_ROUTE_REMOVE)) { + if (IS_OSPF6_DEBUG_NSSA) + zlog_debug( + "%s: LSA Id %pI4 suppressed by range %pFX of area %s", + __func__, &lsa->header->id, + &match->prefix, area->name); + /* LSA will be suppressed by area-range command, + * no need to refresh + */ + return; + } + } + } + /* Find the existing AS-External LSA for this prefix */ match = ospf6_route_lookup(&prefix, ospf6->route_table); if (match) { old = ospf6_lsdb_lookup(htons(OSPF6_LSTYPE_AS_EXTERNAL), - lsa->external_lsa_id, ospf6->router_id, - ospf6->lsdb); + lsa->external_lsa_id, ospf6->router_id, + ospf6->lsdb); } if (OSPF6_LSA_IS_MAXAGE(lsa)) { From 9a974f22928a2795843e68ac823f323ea81304f1 Mon Sep 17 00:00:00 2001 From: ckishimo Date: Wed, 5 Jan 2022 23:46:24 +0100 Subject: [PATCH 3/3] ospf6d: fix topotest The routes in the test_nssa_range() are E2 "routes":{ "2001:db8:1000::2/128":{ "destinationType":"Network", "installedTimeSince":"00:06:29", "changedTimeSince":"00:06:29", "numberOfLock":2, "flags":"BA--", "associatedArea":"0.0.0.2", "pathType":"External-2", "lsOriginRoutePathType":"NSSA", "lsId":"0.0.0.3", "lsAdvertisingRouter":"10.254.254.4", "options":"--|-|-|--|-|--", "routerBits":"--------", "prefixOptions":"--|P|--|--|--", "metricType":2, "metricCost":10, "metricCostE2":20, "pathsCount":1, "nextHopCount":1, "nextHops":[ { "nextHop":"::", "interfaceName":"r2-eth2" } ] } This PR fixes the key from `metricCost` to `metricCostE2` Signed-off-by: ckishimo --- .../topotests/ospf6_topo2/test_ospf6_topo2.py | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/topotests/ospf6_topo2/test_ospf6_topo2.py b/tests/topotests/ospf6_topo2/test_ospf6_topo2.py index eb8561c404..d17aeda3ea 100644 --- a/tests/topotests/ospf6_topo2/test_ospf6_topo2.py +++ b/tests/topotests/ospf6_topo2/test_ospf6_topo2.py @@ -134,6 +134,7 @@ def build_topo(tgen): switch = tgen.add_switch("s4") switch.add_link(tgen.gears["r4"], nodeif="r4-stubnet") + def setup_module(mod): "Sets up the pytest environment" tgen = Topogen(build_topo, mod.__name__) @@ -585,10 +586,11 @@ def test_nssa_range(): logger.info("Expecting NSSA range to be added on r3") routes = { "2001:db8:1000::/64": { - "metricType":2, - "metricCost":20, - "metricCostE2":10, - }} + "metricType": 2, + "metricCost": 20, + "metricCostE2": 10, + } + } expect_ospfv3_routes("r3", routes, wait=30, type="external-2", detail=True) # Change the NSSA range cost. @@ -601,10 +603,11 @@ def test_nssa_range(): logger.info("Expecting NSSA range to be updated with new cost") routes = { "2001:db8:1000::/64": { - "metricType":2, - "metricCost":20, - "metricCostE2":1000, - }} + "metricType": 2, + "metricCost": 20, + "metricCostE2": 1000, + } + } expect_ospfv3_routes("r3", routes, wait=30, type="external-2", detail=True) # Configure the NSSA range to not be advertised. @@ -631,12 +634,12 @@ def test_nssa_range(): logger.info("Expecting previously summarized routes to be re-added") routes = { "2001:db8:1000::1/128": { - "metricType":2, - "metricCost":20, + "metricType": 2, + "metricCostE2": 20, }, "2001:db8:1000::2/128": { - "metricType":2, - "metricCost":20, + "metricType": 2, + "metricCostE2": 20, }, } expect_ospfv3_routes("r3", routes, wait=30, type="external-2", detail=True)