zebra, topotests: do not set nexthop's FIB flag when DUPLICATE present

The bgp_duplicate_nexthop test installs routes with nexthop's
flags set to both DUPLICATE and FIB: this should not happen.

The DUPLICATE flag of a nexthop indicates this nexthop is already
used in the same nexthop-group, and there is no need to install it
twice in the system; having the FIB flag set indicates that the
nexthop is installed in the system. This is why both flags should
not be set on the same nexthop.

This case happens at installation time, but can also happen
at update time.
- Fix this by not setting the FIB flag value when the DUPLICATE
flag is present.
- Modify the bgp_duplicate_test to check that the FIB flag is not
present on duplicated nexthops.
- Modify the bgp_peer_type_multipath_relax test.

Link: FRRouting#16342
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>

tests/topotests/bgp_duplicate_nexthop/test_bgp_duplicate_nexthop.py

@@ -320,7 +320,7 @@ def check_ipv4_prefix_recursive_with_multiple_nexthops(
             )
 
     test_func = functools.partial(
-        ip_check_path_selection, tgen.gears["r1"], prefix, expected
+        ip_check_path_selection, tgen.gears["r1"], prefix, expected, check_fib=True
     )
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
     assert (

tests/topotests/bgp_peer_type_multipath_relax/r1/prefix1-eBGP-confed.json

@@ -23,7 +23,6 @@
           "recursive":true
         },
         {
-          "fib":true,
           "duplicate":true,
           "ip":"10.0.3.2",
           "active":true

tests/topotests/bgp_peer_type_multipath_relax/r1/prefix1-eBGP-iBGP.json

@@ -23,7 +23,6 @@
           "recursive":true
         },
         {
-          "fib":true,
           "duplicate":true,
           "ip":"10.0.3.2",
           "active":true

tests/topotests/lib/common_check.py

@@ -10,11 +10,13 @@
 from lib import topotest
 
 
-def ip_check_path_selection(router, ipaddr_str, expected, vrf_name=None):
+def ip_check_path_selection(
+    router, ipaddr_str, expected, vrf_name=None, check_fib=False
+):
     if vrf_name:
-        cmdstr = f'show ip route vrf {vrf_name} {ipaddr_str} json'
+        cmdstr = f"show ip route vrf {vrf_name} {ipaddr_str} json"
     else:
-        cmdstr = f'show ip route {ipaddr_str} json'
+        cmdstr = f"show ip route {ipaddr_str} json"
     try:
         output = json.loads(router.vtysh_cmd(cmdstr))
     except:
@@ -25,6 +27,21 @@ def ip_check_path_selection(router, ipaddr_str, expected, vrf_name=None):
         num_nh_expected = len(expected[ipaddr_str][0]["nexthops"])
         num_nh_observed = len(output[ipaddr_str][0]["nexthops"])
         if num_nh_expected == num_nh_observed:
+            if check_fib:
+                # special case: when fib flag is unset,
+                # an extra test should be done to check that the flag is really unset
+                for nh_output, nh_expected in zip(
+                    output[ipaddr_str][0]["nexthops"],
+                    expected[ipaddr_str][0]["nexthops"],
+                ):
+                    if (
+                        "fib" in nh_output.keys()
+                        and nh_output["fib"]
+                        and ("fib" not in nh_expected.keys() or not nh_expected["fib"])
+                    ):
+                        return "{}, prefix {} nexthop {} has the fib flag set, whereas it is not expected".format(
+                            router.name, ipaddr_str, nh_output["ip"]
+                        )
             return ret
         return "{}, prefix {} does not have the correct number of nexthops : observed {}, expected {}".format(
             router.name, ipaddr_str, num_nh_observed, num_nh_expected
@@ -37,9 +54,9 @@ def iproute2_check_path_selection(router, ipaddr_str, expected, vrf_name=None):
         return None
 
     if vrf_name:
-        cmdstr = f'ip -json route show vrf {vrf_name} {ipaddr_str}'
+        cmdstr = f"ip -json route show vrf {vrf_name} {ipaddr_str}"
     else:
-        cmdstr = f'ip -json route show {ipaddr_str}'
+        cmdstr = f"ip -json route show {ipaddr_str}"
     try:
         output = json.loads(cmdstr)
     except:

zebra/zebra_dplane.c

@@ -4314,6 +4314,10 @@ dplane_route_update_internal(struct route_node *rn,
 					       NEXTHOP_FLAG_RECURSIVE))
 					continue;
 
+				if (CHECK_FLAG(nexthop->flags,
+					       NEXTHOP_FLAG_DUPLICATE))
+					continue;
+
 				if (CHECK_FLAG(nexthop->flags,
 					       NEXTHOP_FLAG_ACTIVE))
 					SET_FLAG(nexthop->flags,

zebra/zebra_rib.c

@@ -1659,6 +1659,9 @@ static bool rib_update_nhg_from_ctx(struct nexthop_group *re_nhg,
 		if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE))
 			continue;
 
+		if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_DUPLICATE))
+			continue;
+
 		/* Check for a FIB nexthop corresponding to the RIB nexthop */
 		if (!nexthop_same(ctx_nexthop, nexthop)) {
 			/* If the FIB doesn't know about the nexthop,