From: Stefan Sperling Subject: Re: make qwx(4) flush Rx rings when the interface goes down To: Mark Kettenis Cc: tech@openbsd.org Date: Sat, 26 Apr 2025 15:07:00 +0200 On Fri, Apr 25, 2025 at 07:17:31PM +0200, Mark Kettenis wrote: > However, when suspending the laptop, I see some error messages upon > resume. The machine recovers and qwx(4) works fine afterwards. But > it suggests there is still something not quite right with the state > machine? > qwx0: scan stop timeout > qwx0: failed to abort scan: 35 The scan errors are caused by a missing wakeup() in the code path which stops scans. An obvious bug. The thread which sends the stop command is never woken up. > qwx0: peer create command timeout > qwx0: Failed to add peer: 6c:71:d9:cd:39:76 for VDEV: 0 I am seeing this one, too. The best explanation I have is that the newstate task gets scheduled because of a frame received in parallel to qwx_stop(), after the FLUSH_CRASH flag is cleared (stupid flag name, I know -- this flag from the Linux driver signals crashed firmware. Very early during porting of this driver I repurposed this flag to prevent scheduling of new tasks as well, and never got around to making this pretty). This error can be prevented by checking for the IFF_RUNNING flag before scheduling the newstate task. Both fixes combined below, on top of previous diffs. I don't see any more errors now. M sys/dev/ic/qwx.c | 8+ 6- 1 file changed, 8 insertions(+), 6 deletions(-) commit - a9a3e6e76dd5e088a6557077d5b215526a7848e3 commit + aad1f217b0653b67795cd712afa39450765c96fd blob - 731d85bacb2e762cb57c5abb8f0ed4bf9843b6d2 blob + c3bffe842e8ff319f1c7f5de4a859a8cd4d2a4ef --- sys/dev/ic/qwx.c +++ sys/dev/ic/qwx.c @@ -338,13 +338,13 @@ qwx_stop(struct ifnet *ifp) qwx_setkey_clear(sc); - clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); - ifp->if_timer = sc->sc_tx_timer = 0; ifp->if_flags &= ~IFF_RUNNING; ifq_clr_oactive(&ifp->if_snd); + clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags); + /* * Manually run the newstate task's code for switching to INIT state. * This reconfigures firmware state to stop scanning, or disassociate @@ -846,6 +846,10 @@ qwx_newstate(struct ieee80211com *ic, enum ieee80211_s struct ifnet *ifp = &ic->ic_if; struct qwx_softc *sc = ifp->if_softc; + /* We may get triggered by received frames during qwx_stop(). */ + if (!(ifp->if_flags & IFF_RUNNING)) + return 0; + /* * Prevent attempts to transition towards the same state, unless * we are scanning in which case a SCAN -> SCAN transition @@ -12976,7 +12980,6 @@ qwx_wmi_event_scan_start_failed(struct qwx_softc *sc) qwx_scan_state_str(sc->scan.state), sc->scan.state); break; case ATH11K_SCAN_STARTING: - wakeup(&sc->scan.state); qwx_mac_scan_finish(sc); break; } @@ -23573,9 +23576,8 @@ qwx_mac_scan_finish(struct qwx_softc *sc) timeout_del(&sc->scan.timeout); if (!sc->scan.is_roc) ieee80211_end_scan(ifp); -#if 0 - complete_all(&ar->scan.completed); -#endif + + wakeup(&sc->scan.state); break; } }