Skip to content

Commit b0f36d0

Browse files
ton31337ton31337
authored
Merge pull request #18751 from donaldsharp/keep_original_nhe
Keep the original NHE associated with a re around
2 parents a8a5e76 + 9524330 commit b0f36d0

File tree

6 files changed

+331
-0
lines changed

6 files changed

+331
-0
lines changed

‎tests/topotests/bgp_redistribute_table/test_bgp_redistribute_table.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ def _router_json_cmp_exact_filter(router, cmd, expected):
108108
attr.pop("prefixLen")
109109
if "asPath" in attr:
110110
attr.pop("asPath")
111+
if "receivedNexthopGroupId" in attr:
112+
attr.pop("receivedNexthopGroupId")
111113
for nexthop in attr.get("nexthops", []):
112114
if "flags" in nexthop:
113115
nexthop.pop("flags")

‎tests/topotests/zebra_received_nhe_kept/r1/frr.conf

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
!
2+
interface r1-eth0
3+
ip address 192.168.1.1/24
4+
!
5+
!
6+
ip route 10.0.0.0/32 192.168.1.1
7+
ip route 10.0.0.1/32 10.0.0.0
8+
ip route 10.0.0.2/32 10.0.0.0
9+
ip route 10.0.0.3/32 10.0.0.0
10+
ip route 10.0.0.4/32 10.0.0.0
11+
ip route 10.0.0.5/32 10.0.0.0
12+
!
13+
line vty
14+
!

‎tests/topotests/zebra_received_nhe_kept/test_zebra_received_nhe_kept.py

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
#!/usr/bin/env python
2+
# SPDX-License-Identifier: ISC
3+
4+
#
5+
# test_zebra_received_nhe_kept.py
6+
#
7+
# Copyright (c) 2025 by Donald Sharp, Nvidia Inc.
8+
#
9+
10+
"""
11+
test_zebra_received_nhe_kept.py: Test of Zebra Next Hop Entry Kept
12+
"""
13+
14+
import os
15+
import sys
16+
import pytest
17+
import json
18+
from functools import partial
19+
20+
CWD = os.path.dirname(os.path.realpath(__file__))
21+
sys.path.append(os.path.join(CWD, "../"))
22+
23+
# pylint: disable=C0413
24+
from lib import topotest
25+
from lib.topogen import Topogen, TopoRouter, get_topogen
26+
from lib.topolog import logger
27+
from lib.common_config import step
28+
29+
30+
def build_topo(tgen):
31+
"Build function"
32+
33+
# Create router r1
34+
tgen.add_router("r1")
35+
36+
# Create a switch to connect to r1's interface
37+
switch = tgen.add_switch("s1")
38+
switch.add_link(tgen.gears["r1"])
39+
40+
41+
def setup_module(mod):
42+
"Sets up the pytest environment"
43+
tgen = Topogen(build_topo, mod.__name__)
44+
tgen.start_topology()
45+
46+
# For all registered routers, load the zebra configuration file
47+
for rname, router in tgen.routers().items():
48+
logger.info("Loading router %s" % rname)
49+
router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname)))
50+
51+
# After loading the configurations, this function loads configured daemons.
52+
tgen.start_router()
53+
54+
55+
def teardown_module():
56+
"Teardown the pytest environment"
57+
tgen = get_topogen()
58+
59+
# This function tears down the whole topology.
60+
tgen.stop_topology()
61+
62+
63+
def test_zebra_received_nhe_kept():
64+
"Test that zebra keeps received next hop entries"
65+
tgen = get_topogen()
66+
if tgen.routers_have_failure():
67+
pytest.skip(tgen.errors)
68+
69+
r1 = tgen.gears["r1"]
70+
71+
step("Get the route information")
72+
# Get the route information
73+
route_info = r1.vtysh_cmd("show ip route json")
74+
route_json = json.loads(route_info)
75+
76+
# Get the NHG ID for 10.0.0.1
77+
nhg_id = None
78+
for prefix, route in route_json.items():
79+
if prefix == "10.0.0.1/32":
80+
nhg_id = route[0].get("receivedNexthopGroupId")
81+
break
82+
83+
step("Verify all routes 10.0.0.1-.5 have the same NHG ID {}".format(nhg_id))
84+
# Verify all routes 10.0.0.1-.5 have the same NHG ID
85+
for i in range(1, 6):
86+
prefix = f"10.0.0.{i}/32"
87+
assert prefix in route_json, f"Route {prefix} not found"
88+
route = route_json[prefix][0]
89+
assert (
90+
"receivedNexthopGroupId" in route
91+
), f"Route {prefix} missing receivedNexthopGroupId"
92+
assert (
93+
route["receivedNexthopGroupId"] == nhg_id
94+
), f"Route {prefix} has different NHG ID"
95+
96+
step("Verify NHG {} has refcount of 10".format(nhg_id))
97+
# Get the NHG information
98+
nhg_info = r1.vtysh_cmd("show nexthop-group rib {} json".format(nhg_id))
99+
nhg_json = json.loads(nhg_info)
100+
assert nhg_json[str(nhg_id)]["refCount"] == 10, "NHG refcount is not 10"
101+
102+
103+
def test_zebra_received_nhe_kept_remove_routes():
104+
"Test that zebra updates refcount when routes are removed"
105+
tgen = get_topogen()
106+
if tgen.routers_have_failure():
107+
pytest.skip(tgen.errors)
108+
109+
r1 = tgen.gears["r1"]
110+
111+
step("Remove routes 10.0.0.3, 10.0.0.4, and 10.0.0.5")
112+
# Remove the routes
113+
for i in range(3, 6):
114+
r1.vtysh_cmd(
115+
"configure terminal\n no ip route 10.0.0.{}/32 10.0.0.0\n end".format(i)
116+
)
117+
118+
# Wait for zebra to process the route removals
119+
def check_routes_removed():
120+
route_info = r1.vtysh_cmd("show ip route json")
121+
route_json = json.loads(route_info)
122+
123+
# Check routes 10.0.0.1 and 10.0.0.2 exist
124+
for i in range(1, 3):
125+
prefix = f"10.0.0.{i}/32"
126+
if prefix not in route_json:
127+
return False
128+
129+
# Check routes 10.0.0.3 through 10.0.0.5 don't exist
130+
for i in range(3, 6):
131+
prefix = f"10.0.0.{i}/32"
132+
if prefix in route_json:
133+
return False
134+
135+
return True
136+
137+
step("Wait for routes to be removed")
138+
_, result = topotest.run_and_expect(check_routes_removed, True, count=30, wait=1)
139+
assert result, "Routes were not properly removed"
140+
141+
step("Get the route information")
142+
# Get the route information
143+
route_info = r1.vtysh_cmd("show ip route json")
144+
route_json = json.loads(route_info)
145+
146+
# Get the NHG ID for 10.0.0.1
147+
nhg_id = None
148+
for prefix, route in route_json.items():
149+
if prefix == "10.0.0.1/32":
150+
nhg_id = route[0].get("receivedNexthopGroupId")
151+
break
152+
153+
step("Verify NHG {} has refcount of 2".format(nhg_id))
154+
# Get the NHG information
155+
nhg_info = r1.vtysh_cmd("show nexthop-group rib {} json".format(nhg_id))
156+
nhg_json = json.loads(nhg_info)
157+
assert nhg_json[str(nhg_id)]["refCount"] == 4, "NHG refcount is not 4"
158+
159+
160+
def test_zebra_received_nhe_kept_add_routes():
161+
"Test that zebra updates refcount when routes are added"
162+
tgen = get_topogen()
163+
if tgen.routers_have_failure():
164+
pytest.skip(tgen.errors)
165+
166+
r1 = tgen.gears["r1"]
167+
168+
step("Add routes 10.0.0.3 through 10.0.0.10")
169+
# Add the routes
170+
for i in range(3, 11):
171+
r1.vtysh_cmd(
172+
"configure terminal\n ip route 10.0.0.{}/32 10.0.0.0\n end".format(i)
173+
)
174+
175+
# Wait for zebra to process the route additions
176+
def check_routes_added():
177+
route_info = r1.vtysh_cmd("show ip route json")
178+
route_json = json.loads(route_info)
179+
180+
# Check all routes 10.0.0.1 through 10.0.0.10 exist
181+
for i in range(1, 11):
182+
prefix = f"10.0.0.{i}/32"
183+
if prefix not in route_json:
184+
return False
185+
route = route_json[prefix][0]
186+
if "receivedNexthopGroupId" not in route:
187+
return False
188+
189+
return True
190+
191+
step("Wait for routes to be added")
192+
_, result = topotest.run_and_expect(check_routes_added, True, count=30, wait=1)
193+
assert result, "Routes were not properly added"
194+
195+
step("Get the route information")
196+
# Get the route information
197+
route_info = r1.vtysh_cmd("show ip route json")
198+
route_json = json.loads(route_info)
199+
200+
# Get the NHG ID for 10.0.0.1
201+
nhg_id = None
202+
for prefix, route in route_json.items():
203+
if prefix == "10.0.0.1/32":
204+
nhg_id = route[0].get("receivedNexthopGroupId")
205+
break
206+
207+
step("Verify all routes 10.0.0.1-.10 have the same NHG ID {}".format(nhg_id))
208+
# Verify all routes 10.0.0.1-.10 have the same NHG ID
209+
for i in range(1, 11):
210+
prefix = f"10.0.0.{i}/32"
211+
assert prefix in route_json, f"Route {prefix} not found"
212+
route = route_json[prefix][0]
213+
assert (
214+
"receivedNexthopGroupId" in route
215+
), f"Route {prefix} missing receivedNexthopGroupId"
216+
assert (
217+
route["receivedNexthopGroupId"] == nhg_id
218+
), f"Route {prefix} has different NHG ID"
219+
220+
step("Verify NHG {} has refcount of 10".format(nhg_id))
221+
# Get the NHG information
222+
nhg_info = r1.vtysh_cmd("show nexthop-group rib {} json".format(nhg_id))
223+
nhg_json = json.loads(nhg_info)
224+
assert nhg_json[str(nhg_id)]["refCount"] == 20, "NHG refcount is not 20"
225+
226+
227+
def test_zebra_received_nhe_kept_remove_all_routes():
228+
"Test that zebra removes NHG when all routes are removed"
229+
tgen = get_topogen()
230+
if tgen.routers_have_failure():
231+
pytest.skip(tgen.errors)
232+
233+
r1 = tgen.gears["r1"]
234+
235+
step("Get the NHG ID before removing routes")
236+
# Get the route information
237+
route_info = r1.vtysh_cmd("show ip route 10.0.0.1/32 json")
238+
route_json = json.loads(route_info)
239+
240+
# Get the NHG ID for 10.0.0.1
241+
nhg_id = None
242+
for prefix, route in route_json.items():
243+
if prefix == "10.0.0.1/32":
244+
nhg_id = route[0].get("receivedNexthopGroupId")
245+
break
246+
247+
step("Remove all routes 10.0.0.1 through 10.0.0.10")
248+
# Remove all routes
249+
for i in range(1, 11):
250+
r1.vtysh_cmd(
251+
"configure terminal\n no ip route 10.0.0.{}/32 10.0.0.0\n end".format(i)
252+
)
253+
254+
# Wait for zebra to process the route removals
255+
def check_all_routes_removed():
256+
route_info = r1.vtysh_cmd("show ip route json")
257+
route_json = json.loads(route_info)
258+
259+
# Check no 10.0.0.x routes exist
260+
for i in range(1, 11):
261+
prefix = f"10.0.0.{i}/32"
262+
if prefix in route_json:
263+
return False
264+
265+
return True
266+
267+
step("Wait for all routes to be removed")
268+
_, result = topotest.run_and_expect(
269+
check_all_routes_removed, True, count=30, wait=1
270+
)
271+
assert result, "Routes were not properly removed"
272+
273+
step("Get NHG information")
274+
# Get all NHG information
275+
nhg_info = r1.vtysh_cmd("show nexthop-group rib json")
276+
nhg_json = json.loads(nhg_info)
277+
278+
# Verify the specific NHG we looked up is no longer present
279+
assert (
280+
str(nhg_id) not in nhg_json
281+
), f"NHG {nhg_id} still exists after removing all routes"
282+
283+
284+
if __name__ == "__main__":
285+
args = ["-s"] + sys.argv[1:]
286+
sys.exit(pytest.main(args))

‎zebra/rib.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ struct route_entry {
8585
*/
8686
struct nhg_hash_entry *nhe;
8787

88+
struct nhg_hash_entry *nhe_received;
89+
8890
/*
8991
* Nexthop group hash entry IDs.
9092
* Since the nhe_id is used as a temporary holder

‎zebra/zebra_rib.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,12 @@ static void route_entry_attach_ref(struct route_entry *re,
474474
zebra_nhg_increment_ref(new);
475475
}
476476

477+
static void route_entry_update_original_nhe(struct route_entry *re, struct nhg_hash_entry *nhe)
478+
{
479+
re->nhe_received = nhe;
480+
zebra_nhg_increment_ref(nhe);
481+
}
482+
477483
/* Replace (if 'new_nhghe') or clear (if that's NULL) an re's nhe. */
478484
int route_entry_update_nhe(struct route_entry *re,
479485
struct nhg_hash_entry *new_nhghe)
@@ -503,6 +509,14 @@ int route_entry_update_nhe(struct route_entry *re,
503509
/* Detach / deref previous nhg */
504510

505511
if (old_nhg) {
512+
if (re->nhe_received == old_nhg) {
513+
zebra_nhg_decrement_ref(old_nhg);
514+
}
515+
if (new_nhghe)
516+
zebra_nhg_increment_ref(new_nhghe);
517+
518+
re->nhe_received = new_nhghe;
519+
506520
/*
507521
* Return true if we are deleting the previous NHE
508522
* Note: we dont check the return value of the function anywhere
@@ -2703,6 +2717,11 @@ static void rib_re_nhg_free(struct route_entry *re)
27032717
nexthops_free(re->nhe->nhg.nexthop);
27042718

27052719
nexthops_free(re->fib_ng.nexthop);
2720+
2721+
if (re->nhe_received) {
2722+
zebra_nhg_decrement_ref(re->nhe_received);
2723+
re->nhe_received = NULL;
2724+
}
27062725
}
27072726

27082727
struct zebra_early_route {
@@ -2813,6 +2832,7 @@ static void process_subq_early_route_add(struct zebra_early_route *ere)
28132832
* if old_id != new_id.
28142833
*/
28152834
route_entry_update_nhe(re, nhe);
2835+
route_entry_update_original_nhe(re, nhe);
28162836

28172837
/* Make it sure prefixlen is applied to the prefix. */
28182838
apply_mask(&ere->p);

‎zebra/zebra_vty.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,10 @@ static void vty_show_ip_route_detail(struct vty *vty, struct route_node *rn,
477477
vty_out(vty,
478478
" Installed Nexthop Group ID: %u\n",
479479
re->nhe_installed_id);
480+
481+
if (re->nhe_received)
482+
vty_out(vty, " Received Nexthop Group ID: %u\n",
483+
re->nhe_received->id);
480484
}
481485

482486
for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) {
@@ -589,6 +593,9 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn,
589593
json_object_int_add(json_route,
590594
"installedNexthopGroupId",
591595
re->nhe_installed_id);
596+
if (re->nhe_received)
597+
json_object_int_add(json_route, "receivedNexthopGroupId",
598+
re->nhe_received->id);
592599

593600
json_object_string_add(json_route, "uptime", up_str);
594601

0 commit comments

Comments
 (0)