Merge pull request #15899 from gromit1811/ospf6_ecmp_inter_area_bugfix_15777_pr

ospf6d: Fix nexthop computation for inter-area multi-ABR ECMP

ospf6d/ospf6_abr.c

@@ -1275,8 +1275,6 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 			continue;
 		}
 
-		list_delete_all_node(old_route->nh_list);
-		ospf6_route_copy_nexthops(old_route, route);
 		old_entry_updated = true;
 
 		for (ALL_LIST_ELEMENTS_RO(old_route->paths, anode,
@@ -1330,6 +1328,15 @@ void ospf6_abr_examin_summary(struct ospf6_lsa *lsa, struct ospf6_area *oa)
 			}
 		}
 
+		/* We added a path or updated a path's nexthops above,
+		 * recompute (old) route nexthops by merging all path nexthops
+		 */
+		list_delete_all_node(old_route->nh_list);
+		for (ALL_LIST_ELEMENTS_RO(old_route->paths, anode, o_path)) {
+			ospf6_merge_nexthops(old_route->nh_list,
+					     o_path->nh_list);
+		}
+
 		if (is_debug)
 			zlog_debug(
 				"%s: Update route: %s %p old cost %u new cost %u nh %u",

tests/topotests/ospf6_ecmp_inter_area/r5/ospf6d.conf

@@ -4,7 +4,7 @@ interface r5-eth0
  ipv6 ospf6 dead-interval 10
 !
 interface r5-eth1
- ipv6 ospf6 area 0
+ ipv6 ospf6 area 1
  ipv6 ospf6 hello-interval 2
  ipv6 ospf6 dead-interval 10
 !
@@ -13,26 +13,6 @@ interface r5-eth2
  ipv6 ospf6 hello-interval 2
  ipv6 ospf6 dead-interval 10
 !
-interface r5-eth3
- ipv6 ospf6 area 1
- ipv6 ospf6 hello-interval 2
- ipv6 ospf6 dead-interval 10
-!
-interface r5-eth4
- ipv6 ospf6 area 1
- ipv6 ospf6 hello-interval 2
- ipv6 ospf6 dead-interval 10
-!
-interface r5-eth5
- ipv6 ospf6 area 0
- ipv6 ospf6 hello-interval 2
- ipv6 ospf6 dead-interval 10
-!
-interface r5-eth6
- ipv6 ospf6 area 0
- ipv6 ospf6 hello-interval 2
- ipv6 ospf6 dead-interval 10
-!
 router ospf6
  ospf6 router-id 10.254.254.5
  redistribute connected

tests/topotests/ospf6_ecmp_inter_area/r6/ospf6d.conf

@@ -1,8 +1,23 @@
 interface r6-eth0
+ ipv6 ospf6 area 0
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r6-eth1
+ ipv6 ospf6 area 0
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r6-eth2
  ipv6 ospf6 area 1
  ipv6 ospf6 hello-interval 2
  ipv6 ospf6 dead-interval 10
 !
+interface r6-eth3
+ ipv6 ospf6 area 0
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
 router ospf6
  ospf6 router-id 10.254.254.6
  redistribute connected

tests/topotests/ospf6_ecmp_inter_area/r7/ospf6d.conf

@@ -1,11 +1,22 @@
-interface lo
- ipv6 ospf6 area 1
-!
 interface r7-eth0
  ipv6 ospf6 area 1
  ipv6 ospf6 hello-interval 2
  ipv6 ospf6 dead-interval 10
 !
+interface r7-eth1
+ ipv6 ospf6 area 1
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r7-eth2
+ ipv6 ospf6 area 1
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r7-eth3
+ shutdown
+!
 router ospf6
  ospf6 router-id 10.254.254.7
+ redistribute connected
 !

tests/topotests/ospf6_ecmp_inter_area/r7/zebra.conf

@@ -3,3 +3,6 @@ ipv6 forwarding
 interface lo
  ipv6 address 2001:db8:7::1/64
 !
+interface r7-eth2
+ ipv6 address 2001:db8:8007::1/64
+!

tests/topotests/ospf6_ecmp_inter_area/r8/ospf6d.conf

@@ -3,6 +3,19 @@ interface r8-eth0
  ipv6 ospf6 hello-interval 2
  ipv6 ospf6 dead-interval 10
 !
+interface r8-eth1
+ ipv6 ospf6 area 0
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r8-eth2
+ ipv6 ospf6 area 0
+ ipv6 ospf6 hello-interval 2
+ ipv6 ospf6 dead-interval 10
+!
+interface r8-eth3
+ shutdown
+!
 router ospf6
  ospf6 router-id 10.254.254.8
  redistribute connected

tests/topotests/ospf6_ecmp_inter_area/r8/zebra.conf

@@ -3,3 +3,6 @@ ipv6 forwarding
 interface lo
  ipv6 address 2001:db8:8::1/64
 !
+interface r8-eth2
+ ipv6 address 2001:db8:8008::1/64
+!

tests/topotests/ospf6_ecmp_inter_area/r9/ospf6d.conf

@@ -1,11 +0,0 @@
-interface lo
- ipv6 ospf6 area 0
-!
-interface r9-eth0
- ipv6 ospf6 area 0
- ipv6 ospf6 hello-interval 2
- ipv6 ospf6 dead-interval 10
-!
-router ospf6
- ospf6 router-id 10.254.254.9
-!

tests/topotests/ospf6_ecmp_inter_area/r9/zebra.conf

@@ -1,5 +0,0 @@
-ipv6 forwarding
-!
-interface lo
- ipv6 address 2001:db8:9::1/64
-!

tests/topotests/ospf6_ecmp_inter_area/test_ospf6_ecmp_inter_area.py

@@ -3,7 +3,7 @@
 
 # test_ospf6_ecmp_inter_area.py
 #
-# Copyright (c) 2021, 2022 by Martin Buck
+# Copyright (c) 2021, 2022, 2024 by Martin Buck
 # Copyright (c) 2016 by
 # Network Device Education Foundation, Inc. ("NetDEF")
 #
@@ -11,35 +11,42 @@
 r"""
 test_ospf6_ecmp_inter_area.py: Test OSPFv3 ECMP inter-area nexthop update
 
-Check proper removal of ECMP nexthops after a path used by one nexthop
+Check proper addition and removal of ECMP nexthops in 2 cases: Parallel
-disappears. We remove a path by bringing down a link required by that
+paths to one ABR and parallel ABRs. We test nexthop removal triggered by
-path which is not adjacent to the router being checked. This is important
+path removal by bringing down a link required by that path which is not
-because when bringing down adjacent links, the kernel might remove the
+adjacent to the router being checked. This is important because when
-nexthops itself without ospf6d having to do anything.
+bringing down adjacent links, the kernel might remove the nexthops itself
+without ospf6d having to do anything.
 
-Useful as a regression test for #9720.
+Useful as a regression test for #9720 and #15777.
 
 Topology:
-             .
+                  .
-      Area 0 . Area 1
+           Area 0 . Area 1
-             .
+                  .
-   -- R2 --  . ---- R6
+    -- R2 ------ R5 -----
-  /        \ ./
+   /              .\     \
-R1 -- R3 -- R5 ---- R7  Area 1
+  /               . |     \
-  \        /  \\ ..............
+R1 --- R3 ------ R6 ------ R7
-   -- R4 --    \--- R8  Area 0
+  \            / |. |
-                \
+   \          /  |. |
-                 -- R9
+    -- R4 ----   |. |
+                / ./
+              R8 --
+                  .
 
-We check routes on R1, primarily those towards R6/7/8/9. Those to R6/7 are
+We check routes on R1, primarily those towards R7/8. Those to R7 are
-inter-area routes with R5 being ABR, those to R8/9 are intra-area routes
+inter-area routes with R5/6 being ABRs, those to R8 are intra-area routes
-and are used for reference. R6/R8 announce external routes, R7/R9 announce
+and are used for reference. R7/R8 announce one internal and one external
-internal routes.
+route each.
 
 With all links up, we expect 3 ECMP paths and 3 nexthops on R1 towards each
-of R6/7/8/9. Then we bring down the R2-R5 link, causing only 2 remaining
+of R7/8. Then we bring down the R3-R6 link, causing only 2 remaining
-paths and 2 nexthops on R1. The test is successful if the number of nexthops
+paths and 2 nexthops on R1. Then we bring down the R2-R5 link, causing only
-for the routes on R1 is as expected.
+1 remaining path and 1 nexthop on R1. 
+
+The test is successful if the number of nexthops for the routes on R1 is as
+expected.
 """
 
 import os
@@ -65,20 +72,24 @@ pytestmark = [pytest.mark.ospf6d]
 def build_topo(tgen):
     "Build function"
 
-    # Create 9 routers
+    # Create 8 routers
-    for routern in range(1, 10):
+    for routern in range(1, 9):
         tgen.add_router("r{}".format(routern))
-
     tgen.gears["r1"].add_link(tgen.gears["r2"])
     tgen.gears["r1"].add_link(tgen.gears["r3"])
     tgen.gears["r1"].add_link(tgen.gears["r4"])
     tgen.gears["r2"].add_link(tgen.gears["r5"])
-    tgen.gears["r3"].add_link(tgen.gears["r5"])
+    tgen.gears["r3"].add_link(tgen.gears["r6"])
-    tgen.gears["r4"].add_link(tgen.gears["r5"])
+    tgen.gears["r4"].add_link(tgen.gears["r6"])
-    tgen.gears["r5"].add_link(tgen.gears["r6"])
     tgen.gears["r5"].add_link(tgen.gears["r7"])
     tgen.gears["r5"].add_link(tgen.gears["r8"])
-    tgen.gears["r5"].add_link(tgen.gears["r9"])
+    tgen.gears["r6"].add_link(tgen.gears["r7"])
+    tgen.gears["r6"].add_link(tgen.gears["r8"])
+    # Additional "loopback" interfaces. Not used for communication, just to
+    # hold an address we use to inject intra-/inter-area routes (the one on
+    # the real "lo" loopback is used for external routes).
+    tgen.gears["r7"].add_link(tgen.gears["r7"])
+    tgen.gears["r8"].add_link(tgen.gears["r8"])
 
 
 def setup_module(mod):
@@ -131,20 +142,19 @@ def test_wait_protocol_convergence():
     expect_neighbor_full("r2", "10.254.254.1")
     expect_neighbor_full("r2", "10.254.254.5")
     expect_neighbor_full("r3", "10.254.254.1")
-    expect_neighbor_full("r3", "10.254.254.5")
+    expect_neighbor_full("r3", "10.254.254.6")
     expect_neighbor_full("r4", "10.254.254.1")
-    expect_neighbor_full("r4", "10.254.254.5")
+    expect_neighbor_full("r4", "10.254.254.6")
     expect_neighbor_full("r5", "10.254.254.2")
-    expect_neighbor_full("r5", "10.254.254.3")
-    expect_neighbor_full("r5", "10.254.254.4")
-    expect_neighbor_full("r5", "10.254.254.6")
     expect_neighbor_full("r5", "10.254.254.7")
     expect_neighbor_full("r5", "10.254.254.8")
-    expect_neighbor_full("r5", "10.254.254.9")
+    expect_neighbor_full("r6", "10.254.254.3")
-    expect_neighbor_full("r6", "10.254.254.5")
+    expect_neighbor_full("r6", "10.254.254.7")
+    expect_neighbor_full("r6", "10.254.254.8")
     expect_neighbor_full("r7", "10.254.254.5")
+    expect_neighbor_full("r7", "10.254.254.6")
     expect_neighbor_full("r8", "10.254.254.5")
-    expect_neighbor_full("r9", "10.254.254.5")
+    expect_neighbor_full("r8", "10.254.254.6")
 
 
 def test_ecmp_inter_area():
@@ -154,9 +164,16 @@ def test_ecmp_inter_area():
         pytest.skip(tgen.errors)
 
     def num_nexthops(router):
-        routes = tgen.gears[router].vtysh_cmd("show ipv6 ospf6 route json", isjson=True)
+        # Careful: "show ipv6 ospf6 route json" doesn't work here. It will
-        route_prefixes_infos = sorted(routes.get("routes", {}).items())
+        # only list one route type per prefix and that might not necessarily
-        return [len(ri.get("nextHops", [])) for rp, ri in route_prefixes_infos]
+        # be the best/selected route. "show ipv6 route ospf6 json" only
+        # lists selected routes, so that's more useful in this case.
+        routes = tgen.gears[router].vtysh_cmd("show ipv6 route ospf6 json", isjson=True)
+        route_prefixes_infos = sorted(routes.items())
+        # Note: ri may contain one entry per routing protocol, but since
+        # we've explicitly requested only ospf6 above, we can count on ri[0]
+        # being the entry we're looking for.
+        return [ri[0]["internalNextHopActiveNum"] for rp, ri in route_prefixes_infos]
 
     def expect_num_nexthops(router, expected_num_nexthops, count):
         "Wait until number of nexthops for routes matches expectation"
@@ -174,14 +191,22 @@ def test_ecmp_inter_area():
         ), "'{}' wrong number of route nexthops".format(router)
 
     # Check nexthops pre link-down
-    expect_num_nexthops("r1", [1, 1, 1, 3, 3, 3, 3, 3, 3, 3], 4)
+    # tgen.mininet_cli()
+    expect_num_nexthops("r1", [1, 1, 1, 1, 2, 3, 3, 3, 3], 4)
 
-    logger.info("triggering R2-R4 link down")
+    logger.info("triggering R3-R6 link down")
+    tgen.gears["r3"].run("ip link set r3-eth1 down")
+
+    # tgen.mininet_cli()
+    # Check nexthops post link-down
+    expect_num_nexthops("r1", [1, 1, 1, 1, 1, 2, 2, 2, 2], 8)
+
+    logger.info("triggering R2-R5 link down")
     tgen.gears["r2"].run("ip link set r2-eth1 down")
 
     # tgen.mininet_cli()
     # Check nexthops post link-down
-    expect_num_nexthops("r1", [1, 1, 1, 2, 2, 2, 2, 2, 2, 2], 8)
+    expect_num_nexthops("r1", [1, 1, 1, 1, 1, 1, 1, 1, 1], 8)
 
 
 def teardown_module(_mod):