On 13/12/16(Tue) 17:01, David Gwynne wrote:
> at the moment if ifconfig foo0 up fails, nothing says so because
> the kernel doesnt propagate the return code from the IFFLAGS ioctl
> back to userland.
>
> it also reports a link state change before it actually tries to
> bring the interface up or down.
>
> the diff below shuffles the SIOCSIFFLAGS handling in if.c so itll
> try to run the ioctl first, and only report link state changes if
> IFF_UP changed.
>
> ok?
Are you sure driver ioctl routines do not peak at if_flags?
This is the kind of good change that ends up blowing some crazy part of
the stack.
So if you're around to deal with regressions, ok mpi@ :)
> Index: if.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.464
> diff -u -p -r1.464 if.c
> --- if.c 2 Dec 2016 18:32:38 -0000 1.464
> +++ if.c 13 Dec 2016 06:55:31 -0000
> @@ -1776,20 +1776,26 @@ ifioctl(struct socket *so, u_long cmd, c
> case SIOCSIFFLAGS:
> if ((error = suser(p, 0)) != 0)
> return (error);
> - if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) {
> - s = splnet();
> - if_down(ifp);
> - splx(s);
> +
> + ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
> + (ifr->ifr_flags & ~IFF_CANTCHANGE);
> +
> + if (ifp->if_ioctl != NULL) {
> + error = (*ifp->if_ioctl)(ifp, cmd, data);
> + if (error != 0) {
> + ifp->if_flags = oif_flags;
> + break;
> + }
> }
> - if (ifr->ifr_flags & IFF_UP && (ifp->if_flags & IFF_UP) == 0) {
> +
> + if (ISSET(oif_flags ^ ifp->if_flags, IFF_UP)) {
> s = splnet();
> - if_up(ifp);
> + if (ISSET(ifp->if_flags, IFF_UP))
> + if_up(ifp);
> + else
> + if_down(ifp);
> splx(s);
> }
> - ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
> - (ifr->ifr_flags & ~IFF_CANTCHANGE);
> - if (ifp->if_ioctl)
> - (void) (*ifp->if_ioctl)(ifp, cmd, data);
> break;
>
> case SIOCSIFXFLAGS:
>
