Some fixes in route export limits.

diff --git a/doc/bird.sgml b/doc/bird.sgml
index df6e26108d64c7f5dc615fb951894363a984e492..91cad0851b08b27b86001a4340a34637b0b5ce37 100644 (file)
--- 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 3f9ed96eef5f1248dedff01f1aa9109af9c703e8..8a632715abcc2e5d8b536b131e0b592d54058b41 100644 (file)
--- 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 6976ddcd3153a81c00294815828b13db6f180bae..bb0ee4c88a9b13beb3b94a1dcbf76720bf79274f 100644 (file)
--- 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)