]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
Table: Do not attempt to prune an empty table
authorMaria Matejka <mq@ucw.cz>
Sat, 24 May 2025 12:16:44 +0000 (14:16 +0200)
committerMaria Matejka <mq@ucw.cz>
Sun, 25 May 2025 19:03:44 +0000 (21:03 +0200)
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.

nest/rt-export.c
nest/rt-table.c
proto/bgp/attrs.c

index 0b626b04c779a324da258e087cb0d1aa48b3a983..8fcce737c726d124777c91245545aae7f6b96f58 100644 (file)
@@ -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
index 8046bcc863528b2297aabd9c251786f72b59f630..f4d6b3043e700dfa4895774fc8762407a9e8d2b0 100644 (file)
@@ -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)
   {
index 75b8d8e53f370e7eabdaf0b9f9edee575c31d462..85aa356cf9dd6c2b28e29c62cb6014c13e3a25a8 100644 (file)
@@ -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,
     },