From 9da38d495abacbce9e40239f46779ad86dcf778a Mon Sep 17 00:00:00 2001 From: Stefan Sperling Date: Tue, 5 May 2020 09:41:34 +0000 Subject: Revert parts of CVS commit Jdqd26bn9Ev6aFMc ("Fix processing of compressed block ack notifications sent by iwn(4) firmware"). This effectively reverts changes in how the driver interacts with firmware and fixes connections getting stuck for unknown reasons, in two known cases: One with an Airport Extreme 6th gen AP and another with a b-box 3V+ modem using a Sagemcom MAC address for its built-in AP. The Sagemcom case was observed by myself. The Airport case was reported by Jeremy O'Brien via abieber@. I am committing this now to prevent the problem from affecting 6.7 release even though we don't yet understand what caused the problem. ok mpi@ --- sys/dev/pci/if_iwn.c | 192 +++++++++++++++++++++++++++++---------------------- 1 file changed, 108 insertions(+), 84 deletions(-) (limited to 'sys/dev/pci/if_iwn.c') diff --git a/sys/dev/pci/if_iwn.c b/sys/dev/pci/if_iwn.c index 6b6331d167a..dc4b5863f7d 100644 --- a/sys/dev/pci/if_iwn.c +++ b/sys/dev/pci/if_iwn.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_iwn.c,v 1.228 2020/05/01 14:04:17 stsp Exp $ */ +/* $OpenBSD: if_iwn.c,v 1.229 2020/05/05 09:41:33 stsp Exp $ */ /*- * Copyright (c) 2007-2010 Damien Bergamini @@ -158,6 +158,8 @@ void iwn_rx_phy(struct iwn_softc *, struct iwn_rx_desc *, 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 *, @@ -2282,82 +2284,15 @@ iwn_mira_choose(struct iwn_softc *sc, struct ieee80211_node *ni) 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; - - 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); + int idx, end_idx; /* Determine the min/max IDs we assigned to AMPDUs in this range. */ idx = IWN_AGG_SSN_TO_TXQ_IDX(seq); @@ -2413,7 +2348,7 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc, 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; @@ -2427,6 +2362,81 @@ iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc, } } +/* + * 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). @@ -2696,18 +2706,32 @@ iwn_ampdu_tx_done(struct iwn_softc *sc, struct iwn_tx_ring *txq, 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); } /* -- cgit v1.2.3