[PATCH v2 1/2] Babel: Remove unecessary FIB_ITERATE restart
Daniel Gröber
dxld at darkboxed.org
Mon Jan 30 22:30:16 CET 2023
The route expiration code appears to have been stolen from rip.c, in that
code the rt_notify function actually does modify the rtable fib by calling
fib_get. The babel code however does no such thing, so this inefficient
restart is just entirely uneccesarry.
To prove this is true I add a bunch of checking code that can be removed
later.
---
Changes in v2:
- Fix assert due to misplaced lock/unlock
proto/babel/babel.c | 64 ++++++++++++++++++++++++++-------------------
proto/babel/babel.h | 8 +++---
2 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/proto/babel/babel.c b/proto/babel/babel.c
index ff8b6b52..a52125ff 100644
--- a/proto/babel/babel.c
+++ b/proto/babel/babel.c
@@ -58,6 +58,9 @@ static void babel_update_cost(struct babel_neighbor *n);
static inline void babel_kick_timer(struct babel_proto *p);
static inline void babel_iface_kick_timer(struct babel_iface *ifa);
+#define rtable_lock(rtable) ASSERT(!(rtable)->lock++)
+#define rtable_unlock(rtable) ASSERT(!--(rtable)->lock)
+
/*
* Functions to maintain data structures
*/
@@ -76,15 +79,16 @@ babel_init_entry(struct fib *f UNUSED, void *E)
static inline struct babel_entry *
babel_find_entry(struct babel_proto *p, const net_addr *n)
{
- struct fib *rtable = (n->type == NET_IP4) ? &p->ip4_rtable : &p->ip6_rtable;
+ struct fib *rtable = (n->type == NET_IP4) ? &p->ip4_rtable.fib : &p->ip6_rtable.fib;
return fib_find(rtable, n);
}
static struct babel_entry *
babel_get_entry(struct babel_proto *p, const net_addr *n)
{
- struct fib *rtable = (n->type == NET_IP4) ? &p->ip4_rtable : &p->ip6_rtable;
- struct babel_entry *e = fib_get(rtable, n);
+ struct babel_rtable *rtable = (n->type == NET_IP4) ? &p->ip4_rtable : &p->ip6_rtable;
+ ASSERT(!rtable->lock);
+ struct babel_entry *e = fib_get(&rtable->fib, n);
return e;
}
@@ -217,17 +221,18 @@ babel_refresh_route(struct babel_proto *p, struct babel_route *r)
}
static void
-babel_expire_routes_(struct babel_proto *p, struct fib *rtable)
+babel_expire_routes_(struct babel_proto *p, struct babel_rtable *rtable)
{
struct babel_config *cf = (void *) p->p.cf;
struct babel_route *r, *rx;
struct fib_iterator fit;
btime now_ = current_time();
- FIB_ITERATE_INIT(&fit, rtable);
+ FIB_ITERATE_INIT(&fit, &rtable->fib);
loop:
- FIB_ITERATE_START(rtable, &fit, struct babel_entry, e)
+ rtable_lock(rtable);
+ FIB_ITERATE_START(&rtable->fib, &fit, struct babel_entry, e)
{
int changed = 0;
@@ -244,16 +249,7 @@ loop:
}
if (changed)
- {
- /*
- * We have to restart the iteration because there may be a cascade of
- * synchronous events babel_select_route() -> nest table change ->
- * babel_rt_notify() -> rtable change, invalidating hidden variables.
- */
- FIB_ITERATE_PUT(&fit);
babel_select_route(p, e, NULL);
- goto loop;
- }
/* Clean up stale entries */
if ((e->valid == BABEL_ENTRY_STALE) && ((e->updated + cf->hold_time) <= now_))
@@ -263,6 +259,7 @@ loop:
if (e->unreachable && (!e->valid || (e->router_id == p->router_id)))
{
FIB_ITERATE_PUT(&fit);
+ rtable_unlock(rtable);
babel_announce_retraction(p, e);
goto loop;
}
@@ -274,11 +271,13 @@ loop:
if (!e->valid && EMPTY_LIST(e->routes) && EMPTY_LIST(e->sources) && EMPTY_LIST(e->requests))
{
FIB_ITERATE_PUT(&fit);
- fib_delete(rtable, e);
+ rtable_unlock(rtable);
+ fib_delete(&rtable->fib, e);
goto loop;
}
}
FIB_ITERATE_END;
+ rtable_unlock(rtable);
}
static void
@@ -959,7 +958,7 @@ babel_send_seqno_request(struct babel_proto *p, struct babel_entry *e,
* transmitted entry is updated.
*/
static void
-babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable)
+babel_send_update_(struct babel_iface *ifa, btime changed, struct babel_rtable *rtable)
{
struct babel_proto *p = ifa->proto;
@@ -970,7 +969,8 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable)
p->update_seqno_inc = 0;
}
- FIB_WALK(rtable, struct babel_entry, e)
+ rtable_lock(rtable);
+ FIB_WALK(&rtable->fib, struct babel_entry, e)
{
if (!e->valid)
continue;
@@ -1022,6 +1022,7 @@ babel_send_update_(struct babel_iface *ifa, btime changed, struct fib *rtable)
}
}
FIB_WALK_END;
+ rtable_unlock(rtable);
}
static void
@@ -2055,16 +2056,21 @@ babel_dump(struct proto *P)
WALK_LIST(ifa, p->interfaces)
babel_dump_iface(ifa);
- FIB_WALK(&p->ip4_rtable, struct babel_entry, e)
+ rtable_lock(&p->ip4_rtable);
+ FIB_WALK(&p->ip4_rtable.fib, struct babel_entry, e)
{
babel_dump_entry(e);
}
FIB_WALK_END;
- FIB_WALK(&p->ip6_rtable, struct babel_entry, e)
+ rtable_unlock(&p->ip4_rtable);
+
+ rtable_lock(&p->ip6_rtable);
+ FIB_WALK(&p->ip6_rtable.fib, struct babel_entry, e)
{
babel_dump_entry(e);
}
FIB_WALK_END;
+ rtable_unlock(&p->ip6_rtable);
}
static void
@@ -2180,11 +2186,12 @@ babel_show_neighbors(struct proto *P, const char *iff)
}
static void
-babel_show_entries_(struct babel_proto *p, struct fib *rtable)
+babel_show_entries_(struct babel_proto *p, struct babel_rtable *rtable)
{
int width = babel_sadr_enabled(p) ? -54 : -24;
- FIB_WALK(rtable, struct babel_entry, e)
+ rtable_lock(rtable);
+ FIB_WALK(&rtable->fib, struct babel_entry, e)
{
struct babel_route *r = NULL;
uint rts = 0, srcs = 0;
@@ -2207,6 +2214,7 @@ babel_show_entries_(struct babel_proto *p, struct fib *rtable)
e->n.addr, "<none>", "-", "-", rts, srcs);
}
FIB_WALK_END;
+ rtable_unlock(rtable);
}
void
@@ -2230,11 +2238,12 @@ babel_show_entries(struct proto *P)
}
static void
-babel_show_routes_(struct babel_proto *p, struct fib *rtable)
+babel_show_routes_(struct babel_proto *p, struct babel_rtable *rtable)
{
int width = babel_sadr_enabled(p) ? -54 : -24;
- FIB_WALK(rtable, struct babel_entry, e)
+ rtable_lock(rtable);
+ FIB_WALK(&rtable->fib, struct babel_entry, e)
{
struct babel_route *r;
WALK_LIST(r, e->routes)
@@ -2247,6 +2256,7 @@ babel_show_routes_(struct babel_proto *p, struct fib *rtable)
}
}
FIB_WALK_END;
+ rtable_unlock(rtable);
}
void
@@ -2446,9 +2456,9 @@ babel_start(struct proto *P)
struct babel_config *cf = (void *) P->cf;
u8 ip6_type = cf->ip6_channel ? cf->ip6_channel->net_type : NET_IP6;
- fib_init(&p->ip4_rtable, P->pool, NET_IP4, sizeof(struct babel_entry),
+ fib_init(&p->ip4_rtable.fib, P->pool, NET_IP4, sizeof(struct babel_entry),
OFFSETOF(struct babel_entry, n), 0, babel_init_entry);
- fib_init(&p->ip6_rtable, P->pool, ip6_type, sizeof(struct babel_entry),
+ fib_init(&p->ip6_rtable.fib, P->pool, ip6_type, sizeof(struct babel_entry),
OFFSETOF(struct babel_entry, n), 0, babel_init_entry);
init_list(&p->interfaces);
@@ -2508,7 +2518,7 @@ babel_reconfigure(struct proto *P, struct proto_config *CF)
TRACE(D_EVENTS, "Reconfiguring");
- if (p->ip6_rtable.addr_type != ip6_type)
+ if (p->ip6_rtable.fib.addr_type != ip6_type)
return 0;
if (!proto_configure_channel(P, &p->ip4_channel, new->ip4_channel) ||
diff --git a/proto/babel/babel.h b/proto/babel/babel.h
index da8386b3..e55523ed 100644
--- a/proto/babel/babel.h
+++ b/proto/babel/babel.h
@@ -158,8 +158,10 @@ struct babel_iface_config {
struct babel_proto {
struct proto p;
timer *timer;
- struct fib ip4_rtable;
- struct fib ip6_rtable;
+ struct babel_rtable {
+ struct fib fib;
+ u8 lock; /* Ensure no changes during FIB_ITERATE */
+ } ip4_rtable, ip6_rtable;
struct channel *ip4_channel;
struct channel *ip6_channel;
@@ -408,7 +410,7 @@ struct babel_msg_auth {
};
static inline int babel_sadr_enabled(struct babel_proto *p)
-{ return p->ip6_rtable.addr_type == NET_IP6_SADR; }
+{ return p->ip6_rtable.fib.addr_type == NET_IP6_SADR; }
/* babel.c */
void babel_handle_ack_req(union babel_msg *msg, struct babel_iface *ifa);
--
2.30.2
More information about the Bird-users
mailing list