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.

: as consequence, the FIB flag should not be
used on that 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.

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,