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

[Jonas Jelonek <jelonek.jonas@gmail.com>]

> On 28. Feb 2023, at 18:44, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> On Thu, 2023-01-26 at 17:53 +0100, Jonas Jelonek 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.
> 
> Well you have to think about the lifetime. In most cases you do a lookup
> of the STA (under RCU) etc. but 
> 
>> I could think of two ways:
>> - add NULL checks for the case that the sta pointer might be freed as
>> you said
> 
> How would that pointer even go NULL though? The pointer would remain,
> but the STA can be freed, no?

Yes, you’re right. Sorry, I mixed that up somehow.

> 
>> - 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?
> 
> It should use the hashtable?
> 
>> Or do you suggest something different?
> 
> Well you could keep it here and walk the list of queued skbs (?) when a
> STA is removed, and kill them all at that point, or something. Not sure
> it's worth it vs. the hash table lookup, this is just hwsim after all.
> 
> johannes

That’s also correct. Sorry, I didn’t have a deeper look into this function at the
time of writing. So I guess I will stick to this for now.

Jonas