Source-Changes-HG archive

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

[src/trunk]: src/sys Protect ec_multi* with mutex



details:   https://anonhg.NetBSD.org/src/rev/328281624b89
branches:  trunk
changeset: 820067:328281624b89
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Wed Dec 28 07:32:16 2016 +0000

description:
Protect ec_multi* with mutex

The data can be accessed from sysctl, ioctl, interface watchdog
(if_slowtimo) and interrupt handlers. We need to protect the data against
parallel accesses from them.

Currently the mutex is applied to some drivers, we need to apply it to all
drivers in the future.

Note that the mutex is adaptive one for ease of implementation but some
drivers access the data in interrupt context so we cannot apply the mutex
to every drivers as is. We have two options: one is to replace the mutex
with a spin one, which requires some additional works (see
ether_multicast_sysctl), and the other is to modify the drivers to access
the data not in interrupt context somehow.

diffstat:

 sys/arch/x86/pci/if_vmx.c |   7 ++-
 sys/dev/pci/if_vioif.c    |   7 ++-
 sys/dev/pci/if_wm.c       |   7 ++-
 sys/net/if_ether.h        |   6 ++-
 sys/net/if_ethersubr.c    |  92 +++++++++++++++++++++++++++++-----------------
 5 files changed, 77 insertions(+), 42 deletions(-)

diffs (truncated from 360 to 300 lines):

diff -r eed46a442b56 -r 328281624b89 sys/arch/x86/pci/if_vmx.c
--- a/sys/arch/x86/pci/if_vmx.c Wed Dec 28 07:26:24 2016 +0000
+++ b/sys/arch/x86/pci/if_vmx.c Wed Dec 28 07:32:16 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_vmx.c,v 1.14 2016/12/27 03:09:55 hikaru Exp $       */
+/*     $NetBSD: if_vmx.c,v 1.15 2016/12/28 07:32:16 ozaki-r Exp $      */
 /*     $OpenBSD: if_vmx.c,v 1.16 2014/01/22 06:04:17 brad Exp $        */
 
 /*
@@ -19,7 +19,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.14 2016/12/27 03:09:55 hikaru Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vmx.c,v 1.15 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #include <sys/param.h>
 #include <sys/cpu.h>
@@ -2818,9 +2818,11 @@
                goto allmulti;
 
        p = sc->vmx_mcast;
+       ETHER_LOCK(ec);
        ETHER_FIRST_MULTI(step, ec, enm);
        while (enm != NULL) {
                if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
+                       ETHER_UNLOCK(ec);
                        /*
                         * We must listen to a range of multicast addresses.
                         * For now, just accept all multicasts, rather than
@@ -2837,6 +2839,7 @@
 
                ETHER_NEXT_MULTI(step, enm);
        }
+       ETHER_UNLOCK(ec);
 
        if (ec->ec_multicnt > 0) {
                SET(mode, VMXNET3_RXMODE_MCAST);
diff -r eed46a442b56 -r 328281624b89 sys/dev/pci/if_vioif.c
--- a/sys/dev/pci/if_vioif.c    Wed Dec 28 07:26:24 2016 +0000
+++ b/sys/dev/pci/if_vioif.c    Wed Dec 28 07:32:16 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_vioif.c,v 1.29 2016/12/15 09:28:05 ozaki-r Exp $    */
+/*     $NetBSD: if_vioif.c,v 1.30 2016/12/28 07:32:16 ozaki-r Exp $    */
 
 /*
  * Copyright (c) 2010 Minoura Makoto.
@@ -26,7 +26,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_vioif.c,v 1.29 2016/12/15 09:28:05 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_vioif.c,v 1.30 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -1446,6 +1446,7 @@
        }
 
        nentries = -1;
+       ETHER_LOCK(&sc->sc_ethercom);
        ETHER_FIRST_MULTI(step, &sc->sc_ethercom, enm);
        while (nentries++, enm != NULL) {
                if (nentries >= VIRTIO_NET_CTRL_MAC_MAXENTRIES) {
@@ -1464,6 +1465,8 @@
        rxfilter = 1;
 
 set:
+       ETHER_UNLOCK(&sc->sc_ethercom);
+
        if (rxfilter) {
                sc->sc_ctrl_mac_tbl_uc->nentries = 0;
                sc->sc_ctrl_mac_tbl_mc->nentries = nentries;
diff -r eed46a442b56 -r 328281624b89 sys/dev/pci/if_wm.c
--- a/sys/dev/pci/if_wm.c       Wed Dec 28 07:26:24 2016 +0000
+++ b/sys/dev/pci/if_wm.c       Wed Dec 28 07:32:16 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_wm.c,v 1.459 2016/12/26 07:55:00 msaitoh Exp $      */
+/*     $NetBSD: if_wm.c,v 1.460 2016/12/28 07:32:16 ozaki-r Exp $      */
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -84,7 +84,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.459 2016/12/26 07:55:00 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.460 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -3249,9 +3249,11 @@
        for (i = 0; i < size; i++)
                CSR_WRITE(sc, mta_reg + (i << 2), 0);
 
+       ETHER_LOCK(ec);
        ETHER_FIRST_MULTI(step, ec, enm);
        while (enm != NULL) {
                if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
+                       ETHER_UNLOCK(ec);
                        /*
                         * We must listen to a range of multicast addresses.
                         * For now, just accept all multicasts, rather than
@@ -3293,6 +3295,7 @@
 
                ETHER_NEXT_MULTI(step, enm);
        }
+       ETHER_UNLOCK(ec);
 
        ifp->if_flags &= ~IFF_ALLMULTI;
        goto setit;
diff -r eed46a442b56 -r 328281624b89 sys/net/if_ether.h
--- a/sys/net/if_ether.h        Wed Dec 28 07:26:24 2016 +0000
+++ b/sys/net/if_ether.h        Wed Dec 28 07:32:16 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ether.h,v 1.65 2015/11/19 16:23:54 christos Exp $   */
+/*     $NetBSD: if_ether.h,v 1.66 2016/12/28 07:32:16 ozaki-r Exp $    */
 
 /*
  * Copyright (c) 1982, 1986, 1993
@@ -177,6 +177,7 @@
         * ec_if.if_init, 0 on success, not 0 on failure.
         */
        ether_cb_t                              ec_ifflags_cb;
+       kmutex_t                                *ec_lock;
 #ifdef MBUFTRACE
        struct  mowner ec_rx_mowner;            /* mbufs received */
        struct  mowner ec_tx_mowner;            /* mbufs transmitted */
@@ -286,6 +287,9 @@
 
 #ifdef _KERNEL
 
+#define ETHER_LOCK(ec)         mutex_enter((ec)->ec_lock)
+#define ETHER_UNLOCK(ec)       mutex_exit((ec)->ec_lock)
+
 /*
  * Ethernet 802.1Q VLAN structures.
  */
diff -r eed46a442b56 -r 328281624b89 sys/net/if_ethersubr.c
--- a/sys/net/if_ethersubr.c    Wed Dec 28 07:26:24 2016 +0000
+++ b/sys/net/if_ethersubr.c    Wed Dec 28 07:32:16 2016 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_ethersubr.c,v 1.230 2016/12/28 07:26:24 ozaki-r Exp $       */
+/*     $NetBSD: if_ethersubr.c,v 1.231 2016/12/28 07:32:16 ozaki-r Exp $       */
 
 /*
  * Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@@ -61,7 +61,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.230 2016/12/28 07:26:24 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.231 2016/12/28 07:32:16 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -89,6 +89,7 @@
 #include <sys/rnd.h>
 #include <sys/rndsource.h>
 #include <sys/cpu.h>
+#include <sys/kmem.h>
 
 #include <net/if.h>
 #include <net/netisr.h>
@@ -966,6 +967,7 @@
                if_set_sadl(ifp, lla, ETHER_ADDR_LEN, !ETHER_IS_LOCAL(lla));
 
        LIST_INIT(&ec->ec_multiaddrs);
+       ec->ec_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
        ifp->if_broadcastaddr = etherbroadcastaddr;
        bpf_attach(ifp, DLT_EN10MB, sizeof(struct ether_header));
 #ifdef MBUFTRACE
@@ -1012,13 +1014,17 @@
 #endif
 
        s = splnet();
+       mutex_enter(ec->ec_lock);
        while ((enm = LIST_FIRST(&ec->ec_multiaddrs)) != NULL) {
                LIST_REMOVE(enm, enm_list);
-               free(enm, M_IFMADDR);
+               kmem_free(enm, sizeof(*enm));
                ec->ec_multicnt--;
        }
+       mutex_exit(ec->ec_lock);
        splx(s);
 
+       mutex_destroy(ec->ec_lock);
+
        ifp->if_mowner = NULL;
        MOWNER_DETACH(&ec->ec_rx_mowner);
        MOWNER_DETACH(&ec->ec_tx_mowner);
@@ -1224,56 +1230,62 @@
 int
 ether_addmulti(const struct sockaddr *sa, struct ethercom *ec)
 {
-       struct ether_multi *enm;
+       struct ether_multi *enm, *_enm;
        u_char addrlo[ETHER_ADDR_LEN];
        u_char addrhi[ETHER_ADDR_LEN];
-       int s = splnet(), error;
+       int s, error = 0;
 
+       /* Allocate out of lock */
+       enm = kmem_alloc(sizeof(*enm), KM_SLEEP);
+       if (enm == NULL)
+               return ENOBUFS;
+
+       s = splnet();
+       mutex_enter(ec->ec_lock);
        error = ether_multiaddr(sa, addrlo, addrhi);
-       if (error != 0) {
-               splx(s);
-               return error;
-       }
+       if (error != 0)
+               goto out;
 
        /*
         * Verify that we have valid Ethernet multicast addresses.
         */
        if (!ETHER_IS_MULTICAST(addrlo) || !ETHER_IS_MULTICAST(addrhi)) {
-               splx(s);
-               return EINVAL;
+               error = EINVAL;
+               goto out;
        }
        /*
         * See if the address range is already in the list.
         */
-       ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, enm);
-       if (enm != NULL) {
+       ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, _enm);
+       if (_enm != NULL) {
                /*
                 * Found it; just increment the reference count.
                 */
-               ++enm->enm_refcount;
-               splx(s);
-               return 0;
+               ++_enm->enm_refcount;
+               error = 0;
+               goto out;
        }
        /*
         * New address or range; malloc a new multicast record
         * and link it into the interface's multicast list.
         */
-       enm = (struct ether_multi *)malloc(sizeof(*enm), M_IFMADDR, M_NOWAIT);
-       if (enm == NULL) {
-               splx(s);
-               return ENOBUFS;
-       }
        memcpy(enm->enm_addrlo, addrlo, 6);
        memcpy(enm->enm_addrhi, addrhi, 6);
        enm->enm_refcount = 1;
        LIST_INSERT_HEAD(&ec->ec_multiaddrs, enm, enm_list);
        ec->ec_multicnt++;
-       splx(s);
        /*
         * Return ENETRESET to inform the driver that the list has changed
         * and its reception filter should be adjusted accordingly.
         */
-       return ENETRESET;
+       error = ENETRESET;
+       enm = NULL;
+out:
+       mutex_exit(ec->ec_lock);
+       splx(s);
+       if (enm != NULL)
+               kmem_free(enm, sizeof(*enm));
+       return error;
 }
 
 /*
@@ -1285,41 +1297,47 @@
        struct ether_multi *enm;
        u_char addrlo[ETHER_ADDR_LEN];
        u_char addrhi[ETHER_ADDR_LEN];
-       int s = splnet(), error;
+       int s, error;
 
+       s = splnet();
+       mutex_enter(ec->ec_lock);
        error = ether_multiaddr(sa, addrlo, addrhi);
-       if (error != 0) {
-               splx(s);
-               return (error);
-       }
+       if (error != 0)
+               goto error;
 
        /*
         * Look ur the address in our list.
         */
        ETHER_LOOKUP_MULTI(addrlo, addrhi, ec, enm);
        if (enm == NULL) {
-               splx(s);
-               return (ENXIO);
+               error = ENXIO;
+               goto error;



Home | Main Index | Thread Index | Old Index