commit 417afbee675cded44e18ab4835300cca01a1b2a3
Author: Ryota Ozaki <ozaki-r@iij.ad.jp>
Date:   Tue Mar 26 14:14:43 2019 +0900

    Store IFF_ALLMULTI in ec_flags instead of if_flags
    
    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 and avoid 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.
    
    The upcoming commit will remove IFNET_LOCK.

diff --git a/sys/dev/ic/dwc_gmac.c b/sys/dev/ic/dwc_gmac.c
index af3d32d5258..d22a8e6659f 100644
--- a/sys/dev/ic/dwc_gmac.c
+++ b/sys/dev/ic/dwc_gmac.c
@@ -1357,21 +1357,21 @@ dwc_gmac_setmulti(struct dwc_gmac_softc *sc)
 		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 *sc)
 	    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);
diff --git a/sys/dev/pci/if_wm.c b/sys/dev/pci/if_wm.c
index 74473d7be49..5d3c1e48823 100644
--- a/sys/dev/pci/if_wm.c
+++ b/sys/dev/pci/if_wm.c
@@ -3311,6 +3311,7 @@ wm_ifflags_cb(struct ethercom *ec)
 	 * Main usage is to prevent linkdown when opening bpf.
 	 */
 	iffchange = ifp->if_flags ^ sc->sc_if_flags;
+	KASSERT((iffchange & IFF_ALLMULTI) == 0);
 	sc->sc_if_flags = ifp->if_flags;
 	if ((iffchange & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0) {
 		needreset = true;
@@ -3318,7 +3319,7 @@ wm_ifflags_cb(struct ethercom *ec)
 	}
 
 	/* iff related updates */
-	if ((iffchange & (IFF_PROMISC | IFF_ALLMULTI)) != 0)
+	if ((iffchange & IFF_PROMISC) != 0)
 		wm_set_filter(sc);
 
 	wm_set_vlan(sc);
@@ -3707,6 +3708,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,7 +3761,6 @@ 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)) {
-			ETHER_UNLOCK(ec);
 			/*
 			 * We must listen to a range of multicast addresses.
 			 * For now, just accept all multicasts, rather than
@@ -3766,6 +3769,8 @@ wm_set_filter(struct wm_softc *sc)
 			 * ranges is for IP multicast routing, for which the
 			 * range is big enough to require all bits set.)
 			 */
+			ec->ec_flags |= ETHER_F_ALLMULTI;
+			ETHER_UNLOCK(ec);
 			goto allmulti;
 		}
 
@@ -3804,13 +3809,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:
diff --git a/sys/dev/pci/ixgbe/ixgbe.c b/sys/dev/pci/ixgbe/ixgbe.c
index 56ac42ffd68..0b9440d2b6d 100644
--- a/sys/dev/pci/ixgbe/ixgbe.c
+++ b/sys/dev/pci/ixgbe/ixgbe.c
@@ -3018,10 +3018,10 @@ ixgbe_set_promisc(struct adapter *adapter)
 	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)
@@ -3029,7 +3029,6 @@ ixgbe_set_promisc(struct adapter *adapter)
 			mcnt++;
 			ETHER_NEXT_MULTI(step, enm);
 		}
-		ETHER_UNLOCK(ec);
 	}
 	if (mcnt < MAX_NUM_MULTICAST_ADDRESSES)
 		rctl &= (~IXGBE_FCTRL_MPE);
@@ -3038,11 +3037,12 @@ ixgbe_set_promisc(struct adapter *adapter)
 	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 */
 
 /************************************************************************
@@ -4355,14 +4355,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,
@@ -4371,15 +4371,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);
 
@@ -6159,12 +6159,13 @@ ixgbe_ifflags_cb(struct ethercom *ec)
 	IXGBE_CORE_LOCK(adapter);
 
 	change = ifp->if_flags ^ adapter->if_flags;
+	KASSERT((change & IFF_ALLMULTI) == 0);
 	if (change != 0)
 		adapter->if_flags = ifp->if_flags;
 
 	if ((change & ~(IFF_CANTCHANGE | IFF_DEBUG)) != 0)
 		rc = ENETRESET;
-	else if ((change & (IFF_PROMISC | IFF_ALLMULTI)) != 0)
+	else if ((change & IFF_PROMISC) != 0)
 		ixgbe_set_promisc(adapter);
 
 	/* Set up VLAN support and filter */
diff --git a/sys/net/if.c b/sys/net/if.c
index 79272c2035b..18c4fd0b899 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -3621,13 +3621,6 @@ if_mcast_op(ifnet_t *ifp, const unsigned long cmd, const struct sockaddr *sa)
 	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 {
diff --git a/sys/net/if_ether.h b/sys/net/if_ether.h
index b673618b3c9..410adf7484a 100644
--- a/sys/net/if_ether.h
+++ b/sys/net/if_ether.h
@@ -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];
diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index 4180b6f6747..1464c90485f 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -997,6 +997,7 @@ ether_ifattach(struct ifnet *ifp, const uint8_t *lla)
 
 	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 cmd, void *data)
 		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;