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; };