Export: More strict export state checking on change

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

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))
-    ok = rt_table_export_stop_locked(hh);
-  else
-    RT_LOCKED(t, tab)
+
+  RT_LOCKED_IF_NEEDED(SKIP_BACK(rtable, priv.exporter, hook->table), 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)
+  /* Run the stop code. Must:
+   * _locked_ update export state to TES_STOP
+   * and _unlocked_ call rt_stop_export_common() */
   hook->table->class->stop(hook);
-  else
-    rt_stop_export_common(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);
 }

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);
 

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,
 };