]> git.proxmox.com Git - mirror_frr.git/commitdiff
zebra: Add a timer to nexthop group deletion
authorDonald Sharp <sharpd@nvidia.com>
Fri, 29 Oct 2021 12:16:13 +0000 (08:16 -0400)
committerDonald Sharp <sharpd@nvidia.com>
Thu, 16 Jun 2022 18:47:19 +0000 (14:47 -0400)
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 <sharpd@nvidia.com>
zebra/zebra_nhg.c
zebra/zebra_nhg.h
zebra/zebra_vty.c

index e27078d138328c32ca3a36132a487ee31b063971..542972d175066945d27d63100ac87f49eb9c7035 100644 (file)
@@ -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;
        }
index 56de336e4724798183a4afdfad07f9cc33c69f69..6d2ab248f9a7d6078967e4e9c81712a47cba95b5 100644 (file)
@@ -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..
  */
index be19b07d9d65b1f44ad2994929258b66a6d8a407..88752edb9fb48566b50a5ec3a2231d7ae205ce37 100644 (file)
@@ -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))