From 859b0dc505b6ab7770d3be2daf40c33de9895a9c Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 30 Nov 2023 14:04:56 +0100 Subject: [PATCH] zebra: remove proto nhid from system, when NHG_DEL is sent When suppressing a nexthop group, the proto nhid is still present in the system: > ubuntu2204(config)# nexthop-group LL > 2023/12/04 21:23:44 SHARP: [Q5NBA-GN1BG] NHG ID assigned: 179687510 > ubuntu2204(config-nh-group)# nexthop 192.0.2.43 loop1 > ubuntu2204(config-nh-group)# no nexthop 192.0.2.43 loop1 > 2023/12/04 21:23:56 ZEBRA: [RY75W-058E9] zebra_nhg_proto_del: deleted nhe 0x55edc18eb870 (179687510[260]), vrf 0, type sharp > 2023/12/04 21:23:56 ZEBRA: [WDEB1-93HCZ] zebra_nhg_decrement_ref: nhe 0x55edc18eb870 (179687510[260]) 1 => 0 > 2023/12/04 21:23:56 SHARP: [H3QKG-WH8ZV] Removed nhg 179687510 > ubuntu2204(config-nh-group)# > ubuntu2204(config-nh-group)# exi > ubuntu2204(config)# > # ip nexthop ls > id 179687510 group 260 proto 194 > id 260 via 192.0.2.43 dev loop1 scope link proto 194 There is no need to keep nexthop group from protocol levels if there are no references to that nexthop group. Fix this by forcing the system removal only if it is a protocol nhid. Fixes: 35729f38fa57 ("zebra: Add a timer to nexthop group deletion") Signed-off-by: Philippe Guibert --- .../test_all_protocol_startup.py | 36 +++++++++++++++++++ zebra/zebra_nhg.c | 3 +- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py index ca340749feb7..62ed2c5f85cf 100644 --- a/tests/topotests/all_protocol_startup/test_all_protocol_startup.py +++ b/tests/topotests/all_protocol_startup/test_all_protocol_startup.py @@ -465,6 +465,20 @@ def verify_nexthop_group(nhg_id, recursive=False, ecmp=0): "Nexthop Group ID=%d not marked Installed" % nhg_id ) +def verify_nexthop_group_on_system(nhg_id, present): + # Verify nhgid presence on the system + net = get_topogen().net + output = net["r1"].cmd("ip nexthop show id %s" % nhg_id) + match = re.search(r"id %s" % nhg_id, output) + if present: + assert match is not None, ( + "Nexthop Group ID=%d should be present on the system, but is not" % nhg_id + ) + else: + assert match is None, ( + "Nexthop Group ID=%d should not be present on the system, but is not" % nhg_id + ) + def verify_route_nexthop_group(route_str, recursive=False, ecmp=0): # Verify route and that zebra created NHGs for and they are valid/installed @@ -613,6 +627,28 @@ def test_nexthop_groups(): % nhg_id ) + ## nexthop suppression + + net["r1"].cmd( + 'vtysh -c "c t" -c "nexthop-group suppress" -c "nexthop 192.168.0.165 r1-eth0"' + ) + net["r1"].cmd( + 'vtysh -c "sharp install routes 7.7.7.1 nexthop-group suppress 1"' + ) + verify_route_nexthop_group("7.7.7.1/32") + nhg_id = route_get_nhg_id("7.7.7.1/32") + verify_nexthop_group_on_system(nhg_id, True) + net["r1"].cmd( + 'vtysh -c "sharp remove routes 7.7.7.1 1"' + ) + net["r1"].cmd( + 'vtysh -c "c t" -c "nexthop-group suppress" -c "no nexthop 192.168.0.165 r1-eth0"' + ) + net["r1"].cmd( + 'vtysh -c "c t" -c "no nexthop-group suppress"' + ) + verify_nexthop_group_on_system(nhg_id, False) + ## Remove all NHG routes net["r1"].cmd('vtysh -c "sharp remove routes 2.2.2.1 1"') diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index c172f18cf190..73a7ab7c4924 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -1719,7 +1719,8 @@ void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe) nhe->refcnt--; - if (!zebra_router_in_shutdown() && nhe->refcnt <= 0 && + if (!PROTO_OWNED(nhe) && !zebra_router_in_shutdown() && + nhe->refcnt <= 0 && CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_INSTALLED) && !CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_KEEP_AROUND)) { nhe->refcnt = 1;