From 9135c1f0ca6322bff9648895b5394b97761b4bcb Mon Sep 17 00:00:00 2001 From: Ondrej Zajicek Date: Wed, 24 Jul 2013 14:11:12 +0200 Subject: [PATCH] Fixes bug in protocol flushing and rtable pruning. When route was propagated to another rtable through a pipe and then the pipe was reconfigured softly in such a way that any subsequent route updates are filtered, then the source protocol shutdown didn't clean up the route in the second rtable which caused stale routes and potential crashes. --- nest/proto.c | 6 +++++- nest/route.h | 3 +-- nest/rt-table.c | 48 ++++++++++++++++++++++++++++++------------------ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/nest/proto.c b/nest/proto.c index 7e7fb7fa..60495aa0 100644 --- a/nest/proto.c +++ b/nest/proto.c @@ -835,14 +835,18 @@ static void proto_schedule_flush_loop(void) { struct proto *p; + struct announce_hook *h; if (flush_loop_state) return; flush_loop_state = 1; - rt_schedule_prune_all(); WALK_LIST(p, flush_proto_list) + { p->flushing = 1; + for (h=p->ahooks; h; h=h->next) + h->table->prune_state = 1; + } ev_schedule(proto_flush_event); } diff --git a/nest/route.h b/nest/route.h index 8fd01a66..35b5fa19 100644 --- a/nest/route.h +++ b/nest/route.h @@ -141,7 +141,7 @@ typedef struct rtable { int gc_counter; /* Number of operations since last GC */ bird_clock_t gc_time; /* Time of last GC */ byte gc_scheduled; /* GC is scheduled */ - byte prune_state; /* Table prune state, 1 -> prune is running */ + byte prune_state; /* Table prune state, 1 -> scheduled, 2-> running */ byte hcu_scheduled; /* Hostcache update is scheduled */ byte nhu_state; /* Next Hop Update state */ struct fib_iterator prune_fit; /* Rtable prune FIB iterator */ @@ -265,7 +265,6 @@ void rt_dump(rtable *); void rt_dump_all(void); int rt_feed_baby(struct proto *p); void rt_feed_baby_abort(struct proto *p); -void rt_schedule_prune_all(void); int rt_prune_loop(void); struct rtable_config *rt_new_table(struct symbol *s); diff --git a/nest/rt-table.c b/nest/rt-table.c index ebdac833..16dd9bcd 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -1268,19 +1268,8 @@ rt_init(void) } -/* Called from proto_schedule_flush_loop() only, - ensuring that all prune states are zero */ -void -rt_schedule_prune_all(void) -{ - rtable *t; - - WALK_LIST(t, routing_tables) - t->prune_state = 1; -} - static inline int -rt_prune_step(rtable *tab, int *max_feed) +rt_prune_step(rtable *tab, int step, int *max_feed) { struct fib_iterator *fit = &tab->prune_fit; @@ -1306,8 +1295,8 @@ again: rescan: for (e=n->routes; e; e=e->next) - if (e->sender->proto->core_state != FS_HAPPY && - e->sender->proto->core_state != FS_FEEDING) + if (e->sender->proto->flushing || + (step && e->attrs->proto->flushing)) { if (*max_feed <= 0) { @@ -1315,6 +1304,10 @@ again: return 0; } + if (step) + log(L_WARN "Route %I/%d from %s still in %s after flush", + n->n.prefix, n->n.pxlen, e->attrs->proto->name, tab->name); + rte_discard(tab, e); (*max_feed)--; @@ -1339,23 +1332,42 @@ again: /** * rt_prune_loop - prune routing tables - * @tab: routing table to be pruned * * The prune loop scans routing tables and removes routes belonging to - * inactive protocols and also stale network entries. Returns 1 when + * flushing protocols and also stale network entries. Returns 1 when * all such routes are pruned. It is a part of the protocol flushing * loop. + * + * The prune loop runs in two steps. In the first step it prunes just + * the routes with flushing senders (in explicitly marked tables) so + * the route removal is propagated as usual. In the second step, all + * remaining relevant routes are removed. Ideally, there shouldn't be + * any, but it happens when pipe filters are changed. */ int rt_prune_loop(void) { - rtable *t; + static int step = 0; int max_feed = 512; + rtable *t; + again: WALK_LIST(t, routing_tables) - if (! rt_prune_step(t, &max_feed)) + if (! rt_prune_step(t, step, &max_feed)) return 0; + if (step == 0) + { + /* Prepare for the second step */ + WALK_LIST(t, routing_tables) + t->prune_state = 1; + + step = 1; + goto again; + } + + /* Done */ + step = 0; return 1; }