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