[src/trunk]: src/sys/net Make bridge MPSAFE

[ozaki-r <ozaki-r%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/73ce265aaa65
branches:  trunk
changeset: 330561:73ce265aaa65
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Mon Jul 14 02:34:36 2014 +0000

description:
Make bridge MPSAFE

- Introduce BRIDGE_MPSAFE
  - It's enabled only when NET_MPSAFE is defined
    in if.h or the kernel config
- Add iflist and rtlist mutex locks
  - Locking iflist is performance sensitive,
    so it's not used when !BRIDGE_MPSAFE
- Add bif object reference counting
  - It enables fine-grain locking for bridge member lists
    by allowing to not hold a lock during touching a bif
  - bridge_release_member is added to decrement the
    reference count
  - A condition variable is added to do bridge_delete_member
    gracefully
- Add if_bridgeif to ifnet
  - It's a shortcut to a bif object of a bridge member
  - It reduces a bif lookup cost and so lock contention on iflist
- Make bridgestp MPSAFE too

diffstat:

 sys/net/bridgestp.c    |   39 +++++-
 sys/net/if.h           |    3 +-
 sys/net/if_bridge.c    |  347 +++++++++++++++++++++++++++++++++++++++---------
 sys/net/if_bridgevar.h |   20 ++-
 4 files changed, 339 insertions(+), 70 deletions(-)

diffs (truncated from 1127 to 300 lines):

diff -r 502c425df029 -r 73ce265aaa65 sys/net/bridgestp.c
--- a/sys/net/bridgestp.c       Mon Jul 14 01:24:58 2014 +0000
+++ b/sys/net/bridgestp.c       Mon Jul 14 02:34:36 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: bridgestp.c,v 1.16 2014/06/18 01:19:19 ozaki-r Exp $   */
+/*     $NetBSD: bridgestp.c,v 1.17 2014/07/14 02:34:36 ozaki-r Exp $   */
 
 /*
  * Copyright (c) 2000 Jason L. Wright (jason%thought.net@localhost)
@@ -40,7 +40,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: bridgestp.c,v 1.16 2014/06/18 01:19:19 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: bridgestp.c,v 1.17 2014/07/14 02:34:36 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -221,6 +221,8 @@
        struct bstp_cbpdu bpdu;
        int s;
 
+       KASSERT(BRIDGE_LOCKED(sc));
+
        ifp = bif->bif_ifp;
 
        if ((ifp->if_flags & IFF_RUNNING) == 0)
@@ -274,9 +276,11 @@
 
        memcpy(mtod(m, char *) + sizeof(*eh), &bpdu, sizeof(bpdu));
 
+       BRIDGE_UNLOCK(sc);
        s = splnet();
        bridge_enqueue(sc, ifp, m, 0);
        splx(s);
+       BRIDGE_LOCK(sc);
 }
 
 static int
@@ -363,6 +367,8 @@
        struct mbuf *m;
        int s;
 
+       KASSERT(BRIDGE_LOCKED(sc));
+
        KASSERT(bif != NULL);
        ifp = bif->bif_ifp;
        if ((ifp->if_flags & IFF_RUNNING) == 0)
@@ -390,9 +396,11 @@
 
        memcpy(mtod(m, char *) + sizeof(*eh), &bpdu, sizeof(bpdu));
 
+       BRIDGE_UNLOCK(sc);
        s = splnet();
        bridge_enqueue(sc, ifp, m, 0);
        splx(s);
+       BRIDGE_LOCK(sc);
 }
 
 static void
@@ -592,6 +600,10 @@
        struct bstp_tcn_unit tu;
        uint16_t len;
 
+#ifdef BRIDGE_MPSAFE
+       KASSERT(bif->bif_refs > 0);
+#endif
+
        eh = mtod(m, struct ether_header *);
 
        if ((bif->bif_flags & IFBIF_STP) == 0)
@@ -621,7 +633,11 @@
        switch (tpdu.tbu_bpdutype) {
        case BSTP_MSGTYPE_TCN:
                tu.tu_message_type = tpdu.tbu_bpdutype;
+
+               BRIDGE_LOCK(sc);
                bstp_received_tcn_bpdu(sc, bif, &tu);
+               BRIDGE_UNLOCK(sc);
+
                break;
        case BSTP_MSGTYPE_CFG:
                if (m->m_len < sizeof(cpdu) &&
@@ -658,7 +674,11 @@
                    (cpdu.cbu_flags & BSTP_FLAG_TCA) ? 1 : 0;
                cu.cu_topology_change =
                    (cpdu.cbu_flags & BSTP_FLAG_TC) ? 1 : 0;
+
+               BRIDGE_LOCK(sc);
                bstp_received_config_bpdu(sc, bif, &cu);
+               BRIDGE_UNLOCK(sc);
+
                break;
        default:
                goto out;
@@ -805,6 +825,9 @@
        struct bridge_iflist *bif, *mif;
 
        mif = NULL;
+
+       BRIDGE_LOCK(sc);
+
        LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
                if ((bif->bif_flags & IFBIF_STP) == 0)
                        continue;
@@ -823,7 +846,9 @@
                        continue;
                }
        }
+
        if (mif == NULL) {
+               BRIDGE_UNLOCK(sc);
                bstp_stop(sc);
                return;
        }
@@ -837,6 +862,8 @@
            (((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);
+
        sc->sc_designated_root = sc->sc_bridge_id;
        sc->sc_root_path_cost = 0;
        sc->sc_root_port = NULL;
@@ -853,6 +880,8 @@
                callout_reset(&sc->sc_bstpcallout, hz,
                    bstp_tick, sc);
 
+       BRIDGE_LOCK(sc);
+
        LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
                if (bif->bif_flags & IFBIF_STP)
                        bstp_enable_port(sc, bif);
@@ -863,6 +892,8 @@
        bstp_port_state_selection(sc);
        bstp_config_bpdu_generation(sc);
        bstp_timer_start(&sc->sc_hello_timer, 0);
+
+       BRIDGE_UNLOCK(sc);
 }
 
 void
@@ -870,12 +901,14 @@
 {
        struct bridge_iflist *bif;
 
+       BRIDGE_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);
 
        callout_stop(&sc->sc_bstpcallout);
 
@@ -1032,6 +1065,7 @@
        int s;
 
        s = splnet();
+       BRIDGE_LOCK(sc);
 
        LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
                if ((bif->bif_flags & IFBIF_STP) == 0)
@@ -1079,6 +1113,7 @@
        if (sc->sc_if.if_flags & IFF_RUNNING)
                callout_reset(&sc->sc_bstpcallout, hz, bstp_tick, sc);
 
+       BRIDGE_UNLOCK(sc);
        splx(s);
 }
 
diff -r 502c425df029 -r 73ce265aaa65 sys/net/if.h
--- a/sys/net/if.h      Mon Jul 14 01:24:58 2014 +0000
+++ b/sys/net/if.h      Mon Jul 14 02:34:36 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if.h,v 1.170 2014/07/01 14:04:40 ozaki-r Exp $ */
+/*     $NetBSD: if.h,v 1.171 2014/07/14 02:34:36 ozaki-r Exp $ */
 
 /*-
  * Copyright (c) 1999, 2000, 2001 The NetBSD Foundation, Inc.
@@ -290,6 +290,7 @@
        struct ifaddr   *if_hwdl;
        const uint8_t *if_broadcastaddr;/* linklevel broadcast bytestring */
        void    *if_bridge;             /* bridge glue */
+       void    *if_bridgeif;           /* shortcut to interface list entry */
        int     if_dlt;                 /* data link type (<net/dlt.h>) */
        pfil_head_t *   if_pfil;        /* filtering point */
        uint64_t if_capabilities;       /* interface capabilities */
diff -r 502c425df029 -r 73ce265aaa65 sys/net/if_bridge.c
--- a/sys/net/if_bridge.c       Mon Jul 14 01:24:58 2014 +0000
+++ b/sys/net/if_bridge.c       Mon Jul 14 02:34:36 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_bridge.c,v 1.86 2014/07/02 09:51:08 ozaki-r Exp $   */
+/*     $NetBSD: if_bridge.c,v 1.87 2014/07/14 02:34:36 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.86 2014/07/02 09:51:08 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.87 2014/07/14 02:34:36 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_bridge_ipf.h"
@@ -219,6 +219,7 @@
                                                  const char *name);
 static struct bridge_iflist *bridge_lookup_member_if(struct bridge_softc *,
                                                     struct ifnet *ifp);
+static void    bridge_release_member(struct bridge_softc *, struct bridge_iflist *);
 static void    bridge_delete_member(struct bridge_softc *,
                                     struct bridge_iflist *);
 
@@ -365,6 +366,12 @@
        callout_init(&sc->sc_bstpcallout, 0);
 
        LIST_INIT(&sc->sc_iflist);
+#ifdef BRIDGE_MPSAFE
+       sc->sc_iflist_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
+#else
+       sc->sc_iflist_lock = NULL;
+#endif
+       cv_init(&sc->sc_iflist_cv, "if_bridge_cv");
 
        if_initname(ifp, ifc->ifc_name, unit);
        ifp->if_softc = sc;
@@ -414,8 +421,10 @@
 
        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);
@@ -432,6 +441,10 @@
        /* Tear down the routing table. */
        bridge_rtable_fini(sc);
 
+       cv_destroy(&sc->sc_iflist_cv);
+       if (sc->sc_iflist_lock)
+               mutex_obj_free(sc->sc_iflist_lock);
+
        free(sc, M_DEVBUF);
 
        return (0);
@@ -648,7 +661,7 @@
 /*
  * bridge_lookup_member:
  *
- *     Lookup a bridge member interface.  Must be called at splnet().
+ *     Lookup a bridge member interface.
  */
 static struct bridge_iflist *
 bridge_lookup_member(struct bridge_softc *sc, const char *name)
@@ -656,31 +669,76 @@
        struct bridge_iflist *bif;
        struct ifnet *ifp;
 
+       BRIDGE_LOCK(sc);
+
        LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
                ifp = bif->bif_ifp;
                if (strcmp(ifp->if_xname, name) == 0)
-                       return (bif);
+                       break;
        }
 
-       return (NULL);
+#ifdef BRIDGE_MPSAFE
+       if (bif != NULL) {
+               if (bif->bif_waiting)
+                       bif = NULL;
+               else
+                       atomic_inc_32(&bif->bif_refs);
+       }
+#endif
+
+       BRIDGE_UNLOCK(sc);
+
+       return bif;
 }
 
 /*
  * bridge_lookup_member_if:
  *
- *     Lookup a bridge member interface by ifnet*.  Must be called at splnet().
+ *     Lookup a bridge member interface by ifnet*.
  */
 static struct bridge_iflist *
 bridge_lookup_member_if(struct bridge_softc *sc, struct ifnet *member_ifp)
 {
        struct bridge_iflist *bif;
 
-       LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {