Nest: Fix race condition during reconfiguration

If export filter is changed during reconfiguration and a route disappears
between reconfiguration and refeed (e.g., if the route is a static route
also removed during the reconfiguration), the route is not withdrawn.
The patch fixes that by adding tx reconfiguration timestamp.

diff --git a/nest/proto.c b/nest/proto.c
index 49f71304b61c6acdaa4e2af602564d4d2f3b4e8c..8a8221a807388c267b4da57371bebb62b352fcbd 100644 (file)
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -165,6 +165,7 @@ proto_add_channel(struct proto *p, struct channel_config *cf)
   c->channel_state = CS_DOWN;
   c->export_state = ES_DOWN;
   c->last_state_change = current_time();
+  c->last_tx_filter_change = current_time();
   c->reloadable = 1;
 
   CALL(c->channel->init, c, cf);
@@ -557,6 +558,9 @@ channel_reconfigure(struct channel *c, struct channel_config *cf)
 
   channel_verify_limits(c);
 
+  if (export_changed)
+    c->last_tx_filter_change = current_time();
+
   /* Execute channel-specific reconfigure hook */
   if (c->channel->reconfigure && !c->channel->reconfigure(c, cf))
     return 0;

diff --git a/nest/protocol.h b/nest/protocol.h
index dd942c101558b2dba927c8c81238532b58b9c3bb..f0b65598aedea34d9d2464f83f4e73e13cf9f33d 100644 (file)
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -536,6 +536,7 @@ struct channel {
   u8 gr_wait;                          /* Route export to channel is postponed until graceful restart */
 
   btime last_state_change;             /* Time of last state transition */
+  btime last_tx_filter_change;
 };
 
 

diff --git a/nest/rt-table.c b/nest/rt-table.c
index 0c0e365e756c7c70a754149fbf508629f68f03a1..9ce524289e70663e73b7cabfe4eb7b4333ed2b57 100644 (file)
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -549,29 +549,29 @@ rt_notify_basic(struct channel *c, net *net, rte *new0, rte *old0, int refeed)
     c->stats.exp_withdraws_received++;
 
   /*
-   * This is a tricky part - we don't know whether route 'old' was
-   * exported to protocol 'p' or was filtered by the export filter.
-   * We try to run the export filter to know this to have a correct
-   * value in 'old' argument of rte_update (and proper filter value)
+   * This is a tricky part - we don't know whether route 'old' was exported to
+   * protocol 'p' or was filtered by the export filter. We try to run the export
+   * filter to know this to have a correct value in 'old' argument of rte_update
+   * (and proper filter value).
    *
-   * FIXME - this is broken because 'configure soft' may change
-   * filters but keep routes. Refeed is expected to be called after
-   * change of the filters and with old == new, therefore we do not
-   * even try to run the filter on an old route, This may lead to
-   * 'spurious withdraws' but ensure that there are no 'missing
+   * This is broken because 'configure soft' may change filters but keep routes.
+   * Refeed cycle is expected to be called after change of the filters and with
+   * old == new, therefore we do not even try to run the filter on an old route.
+   * This may lead to 'spurious withdraws' but ensure that there are no 'missing
    * withdraws'.
    *
-   * This is not completely safe as there is a window between
-   * reconfiguration and the end of refeed - if a newly filtered
-   * route disappears during this period, proper withdraw is not
-   * sent (because old would be also filtered) and the route is
-   * not refeeded (because it disappeared before that).
+   * This is not completely safe as there is a window between reconfiguration
+   * and the end of refeed - if a newly filtered route disappears during this
+   * period, proper withdraw is not sent (because old would be also filtered)
+   * and the route is not refeeded (because it disappeared before that).
+   * Therefore, we also do not try to run the filter on old routes that are
+   * older than the last filter change.
    */
 
   if (new)
     new = export_filter(c, new, &new_free, 0);
 
-  if (old && !refeed)
+  if (old && !(refeed || (old->lastmod <= c->last_tx_filter_change)))
     old = export_filter(c, old, &old_free, 1);
 
   if (!new && !old)