Merge pull request #13808 from anlancs/fix/zebra-kernel-route-reserved

zebra: fix wrong nexthop check for kernel routes

tests/topotests/zebra_rib/r1/v4_route_1_vrf_before.json

@@ -0,0 +1,27 @@
+{
+    "3.5.1.0/24":[
+        {
+            "prefix":"3.5.1.0/24",
+            "prefixLen":24,
+            "protocol":"kernel",
+            "vrfId":0,
+            "vrfName":"default",
+            "selected":true,
+            "destSelected":true,
+            "distance":0,
+            "metric":0,
+            "installed":true,
+            "table":254,
+            "nexthops":[
+                {
+                    "flags":3,
+                    "fib":true,
+                    "ip":"192.168.210.1",
+                    "afi":"ipv4",
+                    "interfaceName":"r1-eth0",
+                    "active":true
+                }
+            ]
+        }
+    ]
+}

tests/topotests/zebra_rib/test_zebra_rib.py

@@ -77,6 +77,41 @@ def teardown_module(mod):
     tgen.stop_topology()
 
 
+def test_zebra_kernel_route_vrf():
+    "Test kernel routes should be removed after interface changes vrf"
+    logger.info("Test kernel routes should be removed after interface changes vrf")
+    vrf = "RED"
+    tgen = get_topogen()
+    r1 = tgen.gears["r1"]
+
+    # Add kernel routes, the interface is initially in default vrf
+    r1.run("ip route add 3.5.1.0/24 via 192.168.210.1 dev r1-eth0")
+    json_file = "{}/r1/v4_route_1_vrf_before.json".format(CWD)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route 3.5.1.0/24 json", expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=5, wait=1)
+    assert result is None, '"r1" JSON output mismatches'
+
+    # Change the interface's vrf
+    r1.run("ip link add {} type vrf table 1".format(vrf))
+    r1.run("ip link set {} up".format(vrf))
+    r1.run("ip link set dev r1-eth0 master {}".format(vrf))
+
+    expected = "{}"
+    test_func = partial(
+        topotest.router_output_cmp, r1, "show ip route 3.5.1.0/24 json", expected
+    )
+    result, diff = topotest.run_and_expect(test_func, "", count=5, wait=1)
+    assertmsg = "{} should not have the kernel route.\n{}".format('"r1"', diff)
+    assert result, assertmsg
+
+    # Clean up
+    r1.run("ip link set dev r1-eth0 nomaster")
+    r1.run("ip link del dev {}".format(vrf))
+
+
 def test_zebra_kernel_admin_distance():
     "Test some basic kernel routes added that should be accepted"
     logger.info("Test some basic kernel routes that should be accepted")

zebra/zebra_nhg.c

@@ -2181,11 +2181,7 @@ static int nexthop_active(struct nexthop *nexthop, struct nhg_hash_entry *nhe,
 	case NEXTHOP_TYPE_IFINDEX:
 
 		ifp = if_lookup_by_index(nexthop->ifindex, nexthop->vrf_id);
-		/*
+		/* If the interface exists and its operative, it's active */
-		 * If the interface exists and its operative or its a kernel
-		 * route and interface is up, its active. We trust kernel routes
-		 * to be good.
-		 */
 		if (ifp && (if_is_operative(ifp)))
 			return 1;
 		else
@@ -2586,6 +2582,8 @@ static unsigned nexthop_active_check(struct route_node *rn,
 	if (IS_ZEBRA_DEBUG_NHG_DETAIL)
 		zlog_debug("%s: re %p, nexthop %pNHv", __func__, re, nexthop);
 
+	vrf_id = zvrf_id(rib_dest_vrf(rib_dest_from_rnode(rn)));
+
 	/*
 	 * If this is a kernel route, then if the interface is *up* then
 	 * by golly gee whiz it's a good route.
@@ -2595,13 +2593,12 @@ static unsigned nexthop_active_check(struct route_node *rn,
 
 		ifp = if_lookup_by_index(nexthop->ifindex, nexthop->vrf_id);
 
-		if (ifp && (if_is_operative(ifp) || if_is_up(ifp))) {
+		if (ifp && ifp->vrf->vrf_id == vrf_id && if_is_up(ifp)) {
 			SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE);
 			goto skip_check;
 		}
 	}
 
-	vrf_id = zvrf_id(rib_dest_vrf(rib_dest_from_rnode(rn)));
 	switch (nexthop->type) {
 	case NEXTHOP_TYPE_IFINDEX:
 		if (nexthop_active(nexthop, nhe, &rn->p, re->type, re->flags,