From c262c728eb974e13e333cc87d3aa65183e8890b5 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Tue, 15 Aug 2023 12:31:28 +0200 Subject: [PATCH] Export: More strict export state checking on change --- lib/birdlib.h | 5 +++++ nest/rt-table.c | 44 +++++++++++++++++++++++--------------------- nest/rt.h | 7 ++++++- proto/bgp/attrs.c | 10 +++++++++- 4 files changed, 43 insertions(+), 23 deletions(-) diff --git a/lib/birdlib.h b/lib/birdlib.h index 8146e63b..97886646 100644 --- a/lib/birdlib.h +++ b/lib/birdlib.h @@ -13,6 +13,7 @@ #include "sysdep/config.h" #include "lib/alloca.h" +#include "lib/macro.h" /* Ugly structure offset handling macros */ @@ -89,6 +90,10 @@ static inline int u64_cmp(u64 i1, u64 i2) #define BIT32R_CLR(b,p) ((b)[(p)/32] &= ~BIT32R_VAL(p)) #define BIT32R_ZERO(b,l) memset((b), 0, (l)/8) +/* Short Bitmask Constructor */ +#define BIT32_ALL_HELPER(x) (1 << (x)) | +#define BIT32_ALL(...) (MACRO_FOREACH(BIT32_ALL_HELPER, __VA_ARGS__) 0) + #ifndef NULL #define NULL ((void *) 0) #endif diff --git a/nest/rt-table.c b/nest/rt-table.c index c3236200..6cd44564 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2044,14 +2044,22 @@ rt_set_import_state(struct rt_import_hook *hook, u8 state) CALL(hook->req->log_state_change, hook->req, state); } -void -rt_set_export_state(struct rt_export_hook *hook, u8 state) +u8 +rt_set_export_state(struct rt_export_hook *hook, u32 expected_mask, u8 state) { hook->last_state_change = current_time(); u8 old = atomic_exchange_explicit(&hook->export_state, state, memory_order_release); + if (!((1 << old) & expected_mask)) + bug("Unexpected export state change from %s to %s, expected mask %02x", + rt_export_state_name(old), + rt_export_state_name(state), + expected_mask + ); if (old != state) CALL(hook->req->log_state_change, hook->req, state); + + return old; } void @@ -2148,7 +2156,7 @@ rt_table_export_start_locked(struct rtable_private *tab, struct rt_export_reques }; if (rt_cork_check(&hook->h.event)) - rt_set_export_state(&hook->h, TES_HUNGRY); + rt_set_export_state(&hook->h, BIT32_ALL(TES_DOWN), TES_HUNGRY); else rt_table_export_start_feed(tab, hook); } @@ -2226,6 +2234,7 @@ rt_alloc_export(struct rt_exporter *re, pool *pp, uint size) hook->pool = p; hook->table = re; + atomic_store_explicit(&hook->export_state, TES_DOWN, memory_order_release); hook->n = (node) {}; add_tail(&re->hooks, &hook->n); @@ -2241,7 +2250,7 @@ rt_init_export(struct rt_exporter *re UNUSED, struct rt_export_hook *hook) bmap_init(&hook->seq_map, hook->pool, 16); /* Regular export */ - rt_set_export_state(hook, TES_FEEDING); + rt_set_export_state(hook, BIT32_ALL(TES_DOWN, TES_HUNGRY), TES_FEEDING); rt_send_export_event(hook); } @@ -2251,7 +2260,8 @@ rt_table_export_stop_locked(struct rt_export_hook *hh) struct rt_table_export_hook *hook = SKIP_BACK(struct rt_table_export_hook, h, hh); struct rtable_private *tab = SKIP_BACK(struct rtable_private, exporter, hook->table); - switch (atomic_load_explicit(&hh->export_state, memory_order_relaxed)) + /* Update export state, get old */ + switch (rt_set_export_state(hh, BIT32_ALL(TES_HUNGRY, TES_FEEDING, TES_READY), TES_STOP)) { case TES_HUNGRY: rt_trace(tab, D_EVENTS, "Stopping export hook %s must wait for uncorking", hook->h.req->name); @@ -2288,17 +2298,12 @@ rt_table_export_stop(struct rt_export_hook *hh) { struct rt_table_export_hook *hook = SKIP_BACK(struct rt_table_export_hook, h, hh); int ok = 0; - rtable *t = SKIP_BACK(rtable, priv.exporter, hook->table); - if (RT_IS_LOCKED(t)) + + RT_LOCKED_IF_NEEDED(SKIP_BACK(rtable, priv.exporter, hook->table), tab) ok = rt_table_export_stop_locked(hh); - else - RT_LOCKED(t, tab) - ok = rt_table_export_stop_locked(hh); if (ok) rt_stop_export_common(hh); - else - rt_set_export_state(&hook->h, TES_STOP); } void @@ -2311,19 +2316,16 @@ rt_stop_export(struct rt_export_request *req, void (*stopped)(struct rt_export_r /* Set the stopped callback */ hook->stopped = stopped; - /* Run the stop code */ - if (hook->table->class->stop) - hook->table->class->stop(hook); - else - rt_stop_export_common(hook); + /* Run the stop code. Must: + * _locked_ update export state to TES_STOP + * and _unlocked_ call rt_stop_export_common() */ + hook->table->class->stop(hook); } +/* Call this common code from the stop code in table export class */ void rt_stop_export_common(struct rt_export_hook *hook) { - /* Update export state */ - rt_set_export_state(hook, TES_STOP); - /* Reset the event as the stopped event */ hook->event.hook = hook->table->class->done; @@ -4242,7 +4244,7 @@ rt_feed_done(struct rt_export_hook *c) { c->event.hook = rt_export_hook; - rt_set_export_state(c, TES_READY); + rt_set_export_state(c, BIT32_ALL(TES_FEEDING), TES_READY); rt_send_export_event(c); } diff --git a/nest/rt.h b/nest/rt.h index c1938c18..e0a099e1 100644 --- a/nest/rt.h +++ b/nest/rt.h @@ -177,6 +177,11 @@ typedef union rtable { #define RT_PUB(tab) SKIP_BACK(rtable, priv, tab) #define RT_LOCKED(tpub, tpriv) for (struct rtable_private *tpriv = RT_LOCK(tpub); tpriv; RT_UNLOCK(tpriv), (tpriv = NULL)) +#define RT_LOCKED_IF_NEEDED(_tpub, _tpriv) for ( \ + struct rtable_private *_al = RT_IS_LOCKED(_tpub) ? &(_tpub)->priv : NULL, *_tpriv = _al ?: RT_LOCK(_tpub); \ + _tpriv; \ + _al ?: RT_UNLOCK(_tpriv), (_tpriv = NULL)) + #define RT_RETURN(tpriv, ...) do { RT_UNLOCK(tpriv); return __VA_ARGS__; } while (0) #define RT_PRIV_SAME(tpriv, tpub) (&(tpub)->priv == (tpriv)) @@ -415,7 +420,7 @@ const char *rt_export_state_name(u8 state); static inline u8 rt_import_get_state(struct rt_import_hook *ih) { return ih ? ih->import_state : TIS_DOWN; } static inline u8 rt_export_get_state(struct rt_export_hook *eh) { return eh ? eh->export_state : TES_DOWN; } -void rt_set_export_state(struct rt_export_hook *hook, u8 state); +u8 rt_set_export_state(struct rt_export_hook *hook, u32 expected_mask, u8 state); void rte_import(struct rt_import_request *req, const net_addr *net, rte *new, struct rte_src *src); diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 19a09238..973e2935 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -1981,7 +1981,7 @@ bgp_out_table_feed(void *data) if (hook->hash_iter) ev_schedule_work(&hook->h.event); else - rt_set_export_state(&hook->h, TES_READY); + rt_set_export_state(&hook->h, BIT32_ALL(TES_FEEDING), TES_READY); } static void @@ -1996,6 +1996,13 @@ bgp_out_table_export_start(struct rt_exporter *re, struct rt_export_request *req rt_init_export(re, req->hook); } +static void +bgp_out_table_export_stop(struct rt_export_hook *hook) +{ + rt_set_export_state(hook, BIT32_ALL(TES_HUNGRY, TES_FEEDING, TES_READY), TES_STOP); + rt_stop_export_common(hook); +} + static void bgp_out_table_export_done(void *data) { @@ -2009,6 +2016,7 @@ bgp_out_table_export_done(void *data) static const struct rt_exporter_class bgp_out_table_export_class = { .start = bgp_out_table_export_start, + .stop = bgp_out_table_export_stop, .done = bgp_out_table_export_done, };