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 :)
> 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.
