Re: [RFC v2 5/6] mac80211_hwsim: add TPC per packet support

From: Jonas Jelonek <jelonek.jonas@gmail.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org, Felix Fietkau <nbd@nbd.name>
Subject: Re: [RFC v2 5/6] mac80211_hwsim: add TPC per packet support
Date: Thu, 26 Jan 2023 17:53:37 +0100	[thread overview]
Message-ID: <03B7A24C-D6E2-4DBB-B52D-6174539BB781@gmail.com> (raw)
In-Reply-To: <5d15d193c76dcc2cbd59b26912973e3bce34c776.camel@sipsolutions.net>

> On 12. Jan 2023, at 11:31, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> On Tue, 2022-09-20 at 12:40 +0200, Jonas Jelonek wrote:
>> @@ -4846,16 +4989,32 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
>> 
>> tx_attempts = (struct hwsim_tx_rate *)nla_data(
>>       info->attrs[HWSIM_ATTR_TX_INFO]);
>> + tx_attempts_flags = (struct hwsim_tx_rate_flag *)nla_data(
>> +     info->attrs[HWSIM_ATTR_TX_INFO_FLAGS]);
>> + sta = (struct ieee80211_sta *)txi->rate_driver_data[1];
> 
> That seems dangerous - what if the STA was freed already? You don't walk
> the pending list or something if the STA goes away.

Yes, I see. Is it in general a bad idea to take the sta reference from ieee80211_control, put
it in rate_driver_data and use it for tx-status? I guess I should pass sta to tx_status_ext whenever
possible because it is used for several statistics.

I could think of two ways:
- add NULL checks for the case that the sta pointer might be freed as you said
- get sta by using, e.g., sta_info_get_by_addrs to get the sta if it is available. However, this always
loops through the sta list. Might be a performance issue?

Or do you suggest something different?

Jonas