From a4a7e09478e9862b7e71ba0a88b3c88ccd0a8729 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Tue, 25 Jun 2024 17:08:57 +0200
Subject: [PATCH] Revert "BGP: Export uses common attribute cache"

This reverts commit d01a7c2bdaf50aff23cf5e1b4e494328c5294960.

It seems that the performance penalty in global ea cache is actually
very high so returning back to local attribute caches in every BGP.
---
 proto/bgp/attrs.c   | 35 ++++++++++++++++++-----------------
 proto/bgp/bgp.h     |  3 ++-
 proto/bgp/packets.c |  4 ++--
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index bab8bb5e..7ed898d2 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->attrs
+#define RBH_KEY(b)		b->eattrs, b->hash
 #define RBH_NEXT(b)		b->next
-#define RBH_EQ(a1,a2)		a1 == a2
-#define RBH_FN(a)		ea_get_storage(a)->hash_key
+#define RBH_EQ(a1,h1,a2,h2)	h1 == h2 && ea_same(a1, a2)
+#define RBH_FN(a,h)		h
 
 #define RBH_REHASH		bgp_rbh_rehash
 #define RBH_PARAMS		/8, *2, 2, 2, 12, 20
@@ -1589,7 +1589,6 @@ static void
 bgp_init_bucket_table(struct bgp_ptx_private *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;
@@ -1599,21 +1598,24 @@ static struct bgp_bucket *
 bgp_get_bucket(struct bgp_ptx_private *c, ea_list *new)
 {
   /* Hash and lookup */
-  ea_list *ns = ea_lookup(new, 0, EALS_CUSTOM);
-  struct bgp_bucket *b = HASH_FIND(c->bucket_hash, RBH, ns);
+  u32 hash = ea_hash(new);
+  struct bgp_bucket *b = HASH_FIND(c->bucket_hash, RBH, new, hash);
 
   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 = sl_alloc(c->bucket_slab);
-  *b = (struct bgp_bucket) {
-    .attrs = ns,
-  };
+  b = mb_alloc(c->pool, size);
+  *b = (struct bgp_bucket) { };
   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);
@@ -1637,8 +1639,7 @@ static void
 bgp_free_bucket(struct bgp_ptx_private *c, struct bgp_bucket *b)
 {
   HASH_REMOVE2(c->bucket_hash, RBH, c->pool, b);
-  ea_free(b->attrs);
-  sl_free(b);
+  mb_free(b);
 }
 
 int
@@ -1917,7 +1918,7 @@ bgp_out_feed_net(struct rt_exporter *e, struct rcu_unwinder *u, u32 index, _Bool
       {
 	if (px->cur)
 	  feed->block[pos++] = (rte) {
-	    .attrs = px->cur->attrs ? ea_ref_tmp(px->cur->attrs) : NULL,
+	    .attrs = (px->cur == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->cur->eattrs, 0, EALS_CUSTOM)),
 	    .net = ni->addr,
 	    .src = px->src,
 	    .lastmod = px->lastmod,
@@ -1926,7 +1927,7 @@ bgp_out_feed_net(struct rt_exporter *e, struct rcu_unwinder *u, u32 index, _Bool
 
 	if (px->last)
 	  feed->block[pos++] = (rte) {
-	    .attrs = px->last->attrs ? ea_ref_tmp(px->last->attrs) : NULL,
+	    .attrs = (px->last == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->last->eattrs, 0, EALS_CUSTOM)),
 	    .net = ni->addr,
 	    .src = px->src,
 	    .lastmod = px->lastmod,
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 00dbe496..a4fcc65b 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -467,9 +467,10 @@ 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) */
-  ea_list *attrs;			/* Attributes to encode */
+  u32 hash;				/* Hash over extended attributes */
   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 0d1c0598..530b2d74 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -2313,7 +2313,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->attrs, buf+4, buf + MAX_ATTRS_LENGTH);
+  la = bgp_encode_attrs(s, buck->eattrs, buf+4, buf + MAX_ATTRS_LENGTH);
   if (la < 0)
   {
     /* Attribute list too long */
@@ -2362,7 +2362,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->attrs, abuf, abuf + MAX_ATTRS_LENGTH);
+  la = bgp_encode_attrs(s, buck->eattrs, abuf, abuf + MAX_ATTRS_LENGTH);
   if (la < 0)
   {
     /* Attribute list too long */