Let iwn(4) simply clear frames before the firmware's BA window, instead
authorstsp <stsp@openbsd.org>
Mon, 22 Mar 2021 09:52:49 +0000 (09:52 +0000)
committerstsp <stsp@openbsd.org>
Mon, 22 Mar 2021 09:52:49 +0000 (09:52 +0000)
of trying to be smart and clearing already acknowledged frames which are
still within the firmware's BA window.

This matches what the Linux driver does and makes our driver code simpler.

Also, Tx rate control code relies on sequence numbers falling into the
BA window so let's skip Tx rate control for frames before this window.

Tested by:
myself on 6205 and 6300
afresh1, bluhm, and paco on 6300
jmatthew on 5100
Balder Oddson on 6205

sys/dev/pci/if_iwn.c

index db09809..241fff3 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: if_iwn.c,v 1.246 2021/03/17 15:34:21 stsp Exp $       */
+/*     $OpenBSD: if_iwn.c,v 1.247 2021/03/22 09:52:49 stsp Exp $       */
 
 /*-
  * Copyright (c) 2007-2010 Damien Bergamini <damien.bergamini@free.fr>
@@ -2383,6 +2383,9 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
         */
        ssn = le16toh(cba->ssn);
 
+       if (SEQ_LT(ssn, ba->ba_winstart))
+               return;
+
        /* Skip rate control if our Tx rate is fixed. */
        if (ic->ic_fixed_mcs == -1)
                iwn_ampdu_rate_control(sc, ni, txq, ba->ba_winstart, ssn);
@@ -2392,12 +2395,10 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
         * in firmware's BA window. Firmware is not going to retransmit any
         * frames before its BA window so mark them all as done.
         */
-       if (SEQ_LT(ba->ba_winstart, ssn)) {
-               ieee80211_output_ba_move_window(ic, ni, cba->tid, ssn);
-               iwn_ampdu_txq_advance(sc, txq, qid,
-                   IWN_AGG_SSN_TO_TXQ_IDX(ssn));
-               iwn_clear_oactive(sc, txq);
-       }
+       ieee80211_output_ba_move_window(ic, ni, cba->tid, ssn);
+       iwn_ampdu_txq_advance(sc, txq, qid,
+           IWN_AGG_SSN_TO_TXQ_IDX(ssn));
+       iwn_clear_oactive(sc, txq);
 }
 
 /*
@@ -2607,6 +2608,8 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, struct iwn_tx_ring *txq,
        ba = &ni->ni_tx_ba[tid];
        if (ba->ba_state != IEEE80211_BA_AGREED)
                return;
+       if (SEQ_LT(ssn, ba->ba_winstart))
+               return;
 
        /* This was a final single-frame Tx attempt for frame SSN-1. */
        seq = (ssn - 1) & 0xfff;
@@ -2635,29 +2638,14 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, struct iwn_tx_ring *txq,
 
        if (txfail)
                ieee80211_tx_compressed_bar(ic, ni, tid, ssn);
-       else if (!SEQ_LT(seq, ba->ba_winstart)) {
-               /*
-                * Move window forward if SEQ lies beyond end of window,
-                * otherwise we can't record the ACK for this frame.
-                * Non-acked frames which left holes in the bitmap near
-                * the beginning of the window must be discarded.
-                */
-               uint16_t s = seq;
-               while (SEQ_LT(ba->ba_winend, s)) {
-                       ieee80211_output_ba_move_window(ic, ni, tid, s);
-                       iwn_ampdu_txq_advance(sc, txq, desc->qid,
-                           IWN_AGG_SSN_TO_TXQ_IDX(s));
-                       s = (s + 1) % 0xfff;
-               }
-               /* SEQ should now be within window; set corresponding bit. */
-               ieee80211_output_ba_record_ack(ic, ni, tid, seq);
-       }
-
-       /* Move window forward up to the first hole in the bitmap. */
-       ieee80211_output_ba_move_window_to_first_unacked(ic, ni, tid, ssn);
-       iwn_ampdu_txq_advance(sc, txq, desc->qid,
-           IWN_AGG_SSN_TO_TXQ_IDX(ba->ba_winstart));
 
+       /*
+        * SSN corresponds to the first (perhaps not yet transmitted) frame
+        * in firmware's BA window. Firmware is not going to retransmit any
+        * frames before its BA window so mark them all as done.
+        */
+       ieee80211_output_ba_move_window(ic, ni, tid, ssn);
+       iwn_ampdu_txq_advance(sc, txq, desc->qid, IWN_AGG_SSN_TO_TXQ_IDX(ssn));
        iwn_clear_oactive(sc, txq);
 }