Module Name: src
Committed By: ozaki-r
Date: Wed May 15 02:56:48 UTC 2019
Modified Files:
src/sys/dev/ic: dwc_gmac.c
src/sys/dev/pci: if_wm.c
src/sys/dev/pci/ixgbe: ixgbe.c
src/sys/net: if.c if_ether.h if_ethersubr.c
Log Message:
Store IFF_ALLMULTI in ec_flags instead of if_flags to avoid data races
IFF_ALLMULTI is set/unset to if_flags via if_mcast_op. To avoid data races on
if_flags, IFNET_LOCK was added for if_mcast_op. Unfortunately it produces
a deadlock so we want to remove added IFNET_LOCK by avoiding the data races by
another approach.
This fix introduces ec_flags to struct ethercom and stores IFF_ALLMULTI to it.
ec_flags is protected by ETHER_LOCK and thus IFNET_LOCK is no longer necessary
for if_mcast_op. Note that the fix is applied only to MP-safe drivers that
the data races matter.
In the kernel, IFF_ALLMULTI is set by a driver and used by the driver itself.
So changing the storing place doesn't break anything. One exception is
ioctl(SIOCGIFFLAGS); we have to include IFF_ALLMULTI in a result if needed to
export the flag as well as before.
A upcoming commit will remove IFNET_LOCK.
PR kern/54189
To generate a diff of this commit:
cvs rdiff -u -r1.59 -r1.60 src/sys/dev/ic/dwc_gmac.c
cvs rdiff -u -r1.635 -r1.636 src/sys/dev/pci/if_wm.c
cvs rdiff -u -r1.182 -r1.183 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.451 -r1.452 src/sys/net/if.c
cvs rdiff -u -r1.77 -r1.78 src/sys/net/if_ether.h
cvs rdiff -u -r1.273 -r1.274 src/sys/net/if_ethersubr.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/ic/dwc_gmac.c
diff -u src/sys/dev/ic/dwc_gmac.c:1.59 src/sys/dev/ic/dwc_gmac.c:1.60
--- src/sys/dev/ic/dwc_gmac.c:1.59 Mon Apr 15 06:00:04 2019
+++ src/sys/dev/ic/dwc_gmac.c Wed May 15 02:56:47 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: dwc_gmac.c,v 1.59 2019/04/15 06:00:04 ozaki-r Exp $ */
+/* $NetBSD: dwc_gmac.c,v 1.60 2019/05/15 02:56:47 ozaki-r Exp $ */
/*-
* Copyright (c) 2013, 2014 The NetBSD Foundation, Inc.
@@ -41,7 +41,7 @@
#include <sys/cdefs.h>
-__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.59 2019/04/15 06:00:04 ozaki-r Exp $");
+__KERNEL_RCSID(1, "$NetBSD: dwc_gmac.c,v 1.60 2019/05/15 02:56:47 ozaki-r Exp $");
/* #define DWC_GMAC_DEBUG 1 */
@@ -1357,21 +1357,21 @@ dwc_gmac_setmulti(struct dwc_gmac_softc
goto special_filter;
}
- ifp->if_flags &= ~IFF_ALLMULTI;
ffilt &= ~(AWIN_GMAC_MAC_FFILT_PM|AWIN_GMAC_MAC_FFILT_PR);
bus_space_write_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_HTLOW, 0);
bus_space_write_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_HTHIGH, 0);
ETHER_LOCK(ec);
+ ec->ec_flags &= ~ETHER_F_ALLMULTI;
ETHER_FIRST_MULTI(step, ec, enm);
mcnt = 0;
while (enm != NULL) {
if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
ETHER_ADDR_LEN) != 0) {
- ETHER_UNLOCK(ec);
ffilt |= AWIN_GMAC_MAC_FFILT_PM;
- ifp->if_flags |= IFF_ALLMULTI;
+ ec->ec_flags |= ETHER_F_ALLMULTI;
+ ETHER_UNLOCK(ec);
goto special_filter;
}
@@ -1395,7 +1395,7 @@ dwc_gmac_setmulti(struct dwc_gmac_softc
hashes[0]);
bus_space_write_4(sc->sc_bst, sc->sc_bsh, AWIN_GMAC_MAC_HTHIGH,
hashes[1]);
- sc->sc_if_flags = sc->sc_ec.ec_if.if_flags;
+ sc->sc_if_flags = ifp->if_flags;
#ifdef DWC_GMAC_DEBUG
dwc_gmac_dump_ffilt(sc, ffilt);
Index: src/sys/dev/pci/if_wm.c
diff -u src/sys/dev/pci/if_wm.c:1.635 src/sys/dev/pci/if_wm.c:1.636
--- src/sys/dev/pci/if_wm.c:1.635 Tue May 14 09:43:55 2019
+++ src/sys/dev/pci/if_wm.c Wed May 15 02:56:47 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: if_wm.c,v 1.635 2019/05/14 09:43:55 ozaki-r Exp $ */
+/* $NetBSD: if_wm.c,v 1.636 2019/05/15 02:56:47 ozaki-r Exp $ */
/*
* Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -82,7 +82,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.635 2019/05/14 09:43:55 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.636 2019/05/15 02:56:47 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_net_mpsafe.h"
@@ -3707,6 +3707,9 @@ wm_set_filter(struct wm_softc *sc)
sc->sc_rctl |= RCTL_BAM;
if (ifp->if_flags & IFF_PROMISC) {
sc->sc_rctl |= RCTL_UPE;
+ ETHER_LOCK(ec);
+ ec->ec_flags |= ETHER_F_ALLMULTI;
+ ETHER_UNLOCK(ec);
goto allmulti;
}
@@ -3757,6 +3760,7 @@ wm_set_filter(struct wm_softc *sc)
ETHER_FIRST_MULTI(step, ec, enm);
while (enm != NULL) {
if (memcmp(enm->enm_addrlo, enm->enm_addrhi, ETHER_ADDR_LEN)) {
+ ec->ec_flags |= ETHER_F_ALLMULTI;
ETHER_UNLOCK(ec);
/*
* We must listen to a range of multicast addresses.
@@ -3804,13 +3808,12 @@ wm_set_filter(struct wm_softc *sc)
ETHER_NEXT_MULTI(step, enm);
}
+ ec->ec_flags &= ~ETHER_F_ALLMULTI;
ETHER_UNLOCK(ec);
- ifp->if_flags &= ~IFF_ALLMULTI;
goto setit;
allmulti:
- ifp->if_flags |= IFF_ALLMULTI;
sc->sc_rctl |= RCTL_MPE;
setit:
Index: src/sys/dev/pci/ixgbe/ixgbe.c
diff -u src/sys/dev/pci/ixgbe/ixgbe.c:1.182 src/sys/dev/pci/ixgbe/ixgbe.c:1.183
--- src/sys/dev/pci/ixgbe/ixgbe.c:1.182 Tue May 14 09:43:55 2019
+++ src/sys/dev/pci/ixgbe/ixgbe.c Wed May 15 02:56:47 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: ixgbe.c,v 1.182 2019/05/14 09:43:55 ozaki-r Exp $ */
+/* $NetBSD: ixgbe.c,v 1.183 2019/05/15 02:56:47 ozaki-r Exp $ */
/******************************************************************************
@@ -3017,10 +3017,10 @@ ixgbe_set_promisc(struct adapter *adapte
KASSERT(mutex_owned(&adapter->core_mtx));
rctl = IXGBE_READ_REG(&adapter->hw, IXGBE_FCTRL);
rctl &= (~IXGBE_FCTRL_UPE);
- if (ifp->if_flags & IFF_ALLMULTI)
+ ETHER_LOCK(ec);
+ if (ec->ec_flags & ETHER_F_ALLMULTI)
mcnt = MAX_NUM_MULTICAST_ADDRESSES;
else {
- ETHER_LOCK(ec);
ETHER_FIRST_MULTI(step, ec, enm);
while (enm != NULL) {
if (mcnt == MAX_NUM_MULTICAST_ADDRESSES)
@@ -3028,7 +3028,6 @@ ixgbe_set_promisc(struct adapter *adapte
mcnt++;
ETHER_NEXT_MULTI(step, enm);
}
- ETHER_UNLOCK(ec);
}
if (mcnt < MAX_NUM_MULTICAST_ADDRESSES)
rctl &= (~IXGBE_FCTRL_MPE);
@@ -3037,11 +3036,12 @@ ixgbe_set_promisc(struct adapter *adapte
if (ifp->if_flags & IFF_PROMISC) {
rctl |= (IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
IXGBE_WRITE_REG(&adapter->hw, IXGBE_FCTRL, rctl);
- } else if (ifp->if_flags & IFF_ALLMULTI) {
+ } else if (ec->ec_flags & ETHER_F_ALLMULTI) {
rctl |= IXGBE_FCTRL_MPE;
rctl &= ~IXGBE_FCTRL_UPE;
IXGBE_WRITE_REG(&adapter->hw, IXGBE_FCTRL, rctl);
}
+ ETHER_UNLOCK(ec);
} /* ixgbe_set_promisc */
/************************************************************************
@@ -4354,14 +4354,14 @@ ixgbe_set_multi(struct adapter *adapter)
mta = adapter->mta;
bzero(mta, sizeof(*mta) * MAX_NUM_MULTICAST_ADDRESSES);
- ifp->if_flags &= ~IFF_ALLMULTI;
ETHER_LOCK(ec);
+ ec->ec_flags &= ~ETHER_F_ALLMULTI;
ETHER_FIRST_MULTI(step, ec, enm);
while (enm != NULL) {
if ((mcnt == MAX_NUM_MULTICAST_ADDRESSES) ||
(memcmp(enm->enm_addrlo, enm->enm_addrhi,
ETHER_ADDR_LEN) != 0)) {
- ifp->if_flags |= IFF_ALLMULTI;
+ ec->ec_flags |= ETHER_F_ALLMULTI;
break;
}
bcopy(enm->enm_addrlo,
@@ -4370,15 +4370,15 @@ ixgbe_set_multi(struct adapter *adapter)
mcnt++;
ETHER_NEXT_MULTI(step, enm);
}
- ETHER_UNLOCK(ec);
fctrl = IXGBE_READ_REG(&adapter->hw, IXGBE_FCTRL);
fctrl &= ~(IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
if (ifp->if_flags & IFF_PROMISC)
fctrl |= (IXGBE_FCTRL_UPE | IXGBE_FCTRL_MPE);
- else if (ifp->if_flags & IFF_ALLMULTI) {
+ else if (ec->ec_flags & ETHER_F_ALLMULTI) {
fctrl |= IXGBE_FCTRL_MPE;
}
+ ETHER_UNLOCK(ec);
IXGBE_WRITE_REG(&adapter->hw, IXGBE_FCTRL, fctrl);
Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.451 src/sys/net/if.c:1.452
--- src/sys/net/if.c:1.451 Sat Apr 20 22:16:47 2019
+++ src/sys/net/if.c Wed May 15 02:56:48 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: if.c,v 1.451 2019/04/20 22:16:47 pgoyette Exp $ */
+/* $NetBSD: if.c,v 1.452 2019/05/15 02:56:48 ozaki-r Exp $ */
/*-
* Copyright (c) 1999, 2000, 2001, 2008 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.451 2019/04/20 22:16:47 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.452 2019/05/15 02:56:48 ozaki-r Exp $");
#if defined(_KERNEL_OPT)
#include "opt_inet.h"
@@ -3625,13 +3625,6 @@ if_mcast_op(ifnet_t *ifp, const unsigned
int rc;
struct ifreq ifr;
- /* There remain some paths that don't hold IFNET_LOCK yet */
-#ifdef NET_MPSAFE
- /* CARP and MROUTING still don't deal with the lock yet */
-#if (!defined(NCARP) || (NCARP == 0)) && !defined(MROUTING)
- KASSERT(IFNET_LOCKED(ifp));
-#endif
-#endif
if (ifp->if_mcastop != NULL)
rc = (*ifp->if_mcastop)(ifp, cmd, sa);
else {
Index: src/sys/net/if_ether.h
diff -u src/sys/net/if_ether.h:1.77 src/sys/net/if_ether.h:1.78
--- src/sys/net/if_ether.h:1.77 Tue Mar 5 08:25:03 2019
+++ src/sys/net/if_ether.h Wed May 15 02:56:48 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: if_ether.h,v 1.77 2019/03/05 08:25:03 msaitoh Exp $ */
+/* $NetBSD: if_ether.h,v 1.78 2019/05/15 02:56:48 ozaki-r Exp $ */
/*
* Copyright (c) 1982, 1986, 1993
@@ -189,6 +189,8 @@ struct ethercom {
*/
ether_cb_t ec_ifflags_cb;
kmutex_t *ec_lock;
+ /* Flags used only by the kernel */
+ int ec_flags;
#ifdef MBUFTRACE
struct mowner ec_rx_mowner; /* mbufs received */
struct mowner ec_tx_mowner; /* mbufs transmitted */
@@ -225,6 +227,12 @@ struct ether_multi_sysctl {
};
#ifdef _KERNEL
+/*
+ * Flags for ec_flags
+ */
+/* Store IFF_ALLMULTI in ec_flags instead of if_flags to avoid data races. */
+#define ETHER_F_ALLMULTI __BIT(0)
+
extern const uint8_t etherbroadcastaddr[ETHER_ADDR_LEN];
extern const uint8_t ethermulticastaddr_slowprotocols[ETHER_ADDR_LEN];
extern const uint8_t ether_ipmulticast_min[ETHER_ADDR_LEN];
Index: src/sys/net/if_ethersubr.c
diff -u src/sys/net/if_ethersubr.c:1.273 src/sys/net/if_ethersubr.c:1.274
--- src/sys/net/if_ethersubr.c:1.273 Mon Feb 4 10:11:34 2019
+++ src/sys/net/if_ethersubr.c Wed May 15 02:56:48 2019
@@ -1,4 +1,4 @@
-/* $NetBSD: if_ethersubr.c,v 1.273 2019/02/04 10:11:34 mrg Exp $ */
+/* $NetBSD: if_ethersubr.c,v 1.274 2019/05/15 02:56:48 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.273 2019/02/04 10:11:34 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ethersubr.c,v 1.274 2019/05/15 02:56:48 ozaki-r Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -997,6 +997,7 @@ ether_ifattach(struct ifnet *ifp, const
LIST_INIT(&ec->ec_multiaddrs);
ec->ec_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NET);
+ ec->ec_flags = 0;
ifp->if_broadcastaddr = etherbroadcastaddr;
bpf_attach(ifp, DLT_EN10MB, sizeof(struct ether_header));
#ifdef MBUFTRACE
@@ -1470,6 +1471,14 @@ ether_ioctl(struct ifnet *ifp, u_long cm
if ((error = ifioctl_common(ifp, cmd, data)) != 0)
return error;
return ether_ioctl_reinit(ec);
+ case SIOCGIFFLAGS:
+ error = ifioctl_common(ifp, cmd, data);
+ if (error == 0) {
+ /* Set IFF_ALLMULTI for backcompat */
+ ifr->ifr_flags |= (ec->ec_flags & ETHER_F_ALLMULTI) ?
+ IFF_ALLMULTI : 0;
+ }
+ return error;
case SIOCGETHERCAP:
eccr = (struct eccapreq *)data;
eccr->eccr_capabilities = ec->ec_capabilities;
