On Mon, Dec 18, 2017 at 04:01:12PM +0100, Martin Pieuchot wrote:
> Diff below moves the NET_LOCK() into every switch case that need it and
> document locking for most of the fields of 'struct ifnet'.
> case SIOCSIFXFLAGS:
> if ((error = suser(p, 0)) != 0)
> break;
>
> + NET_LOCK();
> #ifdef INET6
> if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) {
> error = in6_ifattach(ifp);
Just a line below is an "if (error != 0) break;". There the unlock
is missing.
> case SIOCSIFMTU:
> if ((error = suser(p, 0)) != 0)
> break;
> + NET_LOCK();
> error = (*ifp->if_ioctl)(ifp, cmd, data);
> + NET_UNLOCK();
> if (!error)
> rtm_ifchg(ifp);
> break;
rtm_ifchg() calls route_input() which needs the net lock.
> @@ -2103,8 +2118,6 @@ ifioctl(struct socket *so, u_long cmd, c
>
> if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0)
> getmicrotime(&ifp->if_lastchange);
> -
> - NET_UNLOCK();
>
> return (error);
> }
May we access the ifp->if_flags without net lock? As we want to
compare them to their value before we changed them, would it be
even better to put the "oif_flags = ifp->if_flags;" under the lock?
What is the advantage to move the net lock into the cases?
bluhm
