From b901cca2df7c57c0e5840454115bb5e12601a714 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Fri, 13 Oct 2023 10:22:09 +0200
Subject: [PATCH] Protocol: better granularity of pool management

There are now 3 different pools with specific lifetime. All of these are
available since protocol start, anyway they get freed in different
moments.

First, pool_up gets freed immediately after announcing PS_STOP, to e.g.
stop all timers and events regularly updating the routing table when the
imports are already flushing.

Then, pool_inloop gets freed just before the protocol loop is finally
stopped, after all channels, imports and exports and other hooks are
cleaned up.

And finally, the pool itself is freed the last. Unless you explicitly
need the early free, use this pool.
---
 nest/proto.c      | 12 ++++++++----
 nest/protocol.h   |  4 +++-
 proto/bgp/bgp.c   |  2 +-
 proto/ospf/ospf.c |  5 +----
 proto/rip/rip.c   |  6 +++---
 5 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/nest/proto.c b/nest/proto.c
index 9da594dc..cd887a48 100644
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -1306,10 +1306,10 @@ proto_event(void *ptr)
     p->do_stop = 0;
   }
 
-  if (proto_is_done(p) && p->pool_fragile)  /* perusing pool_fragile to do this once only */
+  if (proto_is_done(p) && p->pool_inloop)  /* perusing pool_inloop to do this once only */
   {
-    rp_free(p->pool_fragile);
-    p->pool_fragile = NULL;
+    rp_free(p->pool_inloop);
+    p->pool_inloop = NULL;
     if (p->loop != &main_birdloop)
       birdloop_stop_self(p->loop, proto_loop_stopped, p);
     else
@@ -1391,7 +1391,8 @@ proto_start(struct proto *p)
 
   PROTO_LOCKED_FROM_MAIN(p)
   {
-    p->pool_fragile = rp_newf(p->pool, birdloop_domain(p->loop), "Protocol %s fragile objects", p->cf->name);
+    p->pool_inloop = rp_newf(p->pool, birdloop_domain(p->loop), "Protocol %s early cleanup objects", p->cf->name);
+    p->pool_up = rp_newf(p->pool, birdloop_domain(p->loop), "Protocol %s stop-free objects", p->cf->name);
     proto_notify_state(p, (p->proto->start ? p->proto->start(p) : PS_UP));
   }
 }
@@ -2324,6 +2325,9 @@ proto_do_stop(struct proto *p)
     p->main_source = NULL;
   }
 
+  rp_free(p->pool_up);
+  p->pool_up = NULL;
+
   proto_stop_channels(p);
   rt_destroy_sources(&p->sources, p->event);
 
diff --git a/nest/protocol.h b/nest/protocol.h
index 70b2022d..635f84b9 100644
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -139,7 +139,9 @@ struct proto {
   struct proto_config *cf;		/* Configuration data */
   struct proto_config *cf_new;		/* Configuration we want to switch to after shutdown (NULL=delete) */
   pool *pool;				/* Pool containing local objects */
-  pool *pool_fragile;			/* Pool containing fragile local objects which need to be freed
+  pool *pool_up;			/* Pool containing local objects which should be dropped as soon
+					   as the protocol enters the STOP / DOWN state */
+  pool *pool_inloop;			/* Pool containing local objects which need to be freed
 					   before the protocol's birdloop actually stops, like olocks */
   event *event;				/* Protocol event */
   timer *restart_timer;			/* Timer to restart the protocol from limits */
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 2257a8a9..d14facfd 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -1781,7 +1781,7 @@ bgp_start(struct proto *P)
    * so that we are the only instance attempting to talk with that neighbor.
    */
   struct object_lock *lock;
-  lock = p->lock = olock_new(P->pool_fragile);
+  lock = p->lock = olock_new(P->pool_inloop);
   lock->addr = p->remote_ip;
   lock->port = p->cf->remote_port;
   lock->iface = p->cf->iface;
diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c
index 896bf5a3..cd839656 100644
--- a/proto/ospf/ospf.c
+++ b/proto/ospf/ospf.c
@@ -295,7 +295,7 @@ ospf_start(struct proto *P)
   p->gr_mode = c->gr_mode;
   p->gr_time = c->gr_time;
   p->tick = c->tick;
-  p->disp_timer = tm_new_init(P->pool, ospf_disp, p, p->tick S, 0);
+  p->disp_timer = tm_new_init(P->pool_up, ospf_disp, p, p->tick S, 0);
   tm_start(p->disp_timer, 100 MS);
   p->lsab_size = 256;
   p->lsab_used = 0;
@@ -539,9 +539,6 @@ ospf_shutdown(struct proto *P)
   }
   FIB_WALK_END;
 
-  if (tm_active(p->disp_timer))
-    tm_stop(p->disp_timer);
-
   return PS_DOWN;
 }
 
diff --git a/proto/rip/rip.c b/proto/rip/rip.c
index e09d082b..a8f9ffcd 100644
--- a/proto/rip/rip.c
+++ b/proto/rip/rip.c
@@ -769,8 +769,8 @@ rip_add_iface(struct rip_proto *p, struct iface *iface, struct rip_iface_config
 
   add_tail(&p->iface_list, NODE ifa);
 
-  ifa->timer = tm_new_init(p->p.pool, rip_iface_timer, ifa, 0, 0);
-  ifa->rxmt_timer = tm_new_init(p->p.pool, rip_rxmt_timeout, ifa, 0, 0);
+  ifa->timer = tm_new_init(p->p.pool_up, rip_iface_timer, ifa, 0, 0);
+  ifa->rxmt_timer = tm_new_init(p->p.pool_up, rip_rxmt_timeout, ifa, 0, 0);
 
   struct object_lock *lock = olock_new(p->p.pool);
   lock->type = OBJLOCK_UDP;
@@ -1220,7 +1220,7 @@ rip_start(struct proto *P)
   fib_init(&p->rtable, P->pool, cf->rip2 ? NET_IP4 : NET_IP6,
 	   sizeof(struct rip_entry), OFFSETOF(struct rip_entry, n), 0, NULL);
   p->rte_slab = sl_new(P->pool, sizeof(struct rip_rte));
-  p->timer = tm_new_init(P->pool, rip_timer, p, 0, 0);
+  p->timer = tm_new_init(P->pool_up, rip_timer, p, 0, 0);
 
   p->rip2 = cf->rip2;
   p->ecmp = cf->ecmp;