2019-11-19 22:21:39

by James Prestwood

[permalink] [raw]
Subject: [PATCH] mac80211: set start_time_tsf/tsf_bssid for sw scans

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 <[email protected]>
---
net/mac80211/main.c | 2 ++
net/mac80211/scan.c | 6 ++++++
2 files changed, 8 insertions(+)

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) &&
--
2.17.1



2019-11-22 12:04:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: set start_time_tsf/tsf_bssid for sw scans

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

2019-11-22 17:43:35

by James Prestwood

[permalink] [raw]
Subject: Re: [PATCH] mac80211: set start_time_tsf/tsf_bssid for sw scans

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
>

2019-11-22 17:46:46

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: set start_time_tsf/tsf_bssid for sw scans


> > > + 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