From: Maria Matejka <mq@ucw.cz>
Date: Sat, 24 May 2025 12:16:44 +0000 (+0200)
Subject: Table: Do not attempt to prune an empty table
X-Git-Tag: v3.0.4~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e77d5e7cb6b51cedefef0c10ad4ac65fecc81937;p=thirdparty%2Fbird.git

Table: Do not attempt to prune an empty table

Table pruning is requested from multiple places, including
a timer. Sometimes this may happen in a race condition with
table shutdown, and therefore we explicitly refuse to schedule
pruning if the table is empty.

Also added a full-blown check that a shutting-down table
is indeed empty, and several more asserts to catch imminent crashes
before they happen in hard-to-debug places.

This fixes #254.
---

diff --git a/nest/rt-export.c b/nest/rt-export.c
index 0b626b04c..8fcce737c 100644
--- a/nest/rt-export.c
+++ b/nest/rt-export.c
@@ -448,6 +448,7 @@ void
 rt_exporter_init(struct rt_exporter *e, struct settle_config *scf)
 {
   rtex_trace(e, D_STATES, "Exporter init");
+  ASSERT_DIE(e->journal.domain);
   e->journal.cleanup_done = rt_exporter_cleanup_done;
   lfjour_init(&e->journal, scf);
   ASSERT_DIE(e->feed_net);
@@ -540,10 +541,7 @@ rt_exporter_shutdown(struct rt_exporter *e, void (*stopped)(struct rt_exporter *
   rtex_trace(e, D_STATES, "Exporter shutdown");
 
   /* Last lock check before dropping the domain reference */
-  if (e->journal.domain)
-    ASSERT_DIE(DG_IS_LOCKED(e->journal.domain));
-
-  e->journal.domain = NULL;
+  ASSERT_DIE(DG_IS_LOCKED(e->journal.domain));
 
   /* We have to tell every receiver to stop */
   bool done = 1;
@@ -569,13 +567,22 @@ rt_exporter_shutdown(struct rt_exporter *e, void (*stopped)(struct rt_exporter *
   /* Wait for feeders to finish */
   synchronize_rcu();
 
-  /* The rest is done via the cleanup routine */
-  lfjour_do_cleanup_now(&e->journal);
-
   if (done)
   {
+    /* Inhibit locking in cleanup */
+    e->journal.domain = NULL;
+
+    /* The rest is done via the cleanup routine */
+    lfjour_do_cleanup_now(&e->journal);
+
+    /* Invalidate the cleanup event */
+    e->journal.cleanup_event.hook = NULL;
     ev_postpone(&e->journal.cleanup_event);
+
+    /* No announcement timer either */
     settle_cancel(&e->journal.announce_timer);
+
+    /* Done! */
     CALL(stopped, e);
   }
   else
diff --git a/nest/rt-table.c b/nest/rt-table.c
index 8046bcc86..f4d6b3043 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -2973,6 +2973,10 @@ rt_schedule_nhu(struct rtable_private *tab)
 void
 rt_schedule_prune(struct rtable_private *tab)
 {
+  /* The table is empty if there are no imports */
+  if (EMPTY_LIST(tab->imports))
+    return;
+
   /* state change 0->1, 2->3 */
   tab->prune_state |= 1;
   if (!tab->reconf_end)
@@ -4670,6 +4674,15 @@ rt_shutdown(void *tab_)
 {
   rtable *t = tab_;
   RT_LOCK(t, tab);
+  ASSERT_DIE(birdloop_inside(tab->loop));
+
+  /* Check that the table is indeed pruned */
+  tab->prune_state = 0;
+  ASSERT_DIE(EMPTY_LIST(tab->imports));
+  u32 bs = atomic_load_explicit(&tab->routes_block_size, memory_order_relaxed);
+  net *routes = atomic_load_explicit(&tab->routes, memory_order_relaxed);
+  for (u32 i = 0; i < bs; i++)
+    ASSERT_DIE(atomic_load_explicit(&routes[i].routes, memory_order_relaxed) == NULL);
 
   if (tab->export_digest)
   {
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 75b8d8e53..85aa356cf 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -2122,6 +2122,7 @@ bgp_init_pending_tx(struct bgp_channel *c)
   bpp->exporter = (struct rt_exporter) {
     .journal = {
       .loop = c->c.proto->loop,
+      .domain = dom.rtable,
       .item_size = sizeof(struct rt_export_item),
       .item_done = bgp_out_item_done,
     },