From 35729f38fa5713b923782ca9921c893bb8d3bc25 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 29 Oct 2021 08:16:13 -0400 Subject: [PATCH] zebra: Add a timer to nexthop group deletion Before deleting nexthop groups, that are installed, from the system, start a timer and hold the nexthop group for that time. Suppose you have this scenario a) create a static route with 1 x ecmp creates a nhg with 1 x ecmp b) create a static route with 2 x ecmp creates a nhg with 2 x ecmp deletes a's nhg c) create a static route with 3 x ecmp creates a nhg with 3 x ecmp deletes b's nhg d) create a different route with 1 x ecmp creates another 1 x ecmp ( since a's ecmp was deleted ) e) create a different route with 2 x ecmp creates another 2 x ecmp ( since b's ecmp was deleted ) If you don't delete the nhg, start a timer, the nhg's used in steps a and b can be reused for steps d and e. This reduces overhead work with zebra <-> kernel interactions and improves the speed of the system. So modify the code to note that an installed nexthop group should be kept around a bit and hopefully reused. Signed-off-by: Donald Sharp --- zebra/zebra_nhg.c | 30 +++++++++++++++++++++++++++--- zebra/zebra_nhg.h | 11 +++++++++++ zebra/zebra_vty.c | 10 +++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index e27078d138..542972d175 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -1625,6 +1625,17 @@ void zebra_nhg_hash_free(void *p) zebra_nhg_free((struct nhg_hash_entry *)p); } +static void zebra_nhg_timer(struct thread *thread) +{ + struct nhg_hash_entry *nhe = THREAD_ARG(thread); + + if (IS_ZEBRA_DEBUG_NHG_DETAIL) + zlog_debug("Nexthop Timer for nhe: %pNG", nhe); + + if (nhe->refcnt == 1) + zebra_nhg_decrement_ref(nhe); +} + void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe) { if (IS_ZEBRA_DEBUG_NHG_DETAIL) @@ -1633,6 +1644,15 @@ void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe) nhe->refcnt--; + if (!zrouter.in_shutdown && nhe->refcnt <= 0 && + CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_INSTALLED) && + !CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_KEEP_AROUND)) { + nhe->refcnt = 1; + SET_FLAG(nhe->flags, NEXTHOP_GROUP_KEEP_AROUND); + thread_add_timer(zrouter.master, zebra_nhg_timer, nhe, 180, + &nhe->timer); + } + if (!zebra_nhg_depends_is_empty(nhe)) nhg_connected_tree_decrement_ref(&nhe->nhg_depends); @@ -1648,6 +1668,12 @@ void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe) nhe->refcnt++; + if (thread_is_scheduled(nhe->timer)) { + THREAD_OFF(nhe->timer); + nhe->refcnt--; + UNSET_FLAG(nhe->flags, NEXTHOP_GROUP_KEEP_AROUND); + } + if (!zebra_nhg_depends_is_empty(nhe)) nhg_connected_tree_increment_ref(&nhe->nhg_depends); } @@ -3296,9 +3322,6 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, rib_handle_nhg_replace(old, new); - /* if this != 1 at this point, we have a bug */ - assert(old->refcnt == 1); - /* We have to decrement its singletons * because some might not exist in NEW. */ @@ -3310,6 +3333,7 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, /* Dont call the dec API, we dont want to uninstall the ID */ old->refcnt = 0; + THREAD_OFF(old->timer); zebra_nhg_free(old); old = NULL; } diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h index 56de336e47..6d2ab248f9 100644 --- a/zebra/zebra_nhg.h +++ b/zebra/zebra_nhg.h @@ -105,6 +105,8 @@ struct nhg_hash_entry { */ struct nhg_connected_tree_head nhg_depends, nhg_dependents; + struct thread *timer; + /* * Is this nexthop group valid, ie all nexthops are fully resolved. * What is fully resolved? It's a nexthop that is either self contained @@ -145,6 +147,15 @@ struct nhg_hash_entry { */ #define NEXTHOP_GROUP_PROTO_RELEASED (1 << 5) +/* + * When deleting a NHG notice that it is still installed + * and if it is, slightly delay the actual removal to + * the future. So that upper level protocols might + * be able to take advantage of some NHG's that + * are there + */ +#define NEXTHOP_GROUP_KEEP_AROUND (1 << 6) + /* * Track FPM installation status.. */ diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index be19b07d9d..88752edb9f 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1433,14 +1433,22 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe) struct nhg_connected *rb_node_dep = NULL; struct nexthop_group *backup_nhg; char up_str[MONOTIME_STRLEN]; + char time_left[MONOTIME_STRLEN]; uptime2str(nhe->uptime, up_str, sizeof(up_str)); vty_out(vty, "ID: %u (%s)\n", nhe->id, zebra_route_string(nhe->type)); - vty_out(vty, " RefCnt: %u\n", nhe->refcnt); + vty_out(vty, " RefCnt: %u", nhe->refcnt); + if (thread_is_scheduled(nhe->timer)) + vty_out(vty, " Time to Deletion: %s", + thread_timer_to_hhmmss(time_left, sizeof(time_left), + nhe->timer)); + vty_out(vty, "\n"); + vty_out(vty, " Uptime: %s\n", up_str); vty_out(vty, " VRF: %s\n", vrf_id_to_name(nhe->vrf_id)); + if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_VALID)) { vty_out(vty, " Valid"); if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_INSTALLED)) -- 2.39.5