Merge pull request FRRouting#13808 from anlancs/fix/zebra-kernel-rout…

…e-reserved zebra: fix wrong nexthop check for kernel routes
opensourcerouting · Jul 6, 2023 · 2ec7477 · 2ec7477
2 parents d42ec9c + 019ac03
commit 2ec7477
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 7 deletions.
diff --git a/tests/topotests/zebra_rib/r1/v4_route_1_vrf_before.json b/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
+ }
+ ]
+ }
+ ]
+}
diff --git a/tests/topotests/zebra_rib/test_zebra_rib.py b/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")

diff --git a/zebra/zebra_nhg.c b/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 or its a kernel
- * route and interface is up, its active. We trust kernel routes
- * to be good.
- */
+ /* If the interface exists and its operative, it's active */
  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,