more smp cleanup for ure(4)/axen(4)/cdce(4):

- convert IFF_ALLMULTI to ETHER_F_ALLMULTI, using ETHER_LOCK()
- remove IFF_OACTIVE use, and simply check the ring count in start
- assert/take more locks
- fix axen_timer setting so it actually runs
- XXX: IFF_RUNNING is not properly protected (all driver problem)
- document a locking issue in stop callback:
  stop is called with the softc lock held, but the lock order
  in all other places is ifnet -> softc -> rx -> tx, so taking
  ifnet lock when softc lock is held would be problematic
- add missing USBD_MPSAFE and cdce_stopping resetting for cdce(4)
- in rxeof check for stopping/dying more often.  i managed to
  trigger a pagefault in cdce_rxeof() when yanking an active
  device as it attempted to usbd_setup_xfer() on closed pipes.

between this and other recent clean ups increase performance of
these drivers mostly.  some numbers:

mbit:	old:	in	out	in+out	new:	in	out	in+out
----	----    --	---     ------	----    --	---	------
cdce		39	32	44mbit		38	33	54mbit
axen		44	34	45		48	37	42
ure		36	34	35		36	38	38

i'm not sure why axen drops a little with in+out.  cdce is
helped quite a lot, and ure a little.  it is disappointing that
ure does not outperform cdce -- it's the same actual hardware,
and the device-specific (ure) should outperform the generic
cdce driver...


Index: if_cdce.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/if_cdce.c,v
retrieving revision 1.50
diff -p -u -r1.50 if_cdce.c
--- if_cdce.c	23 Jun 2019 02:14:14 -0000	1.50
+++ if_cdce.c	27 Jun 2019 10:59:10 -0000
@@ -146,7 +146,6 @@ static void	 cdce_txeof(struct usbd_xfer
 static void	 cdce_start(struct ifnet *);
 static int	 cdce_ioctl(struct ifnet *, u_long, void *);
 static void	 cdce_init(void *);
-static void	 cdce_watchdog(struct ifnet *);
 static void	 cdce_stop(struct cdce_softc *);
 static void	 cdce_tick(void *);
 static void	 cdce_tick_task(void *);
@@ -384,8 +383,11 @@ cdce_detach(device_t self, int flags)
 	usb_rem_task_wait(sc->cdce_udev, &sc->cdce_tick_task,
 	    USB_TASKQ_DRIVER, NULL);
 
-	if (ifp->if_flags & IFF_RUNNING)
+	if (ifp->if_flags & IFF_RUNNING) {
+		IFNET_LOCK(ifp);
 		cdce_stop(sc);
+		IFNET_UNLOCK(ifp);
+	}
 
 	callout_destroy(&sc->cdce_stat_ch);
 	ether_ifdetach(ifp);
@@ -408,8 +410,8 @@ cdce_start_locked(struct ifnet *ifp)
 	struct mbuf		*m_head = NULL;
 
 	KASSERT(mutex_owned(&sc->cdce_txlock));
-	if (sc->cdce_dying || sc->cdce_stopping ||
-	    (ifp->if_flags & IFF_OACTIVE))
+
+	if (sc->cdce_dying || sc->cdce_cdata.cdce_tx_cnt == CDCE_TX_LIST_CNT)
 		return;
 
 	IFQ_POLL(&ifp->if_snd, m_head);
@@ -425,12 +427,10 @@ cdce_start_locked(struct ifnet *ifp)
 
 	bpf_mtap(ifp, m_head, BPF_D_OUT);
 
-	ifp->if_flags |= IFF_OACTIVE;
-
 	/*
 	 * Set a timeout in case the chip goes out to lunch.
 	 */
-	ifp->if_timer = 6;
+	sc->cdce_timer = 6;
 }
 
 static void
@@ -469,30 +469,42 @@ cdce_encap(struct cdce_softc *sc, struct
 	    USBD_FORCE_SHORT_XFER, 10000, cdce_txeof);
 	err = usbd_transfer(c->cdce_xfer);
 	if (err != USBD_IN_PROGRESS) {
+		/* XXXSMP IFNET_LOCK */
 		cdce_stop(sc);
 		return EIO;
 	}
 
 	sc->cdce_cdata.cdce_tx_cnt++;
+	KASSERT(sc->cdce_cdata.cdce_tx_cnt <= CDCE_TX_LIST_CNT);
 
 	return 0;
 }
 
 static void
-cdce_stop(struct cdce_softc *sc)
+cdce_stop_locked(struct cdce_softc *sc)
 {
 	usbd_status	 err;
 	struct ifnet	*ifp = GET_IFP(sc);
 	int		 i;
 
+	/* XXXSMP can't KASSERT(IFNET_LOCKED(ifp)); */
+	KASSERT(mutex_owned(&sc->cdce_lock));
+
 	mutex_enter(&sc->cdce_rxlock);
 	mutex_enter(&sc->cdce_txlock);
 	sc->cdce_stopping = true;
 	mutex_exit(&sc->cdce_txlock);
 	mutex_exit(&sc->cdce_rxlock);
 
-	ifp->if_timer = 0;
-	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
+	/*
+	 * XXXSMP Would like to
+	 *	KASSERT(IFNET_LOCKED(ifp))
+	 * here but the locking order is:
+	 *	ifnet -> sc lock -> rxlock -> txlock
+	 * and sc lock is already held.
+	 */
+	ifp->if_flags &= ~IFF_RUNNING;
+	sc->cdce_timer = 0;
 
 	callout_stop(&sc->cdce_stat_ch);
 
@@ -549,7 +561,15 @@ cdce_stop(struct cdce_softc *sc)
 			    device_xname(sc->cdce_dev), usbd_errstr(err));
 		sc->cdce_bulkout_pipe = NULL;
 	}
-	mutex_exit(&sc->cdce_txlock);
+}
+
+static void
+cdce_stop(struct cdce_softc *sc)
+{
+
+	mutex_enter(&sc->cdce_lock);
+	cdce_stop_locked(sc);
+	mutex_exit(&sc->cdce_lock);
 }
 
 static int
@@ -613,7 +633,7 @@ cdce_ioctl(struct ifnet *ifp, u_long com
 static void
 cdce_watchdog(struct ifnet *ifp)
 {
-	struct cdce_softc	*sc = ifp->if_softc;
+	struct cdce_softc *sc = ifp->if_softc;
 	struct cdce_chain *c;
 	usbd_status stat;
 
@@ -625,12 +645,16 @@ cdce_watchdog(struct ifnet *ifp)
 	ifp->if_oerrors++;
 	aprint_error_dev(sc->cdce_dev, "watchdog timeout\n");
 
+	mutex_enter(&sc->cdce_txlock);
+
 	c = &sc->cdce_cdata.cdce_rx_chain[0];
 	usbd_get_xfer_status(c->cdce_xfer, NULL, NULL, NULL, &stat);
 	cdce_txeof(c->cdce_xfer, c, stat);
 
 	if (!IFQ_IS_EMPTY(&ifp->if_snd))
 		cdce_start_locked(ifp);
+
+	mutex_exit(&sc->cdce_txlock);
 }
 
 static void
@@ -649,7 +673,7 @@ cdce_init(void *xsc)
 	/* Maybe set multicast / broadcast here??? */
 
 	err = usbd_open_pipe(sc->cdce_data_iface, sc->cdce_bulkin_no,
-	    USBD_EXCLUSIVE_USE, &sc->cdce_bulkin_pipe);
+	    USBD_EXCLUSIVE_USE | USBD_MPSAFE, &sc->cdce_bulkin_pipe);
 	if (err) {
 		printf("%s: open rx pipe failed: %s\n", device_xname(sc->cdce_dev),
 		    usbd_errstr(err));
@@ -657,7 +681,7 @@ cdce_init(void *xsc)
 	}
 
 	err = usbd_open_pipe(sc->cdce_data_iface, sc->cdce_bulkout_no,
-	    USBD_EXCLUSIVE_USE, &sc->cdce_bulkout_pipe);
+	    USBD_EXCLUSIVE_USE | USBD_MPSAFE, &sc->cdce_bulkout_pipe);
 	if (err) {
 		printf("%s: open tx pipe failed: %s\n",
 		    device_xname(sc->cdce_dev), usbd_errstr(err));
@@ -674,6 +698,10 @@ cdce_init(void *xsc)
 		goto out;
 	}
 
+	mutex_enter(&sc->cdce_rxlock);
+	mutex_enter(&sc->cdce_txlock);
+	sc->cdce_stopping = false;
+
 	for (i = 0; i < CDCE_RX_LIST_CNT; i++) {
 		c = &sc->cdce_cdata.cdce_rx_chain[i];
 
@@ -682,8 +710,10 @@ cdce_init(void *xsc)
 		usbd_transfer(c->cdce_xfer);
 	}
 
+	mutex_exit(&sc->cdce_txlock);
+	mutex_exit(&sc->cdce_rxlock);
+
 	ifp->if_flags |= IFF_RUNNING;
-	ifp->if_flags &= ~IFF_OACTIVE;
 
 	callout_schedule(&sc->cdce_stat_ch, hz);
 
@@ -831,12 +861,12 @@ cdce_rxeof(struct usbd_xfer *xfer, void 
 	if_percpuq_enqueue(ifp->if_percpuq, m);
 	mutex_enter(&sc->cdce_rxlock);
 
+done:
 	if (sc->cdce_stopping || sc->cdce_dying) {
 		mutex_exit(&sc->cdce_rxlock);
 		return;
 	}
 
-done:
 	mutex_exit(&sc->cdce_rxlock);
 
 	/* Setup new transfer. */
@@ -858,8 +888,10 @@ cdce_txeof(struct usbd_xfer *xfer, void 
 	if (sc->cdce_dying)
 		goto out;
 
+	KASSERT(sc->cdce_cdata.cdce_tx_cnt > 0);
+	sc->cdce_cdata.cdce_tx_cnt--;
+
 	sc->cdce_timer = 0;
-	ifp->if_flags &= ~IFF_OACTIVE;
 
 	switch (status) {
 	case USBD_NOT_STARTED:
Index: if_ure.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/if_ure.c,v
retrieving revision 1.12
diff -p -u -r1.12 if_ure.c
--- if_ure.c	24 Jun 2019 04:42:06 -0000	1.12
+++ if_ure.c	27 Jun 2019 10:59:10 -0000
@@ -436,7 +436,6 @@ ure_iff_locked(struct ure_softc *sc)
 
 	rxmode = ure_read_4(sc, URE_PLA_RCR, URE_MCU_TYPE_PLA);
 	rxmode &= ~URE_RCR_ACPT_ALL;
-	ifp->if_flags &= ~IFF_ALLMULTI;
 
 	/*
 	 * Always accept frames destined to our station address.
@@ -446,13 +445,18 @@ ure_iff_locked(struct ure_softc *sc)
 
 	if (ifp->if_flags & IFF_PROMISC) {
 		rxmode |= URE_RCR_AAP;
-allmulti:	ifp->if_flags |= IFF_ALLMULTI;
+allmulti:	
+		ETHER_LOCK(ec);
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		rxmode |= URE_RCR_AM;
 		hashes[0] = hashes[1] = 0xffffffff;
 	} else {
 		rxmode |= URE_RCR_AM;
 
 		ETHER_LOCK(ec);
+		ec->ec_flags &= ~ETHER_F_ALLMULTI;
+
 		ETHER_FIRST_MULTI(step, ec, enm);
 		while (enm != NULL) {
 			if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
@@ -597,8 +601,8 @@ ure_init_locked(struct ifnet *ifp)
 	mutex_exit(&sc->ure_rxlock);
 
 	/* Indicate we are up and running. */
+	KASSERT(IFNET_LOCKED(ifp));
 	ifp->if_flags |= IFF_RUNNING;
-	ifp->if_flags &= ~IFF_OACTIVE;
 
 	callout_reset(&sc->ure_stat_ch, hz, ure_tick, sc);
 
@@ -625,9 +629,12 @@ ure_start_locked(struct ifnet *ifp)
 	struct ure_cdata *cd = &sc->ure_cdata;
 	int idx;
 
+	KASSERT(cd->tx_cnt <= URE_TX_LIST_CNT);
+
 	if (sc->ure_dying || sc->ure_stopping ||
 	    (sc->ure_flags & URE_FLAG_LINK) == 0 ||
-	    (ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) != IFF_RUNNING) {
+	    (ifp->if_flags & IFF_RUNNING) == 0 ||
+	    cd->tx_cnt == URE_TX_LIST_CNT) {
 		return;
 	}
 
@@ -650,9 +657,6 @@ ure_start_locked(struct ifnet *ifp)
 		cd->tx_cnt++;
 	}
 	cd->tx_prod = idx;
-
-	if (cd->tx_cnt >= URE_TX_LIST_CNT)
-		ifp->if_flags |= IFF_OACTIVE;
 }
 
 static void
@@ -698,7 +702,14 @@ ure_stop_locked(struct ifnet *ifp, int d
 
 	ure_reset(sc);
 
-	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
+	/*
+	 * XXXSMP Would like to
+	 *	KASSERT(IFNET_LOCKED(ifp))
+	 * here but the locking order is:
+	 *	ifnet -> sc lock -> rxlock -> txlock
+	 * and sc lock is already held.
+	 */
+	ifp->if_flags &= ~IFF_RUNNING;
 
 	callout_stop(&sc->ure_stat_ch);
 
@@ -1350,8 +1361,11 @@ ure_detach(device_t self, int flags)
 
 	/* partial-attach, below items weren't configured. */
 	if (sc->ure_phyno != -1) {
-		if (ifp->if_flags & IFF_RUNNING)
+		if (ifp->if_flags & IFF_RUNNING) {
+			IFNET_LOCK(ifp);
 			ure_stop(ifp, 1);
+			IFNET_UNLOCK(ifp);
+		}
 
 		rnd_detach_source(&sc->ure_rnd_source);
 		mii_detach(&sc->ure_mii, MII_PHY_ANY, MII_OFFSET_ANY);
@@ -1535,7 +1549,7 @@ ure_rxeof(struct usbd_xfer *xfer, void *
 		if_percpuq_enqueue(ifp->if_percpuq, m);
 		mutex_enter(&sc->ure_rxlock);
 
-		if (sc->ure_stopping) {
+		if (sc->ure_dying || sc->ure_stopping) {
 			mutex_exit(&sc->ure_rxlock);
 			return;
 		}
@@ -1543,6 +1557,10 @@ ure_rxeof(struct usbd_xfer *xfer, void *
 	} while (total_len > 0);
 
 done:
+	if (sc->ure_dying || sc->ure_stopping) {
+		mutex_exit(&sc->ure_rxlock);
+		return;
+	}
 	mutex_exit(&sc->ure_rxlock);
 
 	/* Setup new transfer. */
@@ -1609,8 +1627,6 @@ ure_txeof(struct usbd_xfer *xfer, void *
 	KASSERT(cd->tx_cnt > 0);
 	cd->tx_cnt--;
 
-	ifp->if_flags &= ~IFF_OACTIVE;
-
 	switch (status) {
 	case USBD_NOT_STARTED:
 	case USBD_CANCELLED:
@@ -1721,6 +1737,7 @@ ure_encap(struct ure_softc *sc, struct m
 
 	err = usbd_transfer(c->uc_xfer);
 	if (err != USBD_IN_PROGRESS) {
+		/* XXXSMP IFNET_LOCK */
 		ure_stop(ifp, 0);
 		return EIO;
 	}
Index: if_axen.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/if_axen.c,v
retrieving revision 1.48
diff -p -u -r1.48 if_axen.c
--- if_axen.c	22 Jun 2019 10:58:39 -0000	1.48
+++ if_axen.c	27 Jun 2019 10:59:10 -0000
@@ -455,12 +455,14 @@ axen_iff_locked(struct axen_softc *sc)
 	rxmode = le16toh(wval);
 	rxmode &= ~(AXEN_RXCTL_ACPT_ALL_MCAST | AXEN_RXCTL_PROMISC |
 	    AXEN_RXCTL_ACPT_MCAST);
-	ifp->if_flags &= ~IFF_ALLMULTI;
 
 	if (ifp->if_flags & IFF_PROMISC) {
 		DPRINTF(("%s: promisc\n", device_xname(sc->axen_dev)));
 		rxmode |= AXEN_RXCTL_PROMISC;
-allmulti:	ifp->if_flags |= IFF_ALLMULTI;
+allmulti:
+		ETHER_LOCK(ec);
+		ec->ec_flags |= ETHER_F_ALLMULTI;
+		ETHER_UNLOCK(ec);
 		rxmode |= AXEN_RXCTL_ACPT_ALL_MCAST
 		/* | AXEN_RXCTL_ACPT_PHY_MCAST */;
 	} else {
@@ -468,6 +470,8 @@ allmulti:	ifp->if_flags |= IFF_ALLMULTI;
 		DPRINTF(("%s: initializing hash table\n",
 		    device_xname(sc->axen_dev)));
 		ETHER_LOCK(ec);
+		ec->ec_flags &= ~ETHER_F_ALLMULTI;
+
 		ETHER_FIRST_MULTI(step, ec, enm);
 		while (enm != NULL) {
 			if (memcmp(enm->enm_addrlo, enm->enm_addrhi,
@@ -979,8 +983,11 @@ axen_detach(device_t self, int flags)
 	usb_rem_task_wait(sc->axen_udev, &sc->axen_tick_task,
 	    USB_TASKQ_DRIVER, NULL);
 
-	if (ifp->if_flags & IFF_RUNNING)
+	if (ifp->if_flags & IFF_RUNNING) {
+		IFNET_LOCK(ifp);
 		axen_stop(ifp, 1);
+		IFNET_UNLOCK(ifp);
+	}
 
 	mutex_enter(&sc->axen_lock);
 	sc->axen_refcnt--;
@@ -1256,7 +1263,7 @@ axen_rxeof(struct usbd_xfer *xfer, void 
 		if_percpuq_enqueue((ifp)->if_percpuq, (m));
 
 		mutex_enter(&sc->axen_rxlock);
-		if (sc->axen_stopping) {
+		if (sc->axen_dying || sc->axen_stopping) {
 			mutex_exit(&sc->axen_rxlock);
 			return;
 		}
@@ -1274,6 +1281,11 @@ nextpkt:
 	} while (pkt_count > 0);
 
 done:
+	if (sc->axen_dying || sc->axen_stopping) {
+		mutex_exit(&sc->axen_rxlock);
+		return;
+	}
+
 	mutex_exit(&sc->axen_rxlock);
 
 	/* Setup new transfer. */
@@ -1351,7 +1363,6 @@ axen_txeof(struct usbd_xfer *xfer, void 
 	cd->axen_tx_cnt--;
 
 	sc->axen_timer = 0;
-	ifp->if_flags &= ~IFF_OACTIVE;
 
 	switch (status) {
 	case USBD_NOT_STARTED:
@@ -1489,6 +1500,7 @@ axen_encap(struct axen_softc *sc, struct
 	/* Transmit */
 	err = usbd_transfer(c->axen_xfer);
 	if (err != USBD_IN_PROGRESS) {
+		/* XXXSMP IFNET_LOCK */
 		axen_stop(ifp, 0);
 		return EIO;
 	}
@@ -1505,11 +1517,9 @@ axen_start_locked(struct ifnet *ifp)
 	int idx;
 
 	KASSERT(mutex_owned(&sc->axen_txlock));
+	KASSERT(cd->axen_tx_cnt <= AXEN_TX_LIST_CNT);
 
-	if (sc->axen_link == 0)
-		return;
-
-	if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) != IFF_RUNNING)
+	if (sc->axen_link == 0 || (ifp->if_flags & IFF_RUNNING) == 0)
 		return;
 
 	idx = cd->axen_tx_prod;
@@ -1519,7 +1529,6 @@ axen_start_locked(struct ifnet *ifp)
 			break;
 
 		if (axen_encap(sc, m, idx)) {
-			ifp->if_flags |= IFF_OACTIVE; /* XXX */
 			ifp->if_oerrors++;
 			break;
 		}
@@ -1537,9 +1546,6 @@ axen_start_locked(struct ifnet *ifp)
 	}
 	cd->axen_tx_prod = idx;
 
-	if (cd->axen_tx_cnt >= AXEN_TX_LIST_CNT)
-		ifp->if_flags |= IFF_OACTIVE;
-
 	/*
 	 * Set a timeout in case the chip goes out to lunch.
 	 */
@@ -1646,8 +1652,8 @@ axen_init_locked(struct ifnet *ifp)
 	mutex_exit(&sc->axen_rxlock);
 
 	/* Indicate we are up and running. */
+	KASSERT(IFNET_LOCKED(ifp));
 	ifp->if_flags |= IFF_RUNNING;
-	ifp->if_flags &= ~IFF_OACTIVE;
 
 	callout_schedule(&sc->axen_stat_ch, hz);
 	return 0;
@@ -1771,8 +1777,15 @@ axen_stop_locked(struct ifnet *ifp, int 
 	axen_cmd(sc, AXEN_CMD_MAC_WRITE2, 2, AXEN_MAC_RXCTL, &wval);
 	axen_unlock_mii_sc_locked(sc);
 
+	/*
+	 * XXXSMP Would like to
+	 *	KASSERT(IFNET_LOCKED(ifp))
+	 * here but the locking order is:
+	 *	ifnet -> sc lock -> rxlock -> txlock
+	 * and sc lock is already held.
+	 */
+	ifp->if_flags &= ~IFF_RUNNING;
 	sc->axen_timer = 0;
-	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
 
 	callout_stop(&sc->axen_stat_ch);
 	sc->axen_link = 0;