On 28-11-2016 15:38, Johannes Berg wrote: > >> * the nl80211 feature flags for the device. >> + * @NL80211_SCAN_FLAGS_IE_DATA: request the device to supply IE data >> in the >> + * request. > > What does that mean?
I meant "supply IE data in the result". So it informs the driver that user-space is interested in IE data, but I guess it does not need to be explicit. It is not today. The result reporting is still something I am not sure about. I would prefer to use the bss list in cfg80211 like all other scan functions, but I am not sure if that is sufficient for user-space functionality. I did not get a clear answer on that for Android. >> + * @NL80211_GSCAN_CHAN_ATTR_NO_IR: scanning should be done passive. > > why not call that passive? No-IR is something we use in regulatory code > to be more generic than "passive" (since it's also about beaconing > etc.) but here? Ok. Makes sense as we are talking just probe requests here and passive scanning is familiar term. >> + * @NL80211_GSCAN_CHAN_ATTR_MAX: highest GScan channel attribute. > > Generally, you should also document the attribute types here (and > everywhere else really) Will do. >> + NL80211_BUCKET_BAND_2GHZ = (1 << 0), > > no need for parentheses with enums :) Ok. >> + if (tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]) >> + chan->dwell_time = >> nla_get_u32(tb[NL80211_GSCAN_CHAN_ATTR_DWELL_TIME]); > > Maybe that should have some kind of "reasonable range" limit? Agree. > So I mostly looked at this from a pure code POV - need to compare with > our implementation, but I guess the basis is the same ... Given its origin I would guess so too. What I propose here is minimal behavior so just the logic around the buckets. I wanted to add features like BSS hotlist and ePNO stuff in separate patches. Apart from that the iwlwifi implementation has some differences in details like attribute validation and there is no base period attribute as that must be the gcd of the bucket periods. So I might drop that as well. Regards, Arend
