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.

Reply via email to