From 7660a4eb7997762ba8a02df588f4f1c32313c4a3 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:05:12 +0000 Subject: [PATCH 1/4] 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 3b0ee000af0a..f48f02c94235 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -453,7 +453,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 @@ -684,6 +685,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); @@ -716,7 +718,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 adb0b68f3f62..bb7dd41021af 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -70,6 +70,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 c7fb9826f9dd62bc1e36cff26f06968a15286ea8 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:08:44 +0000 Subject: [PATCH 2/4] 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 f48f02c94235..50306305f5e6 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -685,6 +685,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; @@ -718,6 +721,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); @@ -997,7 +1004,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 bb7dd41021af..b1dba28100e1 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -71,6 +71,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 bf06ff4df652c9d50f32e767ee325d49c4e1b530 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 17 Oct 2022 11:12:30 +0000 Subject: [PATCH 3/4] 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 50306305f5e6..cadfbb3a063f 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -494,10 +494,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; @@ -602,6 +602,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); @@ -1219,9 +1221,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 b1dba28100e1..954ca89488f3 100644 --- a/sys/dev/ic/dwc_eqos_var.h +++ b/sys/dev/ic/dwc_eqos_var.h @@ -72,6 +72,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; From fc1e585cf1b0ee0391a57957ef14500e23d56273 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 13 Oct 2023 09:56:17 +0000 Subject: [PATCH 4/4] eqos(4): Fix multicast filter updates. 1. Don't touch the obsolete IFF_ALLMULTI. 2. Set ETHER_F_ALLMULTI if we're accepting all multicast addresses. 3. If any multicast entry range is not a single address, accept all multicast addresses. --- sys/dev/ic/dwc_eqos.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/sys/dev/ic/dwc_eqos.c b/sys/dev/ic/dwc_eqos.c index cadfbb3a063f..b18bd8ac3fe7 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -494,17 +494,29 @@ eqos_setup_rxfilter(struct eqos_softc *sc) GMAC_MAC_PACKET_FILTER_PCF_MASK); hash[0] = hash[1] = ~0U; + ETHER_LOCK(ec); if (sc->sc_promisc) { + ec->ec_flags |= ETHER_F_ALLMULTI; pfil |= GMAC_MAC_PACKET_FILTER_PR | GMAC_MAC_PACKET_FILTER_PCF_ALL; - } else if (sc->sc_allmulti) { - pfil |= GMAC_MAC_PACKET_FILTER_PM; } else { - hash[0] = hash[1] = 0; pfil |= GMAC_MAC_PACKET_FILTER_HMC; - ETHER_LOCK(ec); + hash[0] = hash[1] = 0; + ec->ec_flags &= ~ETHER_F_ALLMULTI; ETHER_FIRST_MULTI(step, ec, enm); while (enm != NULL) { + if (memcmp(enm->enm_addrlo, enm->enm_addrhi, + ETHER_ADDR_LEN) != 0) { + ec->ec_flags |= ETHER_F_ALLMULTI; + pfil &= ~GMAC_MAC_PACKET_FILTER_HMC; + pfil |= GMAC_MAC_PACKET_FILTER_PM; + /* + * Shouldn't matter if we clear HMC but + * let's avoid using different values. + */ + hash[0] = hash[1] = 0xffffffff; + break; + } crc = ether_crc32_le(enm->enm_addrlo, ETHER_ADDR_LEN); crc &= 0x7f; crc = eqos_bitrev32(~crc) >> 26; @@ -513,8 +525,8 @@ eqos_setup_rxfilter(struct eqos_softc *sc) hash[hashreg] |= (1 << hashbit); ETHER_NEXT_MULTI(step, enm); } - ETHER_UNLOCK(ec); } + ETHER_UNLOCK(ec); /* Write our unicast address */ eaddr = CLLADDR(ifp->if_sadl); @@ -603,7 +615,6 @@ eqos_init_locked(struct eqos_softc *sc) /* 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);