Hi Toke, On Mon, May 09, 2022 at 03:21:15PM +0200, Toke Høiland-Jørgensen wrote: > >> > + /* Update all routes to refresh ecmp settings. */ > >> > + FIB_WALK(&p->ip6_rtable, struct babel_entry, e) { > >> > + babel_select_route(p, e, NULL); > >> > + } > >> > + FIB_WALK_END; > >> > + FIB_WALK(&p->ip4_rtable, struct babel_entry, e) { > >> > + babel_select_route(p, e, NULL); > >> > + } > >> > + FIB_WALK_END; > >> > + > >> > >> I don't think this is the right thing to do; you should probably just > >> refuse the reconfiguration if ECMP settings changed (see the 0-return at > >> the top when TOS changes)? > > > > Why not? Works fine AFAICT. I tested the reload stuff and the nexthops get > > updated when the limit changes just as I would expect. > > Because walking the FIB like this is an ugly hack (as you yourself said > in the description)?
Well I really just meant inlining the interations, I hadn't though about the iterator invalidation ascpect actually. > > If I refuse the reconfiguration the user would have to restart bird to > > get this to apply, right? I don't want that. > > No, if you refuse reconfig, bird will automatically bring the interface > down, then up again. This will flush all the neighbours, but I think > that's OK; switching ECMP on and off doesn't have to be a > non-destructive operation. Hmm, that might be acceptable. Given that I might need a babel_route->fib mapping anyway (see below) this might end up coming for free though. > Oh, and when looking at this, I noticed that babel_retract_route() only > conditionally calls babel_select_route(); this is the case in quite a > few places, actually, you'd have to update all of them. Right to do babel_route removal efficiently I'll probably need some more machinery to keep track of which fib route/nexhop a given babel_route maps to. Just iterating over all fib routes to remove one babel_route is definetly too inefficient. --Daniel