From 06963f96b363680380f83ccbb073d7e42f811afd Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 19 Apr 2023 17:52:52 +0200 Subject: Typed lists keep an explicit pointer to the list head. This change adds one pointer worth of memory to every list node. Keeping this information helps auditing the lists, checking that the node indeed is outside of list or inside the right one. The typed lists shouldn't be used anywhere with memory pressure anyway, thus the one added pointer isn't significant. --- lib/tlists.h | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/lib/tlists.h b/lib/tlists.h index 1437e17e..ea21b141 100644 --- a/lib/tlists.h +++ b/lib/tlists.h @@ -79,6 +79,11 @@ typedef struct TLIST_LIST_STRUCT { TLIST_TYPE *last; } TLIST_LIST_STRUCT; +static inline struct TLIST_LIST_STRUCT * TLIST_NAME(enlisted)(TLIST_TYPE *node) +{ + return node->TLIST_ITEM.list; +} + #ifdef TLIST_WANT_WALK static inline struct TLIST_NAME(node) * TLIST_NAME(node_get)(TLIST_TYPE *node) { return &(node->TLIST_ITEM); } @@ -87,11 +92,14 @@ static inline struct TLIST_NAME(node) * TLIST_NAME(node_get)(TLIST_TYPE *node) #ifdef TLIST_WANT_ADD_HEAD static inline void TLIST_NAME(add_head)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) { - ASSERT_DIE(!node->TLIST_ITEM.prev && !node->TLIST_ITEM.next); + ASSERT_DIE(!TLIST_NAME(enlisted)(node)); + node->TLIST_ITEM.list = list; + if (node->TLIST_ITEM.next = list->first) list->first->TLIST_ITEM.prev = node; else list->last = node; + list->first = node; } #endif @@ -99,17 +107,39 @@ static inline void TLIST_NAME(add_head)(TLIST_LIST_STRUCT *list, TLIST_TYPE *nod #ifdef TLIST_WANT_ADD_TAIL static inline void TLIST_NAME(add_tail)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) { - ASSERT_DIE(!node->TLIST_ITEM.prev && !node->TLIST_ITEM.next); + ASSERT_DIE(!TLIST_NAME(enlisted)(node)); + node->TLIST_ITEM.list = list; + if (node->TLIST_ITEM.prev = list->last) list->last->TLIST_ITEM.next = node; else list->first = node; + list->last = node; } #endif +#ifdef TLIST_WANT_UPDATE_NODE +static inline void TLIST_NAME(update_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) +{ + ASSERT_DIE(TLIST_NAME(enlisted)(node) == list); + + if (node->TLIST_ITEM.prev) + node->TLIST_ITEM.prev->TLIST_ITEM.next = node; + else + list->first = node; + + if (node->TLIST_ITEM.next) + node->TLIST_ITEM.next->TLIST_ITEM.prev = node; + else + list->last = node; +} +#endif + static inline void TLIST_NAME(rem_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) { + ASSERT_DIE(TLIST_NAME(enlisted)(node) == list); + if (node->TLIST_ITEM.prev) node->TLIST_ITEM.prev->TLIST_ITEM.next = node->TLIST_ITEM.next; else @@ -127,6 +157,7 @@ static inline void TLIST_NAME(rem_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *nod } node->TLIST_ITEM.next = node->TLIST_ITEM.prev = NULL; + node->TLIST_ITEM.list = NULL; } #undef TLIST_PREFIX @@ -136,6 +167,7 @@ static inline void TLIST_NAME(rem_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *nod #undef TLIST_ITEM #undef TLIST_WANT_ADD_HEAD #undef TLIST_WANT_ADD_TAIL +#undef TLIST_WANT_UPDATE_NODE # endif #else @@ -147,12 +179,12 @@ static inline void TLIST_NAME(rem_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *nod #error "You should first include lib/tlists.h without requesting a TLIST" #endif -#define TLIST_NODE_CONTENTS(_type) { _type *next; _type *prev; } -#define TLIST_NODE(_name, _type) struct _name##_node TLIST_NODE_CONTENTS(_type) -#define TLIST_DEFAULT_NODE struct MACRO_CONCAT_AFTER(TLIST_PREFIX,_node) \ - TLIST_NODE_CONTENTS(TLIST_TYPE) TLIST_ITEM +#define TLIST_LIST(_name) struct _name##_list -#define TLIST_LIST(_name) struct _name##_list +#define TLIST_NODE_IN(_name, _type) { _type *next; _type *prev; TLIST_LIST(_name) *list; } +#define TLIST_NODE(_name, _type) struct _name##_node TLIST_NODE_IN(_name, _type) +#define TLIST_DEFAULT_NODE struct MACRO_CONCAT_AFTER(TLIST_PREFIX,_node) \ + TLIST_NODE_IN(TLIST_PREFIX,TLIST_TYPE) TLIST_ITEM /* Use ->first and ->last to access HEAD and TAIL */ -- cgit v1.2.3 From 48dfcb60d61ab7aec478468bd50eb0196c332ee3 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 19 Apr 2023 21:02:20 +0200 Subject: Typed lists: added add_after() and unit tests --- lib/Makefile | 2 +- lib/tlists.h | 28 ++++- lib/tlists_test.c | 314 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 lib/tlists_test.c diff --git a/lib/Makefile b/lib/Makefile index f4ade9a6..6ed778cc 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -2,6 +2,6 @@ src := a-path.c a-set.c bitmap.c bitops.c blake2s.c blake2b.c checksum.c event.c obj := $(src-o-files) $(all-daemon) -tests_src := a-set_test.c a-path_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c type_test.c +tests_src := a-set_test.c a-path_test.c bitmap_test.c heap_test.c buffer_test.c event_test.c flowspec_test.c bitops_test.c patmatch_test.c fletcher16_test.c slist_test.c checksum_test.c lists_test.c mac_test.c ip_test.c hash_test.c printf_test.c slab_test.c tlists_test.c type_test.c tests_targets := $(tests_targets) $(tests-target-files) tests_objs := $(tests_objs) $(src-o-files) diff --git a/lib/tlists.h b/lib/tlists.h index ea21b141..d7f1cf03 100644 --- a/lib/tlists.h +++ b/lib/tlists.h @@ -89,7 +89,7 @@ static inline struct TLIST_NAME(node) * TLIST_NAME(node_get)(TLIST_TYPE *node) { return &(node->TLIST_ITEM); } #endif -#ifdef TLIST_WANT_ADD_HEAD +#if defined(TLIST_WANT_ADD_HEAD) || defined(TLIST_WANT_ADD_AFTER) static inline void TLIST_NAME(add_head)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) { ASSERT_DIE(!TLIST_NAME(enlisted)(node)); @@ -136,6 +136,32 @@ static inline void TLIST_NAME(update_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE * } #endif +#ifdef TLIST_WANT_ADD_AFTER +static inline void TLIST_NAME(add_after)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node, TLIST_TYPE *after) +{ + ASSERT_DIE(!TLIST_NAME(enlisted)(node)); + + /* Adding to beginning */ + if (!(node->TLIST_ITEM.prev = after)) + return TLIST_NAME(add_head)(list, node); + + /* OK, Adding after a real node */ + node->TLIST_ITEM.list = list; + + /* There is another node after the anchor */ + if (node->TLIST_ITEM.next = after->TLIST_ITEM.next) + /* Link back */ + node->TLIST_ITEM.next->TLIST_ITEM.prev = node; + else + /* Or we are adding the last node */ + list->last = node; + + /* Link forward from "after" */ + after->TLIST_ITEM.next = node; +} +#endif + + static inline void TLIST_NAME(rem_node)(TLIST_LIST_STRUCT *list, TLIST_TYPE *node) { ASSERT_DIE(TLIST_NAME(enlisted)(node) == list); diff --git a/lib/tlists_test.c b/lib/tlists_test.c new file mode 100644 index 00000000..8ba080a6 --- /dev/null +++ b/lib/tlists_test.c @@ -0,0 +1,314 @@ +/* + * BIRD Library -- Linked Lists Tests + * + * (c) 2015 CZ.NIC z.s.p.o. + * + * Can be freely distributed and used under the terms of the GNU GPL. + */ + +#include "test/birdtest.h" +#include "lib/tlists.h" + +#define TLIST_PREFIX tp +#define TLIST_TYPE struct test_node +#define TLIST_ITEM n +#define TLIST_WANT_ADD_HEAD +#define TLIST_WANT_ADD_TAIL +#define TLIST_WANT_ADD_AFTER +#define TLIST_WANT_UPDATE_NODE + +struct test_node { + TLIST_DEFAULT_NODE; +}; + +#include "lib/tlists.h" + +#define MAX_NUM 1000 + +static struct test_node nodes[MAX_NUM]; +static TLIST_LIST(tp) l; + +static void +show_list(void) +{ + bt_debug("\n"); + bt_debug("list.first points to %p\n", l.first); + bt_debug("list.last points to %p\n", l.last); + + int i; + for (i = 0; i < MAX_NUM; i++) + { + bt_debug("n[%3i] is at %p\n", i, &nodes[i]); + bt_debug(" prev is at %p and point to %p\n", &(nodes[i].n.prev), nodes[i].n.prev); + bt_debug(" next is at %p and point to %p\n", &(nodes[i].n.next), nodes[i].n.next); + } +} + +static int +is_filled_list_well_linked(void) +{ + int i; + bt_assert(l.first == &nodes[0]); + bt_assert(l.last == &nodes[MAX_NUM-1]); + bt_assert(!nodes[0].n.prev); + bt_assert(!nodes[MAX_NUM-1].n.next); + + for (i = 0; i < MAX_NUM; i++) + { + bt_assert(nodes[i].n.list == &l); + + if (i < (MAX_NUM-1)) + bt_assert(nodes[i].n.next == &nodes[i+1]); + + if (i > 0) + bt_assert(nodes[i].n.prev == &nodes[i-1]); + } + + return 1; +} + +static int +is_empty_list_well_unlinked(void) +{ + int i; + + bt_assert(!l.first); + bt_assert(!l.last); + bt_assert(EMPTY_TLIST(tp, &l)); + + for (i = 0; i < MAX_NUM; i++) + { + bt_assert(nodes[i].n.next == NULL); + bt_assert(nodes[i].n.prev == NULL); + bt_assert(nodes[i].n.list == NULL); + } + + return 1; +} + +static void +init_list__(TLIST_LIST(tp) *l, struct test_node nodes[]) +{ + *l = (TLIST_LIST(tp)) {}; + + int i; + for (i = 0; i < MAX_NUM; i++) + { + nodes[i].n.next = NULL; + nodes[i].n.prev = NULL; + nodes[i].n.list = NULL; + } +} + +static void +init_list_(void) +{ + init_list__(&l, nodes); +} + +static int +t_add_tail(void) +{ + int i; + + init_list_(); + for (i = 0; i < MAX_NUM; i++) + { + tp_add_tail(&l, &nodes[i]); + bt_debug("."); + bt_assert(l.last == &nodes[i]); + bt_assert(l.first == &nodes[0]); + + bt_assert(nodes[i].n.list == &l); + bt_assert(!nodes[i].n.next); + + if (i > 0) + { + bt_assert(nodes[i-1].n.next == &nodes[i]); + bt_assert(nodes[i].n.prev == &nodes[i-1]); + } + } + show_list(); + bt_assert(is_filled_list_well_linked()); + + return 1; +} + +static int +t_add_head(void) +{ + int i; + + init_list_(); + for (i = MAX_NUM-1; i >= 0; i--) + { + tp_add_head(&l, &nodes[i]); + bt_debug("."); + bt_assert(l.first == &nodes[i]); + bt_assert(l.last == &nodes[MAX_NUM-1]); + if (i < MAX_NUM-1) + { + bt_assert(nodes[i+1].n.prev == &nodes[i]); + bt_assert(nodes[i].n.next == &nodes[i+1]); + } + } + show_list(); + bt_assert(is_filled_list_well_linked()); + + return 1; +} + +static void +insert_node_(TLIST_LIST(tp) *l, struct test_node *n, struct test_node *after) +{ + tp_add_after(l, n, after); + bt_debug("."); +} + +static int +t_insert_node(void) +{ + int i; + + init_list_(); + + // add first node + insert_node_(&l, &nodes[0], NULL); + + // add odd nodes + for (i = 2; i < MAX_NUM; i+=2) + insert_node_(&l, &nodes[i], &nodes[i-2]); + + // add even nodes + for (i = 1; i < MAX_NUM; i+=2) + insert_node_(&l, &nodes[i], &nodes[i-1]); + + bt_debug("\n"); + bt_assert(is_filled_list_well_linked()); + + return 1; +} + +static void +fill_list2(TLIST_LIST(tp) *l, struct test_node nodes[]) +{ + int i; + for (i = 0; i < MAX_NUM; i++) + tp_add_tail(l, &nodes[i]); +} + +static void +fill_list(void) +{ + fill_list2(&l, nodes); +} + +static int +t_remove_node(void) +{ + int i; + + init_list_(); + + /* Fill & Remove & Check */ + fill_list(); + for (i = 0; i < MAX_NUM; i++) + tp_rem_node(&l, &nodes[i]); + bt_assert(is_empty_list_well_unlinked()); + + /* Fill & Remove the half of nodes & Check & Remove the rest nodes & Check */ + fill_list(); + for (i = 0; i < MAX_NUM; i+=2) + tp_rem_node(&l, &nodes[i]); + + int tail_node_index = (MAX_NUM % 2) ? MAX_NUM - 2 : MAX_NUM - 1; + bt_assert(l.first == &nodes[1]); + bt_assert(l.last == &nodes[tail_node_index]); + bt_assert(!nodes[tail_node_index].n.next); + + for (i = 1; i < MAX_NUM; i+=2) + { + if (i > 1) + bt_assert(nodes[i].n.prev == &nodes[i-2]); + if (i < tail_node_index) + bt_assert(nodes[i].n.next == &nodes[i+2]); + } + + for (i = 1; i < MAX_NUM; i+=2) + tp_rem_node(&l, &nodes[i]); + bt_assert(is_empty_list_well_unlinked()); + + return 1; +} + +static int +t_update_node(void) +{ + struct test_node head, inside, tail; + + init_list_(); + fill_list(); + + head = nodes[0]; + tp_update_node(&l, &head); + bt_assert(l.first == &head); + bt_assert(head.n.prev == NULL); + bt_assert(head.n.next == &nodes[1]); + bt_assert(nodes[1].n.prev == &head); + + inside = nodes[MAX_NUM/2]; + tp_update_node(&l, &inside); + bt_assert(nodes[MAX_NUM/2-1].n.next == &inside); + bt_assert(nodes[MAX_NUM/2+1].n.prev == &inside); + bt_assert(inside.n.prev == &nodes[MAX_NUM/2-1]); + bt_assert(inside.n.next == &nodes[MAX_NUM/2+1]); + + tail = nodes[MAX_NUM-1]; + tp_update_node(&l, &tail); + bt_assert(l.last == &tail); + bt_assert(tail.n.prev == &nodes[MAX_NUM-2]); + bt_assert(tail.n.next == NULL); + bt_assert(nodes[MAX_NUM-2].n.next == &tail); + + return 1; +} + +#if 0 +static int +t_add_tail_list(void) +{ + node nodes2[MAX_NUM]; + list l2; + + init_list__(&l, (node *) nodes); + fill_list2(&l, (node *) nodes); + + init_list__(&l2, (node *) nodes2); + fill_list2(&l2, (node *) nodes2); + + add_tail_list(&l, &l2); + + bt_assert(nodes[MAX_NUM-1].next == &nodes2[0]); + bt_assert(nodes2[0].prev == &nodes[MAX_NUM-1]); + bt_assert(l.tail == &nodes2[MAX_NUM-1]); + + return 1; +} +#endif + +int +main(int argc, char *argv[]) +{ + bt_init(argc, argv); + + bt_test_suite(t_add_tail, "Adding nodes to tail of list"); + bt_test_suite(t_add_head, "Adding nodes to head of list"); + bt_test_suite(t_insert_node, "Inserting nodes to list"); + bt_test_suite(t_remove_node, "Removing nodes from list"); + bt_test_suite(t_update_node, "Updating nodes in list"); +#if 0 + bt_test_suite(t_add_tail_list, "At the tail of a list adding the another list"); +#endif + + return bt_exit_value(); +} -- cgit v1.2.3 From 074739e0e9baaba53f2a99edd348b76c3613b455 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 19 Apr 2023 21:18:12 +0200 Subject: Global protocol list is typed to avoid typecast confusion --- nest/proto.c | 46 +++++++++++++++++++++------------------------- nest/protocol.h | 12 ++++++++++-- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/nest/proto.c b/nest/proto.c index cd6a3faa..280585dd 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -23,7 +23,7 @@ #include "filter/f-inst.h" pool *proto_pool; -list STATIC_LIST_INIT(proto_list); +static TLIST_LIST(proto) global_proto_list; static list STATIC_LIST_INIT(protocol_list); @@ -1190,7 +1190,7 @@ proto_new(struct proto_config *cf) } static struct proto * -proto_init(struct proto_config *c, node *n) +proto_init(struct proto_config *c, struct proto *after) { struct protocol *pr = c->protocol; struct proto *p = pr->init(c); @@ -1199,7 +1199,7 @@ proto_init(struct proto_config *c, node *n) p->proto_state = PS_DOWN; p->last_state_change = current_time(); p->vrf = c->vrf; - insert_node(&p->n, n); + proto_add_after(&global_proto_list, p, after); p->event = ev_new_init(proto_pool, proto_event, p); @@ -1430,8 +1430,6 @@ protos_commit(struct config *new, struct config *old, int force_reconfig, int ty struct proto_config *oc, *nc; struct symbol *sym; struct proto *p; - node *n; - DBG("protos_commit:\n"); if (old) @@ -1518,8 +1516,8 @@ protos_commit(struct config *new, struct config *old, int force_reconfig, int ty } struct proto *first_dev_proto = NULL; + struct proto *after = NULL; - n = NODE &(proto_list.head); WALK_LIST(nc, new->protos) if (!nc->proto) { @@ -1527,14 +1525,14 @@ protos_commit(struct config *new, struct config *old, int force_reconfig, int ty if (old) log(L_INFO "Adding protocol %s", nc->name); - p = proto_init(nc, n); - n = NODE p; + p = proto_init(nc, after); + after = p; if (p->proto == &proto_unix_iface) first_dev_proto = p; } else - n = NODE nc->proto; + after = nc->proto; DBG("Protocol start\n"); @@ -1552,7 +1550,7 @@ protos_commit(struct config *new, struct config *old, int force_reconfig, int ty } /* Start all new protocols */ - WALK_LIST_DELSAFE(p, n, proto_list) + WALK_TLIST_DELSAFE(proto, p, &global_proto_list) proto_rethink_goal(p); } @@ -1574,18 +1572,19 @@ proto_rethink_goal(struct proto *p) if (p->reconfiguring && !p->active) { struct proto_config *nc = p->cf_new; - node *n = p->n.prev; + struct proto *after = p->n.prev; + DBG("%s has shut down for reconfiguration\n", p->name); p->cf->proto = NULL; config_del_obstacle(p->cf->global); proto_remove_channels(p); - rem_node(&p->n); + proto_rem_node(&global_proto_list, p); rfree(p->event); mb_free(p->message); mb_free(p); if (!nc) return; - p = proto_init(nc, n); + p = proto_init(nc, after); } /* Determine what state we want to reach */ @@ -1601,7 +1600,7 @@ proto_rethink_goal(struct proto *p) struct proto * proto_spawn(struct proto_config *cf, uint disabled) { - struct proto *p = proto_init(cf, TAIL(proto_list)); + struct proto *p = proto_init(cf, global_proto_list.last); p->disabled = disabled; proto_rethink_goal(p); return p; @@ -1697,8 +1696,7 @@ graceful_restart_done(timer *t UNUSED) log(L_INFO "Graceful restart done"); graceful_restart_state = GRS_DONE; - struct proto *p; - WALK_LIST(p, proto_list) + WALK_TLIST(proto, p, &global_proto_list) { if (!p->gr_recovery) continue; @@ -1794,8 +1792,7 @@ protos_dump_all(void) { debug("Protocols:\n"); - struct proto *p; - WALK_LIST(p, proto_list) PROTO_LOCKED_FROM_MAIN(p) + WALK_TLIST(proto, p, &global_proto_list) PROTO_LOCKED_FROM_MAIN(p) { #define DPF(x) (p->x ? " " #x : "") debug(" protocol %s (%p) state %s with %d active channels flags: %s%s%s%s\n", @@ -2481,10 +2478,9 @@ proto_apply_cmd_symbol(const struct symbol *s, void (* cmd)(struct proto *, uint static void proto_apply_cmd_patt(const char *patt, void (* cmd)(struct proto *, uintptr_t, int), uintptr_t arg) { - struct proto *p; int cnt = 0; - WALK_LIST(p, proto_list) + WALK_TLIST(proto, p, &global_proto_list) if (!patt || patmatch(patt, p->name)) PROTO_LOCKED_FROM_MAIN(p) cmd(p, arg, cnt++); @@ -2511,7 +2507,7 @@ proto_apply_cmd(struct proto_spec ps, void (* cmd)(struct proto *, uintptr_t, in struct proto * proto_get_named(struct symbol *sym, struct protocol *pr) { - struct proto *p, *q; + struct proto *p; if (sym) { @@ -2525,7 +2521,7 @@ proto_get_named(struct symbol *sym, struct protocol *pr) else { p = NULL; - WALK_LIST(q, proto_list) + WALK_TLIST(proto, q, &global_proto_list) if ((q->proto == pr) && (q->proto_state != PS_DOWN)) { if (p) @@ -2562,9 +2558,9 @@ proto_iterate_named(struct symbol *sym, struct protocol *proto, struct proto *ol } else { - for (struct proto *p = !old ? HEAD(proto_list) : NODE_NEXT(old); - NODE_VALID(p); - p = NODE_NEXT(p)) + for (struct proto *p = old ? old->n.next : global_proto_list.first; + p; + p = p->n.next) { if ((p->proto == proto) && (p->proto_state != PS_DOWN)) { diff --git a/nest/protocol.h b/nest/protocol.h index 01153162..eccfcb73 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -116,9 +116,16 @@ struct proto_config { /* Protocol-specific data follow... */ }; +#define TLIST_PREFIX proto +#define TLIST_TYPE struct proto +#define TLIST_ITEM n +#define TLIST_WANT_WALK +#define TLIST_WANT_ADD_TAIL +#define TLIST_WANT_ADD_AFTER + /* Protocol statistics */ struct proto { - node n; /* Node in global proto_list */ + TLIST_DEFAULT_NODE; /* Node in global proto_list */ struct protocol *proto; /* Protocol */ struct proto_config *cf; /* Configuration data */ struct proto_config *cf_new; /* Configuration we want to switch to after shutdown (NULL=delete) */ @@ -198,6 +205,8 @@ struct proto { /* Hic sunt protocol-specific data */ }; +#include "lib/tlists.h" + struct proto_spec { const void *ptr; int patt; @@ -284,7 +293,6 @@ proto_get_router_id(struct proto_config *pc) extern pool *proto_pool; -extern list proto_list; /* * Each protocol instance runs two different state machines: -- cgit v1.2.3 From b3f805ce29487f790090fcf31096f5a7cf1d585d Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 20 Apr 2023 21:06:42 +0200 Subject: Socket closing has its dedicated function --- lib/socket.h | 1 + nest/cli.c | 2 +- proto/babel/packets.c | 2 +- proto/bfd/bfd.c | 4 ++-- proto/bfd/packets.c | 6 +++--- proto/bgp/bgp.c | 12 ++++++------ proto/ospf/iface.c | 4 ++-- proto/radv/packets.c | 2 +- proto/rip/packets.c | 2 +- proto/rip/rip.c | 2 +- proto/rpki/transport.c | 2 +- sysdep/bsd/krt-sock.c | 4 ++-- sysdep/unix/io.c | 2 +- 13 files changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/socket.h b/lib/socket.h index 4b169581..6225977b 100644 --- a/lib/socket.h +++ b/lib/socket.h @@ -88,6 +88,7 @@ sock *sock_new(pool *); /* Allocate new socket */ int sk_open(sock *, struct birdloop *); /* Open socket */ void sk_reloop(sock *, struct birdloop *); /* Move socket to another loop. Both loops must be locked. */ +static inline void sk_close(sock *s) { rfree(&s->r); } /* Explicitly close socket */ int sk_rx_ready(sock *s); _Bool sk_tx_pending(sock *s); diff --git a/nest/cli.c b/nest/cli.c index 1debfccf..39a3eef7 100644 --- a/nest/cli.c +++ b/nest/cli.c @@ -417,7 +417,7 @@ cli_free(cli *c) if (defer) { - rfree(c->sock); + sk_close(c->sock); c->sock = NULL; } else diff --git a/proto/babel/packets.c b/proto/babel/packets.c index d26ee5c6..47d065cd 100644 --- a/proto/babel/packets.c +++ b/proto/babel/packets.c @@ -1680,7 +1680,7 @@ babel_open_socket(struct babel_iface *ifa) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return 0; } diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c index c88c1cb2..ddbc4948 100644 --- a/proto/bfd/bfd.c +++ b/proto/bfd/bfd.c @@ -604,10 +604,10 @@ bfd_free_iface(struct bfd_iface *ifa) return; if (ifa->sk) - rfree(ifa->sk); + sk_close(ifa->sk); if (ifa->rx) - rfree(ifa->rx); + sk_close(ifa->rx); rem_node(&ifa->n); mb_free(ifa); diff --git a/proto/bfd/packets.c b/proto/bfd/packets.c index a22f223b..fa8c328f 100644 --- a/proto/bfd/packets.c +++ b/proto/bfd/packets.c @@ -439,7 +439,7 @@ bfd_open_rx_sk(struct bfd_proto *p, int multihop, int af) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return NULL; } @@ -470,7 +470,7 @@ bfd_open_rx_sk_bound(struct bfd_proto *p, ip_addr local, struct iface *ifa) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return NULL; } @@ -501,6 +501,6 @@ bfd_open_tx_sk(struct bfd_proto *p, ip_addr local, struct iface *ifa) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return NULL; } diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index cda0eb8d..0525d502 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -294,7 +294,7 @@ bgp_listen_create(void *_ UNUSED) { sk_log_error(sk, p->p.name); log(L_ERR "%s: Cannot open listening socket", p->p.name); - rfree(sk); + sk_close(sk); UNLOCK_DOMAIN(rtable, bgp_listen_domain); bgp_initiate_disable(p, BEM_NO_SOCKET); @@ -335,7 +335,7 @@ bgp_listen_create(void *_ UNUSED) WALK_LIST_DELSAFE(bs, nxt, bgp_sockets) if (EMPTY_LIST(bs->requests)) { - rfree(bs->sk); + sk_close(bs->sk); rem_node(&bs->n); mb_free(bs); } @@ -465,7 +465,7 @@ bgp_close_conn(struct bgp_conn *conn) rfree(conn->tx_ev); conn->tx_ev = NULL; - rfree(conn->sk); + sk_close(conn->sk); conn->sk = NULL; mb_free(conn->local_caps); @@ -1336,7 +1336,7 @@ 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); - rfree(sk); + sk_close(sk); return 0; } @@ -1370,7 +1370,7 @@ bgp_incoming_connection(sock *sk, uint dummy UNUSED) if (!acc) { - rfree(sk); + sk_close(sk); goto leave; } @@ -1410,7 +1410,7 @@ bgp_incoming_connection(sock *sk, uint dummy UNUSED) err: sk_log_error(sk, p->p.name); log(L_ERR "%s: Incoming connection aborted", p->p.name); - rfree(sk); + sk_close(sk); leave: birdloop_leave(p->p.loop); diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index 0aa7fa00..37f642d1 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -173,7 +173,7 @@ ospf_sk_open(struct ospf_iface *ifa) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return 0; } @@ -234,7 +234,7 @@ ospf_open_vlink_sk(struct ospf_proto *p) err: sk_log_error(sk, p->p.name); log(L_ERR "%s: Cannot open virtual link socket", p->p.name); - rfree(sk); + sk_close(sk); } static void diff --git a/proto/radv/packets.c b/proto/radv/packets.c index c6b565d2..d1d8663f 100644 --- a/proto/radv/packets.c +++ b/proto/radv/packets.c @@ -511,7 +511,7 @@ radv_sk_open(struct radv_iface *ifa) err: sk_log_error(sk, ifa->ra->p.name); - rfree(sk); + sk_close(sk); return 0; } diff --git a/proto/rip/packets.c b/proto/rip/packets.c index fecdf896..70108ac3 100644 --- a/proto/rip/packets.c +++ b/proto/rip/packets.c @@ -1040,6 +1040,6 @@ rip_open_socket(struct rip_iface *ifa) err: sk_log_error(sk, p->p.name); - rfree(sk); + sk_close(sk); return 0; } diff --git a/proto/rip/rip.c b/proto/rip/rip.c index d15177da..dded05c7 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -750,7 +750,7 @@ rip_remove_iface(struct rip_proto *p, struct rip_iface *ifa) rem_node(NODE ifa); - rfree(ifa->sk); + sk_close(ifa->sk); rfree(ifa->lock); rfree(ifa->timer); diff --git a/proto/rpki/transport.c b/proto/rpki/transport.c index 81bd6dd8..8c6dcc11 100644 --- a/proto/rpki/transport.c +++ b/proto/rpki/transport.c @@ -120,7 +120,7 @@ rpki_tr_close(struct rpki_tr_sock *tr) if (tr->sk) { - rfree(tr->sk); + sk_close(tr->sk); tr->sk = NULL; } } diff --git a/sysdep/bsd/krt-sock.c b/sysdep/bsd/krt-sock.c index 094268b7..6bf756ff 100644 --- a/sysdep/bsd/krt-sock.c +++ b/sysdep/bsd/krt-sock.c @@ -1118,7 +1118,7 @@ krt_sock_close_shared(void) if (!krt_sock_count) { - rfree(krt_sock); + sk_close(krt_sock); krt_sock = NULL; } } @@ -1181,7 +1181,7 @@ krt_sys_shutdown(struct krt_proto *p) { krt_table_cf[(KRT_CF->sys.table_id)/32][!!(p->af == AF_INET6)] &= ~(1 << ((KRT_CF->sys.table_id)%32)); - rfree(p->sys.sk); + sk_close(p->sys.sk); p->sys.sk = NULL; krt_buffer_release(&p->p); diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c index 88d187a4..40a6f114 100644 --- a/sysdep/unix/io.c +++ b/sysdep/unix/io.c @@ -1081,7 +1081,7 @@ sk_passive_connected(sock *s, int type) /* FIXME: handle it better in rfree() */ close(t->fd); t->fd = -1; - rfree(t); + sk_close(t); return 1; } -- cgit v1.2.3 From 1141ce4e2d924f29e6e31ccf5e325f870c8895dd Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Thu, 20 Apr 2023 21:08:38 +0200 Subject: Resource pool closing has its dedicated function --- conf/conf.c | 2 +- lib/resource.c | 4 ++-- lib/resource.h | 7 +++++++ nest/cli.c | 2 +- nest/proto.c | 2 +- nest/rt-show.c | 2 +- nest/rt-table.c | 2 +- proto/babel/babel.c | 2 +- proto/bgp/attrs.c | 2 +- proto/mrt/mrt.c | 2 +- proto/ospf/iface.c | 2 +- proto/ospf/neighbor.c | 2 +- proto/radv/radv.c | 2 +- sysdep/unix/io-loop.c | 8 ++++---- 14 files changed, 24 insertions(+), 17 deletions(-) diff --git a/conf/conf.c b/conf/conf.c index daac85c1..2cfe7b15 100644 --- a/conf/conf.c +++ b/conf/conf.c @@ -200,7 +200,7 @@ config_free(struct config *c) ASSERT(!c->obstacle_count); - rfree(c->pool); + rp_free(c->pool); } /** diff --git a/lib/resource.c b/lib/resource.c index 94b8d019..0006bc8d 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -298,7 +298,7 @@ void tmp_flush(void) { lp_flush(tmp_linpool); - rfree(tmp_res.pool); + rp_free(tmp_res.pool); tmp_res.pool = rp_new(tmp_res.parent, "TMP"); } @@ -449,7 +449,7 @@ mb_free(void *m) return; struct mblock *b = SKIP_BACK(struct mblock, data, m); - rfree(b); + rfree(&b->r); } diff --git a/lib/resource.h b/lib/resource.h index 911b990d..64803778 100644 --- a/lib/resource.h +++ b/lib/resource.h @@ -58,6 +58,12 @@ void rmove(void *, pool *); /* Move to a different pool */ void *ralloc(pool *, struct resclass *); +pool *rp_new(pool *, const char *); /* Create a new pool */ +pool *rp_newf(pool *, const char *, ...); /* Create a new pool with a formatted string as its name */ +void rp_init(pool *, const char *); /* Init a new pool */ +void rp_initf(pool *, const char *, ...); /* Init a new pool with a formatted string as its name */ +static inline void rp_free(pool *p) { rfree(&p->r); } /* Free the whole pool */ + extern pool root_pool; /* Normal memory blocks */ @@ -111,6 +117,7 @@ slab *sl_new(pool *, unsigned size); void *sl_alloc(slab *); void *sl_allocz(slab *); void sl_free(void *); +void sl_delete(slab *); /* * Low-level memory allocation functions, please don't use diff --git a/nest/cli.c b/nest/cli.c index 39a3eef7..29591d26 100644 --- a/nest/cli.c +++ b/nest/cli.c @@ -421,7 +421,7 @@ cli_free(cli *c) c->sock = NULL; } else - rfree(c->pool); + rp_free(c->pool); } /** diff --git a/nest/proto.c b/nest/proto.c index 280585dd..1b25dfe9 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -1116,7 +1116,7 @@ proto_cleanup(struct proto *p) { CALL(p->proto->cleanup, p); - rfree(p->pool); + rp_free(p->pool); p->pool = NULL; p->active = 0; diff --git a/nest/rt-show.c b/nest/rt-show.c index a5c7dc8f..eacd4e31 100644 --- a/nest/rt-show.c +++ b/nest/rt-show.c @@ -227,7 +227,7 @@ rt_show_export_stopped_cleanup(struct rt_export_request *req) req->hook = NULL; /* And free the CLI (deferred) */ - rfree(d->cli->pool); + rp_free(d->cli->pool); } static int diff --git a/nest/rt-table.c b/nest/rt-table.c index b18727b1..cec13318 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2027,7 +2027,7 @@ rt_export_stopped(struct rt_export_hook *hook) rem_node(&hook->n); /* Free the hook itself together with its pool */ - rfree(hook->pool); + rp_free(hook->pool); } static inline void diff --git a/proto/babel/babel.c b/proto/babel/babel.c index d398da8e..e95a0ead 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1881,7 +1881,7 @@ babel_remove_iface(struct babel_proto *p, struct babel_iface *ifa) rem_node(NODE ifa); - rfree(ifa->pool); /* contains ifa itself, locks, socket, etc */ + rp_free(ifa->pool); /* contains ifa itself, locks, socket, etc */ } static int diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 8bff4c78..4e6524f4 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -1867,7 +1867,7 @@ bgp_free_pending_tx(struct bgp_channel *c) ASSERT_DIE(c->ptx); ASSERT_DIE(c->ptx->pool); - rfree(c->ptx->pool); + rp_free(c->ptx->pool); c->ptx = NULL; } diff --git a/proto/mrt/mrt.c b/proto/mrt/mrt.c index 82fd426a..92c19b63 100644 --- a/proto/mrt/mrt.c +++ b/proto/mrt/mrt.c @@ -590,7 +590,7 @@ mrt_table_dump_free(struct mrt_table_dump_state *s) config_del_obstacle(s->config); - rfree(s->pool); + rp_free(s->pool); } diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index 37f642d1..0eecb432 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -311,7 +311,7 @@ ospf_iface_remove(struct ospf_iface *ifa) ospf_iface_sm(ifa, ISM_DOWN); rem_node(NODE ifa); - rfree(ifa->pool); + rp_free(ifa->pool); } void diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index b0fdc42f..2c73d251 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -120,7 +120,7 @@ ospf_neigh_down(struct ospf_neighbor *n) s_get(&(n->dbsi)); release_lsrtl(p, n); rem_node(NODE n); - rfree(n->pool); + rp_free(n->pool); OSPF_TRACE(D_EVENTS, "Neighbor %R on %s removed", rid, ifa->ifname); } diff --git a/proto/radv/radv.c b/proto/radv/radv.c index 434155dc..eb15f9cb 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -323,7 +323,7 @@ radv_iface_remove(struct radv_iface *ifa) rem_node(NODE ifa); - rfree(ifa->pool); + rp_free(ifa->pool); } static void diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c index efb408e0..8481bb6e 100644 --- a/sysdep/unix/io-loop.c +++ b/sysdep/unix/io-loop.c @@ -746,7 +746,7 @@ bird_thread_cleanup(void *_thr) pthread_attr_destroy(&thr->thread_attr); /* Free all remaining memory */ - rfree(thr->pool); + rp_free(thr->pool); } static struct bird_thread * @@ -839,7 +839,7 @@ bird_thread_shutdown(void * _ UNUSED) /* Stop the meta loop */ birdloop_leave(thr->meta); domain_free(thr->meta->time.domain); - rfree(thr->meta->pool); + rp_free(thr->meta->pool); /* Local pages not needed anymore */ flush_local_pages(); @@ -990,7 +990,7 @@ bird_thread_show(void *data) cli_write_trigger(tsd->cli); DOMAIN_FREE(control, tsd->lock); - rfree(tsd->pool); + rp_free(tsd->pool); the_bird_unlock(); } @@ -1258,7 +1258,7 @@ birdloop_free(struct birdloop *loop) ASSERT_DIE(loop->thread == NULL); domain_free(loop->time.domain); - rfree(loop->pool); + rp_free(loop->pool); } static void -- cgit v1.2.3 From 6230d87c74e3629e21f1e0fe22a874a58302a01e Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 22 Apr 2023 21:20:19 +0200 Subject: Protocols and tables now use the birdloop pools as primary --- lib/io-loop.h | 5 ++++- lib/resource.c | 10 ++++++++-- lib/resource.h | 3 +++ nest/proto.c | 18 +++++++++++++----- nest/rt-table.c | 12 ++++++++---- sysdep/unix/io-loop.c | 36 ++++++++++++++++++++++++++++-------- 6 files changed, 64 insertions(+), 20 deletions(-) diff --git a/lib/io-loop.h b/lib/io-loop.h index 877cd5ce..b0d3d6cc 100644 --- a/lib/io-loop.h +++ b/lib/io-loop.h @@ -17,7 +17,7 @@ extern struct birdloop main_birdloop; /* Start a new birdloop owned by given pool and domain */ -struct birdloop *birdloop_new(pool *p, uint order, const char *name, btime max_latency); +struct birdloop *birdloop_new(pool *p, uint order, btime max_latency, const char *fmt, ...); /* Stop the loop. At the end, the @stopped callback is called unlocked in tail * position to finish cleanup. Run birdloop_free() from that callback to free @@ -32,6 +32,9 @@ event_list *birdloop_event_list(struct birdloop *loop); /* Get birdloop's time heap */ struct timeloop *birdloop_time_loop(struct birdloop *loop); +/* Get birdloop's pool */ +pool *birdloop_pool(struct birdloop *loop); + /* Enter and exit the birdloop */ void birdloop_enter(struct birdloop *loop); void birdloop_leave(struct birdloop *loop); diff --git a/lib/resource.c b/lib/resource.c index 0006bc8d..b1b89bdf 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -64,13 +64,19 @@ rp_new(pool *p, const char *name) } pool * -rp_newf(pool *p, const char *fmt, ...) +rp_vnewf(pool *p, const char *fmt, va_list args) { pool *z = rp_new(p, NULL); + z->name = mb_vsprintf(p, fmt, args); + return z; +} +pool * +rp_newf(pool *p, const char *fmt, ...) +{ va_list args; va_start(args, fmt); - z->name = mb_vsprintf(p, fmt, args); + pool *z = rp_vnewf(p, fmt, args); va_end(args); return z; diff --git a/lib/resource.h b/lib/resource.h index 64803778..2adb9de0 100644 --- a/lib/resource.h +++ b/lib/resource.h @@ -12,6 +12,8 @@ #include "lib/lists.h" +#include + struct resmem { size_t effective; /* Memory actually used for data storage */ size_t overhead; /* Overhead memory imposed by allocator strategies */ @@ -60,6 +62,7 @@ void *ralloc(pool *, struct resclass *); pool *rp_new(pool *, const char *); /* Create a new pool */ pool *rp_newf(pool *, const char *, ...); /* Create a new pool with a formatted string as its name */ +pool *rp_vnewf(pool *, const char *, va_list); /* Create a new pool with a formatted string as its name */ void rp_init(pool *, const char *); /* Init a new pool */ void rp_initf(pool *, const char *, ...); /* Init a new pool with a formatted string as its name */ static inline void rp_free(pool *p) { rfree(&p->r); } /* Free the whole pool */ diff --git a/nest/proto.c b/nest/proto.c index 1b25dfe9..d6269272 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -1116,8 +1116,11 @@ proto_cleanup(struct proto *p) { CALL(p->proto->cleanup, p); - rp_free(p->pool); - p->pool = NULL; + if (p->pool) + { + rp_free(p->pool); + p->pool = NULL; + } p->active = 0; proto_log_state_change(p); @@ -1131,8 +1134,10 @@ proto_loop_stopped(void *ptr) birdloop_enter(&main_birdloop); + p->pool = NULL; /* is freed by birdloop_free() */ birdloop_free(p->loop); p->loop = &main_birdloop; + proto_cleanup(p); birdloop_leave(&main_birdloop); @@ -1214,13 +1219,16 @@ proto_start(struct proto *p) DBG("Kicking %s up\n", p->name); PD(p, "Starting"); - p->pool = rp_newf(proto_pool, "Protocol %s", p->cf->name); - if (graceful_restart_state == GRS_INIT) p->gr_recovery = 1; if (p->cf->loop_order != DOMAIN_ORDER(the_bird)) - p->loop = birdloop_new(p->pool, p->cf->loop_order, p->pool->name, p->cf->loop_max_latency); + { + p->loop = birdloop_new(proto_pool, p->cf->loop_order, p->cf->loop_max_latency, "Protocol %s", p->cf->name); + p->pool = birdloop_pool(p->loop); + } + else + p->pool = rp_newf(proto_pool, "Protocol %s", p->cf->name); p->iface_sub.target = proto_event_list(p); diff --git a/nest/rt-table.c b/nest/rt-table.c index cec13318..9629db2c 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2845,10 +2845,15 @@ rt_setup(pool *pp, struct rtable_config *cf) { ASSERT_DIE(birdloop_inside(&main_birdloop)); - pool *p = rp_newf(pp, "Routing table %s", cf->name); + /* Start the service thread */ + struct birdloop *loop = birdloop_new(pp, DOMAIN_ORDER(service), 0, "Routing table service %s", cf->name); + pool *sp = birdloop_pool(loop); + pool *p = rp_newf(sp, "Routing table data %s", cf->name); + /* Create the actual table */ struct rtable_private *t = ralloc(p, &rt_class); t->rp = p; + t->loop = loop; t->rte_slab = sl_new(p, sizeof(struct rte_storage)); @@ -2906,8 +2911,7 @@ rt_setup(pool *pp, struct rtable_config *cf) t->flowspec_trie->ipv4 = (t->addr_type == NET_FLOW4); } - /* Start the service thread */ - t->loop = birdloop_new(p, DOMAIN_ORDER(service), mb_sprintf(p, "Routing table %s", t->name), 0); + /* Setup the service thread flag handler */ birdloop_enter(t->loop); birdloop_flag_set_handler(t->loop, &t->fh); birdloop_leave(t->loop); @@ -4073,8 +4077,8 @@ rt_delete(void *tab_) RT_UNLOCK(RT_PUB(tab)); + /* Everything is freed by freeing the loop */ birdloop_free(tab->loop); - rfree(tab->rp); config_del_obstacle(conf); birdloop_leave(&main_birdloop); diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c index 8481bb6e..701eddf7 100644 --- a/sysdep/unix/io-loop.c +++ b/sysdep/unix/io-loop.c @@ -31,7 +31,7 @@ #define THREAD_STACK_SIZE 65536 /* To be lowered in near future */ -static struct birdloop *birdloop_new_internal(pool *pp, uint order, const char *name, int request_pickup, struct birdloop_pickup_group *group); +static struct birdloop *birdloop_new_no_pickup(pool *pp, uint order, const char *name, ...); /* * Nanosecond time for accounting purposes @@ -87,6 +87,12 @@ birdloop_time_loop(struct birdloop *loop) return &loop->time; } +pool * +birdloop_pool(struct birdloop *loop) +{ + return loop->pool; +} + _Bool birdloop_inside(struct birdloop *loop) { @@ -627,7 +633,7 @@ bird_thread_main(void *arg) tmp_init(thr->pool); init_list(&thr->loops); - thr->meta = birdloop_new_internal(thr->pool, DOMAIN_ORDER(meta), "Thread Meta", 0, thr->group); + thr->meta = birdloop_new_no_pickup(thr->pool, DOMAIN_ORDER(meta), "Thread Meta"); thr->meta->thread = thr; birdloop_enter(thr->meta); @@ -885,7 +891,7 @@ bird_thread_commit(struct config *new, struct config *old UNUSED) if ((dif > 0) && !thread_dropper_running) { - struct birdloop *tdl = birdloop_new(&root_pool, DOMAIN_ORDER(control), "Thread dropper", group->max_latency); + struct birdloop *tdl = birdloop_new(&root_pool, DOMAIN_ORDER(control), group->max_latency, "Thread dropper"); event *tde = ev_new_init(tdl->pool, bird_thread_shutdown, NULL); LOCK_DOMAIN(resource, group->domain); @@ -1178,11 +1184,11 @@ birdloop_run_timer(timer *tm) } static struct birdloop * -birdloop_new_internal(pool *pp, uint order, const char *name, int request_pickup, struct birdloop_pickup_group *group) +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); - pool *p = rp_new(pp, name); + pool *p = rp_vnewf(pp, name, args); struct birdloop *loop = mb_allocz(p, sizeof(struct birdloop)); loop->pool = p; @@ -1200,7 +1206,7 @@ birdloop_new_internal(pool *pp, uint order, const char *name, int request_pickup loop->event = (event) { .hook = birdloop_run, .data = loop, }; loop->timer = (timer) { .hook = birdloop_run_timer, .data = loop, }; - if (request_pickup) + if (group) { LOCK_DOMAIN(resource, group->domain); add_tail(&group->loops, &loop->n); @@ -1218,10 +1224,24 @@ birdloop_new_internal(pool *pp, uint order, const char *name, int request_pickup return loop; } +static struct birdloop * +birdloop_new_no_pickup(pool *pp, uint order, const char *name, ...) +{ + va_list args; + va_start(args, name); + struct birdloop *loop = birdloop_vnew_internal(pp, order, NULL, name, args); + va_end(args); + return loop; +} + struct birdloop * -birdloop_new(pool *pp, uint order, const char *name, btime max_latency) +birdloop_new(pool *pp, uint order, btime max_latency, const char *name, ...) { - return birdloop_new_internal(pp, order, name, 1, max_latency ? &pickup_groups[1] : &pickup_groups[0]); + va_list args; + va_start(args, name); + struct birdloop *loop = birdloop_vnew_internal(pp, order, max_latency ? &pickup_groups[1] : &pickup_groups[0], name, args); + va_end(args); + return loop; } static void -- cgit v1.2.3 From 22f54eaee6c6dbe12ad7bb0ee1da09e3e026b970 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Fri, 21 Apr 2023 15:26:06 +0200 Subject: Resource pools are now bound with domains. Memory allocation is a fragile part of BIRD and we need checking that everybody is using the resource pools in an appropriate way. To assure this, all the resource pools are associated with locking domains and every resource manipulation is thoroughly checked whether the appropriate locking domain is locked. With transitive resource manipulation like resource dumping or mass free operations, domains are locked and unlocked on the go, thus we require pool domains to have higher order than their parent to allow for this transitive operations. Adding pool locking revealed some cases of insecure memory manipulation and this commit fixes that as well. --- conf/conf.c | 4 +- lib/hash_test.c | 2 +- lib/io-loop.h | 1 + lib/locking.h | 2 +- lib/resource.c | 139 ++++++++++++++++++++++++++++++++++++-------------- lib/resource.h | 34 +++++++----- nest/cli.c | 6 +-- nest/cli.h | 1 + nest/iface.c | 7 ++- nest/locks.h | 1 + nest/password.h | 2 + nest/proto.c | 12 +++-- nest/protocol.h | 2 + nest/rt-attr.c | 20 ++++++-- nest/rt-show.c | 1 + nest/rt-table.c | 63 ++++++++++++++--------- nest/rt.h | 3 +- proto/babel/babel.c | 2 +- proto/bfd/bfd.c | 2 +- proto/bgp/attrs.c | 4 +- proto/bgp/bgp.c | 3 +- proto/ospf/iface.c | 4 +- proto/ospf/neighbor.c | 2 +- proto/radv/radv.c | 2 +- proto/rpki/rpki.c | 2 +- sysdep/unix/io-loop.c | 59 ++++++++++++++------- sysdep/unix/krt.c | 2 +- 27 files changed, 257 insertions(+), 125 deletions(-) diff --git a/conf/conf.c b/conf/conf.c index 2cfe7b15..c7cd81fe 100644 --- a/conf/conf.c +++ b/conf/conf.c @@ -90,7 +90,7 @@ int undo_available; /* Undo was not requested from last reconfiguration */ struct config * config_alloc(const char *name) { - pool *p = rp_new(config_pool, "Config"); + pool *p = rp_new(config_pool, the_bird_domain.the_bird, "Config"); linpool *l = lp_new_default(p); struct config *c = lp_allocz(l, sizeof(struct config)); @@ -515,7 +515,7 @@ config_timeout(timer *t UNUSED) void config_init(void) { - config_pool = rp_new(&root_pool, "Configurations"); + config_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Configurations"); config_event = ev_new(config_pool); config_event->hook = config_done; diff --git a/lib/hash_test.c b/lib/hash_test.c index ecfcdd66..30213320 100644 --- a/lib/hash_test.c +++ b/lib/hash_test.c @@ -61,7 +61,7 @@ dump_nodes(void) static void init_hash_(uint order) { - my_pool = rp_new(&root_pool, "Test pool"); + my_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Test pool"); HASH_INIT(hash, my_pool, order); diff --git a/lib/io-loop.h b/lib/io-loop.h index b0d3d6cc..80cd2ea2 100644 --- a/lib/io-loop.h +++ b/lib/io-loop.h @@ -31,6 +31,7 @@ event_list *birdloop_event_list(struct birdloop *loop); /* Get birdloop's time heap */ struct timeloop *birdloop_time_loop(struct birdloop *loop); +#define birdloop_domain(l) (birdloop_time_loop((l))->domain) /* Get birdloop's pool */ pool *birdloop_pool(struct birdloop *loop); diff --git a/lib/locking.h b/lib/locking.h index 6bed14bf..751eecbd 100644 --- a/lib/locking.h +++ b/lib/locking.h @@ -13,8 +13,8 @@ struct domain_generic; /* Here define the global lock order; first to last. */ struct lock_order { - struct domain_generic *meta; struct domain_generic *the_bird; + struct domain_generic *meta; struct domain_generic *control; struct domain_generic *proto; struct domain_generic *service; diff --git a/lib/resource.c b/lib/resource.c index b1b89bdf..2071a411 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -46,6 +46,16 @@ static struct resclass pool_class = { pool root_pool; +static void +rp_init(pool *z, struct domain_generic *dom, const char *name) +{ + ASSERT_DIE(DG_IS_LOCKED(dom)); + + z->name = name; + z->domain = dom; + z->inside = (TLIST_LIST(resource)) {}; +} + /** * rp_new - create a resource pool * @p: parent pool @@ -55,77 +65,106 @@ pool root_pool; * parent pool. */ pool * -rp_new(pool *p, const char *name) +rp_new(pool *p, struct domain_generic *dom, const char *name) { pool *z = ralloc(p, &pool_class); - z->name = name; - init_list(&z->inside); + + if (dg_order(p->domain) > dg_order(dom)) + bug("Requested reverse order pool creation: %s (%d) can't be a parent of %s (%d)", + domain_name(p->domain), dg_order(p->domain), + domain_name(dom), dg_order(dom)); + + if ((dg_order(p->domain) == dg_order(dom)) && (p->domain != dom)) + bug("Requested incomparable order pool creation: %s (%d) can't be a parent of %s (%d)", + domain_name(p->domain), dg_order(p->domain), + domain_name(dom), dg_order(dom)); + + rp_init(z, dom, name); return z; } pool * -rp_vnewf(pool *p, const char *fmt, va_list args) +rp_vnewf(pool *p, struct domain_generic *dom, const char *fmt, va_list args) { - pool *z = rp_new(p, NULL); + pool *z = rp_new(p, dom, NULL); z->name = mb_vsprintf(p, fmt, args); return z; } pool * -rp_newf(pool *p, const char *fmt, ...) +rp_newf(pool *p, struct domain_generic *dom, const char *fmt, ...) { va_list args; va_start(args, fmt); - pool *z = rp_vnewf(p, fmt, args); + pool *z = rp_vnewf(p, dom, fmt, args); va_end(args); return z; } +#define POOL_LOCK \ + struct domain_generic *dom = p->domain; \ + int locking = !DG_IS_LOCKED(dom); \ + if (locking) \ + DG_LOCK(dom); \ + +#define POOL_UNLOCK if (locking) DG_UNLOCK(dom);\ + +void rp_free(pool *p) +{ + ASSERT_DIE(DG_IS_LOCKED(p->domain)); + rfree(p); +} static void pool_free(resource *P) { pool *p = (pool *) P; - resource *r, *rr; - r = HEAD(p->inside); - while (rr = (resource *) r->n.next) + POOL_LOCK; + WALK_TLIST_DELSAFE(resource, r, &p->inside) { r->class->free(r); xfree(r); - r = rr; } + POOL_UNLOCK; } + static void pool_dump(resource *P, unsigned indent) { pool *p = (pool *) P; - resource *r; + + POOL_LOCK; debug("%s\n", p->name); - WALK_LIST(r, p->inside) + WALK_TLIST_DELSAFE(resource, r, &p->inside) rdump(r, indent + 3); + + POOL_UNLOCK; } static struct resmem pool_memsize(resource *P) { pool *p = (pool *) P; - resource *r; struct resmem sum = { .effective = 0, .overhead = sizeof(pool) + ALLOC_OVERHEAD, }; - WALK_LIST(r, p->inside) + POOL_LOCK; + + WALK_TLIST(resource, r, &p->inside) { struct resmem add = rmemsize(r); sum.effective += add.effective; sum.overhead += add.overhead; } + POOL_UNLOCK; + return sum; } @@ -133,14 +172,25 @@ static resource * pool_lookup(resource *P, unsigned long a) { pool *p = (pool *) P; - resource *r, *q; + resource *q = NULL; + + POOL_LOCK; - WALK_LIST(r, p->inside) + WALK_TLIST(resource, r, &p->inside) if (r->class->lookup && (q = r->class->lookup(r, a))) - return q; - return NULL; + break; + + POOL_UNLOCK; + return q; +} + +static pool * +resource_parent(resource *r) +{ + return SKIP_BACK(pool, inside, resource_enlisted(r)); } + /** * rmove - move a resource * @res: resource @@ -152,13 +202,13 @@ pool_lookup(resource *P, unsigned long a) void rmove(void *res, pool *p) { resource *r = res; + pool *orig = resource_parent(r); - if (r) - { - if (r->n.next) - rem_node(&r->n); - add_tail(&p->inside, &r->n); - } + ASSERT_DIE(DG_IS_LOCKED(orig->domain)); + ASSERT_DIE(DG_IS_LOCKED(p->domain)); + + resource_rem_node(&orig->inside, r); + resource_add_tail(&p->inside, r); } /** @@ -179,8 +229,10 @@ rfree(void *res) if (!r) return; - if (r->n.next) - rem_node(&r->n); + pool *orig = resource_parent(r); + ASSERT_DIE(DG_IS_LOCKED(orig->domain)); + resource_rem_node(&orig->inside, r); + r->class->free(r); r->class = NULL; xfree(r); @@ -239,12 +291,14 @@ rmemsize(void *res) void * ralloc(pool *p, struct resclass *c) { + ASSERT_DIE(DG_IS_LOCKED(p->domain)); + resource *r = xmalloc(c->size); bzero(r, c->size); r->class = c; - if (p) - add_tail(&p->inside, &r->n); + resource_add_tail(&p->inside, r); + return r; } @@ -284,28 +338,29 @@ resource_init(void) rcu_init(); resource_sys_init(); - root_pool.r.class = &pool_class; - root_pool.name = "Root"; - init_list(&root_pool.inside); - tmp_init(&root_pool); + rp_init(&root_pool, the_bird_domain.the_bird, "Root"); + tmp_init(&root_pool, the_bird_domain.the_bird); } _Thread_local struct tmp_resources tmp_res; void -tmp_init(pool *p) +tmp_init(pool *p, struct domain_generic *dom) { tmp_res.lp = lp_new_default(p); tmp_res.parent = p; - tmp_res.pool = rp_new(p, "TMP"); + tmp_res.pool = rp_new(p, dom, "TMP"); + tmp_res.domain = dom; } void tmp_flush(void) { + ASSERT_DIE(DG_IS_LOCKED(tmp_res.domain)); + lp_flush(tmp_linpool); rp_free(tmp_res.pool); - tmp_res.pool = rp_new(tmp_res.parent, "TMP"); + tmp_res.pool = rp_new(tmp_res.parent, tmp_res.domain, "TMP"); } @@ -385,11 +440,13 @@ static struct resclass mb_class = { void * mb_alloc(pool *p, unsigned size) { + ASSERT_DIE(DG_IS_LOCKED(p->domain)); + struct mblock *b = xmalloc(sizeof(struct mblock) + size); b->r.class = &mb_class; - b->r.n = (node) {}; - add_tail(&p->inside, &b->r.n); + b->r.n = (struct resource_node) {}; + resource_add_tail(&p->inside, &b->r); b->size = size; return b->data; } @@ -434,10 +491,14 @@ void * mb_realloc(void *m, unsigned size) { struct mblock *b = SKIP_BACK(struct mblock, data, m); + struct pool *p = resource_parent(&b->r); + + ASSERT_DIE(DG_IS_LOCKED(p->domain)); b = xrealloc(b, sizeof(struct mblock) + size); - update_node(&b->r.n); b->size = size; + + resource_update_node(&p->inside, &b->r); return b->data; } diff --git a/lib/resource.h b/lib/resource.h index 2adb9de0..810334c1 100644 --- a/lib/resource.h +++ b/lib/resource.h @@ -10,7 +10,8 @@ #ifndef _BIRD_RESOURCE_H_ #define _BIRD_RESOURCE_H_ -#include "lib/lists.h" +#include "lib/locking.h" +#include "lib/tlists.h" #include @@ -21,11 +22,20 @@ struct resmem { /* Resource */ +#define TLIST_PREFIX resource +#define TLIST_TYPE struct resource +#define TLIST_ITEM n +#define TLIST_WANT_WALK +#define TLIST_WANT_ADD_TAIL +#define TLIST_WANT_UPDATE_NODE + typedef struct resource { - node n; /* Inside resource pool */ - struct resclass *class; /* Resource class */ + TLIST_DEFAULT_NODE; /* Inside resource pool */ + const struct resclass *class; /* Resource class */ } resource; +#include "lib/tlists.h" + /* Resource class */ struct resclass { @@ -44,14 +54,13 @@ struct resclass { typedef struct pool { resource r; - list inside; + TLIST_LIST(resource) inside; + struct domain_generic *domain; const char *name; } pool; void resource_init(void); -pool *rp_new(pool *, const char *); /* Create new pool */ -pool *rp_newf(pool *, const char *, ...); /* Create a new pool with a formatted string as its name */ void rfree(void *); /* Free single resource */ void rdump(void *, unsigned indent); /* Dump to debug output */ struct resmem rmemsize(void *res); /* Return size of memory used by the resource */ @@ -60,12 +69,10 @@ void rmove(void *, pool *); /* Move to a different pool */ void *ralloc(pool *, struct resclass *); -pool *rp_new(pool *, const char *); /* Create a new pool */ -pool *rp_newf(pool *, const char *, ...); /* Create a new pool with a formatted string as its name */ -pool *rp_vnewf(pool *, const char *, va_list); /* Create a new pool with a formatted string as its name */ -void rp_init(pool *, const char *); /* Init a new pool */ -void rp_initf(pool *, const char *, ...); /* Init a new pool with a formatted string as its name */ -static inline void rp_free(pool *p) { rfree(&p->r); } /* Free the whole pool */ +pool *rp_new(pool *, struct domain_generic *, const char *); /* Create a new pool */ +pool *rp_newf(pool *, struct domain_generic *, const char *, ...); /* Create a new pool with a formatted string as its name */ +pool *rp_vnewf(pool *, struct domain_generic *, const char *, va_list); /* Create a new pool with a formatted string as its name */ +void rp_free(pool *p); /* Free the whole pool */ extern pool root_pool; @@ -97,6 +104,7 @@ void lp_restore(linpool *m, lp_state *p); /* Restore state */ struct tmp_resources { pool *pool, *parent; linpool *lp; + struct domain_generic *domain; }; extern _Thread_local struct tmp_resources tmp_res; @@ -106,7 +114,7 @@ extern _Thread_local struct tmp_resources tmp_res; #define tmp_allocu(sz) lp_allocu(tmp_linpool, sz) #define tmp_allocz(sz) lp_allocz(tmp_linpool, sz) -void tmp_init(pool *p); +void tmp_init(pool *p, struct domain_generic *dg); void tmp_flush(void); diff --git a/nest/cli.c b/nest/cli.c index 29591d26..b74edffb 100644 --- a/nest/cli.c +++ b/nest/cli.c @@ -262,7 +262,7 @@ cli_command(struct cli *c) log(L_TRACE "CLI: %s", c->rx_buf); bzero(&f, sizeof(f)); f.mem = c->parser_pool; - f.pool = rp_new(c->pool, "Config"); + f.pool = rp_new(c->pool, the_bird_domain.the_bird, "Config"); init_list(&f.symbols); cf_read_hook = cli_cmd_read_hook; cli_rh_pos = c->rx_buf; @@ -309,7 +309,7 @@ cli_event(void *data) cli * cli_new(struct birdsock *sock) { - pool *p = rp_new(cli_pool, "CLI"); + pool *p = rp_new(cli_pool, the_bird_domain.the_bird, "CLI"); cli *c = mb_alloc(p, sizeof(cli)); bzero(c, sizeof(cli)); @@ -433,7 +433,7 @@ cli_free(cli *c) void cli_init(void) { - cli_pool = rp_new(&root_pool, "CLI"); + cli_pool = rp_new(&root_pool, the_bird_domain.the_bird, "CLI"); init_list(&cli_log_hooks); cli_log_inited = 1; } diff --git a/nest/cli.h b/nest/cli.h index 2d876571..c5cbd8d7 100644 --- a/nest/cli.h +++ b/nest/cli.h @@ -10,6 +10,7 @@ #define _BIRD_CLI_H_ #include "lib/resource.h" +#include "lib/lists.h" #include "lib/event.h" #define CLI_RX_BUF_SIZE 4096 diff --git a/nest/iface.c b/nest/iface.c index f1938664..a024b943 100644 --- a/nest/iface.c +++ b/nest/iface.c @@ -1018,12 +1018,15 @@ if_choose_router_id(struct iface_patt *mask, u32 old_id) void if_init(void) { - if_pool = rp_new(&root_pool, "Interfaces"); + iface_domain = DOMAIN_NEW(attrs, "Interfaces"); + + IFACE_LOCK; + if_pool = rp_new(&root_pool, iface_domain.attrs, "Interfaces"); init_list(&global_iface_list); iface_sub_slab = sl_new(if_pool, sizeof(struct iface_notification)); strcpy(default_vrf.name, "default"); neigh_init(if_pool); - iface_domain = DOMAIN_NEW(attrs, "Interfaces"); + IFACE_UNLOCK; } /* diff --git a/nest/locks.h b/nest/locks.h index 04571e69..993e296b 100644 --- a/nest/locks.h +++ b/nest/locks.h @@ -10,6 +10,7 @@ #define _BIRD_LOCKS_H_ #include "lib/resource.h" +#include "lib/lists.h" #include "lib/event.h" /* diff --git a/nest/password.h b/nest/password.h index 53168bb7..335b9cd4 100644 --- a/nest/password.h +++ b/nest/password.h @@ -10,6 +10,8 @@ #ifndef PASSWORD_H #define PASSWORD_H +#include "lib/lists.h" + struct password_item { node n; const char *password; /* Key data, null terminated */ diff --git a/nest/proto.c b/nest/proto.c index d6269272..32183c9d 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -367,6 +367,7 @@ channel_roa_subscribe(struct channel *c, rtable *tab, int dir) .name = mb_sprintf(c->proto->pool, "%s.%s.roa-%s.%s", c->proto->name, c->name, dir ? "in" : "out", tab->name), .list = proto_work_list(c->proto), + .pool = c->proto->pool, .trace_routes = c->debug | c->proto->debug, .dump_req = channel_dump_roa_req, .export_one = channel_export_one_roa, @@ -495,6 +496,7 @@ channel_start_export(struct channel *c) c->out_req = (struct rt_export_request) { .name = mb_sprintf(c->proto->pool, "%s.%s", c->proto->name, c->name), .list = proto_work_list(c->proto), + .pool = c->proto->pool, .addr = c->out_subprefix, .addr_mode = c->out_subprefix ? TE_ADDR_IN : TE_ADDR_NONE, .trace_routes = c->debug | c->proto->debug, @@ -685,6 +687,7 @@ channel_setup_in_table(struct channel *c) c->reload_req = (struct rt_export_request) { .name = mb_sprintf(c->proto->pool, "%s.%s.import", c->proto->name, c->name), .list = proto_work_list(c->proto), + .pool = c->proto->pool, .trace_routes = c->debug | c->proto->debug, .export_bulk = channel_reload_export_bulk, .dump_req = channel_reload_dump_req, @@ -1132,15 +1135,14 @@ proto_loop_stopped(void *ptr) { struct proto *p = ptr; - birdloop_enter(&main_birdloop); + ASSERT_DIE(birdloop_inside(&main_birdloop)); + ASSERT_DIE(p->loop != &main_birdloop); p->pool = NULL; /* is freed by birdloop_free() */ birdloop_free(p->loop); p->loop = &main_birdloop; proto_cleanup(p); - - birdloop_leave(&main_birdloop); } static void @@ -1228,7 +1230,7 @@ proto_start(struct proto *p) p->pool = birdloop_pool(p->loop); } else - p->pool = rp_newf(proto_pool, "Protocol %s", p->cf->name); + p->pool = rp_newf(proto_pool, the_bird_domain.the_bird, "Protocol %s", p->cf->name); p->iface_sub.target = proto_event_list(p); @@ -1859,7 +1861,7 @@ protos_build(void) { protos_build_gen(); - proto_pool = rp_new(&root_pool, "Protocols"); + proto_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Protocols"); } diff --git a/nest/protocol.h b/nest/protocol.h index eccfcb73..02ec5c15 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -280,6 +280,8 @@ struct proto *proto_iterate_named(struct symbol *sym, struct protocol *proto, st #define PROTO_LOCKED_FROM_MAIN(p) for (struct birdloop *_proto_loop = PROTO_ENTER_FROM_MAIN(p); _proto_loop; PROTO_LEAVE_FROM_MAIN(_proto_loop), (_proto_loop = NULL)) +static inline struct domain_generic *proto_domain(struct proto *p) +{ return birdloop_domain(p->loop); } #define CMD_RELOAD 0 #define CMD_RELOAD_IN 1 diff --git a/nest/rt-attr.c b/nest/rt-attr.c index 903926f6..38612a4e 100644 --- a/nest/rt-attr.c +++ b/nest/rt-attr.c @@ -605,9 +605,16 @@ ea_register(pool *p, struct ea_class *def) struct ea_class_ref * ea_register_alloc(pool *p, struct ea_class cl) { + struct ea_class_ref *ref; + + RTA_LOCK; struct ea_class *clp = ea_class_find_by_name(cl.name); if (clp && clp->type == cl.type) - return ea_ref_class(p, clp); + { + ref = ea_ref_class(p, clp); + RTA_UNLOCK; + return ref; + } uint namelen = strlen(cl.name) + 1; @@ -619,14 +626,18 @@ ea_register_alloc(pool *p, struct ea_class cl) memcpy(cla->name, cl.name, namelen); cla->cl.name = cla->name; - return ea_register(p, &cla->cl); + ref = ea_register(p, &cla->cl); + RTA_UNLOCK; + return ref; } void ea_register_init(struct ea_class *clp) { + RTA_LOCK; ASSERT_DIE(!ea_class_find_by_name(clp->name)); ea_register(&root_pool, clp); + RTA_UNLOCK; } struct ea_class * @@ -1598,7 +1609,8 @@ rta_init(void) { attrs_domain = DOMAIN_NEW(attrs, "Attributes"); - rta_pool = rp_new(&root_pool, "Attributes"); + RTA_LOCK; + rta_pool = rp_new(&root_pool, attrs_domain.attrs, "Attributes"); for (uint i=0; iaddr, .name = "CLI Show Route", .list = &global_work_list, + .pool = c->pool, .export_bulk = rt_show_net_export_bulk, .dump_req = rt_show_dump_req, .log_state_change = rt_show_log_state_change, diff --git a/nest/rt-table.c b/nest/rt-table.c index 9629db2c..f1e3c8f7 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2131,7 +2131,7 @@ rt_table_export_start_locked(struct rtable_private *tab, struct rt_export_reques struct rt_exporter *re = &tab->exporter.e; rt_lock_table(tab); - req->hook = rt_alloc_export(re, sizeof(struct rt_table_export_hook)); + req->hook = rt_alloc_export(re, req->pool, sizeof(struct rt_table_export_hook)); req->hook->req = req; struct rt_table_export_hook *hook = SKIP_BACK(struct rt_table_export_hook, h, req->hook); @@ -2212,9 +2212,9 @@ rt_request_export_other(struct rt_exporter *re, struct rt_export_request *req) } struct rt_export_hook * -rt_alloc_export(struct rt_exporter *re, uint size) +rt_alloc_export(struct rt_exporter *re, pool *pp, uint size) { - pool *p = rp_new(re->rp, "Export hook"); + pool *p = rp_new(pp, pp->domain, "Export hook"); struct rt_export_hook *hook = mb_allocz(p, size); hook->pool = p; @@ -2709,13 +2709,14 @@ rt_flowspec_link(rtable *src_pub, rtable *dst_pub) if (!ln) { - pool *p = src->rp; + pool *p = birdloop_pool(dst_pub->loop); ln = mb_allocz(p, sizeof(struct rt_flowspec_link)); ln->src = src_pub; ln->dst = dst_pub; ln->req = (struct rt_export_request) { .name = mb_sprintf(p, "%s.flowspec.notifier", dst_pub->name), .list = birdloop_event_list(dst_pub->loop), + .pool = p, .trace_routes = src->config->debug, .dump_req = rt_flowspec_dump_req, .log_state_change = rt_flowspec_log_state_change, @@ -2781,8 +2782,6 @@ rt_free(resource *_r) { struct rtable_private *r = SKIP_BACK(struct rtable_private, r, _r); - DOMAIN_FREE(rtable, r->lock); - DBG("Deleting routing table %s\n", r->name); ASSERT_DIE(r->use_count == 0); @@ -2847,13 +2846,20 @@ rt_setup(pool *pp, struct rtable_config *cf) /* Start the service thread */ struct birdloop *loop = birdloop_new(pp, DOMAIN_ORDER(service), 0, "Routing table service %s", cf->name); + birdloop_enter(loop); pool *sp = birdloop_pool(loop); - pool *p = rp_newf(sp, "Routing table data %s", cf->name); + + /* Create the table domain and pool */ + DOMAIN(rtable) dom = DOMAIN_NEW(rtable, cf->name); + LOCK_DOMAIN(rtable, dom); + + pool *p = rp_newf(sp, dom.rtable, "Routing table data %s", cf->name); /* Create the actual table */ struct rtable_private *t = ralloc(p, &rt_class); t->rp = p; t->loop = loop; + t->lock = dom; t->rte_slab = sl_new(p, sizeof(struct rte_storage)); @@ -2864,8 +2870,6 @@ rt_setup(pool *pp, struct rtable_config *cf) if (t->id >= rtable_max_id) rtable_max_id = t->id + 1; - t->lock = DOMAIN_NEW(rtable, t->name); - fib_init(&t->fib, p, t->addr_type, sizeof(net), OFFSETOF(net, n), 0, NULL); if (cf->trie_used) @@ -2911,8 +2915,9 @@ rt_setup(pool *pp, struct rtable_config *cf) t->flowspec_trie->ipv4 = (t->addr_type == NET_FLOW4); } + UNLOCK_DOMAIN(rtable, dom); + /* Setup the service thread flag handler */ - birdloop_enter(t->loop); birdloop_flag_set_handler(t->loop, &t->fh); birdloop_leave(t->loop); @@ -2929,7 +2934,7 @@ void rt_init(void) { rta_init(); - rt_table_pool = rp_new(&root_pool, "Routing tables"); + rt_table_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Routing tables"); init_list(&routing_tables); init_list(&deleted_routing_tables); ev_init_list(&rt_cork.queue, &main_birdloop, "Route cork release"); @@ -4067,13 +4072,14 @@ rt_shutdown(void *tab_) static void rt_delete(void *tab_) { - birdloop_enter(&main_birdloop); + ASSERT_DIE(birdloop_inside(&main_birdloop)); /* We assume that nobody holds the table reference now as use_count is zero. * Anyway the last holder may still hold the lock. Therefore we lock and * unlock it the last time to be sure that nobody is there. */ struct rtable_private *tab = RT_LOCK((rtable *) tab_); struct config *conf = tab->deleted; + DOMAIN(rtable) dom = tab->lock; RT_UNLOCK(RT_PUB(tab)); @@ -4081,7 +4087,8 @@ rt_delete(void *tab_) birdloop_free(tab->loop); config_del_obstacle(conf); - birdloop_leave(&main_birdloop); + /* Also drop the domain */ + DOMAIN_FREE(rtable, dom); } @@ -4636,18 +4643,9 @@ rt_init_hostcache(struct rtable_private *tab) .data = tab, }; - hc->req = (struct rt_export_request) { - .name = mb_sprintf(tab->rp, "%s.hcu.notifier", tab->name), - .list = birdloop_event_list(tab->loop), - .trace_routes = tab->config->debug, - .dump_req = hc_notify_dump_req, - .log_state_change = hc_notify_log_state_change, - .export_one = hc_notify_export_one, - }; - - rt_table_export_start_locked(tab, &hc->req); - tab->hostcache = hc; + + ev_send_loop(tab->loop, &hc->update); } static void @@ -4775,9 +4773,24 @@ rt_update_hostcache(void *data) RT_LOCKED((rtable *) data, tab) { - struct hostcache *hc = tab->hostcache; + /* Finish initialization */ + if (!hc->req.name) + { + hc->req = (struct rt_export_request) { + .name = mb_sprintf(tab->rp, "%s.hcu.notifier", tab->name), + .list = birdloop_event_list(tab->loop), + .pool = tab->rp, + .trace_routes = tab->config->debug, + .dump_req = hc_notify_dump_req, + .log_state_change = hc_notify_log_state_change, + .export_one = hc_notify_export_one, + }; + + rt_table_export_start_locked(tab, &hc->req); + } + /* Shutdown shortcut */ if (!hc->req.hook) RT_RETURN(tab); diff --git a/nest/rt.h b/nest/rt.h index 01372b8c..4fe8eb53 100644 --- a/nest/rt.h +++ b/nest/rt.h @@ -294,6 +294,7 @@ struct rt_export_request { u8 addr_mode; /* Network prefilter mode (TE_ADDR_*) */ event_list *list; /* Where to schedule export events */ + pool *pool; /* Pool to use for allocations */ /* There are two methods of export. You can either request feeding every single change * or feeding the whole route feed. In case of regular export, &export_one is preferred. @@ -438,7 +439,7 @@ int rpe_get_seen(struct rt_export_hook *hook, struct rt_pending_export *rpe); */ void rt_init_export(struct rt_exporter *re, struct rt_export_hook *hook); -struct rt_export_hook *rt_alloc_export(struct rt_exporter *re, uint size); +struct rt_export_hook *rt_alloc_export(struct rt_exporter *re, pool *pool, uint size); void rt_stop_export_common(struct rt_export_hook *hook); void rt_export_stopped(struct rt_export_hook *hook); void rt_exporter_init(struct rt_exporter *re); diff --git a/proto/babel/babel.c b/proto/babel/babel.c index e95a0ead..decaa407 100644 --- a/proto/babel/babel.c +++ b/proto/babel/babel.c @@ -1826,7 +1826,7 @@ babel_add_iface(struct babel_proto *p, struct iface *new, struct babel_iface_con TRACE(D_EVENTS, "Adding interface %s", new->name); - pool *pool = rp_new(p->p.pool, new->name); + pool *pool = rp_new(p->p.pool, proto_domain(&p->p), new->name); ifa = mb_allocz(pool, sizeof(struct babel_iface)); ifa->proto = p; diff --git a/proto/bfd/bfd.c b/proto/bfd/bfd.c index ddbc4948..f7d8f8d9 100644 --- a/proto/bfd/bfd.c +++ b/proto/bfd/bfd.c @@ -1073,7 +1073,7 @@ bfd_start(struct proto *P) pthread_spin_init(&p->lock, PTHREAD_PROCESS_PRIVATE); - p->tpool = rp_new(P->pool, "BFD loop pool"); + p->tpool = birdloop_pool(P->loop); p->session_slab = sl_new(P->pool, sizeof(struct bfd_session)); HASH_INIT(p->session_hash_id, P->pool, 8); diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 4e6524f4..82971c01 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -1853,7 +1853,7 @@ bgp_init_pending_tx(struct bgp_channel *c) { ASSERT_DIE(!c->ptx); - pool *p = rp_new(c->pool, "BGP Pending TX"); + pool *p = rp_new(c->pool, proto_domain(c->c.proto), "BGP Pending TX"); c->ptx = ralloc(p, &bgp_pending_tx_class); c->ptx->pool = p; @@ -1981,7 +1981,7 @@ bgp_out_table_feed(void *data) static void bgp_out_table_export_start(struct rt_exporter *re, struct rt_export_request *req) { - req->hook = rt_alloc_export(re, sizeof(struct bgp_out_export_hook)); + req->hook = rt_alloc_export(re, req->pool, sizeof(struct bgp_out_export_hook)); req->hook->req = req; struct bgp_out_export_hook *hook = SKIP_BACK(struct bgp_out_export_hook, h, req->hook); diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 0525d502..e60884ba 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -922,7 +922,8 @@ bgp_graceful_restart_feed(struct bgp_channel *c) { c->stale_feed = (struct rt_export_request) { .name = "BGP-GR", - .list = &global_work_list, + .list = proto_event_list(c->c.proto), + .pool = c->c.proto->pool, .trace_routes = c->c.debug | c->c.proto->debug, .dump_req = bgp_graceful_restart_feed_dump_req, .log_state_change = bgp_graceful_restart_feed_log_state_change, diff --git a/proto/ospf/iface.c b/proto/ospf/iface.c index 0eecb432..c3ec4a4c 100644 --- a/proto/ospf/iface.c +++ b/proto/ospf/iface.c @@ -567,7 +567,7 @@ ospf_iface_new(struct ospf_area *oa, struct ifa *addr, struct ospf_iface_patt *i OSPF_TRACE(D_EVENTS, "Adding interface %s (%N) to area %R", iface->name, &addr->prefix, oa->areaid); - pool = rp_new(p->p.pool, "OSPF Interface"); + pool = rp_new(p->p.pool, proto_domain(&p->p), "OSPF Interface"); ifa = mb_allocz(pool, sizeof(struct ospf_iface)); ifa->iface = iface; ifa->addr = addr; @@ -690,7 +690,7 @@ ospf_iface_new_vlink(struct ospf_proto *p, struct ospf_iface_patt *ip) /* Vlink ifname is stored just after the ospf_iface structure */ - pool = rp_new(p->p.pool, "OSPF Vlink"); + pool = rp_new(p->p.pool, proto_domain(&p->p), "OSPF Vlink"); ifa = mb_allocz(pool, sizeof(struct ospf_iface) + 16); ifa->oa = p->backbone; ifa->cf = ip; diff --git a/proto/ospf/neighbor.c b/proto/ospf/neighbor.c index 2c73d251..59a60587 100644 --- a/proto/ospf/neighbor.c +++ b/proto/ospf/neighbor.c @@ -80,7 +80,7 @@ struct ospf_neighbor * ospf_neighbor_new(struct ospf_iface *ifa) { struct ospf_proto *p = ifa->oa->po; - struct pool *pool = rp_new(p->p.pool, "OSPF Neighbor"); + struct pool *pool = rp_new(p->p.pool, proto_domain(&p->p), "OSPF Neighbor"); struct ospf_neighbor *n = mb_allocz(pool, sizeof(struct ospf_neighbor)); n->pool = pool; diff --git a/proto/radv/radv.c b/proto/radv/radv.c index eb15f9cb..3d5fe5a3 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -287,7 +287,7 @@ radv_iface_new(struct radv_proto *p, struct iface *iface, struct radv_iface_conf RADV_TRACE(D_EVENTS, "Adding interface %s", iface->name); - pool *pool = rp_new(p->p.pool, iface->name); + pool *pool = rp_new(p->p.pool, proto_domain(&p->p), iface->name); ifa = mb_allocz(pool, sizeof(struct radv_iface)); ifa->pool = pool; ifa->ra = p; diff --git a/proto/rpki/rpki.c b/proto/rpki/rpki.c index e5638aff..0fd686b3 100644 --- a/proto/rpki/rpki.c +++ b/proto/rpki/rpki.c @@ -599,7 +599,7 @@ rpki_check_expire_interval(uint seconds) static struct rpki_cache * rpki_init_cache(struct rpki_proto *p, struct rpki_config *cf) { - pool *pool = rp_new(p->p.pool, cf->hostname); + pool *pool = rp_new(p->p.pool, proto_domain(&p->p), cf->hostname); struct rpki_cache *cache = mb_allocz(pool, sizeof(struct rpki_cache)); diff --git a/sysdep/unix/io-loop.c b/sysdep/unix/io-loop.c index 701eddf7..8e95a698 100644 --- a/sysdep/unix/io-loop.c +++ b/sysdep/unix/io-loop.c @@ -503,12 +503,14 @@ sockets_fire(struct birdloop *loop) */ DEFINE_DOMAIN(resource); +static void bird_thread_start_event(void *_data); struct birdloop_pickup_group { DOMAIN(resource) domain; list loops; list threads; btime max_latency; + event start_threads; } pickup_groups[2] = { { /* all zeroes */ @@ -516,6 +518,8 @@ struct birdloop_pickup_group { { /* FIXME: make this dynamic, now it copies the loop_max_latency value from proto/bfd/config.Y */ .max_latency = 10 MS, + .start_threads.hook = bird_thread_start_event, + .start_threads.data = &pickup_groups[1], }, }; @@ -630,13 +634,11 @@ bird_thread_main(void *arg) rcu_thread_start(&thr->rcu); synchronize_rcu(); - tmp_init(thr->pool); - init_list(&thr->loops); - - thr->meta = birdloop_new_no_pickup(thr->pool, DOMAIN_ORDER(meta), "Thread Meta"); - thr->meta->thread = thr; birdloop_enter(thr->meta); + tmp_init(thr->pool, birdloop_domain(thr->meta)); + init_list(&thr->loops); + thr->sock_changed = 1; struct pfd pfd; @@ -759,15 +761,20 @@ static struct bird_thread * bird_thread_start(struct birdloop_pickup_group *group) { ASSERT_DIE(birdloop_inside(&main_birdloop)); - ASSERT_DIE(DOMAIN_IS_LOCKED(resource, group->domain)); - pool *p = rp_new(&root_pool, "Thread"); + struct birdloop *meta = birdloop_new_no_pickup(&root_pool, DOMAIN_ORDER(meta), "Thread Meta"); + pool *p = birdloop_pool(meta); + + birdloop_enter(meta); + LOCK_DOMAIN(resource, group->domain); struct bird_thread *thr = mb_allocz(p, sizeof(*thr)); thr->pool = p; thr->cleanup_event = (event) { .hook = bird_thread_cleanup, .data = thr, }; thr->group = group; thr->max_latency_ns = (group->max_latency ?: 5 S) TO_NS; + thr->meta = meta; + thr->meta->thread = thr; wakeup_init(thr); ev_init_list(&thr->priority_events, NULL, "Thread direct event list"); @@ -790,9 +797,18 @@ bird_thread_start(struct birdloop_pickup_group *group) if (e = pthread_create(&thr->thread_id, &thr->thread_attr, bird_thread_main, thr)) die("pthread_create() failed: %M", e); + UNLOCK_DOMAIN(resource, group->domain); + birdloop_leave(meta); return thr; } +static void +bird_thread_start_event(void *_data) +{ + struct birdloop_pickup_group *group = _data; + bird_thread_start(group); +} + static struct birdloop *thread_dropper; static event *thread_dropper_event; static uint thread_dropper_goal; @@ -880,15 +896,14 @@ bird_thread_commit(struct config *new, struct config *old UNUSED) int dif = list_length(&group->threads) - (thread_dropper_goal = new->thread_count); _Bool thread_dropper_running = !!thread_dropper; + UNLOCK_DOMAIN(resource, group->domain); + if (dif < 0) { bird_thread_start(group); - UNLOCK_DOMAIN(resource, group->domain); continue; } - UNLOCK_DOMAIN(resource, group->domain); - if ((dif > 0) && !thread_dropper_running) { struct birdloop *tdl = birdloop_new(&root_pool, DOMAIN_ORDER(control), group->max_latency, "Thread dropper"); @@ -1006,12 +1021,13 @@ bird_thread_show(void *data) void cmd_show_threads(int show_loops) { - pool *p = rp_new(&root_pool, "Show Threads"); + DOMAIN(control) lock = DOMAIN_NEW(control, "Show Threads"); + pool *p = rp_new(&root_pool, lock.control, "Show Threads"); struct bird_thread_show_data *tsd = mb_allocz(p, sizeof(struct bird_thread_show_data)); - tsd->lock = DOMAIN_NEW(control, "Show Threads"); tsd->cli = this_cli; tsd->pool = p; + tsd->lock = lock; tsd->show_loops = show_loops; this_cli->cont = bird_thread_show_cli_cont; @@ -1112,8 +1128,10 @@ birdloop_stop_internal(struct birdloop *loop) /* Request local socket reload */ this_thread->sock_changed++; - /* Tail-call the stopped hook */ - loop->stopped(loop->stop_data); + /* Call the stopped hook from the main loop */ + loop->event.hook = loop->stopped; + loop->event.data = loop->stop_data; + ev_send_loop(&main_birdloop, &loop->event); } static void @@ -1187,8 +1205,9 @@ 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); + DG_LOCK(dg); - pool *p = rp_vnewf(pp, name, args); + pool *p = rp_vnewf(pp, dg, name, args); struct birdloop *loop = mb_allocz(p, sizeof(struct birdloop)); loop->pool = p; @@ -1197,7 +1216,7 @@ birdloop_vnew_internal(pool *pp, uint order, struct birdloop_pickup_group *group atomic_store_explicit(&loop->thread_transition, 0, memory_order_relaxed); - birdloop_enter(loop); + birdloop_enter_locked(loop); ev_init_list(&loop->event_list, loop, name); timers_init(&loop->time, p); @@ -1211,8 +1230,7 @@ birdloop_vnew_internal(pool *pp, uint order, struct birdloop_pickup_group *group LOCK_DOMAIN(resource, group->domain); add_tail(&group->loops, &loop->n); if (EMPTY_LIST(group->threads)) - bird_thread_start(group); - + ev_send(&global_event_list, &group->start_threads); wakeup_do_kick(SKIP_BACK(struct bird_thread, n, HEAD(group->threads))); UNLOCK_DOMAIN(resource, group->domain); } @@ -1277,8 +1295,11 @@ birdloop_free(struct birdloop *loop) { ASSERT_DIE(loop->thread == NULL); - domain_free(loop->time.domain); + struct domain_generic *dg = loop->time.domain; + DG_LOCK(dg); rp_free(loop->pool); + DG_UNLOCK(dg); + domain_free(dg); } static void diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index 9e6ddb45..6e94b0b6 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -74,7 +74,7 @@ static list krt_proto_list; void krt_io_init(void) { - krt_pool = rp_new(&root_pool, "Kernel Syncer"); + krt_pool = rp_new(&root_pool, the_bird_domain.the_bird, "Kernel Syncer"); krt_filter_lp = lp_new_default(krt_pool); init_list(&krt_proto_list); krt_sys_io_init(); -- cgit v1.2.3