Module Name: src
Committed By: ozaki-r
Date: Wed Dec 31 17:36:24 UTC 2014
Modified Files:
src/sys/net: bridgestp.c if_bridge.c if_bridgevar.h
Log Message:
Use pserialize in bridge
This change enables lockless accesses to bridge member lists.
See locking notes in a comment to know how pserialize and
mutexes are used.
This change also provides support for softint-based interrupt
handling; pserialize readers can run in both HW interrupt and
softint contexts.
As usual, pserialize is used only when NET_MPSAFE on.
To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/net/bridgestp.c
cvs rdiff -u -r1.94 -r1.95 src/sys/net/if_bridge.c
cvs rdiff -u -r1.20 -r1.21 src/sys/net/if_bridgevar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/net/bridgestp.c
diff -u src/sys/net/bridgestp.c:1.17 src/sys/net/bridgestp.c:1.18
--- src/sys/net/bridgestp.c:1.17 Mon Jul 14 02:34:36 2014
+++ src/sys/net/bridgestp.c Wed Dec 31 17:36:24 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: bridgestp.c,v 1.17 2014/07/14 02:34:36 ozaki-r Exp $ */
+/* $NetBSD: bridgestp.c,v 1.18 2014/12/31 17:36:24 ozaki-r Exp $ */
/*
* Copyright (c) 2000 Jason L. Wright ([email protected])
@@ -40,7 +40,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: bridgestp.c,v 1.17 2014/07/14 02:34:36 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: bridgestp.c,v 1.18 2014/12/31 17:36:24 ozaki-r Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -221,7 +221,7 @@ bstp_send_config_bpdu(struct bridge_soft
struct bstp_cbpdu bpdu;
int s;
- KASSERT(BRIDGE_LOCKED(sc));
+ KASSERT(BRIDGE_INTR_LOCKED(sc));
ifp = bif->bif_ifp;
@@ -276,11 +276,11 @@ bstp_send_config_bpdu(struct bridge_soft
memcpy(mtod(m, char *) + sizeof(*eh), &bpdu, sizeof(bpdu));
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
s = splnet();
bridge_enqueue(sc, ifp, m, 0);
splx(s);
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
}
static int
@@ -367,7 +367,7 @@ bstp_transmit_tcn(struct bridge_softc *s
struct mbuf *m;
int s;
- KASSERT(BRIDGE_LOCKED(sc));
+ KASSERT(BRIDGE_INTR_LOCKED(sc));
KASSERT(bif != NULL);
ifp = bif->bif_ifp;
@@ -396,11 +396,11 @@ bstp_transmit_tcn(struct bridge_softc *s
memcpy(mtod(m, char *) + sizeof(*eh), &bpdu, sizeof(bpdu));
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
s = splnet();
bridge_enqueue(sc, ifp, m, 0);
splx(s);
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
}
static void
@@ -634,9 +634,9 @@ bstp_input(struct bridge_softc *sc, stru
case BSTP_MSGTYPE_TCN:
tu.tu_message_type = tpdu.tbu_bpdutype;
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
bstp_received_tcn_bpdu(sc, bif, &tu);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
break;
case BSTP_MSGTYPE_CFG:
@@ -675,9 +675,9 @@ bstp_input(struct bridge_softc *sc, stru
cu.cu_topology_change =
(cpdu.cbu_flags & BSTP_FLAG_TC) ? 1 : 0;
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
bstp_received_config_bpdu(sc, bif, &cu);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
break;
default:
@@ -826,7 +826,7 @@ bstp_initialization(struct bridge_softc
mif = NULL;
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
if ((bif->bif_flags & IFBIF_STP) == 0)
@@ -848,7 +848,7 @@ bstp_initialization(struct bridge_softc
}
if (mif == NULL) {
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
bstp_stop(sc);
return;
}
@@ -862,7 +862,7 @@ bstp_initialization(struct bridge_softc
(((uint64_t)(uint8_t)CLLADDR(mif->bif_ifp->if_sadl)[4]) << 8) |
(((uint64_t)(uint8_t)CLLADDR(mif->bif_ifp->if_sadl)[5]) << 0);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
sc->sc_designated_root = sc->sc_bridge_id;
sc->sc_root_path_cost = 0;
@@ -880,7 +880,7 @@ bstp_initialization(struct bridge_softc
callout_reset(&sc->sc_bstpcallout, hz,
bstp_tick, sc);
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
if (bif->bif_flags & IFBIF_STP)
@@ -893,7 +893,7 @@ bstp_initialization(struct bridge_softc
bstp_config_bpdu_generation(sc);
bstp_timer_start(&sc->sc_hello_timer, 0);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
}
void
@@ -901,14 +901,14 @@ bstp_stop(struct bridge_softc *sc)
{
struct bridge_iflist *bif;
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
bstp_set_port_state(bif, BSTP_IFSTATE_DISABLED);
bstp_timer_stop(&bif->bif_hold_timer);
bstp_timer_stop(&bif->bif_message_age_timer);
bstp_timer_stop(&bif->bif_forward_delay_timer);
}
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
callout_stop(&sc->sc_bstpcallout);
@@ -1065,7 +1065,7 @@ bstp_tick(void *arg)
int s;
s = splnet();
- BRIDGE_LOCK(sc);
+ BRIDGE_INTR_LOCK(sc);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
if ((bif->bif_flags & IFBIF_STP) == 0)
@@ -1113,7 +1113,7 @@ bstp_tick(void *arg)
if (sc->sc_if.if_flags & IFF_RUNNING)
callout_reset(&sc->sc_bstpcallout, hz, bstp_tick, sc);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
splx(s);
}
Index: src/sys/net/if_bridge.c
diff -u src/sys/net/if_bridge.c:1.94 src/sys/net/if_bridge.c:1.95
--- src/sys/net/if_bridge.c:1.94 Thu Dec 25 09:10:01 2014
+++ src/sys/net/if_bridge.c Wed Dec 31 17:36:24 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: if_bridge.c,v 1.94 2014/12/25 09:10:01 ozaki-r Exp $ */
+/* $NetBSD: if_bridge.c,v 1.95 2014/12/31 17:36:24 ozaki-r Exp $ */
/*
* Copyright 2001 Wasabi Systems, Inc.
@@ -80,7 +80,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.94 2014/12/25 09:10:01 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.95 2014/12/31 17:36:24 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_bridge_ipf.h"
@@ -223,6 +223,7 @@ static struct bridge_iflist *bridge_look
static void bridge_release_member(struct bridge_softc *, struct bridge_iflist *);
static void bridge_delete_member(struct bridge_softc *,
struct bridge_iflist *);
+static struct bridge_iflist *bridge_try_hold_bif(struct bridge_iflist *);
static int bridge_ioctl_add(struct bridge_softc *, void *);
static int bridge_ioctl_del(struct bridge_softc *, void *);
@@ -368,8 +369,12 @@ bridge_clone_create(struct if_clone *ifc
LIST_INIT(&sc->sc_iflist);
#ifdef BRIDGE_MPSAFE
- sc->sc_iflist_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
+ sc->sc_iflist_intr_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
+ sc->sc_iflist_psz = pserialize_create();
+ sc->sc_iflist_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
#else
+ sc->sc_iflist_intr_lock = NULL;
+ sc->sc_iflist_psz = NULL;
sc->sc_iflist_lock = NULL;
#endif
cv_init(&sc->sc_iflist_cv, "if_bridge_cv");
@@ -422,10 +427,8 @@ bridge_clone_destroy(struct ifnet *ifp)
bridge_stop(ifp, 1);
- BRIDGE_LOCK(sc);
while ((bif = LIST_FIRST(&sc->sc_iflist)) != NULL)
bridge_delete_member(sc, bif);
- BRIDGE_UNLOCK(sc);
mutex_enter(&bridge_list_lock);
LIST_REMOVE(sc, sc_list);
@@ -443,6 +446,11 @@ bridge_clone_destroy(struct ifnet *ifp)
bridge_rtable_fini(sc);
cv_destroy(&sc->sc_iflist_cv);
+ if (sc->sc_iflist_intr_lock)
+ mutex_obj_free(sc->sc_iflist_intr_lock);
+
+ if (sc->sc_iflist_psz)
+ pserialize_destroy(sc->sc_iflist_psz);
if (sc->sc_iflist_lock)
mutex_obj_free(sc->sc_iflist_lock);
@@ -674,25 +682,18 @@ bridge_lookup_member(struct bridge_softc
{
struct bridge_iflist *bif;
struct ifnet *ifp;
+ int s;
- BRIDGE_LOCK(sc);
+ BRIDGE_PSZ_RENTER(s);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
ifp = bif->bif_ifp;
if (strcmp(ifp->if_xname, name) == 0)
break;
}
+ bif = bridge_try_hold_bif(bif);
-#ifdef BRIDGE_MPSAFE
- if (bif != NULL) {
- if (bif->bif_waiting)
- bif = NULL;
- else
- atomic_inc_32(&bif->bif_refs);
- }
-#endif
-
- BRIDGE_UNLOCK(sc);
+ BRIDGE_PSZ_REXIT(s);
return bif;
}
@@ -706,11 +707,21 @@ static struct bridge_iflist *
bridge_lookup_member_if(struct bridge_softc *sc, struct ifnet *member_ifp)
{
struct bridge_iflist *bif;
+ int s;
- BRIDGE_LOCK(sc);
+ BRIDGE_PSZ_RENTER(s);
bif = member_ifp->if_bridgeif;
+ bif = bridge_try_hold_bif(bif);
+
+ BRIDGE_PSZ_REXIT(s);
+
+ return bif;
+}
+static struct bridge_iflist *
+bridge_try_hold_bif(struct bridge_iflist *bif)
+{
#ifdef BRIDGE_MPSAFE
if (bif != NULL) {
if (bif->bif_waiting)
@@ -719,9 +730,6 @@ bridge_lookup_member_if(struct bridge_so
atomic_inc_32(&bif->bif_refs);
}
#endif
-
- BRIDGE_UNLOCK(sc);
-
return bif;
}
@@ -734,12 +742,13 @@ static void
bridge_release_member(struct bridge_softc *sc, struct bridge_iflist *bif)
{
#ifdef BRIDGE_MPSAFE
- atomic_dec_32(&bif->bif_refs);
- membar_sync();
- if (__predict_false(bif->bif_waiting && bif->bif_refs == 0)) {
- BRIDGE_LOCK(sc);
+ uint32_t refs;
+
+ refs = atomic_dec_uint_nv(&bif->bif_refs);
+ if (__predict_false(refs == 0 && bif->bif_waiting)) {
+ BRIDGE_INTR_LOCK(sc);
cv_broadcast(&sc->sc_iflist_cv);
- BRIDGE_UNLOCK(sc);
+ BRIDGE_INTR_UNLOCK(sc);
}
#else
(void)sc;
@@ -757,7 +766,7 @@ bridge_delete_member(struct bridge_softc
{
struct ifnet *ifs = bif->bif_ifp;
- KASSERT(BRIDGE_LOCKED(sc));
+ BRIDGE_LOCK(sc);
ifs->if_input = ether_input;
ifs->if_bridge = NULL;
@@ -765,13 +774,20 @@ bridge_delete_member(struct bridge_softc
LIST_REMOVE(bif, bif_next);
+ BRIDGE_PSZ_PERFORM(sc);
+
+ BRIDGE_UNLOCK(sc);
+
#ifdef BRIDGE_MPSAFE
+ BRIDGE_INTR_LOCK(sc);
bif->bif_waiting = true;
membar_sync();
while (bif->bif_refs > 0) {
aprint_debug("%s: cv_wait on iflist\n", __func__);
- cv_wait(&sc->sc_iflist_cv, sc->sc_iflist_lock);
+ cv_wait(&sc->sc_iflist_cv, sc->sc_iflist_intr_lock);
}
+ bif->bif_waiting = false;
+ BRIDGE_INTR_UNLOCK(sc);
#endif
kmem_free(bif, sizeof(*bif));
@@ -875,15 +891,16 @@ bridge_ioctl_del(struct bridge_softc *sc
return ENOENT;
}
+ BRIDGE_UNLOCK(sc);
+
bridge_delete_member(sc, bif);
- BRIDGE_UNLOCK(sc);
switch (ifs->if_type) {
case IFT_ETHER:
/*
* Take the interface out of promiscuous mode.
- * Don't call it with holding sc_iflist_lock.
+ * Don't call it with holding a spin lock.
*/
(void) ifpromisc(ifs, 0);
break;
@@ -1510,13 +1527,18 @@ bridge_output(struct ifnet *ifp, struct
struct bridge_iflist *bif;
struct mbuf *mc;
int used = 0;
+ int ss;
- BRIDGE_LOCK(sc);
-
+ BRIDGE_PSZ_RENTER(ss);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
+ bif = bridge_try_hold_bif(bif);
+ if (bif == NULL)
+ continue;
+ BRIDGE_PSZ_REXIT(ss);
+
dst_if = bif->bif_ifp;
if ((dst_if->if_flags & IFF_RUNNING) == 0)
- continue;
+ goto next;
/*
* If this is not the original output interface,
@@ -1530,7 +1552,7 @@ bridge_output(struct ifnet *ifp, struct
case BSTP_IFSTATE_BLOCKING:
case BSTP_IFSTATE_LISTENING:
case BSTP_IFSTATE_DISABLED:
- continue;
+ goto next;
}
}
@@ -1541,14 +1563,16 @@ bridge_output(struct ifnet *ifp, struct
mc = m_copym(m, 0, M_COPYALL, M_NOWAIT);
if (mc == NULL) {
sc->sc_if.if_oerrors++;
- continue;
+ goto next;
}
}
bridge_enqueue(sc, dst_if, mc, 0);
+next:
+ bridge_release_member(sc, bif);
+ BRIDGE_PSZ_RENTER(ss);
}
-
- BRIDGE_UNLOCK(sc);
+ BRIDGE_PSZ_REXIT(ss);
if (used == 0)
m_freem(m);
@@ -1824,23 +1848,30 @@ bridge_input(struct ifnet *ifp, struct m
!bstp_state_before_learning(bif)) {
struct bridge_iflist *_bif;
struct ifnet *_ifp = NULL;
+ int s;
- BRIDGE_LOCK(sc);
+ BRIDGE_PSZ_RENTER(s);
LIST_FOREACH(_bif, &sc->sc_iflist, bif_next) {
/* It is destined for us. */
if (bridge_ourether(_bif, eh, 0)) {
+ _bif = bridge_try_hold_bif(_bif);
+ BRIDGE_PSZ_REXIT(s);
+ if (_bif == NULL)
+ goto out;
if (_bif->bif_flags & IFBIF_LEARNING)
(void) bridge_rtupdate(sc,
eh->ether_shost, ifp, 0, IFBAF_DYNAMIC);
_ifp = m->m_pkthdr.rcvif = _bif->bif_ifp;
- break;
+ bridge_release_member(sc, _bif);
+ goto out;
}
/* We just received a packet that we sent out. */
if (bridge_ourether(_bif, eh, 1))
break;
}
- BRIDGE_UNLOCK(sc);
+ BRIDGE_PSZ_REXIT(s);
+out:
if (_bif != NULL) {
bridge_release_member(sc, bif);
@@ -1892,29 +1923,34 @@ bridge_broadcast(struct bridge_softc *sc
struct mbuf *mc;
struct ifnet *dst_if;
bool used, bmcast;
+ int s;
used = bmcast = m->m_flags & (M_BCAST|M_MCAST);
- BRIDGE_LOCK(sc);
-
+ BRIDGE_PSZ_RENTER(s);
LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
+ bif = bridge_try_hold_bif(bif);
+ if (bif == NULL)
+ continue;
+ BRIDGE_PSZ_REXIT(s);
+
dst_if = bif->bif_ifp;
if (dst_if == src_if)
- continue;
+ goto next;
if (bif->bif_flags & IFBIF_STP) {
switch (bif->bif_state) {
case BSTP_IFSTATE_BLOCKING:
case BSTP_IFSTATE_DISABLED:
- continue;
+ goto next;
}
}
if ((bif->bif_flags & IFBIF_DISCOVER) == 0 && !bmcast)
- continue;
+ goto next;
if ((dst_if->if_flags & IFF_RUNNING) == 0)
- continue;
+ goto next;
if (!used && LIST_NEXT(bif, bif_next) == NULL) {
mc = m;
@@ -1923,13 +1959,16 @@ bridge_broadcast(struct bridge_softc *sc
mc = m_copym(m, 0, M_COPYALL, M_DONTWAIT);
if (mc == NULL) {
sc->sc_if.if_oerrors++;
- continue;
+ goto next;
}
}
bridge_enqueue(sc, dst_if, mc, 1);
+next:
+ bridge_release_member(sc, bif);
+ BRIDGE_PSZ_RENTER(s);
}
- BRIDGE_UNLOCK(sc);
+ BRIDGE_PSZ_REXIT(s);
if (bmcast)
ether_input(src_if, m);
Index: src/sys/net/if_bridgevar.h
diff -u src/sys/net/if_bridgevar.h:1.20 src/sys/net/if_bridgevar.h:1.21
--- src/sys/net/if_bridgevar.h:1.20 Mon Jul 14 02:34:36 2014
+++ src/sys/net/if_bridgevar.h Wed Dec 31 17:36:24 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: if_bridgevar.h,v 1.20 2014/07/14 02:34:36 ozaki-r Exp $ */
+/* $NetBSD: if_bridgevar.h,v 1.21 2014/12/31 17:36:24 ozaki-r Exp $ */
/*
* Copyright 2001 Wasabi Systems, Inc.
@@ -207,6 +207,8 @@ struct ifbrparam {
#define ifbrp_filter ifbrp_ifbrpu.ifbrpu_int32 /* filtering flags */
#ifdef _KERNEL
+#include <sys/pserialize.h>
+
#include <net/pktqueue.h>
/*
@@ -303,8 +305,10 @@ struct bridge_softc {
callout_t sc_brcallout; /* bridge callout */
callout_t sc_bstpcallout; /* STP callout */
LIST_HEAD(, bridge_iflist) sc_iflist; /* member interface list */
- kmutex_t *sc_iflist_lock;
+ kmutex_t *sc_iflist_intr_lock;
kcondvar_t sc_iflist_cv;
+ pserialize_t sc_iflist_psz;
+ kmutex_t *sc_iflist_lock;
LIST_HEAD(, bridge_rtnode) *sc_rthash; /* our forwarding table */
LIST_HEAD(, bridge_rtnode) sc_rtlist; /* list version of above */
kmutex_t *sc_rtlist_lock;
@@ -338,5 +342,51 @@ void bridge_enqueue(struct bridge_softc
#define BRIDGE_LOCKED(_sc) (!(_sc)->sc_iflist_lock || \
mutex_owned((_sc)->sc_iflist_lock))
+#define BRIDGE_INTR_LOCK(_sc) if ((_sc)->sc_iflist_intr_lock) \
+ mutex_enter((_sc)->sc_iflist_intr_lock)
+#define BRIDGE_INTR_UNLOCK(_sc) if ((_sc)->sc_iflist_intr_lock) \
+ mutex_exit((_sc)->sc_iflist_intr_lock)
+#define BRIDGE_INTR_LOCKED(_sc) (!(_sc)->sc_iflist_intr_lock || \
+ mutex_owned((_sc)->sc_iflist_intr_lock))
+
+#ifdef BRIDGE_MPSAFE
+/*
+ * These macros can be used in both HW interrupt and softint contexts.
+ */
+#define BRIDGE_PSZ_RENTER(__s) do { \
+ if (!cpu_intr_p()) \
+ __s = pserialize_read_enter(); \
+ else \
+ __s = splhigh(); \
+ } while (0)
+#define BRIDGE_PSZ_REXIT(__s) do { \
+ if (!cpu_intr_p()) \
+ pserialize_read_exit(__s); \
+ else \
+ splx(__s); \
+ } while (0)
+#else /* BRIDGE_MPSAFE */
+#define BRIDGE_PSZ_RENTER(__s) do { __s = 0; } while (0)
+#define BRIDGE_PSZ_REXIT(__s) do { (void)__s; } while (0)
+#endif /* BRIDGE_MPSAFE */
+
+#define BRIDGE_PSZ_PERFORM(_sc) if ((_sc)->sc_iflist_psz) \
+ pserialize_perform((_sc)->sc_iflist_psz);
+
+/*
+ * Locking notes:
+ * - Updates of sc_iflist are serialized by sc_iflist_lock (an adaptive mutex)
+ * - Items of sc_iflist (bridge_iflist) is protected by both pserialize
+ * (sc_iflist_psz) and reference counting (bridge_iflist#bif_refs)
+ * - Before destroying an item of sc_iflist, we have to do pserialize_perform
+ * and synchronize with the reference counting via a conditional variable
+ * (sc_iflist_cz)
+ * - sc_iflist_intr_lock (a spin mutex) is used for the CV
+ * - A spin mutex is required because the reference counting can be used
+ * in HW interrupt context
+ * - The mutex is also used for STP
+ * - Once we change to execute entire Layer 2 in softint context,
+ * we can get rid of sc_iflist_intr_lock
+ */
#endif /* _KERNEL */
#endif /* !_NET_IF_BRIDGEVAR_H_ */
