From ab758e4fb205346946f2d828236bd23efc2a419e Mon Sep 17 00:00:00 2001
From: Ondrej Zajicek <santiago@crfreenet.org>
Date: Sat, 28 Apr 2012 12:59:40 +0200
Subject: [PATCH] Some fixes in route export limits.

---
 doc/bird.sgml   |  8 +++++++-
 nest/protocol.h |  3 ++-
 nest/rt-table.c | 44 ++++++++++++++++++++++++++++++++++----------
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/doc/bird.sgml b/doc/bird.sgml
index df6e2610..91cad085 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -443,7 +443,13 @@ to zero to disable it. An empty <cf><m/switch/</cf> is equivalent to <cf/on/
 	<tag>export limit <m/number/ [exceed warn | block | restart | disable]</tag>
 	Specify an export route limit, works similarly to
 	the <cf>import limit</cf> option, but for the routes exported
-	to the protocol. Default: <cf/none/.
+	to the protocol. This option is experimental, there are some
+	problems in details of its behavior -- the number of exported
+	routes can temporarily exceed the limit without triggering it
+	during protocol reload, exported routes counter ignores route
+	blocking and block action also blocks route updates of alread
+	accepted routes -- and these details will probably change in
+	the future. Default: <cf/none/.
 
 	<tag>description "<m/text/"</tag> This is an optional
 	description of the protocol. It is displayed as a part of the
diff --git a/nest/protocol.h b/nest/protocol.h
index 3f9ed96e..8a632715 100644
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -387,7 +387,8 @@ struct proto_limit {
 
 void proto_notify_limit(struct announce_hook *ah, struct proto_limit *l, u32 rt_count);
 
-static inline void proto_reset_limit(struct proto_limit *l)
+static inline void
+proto_reset_limit(struct proto_limit *l)
 {
   if (l)
     l->state = PLS_INITIAL;
diff --git a/nest/rt-table.c b/nest/rt-table.c
index 6976ddcd..bb0ee4c8 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -269,27 +269,52 @@ do_rte_announce(struct announce_hook *ah, int type UNUSED, net *net, rte *new, r
 	}
     }
 
-  /* FIXME - This is broken because of incorrect 'old' value (see above) */
-  if (!new && !old)
-    return;
+  /*
+   * Export route limits has several problems. Because exp_routes
+   * counter is reset before refeed, we don't really know whether
+   * limit is breached and whether the update is new or not Therefore
+   * the number of really exported routes may exceed the limit
+   * temporarily (routes exported before and new routes in refeed).
+   *
+   * Minor advantage is that if the limit is decreased and refeed is
+   * requested, the number of exported routes really decrease.
+   *
+   * Second problem is that with export limits, we don't know whether
+   * old was really exported (it might be blocked by limit). When a
+   * withdraw is exported, we announce it even when the previous
+   * update was blocked. This is not a big issue, but the same problem
+   * is in updating exp_routes counter. Therefore, to be consistent in
+   * increases and decreases of exp_routes, we count exported routes
+   * regardless of blocking by limits.
+   *
+   * Similar problem is in handling updates - when a new route is
+   * received and blocking is active, the route would be blocked, but
+   * when an update for the route will be received later, the update
+   * would be propagated (as old != NULL). Therefore, we have to block
+   * also non-new updates (contrary to import blocking).
+   */
 
   struct proto_limit *l = ah->out_limit;
-  if (l && new && (!old || refeed))
+  if (l && new)
     {
-      if (stats->exp_routes >= l->limit)
+      if ((!old || refeed) && (stats->exp_routes >= l->limit))
 	proto_notify_limit(ah, l, stats->exp_routes);
 
       if (l->state == PLS_BLOCKED)
 	{
-	  /* Exported route counter ignores whether the route was
-	     blocked by limit, to be consistent when limits change */
-	  stats->exp_routes++;
+	  stats->exp_routes++;	/* see note above */
 	  stats->exp_updates_rejected++;
 	  rte_trace_out(D_FILTERS, p, new, "rejected [limit]");
-	  goto done;
+	  if (new != new0)
+	    rte_free(new);
+	  new = NULL;
 	}
     }
 
+  /* FIXME - This is broken because of incorrect 'old' value (see above) */
+  if (!new && !old)
+    return;
+
   if (new)
     stats->exp_updates_accepted++;
   else
@@ -325,7 +350,6 @@ do_rte_announce(struct announce_hook *ah, int type UNUSED, net *net, rte *new, r
   else
     p->rt_notify(p, ah->table, net, new, old, new->attrs->eattrs);
 
- done:
   if (new && new != new0)	/* Discard temporary rte's */
     rte_free(new);
   if (old && old != old0)