Inlined patch does the following: - removes functions in initialization per style(9) (splnet) - removes double definition of IWX_NVM_GET_INFO - removes unused 'ring' variable from iwx_sta_tx_agg_start - removes unused 'iwx_tid_to_ac' array - removes iwx_wait_tx_queues_empty function
Regarding the last thing - in our testing of a derivative product we've found that sometimes the queue has some queued packets, the function has to wait until all frames will be de-queued, but there are two problems: 1. There is no corresponding wakeup() for a ring, so this tsleep() should always fail with EWOULDBLOCK, if we have queued frams. This leads to the situation when needed softc flags aren't cleared, and on the next join we get a panic with "MAC already added". 2. Linux driver does the flushes a slightly differently: https://github.com/torvalds/linux/blob/master/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c#L5592 [...] if (drop) { if (iwl_mvm_flush_sta(mvm, mvmsta, false)) IWL_ERR(mvm, "flush request fail\n"); } else { if (iwl_mvm_has_new_tx_api(mvm)) iwl_mvm_wait_sta_queues_empty(mvm, mvmsta); else /* only used for !iwl_mvm_has_new_tx_api() below */ msk |= mvmsta->tfd_queue_msk; } [...] 'drop' var is from Linux 80211 API - "If the parameter drop is set to true, pending frames may be dropped." per https://www.kernel.org/doc/html/v6.5/driver-api/80211/mac80211.html As I can see OpenBSD driver tries to do both things, I propose to drop this iwx_wait_tx_queues_empty function and follow 'drop = true' case from the linux driver. Removal of this function has fixed panics for us. Debugging and code have been sponsored by Serenity Cybersecurity, LLC. diff /usr/src commit - a8cd26ec98e16ab36053bf0a84ae2e4d88c286ea path + /usr/src blob - 612b68e3c6982a8ed4c0bcaca01d8c4e0a81d115 file + sys/dev/pci/if_iwx.c --- sys/dev/pci/if_iwx.c +++ sys/dev/pci/if_iwx.c @@ -402,7 +402,6 @@ void iwx_tx_update_byte_tbl(struct iwx_softc *, struct uint16_t, uint16_t); int iwx_tx(struct iwx_softc *, struct mbuf *, struct ieee80211_node *); int iwx_flush_sta_tids(struct iwx_softc *, int, uint16_t); -int iwx_wait_tx_queues_empty(struct iwx_softc *); int iwx_drain_sta(struct iwx_softc *sc, struct iwx_node *, int); int iwx_flush_sta(struct iwx_softc *, struct iwx_node *); int iwx_beacon_filter_send_cmd(struct iwx_softc *, @@ -2792,18 +2791,6 @@ iwx_nic_init(struct iwx_softc *sc) return 0; } -/* Map a TID to an ieee80211_edca_ac category. */ -const uint8_t iwx_tid_to_ac[IWX_MAX_TID_COUNT] = { - EDCA_AC_BE, - EDCA_AC_BK, - EDCA_AC_BK, - EDCA_AC_BE, - EDCA_AC_VI, - EDCA_AC_VI, - EDCA_AC_VO, - EDCA_AC_VO, -}; - /* Map ieee80211_edca_ac categories to firmware Tx FIFO. */ const uint8_t iwx_ac_to_tx_fifo[] = { IWX_GEN2_EDCA_TX_FIFO_BE, @@ -3548,8 +3535,10 @@ iwx_mac_ctxt_task(void *arg) struct iwx_softc *sc = arg; struct ieee80211com *ic = &sc->sc_ic; struct iwx_node *in = (void *)ic->ic_bss; - int err, s = splnet(); + int err, s; + s = splnet(); + if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) || ic->ic_state != IEEE80211_S_RUN) { refcnt_rele_wake(&sc->task_refs); @@ -3575,8 +3564,10 @@ iwx_phy_ctxt_task(void *arg) struct iwx_node *in = (void *)ic->ic_bss; struct ieee80211_node *ni = &in->in_ni; uint8_t chains, sco, vht_chan_width; - int err, s = splnet(); + int err, s; + s = splnet(); + if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) || ic->ic_state != IEEE80211_S_RUN || in->in_phyctxt == NULL) { @@ -3668,7 +3659,6 @@ iwx_sta_tx_agg_start(struct iwx_softc *sc, struct ieee struct ieee80211com *ic = &sc->sc_ic; struct ieee80211_tx_ba *ba; int err, qid; - struct iwx_tx_ring *ring; /* Ensure we can map this TID to an aggregation queue. */ if (tid >= IWX_MAX_TID_COUNT) @@ -3711,7 +3701,6 @@ iwx_sta_tx_agg_start(struct iwx_softc *sc, struct ieee ba->ba_winend = (ba->ba_winstart + ba->ba_winsize - 1) & 0xfff; - ring = &sc->txq[qid]; ba->ba_timeout_val = 0; ieee80211_addba_resp_accept(ic, ni, tid); sc->aggqid[tid] = qid; @@ -3723,9 +3712,11 @@ iwx_ba_task(void *arg) struct iwx_softc *sc = arg; struct ieee80211com *ic = &sc->sc_ic; struct ieee80211_node *ni = ic->ic_bss; - int s = splnet(); + int s; int tid; + s = splnet(); + for (tid = 0; tid < IWX_MAX_TID_COUNT; tid++) { if (sc->sc_flags & IWX_FLAG_SHUTDOWN) break; @@ -6429,31 +6420,7 @@ out: return err; } -#define IWX_FLUSH_WAIT_MS 2000 - int -iwx_wait_tx_queues_empty(struct iwx_softc *sc) -{ - int i, err; - - for (i = 0; i < nitems(sc->txq); i++) { - struct iwx_tx_ring *ring = &sc->txq[i]; - - if (i == IWX_DQA_CMD_QUEUE) - continue; - - while (ring->queued > 0) { - err = tsleep_nsec(ring, 0, "iwxflush", - MSEC_TO_NSEC(IWX_FLUSH_WAIT_MS)); - if (err) - return err; - } - } - - return 0; -} - -int iwx_drain_sta(struct iwx_softc *sc, struct iwx_node* in, int drain) { struct iwx_add_sta_cmd cmd; @@ -6510,13 +6477,6 @@ iwx_flush_sta(struct iwx_softc *sc, struct iwx_node *i goto done; } - err = iwx_wait_tx_queues_empty(sc); - if (err) { - printf("%s: Could not empty Tx queues (error %d)\n", - DEVNAME(sc), err); - goto done; - } - err = iwx_drain_sta(sc, in, 0); done: sc->sc_flags &= ~IWX_FLAG_TXFLUSH; @@ -7515,8 +7475,10 @@ iwx_clear_statistics(struct iwx_softc *sc) void iwx_add_task(struct iwx_softc *sc, struct taskq *taskq, struct task *task) { - int s = splnet(); + int s; + s = splnet(); + if (sc->sc_flags & IWX_FLAG_SHUTDOWN) { splx(s); return; @@ -7617,8 +7579,10 @@ iwx_bgscan_done_task(void *arg) struct ieee80211com *ic = &sc->sc_ic; struct iwx_node *in = (void *)ic->ic_bss; struct ieee80211_node *ni = &in->in_ni; - int tid, err = 0, s = splnet(); + int tid, err = 0, s; + s = splnet(); + if ((sc->sc_flags & IWX_FLAG_SHUTDOWN) || (ic->ic_flags & IEEE80211_F_BGSCAN) == 0 || ic->ic_state != IEEE80211_S_RUN) { @@ -8540,8 +8504,10 @@ iwx_setkey_task(void *arg) { struct iwx_softc *sc = arg; struct iwx_setkey_task_arg *a; - int err = 0, s = splnet(); + int err = 0, s; + s = splnet(); + while (sc->setkey_nkeys > 0) { if (err || (sc->sc_flags & IWX_FLAG_SHUTDOWN)) break; @@ -8616,8 +8582,10 @@ iwx_newstate_task(void *psc) enum ieee80211_state nstate = sc->ns_nstate; enum ieee80211_state ostate = ic->ic_state; int arg = sc->ns_arg; - int err = 0, s = splnet(); + int err = 0, s; + s = splnet(); + if (sc->sc_flags & IWX_FLAG_SHUTDOWN) { /* iwx_stop() is waiting for us. */ refcnt_rele_wake(&sc->task_refs); @@ -9292,8 +9260,10 @@ iwx_stop(struct ifnet *ifp) struct iwx_softc *sc = ifp->if_softc; struct ieee80211com *ic = &sc->sc_ic; struct iwx_node *in = (void *)ic->ic_bss; - int i, s = splnet(); + int i, s; + s = splnet(); + rw_assert_wrlock(&sc->ioctl_rwl); sc->sc_flags |= IWX_FLAG_SHUTDOWN; /* Disallow new tasks. */ @@ -11395,10 +11365,12 @@ iwx_init_task(void *arg1) { struct iwx_softc *sc = arg1; struct ifnet *ifp = &sc->sc_ic.ic_if; - int s = splnet(); + int s; int generation = sc->sc_generation; int fatal = (sc->sc_flags & (IWX_FLAG_HW_ERR | IWX_FLAG_RFKILL)); + s = splnet(); + rw_enter_write(&sc->ioctl_rwl); if (generation != sc->sc_generation) { rw_exit(&sc->ioctl_rwl); blob - 8713b11a6ea8784f2d5fe115d1eea0e6aa691f0b file + sys/dev/pci/if_iwxreg.h --- sys/dev/pci/if_iwxreg.h +++ sys/dev/pci/if_iwxreg.h @@ -2016,7 +2016,6 @@ struct iwx_tx_queue_cfg_rsp { /* REGULATORY_AND_NVM group subcommand IDs */ #define IWX_NVM_ACCESS_COMPLETE 0x00 #define IWX_NVM_GET_INFO 0x02 -#define IWX_NVM_GET_INFO 0x02 #define IWX_PNVM_INIT_COMPLETE 0xfe /*
