From e41484f38120461a44df20aed293748eeabc0741 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 6bb8a4e8b2a7..1bf4c9601274 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -426,7 +426,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 @@ -652,6 +653,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); @@ -684,7 +686,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 ce329a8cfe4897f44eb23f0e7f08213bca0b5298 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 1bf4c9601274..b7d57d0d8e7a 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -653,6 +653,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; @@ -686,6 +689,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); @@ -961,7 +968,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 078d7d70fbfc4bcb211c14139d410149be219a27 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 b7d57d0d8e7a..306b4482896b 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -466,10 +466,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; @@ -574,6 +574,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); @@ -1181,9 +1183,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; From 7ea97e004ed571453b9b7bc1d96481b4742afd7c 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 306b4482896b..327888ed7455 100644 --- a/sys/dev/ic/dwc_eqos.c +++ b/sys/dev/ic/dwc_eqos.c @@ -466,17 +466,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; @@ -485,8 +497,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); @@ -575,7 +587,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);