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