Re: [PATCH] wifi: mac80211: fix assigning channel in activate links

[Aditya Kumar Singh <quic_adisi@quicinc.com>]

On 10/1/24 12:52, Johannes Berg wrote:
> But that doesn't explain the*comment*, which literally says:
> 
>     inform about the link info changed parameters after all stations are
>     also added
> 
> but you
>   (a) don't add stations here
>   (b) if you're thinking about link stations, the link stations are
>       only added_after_  this comment and the link info change ...
> 

Oops! my bad. Previously I was thinking like this -

First iterate and do only _ieee80211_link_use_channel() this part. Then 
let the flow as usual and after stations are added, do the 
link_info_changed() part.


> 
>> Later, in the loop for_each_set_bit(link_id, &add,
>> IEEE80211_MLD_MAX_NUM_LINKS), channels are added. At this stage, the
>> driver will actually create the link on the interface at its own level.
>> Since here using the channel information, appropriate firmware can be
>> picked. For example 2 GHz or 5 GHz or 6 GHz firmware.
> Picking "firmware" sounds very odd here, I'd say you mean "which device
> to pick"?

:) Yeah.

> 
>> hmm... yeah true that. May be I will move this once the old links are
>> removed?
>>
> I'd think at least that?
> 
> But also this seems to break out driver for other reasons, because it
> initializes rate control somewhere here and needs a station for that.
> Didn't look deeply into that yet though.

Okay so doing as I said above could work -

if (add) {
	...
}

for_each_set_bit(link_id, &rem, ..) {
	...
}

for_each_set_bit(link_id, &add ...) {
	_ieee80211_link_use_channel()
}

list_for_each_entry(sta, &local->sta_list, list) {
	...
}

...

for_each_set_bit(link_id, &add ....) {
	now call
	ieee80211_mgd_set_link_qos_params()
	ieee80211_link_info_change_notify()
}

...


At least I tried both of these ways in hwsim. I dont see any failures. 
Hence I thought why not move whole for loop to top instead.


-- 
Aditya