diff --git a/conf/cf-lex.l b/conf/cf-lex.l index 556def31..a1d62d3a 100644 --- a/conf/cf-lex.l +++ b/conf/cf-lex.l @@ -749,7 +749,8 @@ ea_lex_unregister(struct ea_class *def) struct ea_class * ea_class_find_by_name(const char *name) { - struct symbol *sym = cf_find_symbol_scope(config ? config->root_scope : &global_filter_scope, name); + struct config *c = OBSREF_GET(config); /* FIXME: this may be wrong */ + struct symbol *sym = cf_find_symbol_scope(c ? c->root_scope : &global_filter_scope, name); return sym && (sym->class == SYM_ATTRIBUTE) ? sym->attribute : NULL; } diff --git a/conf/conf.c b/conf/conf.c index 0db7ae47..e20210ed 100644 --- a/conf/conf.c +++ b/conf/conf.c @@ -60,26 +60,47 @@ static jmp_buf conf_jmpbuf; -struct config *config; +config_ref config; _Thread_local struct config *new_config; pool *config_pool; -static struct config *old_config; /* Old configuration */ -static struct config *future_config; /* New config held here if recon requested during recon */ +static config_ref old_config; /* Old configuration */ +static config_ref future_config; /* New config held here if recon requested during recon */ static int old_cftype; /* Type of transition old_config -> config (RECONFIG_SOFT/HARD) */ static int future_cftype; /* Type of scheduled transition, may also be RECONFIG_UNDO */ /* Note that when future_cftype is RECONFIG_UNDO, then future_config is NULL, therefore proper check for future scheduled config checks future_cftype */ -static void config_done(void *cf); +static void config_done(void); static timer *config_timer; /* Timer for scheduled configuration rollback */ /* These are public just for cmd_show_status(), should not be accessed elsewhere */ int shutting_down; /* Shutdown requested, do not accept new config changes */ int configuring; /* Reconfiguration is running */ +struct config *old_config_pending; /* Just for debug convenience */ int undo_available; /* Undo was not requested from last reconfiguration */ /* Note that both shutting_down and undo_available are related to requests, not processing */ +static void +config_obstacles_cleared(struct callback *cb) +{ + SKIP_BACK_DECLARE(struct config, c, obstacles_cleared, cb); + ASSERT_DIE(birdloop_inside(&main_birdloop)); + + if (c == old_config_pending) + { + old_config_pending = NULL; + + if (!shutting_down) + OBSREF_SET(old_config, c); + + if (configuring) + config_done(); + } + else + config_free(c); +} + /** * config_alloc - allocate a new configuration * @name: name of the config @@ -109,7 +130,8 @@ config_alloc(const char *name) c->tf_base = c->tf_log = TM_ISO_LONG_MS; c->gr_wait = DEFAULT_GR_WAIT; - c->done_event = (event) { .hook = config_done, .data = c, }; + callback_init(&c->obstacles_cleared, config_obstacles_cleared, &main_birdloop); + obstacle_target_init(&c->obstacles, &c->obstacles_cleared, p, "Config"); return c; } @@ -196,7 +218,8 @@ config_free(struct config *c) if (!c) return; - ASSERT(!atomic_load_explicit(&c->obstacle_count, memory_order_relaxed)); + synchronize_rcu(); + ASSERT_DIE(!obstacle_target_count(&c->obstacles)); rp_free(c->pool); } @@ -212,30 +235,9 @@ config_free(struct config *c) void config_free_old(void) { - if (!old_config || atomic_load_explicit(&old_config->obstacle_count, memory_order_acquire)) - return; - + OBSREF_CLEAR(old_config); tm_stop(config_timer); undo_available = 0; - - config_free(old_config); - old_config = NULL; -} - -void -config_add_obstacle(struct config *c) -{ - UNUSED int obs = atomic_fetch_add_explicit(&c->obstacle_count, 1, memory_order_acq_rel); - DBG("+++ adding obstacle %d\n", obs); -} - -void -config_del_obstacle(struct config *c) -{ - int obs = atomic_fetch_sub_explicit(&c->obstacle_count, 1, memory_order_acq_rel); - DBG("+++ deleting obstacle %d\n", obs); - if (obs == 1) - ev_send_loop(&main_birdloop, &c->done_event); } struct global_runtime global_runtime_internal[2] = {{ @@ -296,19 +298,24 @@ global_commit(struct config *new, struct config *old) } static int -config_do_commit(struct config *c, int type) +config_do_commit(config_ref *cr, int type) { if (type == RECONFIG_UNDO) { - c = old_config; + OBSREF_SET(*cr, OBSREF_GET(old_config)); type = old_cftype; } - else - config_free(old_config); - old_config = config; + OBSREF_CLEAR(old_config); + old_cftype = type; - config = c; + + struct config *c = OBSREF_GET(*cr); + struct config *oc = OBSREF_GET(config); + old_config_pending = oc; + + OBSREF_CLEAR(config); + OBSREF_SET(config, OBSREF_GET(*cr)); if (!c->hostname) { @@ -319,54 +326,50 @@ config_do_commit(struct config *c, int type) } configuring = 1; - if (old_config && !config->shutdown) + if (oc && !c->shutdown) log(L_INFO "Reconfiguring"); - if (old_config) - config_add_obstacle(old_config); - DBG("filter_commit\n"); - filter_commit(c, old_config); + filter_commit(c, oc); DBG("sysdep_commit\n"); - sysdep_commit(c, old_config); + sysdep_commit(c, oc); DBG("global_commit\n"); - global_commit(c, old_config); - mpls_commit(c, old_config); + global_commit(c, oc); + mpls_commit(c, oc); DBG("rt_commit\n"); - rt_commit(c, old_config); + rt_commit(c, oc); DBG("protos_commit\n"); - protos_commit(c, old_config, type); - int obs = old_config ? - atomic_fetch_sub_explicit(&old_config->obstacle_count, 1, memory_order_acq_rel) - 1 - : 0; + protos_commit(c, oc, type); - DBG("do_commit finished with %d obstacles remaining\n", obs); - return !obs; + /* Can be cleared directly? */ + return !oc || callback_is_active(&oc->obstacles_cleared); } static void -config_done(void *cf) +config_done(void) { - if (cf == config) - return; + struct config *c = OBSREF_GET(config); + ASSERT_DIE(c); - if (config->shutdown) + if (c->shutdown) sysdep_shutdown_done(); configuring = 0; - if (old_config) + if (OBSREF_GET(old_config)) log(L_INFO "Reconfigured"); if (future_cftype) { int type = future_cftype; - struct config *conf = future_config; + + CONFIG_REF_LOCAL(conf, OBSREF_GET(future_config)); + future_cftype = RECONFIG_NONE; - future_config = NULL; + OBSREF_CLEAR(future_config); log(L_INFO "Reconfiguring to queued configuration"); - if (config_do_commit(conf, type)) - config_done(NULL); + if (config_do_commit(&conf, type)) + config_done(); } } @@ -398,11 +401,11 @@ config_done(void *cf) * are accepted. */ int -config_commit(struct config *c, int type, uint timeout) +config_commit(config_ref *cr, int type, uint timeout) { if (shutting_down) { - config_free(c); + OBSREF_CLEAR(*cr); return CONF_SHUTDOWN; } @@ -417,19 +420,19 @@ config_commit(struct config *c, int type, uint timeout) if (future_cftype) { log(L_INFO "Queueing new configuration, ignoring the one already queued"); - config_free(future_config); + OBSREF_CLEAR(future_config); } else log(L_INFO "Queueing new configuration"); future_cftype = type; - future_config = c; + OBSREF_SET(future_config, OBSREF_GET(*cr)); return CONF_QUEUED; } - if (config_do_commit(c, type)) + if (config_do_commit(cr, type)) { - config_done(NULL); + config_done(); return CONF_DONE; } return CONF_PROGRESS; @@ -483,7 +486,7 @@ config_undo(void) if (shutting_down) return CONF_SHUTDOWN; - if (!undo_available || !old_config) + if (!undo_available || !OBSREF_GET(old_config)) return CONF_NOTHING; undo_available = 0; @@ -493,8 +496,7 @@ config_undo(void) { if (future_cftype) { - config_free(future_config); - future_config = NULL; + OBSREF_CLEAR(future_config); log(L_INFO "Removing queued configuration"); future_cftype = RECONFIG_NONE; @@ -508,9 +510,11 @@ config_undo(void) } } - if (config_do_commit(NULL, RECONFIG_UNDO)) + CONFIG_REF_LOCAL_EMPTY(undo_conf); + if (config_do_commit(&undo_conf, RECONFIG_UNDO)) { - config_done(NULL); + OBSREF_CLEAR(undo_conf); + config_done(); return CONF_DONE; } return CONF_PROGRESS; @@ -565,8 +569,6 @@ config_init(void) void order_shutdown(int gr) { - struct config *c; - if (shutting_down) return; @@ -575,17 +577,19 @@ order_shutdown(int gr) else log(L_INFO "Shutting down for graceful restart"); - c = lp_alloc(config->mem, sizeof(struct config)); - memcpy(c, config, sizeof(struct config)); + struct config *c = lp_alloc(OBSREF_GET(config)->mem, sizeof(struct config)); + memcpy(c, OBSREF_GET(config), sizeof(struct config)); init_list(&c->protos); init_list(&c->tables); init_list(&c->mpls_domains); init_list(&c->symbols); + obstacle_target_init(&c->obstacles, &c->obstacles_cleared, c->pool, "Config"); memset(c->def_tables, 0, sizeof(c->def_tables)); c->shutdown = 1; c->gr_down = gr; - config_commit(c, RECONFIG_HARD, 0); + CONFIG_REF_LOCAL(cr, c); + config_commit(&cr, RECONFIG_HARD, 0); shutting_down = 1; } diff --git a/conf/conf.h b/conf/conf.h index 1ce5468c..7ed22afc 100644 --- a/conf/conf.h +++ b/conf/conf.h @@ -13,6 +13,7 @@ #include "lib/ip.h" #include "lib/hash.h" #include "lib/resource.h" +#include "lib/obstacle.h" #include "lib/timer.h" /* Configuration structure */ @@ -68,8 +69,8 @@ struct config { struct sym_scope *root_scope; /* Scope for root symbols */ struct sym_scope *current_scope; /* Current scope where we are actually in while parsing */ int allow_attributes; /* Allow attributes in the current state of configuration parsing */ - _Atomic int obstacle_count; /* Number of items blocking freeing of this config */ - event done_event; /* Called when obstacle_count reaches zero */ + struct obstacle_target obstacles; /* May be externally blocked */ + struct callback obstacles_cleared; /* Called when ready to delete */ int shutdown; /* This is a pseudo-config for daemon shutdown */ int gr_down; /* This is a pseudo-config for graceful restart */ }; @@ -96,7 +97,17 @@ struct global_runtime { extern struct global_runtime * _Atomic global_runtime; /* Please don't use these variables in protocols. Use proto_config->global instead. */ -extern struct config *config; /* Currently active configuration */ +typedef OBSREF(struct config) config_ref; + +/* Self-clearing local reference */ +static inline void config_ref_local_cleanup(config_ref *r) +{ OBSREF_CLEAR(*r); } +#define CONFIG_REF_LOCAL_EMPTY(_ref) \ + CLEANUP(config_ref_local_cleanup) config_ref _ref = {}; +#define CONFIG_REF_LOCAL(_ref, _val) \ + CONFIG_REF_LOCAL_EMPTY(_ref); OBSREF_SET(_ref, _val); + +extern config_ref config; /* Currently active configuration */ extern _Thread_local struct config *new_config; /* Configuration being parsed */ struct config *config_alloc(const char *name); @@ -104,7 +115,7 @@ int config_parse(struct config *); int cli_parse(struct config *, struct config *); void config_free(struct config *); void config_free_old(void); -int config_commit(struct config *, int type, uint timeout); +int config_commit(config_ref *, int type, uint timeout); int config_confirm(void); int config_undo(void); int config_status(void); @@ -112,8 +123,6 @@ btime config_timer_status(void); void config_init(void); void cf_error(const char *msg, ...) NORET; #define cf_warn(msg, args...) log(L_WARN "%s:%d:%d: " msg, ifs->file_name, ifs->lino, ifs->chno - ifs->toklen + 1, ##args) -void config_add_obstacle(struct config *); -void config_del_obstacle(struct config *); void order_shutdown(int gr); #define RECONFIG_NONE 0 diff --git a/filter/filter.c b/filter/filter.c index c9a49bac..38f06fdd 100644 --- a/filter/filter.c +++ b/filter/filter.c @@ -413,7 +413,7 @@ void channel_filter_dump(const struct filter *f) void filters_dump_all(void) { struct symbol *sym; - WALK_LIST(sym, config->symbols) { + WALK_LIST(sym, OBSREF_GET(config)->symbols) { switch (sym->class) { case SYM_FILTER: debug("Named filter %s:\n", sym->name); diff --git a/filter/filter_test.c b/filter/filter_test.c index 65b2e4f0..fe977e8d 100644 --- a/filter/filter_test.c +++ b/filter/filter_test.c @@ -31,7 +31,7 @@ t_reconfig(const void *arg) return 0; struct symbol *s; - WALK_LIST(s, config->symbols) + WALK_LIST(s, OBSREF_GET(config)->symbols) if ((s->class == SYM_FUNCTION) || (s->class == SYM_FILTER)) bt_assert_msg((s->flags & SYM_FLAG_SAME), "Symbol %s same check", s->name); @@ -84,7 +84,7 @@ main(int argc, char *argv[]) bt_test_suite_arg_extra(t_reconfig, BT_CONFIG_FILE, 0, BT_TIMEOUT, "Testing reconfiguration to the same file"); struct f_bt_test_suite *t; - WALK_LIST(t, config->tests) + WALK_LIST(t, OBSREF_GET(config)->tests) bt_test_suite_base(run_function, t->fn_name, t, 0, BT_TIMEOUT, "%s", t->dsc); return bt_exit_value(); diff --git a/lib/event_test.c b/lib/event_test.c index f9692142..b9e102e8 100644 --- a/lib/event_test.c +++ b/lib/event_test.c @@ -59,7 +59,7 @@ t_ev_run_list(void) if_init(); // roa_init(); config_init(); - config = config_alloc(""); + OBSREF_SET(config, config_alloc("")); init_event_check_points(); diff --git a/lib/obstacle.h b/lib/obstacle.h new file mode 100644 index 00000000..7d7bcac7 --- /dev/null +++ b/lib/obstacle.h @@ -0,0 +1,95 @@ +/* + * BIRD Library -- Obstacle Keeper + * + * (c) 2024 CZ.NIC, z.s.p.o. + * (c) 2024 Maria Matejka + * + * Can be freely distributed and used under the terms of the GNU GPL. + */ + +#ifndef _BIRD_OBSTACLE_H_ +#define _BIRD_OBSTACLE_H_ + +#include "lib/event.h" +#include "lib/locking.h" +#include "lib/tlists.h" + +#define TLIST_PREFIX obstacle +#define TLIST_TYPE struct obstacle +#define TLIST_ITEM n + +struct obstacle { + TLIST_DEFAULT_NODE; +}; + +#define TLIST_WANT_ADD_TAIL + +#include "lib/tlists.h" + +struct obstacle_target { + DOMAIN(resource) dom; + TLIST_LIST(obstacle) obstacles; + struct callback *done; +}; + +static inline void +obstacle_put(struct obstacle_target *t, struct obstacle *o) +{ + LOCK_DOMAIN(resource, t->dom); + obstacle_add_tail(&t->obstacles, o); + UNLOCK_DOMAIN(resource, t->dom); +} + +static inline void +obstacle_remove(struct obstacle *o) +{ + SKIP_BACK_DECLARE(struct obstacle_target, t, obstacles, obstacle_enlisted(o)); + LOCK_DOMAIN(resource, t->dom); + obstacle_rem_node(&t->obstacles, o); + if (EMPTY_TLIST(obstacle, &t->obstacles)) + callback_activate(t->done); + UNLOCK_DOMAIN(resource, t->dom); +} + +static inline void +obstacle_target_init(struct obstacle_target *t, struct callback *done, pool *p, const char *fmt, ...) +{ + t->dom = DOMAIN_NEW(resource); + va_list args; + va_start(args, fmt); + DOMAIN_SETUP(resource, t->dom, mb_vsprintf(p, fmt, args), p); + va_end(args); + + t->obstacles = (struct obstacle_list) {}; + t->done = done; +} + +static inline uint +obstacle_target_count(struct obstacle_target *t) +{ + LOCK_DOMAIN(resource, t->dom); + uint len = TLIST_LENGTH(obstacle, &t->obstacles); + UNLOCK_DOMAIN(resource, t->dom); + return len; +} + +#define OBSREF(_type) struct { _type *ref; struct obstacle o; } + +#define OBSREF_SET(_ref, _val) ({ \ + typeof (_ref) *_r = &(_ref); \ + typeof (_val) _v = (_val); \ + ASSERT_DIE(_r->ref == NULL); \ + obstacle_put(&_v->obstacles, &_r->o); \ + _r->ref = _v; \ + }) + +#define OBSREF_CLEAR(_ref) ({ \ + typeof (_ref) *_r = &(_ref); \ + if (_r->ref) { \ + obstacle_remove(&_r->o); \ + _r->ref = NULL; \ + }}) + +#define OBSREF_GET(_ref) ((_ref).ref) + +#endif diff --git a/nest/cli.c b/nest/cli.c index c4acb5a7..24535e3f 100644 --- a/nest/cli.c +++ b/nest/cli.c @@ -221,20 +221,22 @@ cli_command(struct cli *c) struct config f; int res; - if (config->cli_debug > 1) + if (OBSREF_GET(config)->cli_debug > 1) log(L_TRACE "CLI: %s", c->rx_buf); bzero(&f, sizeof(f)); f.mem = c->parser_pool; f.pool = rp_new(c->pool, the_bird_domain.the_bird, "Config"); + obstacle_target_init(&f.obstacles, &f.obstacles_cleared, c->pool, "Config"); + init_list(&f.symbols); cf_read_hook = cli_cmd_read_hook; cli_rh_pos = c->rx_buf; cli_rh_len = strlen(c->rx_buf); cli_rh_trick_flag = 0; this_cli = c; - this_cli->main_config = config; + this_cli->main_config = OBSREF_GET(config); lp_flush(c->parser_pool); - res = cli_parse(config, &f); + res = cli_parse(this_cli->main_config, &f); if (!res) cli_printf(c, 9001, f.err_msg); diff --git a/nest/config.Y b/nest/config.Y index 24e96a03..a0a10daf 100644 --- a/nest/config.Y +++ b/nest/config.Y @@ -734,7 +734,7 @@ r_args: $$ = cfg_allocz(sizeof(struct rt_show_data)); init_list(&($$->tables)); $$->filter = FILTER_ACCEPT; - $$->running_on_config = this_cli->main_config; + OBSREF_SET($$->running_on_config, this_cli->main_config); $$->cli = this_cli; $$->tf_route = this_cli->main_config->tf_route; } diff --git a/nest/mpls-internal.h b/nest/mpls-internal.h index 1c6f380c..f69873fa 100644 --- a/nest/mpls-internal.h +++ b/nest/mpls-internal.h @@ -34,7 +34,7 @@ struct mpls_domain { uint label_count; /* Number of allocated labels */ uint use_count; /* Reference counter */ - struct config *removed; /* Deconfigured, waiting for zero use_count, + config_ref removed; /* Deconfigured, waiting for zero use_count, while keeping config obstacle */ struct mpls_range *static_range; /* Direct static range pointer */ diff --git a/nest/mpls.c b/nest/mpls.c index 76ca0d2b..a362ef80 100644 --- a/nest/mpls.c +++ b/nest/mpls.c @@ -309,21 +309,18 @@ mpls_free_domain(void *_m) ASSERT(m->label_count == 0); ASSERT(EMPTY_LIST(m->ranges)); - struct config *cfg = m->removed; + OBSREF_CLEAR(m->removed); m->cf->domain = NULL; rem_node(&m->n); rfree(m->pool); UNLOCK_DOMAIN(attrs, dom); - - config_del_obstacle(cfg); } static void mpls_remove_domain(struct mpls_domain *m, struct config *cfg) { - m->removed = cfg; - config_add_obstacle(cfg); + OBSREF_SET(m->removed, cfg); struct mpls_range *r, *rnext; WALK_LIST_DELSAFE(r, rnext, m->ranges) @@ -345,7 +342,7 @@ mpls_unlock_domain(struct mpls_domain *m) ASSERT(m->use_count > 0); m->use_count--; - if (!m->use_count && m->removed) + if (!m->use_count && OBSREF_GET(m->removed)) ev_new_send(&main_birdloop, m->pool, mpls_free_domain, m); } diff --git a/nest/proto.c b/nest/proto.c index e6613d24..f74e8063 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -1221,6 +1221,7 @@ proto_new(struct proto_config *cf) { struct proto *p = mb_allocz(proto_pool, cf->protocol->proto_size); + OBSREF_SET(p->global_config, cf->global); p->cf = cf; p->debug = cf->debug; p->mrtdump = cf->mrtdump; @@ -1556,6 +1557,8 @@ protos_do_commit(struct config *new, struct config *old, int type) /* We will try to reconfigure protocol p */ if (proto_reconfigure(p, oc, nc, type)) { + OBSREF_CLEAR(p->global_config); + OBSREF_SET(p->global_config, new); PROTO_LEAVE_FROM_MAIN(proto_loop); continue; } @@ -1598,7 +1601,6 @@ protos_do_commit(struct config *new, struct config *old, int type) p->reconfiguring = 1; PROTO_LEAVE_FROM_MAIN(proto_loop); - config_add_obstacle(old); proto_rethink_goal(p); } } @@ -1675,7 +1677,7 @@ proto_rethink_goal(struct proto *p) DBG("%s has shut down for reconfiguration\n", p->name); p->cf->proto = NULL; - config_del_obstacle(p->cf->global); + OBSREF_CLEAR(p->global_config); proto_remove_channels(p); proto_rem_node(&global_proto_list, p); rfree(p->event); diff --git a/nest/protocol.h b/nest/protocol.h index a4279c5a..ad43e9d9 100644 --- a/nest/protocol.h +++ b/nest/protocol.h @@ -133,6 +133,7 @@ struct proto_config { struct proto { TLIST_DEFAULT_NODE; /* Node in global proto_list */ struct protocol *proto; /* Protocol */ + config_ref global_config; /* Global configuration reference */ struct proto_config *cf; /* Configuration data */ struct proto_config *cf_new; /* Configuration we want to switch to after shutdown (NULL=delete) */ pool *pool; /* Pool containing local objects */ diff --git a/nest/route.h b/nest/route.h index 12edf1a5..452fe518 100644 --- a/nest/route.h +++ b/nest/route.h @@ -17,6 +17,7 @@ #include "lib/resource.h" #include "lib/net.h" #include "lib/netindex.h" +#include "lib/obstacle.h" #include "lib/type.h" #include "lib/fib.h" #include "lib/route.h" @@ -27,6 +28,8 @@ #include "filter/data.h" +#include "conf/conf.h" + #include struct ea_list; @@ -382,7 +385,7 @@ struct rtable_private { struct hmap id_map; struct hostcache *hostcache; - struct config *deleted; /* Table doesn't exist in current configuration, + config_ref deleted; /* Table doesn't exist in current configuration, * delete as soon as use_count becomes 0 and remove * obstacle from this routing table. */ @@ -794,7 +797,7 @@ struct rt_show_data { struct proto *show_protocol; struct proto *export_protocol; struct channel *export_channel; - struct config *running_on_config; + OBSREF(struct config) running_on_config; // struct rt_export_hook *kernel_export_hook; int export_mode, addr_mode, primary_only, filtered, stats; diff --git a/nest/rt-show.c b/nest/rt-show.c index ef136371..2bed1afc 100644 --- a/nest/rt-show.c +++ b/nest/rt-show.c @@ -238,6 +238,9 @@ rt_show_cleanup(struct cli *c) if (rt_export_feed_active(&tab->req)) rt_feeder_unsubscribe(&tab->req); } + + /* Unreference the config */ + OBSREF_CLEAR(d->running_on_config); } static void diff --git a/nest/rt-table.c b/nest/rt-table.c index 065af6aa..593f79f9 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2573,7 +2573,7 @@ rt_dump(rtable *tab) RT_READ(tab, tp); /* Looking at priv.deleted is technically unsafe but we don't care */ - debug("Dump of routing table <%s>%s\n", tab->name, tab->priv.deleted ? " (deleted)" : ""); + debug("Dump of routing table <%s>%s\n", tab->name, OBSREF_GET(tab->priv.deleted) ? " (deleted)" : ""); u32 bs = atomic_load_explicit(&tp->t->routes_block_size, memory_order_relaxed); net *routes = atomic_load_explicit(&tp->t->routes, memory_order_relaxed); @@ -2608,7 +2608,7 @@ rt_dump_hooks(rtable *tp) RT_LOCKED(tp, tab) { - debug("Dump of hooks in routing table <%s>%s\n", tab->name, tab->deleted ? " (deleted)" : ""); + debug("Dump of hooks in routing table <%s>%s\n", tab->name, OBSREF_GET(tab->deleted) ? " (deleted)" : ""); debug(" nhu_state=%u use_count=%d rt_count=%u\n", tab->nhu_state, tab->use_count, tab->rt_count); debug(" last_rt_change=%t gc_time=%t gc_counter=%d prune_state=%u\n", @@ -4313,7 +4313,7 @@ void rt_unlock_table_priv(struct rtable_private *r, const char *file, uint line) { rt_trace(r, D_STATES, "Unlocked at %s:%d", file, line); - if (!--r->use_count && r->deleted) + if (!--r->use_count && OBSREF_GET(r->deleted)) /* Stop the service thread to finish this up */ ev_send_loop(r->loop, ev_new_init(r->rp, rt_shutdown, r)); } @@ -4359,13 +4359,12 @@ rt_delete(void *tab_) * 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_SIMPLE((rtable *) tab_); - struct config *conf = tab->deleted; + OBSREF_CLEAR(tab->deleted); DOMAIN(rtable) dom = tab->lock; RT_UNLOCK_SIMPLE(RT_PUB(tab)); /* Everything is freed by freeing the loop */ birdloop_free(tab->loop); - config_del_obstacle(conf); /* Also drop the domain */ DOMAIN_FREE(rtable, dom); @@ -4378,7 +4377,7 @@ rt_check_cork_low(struct rtable_private *tab) if (!tab->cork_active) return; - if (tab->deleted || + if (OBSREF_GET(tab->deleted) || (lfjour_pending_items(&tab->export_best.journal) < tab->cork_threshold.low) && (lfjour_pending_items(&tab->export_all.journal) < tab->cork_threshold.low)) { @@ -4392,7 +4391,7 @@ rt_check_cork_low(struct rtable_private *tab) static void rt_check_cork_high(struct rtable_private *tab) { - if (!tab->deleted && !tab->cork_active && ( + if (!OBSREF_GET(tab->deleted) && !tab->cork_active && ( (lfjour_pending_items(&tab->export_best.journal) >= tab->cork_threshold.low) || (lfjour_pending_items(&tab->export_all.journal) >= tab->cork_threshold.low))) { @@ -4480,7 +4479,7 @@ rt_commit(struct config *new, struct config *old) _Bool ok; RT_LOCKED(o->table, tab) { - r = tab->deleted ? NULL : rt_find_table_config(new, o->name); + r = OBSREF_GET(tab->deleted) ? NULL : rt_find_table_config(new, o->name); ok = r && !new->shutdown && rt_reconfigure(tab, r, o); } @@ -4491,8 +4490,7 @@ rt_commit(struct config *new, struct config *old) RT_LOCKED(o->table, tab) { DBG("\t%s: deleted\n", o->name); - tab->deleted = old; - config_add_obstacle(old); + OBSREF_SET(tab->deleted, old); rt_lock_table(tab); rt_check_cork_low(tab); diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c index 58af2f75..459ff7b7 100644 --- a/proto/bgp/bgp.c +++ b/proto/bgp/bgp.c @@ -608,10 +608,10 @@ bgp_spawn(struct bgp_proto *pp, ip_addr remote_ip) bsprintf(fmt, "%s%%0%dd", pp->cf->dynamic_name, pp->cf->dynamic_name_digits); /* This is hack, we would like to share config, but we need to copy it now */ - new_config = config; - cfg_mem = config->mem; - config->current_scope = config->root_scope; - sym = cf_default_name(config, fmt, &(pp->dynamic_name_counter)); + new_config = OBSREF_GET(config); + cfg_mem = new_config->mem; + new_config->current_scope = new_config->root_scope; + sym = cf_default_name(new_config, fmt, &(pp->dynamic_name_counter)); proto_clone_config(sym, pp->p.cf); new_config = NULL; cfg_mem = NULL; diff --git a/sysdep/linux/netlink.c b/sysdep/linux/netlink.c index 1db7a468..6e10f543 100644 --- a/sysdep/linux/netlink.c +++ b/sysdep/linux/netlink.c @@ -1974,7 +1974,6 @@ krt_do_scan(struct krt_proto *p) static sock *nl_async_sk; /* BIRD socket for asynchronous notifications */ static byte *nl_async_rx_buffer; /* Receive buffer */ static uint nl_async_bufsize; /* Kernel rx buffer size for the netlink socket */ -static struct config *nl_last_config; /* For tracking changes to nl_async_bufsize */ static void nl_async_msg(struct nlmsghdr *h) @@ -2119,12 +2118,23 @@ nl_update_async_bufsize(void) if (!nl_async_sk) return; + /* For tracking changes to nl_async_bufsize, just a pointer storage */ + static struct config *nl_last_config; + + /* Get current config. This is a bit sketchy but we now + * simply expect that we're running in the mainloop. + * When moving kernel protocol to proper loops, + * this may need to change to avoid funny races. + */ + struct config *cur = OBSREF_GET(config); + ASSERT_DIE(birdloop_inside(&main_birdloop)); + /* Already reconfigured */ - if (nl_last_config == config) + if (nl_last_config == cur) return; /* Update netlink buffer size */ - uint bufsize = nl_cfg_rx_buffer_size(config); + uint bufsize = nl_cfg_rx_buffer_size(cur); if (bufsize && (bufsize != nl_async_bufsize)) { /* Log message for reconfigurations only */ @@ -2135,7 +2145,7 @@ nl_update_async_bufsize(void) nl_async_bufsize = bufsize; } - nl_last_config = config; + nl_last_config = cur; } diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c index 4dece403..254690b1 100644 --- a/sysdep/unix/main.c +++ b/sysdep/unix/main.c @@ -173,6 +173,8 @@ read_iproute_table(struct config *conf, char *file, char *prefix, uint max) #endif // PATH_IPROUTE_DIR +#define CONFIG_COMMIT(cf, ...) ({ CONFIG_REF_LOCAL(cr, cf); config_commit(&cr, __VA_ARGS__); }) + static char *config_name = PATH_CONFIG_FILE; static int @@ -258,7 +260,7 @@ async_config(void) config_free(conf); } else - config_commit(conf, RECONFIG_HARD, 0); + CONFIG_COMMIT(conf, RECONFIG_HARD, 0); } static struct config * @@ -339,7 +341,7 @@ cmd_reconfig(const char *name, int type, uint timeout) if (!conf) return; - int r = config_commit(conf, type, timeout); + int r = CONFIG_COMMIT(conf, type, timeout); if ((r >= 0) && (timeout > 0)) { @@ -961,7 +963,7 @@ main(int argc, char **argv) signal_init(); - config_commit(conf, RECONFIG_HARD, 0); + CONFIG_COMMIT(conf, RECONFIG_HARD, 0); graceful_restart_init(); diff --git a/test/bt-utils.c b/test/bt-utils.c index 56124e1f..a487f43a 100644 --- a/test/bt-utils.c +++ b/test/bt-utils.c @@ -151,8 +151,8 @@ bt_config_parse__(struct config *cfg) return NULL; } - config_commit(cfg, RECONFIG_HARD, 0); - new_config = cfg; + CONFIG_REF_LOCAL(cr, cfg); + config_commit(&cr, RECONFIG_HARD, 0); return cfg; }