From cba69a875a482d6e68ab11c0e867412f4366457b Mon Sep 17 00:00:00 2001 From: ozaki-r Date: Wed, 31 Dec 2014 17:36:24 +0000 Subject: [PATCH] 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. --- sys/net/bridgestp.c | 42 ++++++------- sys/net/if_bridge.c | 133 ++++++++++++++++++++++++++--------------- sys/net/if_bridgevar.h | 54 ++++++++++++++++- 3 files changed, 159 insertions(+), 70 deletions(-) diff --git a/sys/net/bridgestp.c b/sys/net/bridgestp.c index bcad84c9838a..3915a91bc5cd 100644 --- a/sys/net/bridgestp.c +++ b/sys/net/bridgestp.c @@ -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 (jason@thought.net) @@ -40,7 +40,7 @@ */ #include -__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 #include @@ -221,7 +221,7 @@ bstp_send_config_bpdu(struct bridge_softc *sc, struct bridge_iflist *bif, 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_softc *sc, struct bridge_iflist *bif, 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 *sc) 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 *sc) 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, struct bridge_iflist *bif, struct mbuf *m) 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, struct bridge_iflist *bif, struct mbuf *m) 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 *sc) 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 *sc) } if (mif == NULL) { - BRIDGE_UNLOCK(sc); + BRIDGE_INTR_UNLOCK(sc); bstp_stop(sc); return; } @@ -862,7 +862,7 @@ bstp_initialization(struct bridge_softc *sc) (((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 *sc) 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 *sc) 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); } diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index cc9dad4157fe..2e5d247e5d89 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -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 -__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_lookup_member_if(struct bridge_softc *, 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, int unit) 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 *sc, const char *name) { 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_softc *sc, struct ifnet *member_ifp) 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 *sc, struct bridge_iflist *bif) { 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 *sc, struct bridge_iflist *bif) 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, void *arg) 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 mbuf *m, const struct sockaddr *sa, 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 mbuf *m, const struct sockaddr *sa, 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 mbuf *m, const struct sockaddr *sa, 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 mbuf *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 ifnet *src_if, 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, struct ifnet *src_if, 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); diff --git a/sys/net/if_bridgevar.h b/sys/net/if_bridgevar.h index 46d5b9b2d520..59b0ec3ef2af 100644 --- a/sys/net/if_bridgevar.h +++ b/sys/net/if_bridgevar.h @@ -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 + #include /* @@ -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 *, struct ifnet *, struct mbuf *, #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_ */