From 5ed9dcb1b64c8251d9afa39f55d5f589b69504b3 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Wed, 29 May 2024 08:18:31 +0200
Subject: [PATCH] BGP: Export uses common attribute cache

There is no real need for storing bucket attributes locally and we may
save some memory by caching the attributes in one central place.

If this becomes a contention problem, we should reduce the lock load
of the central attribute cache.
---
 proto/bgp/attrs.c   | 31 +++++++++++++++----------------
 proto/bgp/bgp.h     |  4 ++--
 proto/bgp/packets.c |  4 ++--
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 2b9f4717..58c76d59 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1574,10 +1574,10 @@ bgp_finish_attrs(struct bgp_parse_state *s, ea_list **to)
  *	Route bucket hash table
  */
 
-#define RBH_KEY(b)		b->eattrs, b->hash
+#define RBH_KEY(b)		b->attrs
 #define RBH_NEXT(b)		b->next
-#define RBH_EQ(a1,h1,a2,h2)	h1 == h2 && ea_same(a1, a2)
-#define RBH_FN(a,h)		h
+#define RBH_EQ(a1,a2)		a1 == a2
+#define RBH_FN(a)		ea_get_storage(a)->hash_key
 
 #define RBH_REHASH		bgp_rbh_rehash
 #define RBH_PARAMS		/8, *2, 2, 2, 12, 20
@@ -1589,6 +1589,7 @@ static void
 bgp_init_bucket_table(struct bgp_channel *c)
 {
   HASH_INIT(c->bucket_hash, c->pool, 8);
+  c->bucket_slab = sl_new(c->pool, sizeof(struct bgp_bucket));
 
   init_list(&c->bucket_queue);
   c->withdraw_bucket = NULL;
@@ -1598,24 +1599,21 @@ static struct bgp_bucket *
 bgp_get_bucket(struct bgp_channel *c, ea_list *new)
 {
   /* Hash and lookup */
-  u32 hash = ea_hash(new);
-  struct bgp_bucket *b = HASH_FIND(c->bucket_hash, RBH, new, hash);
+  ea_list *ns = ea_lookup(new, 0, EALS_CUSTOM);
+  struct bgp_bucket *b = HASH_FIND(c->bucket_hash, RBH, ns);
 
   if (b)
+  {
+    ea_free(ns);
     return b;
-
-  /* Scan the list for total size */
-  uint ea_size = BIRD_CPU_ALIGN(ea_list_size(new));
-  uint size = sizeof(struct bgp_bucket) + ea_size;
+  }
 
   /* Allocate the bucket */
-  b = mb_alloc(c->pool, size);
-  *b = (struct bgp_bucket) { };
+  b = sl_alloc(c->bucket_slab);
+  *b = (struct bgp_bucket) {
+    .attrs = ns,
+  };
   init_list(&b->prefixes);
-  b->hash = hash;
-
-  /* Copy the ea_list */
-  ea_list_copy(b->eattrs, new, ea_size);
 
   /* Insert the bucket to bucket hash */
   HASH_INSERT2(c->bucket_hash, RBH, c->pool, b);
@@ -1639,7 +1637,8 @@ static void
 bgp_free_bucket(struct bgp_channel *c, struct bgp_bucket *b)
 {
   HASH_REMOVE2(c->bucket_hash, RBH, c->pool, b);
-  mb_free(b);
+  ea_free(b->attrs);
+  sl_free(b);
 }
 
 int
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 9f0402f9..74097cb8 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -401,6 +401,7 @@ struct bgp_channel {
 
   HASH(struct bgp_prefix) prefix_hash;	/* Prefixes to be sent */
   slab *prefix_slab;			/* Slab holding prefix nodes */
+  slab *bucket_slab;			/* Slab holding buckets to send */
 //  struct rt_exporter prefix_exporter;	/* Table-like exporter for ptx */
 
   ip_addr next_hop_addr;		/* Local address for NEXT_HOP attribute */
@@ -442,10 +443,9 @@ struct bgp_bucket {
   node send_node;			/* Node in send queue */
   struct bgp_bucket *next;		/* Node in bucket hash table */
   list prefixes;			/* Prefixes to send in this bucket (struct bgp_prefix) */
-  u32 hash;				/* Hash over extended attributes */
+  ea_list *attrs;			/* Attributes to encode */
   u32 px_uc:31;				/* How many prefixes are linking this bucket */
   u32 bmp:1;				/* Temporary bucket for BMP encoding */
-  ea_list eattrs[0];			/* Per-bucket extended attributes */
 };
 
 struct bgp_export_state {
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index 78962c62..31145ab9 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -2310,7 +2310,7 @@ bgp_create_ip_reach(struct bgp_write_state *s, struct bgp_bucket *buck, byte *bu
 
   int lr, la;
 
-  la = bgp_encode_attrs(s, buck->eattrs, buf+4, buf + MAX_ATTRS_LENGTH);
+  la = bgp_encode_attrs(s, buck->attrs, buf+4, buf + MAX_ATTRS_LENGTH);
   if (la < 0)
   {
     /* Attribute list too long */
@@ -2359,7 +2359,7 @@ bgp_create_mp_reach(struct bgp_write_state *s, struct bgp_bucket *buck, byte *bu
 
   /* Encode attributes to temporary buffer */
   byte *abuf = alloca(MAX_ATTRS_LENGTH);
-  la = bgp_encode_attrs(s, buck->eattrs, abuf, abuf + MAX_ATTRS_LENGTH);
+  la = bgp_encode_attrs(s, buck->attrs, abuf, abuf + MAX_ATTRS_LENGTH);
   if (la < 0)
   {
     /* Attribute list too long */