Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys Hold KERNEL_LOCK on if_ioctl selectively based on IFEF_M...
details: https://anonhg.NetBSD.org/src/rev/b2571a1363b3
branches: trunk
changeset: 828001:b2571a1363b3
user: ozaki-r <ozaki-r%NetBSD.org@localhost>
date: Wed Nov 22 03:03:18 2017 +0000
description:
Hold KERNEL_LOCK on if_ioctl selectively based on IFEF_MPSAFE
If IFEF_MPSAFE is set, hold the lock and otherwise don't hold.
This change requires additions of KERNEL_LOCK to subsequence functions from
if_ioctl such as ifmedia_ioctl and ifioctl_common to protect non-MP-safe
components.
Proposed on tech-kern@ and tech-net@
diffstat:
sys/kern/sys_socket.c | 12 ++++++++----
sys/net/if.c | 18 ++++++++++++++++--
sys/net/if.h | 16 ++++++++++++++--
sys/net/if_media.c | 24 ++++++++++++++++++++----
sys/net/if_vlan.c | 27 +++++++++++++++++++++------
5 files changed, 79 insertions(+), 18 deletions(-)
diffs (truncated from 313 to 300 lines):
diff -r 513e1bef66f0 -r b2571a1363b3 sys/kern/sys_socket.c
--- a/sys/kern/sys_socket.c Wed Nov 22 02:52:42 2017 +0000
+++ b/sys/kern/sys_socket.c Wed Nov 22 03:03:18 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $ */
+/* $NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $ */
/*-
* Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -61,7 +61,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.74 2016/07/07 06:55:43 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sys_socket.c,v 1.75 2017/11/22 03:03:18 ozaki-r Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -197,14 +197,18 @@
* interface and routing ioctls should have a
* different entry since a socket's unnecessary
*/
- KERNEL_LOCK(1, NULL);
if (IOCGROUP(cmd) == 'i')
+ /*
+ * KERNEL_LOCK will be held later if if_ioctl() of the
+ * interface isn't MP-safe.
+ */
error = ifioctl(so, cmd, data, curlwp);
else {
+ KERNEL_LOCK(1, NULL);
error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so,
cmd, data, NULL);
+ KERNEL_UNLOCK_ONE(NULL);
}
- KERNEL_UNLOCK_ONE(NULL);
break;
}
diff -r 513e1bef66f0 -r b2571a1363b3 sys/net/if.c
--- a/sys/net/if.c Wed Nov 22 02:52:42 2017 +0000
+++ b/sys/net/if.c Wed Nov 22 03:03:18 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if.c,v 1.398 2017/11/19 16:59:25 christos Exp $ */
+/* $NetBSD: if.c,v 1.399 2017/11/22 03:03:18 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.398 2017/11/19 16:59:25 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.399 2017/11/22 03:03:18 ozaki-r Exp $");
#if defined(_KERNEL_OPT)
#include "opt_inet.h"
@@ -2766,6 +2766,11 @@
return 0;
case SIOCSIFFLAGS:
ifr = data;
+ /*
+ * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but if_up
+ * and if_down aren't MP-safe yet, so we must hold the lock.
+ */
+ KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
if (ifp->if_flags & IFF_UP && (ifr->ifr_flags & IFF_UP) == 0) {
s = splsoftnet();
if_down(ifp);
@@ -2776,6 +2781,7 @@
if_up(ifp);
splx(s);
}
+ KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
(ifr->ifr_flags &~ IFF_CANTCHANGE);
break;
@@ -2847,8 +2853,10 @@
* If the link MTU changed, do network layer specific procedure.
*/
#ifdef INET6
+ KERNEL_LOCK_UNLESS_NET_MPSAFE();
if (in6_present)
nd6_setmtu(ifp);
+ KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
#endif
return ENETRESET;
default:
@@ -2992,11 +3000,13 @@
return error;
}
}
+ KERNEL_LOCK_UNLESS_NET_MPSAFE();
mutex_enter(&if_clone_mtx);
r = (cmd == SIOCIFCREATE) ?
if_clone_create(ifr->ifr_name) :
if_clone_destroy(ifr->ifr_name);
mutex_exit(&if_clone_mtx);
+ KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
curlwp_bindx(bound);
return r;
@@ -3054,6 +3064,7 @@
oif_flags = ifp->if_flags;
+ KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp);
mutex_enter(ifp->if_ioctl_lock);
error = (*ifp->if_ioctl)(ifp, cmd, data);
@@ -3062,6 +3073,7 @@
else if (so->so_proto == NULL)
error = EOPNOTSUPP;
else {
+ KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
#ifdef COMPAT_OSOCK
if (vec_compat_ifioctl != NULL)
error = (*vec_compat_ifioctl)(so, ocmd, cmd, data, l);
@@ -3069,6 +3081,7 @@
#endif
error = (*so->so_proto->pr_usrreqs->pr_ioctl)(so,
cmd, data, ifp);
+ KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
}
if (((oif_flags ^ ifp->if_flags) & IFF_UP) != 0) {
@@ -3084,6 +3097,7 @@
#endif
mutex_exit(ifp->if_ioctl_lock);
+ KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp);
out:
if_put(ifp, &psref);
curlwp_bindx(bound);
diff -r 513e1bef66f0 -r b2571a1363b3 sys/net/if.h
--- a/sys/net/if.h Wed Nov 22 02:52:42 2017 +0000
+++ b/sys/net/if.h Wed Nov 22 03:03:18 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if.h,v 1.243 2017/11/17 07:37:12 ozaki-r Exp $ */
+/* $NetBSD: if.h,v 1.244 2017/11/22 03:03:18 ozaki-r Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -391,9 +391,11 @@
#define IFEF_NO_LINK_STATE_CHANGE __BIT(1) /* doesn't use link state interrupts */
/*
- * if_XXX() handlers that IFEF_MPSAFE suppresses KERNEL_LOCK:
+ * The following if_XXX() handlers don't take KERNEL_LOCK if the interface
+ * is set IFEF_MPSAFE:
* - if_start
* - if_output
+ * - if_ioctl
*/
#ifdef _KERNEL
@@ -441,6 +443,16 @@
return ((ifp->if_extflags & IFEF_NO_LINK_STATE_CHANGE) == 0);
}
+#define KERNEL_LOCK_IF_IFP_MPSAFE(ifp) \
+ do { if (if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0)
+#define KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp) \
+ do { if (if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0)
+
+#define KERNEL_LOCK_UNLESS_IFP_MPSAFE(ifp) \
+ do { if (!if_is_mpsafe(ifp)) { KERNEL_LOCK(1, NULL); } } while (0)
+#define KERNEL_UNLOCK_UNLESS_IFP_MPSAFE(ifp) \
+ do { if (!if_is_mpsafe(ifp)) { KERNEL_UNLOCK_ONE(NULL); } } while (0)
+
#ifdef _KERNEL_OPT
#include "opt_net_mpsafe.h"
#endif
diff -r 513e1bef66f0 -r b2571a1363b3 sys/net/if_media.c
--- a/sys/net/if_media.c Wed Nov 22 02:52:42 2017 +0000
+++ b/sys/net/if_media.c Wed Nov 22 03:03:18 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $ */
+/* $NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $ */
/*-
* Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -76,7 +76,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.34 2017/10/23 03:54:40 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_media.c,v 1.35 2017/11/22 03:03:18 ozaki-r Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -225,8 +225,8 @@
/*
* Device-independent media ioctl support function.
*/
-int
-ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
+static int
+_ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
u_long cmd)
{
struct ifmedia_entry *match;
@@ -360,6 +360,22 @@
return error;
}
+int
+ifmedia_ioctl(struct ifnet *ifp, struct ifreq *ifr, struct ifmedia *ifm,
+ u_long cmd)
+{
+ int e;
+
+ /*
+ * If if_is_mpsafe(ifp), KERNEL_LOCK isn't held here, but _ifmedia_ioctl
+ * isn't MP-safe yet, so we must hold the lock.
+ */
+ KERNEL_LOCK_IF_IFP_MPSAFE(ifp);
+ e = _ifmedia_ioctl(ifp, ifr, ifm, cmd);
+ KERNEL_UNLOCK_IF_IFP_MPSAFE(ifp);
+ return e;
+}
+
/*
* Find media entry matching a given ifm word.
*/
diff -r 513e1bef66f0 -r b2571a1363b3 sys/net/if_vlan.c
--- a/sys/net/if_vlan.c Wed Nov 22 02:52:42 2017 +0000
+++ b/sys/net/if_vlan.c Wed Nov 22 03:03:18 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $ */
+/* $NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $ */
/*-
* Copyright (c) 2000, 2001 The NetBSD Foundation, Inc.
@@ -78,7 +78,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.108 2017/11/22 02:35:24 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vlan.c,v 1.109 2017/11/22 03:03:18 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -245,6 +245,16 @@
/* Used to pad ethernet frames with < ETHER_MIN_LEN bytes */
static char vlan_zero_pad_buff[ETHER_MIN_LEN];
+static inline int
+vlan_safe_ifpromisc(struct ifnet *ifp, int pswitch)
+{
+ int e;
+ KERNEL_LOCK_UNLESS_NET_MPSAFE();
+ e = ifpromisc(ifp, pswitch);
+ KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
+ return e;
+}
+
void
vlanattach(int n)
{
@@ -612,13 +622,15 @@
kmem_free(omib, sizeof(*omib));
#ifdef INET6
+ KERNEL_LOCK_UNLESS_NET_MPSAFE();
/* To delete v6 link local addresses */
if (in6_present)
in6_ifdetach(ifp);
+ KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
#endif
if ((ifp->if_flags & IFF_PROMISC) != 0)
- ifpromisc(ifp, 0);
+ vlan_safe_ifpromisc(ifp, 0);
if_down(ifp);
ifp->if_flags &= ~(IFF_UP|IFF_RUNNING);
ifp->if_capabilities = 0;
@@ -834,13 +846,13 @@
if ((ifp->if_flags & IFF_PROMISC) != 0) {
if ((ifv->ifv_flags & IFVF_PROMISC) == 0) {
- error = ifpromisc(mib->ifvm_p, 1);
+ error = vlan_safe_ifpromisc(mib->ifvm_p, 1);
if (error == 0)
ifv->ifv_flags |= IFVF_PROMISC;
}
} else {
if ((ifv->ifv_flags & IFVF_PROMISC) != 0) {
- error = ifpromisc(mib->ifvm_p, 0);
+ error = vlan_safe_ifpromisc(mib->ifvm_p, 0);
if (error == 0)
ifv->ifv_flags &= ~IFVF_PROMISC;
}
@@ -917,7 +929,7 @@
if (mib->ifvm_p != NULL &&
(ifp->if_flags & IFF_PROMISC) != 0)
- error = ifpromisc(mib->ifvm_p, 0);
+ error = vlan_safe_ifpromisc(mib->ifvm_p, 0);
Home |
Main Index |
Thread Index |
Old Index