For some reason, changes I made to iwn(4) in the commit quoted below
have caused connections to get stuck on some APs during Tx bursts.
This does not occur with every type of AP. It was observed on an Apple
Airport Extreme 6th gen, and on a b-box 3V+ (Sagemcom Mac address).
The patch below reverts all changes related to interaction between driver
and firmware from the relevant commit and works around the problem in all
known cases. It is unclear why it helps, but at least this patch should
avoid the problem from affecting 6.7-release.
ok?
The relevant commit was:
---
Module name: src
Changes by: s...@cvs.openbsd.org 2020/04/27 02:02:24
Modified files:
sys/dev/pci : if_iwn.c if_iwnvar.h
Log message:
Fix processing of compressed block ack notifications sent by iwn(4) firmware.
Fix wrong assumptions about what the data in these notifications is supposed
to represent, and actually piece information about individual subframes of
aggregated frames (A-MPDUs) back together when reporting to MiRA, rather than
reporting unrelated subframes to MiRA individually.
Testing by cwen@, Josh Grosse, f.holop, benno@
ok jmatthew@
---
diff 523aa541f5e65a29bda64c13043addf764654fa1 /usr/src
blob - 620cbd4c138516398a4683d83c1b6bf8aac57c82
file + sys/dev/pci/if_iwn.c
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -158,6 +158,8 @@ void iwn_rx_phy(struct iwn_softc *, struct
iwn_rx_des
void iwn_rx_done(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *, struct mbuf_list *);
void iwn_mira_choose(struct iwn_softc *, struct ieee80211_node *);
+void iwn_ampdu_rate_control(struct iwn_softc *, struct
ieee80211_node *,
+ struct iwn_tx_ring *, int, uint16_t, uint16_t);
void iwn_rx_compressed_ba(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *);
void iwn5000_rx_calib_results(struct iwn_softc *,
@@ -2248,83 +2250,16 @@ iwn_mira_choose(struct iwn_softc *sc, struct ieee80211
iwn_set_link_quality(sc, ni);
}
-/*
- * Process an incoming Compressed BlockAck.
- * Note that these block ack notifications are generated by firmware and do
- * not necessarily correspond to contents of block ack frames seen on the air.
- */
void
-iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
- struct iwn_rx_data *data)
+iwn_ampdu_rate_control(struct iwn_softc *sc, struct ieee80211_node *ni,
+ struct iwn_tx_ring *txq, int tid, uint16_t seq, uint16_t ssn)
{
- struct iwn_compressed_ba *cba = (struct iwn_compressed_ba *)(desc + 1);
struct ieee80211com *ic = &sc->sc_ic;
- struct ieee80211_node *ni;
- struct ieee80211_tx_ba *ba;
- struct iwn_node *wn;
- struct iwn_tx_ring *txq;
- uint16_t seq, ssn, idx, end_idx;
+ struct iwn_node *wn = (void *)ni;
+ struct ieee80211_tx_ba *ba = &ni->ni_tx_ba[tid];
int min_ampdu_id, max_ampdu_id, id;
- int qid;
+ int idx, end_idx;
- if (ic->ic_state != IEEE80211_S_RUN)
- return;
-
- bus_dmamap_sync(sc->sc_dmat, data->map, sizeof (*desc), sizeof (*cba),
- BUS_DMASYNC_POSTREAD);
-
- if (!IEEE80211_ADDR_EQ(ic->ic_bss->ni_macaddr, cba->macaddr))
- return;
-
- ni = ic->ic_bss;
- wn = (void *)ni;
-
- qid = le16toh(cba->qid);
- if (qid < sc->first_agg_txq || qid >= sc->ntxqs)
- return;
-
- txq = &sc->txq[qid];
-
- /* Protect against a firmware bug where the queue/TID are off. */
- if (qid != sc->first_agg_txq + cba->tid)
- return;
-
- ba = &ni->ni_tx_ba[cba->tid];
- if (ba->ba_state != IEEE80211_BA_AGREED)
- return;
-
- /*
- * The first bit in cba->bitmap corresponds to the sequence number
- * stored in the sequence control field cba->seq.
- * Any frames older than this can now be discarded; they should
- * already have been reported as failures or been acknowledged.
- *
- * Multiple BA notifications in a row may be using this number, with
- * additional bits being set in cba->bitmap. It is unclear how the
- * firmware decides to shift this window forward.
- */
- seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT;
- if (!SEQ_LT(seq, ba->ba_winstart)) {
- ieee80211_output_ba_move_window(ic, ni, cba->tid, seq);
- iwn_ampdu_txq_advance(sc, txq, qid,
- IWN_AGG_SSN_TO_TXQ_IDX(seq));
- iwn_clear_oactive(sc, txq);
- }
- /* Our BA window should now correspond to the bitmap. */
- if (ba->ba_winstart != seq)
- return;
-
- /* Skip rate control if our Tx rate is fixed. */
- if (ic->ic_fixed_mcs != -1)
- return;
-
- /*
- * The firmware's new BA window starting sequence number
- * corresponds to the first hole in cba->bitmap, implying
- * that all frames between 'seq' and 'ssn' have been acked.
- */
- ssn = le16toh(cba->ssn);
-
/* Determine the min/max IDs we assigned to AMPDUs in this range. */
idx = IWN_AGG_SSN_TO_TXQ_IDX(seq);
end_idx = IWN_AGG_SSN_TO_TXQ_IDX(ssn);
@@ -2379,7 +2314,7 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
wn->mn.retries++;
if (!SEQ_LT(ba->ba_winend, s))
ieee80211_output_ba_record_ack(ic, ni,
- cba->tid, s);
+ tid, s);
}
idx = (idx + 1) % IWN_TX_RING_COUNT;
@@ -2394,6 +2329,81 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_
}
/*
+ * Process an incoming Compressed BlockAck.
+ * Note that these block ack notifications are generated by firmware and do
+ * not necessarily correspond to contents of block ack frames seen on the air.
+ */
+void
+iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
+ struct iwn_rx_data *data)
+{
+ struct iwn_compressed_ba *cba = (struct iwn_compressed_ba *)(desc + 1);
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ieee80211_node *ni;
+ struct ieee80211_tx_ba *ba;
+ struct iwn_tx_ring *txq;
+ uint16_t seq, ssn;
+ int qid;
+
+ if (ic->ic_state != IEEE80211_S_RUN)
+ return;
+
+ bus_dmamap_sync(sc->sc_dmat, data->map, sizeof (*desc), sizeof (*cba),
+ BUS_DMASYNC_POSTREAD);
+
+ if (!IEEE80211_ADDR_EQ(ic->ic_bss->ni_macaddr, cba->macaddr))
+ return;
+
+ ni = ic->ic_bss;
+
+ qid = le16toh(cba->qid);
+ if (qid < sc->first_agg_txq || qid >= sc->ntxqs)
+ return;
+
+ txq = &sc->txq[qid];
+
+ /* Protect against a firmware bug where the queue/TID are off. */
+ if (qid != sc->first_agg_txq + cba->tid)
+ return;
+
+ ba = &ni->ni_tx_ba[cba->tid];
+ if (ba->ba_state != IEEE80211_BA_AGREED)
+ return;
+
+ /*
+ * The first bit in cba->bitmap corresponds to the sequence number
+ * stored in the sequence control field cba->seq.
+ * Multiple BA notifications in a row may be using this number, with
+ * additional bits being set in cba->bitmap. It is unclear how the
+ * firmware decides to shift this window forward.
+ */
+ seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT;
+
+ /*
+ * The firmware's new BA window starting sequence number
+ * corresponds to the first hole in cba->bitmap, implying
+ * that all frames between 'seq' and 'ssn' have been acked.
+ */
+ ssn = le16toh(cba->ssn);
+
+ /* Skip rate control if our Tx rate is fixed. */
+ if (ic->ic_fixed_mcs != -1)
+ iwn_ampdu_rate_control(sc, ni, txq, cba->tid, seq, ssn);
+
+ /*
+ * 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.
+ */
+ 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);
+ }
+}
+
+/*
* Process a CALIBRATION_RESULT notification sent by the initialization
* firmware on response to a CMD_CALIB_CONFIG command (5000 only).
*/
@@ -2662,18 +2672,32 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, struct iwn_tx_
iwn_mira_choose(sc, ni);
}
- /*
- * 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);
-
- /* Clear potential gap at the front of receiver's BA window. */
if (txfail)
ieee80211_tx_compressed_bar(ic, ni, tid, ssn);
+ else if (!SEQ_LT(ssn, ba->ba_winstart)) {
+ /*
+ * Move window forward if SSN 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 = ssn;
+ 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;
+ }
+ /* SSN should now be within window; set corresponding bit. */
+ ieee80211_output_ba_record_ack(ic, ni, tid, ssn);
+ }
+
+ /* 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));
+
+ iwn_clear_oactive(sc, txq);
}
/*
