Hello Toke,
On Sat, 2 Apr 2022 14:27:51 +0200, Toke Høiland-Jørgensen toke@toke.dk wrote:
From: Toke Høiland-Jørgensen toke@redhat.com
The ieee80211_tx_info_clear_status() helper also clears the rate counts, so we should restore them after clearing. However, we can get rid of the existing clearing of the counts of invalid rates. Rearrange the code a bit so the order fits the indexes, and so the setting of the count to hw->max_rate_tries on underrun is not immediately overridden.
Cc: stable@vger.kernel.org Reported-by: Peter Seiderer ps.report@gmx.net Fixes: 037250f0a45c ("ath9k: Properly clear TX status area before reporting to mac80211") Signed-off-by: Toke Høiland-Jørgensen toke@redhat.com
drivers/net/wireless/ath/ath9k/xmit.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index cbcf96ac303e..ac7efecff29c 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -2551,16 +2551,19 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb); struct ieee80211_hw *hw = sc->hw; struct ath_hw *ah = sc->sc_ah;
- u8 i, tx_rateindex;
- u8 i, tx_rateindex, tries[IEEE80211_TX_MAX_RATES];
- tx_rateindex = ts->ts_rateindex;
- WARN_ON(tx_rateindex >= hw->max_rates);
- for (i = 0; i < tx_rateindex; i++)
tries[i] = tx_info->status.rates[i].count;
ieee80211_tx_info_clear_status(tx_info); if (txok) tx_info->status.ack_signal = ts->ts_rssi;
- tx_rateindex = ts->ts_rateindex;
- WARN_ON(tx_rateindex >= hw->max_rates);
- if (tx_info->flags & IEEE80211_TX_CTL_AMPDU) { tx_info->flags |= IEEE80211_TX_STAT_AMPDU;
@@ -2569,6 +2572,14 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, tx_info->status.ampdu_len = nframes; tx_info->status.ampdu_ack_len = nframes - nbad;
- for (i = 0; i < tx_rateindex; i++)
tx_info->status.rates[i].count = tries[i];
- tx_info->status.rates[tx_rateindex].count = ts->ts_longretry + 1;
- for (i = tx_rateindex + 1; i < hw->max_rates; i++)
tx_info->status.rates[i].idx = -1;
- if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 && (tx_info->flags & IEEE80211_TX_CTL_NO_ACK) == 0) { /*
@@ -2591,12 +2602,6 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, hw->max_rate_tries; }
The full lines above read:
2597 if (unlikely(ts->ts_flags & (ATH9K_TX_DATA_UNDERRUN | 2598 ATH9K_TX_DELIM_UNDERRUN)) && 2599 ieee80211_is_data(hdr->frame_control) && 2600 ah->tx_trig_level >= sc->sc_ah->config.max_txtrig_level ) 2601 tx_info->status.rates[tx_rateindex].count = 2602 hw->max_rate_tries; 2603 }
So this patch fixes by drive-by a overwrite of tx_info->status.rates[tx_rateindex].count...
- for (i = tx_rateindex + 1; i < hw->max_rates; i++) {
tx_info->status.rates[i].count = 0;
tx_info->status.rates[i].idx = -1;
- }
- tx_info->status.rates[tx_rateindex].count = ts->ts_longretry + 1;
} static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
Otherwise looks good ;-), would like to give a Reviewed-by/Tested-by but still affected by the underlying ieee80211_tx_info status vs. rate_driver_data overwrite as mentioned in the other thread (see [1])...
Regards, Peter
[1] https://lore.kernel.org/linux-wireless/20220404130453.5ec6e045@gmx.net/