Re: NET_RLOCK in ifioctl()

2017-11-14 Thread Alexandr Nedvedicky
Hello,

I like this change and I think it should go in.

ok sashan


On Mon, Nov 13, 2017 at 04:54:11PM +0100, Theo Buehler wrote:
> The diff below pushes the NET_LOCK into ifioctl() and reduces it to
> NET_RLOCK where possible. In particular, this will allow SIOCG* requests
> to run in parallel.
> 
> ok?
> 



NET_RLOCK in ifioctl()

2017-11-13 Thread Theo Buehler
The diff below pushes the NET_LOCK into ifioctl() and reduces it to
NET_RLOCK where possible. In particular, this will allow SIOCG* requests
to run in parallel.

ok?

Index: sys/kern/sys_socket.c
===
RCS file: /var/cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.33
diff -u -p -r1.33 sys_socket.c
--- sys/kern/sys_socket.c   11 Aug 2017 21:24:19 -  1.33
+++ sys/kern/sys_socket.c   13 Nov 2017 08:52:44 -
@@ -125,9 +125,7 @@ soo_ioctl(struct file *fp, u_long cmd, c
 * different entry since a socket's unnecessary
 */
if (IOCGROUP(cmd) == 'i') {
-   NET_LOCK();
error = ifioctl(so, cmd, data, p);
-   NET_UNLOCK();
return (error);
}
if (IOCGROUP(cmd) == 'r')
Index: sys/net/if.c
===
RCS file: /var/cvs/src/sys/net/if.c,v
retrieving revision 1.526
diff -u -p -r1.526 if.c
--- sys/net/if.c12 Nov 2017 14:11:15 -  1.526
+++ sys/net/if.c13 Nov 2017 08:52:44 -
@@ -1810,16 +1810,26 @@ ifioctl(struct socket *so, u_long cmd, c
 
switch (cmd) {
case SIOCIFCREATE:
+   if ((error = suser(p, 0)) != 0)
+   return (error);
+   NET_LOCK();
+   error = if_clone_create(ifr->ifr_name, 0);
+   NET_UNLOCK();
+   return (error);
case SIOCIFDESTROY:
if ((error = suser(p, 0)) != 0)
return (error);
-   return ((cmd == SIOCIFCREATE) ?
-   if_clone_create(ifr->ifr_name, 0) :
-   if_clone_destroy(ifr->ifr_name));
+   NET_LOCK();
+   error = if_clone_destroy(ifr->ifr_name);
+   NET_UNLOCK();
+   return (error);
case SIOCSIFGATTR:
if ((error = suser(p, 0)) != 0)
return (error);
-   return (if_setgroupattribs(data));
+   NET_LOCK();
+   error = if_setgroupattribs(data);
+   NET_UNLOCK();
+   return (error);
case SIOCGIFCONF:
case SIOCIFGCLONERS:
case SIOCGIFGMEMB:
@@ -1845,6 +1855,8 @@ ifioctl(struct socket *so, u_long cmd, c
oif_flags = ifp->if_flags;
oif_xflags = ifp->if_xflags;
 
+   NET_LOCK();
+
switch (cmd) {
case SIOCIFAFATTACH:
case SIOCIFAFDETACH:
@@ -2093,6 +2105,8 @@ ifioctl(struct socket *so, u_long cmd, c
if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0)
getmicrotime(>if_lastchange);
 
+   NET_UNLOCK();
+
return (error);
 }
 
@@ -2109,19 +2123,33 @@ ifioctl_get(u_long cmd, caddr_t data)
 
switch(cmd) {
case SIOCGIFCONF:
-   return (ifconf(data));
+   NET_RLOCK();
+   error = ifconf(data);
+   NET_RUNLOCK();
+   return (error);
case SIOCIFGCLONERS:
-   return (if_clone_list((struct if_clonereq *)data));
+   NET_RLOCK();
+   error = if_clone_list((struct if_clonereq *)data);
+   NET_RUNLOCK();
+   return (error);
case SIOCGIFGMEMB:
-   return (if_getgroupmembers(data));
+   NET_RLOCK();
+   error = if_getgroupmembers(data);
+   NET_RUNLOCK();
+   return (error);
case SIOCGIFGATTR:
-   return (if_getgroupattribs(data));
+   NET_RLOCK();
+   error = if_getgroupattribs(data);
+   NET_RUNLOCK();
+   return (error);
}
 
ifp = ifunit(ifr->ifr_name);
if (ifp == NULL)
return (ENXIO);
 
+   NET_RLOCK();
+
switch(cmd) {
case SIOCGIFFLAGS:
ifr->ifr_flags = ifp->if_flags;
@@ -2187,6 +2215,8 @@ ifioctl_get(u_long cmd, caddr_t data)
default:
panic("invalid ioctl %lu", cmd);
}
+
+   NET_RUNLOCK();
 
return (error);
 }
Index: sys/nfs/nfs_boot.c
===
RCS file: /var/cvs/src/sys/nfs/nfs_boot.c,v
retrieving revision 1.43
diff -u -p -r1.43 nfs_boot.c
--- sys/nfs/nfs_boot.c  11 Aug 2017 21:24:20 -  1.43
+++ sys/nfs/nfs_boot.c  13 Nov 2017 08:52:44 -
@@ -159,15 +159,11 @@ nfs_boot_init(struct nfs_diskless *nd, s
 */
if ((error = socreate(AF_INET, , SOCK_DGRAM, 0)) != 0)
panic("nfs_boot: socreate, error=%d", error);
-   NET_LOCK();
error = ifioctl(so, SIOCGIFFLAGS, (caddr_t), procp);
-   NET_UNLOCK();
if (error)
panic("nfs_boot: GIFFLAGS, error=%d", error);
ireq.ifr_flags |= IFF_UP;
-   NET_LOCK();
error = ifioctl(so,