From ce7495b49a5192b1cba17fd88cdb2495a40824e2 Mon Sep 17 00:00:00 2001
From: Maria Matejka <mq@ucw.cz>
Date: Mon, 24 Apr 2023 16:10:59 +0200
Subject: [PATCH] Refactoring of domains connected to pools

---
 lib/locking.h         |  8 ++++++--
 lib/rcu.c             |  3 ++-
 lib/resource.c        |  7 ++++++-
 nest/iface.c          |  2 +-
 nest/locks.c          |  3 ++-
 nest/proto.c          |  4 ++--
 nest/rt-attr.c        |  2 +-
 nest/rt-table.c       |  2 +-
 proto/bfd/bfd.c       |  4 +++-
 proto/bgp/bgp.c       | 17 +++++++++++++----
 sysdep/unix/alloc.c   |  3 ++-
 sysdep/unix/domain.c  | 17 +++++++++++++----
 sysdep/unix/io-loop.c |  7 ++++---
 sysdep/unix/io.c      | 19 +++++++++++++++----
 14 files changed, 71 insertions(+), 27 deletions(-)

diff --git a/lib/locking.h b/lib/locking.h
index 751eecbd..fc3c59ff 100644
--- a/lib/locking.h
+++ b/lib/locking.h
@@ -10,6 +10,7 @@
 #define _BIRD_LOCKING_H_
 
 struct domain_generic;
+struct pool;
 
 /* Here define the global lock order; first to last. */
 struct lock_order {
@@ -30,8 +31,8 @@ extern _Thread_local struct domain_generic **last_locked;
 #define DEFINE_DOMAIN(type) DOMAIN(type) { struct domain_generic *type; }
 #define DOMAIN_ORDER(type)  OFFSETOF(struct lock_order, type)
 
-#define DOMAIN_NEW(type, name)  (DOMAIN(type)) { .type = domain_new(name, DOMAIN_ORDER(type)) }
-struct domain_generic *domain_new(const char *name, uint order);
+#define DOMAIN_NEW(type)  (DOMAIN(type)) { .type = domain_new(DOMAIN_ORDER(type)) }
+struct domain_generic *domain_new(uint order);
 
 #define DOMAIN_FREE(type, d)	domain_free((d).type)
 void domain_free(struct domain_generic *);
@@ -39,6 +40,9 @@ void domain_free(struct domain_generic *);
 #define DOMAIN_NAME(type, d)	domain_name((d).type)
 const char *domain_name(struct domain_generic *);
 
+#define DOMAIN_SETUP(type, d, n, p)	domain_setup((d).type, n, p)
+void domain_setup(struct domain_generic *, const char *name, struct pool *);
+
 #define DOMAIN_NULL(type)   (DOMAIN(type)) {}
 
 #define LOCK_DOMAIN(type, d)	do_lock(((d).type), &(locking_stack.type))
diff --git a/lib/rcu.c b/lib/rcu.c
index 8ae4f2ab..7ea6ec9b 100644
--- a/lib/rcu.c
+++ b/lib/rcu.c
@@ -73,7 +73,8 @@ rcu_thread_stop(struct rcu_thread *rc)
 void
 rcu_init(void)
 {
-  rcu_domain = DOMAIN_NEW(resource, "Read-Copy-Update");
+  rcu_domain = DOMAIN_NEW(resource);
+  DOMAIN_SETUP(resource, rcu_domain, "Read-Copy-Update", NULL);
   init_list(&rcu_thread_list);
   rcu_thread_start(&main_rcu_thread);
 }
diff --git a/lib/resource.c b/lib/resource.c
index 2071a411..e0aaac0a 100644
--- a/lib/resource.c
+++ b/lib/resource.c
@@ -51,6 +51,9 @@ rp_init(pool *z, struct domain_generic *dom, const char *name)
 {
   ASSERT_DIE(DG_IS_LOCKED(dom));
 
+  if (name && !domain_name(dom))
+    domain_setup(dom, name, z);
+
   z->name = name;
   z->domain = dom;
   z->inside = (TLIST_LIST(resource)) {};
@@ -87,7 +90,9 @@ pool *
 rp_vnewf(pool *p, struct domain_generic *dom, const char *fmt, va_list args)
 {
   pool *z = rp_new(p, dom, NULL);
-  z->name = mb_vsprintf(p, fmt, args);
+  z->name = mb_vsprintf(z, fmt, args);
+  if (!domain_name(dom))
+    domain_setup(dom, z->name, z);
   return z;
 }
 
diff --git a/nest/iface.c b/nest/iface.c
index a024b943..bbe38d3f 100644
--- a/nest/iface.c
+++ b/nest/iface.c
@@ -1018,7 +1018,7 @@ if_choose_router_id(struct iface_patt *mask, u32 old_id)
 void
 if_init(void)
 {
-  iface_domain = DOMAIN_NEW(attrs, "Interfaces");
+  iface_domain = DOMAIN_NEW(attrs);
 
   IFACE_LOCK;
   if_pool = rp_new(&root_pool, iface_domain.attrs, "Interfaces");
diff --git a/nest/locks.c b/nest/locks.c
index 247d0c56..66409c6d 100644
--- a/nest/locks.c
+++ b/nest/locks.c
@@ -197,5 +197,6 @@ olock_init(void)
 {
   DBG("olock: init\n");
   init_list(&olock_list);
-  olock_domain = DOMAIN_NEW(attrs, "Object lock");
+  olock_domain = DOMAIN_NEW(attrs);
+  DOMAIN_SETUP(attrs, olock_domain, "Object lock", NULL);
 }
diff --git a/nest/proto.c b/nest/proto.c
index 32183c9d..647b7f4e 100644
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -1859,9 +1859,9 @@ void protos_build_gen(void);
 void
 protos_build(void)
 {
-  protos_build_gen();
-
   proto_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Protocols");
+
+  protos_build_gen();
 }
 
 
diff --git a/nest/rt-attr.c b/nest/rt-attr.c
index 38612a4e..c035a2dc 100644
--- a/nest/rt-attr.c
+++ b/nest/rt-attr.c
@@ -1607,7 +1607,7 @@ ea_show_list(struct cli *c, ea_list *eal)
 void
 rta_init(void)
 {
-  attrs_domain = DOMAIN_NEW(attrs, "Attributes");
+  attrs_domain = DOMAIN_NEW(attrs);
 
   RTA_LOCK;
   rta_pool = rp_new(&root_pool, attrs_domain.attrs, "Attributes");
diff --git a/nest/rt-table.c b/nest/rt-table.c
index f1e3c8f7..a934d3c3 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -2850,7 +2850,7 @@ rt_setup(pool *pp, struct rtable_config *cf)
   pool *sp = birdloop_pool(loop);
 
   /* Create the table domain and pool */
-  DOMAIN(rtable) dom = DOMAIN_NEW(rtable, cf->name);
+  DOMAIN(rtable) dom = DOMAIN_NEW(rtable);
   LOCK_DOMAIN(rtable, dom);
 
   pool *p = rp_newf(sp, dom.rtable, "Routing table data %s", cf->name);
diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c
index f7d8f8d9..e373e4dd 100644
--- a/proto/bfd/bfd.c
+++ b/proto/bfd/bfd.c
@@ -1232,7 +1232,9 @@ bfd_build(void)
 {
   proto_build(&proto_bfd);
 
-  bfd_global.lock = DOMAIN_NEW(rtable, "BFD Global");
+  bfd_global.lock = DOMAIN_NEW(rtable);
+  DOMAIN_SETUP(rtable, bfd_global.lock, "BFD Global", NULL);
+
   init_list(&bfd_global.wait_list);
   init_list(&bfd_global.pickup_list);
   init_list(&bfd_global.proto_list);
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index e60884ba..f4d80e8d 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -132,7 +132,8 @@ static list STATIC_LIST_INIT(bgp_sockets);		/* Global list of listening sockets
 static list STATIC_LIST_INIT(bgp_listen_pending);	/* Global list of listening socket open requests */
 static event bgp_listen_event = { .hook = bgp_listen_create };
 
-DOMAIN(rtable) bgp_listen_domain;
+static DOMAIN(rtable) bgp_listen_domain;
+static pool *bgp_listen_pool;
 
 static void bgp_connect(struct bgp_proto *p);
 static void bgp_active(struct bgp_proto *p);
@@ -276,7 +277,7 @@ bgp_listen_create(void *_ UNUSED)
     {
       /* Allocating new socket from global protocol pool.
        * We can do this in main_birdloop. */
-      sock *sk = sk_new(proto_pool);
+      sock *sk = sk_new(bgp_listen_pool);
       sk->type = SK_TCP_PASSIVE;
       sk->ttl = 255;
       sk->saddr = req->addr;
@@ -301,7 +302,7 @@ bgp_listen_create(void *_ UNUSED)
 	continue;
       }
 
-      bs = mb_allocz(proto_pool, sizeof(struct bgp_socket));
+      bs = mb_allocz(bgp_listen_pool, sizeof(struct bgp_socket));
       bs->sk = sk;
       sk->data = bs;
 
@@ -1337,7 +1338,9 @@ bgp_incoming_connection(sock *sk, uint dummy UNUSED)
   {
     log(L_WARN "BGP: Unexpected connect from unknown address %I%J (port %d)",
 	sk->daddr, ipa_is_link_local(sk->daddr) ? sk->iface : NULL, sk->dport);
+    LOCK_DOMAIN(rtable, bgp_listen_domain);
     sk_close(sk);
+    UNLOCK_DOMAIN(rtable, bgp_listen_domain);
     return 0;
   }
 
@@ -1365,6 +1368,8 @@ bgp_incoming_connection(sock *sk, uint dummy UNUSED)
       bgp_close_conn(&p->incoming_conn);
   }
 
+  LOCK_DOMAIN(rtable, bgp_listen_domain);
+
   BGP_TRACE(D_EVENTS, "Incoming connection from %I%J (port %d) %s",
 	    sk->daddr, ipa_is_link_local(sk->daddr) ? sk->iface : NULL,
 	    sk->dport, acc ? "accepted" : "rejected");
@@ -1414,6 +1419,7 @@ err:
   sk_close(sk);
 
 leave:
+  UNLOCK_DOMAIN(rtable, bgp_listen_domain);
   birdloop_leave(p->p.loop);
   return 0;
 }
@@ -2843,5 +2849,8 @@ void bgp_build(void)
 {
   proto_build(&proto_bgp);
   bgp_register_attrs();
-  bgp_listen_domain = DOMAIN_NEW(rtable, "BGP Listen Sockets");
+  bgp_listen_domain = DOMAIN_NEW(rtable);
+  LOCK_DOMAIN(rtable, bgp_listen_domain);
+  bgp_listen_pool = rp_new(proto_pool, bgp_listen_domain.rtable, "BGP Listen Sockets");
+  UNLOCK_DOMAIN(rtable, bgp_listen_domain);
 }
diff --git a/sysdep/unix/alloc.c b/sysdep/unix/alloc.c
index cafcc8dd..1b951da2 100644
--- a/sysdep/unix/alloc.c
+++ b/sysdep/unix/alloc.c
@@ -318,7 +318,8 @@ resource_sys_init(void)
     /* We assume that page size has only one bit and is between 1K and 256K (incl.).
      * Otherwise, the assumptions in lib/slab.c (sl_head's num_full range) aren't met. */
 
-    empty_pages_domain = DOMAIN_NEW(resource, "Empty Pages");
+    empty_pages_domain = DOMAIN_NEW(resource);
+    DOMAIN_SETUP(resource, empty_pages_domain, "Empty Pages", NULL);
     initialized = 1;
     return;
   }
diff --git a/sysdep/unix/domain.c b/sysdep/unix/domain.c
index f4ee595d..c04e91ea 100644
--- a/sysdep/unix/domain.c
+++ b/sysdep/unix/domain.c
@@ -46,20 +46,21 @@ struct domain_generic {
   struct domain_generic **prev;
   struct lock_order *locked_by;
   const char *name;
+  pool *pool;
 };
 
-#define DOMAIN_INIT(_name, _order) { .mutex = PTHREAD_MUTEX_INITIALIZER, .name = _name, .order = _order }
+#define DOMAIN_INIT(_order) { .mutex = PTHREAD_MUTEX_INITIALIZER, .order = _order }
 
-static struct domain_generic the_bird_domain_gen = DOMAIN_INIT("The BIRD", OFFSETOF(struct lock_order, the_bird));
+static struct domain_generic the_bird_domain_gen = DOMAIN_INIT(OFFSETOF(struct lock_order, the_bird));
 
 DOMAIN(the_bird) the_bird_domain = { .the_bird = &the_bird_domain_gen };
 
 struct domain_generic *
-domain_new(const char *name, uint order)
+domain_new(uint order)
 {
   ASSERT_DIE(order < sizeof(struct lock_order));
   struct domain_generic *dg = xmalloc(sizeof(struct domain_generic));
-  *dg = (struct domain_generic) DOMAIN_INIT(name, order);
+  *dg = (struct domain_generic) DOMAIN_INIT(order);
   return dg;
 }
 
@@ -81,6 +82,14 @@ uint dg_order(struct domain_generic *dg)
   return dg->order;
 }
 
+void
+domain_setup(struct domain_generic *dg, const char *name, pool *p)
+{
+  ASSERT_DIE(dg->pool == NULL);
+  dg->pool = p;
+  dg->name = name;
+}
+
 void do_lock(struct domain_generic *dg, struct domain_generic **lsp)
 {
   struct lock_order stack_copy;
diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c
index daa86560..1a3333ef 100644
--- a/sysdep/unix/io-loop.c
+++ b/sysdep/unix/io-loop.c
@@ -1029,7 +1029,7 @@ bird_thread_show_finish(void *data)
 void
 cmd_show_threads(int show_loops)
 {
-  DOMAIN(control) lock = DOMAIN_NEW(control, "Show Threads");
+  DOMAIN(control) lock = DOMAIN_NEW(control);
   LOCK_DOMAIN(control, lock);
   pool *p = rp_new(&root_pool, lock.control, "Show Threads");
 
@@ -1084,7 +1084,8 @@ birdloop_init(void)
   {
     struct birdloop_pickup_group *group = &pickup_groups[i];
 
-    group->domain = DOMAIN_NEW(resource, "Loop Pickup");
+    group->domain = DOMAIN_NEW(resource);
+    DOMAIN_SETUP(resource, group->domain, "Loop Pickup", NULL);
     init_list(&group->loops);
     init_list(&group->threads);
   }
@@ -1217,7 +1218,7 @@ birdloop_run_timer(timer *tm)
 static struct birdloop *
 birdloop_vnew_internal(pool *pp, uint order, struct birdloop_pickup_group *group, const char *name, va_list args)
 {
-  struct domain_generic *dg = domain_new(name, order);
+  struct domain_generic *dg = domain_new(order);
   DG_LOCK(dg);
 
   pool *p = rp_vnewf(pp, dg, name, args);
diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c
index 40a6f114..b21df057 100644
--- a/sysdep/unix/io.c
+++ b/sysdep/unix/io.c
@@ -1052,6 +1052,10 @@ sk_passive_connected(sock *s, int type)
     return 0;
   }
 
+  struct domain_generic *sock_lock = DG_IS_LOCKED(s->pool->domain) ? NULL : s->pool->domain;
+  if (sock_lock)
+    DG_LOCK(sock_lock);
+
   sock *t = sk_new(s->pool);
   t->type = type;
   t->data = s->data;
@@ -1082,13 +1086,20 @@ sk_passive_connected(sock *s, int type)
     close(t->fd);
     t->fd = -1;
     sk_close(t);
-    return 1;
+    t = NULL;
+  }
+  else
+  {
+    birdloop_add_socket(s->loop, t);
+    sk_alloc_bufs(t);
   }
 
-  birdloop_add_socket(s->loop, t);
+  if (sock_lock)
+    DG_UNLOCK(sock_lock);
+
+  if (t)
+    s->rx_hook(t, 0);
 
-  sk_alloc_bufs(t);
-  s->rx_hook(t, 0);
   return 1;
 }