Module Name:    src
Committed By:   mrg
Date:           Sun Jun 23 02:14:15 UTC 2019

Modified Files:
        src/sys/dev/usb: if_cdce.c if_ure.c if_urevar.h

Log Message:
make cdce(4) and ure(4) usb and mpsafe:

- introduce locking ala smsc(4)/axen(4) style
- convert to mpsafe interfaces
- add tick task to cdce(4) to deal with missing watchdog, and
  actually make the watchdog do something
- convert DELAY() to usbd_delay_ms() in cdce(4) and don't increase
  the time in a potentially unbounded way
- remove spl calls

tested with network cable and usb adapter pullouts, reboots and
many many GBs of data transferred in either direction under load.


To generate a diff of this commit:
cvs rdiff -u -r1.49 -r1.50 src/sys/dev/usb/if_cdce.c
cvs rdiff -u -r1.10 -r1.11 src/sys/dev/usb/if_ure.c
cvs rdiff -u -r1.2 -r1.3 src/sys/dev/usb/if_urevar.h

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/usb/if_cdce.c
diff -u src/sys/dev/usb/if_cdce.c:1.49 src/sys/dev/usb/if_cdce.c:1.50
--- src/sys/dev/usb/if_cdce.c:1.49	Sat Jun 22 04:45:04 2019
+++ src/sys/dev/usb/if_cdce.c	Sun Jun 23 02:14:14 2019
@@ -1,4 +1,4 @@
-/*	$NetBSD: if_cdce.c,v 1.49 2019/06/22 04:45:04 mrg Exp $ */
+/*	$NetBSD: if_cdce.c,v 1.50 2019/06/23 02:14:14 mrg Exp $ */
 
 /*
  * Copyright (c) 1997, 1998, 1999, 2000-2003 Bill Paul <wp...@windriver.com>
@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 1.49 2019/06/22 04:45:04 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 1.50 2019/06/23 02:14:14 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -71,6 +71,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_cdce.c,v 
 
 #include <dev/usb/usb.h>
 #include <dev/usb/usbdi.h>
+#include <dev/usb/usbdivar.h>
 #include <dev/usb/usbdi_util.h>
 #include <dev/usb/usbdevs.h>
 #include <dev/usb/usbcdc.h>
@@ -116,14 +117,23 @@ struct cdce_softc {
 	struct usbd_pipe *	 cdce_bulkin_pipe;
 	int			 cdce_bulkout_no;
 	struct usbd_pipe *	 cdce_bulkout_pipe;
-	char			 cdce_dying;
 	int			 cdce_unit;
 	struct cdce_cdata	 cdce_cdata;
 	int			 cdce_rxeof_errors;
 	uint16_t		 cdce_flags;
-	char			 cdce_attached;
+	uint16_t		 cdce_timer;
 
-	kmutex_t		 cdce_start_lock;
+	bool			 cdce_attached;
+	bool			 cdce_dying;
+	bool			 cdce_stopping;
+
+	struct usb_task		 cdce_tick_task;
+	struct callout		 cdce_stat_ch;
+
+	kmutex_t		 cdce_lock;
+	kmutex_t		 cdce_mii_lock;
+	kmutex_t		 cdce_rxlock;
+	kmutex_t		 cdce_txlock;
 };
 
 static int	 cdce_tx_list_init(struct cdce_softc *);
@@ -138,6 +148,8 @@ static int	 cdce_ioctl(struct ifnet *, u
 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 *);
 
 static const struct cdce_type cdce_devs[] = {
   {{ USB_VENDOR_ACERLABS, USB_PRODUCT_ACERLABS_M5632 }, CDCE_NO_UNION },
@@ -184,7 +196,6 @@ cdce_attach(device_t parent, device_t se
 	struct cdce_softc *sc = device_private(self);
 	struct usbif_attach_arg *uiaa = aux;
 	char				*devinfop;
-	int				 s;
 	struct ifnet			*ifp;
 	struct usbd_device	        *dev = uiaa->uiaa_device;
 	const struct cdce_type		*t;
@@ -319,16 +330,20 @@ cdce_attach(device_t parent, device_t se
 		eaddr[5] = (uint8_t)(device_unit(sc->cdce_dev));
 	}
 
-	s = splnet();
+	mutex_init(&sc->cdce_txlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+	mutex_init(&sc->cdce_rxlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+	mutex_init(&sc->cdce_lock, MUTEX_DEFAULT, IPL_NONE);
+
+	usb_init_task(&sc->cdce_tick_task, cdce_tick_task, sc, USB_TASKQ_MPSAFE);
 
 	aprint_normal_dev(self, "address %s\n", ether_sprintf(eaddr));
 
 	ifp = GET_IFP(sc);
 	ifp->if_softc = sc;
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+	ifp->if_extflags = IFEF_MPSAFE;
 	ifp->if_ioctl = cdce_ioctl;
 	ifp->if_start = cdce_start;
-	ifp->if_watchdog = cdce_watchdog;
 	strlcpy(ifp->if_xname, device_xname(sc->cdce_dev), IFNAMSIZ);
 
 	IFQ_SET_READY(&ifp->if_snd);
@@ -336,8 +351,10 @@ cdce_attach(device_t parent, device_t se
 	if_attach(ifp);
 	ether_ifattach(ifp, eaddr);
 
-	sc->cdce_attached = 1;
-	splx(s);
+	callout_init(&sc->cdce_stat_ch, CALLOUT_MPSAFE);
+	callout_setfunc(&sc->cdce_stat_ch, cdce_tick, sc);
+
+	sc->cdce_attached = true;
 
 	usbd_add_drv_event(USB_EVENT_DRIVER_ATTACH, sc->cdce_udev,
 	    sc->cdce_dev);
@@ -353,37 +370,46 @@ cdce_detach(device_t self, int flags)
 {
 	struct cdce_softc *sc = device_private(self);
 	struct ifnet	*ifp = GET_IFP(sc);
-	int		 s;
-
-	pmf_device_deregister(self);
 
-	s = splusb();
+	mutex_enter(&sc->cdce_lock);
+	sc->cdce_dying = true;
+	mutex_exit(&sc->cdce_lock);
 
-	if (!sc->cdce_attached) {
-		splx(s);
+	if (!sc->cdce_attached)
 		return 0;
-	}
+
+	pmf_device_deregister(self);
+
+	callout_halt(&sc->cdce_stat_ch, NULL);
+	usb_rem_task_wait(sc->cdce_udev, &sc->cdce_tick_task,
+	    USB_TASKQ_DRIVER, NULL);
 
 	if (ifp->if_flags & IFF_RUNNING)
 		cdce_stop(sc);
 
+	callout_destroy(&sc->cdce_stat_ch);
 	ether_ifdetach(ifp);
-
 	if_detach(ifp);
+	sc->cdce_attached = false;
 
-	sc->cdce_attached = 0;
-	splx(s);
+	usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->cdce_udev,sc->cdce_dev);
+
+	mutex_destroy(&sc->cdce_lock);
+	mutex_destroy(&sc->cdce_rxlock);
+	mutex_destroy(&sc->cdce_txlock);
 
 	return 0;
 }
 
 static void
-cdce_start(struct ifnet *ifp)
+cdce_start_locked(struct ifnet *ifp)
 {
 	struct cdce_softc	*sc = ifp->if_softc;
 	struct mbuf		*m_head = NULL;
 
-	if (sc->cdce_dying || (ifp->if_flags & IFF_OACTIVE))
+	KASSERT(mutex_owned(&sc->cdce_txlock));
+	if (sc->cdce_dying || sc->cdce_stopping ||
+	    (ifp->if_flags & IFF_OACTIVE))
 		return;
 
 	IFQ_POLL(&ifp->if_snd, m_head);
@@ -401,9 +427,22 @@ cdce_start(struct ifnet *ifp)
 
 	ifp->if_flags |= IFF_OACTIVE;
 
+	/*
+	 * Set a timeout in case the chip goes out to lunch.
+	 */
 	ifp->if_timer = 6;
 }
 
+static void
+cdce_start(struct ifnet *ifp)
+{
+	struct cdce_softc * const sc = ifp->if_softc;
+
+	mutex_enter(&sc->cdce_txlock);
+	cdce_start_locked(ifp);
+	mutex_exit(&sc->cdce_txlock);
+}
+
 static int
 cdce_encap(struct cdce_softc *sc, struct mbuf *m, int idx)
 {
@@ -411,6 +450,8 @@ cdce_encap(struct cdce_softc *sc, struct
 	usbd_status		 err;
 	int			 extra = 0;
 
+	KASSERT(mutex_owned(&sc->cdce_txlock));
+
 	c = &sc->cdce_cdata.cdce_tx_chain[idx];
 
 	m_copydata(m, 0, m->m_pkthdr.len, c->cdce_buf);
@@ -444,7 +485,16 @@ cdce_stop(struct cdce_softc *sc)
 	struct ifnet	*ifp = GET_IFP(sc);
 	int		 i;
 
+	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);
+
+	callout_stop(&sc->cdce_stat_ch);
 
 	if (sc->cdce_bulkin_pipe != NULL) {
 		err = usbd_abort_pipe(sc->cdce_bulkin_pipe);
@@ -499,8 +549,7 @@ cdce_stop(struct cdce_softc *sc)
 			    device_xname(sc->cdce_dev), usbd_errstr(err));
 		sc->cdce_bulkout_pipe = NULL;
 	}
-
-	ifp->if_flags &= ~(IFF_RUNNING | IFF_OACTIVE);
+	mutex_exit(&sc->cdce_txlock);
 }
 
 static int
@@ -509,13 +558,11 @@ cdce_ioctl(struct ifnet *ifp, u_long com
 	struct cdce_softc	*sc = ifp->if_softc;
 	struct ifaddr		*ifa = (struct ifaddr *)data;
 	struct ifreq		*ifr = (struct ifreq *)data;
-	int			 s, error = 0;
+	int			 error = 0;
 
 	if (sc->cdce_dying)
 		return EIO;
 
-	s = splnet();
-
 	switch(command) {
 	case SIOCINITIFADDR:
 		ifp->if_flags |= IFF_UP;
@@ -557,8 +604,6 @@ cdce_ioctl(struct ifnet *ifp, u_long com
 		break;
 	}
 
-	splx(s);
-
 	if (error == ENETRESET)
 		error = 0;
 
@@ -569,12 +614,23 @@ static void
 cdce_watchdog(struct ifnet *ifp)
 {
 	struct cdce_softc	*sc = ifp->if_softc;
+	struct cdce_chain *c;
+	usbd_status stat;
+
+	KASSERT(mutex_owned(&sc->cdce_lock));
 
 	if (sc->cdce_dying)
 		return;
 
 	ifp->if_oerrors++;
-	printf("%s: watchdog timeout\n", device_xname(sc->cdce_dev));
+	aprint_error_dev(sc->cdce_dev, "watchdog timeout\n");
+
+	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);
 }
 
 static void
@@ -584,12 +640,11 @@ cdce_init(void *xsc)
 	struct ifnet		*ifp = GET_IFP(sc);
 	struct cdce_chain	*c;
 	usbd_status		 err;
-	int			 s, i;
+	int			 i;
 
+	mutex_enter(&sc->cdce_lock);
 	if (ifp->if_flags & IFF_RUNNING)
-		return;
-
-	s = splnet();
+		goto out;
 
 	/* Maybe set multicast / broadcast here??? */
 
@@ -598,8 +653,7 @@ cdce_init(void *xsc)
 	if (err) {
 		printf("%s: open rx pipe failed: %s\n", device_xname(sc->cdce_dev),
 		    usbd_errstr(err));
-		splx(s);
-		return;
+		goto out;
 	}
 
 	err = usbd_open_pipe(sc->cdce_data_iface, sc->cdce_bulkout_no,
@@ -607,20 +661,17 @@ cdce_init(void *xsc)
 	if (err) {
 		printf("%s: open tx pipe failed: %s\n",
 		    device_xname(sc->cdce_dev), usbd_errstr(err));
-		splx(s);
-		return;
+		goto out;
 	}
 
 	if (cdce_tx_list_init(sc)) {
 		printf("%s: tx list init failed\n", device_xname(sc->cdce_dev));
-		splx(s);
-		return;
+		goto out;
 	}
 
 	if (cdce_rx_list_init(sc)) {
 		printf("%s: rx list init failed\n", device_xname(sc->cdce_dev));
-		splx(s);
-		return;
+		goto out;
 	}
 
 	for (i = 0; i < CDCE_RX_LIST_CNT; i++) {
@@ -634,7 +685,10 @@ cdce_init(void *xsc)
 	ifp->if_flags |= IFF_RUNNING;
 	ifp->if_flags &= ~IFF_OACTIVE;
 
-	splx(s);
+	callout_schedule(&sc->cdce_stat_ch, hz);
+
+out:
+	mutex_exit(&sc->cdce_lock);
 }
 
 static int
@@ -726,20 +780,23 @@ cdce_rxeof(struct usbd_xfer *xfer, void 
 	struct ifnet		*ifp = GET_IFP(sc);
 	struct mbuf		*m;
 	int			 total_len = 0;
-	int			 s;
 
-	if (sc->cdce_dying || !(ifp->if_flags & IFF_RUNNING))
+	mutex_enter(&sc->cdce_rxlock);
+
+	if (sc->cdce_dying || sc->cdce_stopping ||
+	    status == USBD_INVAL || status == USBD_NOT_STARTED ||
+	    status == USBD_CANCELLED || !(ifp->if_flags & IFF_RUNNING)) {
+		mutex_exit(&sc->cdce_rxlock);
 		return;
+	}
 
 	if (status != USBD_NORMAL_COMPLETION) {
-		if (status == USBD_NOT_STARTED || status == USBD_CANCELLED)
-			return;
 		if (sc->cdce_rxeof_errors == 0)
 			printf("%s: usb error on rx: %s\n",
 			    device_xname(sc->cdce_dev), usbd_errstr(status));
 		if (status == USBD_STALLED)
 			usbd_clear_endpoint_stall_async(sc->cdce_bulkin_pipe);
-		DELAY(sc->cdce_rxeof_errors * 10000);
+		usbd_delay_ms(sc->cdce_udev, 10);
 		sc->cdce_rxeof_errors++;
 		goto done;
 	}
@@ -749,8 +806,10 @@ cdce_rxeof(struct usbd_xfer *xfer, void 
 	usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL);
 	if (sc->cdce_flags & CDCE_ZAURUS)
 		total_len -= 4;	/* Strip off CRC added by Zaurus */
-	if (total_len <= 1)
+	if (total_len <= 1) {
+		ifp->if_ierrors++;
 		goto done;
+	}
 
 	m = c->cdce_mbuf;
 	memcpy(mtod(m, char *), c->cdce_buf, total_len);
@@ -763,19 +822,23 @@ cdce_rxeof(struct usbd_xfer *xfer, void 
 	m->m_pkthdr.len = m->m_len = total_len;
 	m_set_rcvif(m, ifp);
 
-	s = splnet();
-
 	if (cdce_newbuf(sc, c, NULL) == ENOBUFS) {
 		ifp->if_ierrors++;
-		goto done1;
+		goto done;
 	}
 
-	if_percpuq_enqueue((ifp)->if_percpuq, (m));
+	mutex_exit(&sc->cdce_rxlock);
+	if_percpuq_enqueue(ifp->if_percpuq, m);
+	mutex_enter(&sc->cdce_rxlock);
 
-done1:
-	splx(s);
+	if (sc->cdce_stopping || sc->cdce_dying) {
+		mutex_exit(&sc->cdce_rxlock);
+		return;
+	}
 
 done:
+	mutex_exit(&sc->cdce_rxlock);
+
 	/* Setup new transfer. */
 	usbd_setup_xfer(c->cdce_xfer, c, c->cdce_buf, CDCE_BUFSZ,
 	    USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, cdce_rxeof);
@@ -783,35 +846,36 @@ done:
 }
 
 static void
-cdce_txeof(struct usbd_xfer *xfer, void *priv,
-    usbd_status status)
+cdce_txeof(struct usbd_xfer *xfer, void *priv, usbd_status status)
 {
 	struct cdce_chain	*c = priv;
 	struct cdce_softc	*sc = c->cdce_sc;
 	struct ifnet		*ifp = GET_IFP(sc);
 	usbd_status		 err;
-	int			 s;
 
-	if (sc->cdce_dying)
-		return;
+	mutex_enter(&sc->cdce_txlock);
 
-	s = splnet();
+	if (sc->cdce_dying)
+		goto out;
 
-	ifp->if_timer = 0;
+	sc->cdce_timer = 0;
 	ifp->if_flags &= ~IFF_OACTIVE;
 
-	if (status != USBD_NORMAL_COMPLETION) {
-		if (status == USBD_NOT_STARTED || status == USBD_CANCELLED) {
-			splx(s);
-			return;
-		}
+	switch (status) {
+	case USBD_NOT_STARTED:
+	case USBD_CANCELLED:
+		goto out;
+
+	case USBD_NORMAL_COMPLETION:
+		break;
+
+	default:
 		ifp->if_oerrors++;
 		printf("%s: usb error on tx: %s\n", device_xname(sc->cdce_dev),
 		    usbd_errstr(status));
 		if (status == USBD_STALLED)
 			usbd_clear_endpoint_stall_async(sc->cdce_bulkout_pipe);
-		splx(s);
-		return;
+		goto out;
 	}
 
 	usbd_get_xfer_status(c->cdce_xfer, NULL, NULL, NULL, &err);
@@ -827,9 +891,57 @@ cdce_txeof(struct usbd_xfer *xfer, void 
 		ifp->if_opackets++;
 
 	if (IFQ_IS_EMPTY(&ifp->if_snd) == 0)
-		cdce_start(ifp);
+		cdce_start_locked(ifp);
+
+out:
+	mutex_exit(&sc->cdce_txlock);
+}
+
+static void
+cdce_tick(void *xsc)
+{
+	struct cdce_softc * const sc = xsc;
+
+	if (sc == NULL)
+		return;
+
+	mutex_enter(&sc->cdce_lock);
+
+	if (sc->cdce_stopping || sc->cdce_dying) {
+		mutex_exit(&sc->cdce_lock);
+		return;
+	}
+
+	/* Perform periodic stuff in process context */
+	usb_add_task(sc->cdce_udev, &sc->cdce_tick_task, USB_TASKQ_DRIVER);
+
+	mutex_exit(&sc->cdce_lock);
+}
+
+static void
+cdce_tick_task(void *xsc)
+{
+	struct cdce_softc * const sc = xsc;
+	struct ifnet *ifp;
 
-	splx(s);
+	if (sc == NULL)
+		return;
+
+	mutex_enter(&sc->cdce_lock);
+
+	if (sc->cdce_stopping || sc->cdce_dying) {
+		mutex_exit(&sc->cdce_lock);
+		return;
+	}
+
+	ifp = GET_IFP(sc);
+
+	if (sc->cdce_timer != 0 && --sc->cdce_timer == 0)
+		cdce_watchdog(ifp);
+	if (!sc->cdce_stopping && !sc->cdce_dying)
+		callout_schedule(&sc->cdce_stat_ch, hz);
+
+	mutex_exit(&sc->cdce_lock);
 }
 
 int
@@ -840,7 +952,17 @@ cdce_activate(device_t self, enum devact
 	switch (act) {
 	case DVACT_DEACTIVATE:
 		if_deactivate(GET_IFP(sc));
-		sc->cdce_dying = 1;
+
+		mutex_enter(&sc->cdce_lock);
+		sc->cdce_dying = true;
+		mutex_exit(&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);
+
 		return 0;
 	default:
 		return EOPNOTSUPP;

Index: src/sys/dev/usb/if_ure.c
diff -u src/sys/dev/usb/if_ure.c:1.10 src/sys/dev/usb/if_ure.c:1.11
--- src/sys/dev/usb/if_ure.c:1.10	Sun Jun 16 21:04:08 2019
+++ src/sys/dev/usb/if_ure.c	Sun Jun 23 02:14:14 2019
@@ -1,4 +1,5 @@
-/*	$NetBSD: if_ure.c,v 1.10 2019/06/16 21:04:08 christos Exp $	*/
+/*	$NetBSD: if_ure.c,v 1.11 2019/06/23 02:14:14 mrg Exp $	*/
+
 /*	$OpenBSD: if_ure.c,v 1.10 2018/11/02 21:32:30 jcs Exp $	*/
 /*-
  * Copyright (c) 2015-2016 Kevin Lo <ke...@freebsd.org>
@@ -29,7 +30,7 @@
 /* RealTek RTL8152/RTL8153 10/100/Gigabit USB Ethernet device */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_ure.c,v 1.10 2019/06/16 21:04:08 christos Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ure.c,v 1.11 2019/06/23 02:14:14 mrg Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -312,8 +313,12 @@ ure_miibus_readreg(device_t dev, int phy
 {
 	struct ure_softc *sc = device_private(dev);
 
-	if (sc->ure_dying || sc->ure_phyno != phy) /* XXX */
+	mutex_enter(&sc->ure_lock);
+	if (sc->ure_dying || sc->ure_phyno != phy) {
+		mutex_exit(&sc->ure_lock);
 		return -1;
+	}
+	mutex_exit(&sc->ure_lock);
 
 	/* Let the rgephy driver read the URE_PLA_PHYSTATUS register. */
 	if (reg == RTK_GMEDIASTAT) {
@@ -333,8 +338,12 @@ ure_miibus_writereg(device_t dev, int ph
 {
 	struct ure_softc *sc = device_private(dev);
 
-	if (sc->ure_dying || sc->ure_phyno != phy) /* XXX */
+	mutex_enter(&sc->ure_lock);
+	if (sc->ure_dying || sc->ure_phyno != phy) {
+		mutex_exit(&sc->ure_lock);
 		return -1;
+	}
+	mutex_exit(&sc->ure_lock);
 
 	ure_lock_mii(sc);
 	ure_ocp_reg_write(sc, URE_OCP_BASE_MII + reg * 2, val);
@@ -410,7 +419,7 @@ ure_ifmedia_sts(struct ifnet *ifp, struc
 }
 
 static void
-ure_iff(struct ure_softc *sc)
+ure_iff_locked(struct ure_softc *sc)
 {
 	struct ethercom *ec = &sc->ure_ec;
 	struct ifnet *ifp = GET_IFP(sc);
@@ -420,6 +429,8 @@ ure_iff(struct ure_softc *sc)
 	uint32_t hash;
 	uint32_t rxmode;
 
+	KASSERT(mutex_owned(&sc->ure_lock));
+
 	if (sc->ure_dying)
 		return;
 
@@ -472,10 +483,21 @@ allmulti:	ifp->if_flags |= IFF_ALLMULTI;
 }
 
 static void
+ure_iff(struct ure_softc *sc)
+{
+
+	mutex_enter(&sc->ure_lock);
+	ure_iff_locked(sc);
+	mutex_exit(&sc->ure_lock);
+}
+
+static void
 ure_reset(struct ure_softc *sc)
 {
 	int i;
 
+	KASSERT(mutex_owned(&sc->ure_lock));
+
 	ure_write_1(sc, URE_PLA_CR, URE_MCU_TYPE_PLA, URE_CR_RST);
 
 	for (i = 0; i < URE_TIMEOUT; i++) {
@@ -489,15 +511,18 @@ ure_reset(struct ure_softc *sc)
 }
 
 static int
-ure_init(struct ifnet *ifp)
+ure_init_locked(struct ifnet *ifp)
 {
-	struct ure_softc *sc = ifp->if_softc;
+	struct ure_softc * const sc = ifp->if_softc;
 	struct ure_chain *c;
 	usbd_status err;
-	int s, i;
+	int i;
 	uint8_t eaddr[8];
 
-	s = splnet();
+	KASSERT(mutex_owned(&sc->ure_lock));
+
+	if (sc->ure_stopping || sc->ure_dying)
+		return EIO;
 
 	/* Cancel pending I/O. */
 	if (ifp->if_flags & IFF_RUNNING)
@@ -529,37 +554,37 @@ ure_init(struct ifnet *ifp)
 	    ~URE_RXDY_GATED_EN);
 
 	/* Load the multicast filter. */
-	ure_iff(sc);
+	ure_iff_locked(sc);
 
 	/* Open RX and TX pipes. */
 	err = usbd_open_pipe(sc->ure_iface, sc->ure_ed[URE_ENDPT_RX],
-	    USBD_EXCLUSIVE_USE, &sc->ure_ep[URE_ENDPT_RX]);
+	    USBD_EXCLUSIVE_USE | USBD_MPSAFE, &sc->ure_ep[URE_ENDPT_RX]);
 	if (err) {
 		URE_PRINTF(sc, "open rx pipe failed: %s\n", usbd_errstr(err));
-		splx(s);
 		return EIO;
 	}
 
 	err = usbd_open_pipe(sc->ure_iface, sc->ure_ed[URE_ENDPT_TX],
-	    USBD_EXCLUSIVE_USE, &sc->ure_ep[URE_ENDPT_TX]);
+	    USBD_EXCLUSIVE_USE | USBD_MPSAFE, &sc->ure_ep[URE_ENDPT_TX]);
 	if (err) {
 		URE_PRINTF(sc, "open tx pipe failed: %s\n", usbd_errstr(err));
-		splx(s);
 		return EIO;
 	}
 
 	if (ure_rx_list_init(sc)) {
 		URE_PRINTF(sc, "rx list init failed\n");
-		splx(s);
 		return ENOBUFS;
 	}
 
 	if (ure_tx_list_init(sc)) {
 		URE_PRINTF(sc, "tx list init failed\n");
-		splx(s);
 		return ENOBUFS;
 	}
 
+	mutex_enter(&sc->ure_rxlock);
+	mutex_enter(&sc->ure_txlock);
+	sc->ure_stopping = false;
+
 	/* Start up the receive pipe. */
 	for (i = 0; i < URE_RX_LIST_CNT; i++) {
 		c = &sc->ure_cdata.rx_chain[i];
@@ -568,26 +593,40 @@ ure_init(struct ifnet *ifp)
 		usbd_transfer(c->uc_xfer);
 	}
 
+	mutex_exit(&sc->ure_txlock);
+	mutex_exit(&sc->ure_rxlock);
+
 	/* Indicate we are up and running. */
 	ifp->if_flags |= IFF_RUNNING;
 	ifp->if_flags &= ~IFF_OACTIVE;
 
-	splx(s);
-
 	callout_reset(&sc->ure_stat_ch, hz, ure_tick, sc);
 
 	return 0;
 }
 
+static int
+ure_init(struct ifnet *ifp)
+{
+	struct ure_softc * const sc = ifp->if_softc;
+
+	mutex_enter(&sc->ure_lock);
+	int ret = ure_init_locked(ifp);
+	mutex_exit(&sc->ure_lock);
+
+	return ret;
+}
+
 static void
-ure_start(struct ifnet *ifp)
+ure_start_locked(struct ifnet *ifp)
 {
 	struct ure_softc *sc = ifp->if_softc;
 	struct mbuf *m;
 	struct ure_cdata *cd = &sc->ure_cdata;
 	int idx;
 
-	if ((sc->ure_flags & URE_FLAG_LINK) == 0 ||
+	if (sc->ure_dying || sc->ure_stopping ||
+	    (sc->ure_flags & URE_FLAG_LINK) == 0 ||
 	    (ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) != IFF_RUNNING) {
 		return;
 	}
@@ -617,6 +656,16 @@ ure_start(struct ifnet *ifp)
 }
 
 static void
+ure_start(struct ifnet *ifp)
+{
+	struct ure_softc * const sc = ifp->if_softc;
+
+	mutex_enter(&sc->ure_txlock);
+	ure_start_locked(ifp);
+	mutex_exit(&sc->ure_txlock);
+}
+
+static void
 ure_tick(void *xsc)
 {
 	struct ure_softc *sc = xsc;
@@ -624,14 +673,16 @@ ure_tick(void *xsc)
 	if (sc == NULL)
 		return;
 
-	if (sc->ure_dying)
-		return;
-
-	usb_add_task(sc->ure_udev, &sc->ure_tick_task, USB_TASKQ_DRIVER);
+	mutex_enter(&sc->ure_lock);
+	if (!sc->ure_stopping && !sc->ure_dying) {
+		/* Perform periodic stuff in process context */
+		usb_add_task(sc->ure_udev, &sc->ure_tick_task, USB_TASKQ_DRIVER);
+	}
+	mutex_exit(&sc->ure_lock);
 }
 
 static void
-ure_stop(struct ifnet *ifp, int disable __unused)
+ure_stop_locked(struct ifnet *ifp, int disable __unused)
 {
 	struct ure_softc *sc = ifp->if_softc;
 	struct ure_chain *c;
@@ -694,6 +745,16 @@ ure_stop(struct ifnet *ifp, int disable 
 }
 
 static void
+ure_stop(struct ifnet *ifp, int disable __unused)
+{
+	struct ure_softc * const sc = ifp->if_softc;
+
+	mutex_enter(&sc->ure_lock);
+	ure_stop_locked(ifp, disable);
+	mutex_exit(&sc->ure_lock);
+}
+
+static void
 ure_rtl8152_init(struct ure_softc *sc)
 {
 	uint32_t pwrctrl;
@@ -1018,9 +1079,7 @@ int
 ure_ioctl(struct ifnet *ifp, u_long cmd, void *data)
 {
 	struct ure_softc *sc = ifp->if_softc;
-	int s, error = 0, oflags = ifp->if_flags;
-
-	s = splnet();
+	int error = 0, oflags = ifp->if_flags;
 
 	switch (cmd) {
 	case SIOCSIFFLAGS:
@@ -1056,8 +1115,6 @@ ure_ioctl(struct ifnet *ifp, u_long cmd,
 		}
 	}
 
-	splx(s);
-
 	return error;
 }
 
@@ -1080,7 +1137,7 @@ ure_attach(device_t parent, device_t sel
 	usb_endpoint_descriptor_t *ed;
 	struct ifnet *ifp;
 	struct mii_data *mii;
-	int error, i, s;
+	int error, i;
 	uint16_t ver;
 	uint8_t eaddr[8]; /* 2byte padded */
 	char *devinfop;
@@ -1095,9 +1152,13 @@ ure_attach(device_t parent, device_t sel
 	aprint_normal_dev(self, "%s\n", devinfop);
 	usbd_devinfo_free(devinfop);
 
-	callout_init(&sc->ure_stat_ch, 0);
-	usb_init_task(&sc->ure_tick_task, ure_tick_task, sc, 0);
+	callout_init(&sc->ure_stat_ch, CALLOUT_MPSAFE);
+	usb_init_task(&sc->ure_tick_task, ure_tick_task, sc, USB_TASKQ_MPSAFE);
 	mutex_init(&sc->ure_mii_lock, MUTEX_DEFAULT, IPL_NONE);
+	mutex_init(&sc->ure_txlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+	mutex_init(&sc->ure_rxlock, MUTEX_DEFAULT, IPL_SOFTUSB);
+	mutex_init(&sc->ure_lock, MUTEX_DEFAULT, IPL_NONE);
+	cv_init(&sc->ure_detachcv, "uredet");
 
 	/*
 	 * ure_phyno is set to 0 below when configuration has succeeded.
@@ -1143,8 +1204,6 @@ ure_attach(device_t parent, device_t sel
 		}
 	}
 
-	s = splnet();
-
 	sc->ure_phyno = 0;
 
 	ver = ure_read_2(sc, URE_PLA_TCR1, URE_MCU_TYPE_PLA) & URE_VERSION_MASK;
@@ -1176,6 +1235,7 @@ ure_attach(device_t parent, device_t sel
 	    (sc->ure_chip != 0) ? "" : "unknown ",
 	    ver);
 
+	mutex_enter(&sc->ure_lock);
 	if (sc->ure_flags & URE_FLAG_8152)
 		ure_rtl8152_init(sc);
 	else
@@ -1187,6 +1247,7 @@ ure_attach(device_t parent, device_t sel
 	else
 		ure_read_mem(sc, URE_PLA_BACKUP, URE_MCU_TYPE_PLA, eaddr,
 		    sizeof(eaddr));
+	mutex_exit(&sc->ure_lock);
 
 	aprint_normal_dev(self, "Ethernet address %s\n", ether_sprintf(eaddr));
 
@@ -1194,6 +1255,7 @@ ure_attach(device_t parent, device_t sel
 	ifp->if_softc = sc;
 	strlcpy(ifp->if_xname, device_xname(sc->ure_dev), IFNAMSIZ);
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+	ifp->if_extflags = IFEF_MPSAFE;
 	ifp->if_init = ure_init;
 	ifp->if_ioctl = ure_ioctl;
 	ifp->if_start = ure_start;
@@ -1243,8 +1305,6 @@ ure_attach(device_t parent, device_t sel
 	rnd_attach_source(&sc->ure_rnd_source, device_xname(sc->ure_dev),
 	    RND_TYPE_NET, RND_FLAG_DEFAULT);
 
-	splx(s);
-
 	usbd_add_drv_event(USB_EVENT_DRIVER_ATTACH, sc->ure_udev, sc->ure_dev);
 
 	if (!pmf_device_register(self, NULL, NULL))
@@ -1256,23 +1316,30 @@ ure_detach(device_t self, int flags)
 {
 	struct ure_softc *sc = device_private(self);
 	struct ifnet *ifp = GET_IFP(sc);
-	int s;
 
 	pmf_device_deregister(self);
 
+	mutex_enter(&sc->ure_lock);
 	sc->ure_dying = true;
+	mutex_exit(&sc->ure_lock);
 
 	callout_halt(&sc->ure_stat_ch, NULL);
 
+	usb_rem_task_wait(sc->ure_udev, &sc->ure_tick_task, USB_TASKQ_DRIVER,
+	    NULL);
+
 	if (sc->ure_ep[URE_ENDPT_TX] != NULL)
 		usbd_abort_pipe(sc->ure_ep[URE_ENDPT_TX]);
 	if (sc->ure_ep[URE_ENDPT_RX] != NULL)
 		usbd_abort_pipe(sc->ure_ep[URE_ENDPT_RX]);
 
-	usb_rem_task_wait(sc->ure_udev, &sc->ure_tick_task, USB_TASKQ_DRIVER,
-	    NULL);
-
-	s = splusb();
+	mutex_enter(&sc->ure_lock);
+	sc->ure_refcnt--;
+	while (sc->ure_refcnt > 0) {
+		/* Wait for processes to go away */
+		cv_wait(&sc->ure_detachcv, &sc->ure_lock);
+	}
+	mutex_exit(&sc->ure_lock);
 
 	/* partial-attach, below items weren't configured. */
 	if (sc->ure_phyno != -1) {
@@ -1288,17 +1355,15 @@ ure_detach(device_t self, int flags)
 		}
 	}
 
-	if (--sc->ure_refcnt >= 0) {
-		/* Wait for processes to go away. */
-		usb_detach_waitold(sc->ure_dev);
-	}
-	splx(s);
+	usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->ure_udev, sc->ure_dev);
 
 	callout_destroy(&sc->ure_stat_ch);
+	cv_destroy(&sc->ure_detachcv);
+	mutex_destroy(&sc->ure_lock);
+	mutex_destroy(&sc->ure_rxlock);
+	mutex_destroy(&sc->ure_txlock);
 	mutex_destroy(&sc->ure_mii_lock);
 
-	usbd_add_drv_event(USB_EVENT_DRIVER_DETACH, sc->ure_udev, sc->ure_dev);
-
 	return 0;
 }
 
@@ -1311,7 +1376,17 @@ ure_activate(device_t self, enum devact 
 	switch (act) {
 	case DVACT_DEACTIVATE:
 		if_deactivate(ifp);
+
+		mutex_enter(&sc->ure_lock);
 		sc->ure_dying = true;
+		mutex_exit(&sc->ure_lock);
+
+		mutex_enter(&sc->ure_rxlock);
+		mutex_enter(&sc->ure_txlock);
+		sc->ure_stopping = true;
+		mutex_exit(&sc->ure_txlock);
+		mutex_exit(&sc->ure_rxlock);
+
 		return 0;
 	default:
 		return EOPNOTSUPP;
@@ -1323,31 +1398,49 @@ static void
 ure_tick_task(void *xsc)
 {
 	struct ure_softc *sc = xsc;
-	struct ifnet *ifp = GET_IFP(sc);
+	struct ifnet *ifp;
 	struct mii_data *mii;
-	int s;
 
 	if (sc == NULL)
 		return;
 
-	if (sc->ure_dying)
+	mutex_enter(&sc->ure_lock);
+	if (sc->ure_stopping || sc->ure_dying) {
+		mutex_exit(&sc->ure_lock);
 		return;
+	}
 
+	ifp = GET_IFP(sc);
 	mii = GET_MII(sc);
+	if (mii == NULL) {
+		mutex_exit(&sc->ure_lock);
+		return;
+	}
+
+	sc->ure_refcnt++;
+	mutex_exit(&sc->ure_lock);
 
-	s = splnet();
 	mii_tick(mii);
+
 	if ((sc->ure_flags & URE_FLAG_LINK) == 0)
 		ure_miibus_statchg(ifp);
-	callout_reset(&sc->ure_stat_ch, hz, ure_tick, sc);
-	splx(s);
+
+	mutex_enter(&sc->ure_lock);
+	if (--sc->ure_refcnt < 0)
+		cv_broadcast(&sc->ure_detachcv);
+	if (!sc->ure_stopping && !sc->ure_dying)
+		callout_schedule(&sc->ure_stat_ch, hz);
+	mutex_exit(&sc->ure_lock);
 }
 
 static void
 ure_lock_mii(struct ure_softc *sc)
 {
 
+	mutex_enter(&sc->ure_lock);
 	sc->ure_refcnt++;
+	mutex_exit(&sc->ure_lock);
+
 	mutex_enter(&sc->ure_mii_lock);
 }
 
@@ -1356,8 +1449,10 @@ ure_unlock_mii(struct ure_softc *sc)
 {
 
 	mutex_exit(&sc->ure_mii_lock);
+	mutex_enter(&sc->ure_lock);
 	if (--sc->ure_refcnt < 0)
-		usb_detach_wakeupold(sc->ure_dev);
+		cv_broadcast(&sc->ure_detachcv);
+	mutex_exit(&sc->ure_lock);
 }
 
 static void
@@ -1370,20 +1465,18 @@ ure_rxeof(struct usbd_xfer *xfer, void *
 	uint32_t total_len;
 	uint16_t pktlen = 0;
 	struct mbuf *m;
-	int s;
 	struct ure_rxpkt rxhdr;
 
-	if (sc->ure_dying)
-		return;
+	mutex_enter(&sc->ure_rxlock);
 
-	if (!(ifp->if_flags & IFF_RUNNING))
+	if (sc->ure_dying || sc->ure_stopping ||
+	    status == USBD_INVAL || status == USBD_NOT_STARTED ||
+	    status == USBD_CANCELLED || !(ifp->if_flags & IFF_RUNNING)) {
+		mutex_exit(&sc->ure_rxlock);
 		return;
+	}
 
 	if (status != USBD_NORMAL_COMPLETION) {
-		if (status == USBD_INVAL)
-			return;	/* XXX plugged out or down */
-		if (status == USBD_NOT_STARTED || status == USBD_CANCELLED)
-			return;
 		if (usbd_ratecheck(&sc->ure_rx_notice))
 			URE_PRINTF(sc, "usb errors on rx: %s\n",
 			    usbd_errstr(status));
@@ -1431,12 +1524,21 @@ ure_rxeof(struct usbd_xfer *xfer, void *
 
 		m->m_pkthdr.csum_flags = ure_rxcsum(ifp, &rxhdr);
 
-		s = splnet();
+		mutex_exit(&sc->ure_rxlock);
 		if_percpuq_enqueue(ifp->if_percpuq, m);
-		splx(s);
+		mutex_enter(&sc->ure_rxlock);
+
+		if (sc->ure_stopping) {
+			mutex_exit(&sc->ure_rxlock);
+			return;
+		}
+		
 	} while (total_len > 0);
 
 done:
+	mutex_exit(&sc->ure_rxlock);
+
+	/* Setup new transfer. */
 	usbd_setup_xfer(xfer, c, c->uc_buf, sc->ure_bufsz,
 	    USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, ure_rxeof);
 	usbd_transfer(xfer);
@@ -1488,23 +1590,34 @@ ure_txeof(struct usbd_xfer *xfer, void *
 	struct ure_softc *sc = c->uc_sc;
 	struct ure_cdata *cd = &sc->ure_cdata;
 	struct ifnet *ifp = GET_IFP(sc);
-	int s;
 
-	if (sc->ure_dying)
+	mutex_enter(&sc->ure_txlock);
+	if (sc->ure_stopping || sc->ure_dying) {
+		mutex_exit(&sc->ure_txlock);
 		return;
+	}
 
 	DPRINTFN(2, ("tx completion\n"));
 
-	s = splnet();
-
 	KASSERT(cd->tx_cnt > 0);
 	cd->tx_cnt--;
 
-	if (status != USBD_NORMAL_COMPLETION) {
-		if (status == USBD_NOT_STARTED || status == USBD_CANCELLED) {
-			splx(s);
-			return;
+	ifp->if_flags &= ~IFF_OACTIVE;
+
+	switch (status) {
+	case USBD_NOT_STARTED:
+	case USBD_CANCELLED:
+		break;
+
+	case USBD_NORMAL_COMPLETION:
+		ifp->if_opackets++;
+
+		if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
+			ure_start_locked(ifp);
 		}
+		break;
+
+	default:
 		ifp->if_oerrors++;
 		if (usbd_ratecheck(&sc->ure_tx_notice))
 			URE_PRINTF(sc, "usb error on tx: %s\n",
@@ -1512,18 +1625,10 @@ ure_txeof(struct usbd_xfer *xfer, void *
 		if (status == USBD_STALLED)
 			usbd_clear_endpoint_stall_async(
 			    sc->ure_ep[URE_ENDPT_TX]);
-		splx(s);
-		return;
-	}
-
-	ifp->if_flags &= ~IFF_OACTIVE;
-	ifp->if_opackets++;
-
-	if (!IFQ_IS_EMPTY(&ifp->if_snd)) {
-		ure_start(ifp);
+		break;
 	}
 
-	splx(s);
+	mutex_exit(&sc->ure_txlock);
 }
 
 static int
@@ -1583,6 +1688,8 @@ ure_encap(struct ure_softc *sc, struct m
 	uint32_t frm_len = 0;
 	uint8_t *buf;
 
+	KASSERT(mutex_owned(&sc->ure_txlock));
+
 	c = &sc->ure_cdata.tx_chain[idx];
 	buf = c->uc_buf;
 

Index: src/sys/dev/usb/if_urevar.h
diff -u src/sys/dev/usb/if_urevar.h:1.2 src/sys/dev/usb/if_urevar.h:1.3
--- src/sys/dev/usb/if_urevar.h:1.2	Thu Feb  7 10:36:20 2019
+++ src/sys/dev/usb/if_urevar.h	Sun Jun 23 02:14:14 2019
@@ -1,4 +1,5 @@
-/*	$NetBSD: if_urevar.h,v 1.2 2019/02/07 10:36:20 mlelstv Exp $	*/
+/*	$NetBSD: if_urevar.h,v 1.3 2019/06/23 02:14:14 mrg Exp $	*/
+
 /*	$OpenBSD: if_urereg.h,v 1.5 2018/11/02 21:32:30 jcs Exp $	*/
 /*-
  * Copyright (c) 2015-2016 Kevin Lo <ke...@freebsd.org>
@@ -110,7 +111,12 @@ struct ure_softc {
 #define GET_IFP(sc)		(&(sc)->ure_ec.ec_if)
 	struct mii_data		ure_mii;
 #define GET_MII(sc)		(&(sc)->ure_mii)
+
 	kmutex_t		ure_mii_lock;
+	kmutex_t		ure_lock;
+	kmutex_t		ure_rxlock;
+	kmutex_t		ure_txlock;
+	kcondvar_t		ure_detachcv;
 	int			ure_refcnt;
 
 	struct ure_cdata	ure_cdata;
@@ -135,5 +141,8 @@ struct ure_softc {
 #define	URE_CHIP_VER_5C30	0x20
 
 	krndsource_t            ure_rnd_source;
+
 	bool			ure_dying;
+	bool			ure_stopping;
+	bool			ure_attached;
 };

Reply via email to