Merge branch 'master' of ssh://git.nic.cz/bird

6 files changed, 69 insertions, 31 deletions

diff --git a/doc/bird.sgml b/doc/bird.sgml
index df6e2610..a94fb9e1 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -426,24 +426,37 @@ to zero to disable it. An empty <cf><m/switch/</cf> is equivalent to <cf/on/
 	uses global router id.
 
 	<tag>import all | none | filter <m/name/ | filter { <m/filter commands/ } | where <m/filter expression/</tag> 
-	Specify a filter to be used for filtering routes coming from the protocol to the routing table. <cf/all/ is shorthand for <cf/where true/ and <cf/none/ is shorthand for <cf/where false/. Default: <cf/all/.
+	Specify a filter to be used for filtering routes coming from
+	the protocol to the routing table. <cf/all/ is shorthand for
+	<cf/where true/ and <cf/none/ is shorthand for
+	<cf/where false/. Default: <cf/all/.
 
-	<tag>export <m/filter/</tag> This is similar to the <cf>import</cf> keyword, except that it
-	works in the direction from the routing table to the protocol. Default: <cf/none/.
+	<tag>export <m/filter/</tag>
+	This is similar to the <cf>import</cf> keyword, except that it
+	works in the direction from the routing table to the protocol.
+	Default: <cf/none/.
 
-	<tag>import limit <m/number/ [exceed warn | block | restart | disable]</tag>
+	<tag>import limit <m/number/ [action warn | block | restart | disable]</tag>
 	Specify an import route limit (a maximum number of routes
 	imported from the protocol) and optionally the action to be
 	taken when the limit is hit. Warn action just prints warning
 	log message. Block action ignores new routes coming from the
 	protocol. Restart and disable actions shut the protocol down
 	like appropriate commands. Disable is the default action if an
-	action is not explicitly specified.  Default: <cf/none/.
+	action is not explicitly specified. Note that limits are reset
+	during protocol reconfigure, reload or restart.
+	Default: <cf/none/.
 
-	<tag>export limit <m/number/ [exceed warn | block | restart | disable]</tag>
+	<tag>export limit <m/number/ [action 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/config.Y b/nest/config.Y
index c59319cb..14cff10a 100644
--- a/nest/config.Y
+++ b/nest/config.Y
@@ -44,7 +44,7 @@ CF_DECLS
 
 CF_KEYWORDS(ROUTER, ID, PROTOCOL, TEMPLATE, PREFERENCE, DISABLED, DEBUG, ALL, OFF, DIRECT)
 CF_KEYWORDS(INTERFACE, IMPORT, EXPORT, FILTER, NONE, TABLE, STATES, ROUTES, FILTERS)
-CF_KEYWORDS(EXCEED, LIMIT, WARN, BLOCK, RESTART, DISABLE)
+CF_KEYWORDS(LIMIT, ACTION, WARN, BLOCK, RESTART, DISABLE)
 CF_KEYWORDS(PASSWORD, FROM, PASSIVE, TO, ID, EVENTS, PACKETS, PROTOCOLS, INTERFACES)
 CF_KEYWORDS(PRIMARY, STATS, COUNT, FOR, COMMANDS, PREEXPORT, GENERATE, ROA, MAX, FLUSH)
 CF_KEYWORDS(LISTEN, BGP, V6ONLY, DUAL, ADDRESS, PORT, PASSWORDS, DESCRIPTION)
@@ -194,10 +194,10 @@ imexport:
 
 limit_action:
    /* default */ { $$ = PLA_DISABLE; }
- | EXCEED WARN { $$ = PLA_WARN; }
- | EXCEED BLOCK { $$ = PLA_BLOCK; }
- | EXCEED RESTART { $$ = PLA_RESTART; }
- | EXCEED DISABLE { $$ = PLA_DISABLE; }
+ | ACTION WARN { $$ = PLA_WARN; }
+ | ACTION BLOCK { $$ = PLA_BLOCK; }
+ | ACTION RESTART { $$ = PLA_RESTART; }
+ | ACTION DISABLE { $$ = PLA_DISABLE; }
  ;
 
 limit_spec:
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 06121ea3..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)
@@ -1781,14 +1805,14 @@ rt_show_net(struct cli *c, net *n, struct rt_show_data *d)
     d->net_counter++;
   for(e=n->routes; e; e=e->next)
     {
-      struct ea_list *tmpa, *old_tmpa;
+      struct ea_list *tmpa;
       struct proto *p0 = e->attrs->proto;
       struct proto *p1 = d->export_protocol;
       struct proto *p2 = d->show_protocol;
       d->rt_counter++;
       ee = e;
       rte_update_lock();		/* We use the update buffer for filtering */
-      old_tmpa = tmpa = p0->make_tmp_attrs ? p0->make_tmp_attrs(e, rte_update_pool) : NULL;
+      tmpa = p0->make_tmp_attrs ? p0->make_tmp_attrs(e, rte_update_pool) : NULL;
       ok = (d->filter == FILTER_ACCEPT || f_run(d->filter, &e, &tmpa, rte_update_pool, FF_FORCE_TMPATTR) <= F_ACCEPT);
       if (p2 && p2 != p0) ok = 0;
       if (ok && d->export_mode)
diff --git a/proto/ospf/ospf.c b/proto/ospf/ospf.c
index 9872faf2..ef7b0363 100644
--- a/proto/ospf/ospf.c
+++ b/proto/ospf/ospf.c
@@ -1145,16 +1145,16 @@ show_lsa_sum_net(struct top_hash_entry *he)
 static inline void
 show_lsa_sum_rt(struct top_hash_entry *he)
 {
-  u32 dst_rid, options;
+  u32 dst_rid;
 
 #ifdef OSPFv2
   struct ospf_lsa_sum *ls = he->lsa_body;
   dst_rid = he->lsa.id;
-  options = 0;
+  // options = 0;
 #else /* OSPFv3 */
   struct ospf_lsa_sum_rt *ls = he->lsa_body;
   dst_rid = ls->drid; 
-  options = ls->options & OPTIONS_MASK;
+  // options = ls->options & OPTIONS_MASK;
 #endif
 
   cli_msg(-1016, "\t\txrouter %R metric %u", dst_rid, ls->metric);
diff --git a/proto/ospf/topology.c b/proto/ospf/topology.c
index 604c8ea7..7e9bad50 100644
--- a/proto/ospf/topology.c
+++ b/proto/ospf/topology.c
@@ -798,7 +798,7 @@ originate_sum_net_lsa(struct ospf_area *oa, struct fib_node *fn, int metric)
 
   body = originate_sum_net_lsa_body(po, &lsa.length, fn, metric);
   lsasum_calculate(&lsa, body);
-  en = lsa_install_new(po, &lsa, dom, body);
+  lsa_install_new(po, &lsa, dom, body);
   ospf_lsupd_flood(po, NULL, NULL, &lsa, dom, 1);
 }
 
@@ -835,7 +835,7 @@ originate_sum_rt_lsa(struct ospf_area *oa, struct fib_node *fn, int metric, u32
 
   body = originate_sum_rt_lsa_body(po, &lsa.length, lsa.id, metric, options);
   lsasum_calculate(&lsa, body);
-  en = lsa_install_new(po, &lsa, dom, body);
+  lsa_install_new(po, &lsa, dom, body);
   ospf_lsupd_flood(po, NULL, NULL, &lsa, dom, 1);
 }
 
@@ -1117,7 +1117,7 @@ originate_ext_lsa(struct ospf_area *oa, struct fib_node *fn, int src,
   if (src) 
     fn->x1 = src;
 
-  en = lsa_install_new(po, &lsa, dom, body);
+  lsa_install_new(po, &lsa, dom, body);
   ospf_lsupd_flood(po, NULL, NULL, &lsa, dom, 1);
 
   if (po->ebit == 0)