Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/net Revert "Make sure to hold if_ioctl_lock when calling...



details:   https://anonhg.NetBSD.org/src/rev/d84fb782d152
branches:  trunk
changeset: 822771:d84fb782d152
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Thu Apr 06 03:54:59 2017 +0000

description:
Revert "Make sure to hold if_ioctl_lock when calling ifp->if_ioctl"

As per pgoyette@ and riastradh@ requests; we shouldn't decide to
hold a lock based on if the lock is held or not.

diffstat:

 sys/net/if.c           |  71 ++++++-------------------------------------------
 sys/net/if.h           |   3 +-
 sys/net/if_ethersubr.c |   6 +--
 sys/net/link_proto.c   |   7 +---
 4 files changed, 14 insertions(+), 73 deletions(-)

diffs (248 lines):

diff -r 304041504d35 -r d84fb782d152 sys/net/if.c
--- a/sys/net/if.c      Thu Apr 06 03:21:01 2017 +0000
+++ b/sys/net/if.c      Thu Apr 06 03:54:59 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.c,v 1.390 2017/04/05 03:47:51 ozaki-r Exp $ */
+/*     $NetBSD: if.c,v 1.391 2017/04/06 03:54:59 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.390 2017/04/05 03:47:51 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.391 2017/04/06 03:54:59 ozaki-r Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -2496,15 +2496,8 @@
 int
 ifpromisc(struct ifnet *ifp, int pswitch)
 {
-       int pcount, ret = 0;
+       int pcount, ret;
        short nflags;
-       bool need_unlock = false;
-
-       /* XXX if_ioctl_lock may or may not be held here */
-       if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
-               mutex_enter(ifp->if_ioctl_lock);
-               need_unlock = true;
-       }
 
        pcount = ifp->if_pcount;
        if (pswitch) {
@@ -2514,11 +2507,11 @@
                 * consult IFF_PROMISC when it is brought up.
                 */
                if (ifp->if_pcount++ != 0)
-                       goto out;
+                       return 0;
                nflags = ifp->if_flags | IFF_PROMISC;
        } else {
                if (--ifp->if_pcount > 0)
-                       goto out;
+                       return 0;
                nflags = ifp->if_flags & ~IFF_PROMISC;
        }
        ret = if_flags_set(ifp, nflags);
@@ -2526,10 +2519,6 @@
        if (ret != 0) {
                ifp->if_pcount = pcount;
        }
-
-out:
-       if (need_unlock)
-               mutex_exit(ifp->if_ioctl_lock);
        return ret;
 }
 
@@ -2690,8 +2679,6 @@
        struct ifcapreq *ifcr;
        struct ifdatareq *ifdr;
 
-       KASSERT(if_ioctl_locked(ifp));
-
        switch (cmd) {
        case SIOCSIFCAP:
                ifcr = data;
@@ -3069,15 +3056,6 @@
        return error;
 }
 
-bool
-if_ioctl_locked(struct ifnet *ifp)
-{
-
-       KASSERT(ifp->if_ioctl_lock != NULL);
-
-       return mutex_owned(ifp->if_ioctl_lock);
-}
-
 /*
  * Return interface configuration
  * of system.  List may be used
@@ -3321,23 +3299,14 @@
 if_addr_init(ifnet_t *ifp, struct ifaddr *ifa, const bool src)
 {
        int rc;
-       bool need_unlock = false;
-
-       /* XXX if_ioctl_lock may or may not be held here */
-       if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
-               mutex_enter(ifp->if_ioctl_lock);
-               need_unlock = true;
-       }
 
        if (ifp->if_initaddr != NULL)
                rc = (*ifp->if_initaddr)(ifp, ifa, src);
        else if (src ||
+               /* FIXME: may not hold if_ioctl_lock */
                 (rc = (*ifp->if_ioctl)(ifp, SIOCSIFDSTADDR, ifa)) == ENOTTY)
                rc = (*ifp->if_ioctl)(ifp, SIOCINITIFADDR, ifa);
 
-       if (need_unlock)
-               mutex_exit(ifp->if_ioctl_lock);
-
        return rc;
 }
 
@@ -3378,13 +3347,6 @@
 if_flags_set(ifnet_t *ifp, const short flags)
 {
        int rc;
-       bool need_unlock = false;
-
-       /* XXX if_ioctl_lock may or may not be held here */
-       if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
-               mutex_enter(ifp->if_ioctl_lock);
-               need_unlock = true;
-       }
 
        if (ifp->if_setflags != NULL)
                rc = (*ifp->if_setflags)(ifp, flags);
@@ -3402,24 +3364,19 @@
                  * setting/clearing only IFF_PROMISC if the interface
                  * isn't IFF_UP.  Uphold that tradition.
                 */
-               if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0) {
-                       rc = 0;
-                       goto out;
-               }
+               if (chgdflags == IFF_PROMISC && (ifp->if_flags & IFF_UP) == 0)
+                       return 0;
 
                memset(&ifr, 0, sizeof(ifr));
 
                ifr.ifr_flags = flags & ~IFF_CANTCHANGE;
+               /* FIXME: may not hold if_ioctl_lock */
                rc = (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, &ifr);
 
                if (rc != 0 && cantflags != 0)
                        ifp->if_flags ^= cantflags;
        }
 
-out:
-       if (need_unlock)
-               mutex_exit(ifp->if_ioctl_lock);
-
        return rc;
 }
 
@@ -3428,13 +3385,6 @@
 {
        int rc;
        struct ifreq ifr;
-       bool need_unlock = false;
-
-       /* XXX if_ioctl_lock may or may not be held here */
-       if (ifp->if_ioctl_lock != NULL && !mutex_owned(ifp->if_ioctl_lock)) {
-               mutex_enter(ifp->if_ioctl_lock);
-               need_unlock = true;
-       }
 
        if (ifp->if_mcastop != NULL)
                rc = (*ifp->if_mcastop)(ifp, cmd, sa);
@@ -3443,9 +3393,6 @@
                rc = (*ifp->if_ioctl)(ifp, cmd, &ifr);
        }
 
-       if (need_unlock)
-               mutex_exit(ifp->if_ioctl_lock);
-
        return rc;
 }
 
diff -r 304041504d35 -r d84fb782d152 sys/net/if.h
--- a/sys/net/if.h      Thu Apr 06 03:21:01 2017 +0000
+++ b/sys/net/if.h      Thu Apr 06 03:54:59 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.237 2017/04/05 03:47:51 ozaki-r Exp $ */
+/*     $NetBSD: if.h,v 1.238 2017/04/06 03:54:59 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -968,7 +968,6 @@
 int    if_mcast_op(ifnet_t *, const unsigned long, const struct sockaddr *);
 int    if_flags_set(struct ifnet *, const short);
 int    if_clone_list(int, char *, int *);
-bool   if_ioctl_locked(struct ifnet *);
 
 struct ifnet *ifunit(const char *);
 struct ifnet *if_get(const char *, struct psref *);
diff -r 304041504d35 -r d84fb782d152 sys/net/if_ethersubr.c
--- a/sys/net/if_ethersubr.c    Thu Apr 06 03:21:01 2017 +0000
+++ b/sys/net/if_ethersubr.c    Thu Apr 06 03:54:59 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $       */
+/*     $NetBSD: if_ethersubr.c,v 1.242 2017/04/06 03:54:59 ozaki-r Exp $       */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.241 2017/04/05 03:47:51 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.242 2017/04/06 03:54:59 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1351,8 +1351,6 @@
        static const uint8_t zero[ETHER_ADDR_LEN];
        int error;
 
-       KASSERT(if_ioctl_locked(ifp));
-
        switch (cmd) {
        case SIOCINITIFADDR:
            {
diff -r 304041504d35 -r d84fb782d152 sys/net/link_proto.c
--- a/sys/net/link_proto.c      Thu Apr 06 03:21:01 2017 +0000
+++ b/sys/net/link_proto.c      Thu Apr 06 03:54:59 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $  */
+/*     $NetBSD: link_proto.c,v 1.36 2017/04/06 03:55:00 ozaki-r Exp $  */
 
 /*-
  * Copyright (c) 1982, 1986, 1993
@@ -32,7 +32,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.35 2017/04/05 03:47:51 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: link_proto.c,v 1.36 2017/04/06 03:55:00 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/socket.h>
@@ -143,9 +143,6 @@
        const struct sockaddr_dl *asdl, *nsdl;
        struct psref psref;
 
-       if (ifp != NULL)
-               KASSERT(if_ioctl_locked(ifp));
-
        switch (cmd) {
        case SIOCALIFADDR:
        case SIOCDLIFADDR:



Home | Main Index | Thread Index | Old Index