From f8e273b5e7a3c721f4a30cf27a0b4fe54602e83f Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 14 Jun 2021 16:30:59 +0200 Subject: Nest: Fix export of tmpattrs through pipes In most cases of export there is no need to store back temporary attributes to rte, as receivers (protocols) access eattr list anyway. But pipe copies the original rte with old values, so we should store tmpattrs also during export. Thanks to Paul Donohue for the bugreport. --- nest/rt-table.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nest/rt-table.c b/nest/rt-table.c index 13209dd7..a7e31d85 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -618,6 +618,9 @@ export_filter_(struct channel *c, rte *rt0, rte **rt_free, linpool *pool, int si goto reject; } + /* Needed for pipes */ + rte_store_tmp_attrs(rt, pool, NULL); + accept: if (rt != rt0) *rt_free = rt; -- cgit v1.2.3 From 3ebabab2778d05212cc07ebccf583159d5e0890a Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 14 Jun 2021 17:58:37 +0200 Subject: Revert "Nest: Fix export of tmpattrs through pipes" This reverts commit f8e273b5e7a3c721f4a30cf27a0b4fe54602e83f. --- nest/rt-table.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/nest/rt-table.c b/nest/rt-table.c index a7e31d85..13209dd7 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -618,9 +618,6 @@ export_filter_(struct channel *c, rte *rt0, rte **rt_free, linpool *pool, int si goto reject; } - /* Needed for pipes */ - rte_store_tmp_attrs(rt, pool, NULL); - accept: if (rt != rt0) *rt_free = rt; -- cgit v1.2.3 From 1b9bf4e192a252db861acadc7f800d7046435a3f Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 14 Jun 2021 20:02:50 +0200 Subject: Nest: Fix export of tmpattrs through pipes Pipes copy the original rte with old values, so they require rte to be exported with stored tmpattrs. Other protocols access stored attributes using eattr list, so they require rte to be exported with expanded tmpattrs. This is temporary hack, we plan to remove whoe tmpattr mechanism. Thanks to Paul Donohue for the bugreport. --- nest/rt-table.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nest/rt-table.c b/nest/rt-table.c index 13209dd7..390b3277 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -618,6 +618,12 @@ export_filter_(struct channel *c, rte *rt0, rte **rt_free, linpool *pool, int si goto reject; } +#ifdef CONFIG_PIPE + /* Pipes need rte with stored tmpattrs, remaining protocols need expanded tmpattrs */ + if (p->proto == &proto_pipe) + rte_store_tmp_attrs(rt, pool, NULL); +#endif + accept: if (rt != rt0) *rt_free = rt; -- cgit v1.2.3 From f761be6b30633054a54369eee7d08b951a366e5e Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Thu, 17 Jun 2021 16:56:51 +0200 Subject: Nest: Clean up main channel handling Remove assumption that main channel is the only channel. --- nest/protocol.h | 2 +- proto/ospf/config.Y | 5 ++--- proto/radv/config.Y | 1 + proto/radv/radv.c | 2 +- proto/rip/rip.c | 2 +- proto/rpki/rpki.c | 2 +- proto/static/static.c | 2 +- sysdep/unix/krt.c | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nest/protocol.h b/nest/protocol.h index 48eb01d2..abcc505d 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -616,7 +616,7 @@ struct channel { struct channel_config *proto_cf_find_channel(struct proto_config *p, uint net_type); static inline struct channel_config *proto_cf_main_channel(struct proto_config *pc) -{ struct channel_config *cc = HEAD(pc->channels); return NODE_VALID(cc) ? cc : NULL; } +{ return proto_cf_find_channel(pc, pc->net_type); } struct channel *proto_find_channel_by_table(struct proto *p, struct rtable *t); struct channel *proto_find_channel_by_name(struct proto *p, const char *n); diff --git a/proto/ospf/config.Y b/proto/ospf/config.Y index fd2cfe8a..4b7d5a36 100644 --- a/proto/ospf/config.Y +++ b/proto/ospf/config.Y @@ -85,7 +85,7 @@ ospf_proto_finish(void) struct ospf_iface_patt *ic; /* Define default channel */ - if (EMPTY_LIST(this_proto->channels)) + if (! proto_cf_main_channel(this_proto)) { uint net_type = this_proto->net_type = ospf_cfg_is_v2() ? NET_IP4 : NET_IP6; channel_config_new(NULL, net_label[net_type], net_type, this_proto); @@ -248,8 +248,7 @@ ospf_channel_start: net_type ospf_af_mc $$ = this_channel = channel_config_get(NULL, net_label[$1], $1, this_proto); /* Save the multicast flag */ - if (this_channel == proto_cf_main_channel(this_proto)) - OSPF_CFG->af_mc = $2; + OSPF_CFG->af_mc = $2; }; ospf_channel: ospf_channel_start channel_opt_list channel_end; diff --git a/proto/radv/config.Y b/proto/radv/config.Y index dda9cfcd..8d4a3ab9 100644 --- a/proto/radv/config.Y +++ b/proto/radv/config.Y @@ -46,6 +46,7 @@ proto: radv_proto ; radv_proto_start: proto_start RADV { this_proto = proto_config_new(&proto_radv, $1); + this_proto->net_type = NET_IP6; init_list(&RADV_CFG->patt_list); init_list(&RADV_CFG->pref_list); diff --git a/proto/radv/radv.c b/proto/radv/radv.c index b4235917..66e8eb4b 100644 --- a/proto/radv/radv.c +++ b/proto/radv/radv.c @@ -564,7 +564,7 @@ radv_postconfig(struct proto_config *CF) // struct radv_config *cf = (void *) CF; /* Define default channel */ - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) channel_config_new(NULL, net_label[NET_IP6], NET_IP6, CF); } diff --git a/proto/rip/rip.c b/proto/rip/rip.c index 8b4719f7..e1a235a0 100644 --- a/proto/rip/rip.c +++ b/proto/rip/rip.c @@ -1105,7 +1105,7 @@ rip_postconfig(struct proto_config *CF) // struct rip_config *cf = (void *) CF; /* Define default channel */ - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) channel_config_new(NULL, net_label[CF->net_type], CF->net_type, CF); } diff --git a/proto/rpki/rpki.c b/proto/rpki/rpki.c index 799cb877..ab0837f3 100644 --- a/proto/rpki/rpki.c +++ b/proto/rpki/rpki.c @@ -923,7 +923,7 @@ rpki_postconfig(struct proto_config *CF) { /* Define default channel */ if (EMPTY_LIST(CF->channels)) - channel_config_new(NULL, net_label[CF->net_type], CF->net_type, CF); + cf_error("Channel not specified"); } static void diff --git a/proto/static/static.c b/proto/static/static.c index 661f1aac..2789c1bb 100644 --- a/proto/static/static.c +++ b/proto/static/static.c @@ -434,7 +434,7 @@ static_postconfig(struct proto_config *CF) struct static_config *cf = (void *) CF; struct static_route *r; - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) cf_error("Channel not specified"); struct channel_config *cc = proto_cf_main_channel(CF); diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c index ceb88563..7c2614b1 100644 --- a/sysdep/unix/krt.c +++ b/sysdep/unix/krt.c @@ -1013,7 +1013,7 @@ krt_postconfig(struct proto_config *CF) if (cf->c.class == SYM_TEMPLATE) return; - if (EMPTY_LIST(CF->channels)) + if (! proto_cf_main_channel(CF)) cf_error("Channel not specified"); #ifdef CONFIG_ALL_TABLES_AT_ONCE -- cgit v1.2.3 From 9f24fef5e91fb4df301242ede91ee7ac1b46b8a8 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Wed, 20 Oct 2021 01:51:28 +0200 Subject: Conf: Fix crash during shutdown BIRD implements shutdown by reconfiguring to fake empty configuration. Such fake config structure is created from the last running config and shares some data, including symbol table. This allows access to (removed) routing tables and causes crash when 'show route' command is used during shutdown. Clean up symbol table, table list and links to default tables, so removed routing tables cannot be accessed during shutdown. --- conf/conf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/conf/conf.c b/conf/conf.c index 58abcde1..a2b01667 100644 --- a/conf/conf.c +++ b/conf/conf.c @@ -520,6 +520,9 @@ order_shutdown(int gr) memcpy(c, config, sizeof(struct config)); init_list(&c->protos); init_list(&c->tables); + init_list(&c->symbols); + memset(c->def_tables, 0, sizeof(c->def_tables)); + HASH_INIT(c->sym_hash, c->pool, 4); c->shutdown = 1; c->gr_down = gr; -- cgit v1.2.3 From 644e9ca94e2d10ba0c2de45f94523da2414328e3 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 24 Nov 2021 17:30:13 +0100 Subject: Directly mapped pages are kept for future use if temporarily not needed --- lib/resource.h | 1 + nest/cmds.c | 7 ++++++- sysdep/unix/alloc.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/lib/resource.h b/lib/resource.h index e65455c8..76e3745f 100644 --- a/lib/resource.h +++ b/lib/resource.h @@ -98,6 +98,7 @@ void buffer_realloc(void **buf, unsigned *size, unsigned need, unsigned item_siz u64 get_page_size(void); void *alloc_page(void); void free_page(void *); +extern uint pages_kept; #ifdef HAVE_LIBDMALLOC /* diff --git a/nest/cmds.c b/nest/cmds.c index 18f39eb5..f58923a7 100644 --- a/nest/cmds.c +++ b/nest/cmds.c @@ -91,7 +91,12 @@ cmd_show_memory(void) print_size("Routing tables:", rmemsize(rt_table_pool)); print_size("Route attributes:", rmemsize(rta_pool)); print_size("Protocols:", rmemsize(proto_pool)); - print_size("Total:", rmemsize(&root_pool)); + size_t total = rmemsize(&root_pool); +#ifdef HAVE_MMAP + print_size("Standby memory:", get_page_size() * pages_kept); + total += get_page_size() * pages_kept; +#endif + print_size("Total:", total); cli_msg(0, ""); } diff --git a/sysdep/unix/alloc.c b/sysdep/unix/alloc.c index c525f713..5dd70c99 100644 --- a/sysdep/unix/alloc.c +++ b/sysdep/unix/alloc.c @@ -8,6 +8,8 @@ #include "nest/bird.h" #include "lib/resource.h" +#include "lib/lists.h" +#include "lib/event.h" #include #include @@ -17,8 +19,17 @@ #endif #ifdef HAVE_MMAP +#define KEEP_PAGES 512 + static u64 page_size = 0; static _Bool use_fake = 0; + +uint pages_kept = 0; +static list pages_list; + +static void cleanup_pages(void *data); +static event page_cleanup_event = { .hook = cleanup_pages }; + #else static const u64 page_size = 4096; /* Fake page size */ #endif @@ -48,6 +59,15 @@ void * alloc_page(void) { #ifdef HAVE_MMAP + if (pages_kept) + { + node *page = TAIL(pages_list); + rem_node(page); + pages_kept--; + memset(page, 0, get_page_size()); + return page; + } + if (!use_fake) { void *ret = mmap(NULL, get_page_size(), PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); @@ -71,10 +91,39 @@ free_page(void *ptr) #ifdef HAVE_MMAP if (!use_fake) { - if (munmap(ptr, get_page_size()) < 0) - bug("munmap(%p) failed: %m", ptr); + if (!pages_kept) + init_list(&pages_list); + + memset(ptr, 0, sizeof(node)); + add_tail(&pages_list, ptr); + + if (++pages_kept > KEEP_PAGES) + ev_schedule(&page_cleanup_event); } else #endif free(ptr); } + +#ifdef HAVE_MMAP +static void +cleanup_pages(void *data UNUSED) +{ + for (uint seen = 0; (pages_kept > KEEP_PAGES) && (seen < KEEP_PAGES); seen++) + { + void *ptr = HEAD(pages_list); + rem_node(ptr); + if (munmap(ptr, get_page_size()) == 0) + pages_kept--; +#ifdef ENOMEM + else if (errno == ENOMEM) + add_tail(&pages_list, ptr); +#endif + else + bug("munmap(%p) failed: %m", ptr); + } + + if (pages_kept > KEEP_PAGES) + ev_schedule(&page_cleanup_event); +} +#endif -- cgit v1.2.3 From f772afc525156498900770ffe5a98349df89a45c Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Sat, 27 Nov 2021 00:21:12 +0100 Subject: Memory statistics split into Effective and Overhead This feature is intended mostly for checking that BIRD's allocation strategies don't consume much memory space. There are some cases where withdrawing routes in a specific order lead to memory fragmentation and this output should give the user at least a notion of how much memory is actually used for data storage and how much memory is "just allocated" or used for overhead. Also raising the "system allocator overhead estimation" from 8 to 16 bytes; it is probably even more. I've found 16 as a local minimum in best scenarios among reachable machines. I couldn't find any reasonable method to estimate this value when BIRD starts up. This commit also fixes the inaccurate computation of memory overhead for slabs where the "system allocater overhead estimation" was improperly added to the size of mmap-ed memory. --- lib/mempool.c | 12 +++++++----- lib/resource.c | 32 +++++++++++++++++++++++--------- lib/resource.h | 12 +++++++++--- lib/slab.c | 29 ++++++++++++++++++++++------- nest/cmds.c | 44 +++++++++++++++++++++++++++++++++++--------- 5 files changed, 96 insertions(+), 33 deletions(-) diff --git a/lib/mempool.c b/lib/mempool.c index 758882ce..90d7c774 100644 --- a/lib/mempool.c +++ b/lib/mempool.c @@ -45,7 +45,7 @@ struct linpool { static void lp_free(resource *); static void lp_dump(resource *); static resource *lp_lookup(resource *, unsigned long); -static size_t lp_memsize(resource *r); +static struct resmem lp_memsize(resource *r); static struct resclass lp_class = { "LinPool", @@ -287,7 +287,7 @@ lp_dump(resource *r) m->total_large); } -static size_t +static struct resmem lp_memsize(resource *r) { linpool *m = (linpool *) r; @@ -299,9 +299,11 @@ lp_memsize(resource *r) for(c=m->first_large; c; c=c->next) cnt++; - return ALLOC_OVERHEAD + sizeof(struct linpool) + - cnt * (ALLOC_OVERHEAD + sizeof(struct lp_chunk)) + - m->total + m->total_large; + return (struct resmem) { + .effective = m->total + m->total_large, + .overhead = ALLOC_OVERHEAD + sizeof(struct linpool) + + cnt * (ALLOC_OVERHEAD + sizeof(struct lp_chunk)), + }; } diff --git a/lib/resource.c b/lib/resource.c index 4c4b92ec..5d4c7780 100644 --- a/lib/resource.c +++ b/lib/resource.c @@ -2,6 +2,7 @@ * BIRD Resource Manager * * (c) 1998--2000 Martin Mares + * (c) 2021 Maria Matejka * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -37,7 +38,7 @@ struct pool { static void pool_dump(resource *); static void pool_free(resource *); static resource *pool_lookup(resource *, unsigned long); -static size_t pool_memsize(resource *P); +static struct resmem pool_memsize(resource *P); static struct resclass pool_class = { "Pool", @@ -97,15 +98,22 @@ pool_dump(resource *P) indent -= 3; } -static size_t +static struct resmem pool_memsize(resource *P) { pool *p = (pool *) P; resource *r; - size_t sum = sizeof(pool) + ALLOC_OVERHEAD; + struct resmem sum = { + .effective = 0, + .overhead = sizeof(pool) + ALLOC_OVERHEAD, + }; WALK_LIST(r, p->inside) - sum += rmemsize(r); + { + struct resmem add = rmemsize(r); + sum.effective += add.effective; + sum.overhead += add.overhead; + } return sum; } @@ -193,14 +201,17 @@ rdump(void *res) debug("NULL\n"); } -size_t +struct resmem rmemsize(void *res) { resource *r = res; if (!r) - return 0; + return (struct resmem) {}; if (!r->class->memsize) - return r->class->size + ALLOC_OVERHEAD; + return (struct resmem) { + .effective = r->class->size - sizeof(resource), + .overhead = ALLOC_OVERHEAD + sizeof(resource), + }; return r->class->memsize(r); } @@ -305,11 +316,14 @@ mbl_lookup(resource *r, unsigned long a) return NULL; } -static size_t +static struct resmem mbl_memsize(resource *r) { struct mblock *m = (struct mblock *) r; - return ALLOC_OVERHEAD + sizeof(struct mblock) + m->size; + return (struct resmem) { + .effective = m->size, + .overhead = ALLOC_OVERHEAD + sizeof(struct mblock), + }; } static struct resclass mb_class = { diff --git a/lib/resource.h b/lib/resource.h index 76e3745f..9ec41ed8 100644 --- a/lib/resource.h +++ b/lib/resource.h @@ -2,6 +2,7 @@ * BIRD Resource Manager * * (c) 1998--1999 Martin Mares + * (c) 2021 Maria Matejka * * Can be freely distributed and used under the terms of the GNU GPL. */ @@ -11,6 +12,11 @@ #include "lib/lists.h" +struct resmem { + size_t effective; /* Memory actually used for data storage */ + size_t overhead; /* Overhead memory imposed by allocator strategies */ +}; + /* Resource */ typedef struct resource { @@ -26,11 +32,11 @@ struct resclass { void (*free)(resource *); /* Freeing function */ void (*dump)(resource *); /* Dump to debug output */ resource *(*lookup)(resource *, unsigned long); /* Look up address (only for debugging) */ - size_t (*memsize)(resource *); /* Return size of memory used by the resource, may be NULL */ + struct resmem (*memsize)(resource *); /* Return size of memory used by the resource, may be NULL */ }; /* Estimate of system allocator overhead per item, for memory consumtion stats */ -#define ALLOC_OVERHEAD 8 +#define ALLOC_OVERHEAD 16 /* Generic resource manipulation */ @@ -40,7 +46,7 @@ void resource_init(void); pool *rp_new(pool *, const char *); /* Create new pool */ void rfree(void *); /* Free single resource */ void rdump(void *); /* Dump to debug output */ -size_t rmemsize(void *res); /* Return size of memory used by the resource */ +struct resmem rmemsize(void *res); /* Return size of memory used by the resource */ void rlookup(unsigned long); /* Look up address (only for debugging) */ void rmove(void *, pool *); /* Move to a different pool */ diff --git a/lib/slab.c b/lib/slab.c index b0a01ae7..6cab6b7b 100644 --- a/lib/slab.c +++ b/lib/slab.c @@ -42,7 +42,7 @@ static void slab_free(resource *r); static void slab_dump(resource *r); static resource *slab_lookup(resource *r, unsigned long addr); -static size_t slab_memsize(resource *r); +static struct resmem slab_memsize(resource *r); #ifdef FAKE_SLAB @@ -128,7 +128,7 @@ slab_dump(resource *r) debug("(%d objects per %d bytes)\n", cnt, s->size); } -static size_t +static struct resmem slab_memsize(resource *r) { slab *s = (slab *) r; @@ -138,7 +138,10 @@ slab_memsize(resource *r) WALK_LIST(o, s->objs) cnt++; - return ALLOC_OVERHEAD + sizeof(struct slab) + cnt * (ALLOC_OVERHEAD + s->size); + return (struct resmem) { + .effective = cnt * s->size, + .overhead = ALLOC_OVERHEAD + sizeof(struct slab) + cnt * ALLOC_OVERHEAD, + }; } @@ -363,21 +366,33 @@ slab_dump(resource *r) debug("(%de+%dp+%df blocks per %d objs per %d bytes)\n", ec, pc, fc, s->objs_per_slab, s->obj_size); } -static size_t +static struct resmem slab_memsize(resource *r) { slab *s = (slab *) r; size_t heads = 0; struct sl_head *h; - WALK_LIST(h, s->empty_heads) + WALK_LIST(h, s->full_heads) heads++; + + size_t items = heads * s->objs_per_slab; + WALK_LIST(h, s->partial_heads) + { heads++; - WALK_LIST(h, s->full_heads) + items += h->num_full; + } + + WALK_LIST(h, s->empty_heads) heads++; - return ALLOC_OVERHEAD + sizeof(struct slab) + heads * (ALLOC_OVERHEAD + get_page_size()); + size_t eff = items * s->obj_size; + + return (struct resmem) { + .effective = eff, + .overhead = ALLOC_OVERHEAD + sizeof(struct slab) + heads * get_page_size() - eff, + }; } static resource * diff --git a/nest/cmds.c b/nest/cmds.c index f58923a7..1a16f9c7 100644 --- a/nest/cmds.c +++ b/nest/cmds.c @@ -67,18 +67,43 @@ cmd_show_symbols(struct sym_show_data *sd) } } -static void -print_size(char *dsc, size_t val) +#define SIZE_SUFFIX " kMGT" +#define SIZE_FORMAT "% 4u.%1u % 1cB" +#define SIZE_ARGS(a) (a).val, (a).decimal, SIZE_SUFFIX[(a).magnitude] + +struct size_args { + u64 val:48; + u64 decimal:8; + u64 magnitude:8; +}; + +static struct size_args +get_size_args(u64 val) { - char *px = " kMG"; - int i = 0; - while ((val >= 10000) && (i < 3)) +#define VALDEC 10 /* One decimal place */ + val *= VALDEC; + + uint i = 0; + while ((val >= 10000 * VALDEC) && (i < 4)) { val = (val + 512) / 1024; i++; } - cli_msg(-1018, "%-17s %4u %cB", dsc, (unsigned) val, px[i]); + return (struct size_args) { + .val = (val / VALDEC), + .decimal = (val % VALDEC), + .magnitude = i, + }; +} + +static void +print_size(char *dsc, struct resmem vals) +{ + struct size_args effective = get_size_args(vals.effective); + struct size_args overhead = get_size_args(vals.overhead); + + cli_msg(-1018, "%-17s " SIZE_FORMAT " " SIZE_FORMAT, dsc, SIZE_ARGS(effective), SIZE_ARGS(overhead)); } extern pool *rt_table_pool; @@ -88,13 +113,14 @@ void cmd_show_memory(void) { cli_msg(-1018, "BIRD memory usage"); + cli_msg(-1018, "%-17s Effective Overhead", ""); print_size("Routing tables:", rmemsize(rt_table_pool)); print_size("Route attributes:", rmemsize(rta_pool)); print_size("Protocols:", rmemsize(proto_pool)); - size_t total = rmemsize(&root_pool); + struct resmem total = rmemsize(&root_pool); #ifdef HAVE_MMAP - print_size("Standby memory:", get_page_size() * pages_kept); - total += get_page_size() * pages_kept; + print_size("Standby memory:", (struct resmem) { .overhead = get_page_size() * pages_kept }); + total.overhead += get_page_size() * pages_kept; #endif print_size("Total:", total); cli_msg(0, ""); -- cgit v1.2.3