... > > To summarize, if BSS channel needs to be updated, then bss_tree should > > be rebuilt in order to put updated BSS entry into a proper location. > > Good catch! > > > This commit suggests the following straightforward solution: > > - if new entry has been already created for BSS after channel switch, > > then remove it completely > > Shouldn't we prefer the new entry? > > OTOH, the old entry will likely have a "hold", so it doesn't get removed > while we're connected ... and the driver etc. might be referencing it. > So I guess the old entry should be updated with info from the newer one? > > > Finally, next scan operation will find BSS entry in expected location > > in rb_tree. So all the IEs, including HT/VHT operation IEs, > > will be properly updated. > > Right. Although if it was there before, then it already has been updated > in a sense... But I guess it's a corner case to even get there? > > > 1. Tested using iwlwifi and qtnfmac drivers, looks good > > Great. > > > 2. Alternative approach: remove old BSS entry and keep new a one > > This approach may have certain benefits for mac80211 drivers. > > For instance, in this case HT/VHT operation IEs are going to be > > valid from the start, no need to wait for the next scan. > > > However the following procedure for replacing current_bss, protected > > by wdev->mtx and rdev->bss_lock locks, seems to be insufficient: > > > > bss_ref_get(rdev, new); > > cfg80211_hold_bss(new); > > wdev->current_bss = new; > > > > cfg80211_unhold_bss(old); > > bss_ref_put(rdev, old); > > __cfg80211_unlink_bss(rdev, old); > > > > When testing this alternative approach using iwlwifi driver, > > occasional general protection fault crashes have been observed > > on ieee80211_rx_mgmt_beacon/ieee80211_bss_info_update code paths. > > So far I haven't yet root caused them. > > At the very least you'd also have to update ifmgd->associated in > mac80211, and that's basically not really possible? Well, I guess we > could change the channel switch API to return the new one or something. > > I guess the better thing would be to go update the old entry with the > new one's data, before killing the new one. > > Not sure it's worth the extra complexity though. Hello Johannes, Thanks for review! Summarizing your comments, here are the options and some of their highlights: 1. replace old entry with the new entry - no easy way to update ifmgd->associated w/o rework of mac80211 csa 2. keep old entry, remove new entry - this is what suggested RFC patch does - works, but update of current_bss entry after csa may be delayed 3. keep old entry, update it using data from new entry, then remove new entry - this looks like a better approach Unless I am missing something, the last option can be implemented on top of the current RFC patch w/o extra complexity. The required bss entry update is what cfg80211_bss_update function does when bss in question already exists. So it should be possible to reuse that code. I will post RFC patch v2 after more testing for both mac80211 and fullmac cases. Regards, Sergey
