From 2dd9800ab51a309add1c56aa9659c41f30481299 Mon Sep 17 00:00:00 2001
From: "Ondrej Zajicek (work)" <santiago@crfreenet.org>
Date: Tue, 19 Feb 2019 16:00:30 +0100
Subject: Nest: Improve export counter handling

One of previous workarounds for phantom route avoidance breaks export
counters by expanding sending of spurious withdraws, which are send when
we are not sure whether we have advertised that routes in the past.
If not, then export counter is decreased, but it was not increased
before, so it overflows under zero.

The patch fixes that by sending spurious withdraws, but not counting them
on export counter. That may lead to error in the other direction, but
that happens only as a race condition (i.e., in normal operation filters
return proper values about old route export state).
---
 nest/rt-table.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

(limited to 'nest/rt-table.c')

diff --git a/nest/rt-table.c b/nest/rt-table.c
index a06b8c35..68ee59b4 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -426,35 +426,48 @@ rt_notify_basic(struct announce_hook *ah, net *net, rte *new0, rte *old0, int re
    * 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.
+   * not refeeded (because it disappeared before that). This is
+   * handled below as a special case.
    */
 
   if (new)
     new = export_filter(ah, new, &new_free, &tmpa, 0);
 
-  if (old && !(refeed || ((old->lastmod <= ah->last_out_filter_change) &&
-			  (p != old->sender->proto))))
+  if (old && !refeed)
     old = export_filter(ah, old, &old_free, NULL, 1);
 
   if (!new && !old)
   {
     /*
      * As mentioned above, 'old' value may be incorrect in some race conditions.
-     * We generally ignore it with the exception of withdraw to pipe protocol.
-     * In that case we rather propagate unfiltered withdraws regardless of
-     * export filters to ensure that when a protocol is flushed, its routes are
-     * removed from all tables. Possible spurious unfiltered withdraws are not
-     * problem here as they are ignored if there is no corresponding route at
-     * the other end of the pipe. We directly call rt_notify() hook instead of
+     * We generally ignore it with two exceptions:
+     *
+     * First, withdraw to pipe protocol. In that case we rather propagate
+     * unfiltered withdraws regardless of export filters to ensure that when a
+     * protocol is flushed, its routes are removed from all tables. Possible
+     * spurious unfiltered withdraws are not problem here as they are ignored if
+     * there is no corresponding route at the other end of the pipe.
+     *
+     * Second, recent filter change. If old route is older than filter change,
+     * then it was previously evaluated by a different filter and we do not know
+     * whether it was really propagated. In that case we rather send spurious
+     * withdraw than do nothing and possibly cause phantom routes.
+     *
+     * In both cases wqe directly call rt_notify() hook instead of
      * do_rt_notify() to avoid logging and stat counters.
      */
 
+    int pipe_withdraw = 0, filter_change = 0;
 #ifdef CONFIG_PIPE
-    if ((p->proto == &proto_pipe) && !new0 && (p != old0->sender->proto))
-      p->rt_notify(p, ah->table, net, NULL, old0, NULL);
+    pipe_withdraw = (p->proto == &proto_pipe) && !new0;
 #endif
+    filter_change = old0 && (old0->lastmod <= ah->last_out_filter_change);
+
+    if ((pipe_withdraw || filter_change) && (p != old0->sender->proto))
+    {
+      stats->exp_withdraws_accepted++;
+      p->rt_notify(p, ah->table, net, NULL, old0, NULL);
+    }
 
     return;
   }
-- 
cgit v1.2.3