From 1d9ef00d7f64b8c5f62f0ad4e39d967cfa6a8e16 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:05:12 +0000 Subject: [PATCH 1/3] eqos(4): Wait for callout to halt and make sure it stays halted. --- sys/dev/ic/dwc_eqos.c | 7 +++++-- sys/dev/ic/dwc_eqos_var.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sys/dev/ic/dwc_eqos.c b/sys/dev/ic/dwc_eqos.c index a0fd5d58c066..d5dba6132acc 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -421,7 +421,8 @@ eqos_tick(void *softc) EQOS_LOCK(sc); mii_tick(mii); - callout_schedule(&sc->sc_stat_ch, hz); + if (sc->sc_running) + callout_schedule(&sc->sc_stat_ch, hz); EQOS_UNLOCK(sc); #ifndef EQOS_MPSAFE @@ -647,6 +648,7 @@ eqos_init_locked(struct eqos_softc *sc) /* Enable interrupts */ eqos_enable_intr(sc); + sc->sc_running = true; ifp->if_flags |= IFF_RUNNING; mii_mediachg(mii); @@ -679,7 +681,8 @@ eqos_stop_locked(struct eqos_softc *sc, int disable) EQOS_ASSERT_LOCKED(sc); - callout_stop(&sc->sc_stat_ch); + sc->sc_running = false; + callout_halt(&sc->sc_stat_ch, &sc->sc_lock); mii_down(&sc->sc_mii); diff --git a/sys/dev/ic/dwc_eqos_var.h b/sys/dev/ic/dwc_eqos_var.h index 9834e83e290f..07833503a5a0 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -67,6 +67,7 @@ struct eqos_softc { callout_t sc_stat_ch; kmutex_t sc_lock; kmutex_t sc_txlock; + bool sc_running; struct eqos_ring sc_tx; struct eqos_ring sc_rx; From e9fead281634f22c9f70a31fc8c31ed3efa8473a Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:08:44 +0000 Subject: [PATCH 2/3] eqos(4): Don't touch if_flags in tx path. Can't touch this without IFNET_LOCK. --- sys/dev/ic/dwc_eqos.c | 9 ++++++++- sys/dev/ic/dwc_eqos_var.h | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/sys/dev/ic/dwc_eqos.c b/sys/dev/ic/dwc_eqos.c index d5dba6132acc..517142ca9cc3 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -648,6 +648,9 @@ eqos_init_locked(struct eqos_softc *sc) /* Enable interrupts */ eqos_enable_intr(sc); + EQOS_ASSERT_TXLOCKED(sc); + sc->sc_txrunning = true; + sc->sc_running = true; ifp->if_flags |= IFF_RUNNING; @@ -681,6 +684,10 @@ eqos_stop_locked(struct eqos_softc *sc, int disable) EQOS_ASSERT_LOCKED(sc); + EQOS_TXLOCK(sc); + sc->sc_txrunning = false; + EQOS_TXUNLOCK(sc); + sc->sc_running = false; callout_halt(&sc->sc_stat_ch, &sc->sc_lock); @@ -956,7 +963,7 @@ eqos_start_locked(struct eqos_softc *sc) EQOS_ASSERT_TXLOCKED(sc); - if ((ifp->if_flags & IFF_RUNNING) == 0) + if (!sc->sc_txrunning) return; for (cnt = 0, start = sc->sc_tx.cur; ; cnt++) { diff --git a/sys/dev/ic/dwc_eqos_var.h b/sys/dev/ic/dwc_eqos_var.h index 07833503a5a0..b7b35fa5c646 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -68,6 +68,7 @@ struct eqos_softc { kmutex_t sc_lock; kmutex_t sc_txlock; bool sc_running; + bool sc_txrunning; struct eqos_ring sc_tx; struct eqos_ring sc_rx; From dc2f0623e3f768f1b5a83f24a92c6503cd55e3eb Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:12:30 +0000 Subject: [PATCH 3/3] eqos(4): Fix locking around multicast filter updates. - Can't touch if_flags without IFNET_LOCK. - Can't take IFNET_LOCK in SIOCADDMULTI/SIOCDELMULTI path. Instead, cache IFF_PROMISC and IFF_ALLMULTI on if_init under a lock we can take in this path. XXX Is IFF_ALLMULTI relevant any more? Hasn't it been moved to ethercom flags? XXX Should not take sc_lock around if_init/stop -- IFNET_LOCK is enough. Should narrow scope of sc_lock to be just tick/mii/multi stuff. --- sys/dev/ic/dwc_eqos.c | 11 +++++++---- sys/dev/ic/dwc_eqos_var.h | 2 ++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sys/dev/ic/dwc_eqos.c b/sys/dev/ic/dwc_eqos.c index 517142ca9cc3..c49cb6c4ee83 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -461,10 +461,10 @@ eqos_setup_rxfilter(struct eqos_softc *sc) GMAC_MAC_PACKET_FILTER_PCF_MASK); hash[0] = hash[1] = ~0U; - if ((ifp->if_flags & IFF_PROMISC) != 0) { + if (sc->sc_promisc) { pfil |= GMAC_MAC_PACKET_FILTER_PR | GMAC_MAC_PACKET_FILTER_PCF_ALL; - } else if ((ifp->if_flags & IFF_ALLMULTI) != 0) { + } else if (sc->sc_allmulti) { pfil |= GMAC_MAC_PACKET_FILTER_PM; } else { hash[0] = hash[1] = 0; @@ -569,6 +569,8 @@ eqos_init_locked(struct eqos_softc *sc) eqos_init_rings(sc, 0); /* Setup RX filter */ + sc->sc_promisc = ifp->if_flags & IFF_PROMISC; + sc->sc_allmulti = ifp->if_flags & IFF_ALLMULTI; /* XXX */ eqos_setup_rxfilter(sc); WR4(sc, GMAC_MAC_1US_TIC_COUNTER, (sc->sc_csr_clock / 1000000) - 1); @@ -1176,9 +1178,10 @@ eqos_ioctl(struct ifnet *ifp, u_long cmd, void *data) error = (*ifp->if_init)(ifp); else if (cmd != SIOCADDMULTI && cmd != SIOCDELMULTI) ; - else if ((ifp->if_flags & IFF_RUNNING) != 0) { + else { EQOS_LOCK(sc); - eqos_setup_rxfilter(sc); + if (sc->sc_running) + eqos_setup_rxfilter(sc); EQOS_UNLOCK(sc); } break; diff --git a/sys/dev/ic/dwc_eqos_var.h b/sys/dev/ic/dwc_eqos_var.h index b7b35fa5c646..789d9e2a6f2c 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -69,6 +69,8 @@ struct eqos_softc { kmutex_t sc_txlock; bool sc_running; bool sc_txrunning; + bool sc_promisc; + bool sc_allmulti; struct eqos_ring sc_tx; struct eqos_ring sc_rx;