Martin Pieuchot(m...@openbsd.org) on 2015.09.13 16:08:50 +0200: > On 13/09/15(Sun) 15:51, Alexander Bluhm wrote: > > On Sun, Sep 13, 2015 at 11:15:50AM +0200, Martin Pieuchot wrote: > > > This makes the kernel simpler as it no longer try to find a new ifa > > > when a route with a stale address is being used. > > > > This makes the code simpler, which is good. > > > > I am still not convinced that we want to loose the feature that the > > routes jump to another interface address. When we have multiple > > suiteable addresses and one gets deleted, the system can use another > > one. > > This is the price to pay for making the code simpler. I strongly > believe this "feature" is a side effect of history that should not > have been added in the first place. > > However I'd like to fix potential issues with this diff before committing > it, so tests are welcome :)
Hi, i found the time to play with your diff. On a router (where you probably wouldn't to this operationaly) you get tons of "route xxx vanished before delete" from bgpd and ospfd, but they continue to work, apparently as intended. I havent found a box with pppoe to test. it would be nice if someone could test that. as far as it goes, ok from me. /Benno > > The patch itself looks correct. Just one question: > > > > > @@ -850,22 +850,10 @@ rtrequest1(int req, struct rt_addrinfo * > > > return (EINVAL); > > > if ((rt->rt_flags & RTF_CLONING) == 0) > > > return (EINVAL); > > > - if (rt->rt_ifa->ifa_ifp) { > > > - info->rti_ifa = rt->rt_ifa; > > > - } else { > > > - /* > > > - * The address of the cloning route is not longer > > > - * configured on an interface, but its descriptor > > > - * is still there because of reference counting. > > > - * > > > - * Try to find a similar active address and use > > > - * it for the cloned route. The cloning route > > > - * will get the new address and interface later. > > > - */ > > > - info->rti_ifa = NULL; > > > - info->rti_info[RTAX_IFA] = rt->rt_ifa->ifa_addr; > > > - } > > > + if (rt->rt_ifa->ifa_ifp == NULL) > > > + return (EAGAIN); > > > > Why return EAGAIN here? Should it be EINVAL like in the other > > cases? Can this happen at all? > > This should never happen but I'd like to take a safe approach and be > able to differentiate the error code if this still happens. I'd > happily turn this into a KASSERT() but not right now. +1