From: Igor Putovny <igor.putovny@nic.cz>
Date: Fri, 28 Feb 2025 17:12:47 +0000 (+0100)
Subject: Bugfix
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b752295fc874d5ed0d6cdbd0362fa3cd83b1fef;p=thirdparty%2Fbird.git

Bugfix

This was another bug tricky to find.
It manifested in a following way. After running aggregator with configuration A,
reconfiguring it to B (this works now as it was fixed in the previous commit)
and reconfiguring back to A, no routes from A were exported to the routing
table, despite prefixes being present in the trie.

It was found that aggregator_bucket_update() is correctly called, however, it
returned on the first "if", meaning that bucket was empty. It was found that
there were two different buckets with the same ID. The culprit turned out to
be freeing bucket ID when the bucket becomes empty in rt_notify() due to
reconfiguration. When that happens, freed ID will be used again for new bucket
created during reconfiguration.
However, the bucket pointer in the bucket list was never updated and pointer to
old bucket was still kept there at position [ID]. When the aggregator decided
for creating new route after reconfiguring back to A, it found bucket pointer
by its ID. However, this pointer was actually pointer to an empty bucket.
This triggered early return from aggregator_bucket_update(), which means that
control flow never reached the point where new route could be exported.
Either way, the empty bucket means that rte_update2() would be called with
wrong arguments and route export would still be unsuccessful.
---

diff --git a/proto/aggregator/aggregator.c b/proto/aggregator/aggregator.c
index da61bf183..b0e8980b7 100644
--- a/proto/aggregator/aggregator.c
+++ b/proto/aggregator/aggregator.c
@@ -724,7 +724,6 @@ aggregator_rt_notify(struct proto *P, struct channel *src_ch, net *net, rte *new
   if (old_bucket && (!old_bucket->rte || !old_bucket->count))
   {
     ASSERT_DIE(!old_bucket->rte && !old_bucket->count);
-    hmap_clear(&p->bucket_id_map, old_bucket->id);
     HASH_REMOVE2(p->buckets, AGGR_BUCK, p->p.pool, old_bucket);
   }
 }