| Message ID | 20220404181151.2669173-1-toke@toke.dk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Toke Høiland-Jørgensen |
| Headers | show |
| Series | [for-5.18,v2] ath9k: Fix usage of driver-private space in tx_info | expand |
Hello Toke, On Mon, 4 Apr 2022 20:11: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 and > the driver-private part of struct ieee80211_tx_info, so using it breaks > quite a few other things. So back out of using it, and instead define a > ath-internal helper that only clears the area between the > status_driver_data and the rates info. Combined with moving the > ath_frame_info struct to status_driver_data, this avoids clearing anything > we shouldn't be, and so we can keep the existing code for handling the rate > information. > > While fixing this I also noticed that the setting of > tx_info->status.rates[tx_rateindex].count on hardware underrun errors was > always immediately overridden by the normal setting of the same fields, so > rearrange the code so that the underrun detection actually takes effect. > > The new helper could be generalised to a 'memset_between()' helper, but > leave it as a driver-internal helper for now since this needs to go to > stable. > > 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 | 30 ++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c > index cbcf96ac303e..db83cc4ba810 100644 > --- a/drivers/net/wireless/ath/ath9k/xmit.c > +++ b/drivers/net/wireless/ath/ath9k/xmit.c > @@ -141,8 +141,8 @@ static struct ath_frame_info *get_frame_info(struct sk_buff *skb) > { > struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb); > BUILD_BUG_ON(sizeof(struct ath_frame_info) > > - sizeof(tx_info->rate_driver_data)); > - return (struct ath_frame_info *) &tx_info->rate_driver_data[0]; > + sizeof(tx_info->status.status_driver_data)); > + return (struct ath_frame_info *) &tx_info->status.status_driver_data[0]; > } Would be too easy if all locations would use get_frame_info()..., at least one location in drivers/net/wireless/ath/ath9k/main.c uses direct access: 841 txinfo = IEEE80211_SKB_CB(bf->bf_mpdu); 842 fi = (struct ath_frame_info *)&txinfo->rate_driver_data[0]; 843 if (fi->keyix == keyix) 844 return true; Regards, Peter > > static void ath_send_bar(struct ath_atx_tid *tid, u16 seqno) > @@ -2542,6 +2542,16 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf, > spin_unlock_irqrestore(&sc->tx.txbuflock, flags); > } > > +static void ath_clear_tx_status(struct ieee80211_tx_info *tx_info) > +{ > + void *ptr = &tx_info->status; > + > + memset(ptr + sizeof(tx_info->status.rates), 0, > + sizeof(tx_info->status) - > + sizeof(tx_info->status.rates) - > + sizeof(tx_info->status.status_driver_data)); > +} > + > static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, > struct ath_tx_status *ts, int nframes, int nbad, > int txok) > @@ -2553,7 +2563,7 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, > struct ath_hw *ah = sc->sc_ah; > u8 i, tx_rateindex; > > - ieee80211_tx_info_clear_status(tx_info); > + ath_clear_tx_status(tx_info); > > if (txok) > tx_info->status.ack_signal = ts->ts_rssi; > @@ -2569,6 +2579,13 @@ 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; > > + 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].count = 0; > + tx_info->status.rates[i].idx = -1; > + } > + > if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 && > (tx_info->flags & IEEE80211_TX_CTL_NO_ACK) == 0) { > /* > @@ -2590,13 +2607,6 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, > tx_info->status.rates[tx_rateindex].count = > hw->max_rate_tries; > } > - > - 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)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c index cbcf96ac303e..db83cc4ba810 100644 --- a/drivers/net/wireless/ath/ath9k/xmit.c +++ b/drivers/net/wireless/ath/ath9k/xmit.c @@ -141,8 +141,8 @@ static struct ath_frame_info *get_frame_info(struct sk_buff *skb) { struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb); BUILD_BUG_ON(sizeof(struct ath_frame_info) > - sizeof(tx_info->rate_driver_data)); - return (struct ath_frame_info *) &tx_info->rate_driver_data[0]; + sizeof(tx_info->status.status_driver_data)); + return (struct ath_frame_info *) &tx_info->status.status_driver_data[0]; } static void ath_send_bar(struct ath_atx_tid *tid, u16 seqno) @@ -2542,6 +2542,16 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf, spin_unlock_irqrestore(&sc->tx.txbuflock, flags); } +static void ath_clear_tx_status(struct ieee80211_tx_info *tx_info) +{ + void *ptr = &tx_info->status; + + memset(ptr + sizeof(tx_info->status.rates), 0, + sizeof(tx_info->status) - + sizeof(tx_info->status.rates) - + sizeof(tx_info->status.status_driver_data)); +} + static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, struct ath_tx_status *ts, int nframes, int nbad, int txok) @@ -2553,7 +2563,7 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, struct ath_hw *ah = sc->sc_ah; u8 i, tx_rateindex; - ieee80211_tx_info_clear_status(tx_info); + ath_clear_tx_status(tx_info); if (txok) tx_info->status.ack_signal = ts->ts_rssi; @@ -2569,6 +2579,13 @@ 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; + 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].count = 0; + tx_info->status.rates[i].idx = -1; + } + if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 && (tx_info->flags & IEEE80211_TX_CTL_NO_ACK) == 0) { /* @@ -2590,13 +2607,6 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf, tx_info->status.rates[tx_rateindex].count = hw->max_rate_tries; } - - 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)