Thanks for reviewing the patch.
On 5/27/2022 1:26 PM, Johannes Berg wrote:
+ for_each_valid_link(cr, link)
+ cfg80211_put_bss(wdev->wiphy, cr->links[link].bss);
reduced the repeated code by using "out" label. Can't make common
utility function
for both roam and connect callbacks since variables in links struct
differs for connect result and roam info.
Also, addressed several other comments about code cleanup and nits in v2.
- if (WARN_ON(!info->bss))
+ if (WARN_ON(!bss_found))
return;
This goes with my question earlier - here you're basically assuming
finding a single BSS is fine. Do we really think so? It used to be we
wanted all of them, and I kind of tend to think drivers should just make
sure they have all of them - once we have the entry it can be updated,
but if we don't have one, we'll never again get the information on the
BSS, for purposes such as "iw link" output.
I tend to think we should always require all BSS entries to exist, even
if the driver initially has to make up a fake entry with pretty much no
information.
Or maybe we have enough information here already (BSSID/frequency) to
make up a fake entry in cfg80211?
Agree, Drivers should have enough formation to create a BSS entry for
all the links.
As suggested added checks for all bss entries availability in v2 patch.
+ if (info->ap_mld_addr) {
+ ev->rm.ap_mld_addr = next;
+ memcpy((void *)ev->rm.ap_mld_addr, info->ap_mld_addr,
+ ETH_ALEN);
Why the cast?
Without casting it gives below error.
note: expected 'void *' but argument is of type 'const u8 *' {aka 'const unsigned char *'}
--
veeru
