Peter Seiderer <[email protected]> writes:
> Hello Toke,
>
> On Mon, 4 Apr 2022 20:11:51 +0200, Toke Høiland-Jørgensen <[email protected]> wrote:
>
>> From: Toke Høiland-Jørgensen <[email protected]>
>>
>> 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: [email protected]
>> Reported-by: Peter Seiderer <[email protected]>
>> Fixes: 037250f0a45c ("ath9k: Properly clear TX status area before reporting to mac80211")
>> Signed-off-by: Toke Høiland-Jørgensen <[email protected]>
>> ---
>> 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;
Ah, bugger; nice find! I'll fix that up as well, but I do believe it's
the only one.
-Toke