Module Name: src
Committed By: riastradh
Date: Thu Mar 3 05:53:23 UTC 2022
Modified Files:
src/sys/dev/usb: if_aue.c if_axe.c if_axen.c if_smsc.c if_udav.c
if_ure.c if_url.c usbnet.c
Log Message:
usbnet: Apply hardware multicast filter updates synchronously again.
To make this work:
1. Do it only under a new lock, unp_mcastlock. This lock lives at
IPL_SOFTCLOCK so it can be taken from network stack callouts. It
is forbidden to acquire the usbnet core lock under unp_mcastlock.
2. Do it only after usbnet_init_rx_tx and before usbnet_stop; if
issued at any other time, drop the update on the floor.
3. Make usbnet_init_rx_tx apply any pending multicast filter updates
under the lock before setting the flag that allows SIOCADDMULTI or
SIOCDELMULTI to apply the updates.
4. Remove core lock asserts from various drivers' register access
routines. This is necessary because the multicast filter updates
are done with register reads/writes, but _cannot_ take the core
lock when the caller holds softnet_lock.
This now programs the hardware multicast filter redundantly in many
drivers which already explicitly call *_uno_mcast from the *_uno_init
routines. This is probably harmless, but it will likely be better to
remove the explicit calls.
To generate a diff of this commit:
cvs rdiff -u -r1.180 -r1.181 src/sys/dev/usb/if_aue.c
cvs rdiff -u -r1.141 -r1.142 src/sys/dev/usb/if_axe.c
cvs rdiff -u -r1.83 -r1.84 src/sys/dev/usb/if_axen.c
cvs rdiff -u -r1.82 -r1.83 src/sys/dev/usb/if_smsc.c
cvs rdiff -u -r1.87 -r1.88 src/sys/dev/usb/if_udav.c src/sys/dev/usb/if_url.c
cvs rdiff -u -r1.48 -r1.49 src/sys/dev/usb/if_ure.c
cvs rdiff -u -r1.81 -r1.82 src/sys/dev/usb/usbnet.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/usb/if_aue.c
diff -u src/sys/dev/usb/if_aue.c:1.180 src/sys/dev/usb/if_aue.c:1.181
--- src/sys/dev/usb/if_aue.c:1.180 Thu Mar 3 05:53:14 2022
+++ src/sys/dev/usb/if_aue.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_aue.c,v 1.180 2022/03/03 05:53:14 riastradh Exp $ */
+/* $NetBSD: if_aue.c,v 1.181 2022/03/03 05:53:23 riastradh Exp $ */
/*
* Copyright (c) 1997, 1998, 1999, 2000
@@ -76,7 +76,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_aue.c,v 1.180 2022/03/03 05:53:14 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_aue.c,v 1.181 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -284,8 +284,6 @@ aue_csr_read_1(struct aue_softc *sc, int
usbd_status err;
uByte val = 0;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -315,8 +313,6 @@ aue_csr_read_2(struct aue_softc *sc, int
usbd_status err;
uWord val;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -346,8 +342,6 @@ aue_csr_write_1(struct aue_softc *sc, in
usbd_status err;
uByte val;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -378,8 +372,6 @@ aue_csr_write_2(struct aue_softc *sc, in
usbd_status err;
uWord val;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -624,8 +616,6 @@ aue_uno_mcast(struct ifnet *ifp)
AUEHIST_FUNC();
AUEHIST_CALLARGSN(5, "aue%jd: enter", device_unit(un->un_dev), 0, 0, 0);
- usbnet_isowned_core(un);
-
if (ifp->if_flags & IFF_PROMISC) {
ETHER_LOCK(ec);
allmulti:
Index: src/sys/dev/usb/if_axe.c
diff -u src/sys/dev/usb/if_axe.c:1.141 src/sys/dev/usb/if_axe.c:1.142
--- src/sys/dev/usb/if_axe.c:1.141 Thu Mar 3 05:53:04 2022
+++ src/sys/dev/usb/if_axe.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_axe.c,v 1.141 2022/03/03 05:53:04 riastradh Exp $ */
+/* $NetBSD: if_axe.c,v 1.142 2022/03/03 05:53:23 riastradh Exp $ */
/* $OpenBSD: if_axe.c,v 1.137 2016/04/13 11:03:37 mpi Exp $ */
/*
@@ -87,7 +87,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_axe.c,v 1.141 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_axe.c,v 1.142 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -293,8 +293,6 @@ axe_cmd(struct axe_softc *sc, int cmd, i
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return -1;
Index: src/sys/dev/usb/if_axen.c
diff -u src/sys/dev/usb/if_axen.c:1.83 src/sys/dev/usb/if_axen.c:1.84
--- src/sys/dev/usb/if_axen.c:1.83 Thu Mar 3 05:53:04 2022
+++ src/sys/dev/usb/if_axen.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_axen.c,v 1.83 2022/03/03 05:53:04 riastradh Exp $ */
+/* $NetBSD: if_axen.c,v 1.84 2022/03/03 05:53:23 riastradh Exp $ */
/* $OpenBSD: if_axen.c,v 1.3 2013/10/21 10:10:22 yuo Exp $ */
/*
@@ -23,7 +23,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_axen.c,v 1.83 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_axen.c,v 1.84 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -108,8 +108,6 @@ axen_cmd(struct usbnet *un, int cmd, int
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -239,8 +237,6 @@ axen_uno_mcast(struct ifnet *ifp)
if (usbnet_isdying(un))
return;
- usbnet_isowned_core(un);
-
rxmode = 0;
/* Enable receiver, set RX mode */
Index: src/sys/dev/usb/if_smsc.c
diff -u src/sys/dev/usb/if_smsc.c:1.82 src/sys/dev/usb/if_smsc.c:1.83
--- src/sys/dev/usb/if_smsc.c:1.82 Thu Mar 3 05:53:14 2022
+++ src/sys/dev/usb/if_smsc.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_smsc.c,v 1.82 2022/03/03 05:53:14 riastradh Exp $ */
+/* $NetBSD: if_smsc.c,v 1.83 2022/03/03 05:53:23 riastradh Exp $ */
/* $OpenBSD: if_smsc.c,v 1.4 2012/09/27 12:38:11 jsg Exp $ */
/* $FreeBSD: src/sys/dev/usb/net/if_smsc.c,v 1.1 2012/08/15 04:03:55 gonzo Exp $ */
@@ -61,7 +61,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_smsc.c,v 1.82 2022/03/03 05:53:14 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_smsc.c,v 1.83 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -211,8 +211,6 @@ smsc_readreg(struct usbnet *un, uint32_t
uint32_t buf;
usbd_status err;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -238,8 +236,6 @@ smsc_writereg(struct usbnet *un, uint32_
uint32_t buf;
usbd_status err;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return 0;
@@ -422,8 +418,6 @@ smsc_uno_mcast(struct ifnet *ifp)
uint32_t hashtbl[2] = { 0, 0 };
uint32_t hash;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return;
Index: src/sys/dev/usb/if_udav.c
diff -u src/sys/dev/usb/if_udav.c:1.87 src/sys/dev/usb/if_udav.c:1.88
--- src/sys/dev/usb/if_udav.c:1.87 Thu Mar 3 05:53:14 2022
+++ src/sys/dev/usb/if_udav.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_udav.c,v 1.87 2022/03/03 05:53:14 riastradh Exp $ */
+/* $NetBSD: if_udav.c,v 1.88 2022/03/03 05:53:23 riastradh Exp $ */
/* $nabe: if_udav.c,v 1.3 2003/08/21 16:57:19 nabe Exp $ */
/*
@@ -45,7 +45,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_udav.c,v 1.87 2022/03/03 05:53:14 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_udav.c,v 1.88 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -364,7 +364,6 @@ udav_csr_read(struct usbnet *un, int off
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
KASSERT(!usbnet_isdying(un));
DPRINTFN(0x200,
@@ -395,7 +394,6 @@ udav_csr_write(struct usbnet *un, int of
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
KASSERT(!usbnet_isdying(un));
DPRINTFN(0x200,
@@ -424,8 +422,6 @@ udav_csr_read1(struct usbnet *un, int of
{
uint8_t val = 0;
- usbnet_isowned_core(un);
-
DPRINTFN(0x200,
("%s: %s: enter\n", device_xname(un->un_dev), __func__));
@@ -442,7 +438,6 @@ udav_csr_write1(struct usbnet *un, int o
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
KASSERT(!usbnet_isdying(un));
DPRINTFN(0x200,
@@ -586,8 +581,6 @@ udav_uno_mcast(struct ifnet *ifp)
DPRINTF(("%s: %s: enter\n", device_xname(un->un_dev), __func__));
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return;
Index: src/sys/dev/usb/if_url.c
diff -u src/sys/dev/usb/if_url.c:1.87 src/sys/dev/usb/if_url.c:1.88
--- src/sys/dev/usb/if_url.c:1.87 Thu Mar 3 05:53:04 2022
+++ src/sys/dev/usb/if_url.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_url.c,v 1.87 2022/03/03 05:53:04 riastradh Exp $ */
+/* $NetBSD: if_url.c,v 1.88 2022/03/03 05:53:23 riastradh Exp $ */
/*
* Copyright (c) 2001, 2002
@@ -44,7 +44,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_url.c,v 1.87 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_url.c,v 1.88 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -268,8 +268,6 @@ url_mem(struct usbnet *un, int cmd, int
usb_device_request_t req;
usbd_status err;
- usbnet_isowned_core(un);
-
DPRINTFN(0x200,
("%s: %s: enter\n", device_xname(un->un_dev), __func__));
@@ -435,8 +433,6 @@ url_uno_mcast(struct ifnet *ifp)
DPRINTF(("%s: %s: enter\n", device_xname(un->un_dev), __func__));
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return;
Index: src/sys/dev/usb/if_ure.c
diff -u src/sys/dev/usb/if_ure.c:1.48 src/sys/dev/usb/if_ure.c:1.49
--- src/sys/dev/usb/if_ure.c:1.48 Thu Mar 3 05:53:04 2022
+++ src/sys/dev/usb/if_ure.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: if_ure.c,v 1.48 2022/03/03 05:53:04 riastradh Exp $ */
+/* $NetBSD: if_ure.c,v 1.49 2022/03/03 05:53:23 riastradh Exp $ */
/* $OpenBSD: if_ure.c,v 1.10 2018/11/02 21:32:30 jcs Exp $ */
/*-
@@ -30,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.48 2022/03/03 05:53:04 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_ure.c,v 1.49 2022/03/03 05:53:23 riastradh Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -340,8 +340,6 @@ ure_uno_mcast(struct ifnet *ifp)
uint32_t mchash[2] = { 0, 0 };
uint32_t h = 0, rxmode;
- usbnet_isowned_core(un);
-
if (usbnet_isdying(un))
return;
Index: src/sys/dev/usb/usbnet.c
diff -u src/sys/dev/usb/usbnet.c:1.81 src/sys/dev/usb/usbnet.c:1.82
--- src/sys/dev/usb/usbnet.c:1.81 Thu Mar 3 05:52:46 2022
+++ src/sys/dev/usb/usbnet.c Thu Mar 3 05:53:23 2022
@@ -1,4 +1,4 @@
-/* $NetBSD: usbnet.c,v 1.81 2022/03/03 05:52:46 riastradh Exp $ */
+/* $NetBSD: usbnet.c,v 1.82 2022/03/03 05:53:23 riastradh Exp $ */
/*
* Copyright (c) 2019 Matthew R. Green
@@ -31,7 +31,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usbnet.c,v 1.81 2022/03/03 05:52:46 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbnet.c,v 1.82 2022/03/03 05:53:23 riastradh Exp $");
#include <sys/param.h>
#include <sys/kernel.h>
@@ -59,6 +59,7 @@ struct usbnet_private {
*
* the lock ordering is:
* ifnet lock -> unp_core_lock -> unp_rxlock -> unp_txlock
+ * -> unp_mcastlock
* - ifnet lock is not needed for unp_core_lock, but if ifnet lock is
* involved, it must be taken first
*/
@@ -66,11 +67,13 @@ struct usbnet_private {
kmutex_t unp_rxlock;
kmutex_t unp_txlock;
+ kmutex_t unp_mcastlock;
+ bool unp_mcastactive;
+
struct usbnet_cdata unp_cdata;
struct ethercom unp_ec;
struct mii_data unp_mii;
- struct usb_task unp_mcasttask;
struct usb_task unp_ticktask;
struct callout unp_stat_ch;
struct usbd_pipe *unp_ep[USBNET_ENDPT_MAX];
@@ -866,6 +869,17 @@ usbnet_init_rx_tx(struct usbnet * const
"%s", ifp->if_xname);
ifp->if_flags |= IFF_RUNNING;
+ /*
+ * If the hardware has a multicast filter, program it and then
+ * allow updates to it while we're running.
+ */
+ if (un->un_ops->uno_mcast) {
+ mutex_enter(&unp->unp_mcastlock);
+ (*un->un_ops->uno_mcast)(ifp);
+ unp->unp_mcastactive = true;
+ mutex_exit(&unp->unp_mcastlock);
+ }
+
/* Start up the receive pipe(s). */
usbnet_rx_start_pipes(un);
@@ -1018,8 +1032,20 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
switch (cmd) {
case SIOCADDMULTI:
case SIOCDELMULTI:
- usb_add_task(un->un_udev, &unp->unp_mcasttask,
- USB_TASKQ_DRIVER);
+ /*
+ * If there's a hardware multicast filter, and
+ * it has been programmed by usbnet_init_rx_tx
+ * and is active, update it now. Otherwise,
+ * drop the update on the floor -- it will be
+ * observed by usbnet_init_rx_tx next time we
+ * bring the interface up.
+ */
+ if (un->un_ops->uno_mcast) {
+ mutex_enter(&unp->unp_mcastlock);
+ if (unp->unp_mcastactive)
+ (*un->un_ops->uno_mcast)(ifp);
+ mutex_exit(&unp->unp_mcastlock);
+ }
error = 0;
break;
default:
@@ -1030,45 +1056,6 @@ usbnet_if_ioctl(struct ifnet *ifp, u_lon
return error;
}
-static void
-usbnet_mcast_task(void *arg)
-{
- USBNETHIST_FUNC();
- struct usbnet * const un = arg;
- struct ifnet * const ifp = usbnet_ifp(un);
-
- USBNETHIST_CALLARGSN(10, "%jd: enter",
- un->un_pri->unp_number, 0, 0, 0);
-
- /*
- * If we're detaching, we must check usbnet_isdying _before_
- * touching IFNET_LOCK -- the ifnet may have been detached by
- * the time this task runs. This is racy -- unp_dying may be
- * set immediately after we test it -- but nevertheless safe,
- * because usbnet_detach waits for the task to complete before
- * issuing if_detach, and necessary, so that we don't touch
- * IFNET_LOCK after if_detach. See usbnet_detach for details.
- */
- if (usbnet_isdying(un))
- return;
-
- /*
- * If the hardware is running, ask the driver to reprogram the
- * multicast filter. If the hardware is not running, the
- * driver is responsible for programming the multicast filter
- * as part of its uno_init routine to bring the hardware up.
- */
- IFNET_LOCK(ifp);
- if (ifp->if_flags & IFF_RUNNING) {
- if (un->un_ops->uno_mcast) {
- mutex_enter(&un->un_pri->unp_core_lock);
- (*un->un_ops->uno_mcast)(ifp);
- mutex_exit(&un->un_pri->unp_core_lock);
- }
- }
- IFNET_UNLOCK(ifp);
-}
-
/*
* Generic stop network function:
* - mark as stopping
@@ -1094,6 +1081,18 @@ usbnet_stop(struct usbnet *un, struct if
usbnet_isowned_core(un);
/*
+ * For drivers with hardware multicast filter update callbacks:
+ * Prevent concurrent access to the hardware registers by
+ * multicast filter updates, which happens without IFNET_LOCK
+ * or the usbnet core lock.
+ */
+ if (un->un_ops->uno_mcast) {
+ mutex_enter(&unp->unp_mcastlock);
+ unp->unp_mcastactive = false;
+ mutex_exit(&unp->unp_mcastlock);
+ }
+
+ /*
* Prevent new activity (rescheduling ticks, xfers, &c.) and
* clear the watchdog timer.
*/
@@ -1387,8 +1386,6 @@ usbnet_attach(struct usbnet *un,
un->un_pri = kmem_zalloc(sizeof(*un->un_pri), KM_SLEEP);
struct usbnet_private * const unp = un->un_pri;
- usb_init_task(&unp->unp_mcasttask, usbnet_mcast_task, un,
- USB_TASKQ_MPSAFE);
usb_init_task(&unp->unp_ticktask, usbnet_tick_task, un,
USB_TASKQ_MPSAFE);
callout_init(&unp->unp_stat_ch, CALLOUT_MPSAFE);
@@ -1397,6 +1394,7 @@ usbnet_attach(struct usbnet *un,
mutex_init(&unp->unp_txlock, MUTEX_DEFAULT, IPL_SOFTUSB);
mutex_init(&unp->unp_rxlock, MUTEX_DEFAULT, IPL_SOFTUSB);
mutex_init(&unp->unp_core_lock, MUTEX_DEFAULT, IPL_NONE);
+ mutex_init(&unp->unp_mcastlock, MUTEX_DEFAULT, IPL_SOFTCLOCK);
rnd_attach_source(&unp->unp_rndsrc, device_xname(un->un_dev),
RND_TYPE_NET, RND_FLAG_DEFAULT);
@@ -1539,9 +1537,6 @@ usbnet_detach(device_t self, int flags)
KASSERT(!callout_pending(&unp->unp_stat_ch));
KASSERT(!usb_task_pending(un->un_udev, &unp->unp_ticktask));
- usb_rem_task_wait(un->un_udev, &unp->unp_mcasttask, USB_TASKQ_DRIVER,
- NULL);
-
if (mii) {
mii_detach(mii, MII_PHY_ANY, MII_OFFSET_ANY);
ifmedia_fini(&mii->mii_media);
@@ -1555,45 +1550,12 @@ usbnet_detach(device_t self, int flags)
}
usbnet_ec(un)->ec_mii = NULL;
- /*
- * We have already waited for the multicast task to complete.
- * Unfortunately, until if_detach, nothing has prevented it
- * from running again -- another thread might issue if_mcast_op
- * between the time of our first usb_rem_task_wait and the time
- * we actually get around to if_detach.
- *
- * Fortunately, the first usb_rem_task_wait ensures that if the
- * task is scheduled again, it will witness our setting of
- * unp_dying to true[*]. So after that point, if the task is
- * scheduled again, it will decline to touch IFNET_LOCK and do
- * nothing. But we still need to wait for it to complete.
- *
- * It would be nice if we could write
- *
- * if_pleasestopissuingmcastopsthanks(ifp);
- * usb_rem_task_wait(..., &unp->unp_mcasttask, ...);
- * if_detach(ifp);
- *
- * and then we would need only one usb_rem_task_wait.
- *
- * Unfortunately, there is no such operation available in
- * sys/net at the moment, and it would require a bit of
- * coordination with if_mcast_op and doifioctl probably under a
- * new lock. So we'll use this kludge until that mechanism is
- * invented.
- *
- * [*] This is not exactly a documented property of the API,
- * but it is implied by the single lock in the task queue
- * serializing changes to the task state.
- */
- usb_rem_task_wait(un->un_udev, &unp->unp_mcasttask, USB_TASKQ_DRIVER,
- NULL);
-
usbnet_rx_list_free(un);
usbnet_tx_list_free(un);
rnd_detach_source(&unp->unp_rndsrc);
+ mutex_destroy(&unp->unp_mcastlock);
mutex_destroy(&unp->unp_core_lock);
mutex_destroy(&unp->unp_rxlock);
mutex_destroy(&unp->unp_txlock);
