From: Igor Putovny <igor.putovny@nic.cz>
Date: Fri, 25 Apr 2025 11:44:51 +0000 (+0200)
Subject: More comments
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=24a3cbf094a6e8d81e7eb874978444560486f324;p=thirdparty%2Fbird.git

More comments
---

diff --git a/proto/aggregator/aggregator.c b/proto/aggregator/aggregator.c
index b70147d4f..e8759b9ea 100644
--- a/proto/aggregator/aggregator.c
+++ b/proto/aggregator/aggregator.c
@@ -128,6 +128,9 @@ aggregator_resize_slab(struct aggregator_proto *p, u32 id)
   p->root = aggregator_root_init(default_rte_bucket, p->trie_slab);
 }
 
+/*
+ * Build and aggregate trie after initial feed ends
+ */
 static void
 aggregator_aggregate_on_feed_end(struct channel *C)
 {
@@ -278,7 +281,7 @@ aggregator_bucket_update(struct aggregator_proto *p, struct aggregator_bucket *b
     log("===================================================");
   }
 
-  /* merge filter needs one argument called "routes" */
+  /* Merge filter needs one argument called "routes" */
   struct f_val val = {
     .type = T_ROUTES_BLOCK,
     .val.rte = bucket->rte,
@@ -538,7 +541,7 @@ aggregator_rt_notify(struct proto *P, struct channel *src_ch, net *net, rte *new
     if (old_route && rte_same(&old_route->rte, new))
       return;
 
-    /* Evaluate route attributes. */
+    /* Evaluate route attributes */
     struct aggregator_bucket *tmp_bucket = allocz(sizeof(*tmp_bucket) + sizeof(tmp_bucket->aggr_data[0]) * p->aggr_on_count);
     ASSERT_DIE(tmp_bucket->id == 0);
 
@@ -760,10 +763,9 @@ aggregator_rt_notify(struct proto *P, struct channel *src_ch, net *net, rte *new
   }
   else if (p->aggr_mode == PREFIX_AGGR)
   {
-    /* When receiving initial feed, whole trie is aggregated at once */
+    /* After initial feed, recompute trie after receiving incremental update */
     if (!p->initial_feed)
     {
-      /* After initial feed, recompute after receiving incremental update */
       aggregator_recompute(p, old_route, new_route);
 
       /* Process route withdrawals triggered by recomputation */
@@ -877,14 +879,15 @@ aggregator_trie_init(struct aggregator_proto *p)
   mem_hash_init(&haux);
   new_bucket->hash = mem_hash_value(&haux);
 
-  /* Assign ID to the root node bucket */
+  /* Assign ID to the bucket */
   new_bucket->id = hmap_first_zero(&p->bucket_id_map);
   hmap_set(&p->bucket_id_map, new_bucket->id);
 
-  /* Add bucket pointer to the list of pointers */
+  /* Save bucket pointer */
   aggregator_add_bucket(p, new_bucket);
   p->buckets_count++;
 
+  /* Prepare route */
   struct aggregator_route *arte = lp_allocz(p->route_pool, sizeof(*arte));
 
   *arte = (struct aggregator_route) {
diff --git a/proto/aggregator/trie.c b/proto/aggregator/trie.c
index acf6f111f..2966e7f75 100644
--- a/proto/aggregator/trie.c
+++ b/proto/aggregator/trie.c
@@ -76,7 +76,7 @@
  *
  * After every aggregation, following invariants are always satisfied:
  *
- *   1. All nodes have some bucket.
+ *   1. All nodes have original bucket set.
  *   2. All nodes have the IN_FIB ancestor pointer set.
  *   3. If a node is IN_FIB, then
  *        a) its selected bucket must not be null,
@@ -705,9 +705,9 @@ aggregator_process_one_child_nodes(struct trie_node *node, const struct aggregat
 {
   ASSERT_DIE(node != NULL);
 
-  /* Imaginary node that would have been added during normalization of the trie */
   const size_t node_size = sizeof(*node) + sizeof(node->potential_buckets[0]) * bitmap_size;
 
+  /* Imaginary node that would have been added during normalization of the trie */
   struct trie_node *imaginary_node = allocz(node_size);
 
   *imaginary_node = (struct trie_node) {
@@ -781,7 +781,7 @@ aggregator_export_node_prefix(struct aggregator_proto *p, struct trie_node *node
   {
     ASSERT_DIE(old_bucket != NULL);
 
-    /* Node's bucket has changed, remove old route before exporting new */
+    /* Node's bucket has changed, remove old route and export new */
     if (old_bucket && old_bucket != node->selected_bucket)
     {
       aggregator_prepare_rte_withdrawal(p, prefix, pxlen, old_bucket);
@@ -1088,7 +1088,6 @@ aggregator_recompute(struct aggregator_proto *p, struct aggregator_route *old, s
   struct trie_node *ancestor = updated_node;
 
   /* Find the closest IN_FIB ancestor of the updated node */
-  // TODO: use node ancestor pointer instead of traversing
   while (ancestor = ancestor->parent)
   {
     ASSERT_DIE(ancestor != updated_node);