Source-Changes-HG archive

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

[src/netbsd-8]: src Pull up following revision(s) (requested by ozaki-r in ti...



details:   https://anonhg.NetBSD.org/src/rev/5c8d8f779fe4
branches:  netbsd-8
changeset: 434937:5c8d8f779fe4
user:      martin <martin%NetBSD.org@localhost>
date:      Wed Apr 18 14:11:42 2018 +0000

description:
Pull up following revision(s) (requested by ozaki-r in ticket #777):

        tests/net/if_bridge/t_rtable.sh: revision 1.3
        sys/net/if_bridge.c: revision 1.150-1.154
        sys/net/if_bridgevar.h: revision 1.32

Remove obsolete NULL checks

Simplify bridge_rtnode_insert (NFC)

bridge: use pslist(9) for rtlist and rthash

The change fixes race conditions on list operations.  One example is that a
reader may see invalid pointers on a looking item in a list due to lack of
membar_producer.

Add a test that checks if brconfig flush surely removes all entries

Get rid of a unnecessary semicolon
Pointed out by kamil@

Add missing PSLIST_ENTRY_INIT and PSLIST_ENTRY_DESTROY

diffstat:

 sys/net/if_bridge.c             |  113 +++++++++++++++++++++------------------
 sys/net/if_bridgevar.h          |   10 +-
 tests/net/if_bridge/t_rtable.sh |   31 ++++++++++-
 3 files changed, 97 insertions(+), 57 deletions(-)

diffs (truncated from 310 to 300 lines):

diff -r fc99c8e17e5b -r 5c8d8f779fe4 sys/net/if_bridge.c
--- a/sys/net/if_bridge.c       Wed Apr 18 14:06:24 2018 +0000
+++ b/sys/net/if_bridge.c       Wed Apr 18 14:11:42 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_bridge.c,v 1.134.6.8 2018/04/10 11:48:29 martin Exp $       */
+/*     $NetBSD: if_bridge.c,v 1.134.6.9 2018/04/18 14:11:42 martin Exp $       */
 
 /*
  * Copyright 2001 Wasabi Systems, Inc.
@@ -80,7 +80,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.134.6.8 2018/04/10 11:48:29 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_bridge.c,v 1.134.6.9 2018/04/18 14:11:42 martin Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_bridge_ipf.h"
@@ -181,20 +181,39 @@
 #define        BRIDGE_RTABLE_PRUNE_PERIOD      (5 * 60)
 #endif
 
-#define BRIDGE_RT_LOCK(_sc)    if ((_sc)->sc_rtlist_lock) \
-                                       mutex_enter((_sc)->sc_rtlist_lock)
-#define BRIDGE_RT_UNLOCK(_sc)  if ((_sc)->sc_rtlist_lock) \
-                                       mutex_exit((_sc)->sc_rtlist_lock)
-#define BRIDGE_RT_LOCKED(_sc)  (!(_sc)->sc_rtlist_lock || \
-                                mutex_owned((_sc)->sc_rtlist_lock))
+#define BRIDGE_RT_LOCK(_sc)    mutex_enter((_sc)->sc_rtlist_lock)
+#define BRIDGE_RT_UNLOCK(_sc)  mutex_exit((_sc)->sc_rtlist_lock)
+#define BRIDGE_RT_LOCKED(_sc)  mutex_owned((_sc)->sc_rtlist_lock)
 
 #define BRIDGE_RT_PSZ_PERFORM(_sc) \
-                               if ((_sc)->sc_rtlist_psz != NULL) \
-                                       pserialize_perform((_sc)->sc_rtlist_psz);
+                               pserialize_perform((_sc)->sc_rtlist_psz)
 
 #define BRIDGE_RT_RENTER(__s)  do { __s = pserialize_read_enter(); } while (0)
 #define BRIDGE_RT_REXIT(__s)   do { pserialize_read_exit(__s); } while (0)
 
+#define BRIDGE_RTLIST_READER_FOREACH(_brt, _sc)                        \
+       PSLIST_READER_FOREACH((_brt), &((_sc)->sc_rtlist),              \
+           struct bridge_rtnode, brt_list)
+#define BRIDGE_RTLIST_WRITER_FOREACH(_brt, _sc)                        \
+       PSLIST_WRITER_FOREACH((_brt), &((_sc)->sc_rtlist),              \
+           struct bridge_rtnode, brt_list)
+#define BRIDGE_RTLIST_WRITER_INSERT_HEAD(_sc, _brt)                    \
+       PSLIST_WRITER_INSERT_HEAD(&(_sc)->sc_rtlist, brt, brt_list)
+#define BRIDGE_RTLIST_WRITER_REMOVE(_brt)                              \
+       PSLIST_WRITER_REMOVE((_brt), brt_list)
+
+#define BRIDGE_RTHASH_READER_FOREACH(_brt, _sc, _hash)                 \
+       PSLIST_READER_FOREACH((_brt), &(_sc)->sc_rthash[(_hash)],       \
+           struct bridge_rtnode, brt_hash)
+#define BRIDGE_RTHASH_WRITER_FOREACH(_brt, _sc, _hash)                 \
+       PSLIST_WRITER_FOREACH((_brt), &(_sc)->sc_rthash[(_hash)],       \
+           struct bridge_rtnode, brt_hash)
+#define BRIDGE_RTHASH_WRITER_INSERT_HEAD(_sc, _hash, _brt)             \
+       PSLIST_WRITER_INSERT_HEAD(&(_sc)->sc_rthash[(_hash)], brt, brt_hash)
+#define BRIDGE_RTHASH_WRITER_INSERT_AFTER(_brt, _new)                  \
+       PSLIST_WRITER_INSERT_AFTER((_brt), (_new), brt_hash)
+#define BRIDGE_RTHASH_WRITER_REMOVE(_brt)                              \
+       PSLIST_WRITER_REMOVE((_brt), brt_hash)
 
 #ifdef NET_MPSAFE
 #define DECLARE_LOCK_VARIABLE
@@ -1043,7 +1062,7 @@
        BRIDGE_RT_LOCK(sc);
 
        len = bac->ifbac_len;
-       LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) {
+       BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
                if (len < sizeof(bareq))
                        goto out;
                memset(&bareq, 0, sizeof(bareq));
@@ -2013,6 +2032,8 @@
        brt->brt_expire = time_uptime + sc->sc_brttimeout;
        brt->brt_flags = IFBAF_DYNAMIC;
        memcpy(brt->brt_addr, dst, ETHER_ADDR_LEN);
+       PSLIST_ENTRY_INIT(brt, brt_list);
+       PSLIST_ENTRY_INIT(brt, brt_hash);
 
        BRIDGE_RT_LOCK(sc);
        error = bridge_rtnode_insert(sc, brt);
@@ -2109,7 +2130,7 @@
 static void
 bridge_rtlist_iterate_remove(struct bridge_softc *sc, bridge_iterate_cb_t func, void *arg)
 {
-       struct bridge_rtnode *brt, *nbrt;
+       struct bridge_rtnode *brt;
        struct bridge_rtnode **brt_list;
        int i, count;
 
@@ -2128,7 +2149,12 @@
        }
 
        i = 0;
-       LIST_FOREACH_SAFE(brt, &sc->sc_rtlist, brt_list, nbrt) {
+       /*
+        * We don't need to use a _SAFE variant here because we know
+        * that a removed item keeps its next pointer as-is thanks to
+        * pslist(9) and isn't freed in the loop.
+        */
+       BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
                bool need_break = false;
                if (func(sc, brt, &need_break, arg)) {
                        bridge_rtnode_remove(sc, brt);
@@ -2302,7 +2328,7 @@
        /* XXX pserialize_perform for each entry is slow */
 again:
        BRIDGE_RT_LOCK(sc);
-       LIST_FOREACH(brt, &sc->sc_rtlist, brt_list) {
+       BRIDGE_RTLIST_WRITER_FOREACH(brt, sc) {
                if (brt->brt_ifp == ifp)
                        break;
        }
@@ -2333,11 +2359,11 @@
            KM_SLEEP);
 
        for (i = 0; i < BRIDGE_RTHASH_SIZE; i++)
-               LIST_INIT(&sc->sc_rthash[i]);
+               PSLIST_INIT(&sc->sc_rthash[i]);
 
        sc->sc_rthash_key = cprng_fast32();
 
-       LIST_INIT(&sc->sc_rtlist);
+       PSLIST_INIT(&sc->sc_rtlist);
 
        sc->sc_rtlist_psz = pserialize_create();
        sc->sc_rtlist_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_SOFTNET);
@@ -2353,10 +2379,8 @@
 {
 
        kmem_free(sc->sc_rthash, sizeof(*sc->sc_rthash) * BRIDGE_RTHASH_SIZE);
-       if (sc->sc_rtlist_lock)
-               mutex_obj_free(sc->sc_rtlist_lock);
-       if (sc->sc_rtlist_psz)
-               pserialize_destroy(sc->sc_rtlist_psz);
+       mutex_obj_free(sc->sc_rtlist_lock);
+       pserialize_destroy(sc->sc_rtlist_psz);
 }
 
 /*
@@ -2408,7 +2432,7 @@
        int dir;
 
        hash = bridge_rthash(sc, addr);
-       LIST_FOREACH(brt, &sc->sc_rthash[hash], brt_hash) {
+       BRIDGE_RTHASH_READER_FOREACH(brt, sc, hash) {
                dir = memcmp(addr, brt->brt_addr, ETHER_ADDR_LEN);
                if (dir == 0)
                        return brt;
@@ -2428,41 +2452,26 @@
 static int
 bridge_rtnode_insert(struct bridge_softc *sc, struct bridge_rtnode *brt)
 {
-       struct bridge_rtnode *lbrt;
+       struct bridge_rtnode *lbrt, *prev = NULL;
        uint32_t hash;
-       int dir;
 
        KASSERT(BRIDGE_RT_LOCKED(sc));
 
        hash = bridge_rthash(sc, brt->brt_addr);
-
-       lbrt = LIST_FIRST(&sc->sc_rthash[hash]);
-       if (lbrt == NULL) {
-               LIST_INSERT_HEAD(&sc->sc_rthash[hash], brt, brt_hash);
-               goto out;
-       }
-
-       do {
-               dir = memcmp(brt->brt_addr, lbrt->brt_addr, ETHER_ADDR_LEN);
+       BRIDGE_RTHASH_WRITER_FOREACH(lbrt, sc, hash) {
+               int dir = memcmp(brt->brt_addr, lbrt->brt_addr, ETHER_ADDR_LEN);
                if (dir == 0)
                        return EEXIST;
-               if (dir > 0) {
-                       LIST_INSERT_BEFORE(lbrt, brt, brt_hash);
-                       goto out;
-               }
-               if (LIST_NEXT(lbrt, brt_hash) == NULL) {
-                       LIST_INSERT_AFTER(lbrt, brt, brt_hash);
-                       goto out;
-               }
-               lbrt = LIST_NEXT(lbrt, brt_hash);
-       } while (lbrt != NULL);
-
-#ifdef DIAGNOSTIC
-       panic("bridge_rtnode_insert: impossible");
-#endif
-
- out:
-       LIST_INSERT_HEAD(&sc->sc_rtlist, brt, brt_list);
+               if (dir > 0)
+                       break;
+               prev = lbrt;
+       }
+       if (prev == NULL)
+               BRIDGE_RTHASH_WRITER_INSERT_HEAD(sc, hash, brt);
+       else
+               BRIDGE_RTHASH_WRITER_INSERT_AFTER(prev, brt);
+
+       BRIDGE_RTLIST_WRITER_INSERT_HEAD(sc, brt);
        sc->sc_brtcnt++;
 
        return 0;
@@ -2479,8 +2488,8 @@
 
        KASSERT(BRIDGE_RT_LOCKED(sc));
 
-       LIST_REMOVE(brt, brt_hash);
-       LIST_REMOVE(brt, brt_list);
+       BRIDGE_RTHASH_WRITER_REMOVE(brt);
+       BRIDGE_RTLIST_WRITER_REMOVE(brt);
        sc->sc_brtcnt--;
 }
 
@@ -2493,6 +2502,8 @@
 bridge_rtnode_destroy(struct bridge_rtnode *brt)
 {
 
+       PSLIST_ENTRY_DESTROY(brt, brt_list);
+       PSLIST_ENTRY_DESTROY(brt, brt_hash);
        pool_put(&bridge_rtnode_pool, brt);
 }
 
diff -r fc99c8e17e5b -r 5c8d8f779fe4 sys/net/if_bridgevar.h
--- a/sys/net/if_bridgevar.h    Wed Apr 18 14:06:24 2018 +0000
+++ b/sys/net/if_bridgevar.h    Wed Apr 18 14:11:42 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_bridgevar.h,v 1.31 2016/04/28 00:16:56 ozaki-r Exp $        */
+/*     $NetBSD: if_bridgevar.h,v 1.31.10.1 2018/04/18 14:11:42 martin Exp $    */
 
 /*
  * Copyright 2001 Wasabi Systems, Inc.
@@ -275,8 +275,8 @@
  * Bridge route node.
  */
 struct bridge_rtnode {
-       LIST_ENTRY(bridge_rtnode) brt_hash;     /* hash table linkage */
-       LIST_ENTRY(bridge_rtnode) brt_list;     /* list linkage */
+       struct pslist_entry     brt_hash;       /* hash table linkage */
+       struct pslist_entry     brt_list;       /* list linkage */
        struct ifnet            *brt_ifp;       /* destination if */
        time_t                  brt_expire;     /* expiration time */
        uint8_t                 brt_flags;      /* address flags */
@@ -319,8 +319,8 @@
        callout_t               sc_brcallout;   /* bridge callout */
        callout_t               sc_bstpcallout; /* STP callout */
        struct bridge_iflist_psref      sc_iflist_psref;
-       LIST_HEAD(, bridge_rtnode) *sc_rthash;  /* our forwarding table */
-       LIST_HEAD(, bridge_rtnode) sc_rtlist;   /* list version of above */
+       struct pslist_head      *sc_rthash;     /* our forwarding table */
+       struct pslist_head      sc_rtlist;      /* list version of above */
        kmutex_t                *sc_rtlist_lock;
        pserialize_t            sc_rtlist_psz;
        struct workqueue        *sc_rtage_wq;
diff -r fc99c8e17e5b -r 5c8d8f779fe4 tests/net/if_bridge/t_rtable.sh
--- a/tests/net/if_bridge/t_rtable.sh   Wed Apr 18 14:06:24 2018 +0000
+++ b/tests/net/if_bridge/t_rtable.sh   Wed Apr 18 14:11:42 2018 +0000
@@ -1,4 +1,4 @@
-#      $NetBSD: t_rtable.sh,v 1.1.8.1 2018/04/10 11:48:28 martin Exp $
+#      $NetBSD: t_rtable.sh,v 1.1.8.2 2018/04/18 14:11:42 martin Exp $
 #
 # Copyright (c) 2017 Internet Initiative Japan Inc.
 # All rights reserved.
@@ -169,6 +169,7 @@
 bridge_rtable_flush_body()
 {
        local addr1= addr3=
+       local n=
 
        setup
        setup_bridge
@@ -195,6 +196,34 @@
        atf_check -s exit:0 -o not-match:"$addr3 shmif1" /sbin/brconfig bridge0
        unset LD_PRELOAD
 
+       # Add extra interfaces and addresses
+       export RUMP_SERVER=$SOCK1
+       rump_server_add_iface $SOCK1 shmif1 bus1
+       atf_check -s exit:0 rump.ifconfig shmif1 10.0.0.11/24
+       atf_check -s exit:0 rump.ifconfig -w 10
+
+       export RUMP_SERVER=$SOCK3
+       rump_server_add_iface $SOCK3 shmif1 bus2
+       atf_check -s exit:0 rump.ifconfig shmif1 10.0.0.12/24
+       atf_check -s exit:0 rump.ifconfig -w 10
+
+       # Let cache entries
+       export RUMP_SERVER=$SOCK1
+       atf_check -s exit:0 -o ignore rump.ping -n -w $TIMEOUT -c 1 10.0.0.12
+       export RUMP_SERVER=$SOCK3
+       atf_check -s exit:0 -o ignore rump.ping -n -w $TIMEOUT -c 1 10.0.0.11
+
+       export RUMP_SERVER=$SOCK2
+       export LD_PRELOAD=/usr/lib/librumphijack.so
+       $DEBUG && /sbin/brconfig bridge0
+       n=$(get_number_of_caches)



Home | Main Index | Thread Index | Old Index