On 3/27/2023 4:31 PM, Johannes Berg wrote:
Hi,
+ list_for_each_entry(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ continue;
+ ret = drv_change_sta_links(local, sdata, &sta->sta,
+ old_active,
+ old_active | active_links);
+ WARN_ON_ONCE(ret);
+ }
+
+ ret = ieee80211_key_switch_links(sdata, rem, add);
I see ieee80211_key_switch_link() only handler the per-link(link_id >=
0) keys,
So I think lower driver also install the pairwise keys(link_id = -1) for
the added links at this moment?
Well from mac80211 POV they're already installed, so we can't really
install them again. We'd have to remove them but that's racy, obviously.
So I think the low-level driver just has to handle that, e.g. when the
station links are updated (and the key belongs to the station.)
Got it, thanks.
+ WARN_ON_ONCE(ret);
+
+ list_for_each_entry(sta, &local->sta_list, list) {
+ if (sdata != sta->sdata)
+ continue;
+ ret = drv_change_sta_links(local, sdata, &sta->sta,
+ old_active | active_links,
+ active_links);
+ WARN_ON_ONCE(ret);
+ }
+
I see 2 times to call drv_change_sta_link() above, and with sequence
old_active->old_active | active_links->active_links
May I know is it has some design here?
The problem is that we can't really have no links active even as an
intermediate step, so you can't just deactivate old and then activate
new.
Got it, thanks.
+ for_each_set_bit(link_id, &add, IEEE80211_MLD_MAX_NUM_LINKS) {
+ struct ieee80211_link_data *link;
+
+ link = sdata_dereference(sdata->link[link_id], sdata);
+
+ ret = ieee80211_link_use_channel(link, &link->conf->chandef,
+ IEEE80211_CHANCTX_SHARED);
For the 1st link of MLO connection/NON-MLO connetion, ieee80211_link_use_channel() is called before drv_change_sta_link(),
And now it is after drv_change_sta_link(), May I know is it also has some design here?
Hmm, probably not really, at least I don't remember anything about that.
Not sure it makes a huge difference? But I suppose we could change it, I
don't really see why not either.
Not huge difference, I have made little change in lower-driver to match
that. So it is OK now.
Also I see commit(8fb7e2ef4bab mac80211_hwsim: always activate all links) and ieee80211_if_parse_active_links()
will use ieee80211_set_active_links(), so I think ieee80211_set_active_links() has passed test case with some type lower driver/chip?
Yes, we have this working on iwlwifi/mvm.
Got it, thanks.
I also have tested ieee80211_set_active_links() to enable the 2nd link
for station success in my lower-driver after a little change in
lower-driver.
johannes
