On 15/08/16(Mon) 10:25, Claudio Jeker wrote: > On Mon, Aug 15, 2016 at 08:42:06AM +0200, Martin Pieuchot wrote: > > On 08/08/16(Mon) 11:48, Martin Pieuchot wrote: > > > The rtable_walk() & prio bug I just sent a fix for should theoretically > > > not cause any trouble. Sadly it is piled on top of another bug for > > > which a fix is attached. > > > > > > When an interface is removed the current code starts by purging all its > > > corresponding route entries. This is wrong because the per-AF code has > > > some knowledge of which automagic route should be removed first. > > > > > > In other words, the rtable_walk() hang should never have been triggered > > > because the IPv4-specific code should take care of removing the > > > RTF_BROADCAST entry. I believe that this ordering problem is the reason > > > why error code are ignored in AF-specific code paths. > > > > > > Diff attached fixes that, ok? > > > > Anyone? > > > > IMO this is a bit of a catch-22 here. Until recently we did store less > interface specific stuff in the routing table and so it made sense to > clean up the routes first (the assumption was that this will first remove > all gateway routes and probably also the arp/ndp caches).
This was done when no RTF_LOCAL/RTF_BROADCAST existed. > This makes > removing of the interface specific stuff simpler because the upper layers > are already clean. How does it make it simpler? > Now we turn this upside down but then we may get burned > because gateway routes are cleaned too late and put the routing table into > an inconsitent state. Which inconsistent state? Cached route are still referenced. > I know you removed a fair amount of references that > would cause crashes because of bad pointers. Need to update my mental > model of the rtable / ifa / ifp interaction to see if this is indeed not > possible. Sure, you need to look at in_ifinit() and in_purgeaddr(). Currently rt_ifa_dellocal() and the rt_ifa_del() are noop and return errors. That's why errors are not checked. > At the same time I wonder if we need multiple walks to make this > save and easier to understand... (replacing dragons with other smaller > dragons is not that helpful because they grow over time). I'm not sure to understand. > I try to find time tonight to look into this. Thanks.