Let iwm(4) use per-Tx-queue interface timers to ensure that the interface

watchdog will trigger a device timeout if a particular Tx queue gets stuck
while other Tx queues keep working.

The Linux driver is using a similar workaround for "stuck queues".
I have only observed this problem on iwx(4) hardware but it won't hurt
to add this workaround to iwm(4) as well.

diff --git a/sys/dev/pci/if_iwm.c b/sys/dev/pci/if_iwm.c
index d3e9a6e..7418b39 100644 (file)
--- a/sys/dev/pci/if_iwm.c
+++ b/sys/dev/pci/if_iwm.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_iwm.c,v 1.382 2021/11/22 11:00:50 stsp Exp $       */
+/*     $OpenBSD: if_iwm.c,v 1.383 2021/11/27 11:22:26 stsp Exp $       */
 
 /*
  * Copyright (c) 2014, 2016 genua gmbh <info@genua.de>
@@ -5569,8 +5569,6 @@ iwm_ampdu_tx_done(struct iwm_softc *sc, struct iwm_cmd_header *cmd_hdr,
            status != IWM_TX_STATUS_DIRECT_DONE);
        uint16_t seq;
 
-       sc->sc_tx_timer = 0;
-
        if (ic->ic_state != IEEE80211_S_RUN)
                return;
 
@@ -5670,17 +5668,19 @@ iwm_rx_tx_cmd(struct iwm_softc *sc, struct iwm_rx_packet *pkt,
        bus_dmamap_sync(sc->sc_dmat, data->map, 0, IWM_RBUF_SIZE,
            BUS_DMASYNC_POSTREAD);
 
-       sc->sc_tx_timer = 0;
-
        /* Sanity checks. */
        if (sizeof(*tx_resp) > len)
                return;
        if (qid < IWM_FIRST_AGG_TX_QUEUE && tx_resp->frame_count > 1)
                return;
+       if (qid > IWM_LAST_AGG_TX_QUEUE)
+               return;
        if (sizeof(*tx_resp) + sizeof(ssn) +
            tx_resp->frame_count * sizeof(tx_resp->status) > len)
                return;
 
+       sc->sc_tx_timer[qid] = 0;
+
        txd = &ring->data[idx];
        if (txd->m == NULL)
                return;
@@ -5790,7 +5790,7 @@ iwm_rx_compressed_ba(struct iwm_softc *sc, struct iwm_rx_packet *pkt,
        if (qid != IWM_FIRST_AGG_TX_QUEUE + ban->tid)
                return;
 
-       sc->sc_tx_timer = 0;
+       sc->sc_tx_timer[qid] = 0;
 
        ba = &ni->ni_tx_ba[ban->tid];
        if (ba->ba_state != IEEE80211_BA_AGREED)
@@ -6733,6 +6733,9 @@ iwm_tx(struct iwm_softc *sc, struct mbuf *m, struct ieee80211_node *ni, int ac)
                sc->qfullmsk |= 1 << ring->qid;
        }
 
+       if (ic->ic_if.if_flags & IFF_UP)
+               sc->sc_tx_timer[ring->qid] = 15;
+
        return 0;
 }
 
@@ -10011,10 +10014,8 @@ iwm_start(struct ifnet *ifp)
                        continue;
                }
 
-               if (ifp->if_flags & IFF_UP) {
-                       sc->sc_tx_timer = 15;
+               if (ifp->if_flags & IFF_UP)
                        ifp->if_timer = 1;
-               }
        }
 
        return;
@@ -10084,7 +10085,8 @@ iwm_stop(struct ifnet *ifp)
                iwm_clear_reorder_buffer(sc, rxba);
        }
        iwm_led_blink_stop(sc);
-       ifp->if_timer = sc->sc_tx_timer = 0;
+       memset(sc->sc_tx_timer, 0, sizeof(sc->sc_tx_timer));
+       ifp->if_timer = 0;
 
        splx(s);
 }
@@ -10093,21 +10095,30 @@ void
 iwm_watchdog(struct ifnet *ifp)
 {
        struct iwm_softc *sc = ifp->if_softc;
+       int i;
 
        ifp->if_timer = 0;
-       if (sc->sc_tx_timer > 0) {
-               if (--sc->sc_tx_timer == 0) {
-                       printf("%s: device timeout\n", DEVNAME(sc));
-                       if (ifp->if_flags & IFF_DEBUG) {
-                               iwm_nic_error(sc);
-                               iwm_dump_driver_status(sc);
+
+       /*
+        * We maintain a separate timer for each Tx queue because
+        * Tx aggregation queues can get "stuck" while other queues
+        * keep working. The Linux driver uses a similar workaround.
+        */
+       for (i = 0; i < nitems(sc->sc_tx_timer); i++) {
+               if (sc->sc_tx_timer[i] > 0) {
+                       if (--sc->sc_tx_timer[i] == 0) {
+                               printf("%s: device timeout\n", DEVNAME(sc));
+                               if (ifp->if_flags & IFF_DEBUG) {
+                                       iwm_nic_error(sc);
+                                       iwm_dump_driver_status(sc);
+                               }
+                               if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) == 0)
+                                       task_add(systq, &sc->init_task);
+                               ifp->if_oerrors++;
+                               return;
                        }
-                       if ((sc->sc_flags & IWM_FLAG_SHUTDOWN) == 0)
-                               task_add(systq, &sc->init_task);
-                       ifp->if_oerrors++;
-                       return;
+                       ifp->if_timer = 1;
                }
-               ifp->if_timer = 1;
        }
 
        ieee80211_watchdog(ifp);

diff --git a/sys/dev/pci/if_iwmvar.h b/sys/dev/pci/if_iwmvar.h
index fe8490a..8ff9f92 100644 (file)
--- a/sys/dev/pci/if_iwmvar.h
+++ b/sys/dev/pci/if_iwmvar.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_iwmvar.h,v 1.71 2021/10/11 09:03:22 stsp Exp $     */
+/*     $OpenBSD: if_iwmvar.h,v 1.72 2021/11/27 11:22:26 stsp Exp $     */
 
 /*
  * Copyright (c) 2014 genua mbh <info@genua.de>
@@ -588,7 +588,7 @@ struct iwm_softc {
 
        struct iwm_bf_data sc_bf;
 
-       int sc_tx_timer;
+       int sc_tx_timer[IWM_MAX_QUEUES];
        int sc_rx_ba_sessions;
        int tx_ba_queue_mask;