On Sat, Jun 23, 2018 at 10:52:13PM +0200, Denis Fondras wrote:
> Here is a diff to allow deletion of lo(4) created by rdomain.

I think this is a good idea.  Interface and route configuration in
the kernel should be revertabel without reboot.

> rtable is still available after all the interfaces are out of the
> rdomain though.

But then we have rtable without loopback again.  I think removing
the final lo interface from the rdomain should also remove the
rtable.

Can we do that?

> +struct ifnet *
> +rdomain_isused(struct ifnet *ifp, int rdomain)
> +{
> +     struct ifnet *ifp_;

Do we have local variable names ending with _ anywhere else?  In
general the name conflict should be avoided by making the parameter
name more specific.

In this case rdomain_isused() should not be called with any interface.
The rdomain is enough if you ignore its loopback interface.

> +     TAILQ_FOREACH(ifp_, &ifnet, if_list) {
> +             if (ifp_ == ifp)

This should be if (ifp->if_index == rtable_loindex(rdomain))

> +                     continue;
> +             if (ifp_->if_rdomain == rdomain)
> +                     return (ifp_);
> +     }
> +     return (NULL);
> +}

And a ..._isused() function should be boolean, return 1 or 0.

>  int
>  if_setrdomain(struct ifnet *ifp, int rdomain)
>  {
>       struct ifreq ifr;
>       int error, up = 0, s;
> +     struct ifnet *loifp;
> +     char loifname[IFNAMSIZ];
> +     unsigned int unit = rdomain;

The variable unit is not needed, just use rdomain.

>       if (rdomain != ifp->if_rdomain) {
>               if ((ifp->if_flags & IFF_LOOPBACK) &&
> -                 (ifp->if_index == rtable_loindex(ifp->if_rdomain)))
> +                 (ifp->if_index == rtable_loindex(ifp->if_rdomain)) &&
> +                 (rdomain_isused(ifp, ifp->if_rdomain)))
>                       return (EPERM);

I think EBUSY would be better now.  It is not a permission problem
anymore.

bluhm

Reply via email to