Merge pull request #16614 from louis-6wind/fix-otable-heap-after-free

zebra: fix table heap-after-free crash

lib/zclient.c

@@ -1362,6 +1362,8 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api)
 	stream_putc(s, api->type);
 
 	stream_putw(s, api->instance);
+	if (CHECK_FLAG(api->message, ZAPI_MESSAGE_TABLEID))
+		SET_FLAG(api->flags, ZEBRA_FLAG_TABLEID);
 	stream_putl(s, api->flags);
 	stream_putl(s, api->message);
 
@@ -1421,6 +1423,8 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api)
 				return -1;
 			}
 
+			if (CHECK_FLAG(api->message, ZAPI_MESSAGE_TABLEID))
+				SET_FLAG(api->flags, ZEBRA_FLAG_TABLEID);
 			if (zapi_nexthop_encode(s, api_nh, api->flags,
 						api->message)
 			    != 0)
@@ -1460,6 +1464,9 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api)
 				return -1;
 			}
 
+			if (CHECK_FLAG(api->message, ZAPI_MESSAGE_TABLEID))
+				SET_FLAG(api->flags, ZEBRA_FLAG_TABLEID);
+
 			if (zapi_nexthop_encode(s, api_nh, api->flags,
 						api->message)
 			    != 0)

lib/zclient.h

@@ -578,6 +578,12 @@ struct zapi_route {
  * kernel (NLM_F_APPEND at the very least )
  */
 #define ZEBRA_FLAG_OUTOFSYNC          0x400
+/*
+ * This flag lets us know that the route entry is
+ * associated to the table ID and must remain when the
+ * table ID is de-associated from a VRF.
+ */
+#define ZEBRA_FLAG_TABLEID 0x800
 
 	/* The older XXX_MESSAGE flags live here */
 	uint32_t message;

tests/topotests/zebra_rib/r1/import_mrib_table_2.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/import_mrib_table_3.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -238,7 +238,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/import_mrib_table_4.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/import_table_2.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/import_table_3.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -238,7 +238,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/import_table_4.json

@@ -68,7 +68,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -97,7 +97,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -126,7 +126,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,
@@ -155,7 +155,7 @@
             "installed": true,
             "table": 254,
             "internalStatus": 16,
-            "internalFlags": 9,
+            "internalFlags": 2057,
             "nexthops": [
                 {
                     "flags": 3,

tests/topotests/zebra_rib/r1/v4_route_table_1_no_vrf.json

@@ -0,0 +1,54 @@
+{
+  "0.0.0.0/0": [
+    {
+      "protocol": "kernel",
+      "vrfName": "default",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "unreachable": true,
+          "blackhole": true,
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.0.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "default",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "unreachable": true,
+          "blackhole": true,
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.1.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "default",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "ip": "192.168.211.254",
+          "interfaceName": "r1-eth1",
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.2.0.0/24": null,
+  "10.3.0.0/24": null,
+  "192.168.210.0/24": null,
+  "192.168.210.1/32": null
+}

tests/topotests/zebra_rib/r1/v4_route_table_1_no_vrf.txt

@@ -0,0 +1,3 @@
+blackhole default
+blackhole 10.0.0.0/24 proto XXXX metric 20
+10.1.0.0/24 via 192.168.211.254 dev r1-eth1 proto XXXX metric 20

tests/topotests/zebra_rib/r1/v4_route_table_1_vrf_red.json

@@ -0,0 +1,115 @@
+{
+  "0.0.0.0/0": [
+    {
+      "protocol": "kernel",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "unreachable": true,
+          "blackhole": true,
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.0.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "unreachable": true,
+          "blackhole": true,
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.1.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "ip": "192.168.211.254",
+          "interfaceName": "r1-eth1",
+          "vrf": "default",
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.2.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "ip": "192.168.210.254",
+          "interfaceName": "r1-eth0",
+          "active": true
+        }
+      ]
+    }
+  ],
+  "10.3.0.0/24": [
+    {
+      "protocol": "static",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "ip": "192.168.212.254",
+          "interfaceName": "r1-eth2",
+          "vrf": "default",
+          "active": true
+        }
+      ]
+    }
+  ],
+  "192.168.210.0/24": [
+    {
+      "protocol": "connected",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "directlyConnected": true,
+          "interfaceName": "r1-eth0",
+          "active": true
+        }
+      ]
+    }
+  ],
+  "192.168.210.1/32": [
+    {
+      "protocol": "local",
+      "vrfName": "RED",
+      "installed": true,
+      "table": 1,
+      "nexthops": [
+        {
+          "fib": true,
+          "interfaceName": "r1-eth0",
+          "active": true
+        }
+      ]
+    }
+  ]
+}

tests/topotests/zebra_rib/r1/v4_route_table_1_vrf_red.txt

@@ -0,0 +1,6 @@
+blackhole default
+blackhole 10.0.0.0/24 proto XXXX metric 20
+10.1.0.0/24 via 192.168.211.254 dev r1-eth1 proto XXXX metric 20
+10.2.0.0/24 via 192.168.210.254 dev r1-eth0 proto XXXX metric 20
+10.3.0.0/24 via 192.168.212.254 dev r1-eth2 proto XXXX metric 20
+192.168.210.0/24 dev r1-eth0 proto XXXX scope link src 192.168.210.1 

tests/topotests/zebra_rib/test_zebra_rib.py

@@ -27,12 +27,13 @@ sys.path.append(os.path.join(CWD, "../"))
 # pylint: disable=C0413
 # Import topogen and topotest helpers
 from lib import topotest
+from lib.common_config import step
 from lib.topogen import Topogen, TopoRouter, get_topogen
 from lib.topolog import logger
 from time import sleep
 
 
-pytestmark = [pytest.mark.sharpd]
+pytestmark = [pytest.mark.sharpd, pytest.mark.staticd]
 krel = platform.release()
 
 
@@ -64,6 +65,7 @@ def setup_module(mod):
         router.load_config(
             TopoRouter.RD_SHARP, os.path.join(CWD, "{}/sharpd.conf".format(rname))
         )
+        router.load_config(TopoRouter.RD_STATIC, "/dev/null")
 
     # Macvlan interface for protodown func test */
     config_macvlan(tgen, "r1", "r1-eth0", "r1-eth0-macvlan")
@@ -77,14 +79,54 @@ def teardown_module():
     tgen.stop_topology()
 
 
+def check_routes_installed(expected, table=None):
+    tgen = get_topogen()
+    r1 = tgen.gears["r1"]
+
+    cmd = "ip route show"
+    if table:
+        cmd += " table {}".format(table)
+    actual = r1.run(cmd)
+    actual = ("\n".join(actual.splitlines()) + "\n").rstrip()
+    actual = re.sub(r" nhid [0-9][0-9]", "", actual)
+    actual = re.sub(r" proto sharp", " proto XXXX", actual)
+    actual = re.sub(r" proto static", " proto XXXX", actual)
+    actual = re.sub(r" proto 194", " proto XXXX", actual)
+    actual = re.sub(r" proto 196", " proto XXXX", actual)
+    actual = re.sub(r" proto kernel", " proto XXXX", actual)
+    actual = re.sub(r" proto 2", " proto XXXX", actual)
+    # Some platforms have double spaces?  Why??????
+    actual = re.sub(r"  proto XXXX  ", " proto XXXX ", actual)
+    actual = re.sub(r"  metric", " metric", actual)
+    actual = re.sub(r" link  ", " link ", actual)
+    actual = actual.splitlines()
+    actual = [
+        line.rstrip()
+        for line in actual
+        if not line.startswith("broadcast") and not line.startswith("local")
+    ]
+
+    expected = ("\n".join(expected.splitlines()) + "\n").rstrip()
+    expected = expected.splitlines()
+    expected = [line.rstrip() for line in expected]
+
+    return topotest.get_textdiff(
+        actual,
+        expected,
+        title1="Actual ip route show",
+        title2="Expected ip route show",
+    )
+
+
 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"
+    table_id = 1
     tgen = get_topogen()
     r1 = tgen.gears["r1"]
 
-    # Add kernel routes, the interface is initially in default vrf
+    step("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())
@@ -94,11 +136,69 @@ def test_zebra_kernel_route_vrf():
     _, 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
+    step("Add routes in table 1")
-    r1.run("ip link add {} type vrf table 1".format(vrf))
+    r1.run("ip route add blackhole default table {}".format(table_id))
+
+    r1.vtysh_cmd(
+        """
+configure terminal
+ ip route 10.0.0.0/24 blackhole table {}
+ ip route 10.1.0.0/24 192.168.211.254 nexthop-vrf default table {}
+""".format(
+            table_id, table_id
+        )
+    )
+
+    json_file = "{}/r1/v4_route_table_1_no_vrf.json".format(CWD)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route table 1 json", expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, '"r1" JSON output mismatches'
+
+    ipfile = "{}/r1/v4_route_table_1_no_vrf.txt".format(CWD)
+    expected = open(ipfile).read().rstrip()
+    expected = ("\n".join(expected.splitlines()) + "\n").rstrip()
+
+    test_func = partial(check_routes_installed, expected, table=1)
+    ok, result = topotest.run_and_expect(test_func, "", count=60, wait=0.5)
+    assert ok, result
+
+    step("Add VRF {} and assign it r1-eth0 interface".format(vrf))
+    r1.run("ip link add {} type vrf table {}".format(vrf, table_id))
     r1.run("ip link set {} up".format(vrf))
     r1.run("ip link set dev r1-eth0 master {}".format(vrf))
 
+    step("Add static routes to VRF {}".format(vrf))
+    r1.vtysh_cmd(
+        """
+configure terminal
+ vrf {}
+  ip route 10.2.0.0/24 192.168.210.254
+  ip route 10.3.0.0/24 192.168.212.254 nexthop-vrf default
+""".format(
+            vrf
+        )
+    )
+
+    json_file = "{}/r1/v4_route_table_1_vrf_red.json".format(CWD)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route table 1 json", expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, '"r1" JSON output mismatches'
+
+    ipfile = "{}/r1/v4_route_table_1_vrf_red.txt".format(CWD)
+    expected = open(ipfile).read().rstrip()
+    expected = ("\n".join(expected.splitlines()) + "\n").rstrip()
+
+    test_func = partial(check_routes_installed, expected, table=1)
+    ok, result = topotest.run_and_expect(test_func, "", count=60, wait=0.5)
+    assert ok, result
+
+    step("check 3.5.1.0/24 absence on VRF default")
     expected = "{}"
     test_func = partial(
         topotest.router_output_cmp, r1, "show ip route 3.5.1.0/24 json", expected
@@ -107,10 +207,26 @@ def test_zebra_kernel_route_vrf():
     assertmsg = "{} should not have the kernel route.\n{}".format('"r1"', diff)
     assert result, assertmsg
 
-    # Clean up
+    step("Remove VRF {}".format(vrf))
     r1.run("ip link set dev r1-eth0 nomaster")
     r1.run("ip link del dev {}".format(vrf))
 
+    json_file = "{}/r1/v4_route_table_1_no_vrf.json".format(CWD)
+    expected = json.loads(open(json_file).read())
+    test_func = partial(
+        topotest.router_json_cmp, r1, "show ip route table 1 json", expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5)
+    assert result is None, '"r1" JSON output mismatches'
+
+    ipfile = "{}/r1/v4_route_table_1_no_vrf.txt".format(CWD)
+    expected = open(ipfile).read().rstrip()
+    expected = ("\n".join(expected.splitlines()) + "\n").rstrip()
+
+    test_func = partial(check_routes_installed, expected, table=1)
+    ok, result = topotest.run_and_expect(test_func, "", count=60, wait=0.5)
+    assert ok, result
+
 
 def test_zebra_kernel_admin_distance():
     "Test some basic kernel routes added that should be accepted"
@@ -295,28 +411,8 @@ def test_route_map_usage():
     expected = open(sharp_ipfile).read().rstrip()
     expected = ("\n".join(expected.splitlines()) + "\n").rstrip()
 
-    def check_routes_installed():
+    test_func = partial(check_routes_installed, expected)
-        actual = r1.run("ip route show")
+    ok, result = topotest.run_and_expect(test_func, "", count=60, wait=0.5)
-        actual = ("\n".join(actual.splitlines()) + "\n").rstrip()
-        actual = re.sub(r" nhid [0-9][0-9]", "", actual)
-        actual = re.sub(r" proto sharp", " proto XXXX", actual)
-        actual = re.sub(r" proto static", " proto XXXX", actual)
-        actual = re.sub(r" proto 194", " proto XXXX", actual)
-        actual = re.sub(r" proto 196", " proto XXXX", actual)
-        actual = re.sub(r" proto kernel", " proto XXXX", actual)
-        actual = re.sub(r" proto 2", " proto XXXX", actual)
-        # Some platforms have double spaces?  Why??????
-        actual = re.sub(r"  proto XXXX  ", " proto XXXX ", actual)
-        actual = re.sub(r"  metric", " metric", actual)
-        actual = re.sub(r" link  ", " link ", actual)
-        return topotest.get_textdiff(
-            actual,
-            expected,
-            title1="Actual ip route show",
-            title2="Expected ip route show",
-        )
-
-    ok, result = topotest.run_and_expect(check_routes_installed, "", count=5, wait=1)
     assert ok, result
 
 

zebra/zebra_nhg.c

@@ -2930,6 +2930,68 @@ static uint32_t proto_nhg_nexthop_active_update(struct nexthop_group *nhg)
 	return curr_active;
 }
 
+void nexthop_vrf_update(struct route_node *rn, struct route_entry *re, vrf_id_t vrf_id)
+{
+	struct nhg_hash_entry *curr_nhe, *new_nhe;
+	afi_t rt_afi = family2afi(rn->p.family);
+	struct nexthop *nexthop;
+
+	re->vrf_id = vrf_id;
+
+	/* Make a local copy of the existing nhe, so we don't work on/modify
+	 * the shared nhe.
+	 */
+	curr_nhe = zebra_nhe_copy(re->nhe, re->nhe->id);
+
+	if (IS_ZEBRA_DEBUG_NHG_DETAIL)
+		zlog_debug("%s: re %p nhe %p (%pNG), curr_nhe %p", __func__, re, re->nhe, re->nhe,
+			   curr_nhe);
+
+	/* Clear the existing id, if any: this will avoid any confusion
+	 * if the id exists, and will also force the creation
+	 * of a new nhe reflecting the changes we may make in this local copy.
+	 */
+	curr_nhe->id = 0;
+
+	curr_nhe->vrf_id = vrf_id;
+	for (ALL_NEXTHOPS(curr_nhe->nhg, nexthop)) {
+		if (!nexthop->ifindex)
+			/* change VRF ID of nexthop without interfaces
+			 * (eg. blackhole)
+			 */
+			nexthop->vrf_id = vrf_id;
+	}
+
+	if (zebra_nhg_get_backup_nhg(curr_nhe)) {
+		for (ALL_NEXTHOPS(curr_nhe->backup_info->nhe->nhg, nexthop)) {
+			if (!nexthop->ifindex)
+				/* change VRF ID of nexthop without interfaces
+				 * (eg. blackhole)
+				 */
+				nexthop->vrf_id = vrf_id;
+		}
+	}
+
+	/*
+	 * Ref or create an nhe that matches the current state of the
+	 * nexthop(s).
+	 */
+	new_nhe = zebra_nhg_rib_find_nhe(curr_nhe, rt_afi);
+
+	if (IS_ZEBRA_DEBUG_NHG_DETAIL)
+		zlog_debug("%s: re %p CHANGED: nhe %p (%pNG) => new_nhe %p (%pNG)", __func__, re,
+			   re->nhe, re->nhe, new_nhe, new_nhe);
+
+	route_entry_update_nhe(re, new_nhe);
+
+	/*
+	 * Do not need the old / copied nhe anymore since it
+	 * was either copied over into a new nhe or not
+	 * used at all.
+	 */
+	zebra_nhg_free(curr_nhe);
+}
+
 /*
  * This function takes the start of two comparable nexthops from two different
  * nexthop groups and walks them to see if they can be considered the same

zebra/zebra_nhg.h

@@ -401,6 +401,7 @@ extern void zebra_nhg_mark_keep(void);
 
 /* Nexthop resolution processing */
 struct route_entry; /* Forward ref to avoid circular includes */
+extern void nexthop_vrf_update(struct route_node *rn, struct route_entry *re, vrf_id_t vrf_id);
 extern int nexthop_active_update(struct route_node *rn, struct route_entry *re,
 				 struct route_entry *old_re);
 

zebra/zebra_rib.c

@@ -903,6 +903,11 @@ void zebra_rtable_node_cleanup(struct route_table *table,
 		rib_unlink(node, re);
 	}
 
+	zebra_node_info_cleanup(node);
+}
+
+void zebra_node_info_cleanup(struct route_node *node)
+{
 	if (node->info) {
 		rib_dest_t *dest = node->info;
 
@@ -4498,6 +4503,12 @@ rib_update_handle_kernel_route_down_possibility(struct route_node *rn,
 	bool alive = false;
 
 	for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) {
+		if (!nexthop->ifindex) {
+			/* blackhole nexthops have no interfaces */
+			alive = true;
+			break;
+		}
+
 		struct interface *ifp = if_lookup_by_index(nexthop->ifindex,
 							   nexthop->vrf_id);
 

zebra/zebra_vrf.c

@@ -162,6 +162,69 @@ static int zebra_vrf_enable(struct vrf *vrf)
 	return 0;
 }
 
+/* update the VRF ID of a routing table and their routing entries */
+static void zebra_vrf_disable_update_vrfid(struct zebra_vrf *zvrf, afi_t afi, safi_t safi)
+{
+	struct rib_table_info *info;
+	struct route_entry *re, *nre;
+	struct route_node *rn, *nrn;
+	bool empty_table = true;
+	bool rn_delete;
+
+	/* Assign the table to the default VRF.
+	 * Although the table is not technically owned by the default VRF,
+	 * the code assumes that unassigned routing tables are
+	 * associated with the default VRF.
+	 */
+	info = route_table_get_info(zvrf->table[afi][safi]);
+	info->zvrf = vrf_info_lookup(VRF_DEFAULT);
+
+	rn = route_top(zvrf->table[afi][safi]);
+	if (rn)
+		empty_table = false;
+	while (rn) {
+		if (!rn->info) {
+			rn = route_next(rn);
+			continue;
+		}
+
+		/* Assign the kernel route entries to the default VRF,
+		 * even though they are not actually owned by it.
+		 *
+		 * Remove route nodes that were created by FRR daemons,
+		 * unless they are associated with the table rather than the VRF.
+		 * Routes associated with the VRF are not needed once the VRF is
+		 * disabled.
+		 */
+		rn_delete = true;
+		RNODE_FOREACH_RE_SAFE (rn, re, nre) {
+			if (re->type == ZEBRA_ROUTE_KERNEL ||
+			    CHECK_FLAG(re->flags, ZEBRA_FLAG_TABLEID)) {
+				nexthop_vrf_update(rn, re, VRF_DEFAULT);
+				if (CHECK_FLAG(re->flags, ZEBRA_FLAG_TABLEID))
+					/* reinstall routes */
+					rib_install_kernel(rn, re, NULL);
+				rn_delete = false;
+			} else
+				rib_unlink(rn, re);
+		}
+		if (rn_delete) {
+			nrn = route_next(rn);
+			zebra_node_info_cleanup(rn);
+			rn->info = NULL;
+			route_unlock_node(rn);
+			rn = nrn;
+		} else {
+			empty_table = false;
+			rn = route_next(rn);
+		}
+	}
+
+	if (empty_table)
+		zebra_router_release_table(zvrf, zvrf->table_id, afi, safi);
+	zvrf->table[afi][safi] = NULL;
+}
+
 /* Callback upon disabling a VRF. */
 static int zebra_vrf_disable(struct vrf *vrf)
 {
@@ -224,9 +287,13 @@ static int zebra_vrf_disable(struct vrf *vrf)
 		 * we no-longer need this pointer.
 		 */
 		for (safi = SAFI_UNICAST; safi <= SAFI_MULTICAST; safi++) {
-			zebra_router_release_table(zvrf, zvrf->table_id, afi,
+			if (!zvrf->table[afi][safi] || vrf->vrf_id == VRF_DEFAULT) {
-						   safi);
+				zebra_router_release_table(zvrf, zvrf->table_id, afi, safi);
-			zvrf->table[afi][safi] = NULL;
+				zvrf->table[afi][safi] = NULL;
+				continue;
+			}
+
+			zebra_vrf_disable_update_vrfid(zvrf, afi, safi);
 		}
 	}
 
@@ -357,19 +424,50 @@ static void zebra_rnhtable_node_cleanup(struct route_table *table,
 static void zebra_vrf_table_create(struct zebra_vrf *zvrf, afi_t afi,
 				   safi_t safi)
 {
+	vrf_id_t vrf_id = zvrf->vrf->vrf_id;
+	struct rib_table_info *info;
+	struct route_entry *re;
 	struct route_node *rn;
 	struct prefix p;
 
 	assert(!zvrf->table[afi][safi]);
 
+	/* Attempt to retrieve the Linux routing table using zvrf->table_id.
+	 * If the table was created before the VRF, it will already exist.
+	 * Otherwise, create a new table.
+	 */
 	zvrf->table[afi][safi] =
 		zebra_router_get_table(zvrf, zvrf->table_id, afi, safi);
 
+	/* If the table existed before the VRF was created, info->zvrf was
+	 * referring to the default VRF.
+	 * Assign the table to the new VRF.
+	 * Note: FRR does not allow multiple VRF interfaces to be created with the
+	 * same table ID.
+	 */
+	info = route_table_get_info(zvrf->table[afi][safi]);
+	info->zvrf = zvrf;
+
+	/* If the table existed before the VRF was created, their routing entries
+	 * was owned by the default VRF.
+	 * Re-assign all the routing entries to the new VRF.
+	 */
+	for (rn = route_top(zvrf->table[afi][safi]); rn; rn = route_next(rn)) {
+		if (!rn->info)
+			continue;
+
+		RNODE_FOREACH_RE (rn, re)
+			nexthop_vrf_update(rn, re, vrf_id);
+	}
+
 	memset(&p, 0, sizeof(p));
 	p.family = afi2family(afi);
 
+	/* create a fake default route or get the existing one */
 	rn = srcdest_rnode_get(zvrf->table[afi][safi], &p, NULL);
-	zebra_rib_create_dest(rn);
+	if (!rn->info)
+		/* do not override the existing default route */
+		zebra_rib_create_dest(rn);
 }
 
 /* Allocate new zebra VRF. */

zebra/zebra_vrf.h

@@ -263,6 +263,8 @@ extern void zebra_vrf_init(void);
 
 extern void zebra_rtable_node_cleanup(struct route_table *table,
 				      struct route_node *node);
+extern void zebra_node_info_cleanup(struct route_node *node);
+
 
 #ifdef __cplusplus
 }