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.

This was a contention problem though, so this commit got reverted,
but now the attribute cache is actually lockless (yay!) and we can
try it again.

If this becomes a contention problem, we should ... no, please not.
Not again! Please!

(Dug out from the history and updated by Katerina Kubecova.)

diff --git a/nest/rt-attr.c b/nest/rt-attr.c
index 2534f3962cc0817332cb61364bb893a3b0367ef7..40e2c32ce498dc27b6324a40d164a84b01002804 100644 (file)
--- a/nest/rt-attr.c
+++ b/nest/rt-attr.c
@@ -2022,7 +2022,7 @@ ea_rehash(void * u UNUSED)
 static void
 ea_dump_esa(struct dump_request *dreq, struct ea_stor_array *esa, u64 order)
 {
-  for (uint i = 0; i < 1 << (order); i++)
+  for (int i = 0; i < 1 << (order); i++)
   {
     struct ea_storage *eap = atomic_load_explicit(&esa->eas[i], memory_order_acquire);
     for (; eap; eap = atomic_load_explicit(&eap->next_hash, memory_order_acquire))

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index cabfc610c25903cf84464fd377aeb8c11ca8c150..d4c465ffd61f180a5da5d909a52f33925de59f42 100644 (file)
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1699,10 +1699,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
@@ -1711,9 +1711,10 @@ bgp_finish_attrs(struct bgp_parse_state *s, ea_list **to)
 HASH_DEFINE_REHASH_FN(RBH, struct bgp_bucket)
 
 static void
-bgp_init_bucket_table(struct bgp_ptx_private *c)
+bgp_init_bucket_table(struct bgp_ptx_private *c, struct event_list *cleanup_ev_list)
 {
   HASH_INIT(c->bucket_hash, c->pool, 8);
+  c->bucket_slab = sl_new(c->pool, cleanup_ev_list, sizeof(struct bgp_bucket));
 
   init_list(&c->bucket_queue);
   c->withdraw_bucket = NULL;
@@ -1723,24 +1724,21 @@ static struct bgp_bucket *
 bgp_get_bucket(struct bgp_ptx_private *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);
@@ -1764,7 +1762,8 @@ static void
 bgp_free_bucket(struct bgp_ptx_private *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
@@ -2048,7 +2047,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 == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->cur->eattrs, 0, EALS_CUSTOM)),
+           .attrs = (px->cur == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->cur->attrs, 0, EALS_CUSTOM)),
            .net = ni->addr,
            .src = px->src,
            .lastmod = px->lastmod,
@@ -2057,7 +2056,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 == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->last->eattrs, 0, EALS_CUSTOM)),
+           .attrs = (px->last == c->withdraw_bucket) ? NULL : ea_free_later(ea_lookup_slow(px->last->attrs, 0, EALS_CUSTOM)),
            .net = ni->addr,
            .src = px->src,
            .lastmod = px->lastmod,
@@ -2088,7 +2087,7 @@ bgp_init_pending_tx(struct bgp_channel *c)
   bpp->pool = p;
   bpp->c = c;
 
-  bgp_init_bucket_table(bpp);
+  bgp_init_bucket_table(bpp, birdloop_event_list(c->c.proto->loop));
   bgp_init_prefix_table(bpp, birdloop_event_list(c->c.proto->loop));
 
   bpp->exporter = (struct rt_exporter) {

diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 202e78ba39a0f4dc03b844e8dd06c07afac811e1..a8565a74c78d4a9f703bed35019ce97efb7c1057 100644 (file)
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -418,6 +418,14 @@ struct bgp_channel {
   pool *pool;
 
   union bgp_ptx *tx;                   /* TX encapsulation */
+  HASH(struct bgp_bucket) bucket_hash; /* Hash table of route buckets */
+  struct bgp_bucket *withdraw_bucket;  /* Withdrawn routes */
+  list bucket_queue;                   /* Queue of buckets to send (struct bgp_bucket) */
+
+  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 */
   ip_addr link_addr;                   /* Link-local version of next_hop_addr */
@@ -489,10 +497,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 a40ca91b406a117b9bd8385b09c62cdbf795d96a..a707d29b029a42fbbd16d2dd315e3a18972f8ff0 100644 (file)
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -2341,7 +2341,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 */
@@ -2391,7 +2391,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 */
@@ -2534,7 +2534,7 @@ bgp_bmp_encode_rte(ea_list *c, byte *buf, byte *end, const struct rte *new)
   init_list(&b->prefixes);
 
   if (new->attrs)
-    memcpy(b->eattrs, new->attrs, ea_size);
+    memcpy(b->attrs, new->attrs, ea_size);
 
   /* Temporary prefix */
   struct bgp_prefix *px = tmp_allocz(prefix_size);