| Message ID | 20191119221509.11370-1-prestwoj@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | mac80211: set start_time_tsf/tsf_bssid for sw scans | expand |
On Tue, 2019-11-19 at 14:15 -0800, James Prestwood wrote: > These values are already tracked so for the software scan path > we can set these into scan_info so NL80211 reports it in > TRIGGER_SCAN. No. > + req->info.scan_start_tsf = req->scan_start; These are two very different things. johannes
On Fri, 2019-11-22 at 13:04 +0100, Johannes Berg wrote: > On Tue, 2019-11-19 at 14:15 -0800, James Prestwood wrote: > > These values are already tracked so for the software scan path > > we can set these into scan_info so NL80211 reports it in > > TRIGGER_SCAN. > > No. > > > + req->info.scan_start_tsf = req->scan_start; > > These are two very different things. Is this a jiffies vs TSF issue? Or are they really completely different things? Looking at iwlwifi I now see that I wrongly read the scan_start values they were using. I thought they were setting mvm->scan_start = jiffies, but that was in another structure. mvm->scan_start gets set to scan_start_tsf, and looks like it comes from a scan complete callback. So if this value is only obtainable in the driver, I'll drop this and maybe add support into mac80211_hwsim. I don't mean to hijack this patch thread (its related), but there is some confusion about these ext features in nl80211: NL80211_EXT_FEATURE_SCAN_START_TIME NL80211_EXT_FEATURE_BSS_PARENT_TSF They were only added into iwlwifi and its not clear why or whether these features should even be checked by userspace (wpa_supplicant does not check). We were planning on using these to enable support for beacon measurement requests (if set) but its looking like, for the majority of hardware, these values are not included in scan results, meaning they end up being zero/unset in the measurement report (as they do with wpa_supplicant). The spec is does not explicitly say these values are required for measurement reports, but it also doesn't say they are optional. Am I just looking too deeply into this? Maybe its the case that an AP which utilizes measurement requests/reports doesn't even care about these values? There is also a ext feature for RRM as a whole which further confuses me. It would seem like if RRM is supported both these other features also need to be supported. And maybe this is the case, and only iwlwifi 'technically' works with RRM (at least if you follow the spec and require inclusion of these two values). Thanks, James > > johannes >
> > > + req->info.scan_start_tsf = req->scan_start; > > > > These are two very different things. > > Is this a jiffies vs TSF issue? Or are they really completely different > things? Two different things. You're supposed to report the TSF of your AP when the scan started, not some local clock. > Looking at iwlwifi I now see that I wrongly read the scan_start values > they were using. I thought they were setting mvm->scan_start = jiffies, > but that was in another structure. mvm->scan_start gets set to > scan_start_tsf, and looks like it comes from a scan complete callback. > > So if this value is only obtainable in the driver, I'll drop this and > maybe add support into mac80211_hwsim. Right, I think you can only do it in the driver, see above. > I don't mean to hijack this patch thread (its related), but there is > some confusion about these ext features in nl80211: > > NL80211_EXT_FEATURE_SCAN_START_TIME > NL80211_EXT_FEATURE_BSS_PARENT_TSF > > They were only added into iwlwifi and its not clear why or whether > these features should even be checked by userspace (wpa_supplicant does > not check). We were planning on using these to enable support for > beacon measurement requests (if set) but its looking like, for the > majority of hardware, these values are not included in scan results, > meaning they end up being zero/unset in the measurement report (as they > do with wpa_supplicant). The spec is does not explicitly say these > values are required for measurement reports, but it also doesn't say > they are optional. I don't recall exactly which program they were required by, off the top of my head. I could look it up but not right now. But yes, this is basically used for measurement request/report. > Am I just looking too deeply into this? Maybe its the case that an AP > which utilizes measurement requests/reports doesn't even care about > these values? There is also a ext feature for RRM as a whole which > further confuses me. It would seem like if RRM is supported both these > other features also need to be supported. And maybe this is the case, > and only iwlwifi 'technically' works with RRM (at least if you follow > the spec and require inclusion of these two values). Well, there's this spec and this spec ... So IEEE 802.11 might have all this be optional, but some WFA programs might require it. I think this is the case of the latter, but I don't know off the top of my head which one (and even if I did I'd have to think about whether I can mention it in public ...) johannes
diff --git a/net/mac80211/main.c b/net/mac80211/main.c index 4c2702f128f3..ccbad5499e40 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -587,6 +587,8 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len, NL80211_EXT_FEATURE_SCAN_RANDOM_SN); wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_SCAN_MIN_PREQ_CONTENT); + wiphy_ext_feature_set(wiphy, + NL80211_EXT_FEATURE_SCAN_START_TIME); } if (!ops->set_key) diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c index adf94ba1ed77..2559ae76b97a 100644 --- a/net/mac80211/scan.c +++ b/net/mac80211/scan.c @@ -700,6 +700,12 @@ static int __ieee80211_start_scan(struct ieee80211_sub_if_data *sdata, else memcpy(local->scan_addr, sdata->vif.addr, ETH_ALEN); + if (!hw_scan && sdata->vif.bss_conf.assoc) { + req->info.scan_start_tsf = req->scan_start; + ether_addr_copy(local->scan_info.tsf_bssid, + sdata->vif.bss_conf.bssid); + } + if (hw_scan) { __set_bit(SCAN_HW_SCANNING, &local->scanning); } else if ((req->n_channels == 1) &&
These values are already tracked so for the software scan path we can set these into scan_info so NL80211 reports it in TRIGGER_SCAN. This patch also sets NL80211_EXT_FEATURE_SCAN_START_TIME in mac80211 if hw scanning is not available since driver support is not required. Signed-off-by: James Prestwood <prestwoj@gmail.com> --- net/mac80211/main.c | 2 ++ net/mac80211/scan.c | 6 ++++++ 2 files changed, 8 insertions(+)