Re: [RFC PATCH 2/2] wifi: mac80211: Assign tx-stats to the proper link.

[Ben Greear <greearb@candelatech.com>]

On 9/3/24 03:13, Johannes Berg wrote:
> On Wed, 2024-08-28 at 08:54 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> For drivers that can report the tx link-id, account tx
>> stats against that link.  If we cannot determine tx link,
>> then use deflink.
> 
> Strictly speaking, that's not what happens, since the link bits in the
> SKB CB might be set on outgoing frames, and then will still be there on
> the status.

Ok, I can update description to mention that.

> Also using deflink is totally useless for MLO, so maybe just don't do
> anything at all? But might be simpler to just do deflink and document
> that the driver must set this? But not sure that really works so well
> for drivers now.

The stats used to all go to deflink, so that seems a good default, and should
work w/out MLO being enabled.

>> +static struct link_sta_info*
>> +ieee80211_get_tx_link_sta(struct sta_info *sta, struct ieee80211_tx_info *info)
>> +{
>> +	u8 link_id = u32_get_bits(info->control.flags, IEEE80211_TX_CTRL_MLO_LINK);
>> +	struct link_sta_info *l_sta_info;
> 
> We usually call that 'link_sta' ...

Yeah, and other similar things are also called link_sta too, which I find confusing.
But I will change it back to be link_sta everywhere I am making changes.

> 
>> @@ -48,6 +65,7 @@ static void ieee80211_handle_filtered_frame(struct ieee80211_local *local,
>>   	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>>   	struct ieee80211_hdr *hdr = (void *)skb->data;
>>   	int ac;
>> +	struct link_sta_info *link_sta = ieee80211_get_tx_link_sta(sta, info);
> 
> and even you do, please be consistent with existing code and yourself.
> 
>>
>> +		link_sta = ieee80211_get_tx_link_sta(sta, info);
> 
> Does it really make sense to keep repeating this, rather than passing an
> argument?

Ok, happy to pass this into the handlers.

Thanks,
Ben

> 
> johannes
> 

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com