From: Vamsi Krishna <[email protected]>
NL80211_ATTR_MAC was used to set both the specific BSSID to be scanned
and the random MAC address to be used when privacy is enabled. When both
the features are enabled, both the BSSID and the local MAC address were
getting same value causing Probe Request frames to go with unintended
DA. Hence, this has been fixed by using a different NL80211_ATTR_BSSID
attribute to set the specific BSSID (which was the more recent addition
in cfg80211) for a scan.
Backwards compatibility with old userspace software is maintained to
some extent by allowing NL80211_ATTR_MAC to be used to set the specific
BSSID when scanning without enabling random MAC address use.
Scanning with random source MAC address was introduced by commit
ad2b26abc157 ("cfg80211: allow drivers to support random MAC addresses
for scan") and the issue was introduced with the addition of the second
user for the same attribute in commit 818965d39177 ("cfg80211: Allow a
scan request for a specific BSSID").
Fixes: 818965d39177 ("cfg80211: Allow a scan request for a specific BSSID")
Signed-off-by: Vamsi Krishna <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
include/uapi/linux/nl80211.h | 7 ++++++-
net/wireless/nl80211.c | 16 +++++++++++++++-
2 files changed, 21 insertions(+), 2 deletions(-)
v2: address comments from Luca and Johannes
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 259c9c7..6b76e3b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -323,7 +323,7 @@
* @NL80211_CMD_GET_SCAN: get scan results
* @NL80211_CMD_TRIGGER_SCAN: trigger a new scan with the given parameters
* %NL80211_ATTR_TX_NO_CCK_RATE is used to decide whether to send the
- * probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used to
+ * probe requests at CCK rate or not. %NL80211_ATTR_BSSID can be used to
* specify a BSSID to scan for; if not included, the wildcard BSSID will
* be used.
* @NL80211_CMD_NEW_SCAN_RESULTS: scan notification (as a reply to
@@ -1977,6 +1977,9 @@ enum nl80211_commands {
* @NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED: Indicates whether or not multicast
* packets should be send out as unicast to all stations (flag attribute).
*
+ * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
+ * used in various commands/events for specifying the BSSID.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2381,6 +2384,8 @@ enum nl80211_attrs {
NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED,
+ NL80211_ATTR_BSSID,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e4f718e..7762231 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -404,6 +404,7 @@ enum nl80211_multicast_groups {
.len = FILS_MAX_KEK_LEN },
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
+ [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
};
/* policy for the key attributes */
@@ -6703,7 +6704,20 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
request->no_cck =
nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
- if (info->attrs[NL80211_ATTR_MAC])
+ /* Initial implementation used NL80211_ATTR_MAC to set the specific
+ * BSSID to scan for. This was problematic because that same attribute
+ * was already used for another purpose (local random MAC address). The
+ * NL80211_ATTR_BSSID attribute was added to fix this. For backwards
+ * compatibility with older userspace components, also use the
+ * NL80211_ATTR_MAC value here if it can be determined to be used for
+ * the specific BSSID use case instead of the random MAC address
+ * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC address use).
+ */
+ if (info->attrs[NL80211_ATTR_BSSID])
+ memcpy(request->bssid,
+ nla_data(info->attrs[NL80211_ATTR_BSSID]), ETH_ALEN);
+ else if (!(request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) &&
+ info->attrs[NL80211_ATTR_MAC])
memcpy(request->bssid, nla_data(info->attrs[NL80211_ATTR_MAC]),
ETH_ALEN);
else
--
1.9.1
Ok... this is getting complicated :)
Regarding reusing attributes, we have (for the BSS selection thing) the
attribute NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, which is really quite
similar to your new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since
while connected (which BSS_SELECT_ATTR_* assumes) the current BSS is
always part of the considered BSSes, I'd think.
However, I tend to think now that reusing the attribute is perhaps not
the right thing to do - but defining them with the same semantics would
still make sense.
Assuming that the value defined in NL80211_BSS_SELECT_ATTR_RSSI_ADJUST
applies also to the *current* BSS, it's actually quite pointless to
define there the band to adjust - if you want to adjust 2.4 GHz
positively you might as well adjust 5 GHz negatively, and vice versa,
and both ways are supported.
OTOH, the new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF doesn't
make this quite clear - is the current BSS to be adjusted before
comparing, if it's 5 GHz? If so, the semantics are equivalent. If not,
it doesn't actually make much sense ;-)
So assuming that it is in fact taken into account after the same
adjustment, the two attributes are equivalent, and then perhaps it
would make sense to use struct nl80211_bss_select_rssi_adjust for the
new attribute. If a driver doesn't support arbitrary bands, but just 5
GHz as in your example, it can just flip it around to 2.4 GHz by
switching the sign.
Perhaps we should even consider doing that in cfg80211 and adjusting
the internal API for both that way?
> I am not saying it should be avoided. Just looking at it conceptually
> the scheduled scan request holds so-called matchsets that specify the
> constraints to determine whether a BSS was found that is worth
> notifying the host/user-space about. As such I would expect the
> relative RSSI attribute(s) to be part of the matchset. That way you
> can specify it together with the currently connected SSID in a single
> matchset.
I think this makes a lot of sense.
We already have NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, which asks to be
reporting only networks that have an *absolute* RSSI value above the
value of the attribute - a new attribute to make it relative to the
current network instead would make sense.
That would indeed be equivalent to NL80211_BSS_SELECT_ATTR_RSSI then.
Now, if we consider this, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
actually is equivalent to NL80211_BSS_SELECT_ATTR_RSSI (a flag
attribute indicating whether or not RSSI-based selection/matching is
done) and NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF is equivalent
to NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, both need to be given with the
flag and affect operation.
However, NL80211_BSS_SELECT_ATTR_BAND_PREF doesn't exist, and reusing
the BSS_SELECT namespace also doesn't make sense.
So, how about we move these into NL80211_SCHED_SCAN_MATCH_ATTR_* as
suggested by Arend, and define them with the same content as the
corresponding NL80211_BSS_SELECT_ATTR_*?
If they're part of match attributes, we might even remove the feature
flag entirely - those were always defined to be optional, but it very
well be worthwhile for userspace to know if they're supported if it
wants to behave differently depending on whether they're supported or
not, I'll leave that up to you since presumably you know the userspace
implementation that you're planning to create.
johannes
T24gVHVlLCBEZWMgMTMsIDIwMTYgYXQgMDQ6NTY6NTVQTSArMDEwMCwgSm9oYW5uZXMgQmVyZyB3
cm90ZToNCj4gUmVnYXJkaW5nIHJldXNpbmcgYXR0cmlidXRlcywgd2UgaGF2ZSAoZm9yIHRoZSBC
U1Mgc2VsZWN0aW9uIHRoaW5nKSB0aGUNCj4gYXR0cmlidXRlIE5MODAyMTFfQlNTX1NFTEVDVF9B
VFRSX1JTU0lfQURKVVNULCB3aGljaCBpcyByZWFsbHkgcXVpdGUNCj4gc2ltaWxhciB0byB5b3Vy
IG5ld8KgTkw4MDIxMV9BVFRSX1NDSEVEX1NDQU5fUkVMQVRJVkVfUlNTSV81R19QUkVGIHNpbmNl
DQo+IHdoaWxlIGNvbm5lY3RlZCAod2hpY2ggQlNTX1NFTEVDVF9BVFRSXyogYXNzdW1lcykgdGhl
IGN1cnJlbnQgQlNTIGlzDQo+IGFsd2F5cyBwYXJ0IG9mIHRoZSBjb25zaWRlcmVkIEJTU2VzLCBJ
J2QgdGhpbmsuDQoNCkl0IHNlZW1zIHRvIGhhdmUgc29tZXdoYXQgc2ltaWxhciBtZWFuaW5nLCBi
dXQgaXQgbG9va3MgbW9yZSBnZW5lcmljIGJ5DQpub3QgYmVpbmcgdGllZCB0byBqdXN0IHR3byBi
YW5kcyAoMi40IGFuZCA1KS4gQW5kIG5vdyB0aGF0IEkgYWN0dWFsbHkNCmxvb2tlZCBhZ2FpbiBh
dCB0aGlzLCBpdCBkb2VzIG5vdCBsb29rIGxpa2UNCk5MODAyMTFfQlNTX1NFTEVDVF9BVFRSX1JT
U0lfQURKVVNUIGFjdHVhbGx5IGFsbG93cyBtb3JlIHRoYW4gYSBzaW5nbGUNCmJhbmQsZGVsdGEg
cGFpciB0byBiZSBwcm92aWRlZCBhbmQgYXMgc3VjaCwgaXQgYWN0dWFsbHkgd291bGQgbm90IHdv
cmsNCnZlcnkgd2VsbCB3aXRoIG1vcmUgdGhhbiB0d28gYmFuZHMgZXZlbiBpZiBpdCBtaWdodCBi
ZSBhIGJpdCBtb3JlDQpnZW5lcmljIGJ5IGFsbG93aW5nIGJhbmQgdG8gYmUgc2V0IHRvIHNvbWV0
aGluZyBlbHNlIHRoYW4gMi40IG9yIDUuIEJ5DQp0aGUgd2F5LCBubDgwMjExLmggZG9lcyBub3Qg
c2VlbSB0byBkb2N1bWVudCB3aGF0IHZhbHVlcyBzdHJ1Y3QNCm5sODIwMTFfYnNzX3NlbGVjdF9y
c3NpX2FkanVzdCBiYW5kIHVzZXMuLiAgSXMgdGhpcyBlbnVtIG5sODAyMTFfYmFuZD8NCk5laXRo
ZXIgZG9lcyBpdCBzYXkgdGhhdCBkZWx0YSBpcyBpbiBkQiAoaXMgaXQ/KS4NCg0KPiBPVE9ILCB0
aGUgbmV3wqBOTDgwMjExX0FUVFJfU0NIRURfU0NBTl9SRUxBVElWRV9SU1NJXzVHX1BSRUYgZG9l
c24ndA0KPiBtYWtlIHRoaXMgcXVpdGUgY2xlYXIgLSBpcyB0aGUgY3VycmVudCBCU1MgdG8gYmUg
YWRqdXN0ZWQgYmVmb3JlDQo+IGNvbXBhcmluZywgaWYgaXQncyA1IEdIej8gSWYgc28sIHRoZSBz
ZW1hbnRpY3MgYXJlIGVxdWl2YWxlbnQuIElmIG5vdCwNCj4gaXQgZG9lc24ndCBhY3R1YWxseSBt
YWtlIG11Y2ggc2Vuc2UgOy0pDQoNCk1heWJlIHRoZSBubDgwMjExLmggZGVzY3JpcHRpb24gd2Fz
IG5vdCBjbGVhciBlbm91Z2gsIGJ1dCB0aGUgY29tbWVudHMNCmluIGNmZzgwMjExLmggc2hvdWxk
IGJlIHF1aXRlIGNsZWFyIG9uIGhvdyB0aGlzIHdhcyBkZXNpZ25lZCB0byB3b3JrIGF0DQp0aGUg
aW1wbGVtZW50YXRpb24gbGV2ZWw6DQoNCiJJZiB0aGUgY3VycmVudCBjb25uZWN0ZWQgQlNTIGlz
IGluIHRoZSAyLjQgR0h6IGJhbmQsIG90aGVyIEJTU3MgaW4gdGhlDQoyLjQgR0h6IGJhbmQgdG8g
YmUgcmVwb3J0ZWQgc2hvdWxkIGhhdmUgYmV0dGVyIFJTU0kgYnkgQHJlbGF0aXZlX3Jzc2kNCmFu
ZCBvdGhlciBCU1NzIGluIHRoZSA1IEdIeiBiYW5kIHRvIGJlIHJlcG9ydGVkIHNob3VsZCBoYXZl
IGJldHRlciBSU1NJDQpieSAoQHJlbGF0aXZlX3Jzc2kgLSBAcmVsYXRpdmVfcnNzaV81Z19wcmVm
KS4NCklmIHRoZSBjdXJyZW50IGNvbm5lY3RlZCBCU1MgaXMgaW4gdGhlIDUgR0h6IGJhbmQsIG90
aGVyIEJTU3MgaW4gdGhlDQoyLjQgR0h6IGJhbmQgdG8gYmUgcmVwb3J0ZWQgc2hvdWxkIGhhdmUg
YmV0dGVyIFJTU0kgYnkNCihAcmVsYXRpdmVfcnNzaSArIEByZWxhdGl2ZV9yc3NpXzVnX3ByZWYp
IGFuZCBvdGhlciBCU1NzIGluIHRoZSA1IEdIeg0KYmFuZCB0byBiZSByZXBvcnRlZCBzaG91bGQg
aGF2ZSBiZXR0ZXIgUlNTSSBieSBieSBAcmVsYXRpdmVfcnNzaS4iDQoNCkknbSBub3Qgc3VyZSBJ
J2QgZGVzY3JpYmUgdGhpcyBhcyBhZGp1c3RpbmcgdGhlIGN1cnJlbnQgQlNTIFJTU0ksIGJ1dA0K
bW9yZSBsaWtlIGFkanVzdGluZyB0aGUgUlNTSSB0aHJlc2hvbGQgdmFsdWUgaWYgcm9hbWluZyB3
b3VsZCBiZSBmcm9tDQpvbmUgYmFuZCB0byBhbm90aGVyIGFuZCBkb2luZyB0aGF0IGFkanVzdG1l
bnQgYnkgYWRkaW5nIG9yIGRlY3JlbWVudGluZw0KYmFzZWQgb24gd2hldGhlciB0aGUgcm9hbSB3
b3VsZCBiZSBmcm9tIDIuNCB0byA1IG9yIGZyb20gNSB0byAyLjQuDQoNCj4gU28gYXNzdW1pbmcg
dGhhdCBpdCBpcyBpbiBmYWN0IHRha2VuIGludG8gYWNjb3VudCBhZnRlciB0aGUgc2FtZQ0KPiBh
ZGp1c3RtZW50LCB0aGUgdHdvIGF0dHJpYnV0ZXMgYXJlIGVxdWl2YWxlbnQsIGFuZCB0aGVuIHBl
cmhhcHMgaXQNCj4gd291bGQgbWFrZSBzZW5zZSB0byB1c2Ugc3RydWN0IG5sODAyMTFfYnNzX3Nl
bGVjdF9yc3NpX2FkanVzdCBmb3IgdGhlDQo+IG5ldyBhdHRyaWJ1dGUuIElmIGEgZHJpdmVyIGRv
ZXNuJ3Qgc3VwcG9ydCBhcmJpdHJhcnkgYmFuZHMsIGJ1dCBqdXN0IDUNCj4gR0h6IGFzIGluIHlv
dXIgZXhhbXBsZSwgaXQgY2FuIGp1c3QgZmxpcCBpdCBhcm91bmQgdG8gMi40IEdIeiBieQ0KPiBz
d2l0Y2hpbmcgdGhlIHNpZ24uDQo+IA0KPiBQZXJoYXBzIHdlIHNob3VsZCBldmVuIGNvbnNpZGVy
IGRvaW5nIHRoYXQgaW4gY2ZnODAyMTEgYW5kIGFkanVzdGluZw0KPiB0aGUgaW50ZXJuYWwgQVBJ
IGZvciBib3RoIHRoYXQgd2F5Pw0KDQpJJ20gbm90IGNvbXBsZXRlbHkgc3VyZSBJIHVuZGVyc3Rv
b2QuIE9uZSB0aGluZyB0byBub3RlIGFib3V0DQpkaWZmZXJlbmNlcyBoZXJlIGlzIHRoYXQgTkw4
MDIxMV9CU1NfU0VMRUNUX0FUVFJfKiBzZWVtcyB0byBiZSBkZWZpbmluZw0Kc29tZSBwcmVmZXJl
bmNlcyBmb3IgQlNTIHNlbGVjdGlvbiBiYXNlZCBvbiBSU1NJIHdpdGggYW4gYWRkaXRpb25hbCBi
YW5kDQpwcmVmZXJlbmNlLCBidXQgaXQgZG9lcyBub3Qgc2VlbSB0byBkZWZpbmUgdGhlIHRocmVz
aG9sZCBieSBob3cgbWFueSBkQg0KdGhlIG5ldyBjYW5kaWRhdGUgQlNTIHNob3VsZCBiZSBiZXR0
ZXIuDQoNCkF0IG1pbmltdW0sIHdlIHdvdWxkIG5lZWQgdG8gY2xlYXJseSBkb2N1bWVudCBzdHJ1
Y3QNCm5sODAyMTFfYnNzX3NlbGVjdF9yc3NpX2FkanVzdCwgYnV0IGV2ZW4gaWYgd2UgZG8sIEkn
bSBub3Qgc3VyZSBpdA0KcmVhbGx5IGlzIGlkZWFsIG1lY2hhbmlzbSB0byBtb3ZlIHRvIG5vdyB0
aGF0IEkgcmVhbGl6ZWQgaXQgaXMgbm90IGFuDQphcnJheSwgYnV0IGEgc2luZ2xlIGJhbmQsZGVs
dGEgcGFpci4NCg0KQXJlbmQ6DQo+ID4gSSBhbSBub3Qgc2F5aW5nIGl0IHNob3VsZCBiZSBhdm9p
ZGVkLiBKdXN0IGxvb2tpbmcgYXQgaXQgY29uY2VwdHVhbGx5DQo+ID4gdGhlIHNjaGVkdWxlZCBz
Y2FuIHJlcXVlc3QgaG9sZHMgc28tY2FsbGVkIG1hdGNoc2V0cyB0aGF0IHNwZWNpZnkgdGhlDQo+
ID4gY29uc3RyYWludHMgdG8gZGV0ZXJtaW5lIHdoZXRoZXIgYSBCU1Mgd2FzIGZvdW5kIHRoYXQg
aXMgd29ydGgNCj4gPiBub3RpZnlpbmcgdGhlIGhvc3QvdXNlci1zcGFjZSBhYm91dC4gQXMgc3Vj
aCBJIHdvdWxkIGV4cGVjdCB0aGUNCj4gPiByZWxhdGl2ZSBSU1NJIGF0dHJpYnV0ZShzKSB0byBi
ZSBwYXJ0IG9mIHRoZSBtYXRjaHNldC4gVGhhdCB3YXkgeW91DQo+ID4gY2FuIHNwZWNpZnkgaXQg
dG9nZXRoZXIgd2l0aCB0aGUgY3VycmVudGx5IGNvbm5lY3RlZCBTU0lEIGluIGEgc2luZ2xlDQo+
ID4gbWF0Y2hzZXQuDQo+IA0KPiBJIHRoaW5rIHRoaXMgbWFrZXMgYSBsb3Qgb2Ygc2Vuc2UuDQoN
CklmIHdlIGFyZSB0YWxraW5nIG9ubHkgYWJvdXQgcm9hbWluZyB3aXRoaW4gYW4gRVNTIChhIHNp
bmdsZSBTU0lEKSwgdGhhdA0Kd291bGQgc291bmQgY2xlYXIsIGJ1dCB3aGljaCByZWxhdGl2ZSBS
U1NJIHJ1bGVzIHdvdWxkIGFwcGx5IGlmIHRoZXJlDQphcmUgbWF0Y2ggc2V0cyBmb3IgYm90aCB0
aGUgY3VycmVudGx5IGNvbm5lY3RlZCBTU0lEIGFuZCBhbm90aGVyIFNTSUQNCnRoYXQgdGhlIGNh
bmRpZGF0ZSBCU1MgaXMgZm9yPw0KDQo+IFdlIGFscmVhZHkgaGF2ZcKgTkw4MDIxMV9TQ0hFRF9T
Q0FOX01BVENIX0FUVFJfUlNTSSwgd2hpY2ggYXNrcyB0byBiZQ0KPiByZXBvcnRpbmcgb25seSBu
ZXR3b3JrcyB0aGF0IGhhdmUgYW4gKmFic29sdXRlKiBSU1NJIHZhbHVlIGFib3ZlIHRoZQ0KPiB2
YWx1ZSBvZiB0aGUgYXR0cmlidXRlIC0gYSBuZXcgYXR0cmlidXRlIHRvIG1ha2UgaXQgcmVsYXRp
dmUgdG8gdGhlDQo+IGN1cnJlbnQgbmV0d29yayBpbnN0ZWFkIHdvdWxkIG1ha2Ugc2Vuc2UuDQoN
CldoZW4geW91IHNheSAiY3VycmVudCBuZXR3b3JrIiwgZG8geW91IG1lYW4gdGhlIGN1cnJlbnQg
QlNTPyBUaGlzIGdldHMNCmNvbXBsZXggd2hlbiB0aGlua2luZyBhYm91dCBtdWx0aXBsZSBTU0lE
cyAod2hpY2ggc29tZSBjYWxsICJuZXR3b3JrcyIpDQphbmQgdGhlcmUgYmVpbmcgTkw4MDIxMV9T
Q0hFRF9TQ0FOX01BVENIX0FUVFJfU1NJRCBhbmQgbXVsdGlwbGUNCm1hdGNoIHNldHMuLg0KDQo+
IFRoYXQgd291bGQgaW5kZWVkIGJlIGVxdWl2YWxlbnQgdG8gTkw4MDIxMV9CU1NfU0VMRUNUX0FU
VFJfUlNTSSB0aGVuLg0KDQpOTDgwMjExX0JTU19TRUxFQ1RfQVRUUl9SU1NJIGlzIGEgZmxhZyBh
dHRyaWJ1dGUuLiBCU1Mgc2VsZWN0IG1lY2hhbmlzbQ0KZG9lcyBub3QgcHJvdmlkZSBhbiBhYnNv
bHV0ZSBSU1NJIHZhbHVlIG9yIHRocmVzaG9sZDsgaXQganVzdCBzZWVtcyB0bw0KaW5kaWNhdGUg
dXNlIG9mIFJTU0ktYmFzZWQgc2VsZWN0aW9uIG1lY2hhbmlzbSB3aXRob3V0IGRlZmluaW5nIHdo
YXQNCmV4YWN0bHkgaXMgYmV0dGVyLiBUaGVyZSBpcyBOTDgwMjExX0JTU19TRUxFQ1RfQVRUUl9C
QU5EX1BSRUYgdGhhdCBnaXZlcw0KYSBwcmVmZXJlbmNlIHRvIGEgc3BlY2lmaWMgYmFuZCAod2l0
aG91dCBkZWZpbmluZyB3aGF0IHRoYXQgcHJlZmVyZW5jZQ0KaXMpIGFuZCB0aGVuIHRoZSBOTDgw
MjExX0JTU19TRUxFQ1RfQVRUUl9SU1NJX0FESlVTVCB0aGF0IGNhbiBhY3R1YWxseQ0KZ2l2ZSBh
IHNwZWNpZmljIFJTU0kgYWRqdXN0bWVudCB2YWx1ZSAoaW4gZEI/KS4NCg0KPiBOb3csIGlmIHdl
IGNvbnNpZGVyIHRoaXMswqBOTDgwMjExX0FUVFJfU0NIRURfU0NBTl9SRUxBVElWRV9SU1NJDQo+
IGFjdHVhbGx5IGlzIGVxdWl2YWxlbnQgdG/CoE5MODAyMTFfQlNTX1NFTEVDVF9BVFRSX1JTU0kg
KGEgZmxhZw0KPiBhdHRyaWJ1dGUgaW5kaWNhdGluZyB3aGV0aGVyIG9yIG5vdCBSU1NJLWJhc2Vk
IHNlbGVjdGlvbi9tYXRjaGluZyBpcw0KPiBkb25lKSBhbmTCoE5MODAyMTFfQVRUUl9TQ0hFRF9T
Q0FOX1JFTEFUSVZFX1JTU0lfNUdfUFJFRiBpcyBlcXVpdmFsZW50DQo+IHRvwqBOTDgwMjExX0JT
U19TRUxFQ1RfQVRUUl9SU1NJX0FESlVTVCwgYm90aCBuZWVkIHRvIGJlIGdpdmVuIHdpdGggdGhl
DQo+IGZsYWcgYW5kIGFmZmVjdCBvcGVyYXRpb24uDQoNCkhtbS4uIFNvIHlvdSBkaWQgbm90aWNl
IGl0IGlzIGEgZmxhZyBhdHRyaWJ1dGUuLiBTbyBob3cgd291bGQgdGhpcyBtYXRjaA0KTkw4MDIx
MV9BVFRSX1NDSEVEX1NDQU5fUkVMQVRJVkVfUlNTSSB3aGljaCBwcm92aWRlcyB0aGUgdGhyZXNo
b2xkIHZhbHVlDQpmb3IgaG93IG1hbnkgZEIgYmV0dGVyIHRoZSBuZXcgQlNTIG5lZWRzIHRvIGJl
IGZvciBpdCB0byBiZSByZXBvcnRlZD8NCg0KPiBTbywgaG93IGFib3V0IHdlIG1vdmUgdGhlc2Ug
aW50b8KgTkw4MDIxMV9TQ0hFRF9TQ0FOX01BVENIX0FUVFJfKiBhcw0KPiBzdWdnZXN0ZWQgYnkg
QXJlbmQsIGFuZCBkZWZpbmUgdGhlbSB3aXRoIHRoZSBzYW1lIGNvbnRlbnQgYXMgwqB0aGUNCj4g
Y29ycmVzcG9uZGluZyBOTDgwMjExX0JTU19TRUxFQ1RfQVRUUl8qPw0KDQpJIHRoaW5rIEknbSBt
aXNzaW5nIHNvbWV0aGluZyBoZXJlLi4gV2hlcmUgd291bGQgdGhlIHRocmVzaG9sZCB2YWx1ZQ0K
KGhvdyBtdWNoIGJldHRlciBuZXcgQlNTIG5lZWRzIHRvIGJlKSBiZSBzdG9yZWQ/IEFuZCBkbyB3
ZSByZWFsbHkgd2FudA0Kc29tZXRoaW5nIGxpa2UgdGhlIGNvbWJpbmF0aW9uIG9mIE5MODAyMTFf
QlNTX1NFTEVDVF9BVFRSX0JBTkRfUFJFRiBhbmQNCk5MODAyMTFfQlNTX1NFTEVDVF9BVFRSX1JT
U0lfQURKVVNUIHdoaWNoIHNlZW1zIHRvIGJlIHR3byBkaWZmZXJlbnQgd2F5cw0Kb2YgZG9pbmcg
YmFuZCBwcmVmZXJlbmNlICh0aGUgZm9ybWVyIHdpdGhvdXQgc3BlY2lmeWluZyBkZWx0YSBhbmQg
dGhlDQpsYXR0ZXIgd2l0aCBzcGVjaWZpYyBkZWx0YSk/IE9yIGFtIEkgc3RpbGwgbm90IHVuZGVy
c3RhbmRpbmcgaG93DQpOTDgwMjExX0JTU19TRUxFQ1RfQVRUUl8qIHJlYWxseSB3b3Jrcz8NCg0K
PiBJZiB0aGV5J3JlIHBhcnQgb2YgbWF0Y2ggYXR0cmlidXRlcywgd2UgbWlnaHQgZXZlbiByZW1v
dmUgdGhlIGZlYXR1cmUNCj4gZmxhZyBlbnRpcmVseSAtIHRob3NlIHdlcmUgYWx3YXlzIGRlZmlu
ZWQgdG8gYmUgb3B0aW9uYWwsIGJ1dCBpdCB2ZXJ5DQo+IHdlbGwgYmUgd29ydGh3aGlsZSBmb3Ig
dXNlcnNwYWNlIHRvIGtub3cgaWYgdGhleSdyZSBzdXBwb3J0ZWQgaWYgaXQNCj4gd2FudHMgdG8g
YmVoYXZlIGRpZmZlcmVudGx5IGRlcGVuZGluZyBvbiB3aGV0aGVyIHRoZXkncmUgc3VwcG9ydGVk
IG9yDQo+IG5vdCwgSSdsbCBsZWF2ZSB0aGF0IHVwIHRvIHlvdSBzaW5jZSBwcmVzdW1hYmx5IHlv
dSBrbm93IHRoZSB1c2Vyc3BhY2UNCj4gaW1wbGVtZW50YXRpb24gdGhhdCB5b3UncmUgcGxhbm5p
bmcgdG8gY3JlYXRlLg0KDQpUaGUgbWFpbiBjb25jZXJuIEkgaGF2ZSBmb3Igb3B0aW9uYWwgZmVh
dHVyZXMgd2l0aCBzY2hlZF9zY2FuIGlzIGluDQp3aGV0aGVyIHRoZSBkZXZpY2UgZW5kcyB1cCBi
ZWluZyB3b2tlbiB1cCBjb25zdGFudGx5IGlmIHRoZSBkcml2ZXIgZG9lcw0Kbm90IHVuZGVyc3Rh
bmQgYSBjb25zdHJhaW50IHRoYXQgdXNlciBzcGFjZSBpcyB0cnlpbmcgdG8gdXNlIHRvIGF2b2lk
DQpiZWluZyBub3RpZmllZCBhbGwgdGhlIHRpbWUuDQoNCi0tIA0KSm91bmkgTWFsaW5lbiAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgUEdQIGlkIEVGQzg5NUZB
On 8-12-2016 18:52, Malinen, Jouni wrote:
> On Wed, Dec 07, 2016 at 09:03:23PM +0100, Arend Van Spriel wrote:
>> On 7-12-2016 10:33, Vamsi, Krishna wrote:
>>>> -----Original Message-----
>>>> From: Johannes Berg [mailto:[email protected]]
>>>
>>>> What about Arend's comment regarding this functionality overlapping with the
>>>> BSS selection offload configuration?
>>>>
>>>> Do you think there's any ability to share attributes/functionality?
>>>
>>> Hi Johannes,
>>>
>>> I think the later comment from Luca on this which I pasted below is more agreeable.
>>>
>>> Yes, similar but not quite the same. The existing case is for finding BSSs that are worth waking the host up for (while disconnected), so it needs to be better than the RSSI passed (absolute number). Now this is about relative RSSI as compared to the current connection, so RELATIVE_RSSI is different than RSSI and I think the same attribute should not be used, to avoid confusion.
>>
>> I noticed the response from Luca, but did not get back on this. I know
>> it is not the same, but what I meant is whether we could extend it so it
>> also covers your scenario.
>
> I'm not sure I see the point of trying to avoid the new RELATIVE_RSSI
> attribute. We need to clearly specify that this new constraint is indeed
> for relative comparison against the currently connected BSS.
Hi Jouni,
I am not saying it should be avoided. Just looking at it conceptually
the scheduled scan request holds so-called matchsets that specify the
constraints to determine whether a BSS was found that is worth notifying
the host/user-space about. As such I would expect the relative RSSI
attribute(s) to be part of the matchset. That way you can specify it
together with the currently connected SSID in a single matchset.
> As far as your second email is concerned, it might make more sense to
> use the existing NL80211_ATTR_BSS_SELECT instead of the new
> NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since they cover very
> similar purpose in defining RSSI preference between bands. We can take a
> look at doing so. One thing to be a careful about this is in what claims
> there are about using NL80211_ATTR_BSS_SELECT for capability indication
> in GET_WIPHY. I guess we can leave that as-is to apply for _CONNECT and
> the new EXT_FEATURE flag we are adding for sched_scan applies for this
> attribute in SCHED_SCAN.
The NL80211_ATTR_BSS_SELECT supports different methods for BSS
selection. One of them being RSSI_ADJUST which is similar to this. It
lets user-space specify the band and adjustment instead of having a band
implied in the attribute name. Agree that if reusing it for scheduled
scan you would still need the EXT_FEATURE flag.
Regards,
Arend
From: vamsi krishna <[email protected]>
Enhance sched scan to support option of finding a better BSS while in
connected state. Firmware scans the medium and reports when it finds a
known BSS which has better RSSI than the current connected BSS. New
attributes to specify the relative RSSI (compared to the current BSS)
are added to the sched scan to implement this.
Signed-off-by: vamsi krishna <[email protected]>
Signed-off-by: Jouni Malinen <[email protected]>
---
include/net/cfg80211.h | 19 +++++++++++++++++++
include/uapi/linux/nl80211.h | 18 ++++++++++++++++++
net/wireless/nl80211.c | 29 +++++++++++++++++++++++++++--
3 files changed, 64 insertions(+), 2 deletions(-)
v2: address comments from Luca, Arend, and Johannes
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ef42749..dcdd0c4 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1626,6 +1626,22 @@ struct cfg80211_sched_scan_plan {
* cycle. The driver may ignore this parameter and start
* immediately (or at any other time), if this feature is not
* supported.
+ * @relative_rssi: Relative RSSI threshold in dB to restrict scan result
+ * reporting in connected state to cases where a matching BSS is determined
+ * to have better RSSI than the current connected BSS. The relative RSSI
+ * threshold values are ignored in disconnected state.
+ * @relative_rssi_5g_pref: The amount of RSSI preference in dB that is given to
+ * a 5 GHz BSS over 2.4 GHz BSS while looking for better BSSs in connected
+ * state. A negative value can be passed if 2.4 GHz band should be given
+ * priority to 5 GHz band.
+ * If the current connected BSS is in the 2.4 GHz band, other BSSs in the
+ * 2.4 GHz band to be reported should have better RSSI by @relative_rssi
+ * and other BSSs in the 5 GHz band to be reported should have better RSSI
+ * by (@relative_rssi - @relative_rssi_5g_pref).
+ * If the current connected BSS is in the 5 GHz band, other BSSs in the
+ * 2.4 GHz band to be reported should have better RSSI by
+ * (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
+ * band to be reported should have better RSSI by by @relative_rssi.
*/
struct cfg80211_sched_scan_request {
struct cfg80211_ssid *ssids;
@@ -1645,6 +1661,9 @@ struct cfg80211_sched_scan_request {
u8 mac_addr[ETH_ALEN] __aligned(2);
u8 mac_addr_mask[ETH_ALEN] __aligned(2);
+ s8 relative_rssi;
+ s8 relative_rssi_5g_pref;
+
/* internal */
struct wiphy *wiphy;
struct net_device *dev;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6b76e3b..fc29bdb 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1980,6 +1980,17 @@ enum nl80211_commands {
* @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
* used in various commands/events for specifying the BSSID.
*
+ * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI: Relative RSSI threshold by which
+ * other BSSs has to be better than the current connected BSS so that they
+ * get reported to user space. This will give an opportunity to userspace
+ * to consider connecting to other matching BSSs which have better RSSI
+ * than the current connected BSS by using an offloaded operation to avoid
+ * unnecessary wakeups.
+ *
+ * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF: The amount of RSSI preference
+ * to be given to 5 GHz APs over 2.4 GHz APs while searching for better
+ * BSSs than the current connected BSS.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2386,6 +2397,9 @@ enum nl80211_attrs {
NL80211_ATTR_BSSID,
+ NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+ NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -4697,6 +4711,9 @@ enum nl80211_feature_flags {
* configuration (AP/mesh) with VHT rates.
* @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
* with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
+ * @NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI: The driver supports sched_scan
+ * for reporting BSSs with better RSSI than the current connected BSS
+ * (%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI).
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4712,6 +4729,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_BEACON_RATE_HT,
NL80211_EXT_FEATURE_BEACON_RATE_VHT,
NL80211_EXT_FEATURE_FILS_STA,
+ NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7762231..549f239 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -405,6 +405,8 @@ enum nl80211_multicast_groups {
[NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
[NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_S8 },
+ [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF] = { .type = NLA_S8 },
};
/* policy for the key attributes */
@@ -6950,6 +6952,12 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
return ERR_PTR(-EINVAL);
+ if (!wiphy_ext_feature_isset(
+ wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
+ (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]))
+ return ERR_PTR(-EINVAL);
+
request = kzalloc(sizeof(*request)
+ sizeof(*request->ssids) * n_ssids
+ sizeof(*request->match_sets) * n_match_sets
@@ -7156,6 +7164,14 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
request->delay =
nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
+ if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI])
+ request->relative_rssi = nla_get_s8(
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
+
+ if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF])
+ request->relative_rssi_5g_pref = nla_get_s8(
+ attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]);
+
err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
if (err)
goto out_free;
@@ -9649,7 +9665,8 @@ static int nl80211_send_wowlan_tcp(struct sk_buff *msg,
return 0;
}
-static int nl80211_send_wowlan_nd(struct sk_buff *msg,
+static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
+ struct sk_buff *msg,
struct cfg80211_sched_scan_request *req)
{
struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
@@ -9670,6 +9687,14 @@ static int nl80211_send_wowlan_nd(struct sk_buff *msg,
if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
return -ENOBUFS;
+ if (wiphy_ext_feature_isset(
+ wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
+ (nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
+ req->relative_rssi) ||
+ nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
+ req->relative_rssi_5g_pref)))
+ return -ENOBUFS;
+
freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
if (!freqs)
return -ENOBUFS;
@@ -9783,7 +9808,7 @@ static int nl80211_get_wowlan(struct sk_buff *skb, struct genl_info *info)
goto nla_put_failure;
if (nl80211_send_wowlan_nd(
- msg,
+ &rdev->wiphy, msg,
rdev->wiphy.wowlan_config->nd_config))
goto nla_put_failure;
--
1.9.1
On 7-12-2016 10:33, Vamsi, Krishna wrote:
>> -----Original Message-----
>> From: Johannes Berg [mailto:[email protected]]
>
>> What about Arend's comment regarding this functionality overlapping with the
>> BSS selection offload configuration?
>>
>> Do you think there's any ability to share attributes/functionality?
>
> Hi Johannes,
>
> I think the later comment from Luca on this which I pasted below is more agreeable.
>
> Yes, similar but not quite the same. The existing case is for finding BSSs that are worth waking the host up for (while disconnected), so it needs to be better than the RSSI passed (absolute number). Now this is about relative RSSI as compared to the current connection, so RELATIVE_RSSI is different than RSSI and I think the same attribute should not be used, to avoid confusion.
I noticed the response from Luca, but did not get back on this. I know
it is not the same, but what I meant is whether we could extend it so it
also covers your scenario.
Regards,
Arend
On 20-12-2016 21:52, Malinen, Jouni wrote:
> On Fri, Dec 16, 2016 at 10:56:51AM +0100, Johannes Berg wrote:
>> On Thu, 2016-12-15 at 11:06 +0000, Malinen, Jouni wrote:
>>> Maybe the nl80211.h description was not clear enough, but the
>>> comments in cfg80211.h should be quite clear on how this was designed
>>> to work at the implementation level:
>>>
>>> "If the current connected BSS is in the 2.4 GHz band, other BSSs in
>>> the 2.4 GHz band to be reported should have better RSSI by
>>> @relative_rssi and other BSSs in the 5 GHz band to be reported should
>>> have better RSSI by (@relative_rssi - @relative_rssi_5g_pref).
>>> If the current connected BSS is in the 5 GHz band, other BSSs in the
>>> 2.4 GHz band to be reported should have better RSSI by
>>> (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
>>> band to be reported should have better RSSI by by @relative_rssi."
>>
>> Oh, right. Should probably be in nl80211.h too, to set expectations
>> from userspace.
>
> Sure, we can update that in the next revision.
>
>>> At minimum, we would need to clearly document struct
>>> nl80211_bss_select_rssi_adjust, but even if we do, I'm not sure it
>>> really is ideal mechanism to move to now that I realized it is not an
>>> array, but a single band,delta pair.
>>
>> We can move to an array easily in the future by extending the attribute
>> length and advertising the number of array entries that are supported,
>> if that's your biggest concern? I don't see it as being very useful
>> right now since I don't think we'll see offloaded roaming between 2.4/5
>> and 60 GHz anytime soon. This may change when we add more bands later,
>> I suppose.
>
> Hmm.. So do you want us to move to using this packed struct in the new
> attribute instead of using a signed 8-bit integer as the variable value?
That was my suggestion so it is more clear what user-space wants by
making it specify the band explicitly. So in the explanation above
reference to "5g" should be "specified band" etc. Whether you reuse the
packed struct nl80211_bss_select_rssi_adjust or come up with a new
(identical?) one is irrelevant to me.
Also I don't see the array issue. @relative_rssi_5g_pref with s8 value
seems same as @rssi_adjust with (band=5g, s8 value) packed together. Or
am I missing something here.
>>> If we are talking only about roaming within an ESS (a single SSID),
>>> that would sound clear, but which relative RSSI rules would apply if
>>> there are match sets for both the currently connected SSID and
>>> another SSID that the candidate BSS is for?
>>
>> Right, I see how this might become a problem. I generally see no issue
>> with supporting multiple matchsets with different SSIDs but all having
>> the "relative to connected BSS RSSI filter" (which would disregard the
>> SSID specified in the matchset), but this then would become a problem
>> when multiple matchsets are specified with *different* such RSSI
>> filters, e.g. one matchset would specify that you want a 5G preference
>> of 10dB, but the other would specify a preference of only 5dB.
>>
>> Conceptually in this approach, that would be supported, but firmware
>> likely would not be able to express this, I suppose?
>
> That's certainly not at the level we were planning on implementing.. :)
Right. So having "relative rssi" matchset attribute is off the table as
far as I am concerned.
>>> I think I'm missing something here.. Where would the threshold value
>>> (how much better new BSS needs to be) be stored? And do we really
>>> want something like the combination of
>>> NL80211_BSS_SELECT_ATTR_BAND_PREF and
>>> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST which seems to be two different
>>> ways of doing band preference (the former without specifying delta
>>> and the latter with specific delta)? Or am I still not understanding
>>> how NL80211_BSS_SELECT_ATTR_* really works?
It is documented here:
/**
* enum nl80211_bss_select_attr - attributes for bss selection.
*
[...]
*
* One and only one of these attributes are found within
%NL80211_ATTR_BSS_SELECT
* for %NL80211_CMD_CONNECT. It specifies the required BSS selection
behaviour
* which the driver shall use.
*/
It is checked in nl80211.c [1]
>> No, you're right, I missed the "better by" threshold.
>>
>> I think between that (unless we add that, we could technically extend
>> flag attributes to allow them being an int as well, or add a new one)
>> and the fact that the device may not support different relative RSSI
>> matches in different matchsets, I'm almost convinced that adding new
>> attributes is better.
>
> I'm not completely sure how to interpret all this and also the last
> email from Arend in this thread. Could either (or both :) of you provide
> more detailed suggestion on what exactly you would like us to change, if
> anything, in the attribute design now so that we can try to close on
> this?
To summarize: 1) stick with the new attributes on request level (so not
matchset level), 2) use packed struct for @relative_rssi_5g_pref.
Regards,
Arend
[1] http://lxr.free-electrons.com/source/net/wireless/nl80211.c#L6382
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBKb2hhbm5lcyBCZXJnIFttYWls
dG86am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldF0NCg0KPiBXaGF0IGFib3V0IEFyZW5kJ3MgY29t
bWVudCByZWdhcmRpbmcgdGhpcyBmdW5jdGlvbmFsaXR5IG92ZXJsYXBwaW5nIHdpdGggdGhlDQo+
IEJTUyBzZWxlY3Rpb24gb2ZmbG9hZCBjb25maWd1cmF0aW9uPw0KPiANCj4gRG8geW91IHRoaW5r
IHRoZXJlJ3MgYW55IGFiaWxpdHkgdG8gc2hhcmUgYXR0cmlidXRlcy9mdW5jdGlvbmFsaXR5Pw0K
DQpIaSBKb2hhbm5lcywNCg0KSSB0aGluayB0aGUgbGF0ZXIgY29tbWVudCBmcm9tIEx1Y2Egb24g
dGhpcyB3aGljaCBJIHBhc3RlZCBiZWxvdyBpcyBtb3JlIGFncmVlYWJsZS4NCg0KWWVzLCBzaW1p
bGFyIGJ1dCBub3QgcXVpdGUgdGhlIHNhbWUuICBUaGUgZXhpc3RpbmcgY2FzZSBpcyBmb3IgZmlu
ZGluZyBCU1NzIHRoYXQgYXJlIHdvcnRoIHdha2luZyB0aGUgaG9zdCB1cCBmb3IgKHdoaWxlIGRp
c2Nvbm5lY3RlZCksIHNvIGl0IG5lZWRzIHRvIGJlIGJldHRlciB0aGFuIHRoZSBSU1NJIHBhc3Nl
ZCAoYWJzb2x1dGUgbnVtYmVyKS4gIE5vdyB0aGlzIGlzIGFib3V0IHJlbGF0aXZlIFJTU0kgYXMg
Y29tcGFyZWQgdG8gdGhlIGN1cnJlbnQgY29ubmVjdGlvbiwgc28gUkVMQVRJVkVfUlNTSSBpcyBk
aWZmZXJlbnQgdGhhbiBSU1NJIGFuZCBJIHRoaW5rIHRoZSBzYW1lIGF0dHJpYnV0ZSBzaG91bGQg
bm90IGJlIHVzZWQsIHRvIGF2b2lkIGNvbmZ1c2lvbi4NCg0KVGhhbmtzLA0KVmFtc2kNCg==
> v2: address comments from Luca, Arend, and Johannes
What about Arend's comment regarding this functionality overlapping
with the BSS selection offload configuration?
Do you think there's any ability to share attributes/functionality?
johannes
On 2-12-2016 22:59, Jouni Malinen wrote:
> From: vamsi krishna <[email protected]>
>
> Enhance sched scan to support option of finding a better BSS while in
> connected state. Firmware scans the medium and reports when it finds a
> known BSS which has better RSSI than the current connected BSS. New
> attributes to specify the relative RSSI (compared to the current BSS)
> are added to the sched scan to implement this.
>
> Signed-off-by: vamsi krishna <[email protected]>
> Signed-off-by: Jouni Malinen <[email protected]>
> ---
> include/net/cfg80211.h | 19 +++++++++++++++++++
> include/uapi/linux/nl80211.h | 18 ++++++++++++++++++
> net/wireless/nl80211.c | 29 +++++++++++++++++++++++++++--
> 3 files changed, 64 insertions(+), 2 deletions(-)
>
> v2: address comments from Luca, Arend, and Johannes
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ef42749..dcdd0c4 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1626,6 +1626,22 @@ struct cfg80211_sched_scan_plan {
> * cycle. The driver may ignore this parameter and start
> * immediately (or at any other time), if this feature is not
> * supported.
> + * @relative_rssi: Relative RSSI threshold in dB to restrict scan result
> + * reporting in connected state to cases where a matching BSS is determined
> + * to have better RSSI than the current connected BSS. The relative RSSI
> + * threshold values are ignored in disconnected state.
> + * @relative_rssi_5g_pref: The amount of RSSI preference in dB that is given to
> + * a 5 GHz BSS over 2.4 GHz BSS while looking for better BSSs in connected
> + * state. A negative value can be passed if 2.4 GHz band should be given
> + * priority to 5 GHz band.
> + * If the current connected BSS is in the 2.4 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by @relative_rssi
> + * and other BSSs in the 5 GHz band to be reported should have better RSSI
> + * by (@relative_rssi - @relative_rssi_5g_pref).
> + * If the current connected BSS is in the 5 GHz band, other BSSs in the
> + * 2.4 GHz band to be reported should have better RSSI by
> + * (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> + * band to be reported should have better RSSI by by @relative_rssi.
The choice of these attributes makes the implicit assumption that the
BSS-es in 5G are preferred. The relative_rssi_5g_pref is actually more a
bonus or penalty is negative value is used. I guess for speed junkies
that want their 11ac card maxed out that is true, but if you need to
cross a couple of concrete floors you might want to stick with 2.4G.
I introduced a similar attribute to be provided in the
NL80211_CMD_CONNECT (see [1]).
Regards,
Arend
[1] http://lxr.free-electrons.com/source/include/uapi/linux/nl80211.h#L1815
> */
> struct cfg80211_sched_scan_request {
> struct cfg80211_ssid *ssids;
> @@ -1645,6 +1661,9 @@ struct cfg80211_sched_scan_request {
> u8 mac_addr[ETH_ALEN] __aligned(2);
> u8 mac_addr_mask[ETH_ALEN] __aligned(2);
>
> + s8 relative_rssi;
> + s8 relative_rssi_5g_pref;
> +
> /* internal */
> struct wiphy *wiphy;
> struct net_device *dev;
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 6b76e3b..fc29bdb 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -1980,6 +1980,17 @@ enum nl80211_commands {
> * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that %NL80211_ATTR_MAC is also
> * used in various commands/events for specifying the BSSID.
> *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI: Relative RSSI threshold by which
> + * other BSSs has to be better than the current connected BSS so that they
> + * get reported to user space. This will give an opportunity to userspace
> + * to consider connecting to other matching BSSs which have better RSSI
> + * than the current connected BSS by using an offloaded operation to avoid
> + * unnecessary wakeups.
> + *
> + * @NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF: The amount of RSSI preference
> + * to be given to 5 GHz APs over 2.4 GHz APs while searching for better
> + * BSSs than the current connected BSS.
> + *
> * @NUM_NL80211_ATTR: total number of nl80211_attrs available
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2386,6 +2397,9 @@ enum nl80211_attrs {
>
> NL80211_ATTR_BSSID,
>
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
> @@ -4697,6 +4711,9 @@ enum nl80211_feature_flags {
> * configuration (AP/mesh) with VHT rates.
> * @NL80211_EXT_FEATURE_FILS_STA: This driver supports Fast Initial Link Setup
> * with user space SME (NL80211_CMD_AUTHENTICATE) in station mode.
> + * @NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI: The driver supports sched_scan
> + * for reporting BSSs with better RSSI than the current connected BSS
> + * (%NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI).
> *
> * @NUM_NL80211_EXT_FEATURES: number of extended features.
> * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
> @@ -4712,6 +4729,7 @@ enum nl80211_ext_feature_index {
> NL80211_EXT_FEATURE_BEACON_RATE_HT,
> NL80211_EXT_FEATURE_BEACON_RATE_VHT,
> NL80211_EXT_FEATURE_FILS_STA,
> + NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI,
>
> /* add new features before the definition below */
> NUM_NL80211_EXT_FEATURES,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 7762231..549f239 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -405,6 +405,8 @@ enum nl80211_multicast_groups {
> [NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
> [NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
> [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_S8 },
> + [NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF] = { .type = NLA_S8 },
> };
>
> /* policy for the key attributes */
> @@ -6950,6 +6952,12 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> if (!n_plans || n_plans > wiphy->max_sched_scan_plans)
> return ERR_PTR(-EINVAL);
>
> + if (!wiphy_ext_feature_isset(
> + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
> + (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] ||
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]))
> + return ERR_PTR(-EINVAL);
> +
> request = kzalloc(sizeof(*request)
> + sizeof(*request->ssids) * n_ssids
> + sizeof(*request->match_sets) * n_match_sets
> @@ -7156,6 +7164,14 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
> request->delay =
> nla_get_u32(attrs[NL80211_ATTR_SCHED_SCAN_DELAY]);
>
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI])
> + request->relative_rssi = nla_get_s8(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI]);
> +
> + if (attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF])
> + request->relative_rssi_5g_pref = nla_get_s8(
> + attrs[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF]);
> +
> err = nl80211_parse_sched_scan_plans(wiphy, n_plans, request, attrs);
> if (err)
> goto out_free;
> @@ -9649,7 +9665,8 @@ static int nl80211_send_wowlan_tcp(struct sk_buff *msg,
> return 0;
> }
>
> -static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> +static int nl80211_send_wowlan_nd(struct wiphy *wiphy,
> + struct sk_buff *msg,
> struct cfg80211_sched_scan_request *req)
> {
> struct nlattr *nd, *freqs, *matches, *match, *scan_plans, *scan_plan;
> @@ -9670,6 +9687,14 @@ static int nl80211_send_wowlan_nd(struct sk_buff *msg,
> if (nla_put_u32(msg, NL80211_ATTR_SCHED_SCAN_DELAY, req->delay))
> return -ENOBUFS;
>
> + if (wiphy_ext_feature_isset(
> + wiphy, NL80211_EXT_FEATURE_SCHED_SCAN_RELATIVE_RSSI) &&
> + (nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI,
> + req->relative_rssi) ||
> + nla_put_s8(msg, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF,
> + req->relative_rssi_5g_pref)))
> + return -ENOBUFS;
> +
> freqs = nla_nest_start(msg, NL80211_ATTR_SCAN_FREQUENCIES);
> if (!freqs)
> return -ENOBUFS;
> @@ -9783,7 +9808,7 @@ static int nl80211_get_wowlan(struct sk_buff *skb, struct genl_info *info)
> goto nla_put_failure;
>
> if (nl80211_send_wowlan_nd(
> - msg,
> + &rdev->wiphy, msg,
> rdev->wiphy.wowlan_config->nd_config))
> goto nla_put_failure;
>
>
On Wed, Dec 07, 2016 at 09:03:23PM +0100, Arend Van Spriel wrote:
> On 7-12-2016 10:33, Vamsi, Krishna wrote:
> >> -----Original Message-----
> >> From: Johannes Berg [mailto:[email protected]]
> >=20
> >> What about Arend's comment regarding this functionality overlapping wi=
th the
> >> BSS selection offload configuration?
> >>
> >> Do you think there's any ability to share attributes/functionality?
> >=20
> > Hi Johannes,
> >=20
> > I think the later comment from Luca on this which I pasted below is mor=
e agreeable.
> >=20
> > Yes, similar but not quite the same. The existing case is for finding =
BSSs that are worth waking the host up for (while disconnected), so it need=
s to be better than the RSSI passed (absolute number). Now this is about r=
elative RSSI as compared to the current connection, so RELATIVE_RSSI is dif=
ferent than RSSI and I think the same attribute should not be used, to avoi=
d confusion.
>=20
> I noticed the response from Luca, but did not get back on this. I know
> it is not the same, but what I meant is whether we could extend it so it
> also covers your scenario.
I'm not sure I see the point of trying to avoid the new RELATIVE_RSSI
attribute. We need to clearly specify that this new constraint is indeed
for relative comparison against the currently connected BSS.
As far as your second email is concerned, it might make more sense to
use the existing NL80211_ATTR_BSS_SELECT instead of the new
NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since they cover very
similar purpose in defining RSSI preference between bands. We can take a
look at doing so. One thing to be a careful about this is in what claims
there are about using NL80211_ATTR_BSS_SELECT for capability indication
in GET_WIPHY. I guess we can leave that as-is to apply for _CONNECT and
the new EXT_FEATURE flag we are adding for sched_scan applies for this
attribute in SCHED_SCAN.
--=20
Jouni Malinen PGP id EFC895FA=
On Fri, Dec 16, 2016 at 10:56:51AM +0100, Johannes Berg wrote:
> On Thu, 2016-12-15 at 11:06 +0000, Malinen, Jouni wrote:
> > Maybe the nl80211.h description was not clear enough, but the
> > comments in cfg80211.h should be quite clear on how this was designed
> > to work at the implementation level:
> >=20
> > "If the current connected BSS is in the 2.4 GHz band, other BSSs in
> > the 2.4 GHz band to be reported should have better RSSI by
> > @relative_rssi and other BSSs in the 5 GHz band to be reported should
> > have better RSSI by (@relative_rssi - @relative_rssi_5g_pref).
> > If the current connected BSS is in the 5 GHz band, other BSSs in the
> > 2.4 GHz band to be reported should have better RSSI by
> > (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> > band to be reported should have better RSSI by by @relative_rssi."
>=20
> Oh, right. Should probably be in nl80211.h too, to set expectations
> from userspace.
Sure, we can update that in the next revision.
> > At minimum, we would need to clearly document struct
> > nl80211_bss_select_rssi_adjust, but even if we do, I'm not sure it
> > really is ideal mechanism to move to now that I realized it is not an
> > array, but a single band,delta pair.
>=20
> We can move to an array easily in the future by extending the attribute
> length and advertising the number of array entries that are supported,
> if that's your biggest concern? I don't see it as being very useful
> right now since I don't think we'll see offloaded roaming between 2.4/5
> and 60 GHz anytime soon. This may change when we add more bands later,
> I suppose.
Hmm.. So do you want us to move to using this packed struct in the new
attribute instead of using a signed 8-bit integer as the variable value?
> > If we are talking only about roaming within an ESS (a single SSID),
> > that would sound clear, but which relative RSSI rules would apply if
> > there are match sets for both the currently connected SSID and
> > another SSID that the candidate BSS is for?
>=20
> Right, I see how this might become a problem. I generally see no issue
> with supporting multiple matchsets with different SSIDs but all having
> the "relative to connected BSS RSSI filter" (which would disregard the
> SSID specified in the matchset), but this then would become a problem
> when multiple matchsets are specified with *different* such RSSI
> filters, e.g. one matchset would specify that you want a 5G preference
> of 10dB, but the other would specify a preference of only 5dB.
>=20
> Conceptually in this approach, that would be supported, but firmware
> likely would not be able to express this, I suppose?
That's certainly not at the level we were planning on implementing.. :)
> > I think I'm missing something here.. Where would the threshold value
> > (how much better new BSS needs to be) be stored? And do we really
> > want something like the combination of
> > NL80211_BSS_SELECT_ATTR_BAND_PREF and
> > NL80211_BSS_SELECT_ATTR_RSSI_ADJUST which seems to be two different
> > ways of doing band preference (the former without specifying delta
> > and the latter with specific delta)? Or am I still not understanding
> > how NL80211_BSS_SELECT_ATTR_* really works?
>=20
> No, you're right, I missed the "better by" threshold.
>=20
> I think between that (unless we add that, we could technically extend
> flag attributes to allow them being an int as well, or add a new one)
> and the fact that the device may not support different relative RSSI
> matches in different matchsets, I'm almost convinced that adding new
> attributes is better.
I'm not completely sure how to interpret all this and also the last
email from Arend in this thread. Could either (or both :) of you provide
more detailed suggestion on what exactly you would like us to change, if
anything, in the attribute design now so that we can try to close on
this?
--=20
Jouni Malinen PGP id EFC895FA=
On Thu, 2016-12-15 at 11:06 +0000, Malinen, Jouni wrote:
> On Tue, Dec 13, 2016 at 04:56:55PM +0100, Johannes Berg wrote:
> > Regarding reusing attributes, we have (for the BSS selection thing)
> > the attribute NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, which is really
> > quite similar to your
> > new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF since while
> > connected (which BSS_SELECT_ATTR_* assumes) the current BSS is
> > always part of the considered BSSes, I'd think.
>
> It seems to have somewhat similar meaning, but it looks more generic
> by not being tied to just two bands (2.4 and 5). And now that I
> actually looked again at this, it does not look like
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST actually allows more than a
> single band,delta pair to be provided and as such, it actually would
> not work very well with more than two bands even if it might be a bit
> more generic by allowing band to be set to something else than 2.4 or
> 5.
Agree, it wouldn't work well with more than 2 bands. Not that we have
them now (60GHz doesn't really count here).
> By the way, nl80211.h does not seem to document what values struct
> nl82011_bss_select_rssi_adjust band uses.. Is this enum
> nl80211_band?
I believe so, we should fix that.
> Neither does it say that delta is in dB (is it?).
We should also fix that, but let's assume so for the sake of this
discussion.
> > OTOH, the new NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF doesn't
> > make this quite clear - is the current BSS to be adjusted before
> > comparing, if it's 5 GHz? If so, the semantics are equivalent. If
> > not, it doesn't actually make much sense ;-)
>
> Maybe the nl80211.h description was not clear enough, but the
> comments in cfg80211.h should be quite clear on how this was designed
> to work at the implementation level:
>
> "If the current connected BSS is in the 2.4 GHz band, other BSSs in
> the 2.4 GHz band to be reported should have better RSSI by
> @relative_rssi and other BSSs in the 5 GHz band to be reported should
> have better RSSI by (@relative_rssi - @relative_rssi_5g_pref).
> If the current connected BSS is in the 5 GHz band, other BSSs in the
> 2.4 GHz band to be reported should have better RSSI by
> (@relative_rssi + @relative_rssi_5g_pref) and other BSSs in the 5 GHz
> band to be reported should have better RSSI by by @relative_rssi."
Oh, right. Should probably be in nl80211.h too, to set expectations
from userspace.
> I'm not sure I'd describe this as adjusting the current BSS RSSI, but
> more like adjusting the RSSI threshold value if roaming would be from
> one band to another and doing that adjustment by adding or
> decrementing based on whether the roam would be from 2.4 to 5 or from
> 5 to 2.4.
It's functionally equivalent to doing the following adjustment
compare_rssi = current_bss_rssi
if current_bss_is_5ghz:
compare_rssi += relative_rssi_5g_pref
and then comparing all values (again adjusted for 5 GHz BSSes) to this.
(which is what I meant by "adjusting the current BSS RSSI").
> > So assuming that it is in fact taken into account after the same
> > adjustment, the two attributes are equivalent, and then perhaps it
> > would make sense to use struct nl80211_bss_select_rssi_adjust for
> > the new attribute. If a driver doesn't support arbitrary bands, but
> > just 5 GHz as in your example, it can just flip it around to 2.4
> > GHz by switching the sign.
> >
> > Perhaps we should even consider doing that in cfg80211 and
> > adjusting the internal API for both that way?
>
> I'm not completely sure I understood. One thing to note about
> differences here is that NL80211_BSS_SELECT_ATTR_* seems to be
> defining some preferences for BSS selection based on RSSI with an
> additional band preference, but it does not seem to define the
> threshold by how many dB the new candidate BSS should be better.
It's defining more than what we're talking about here, absolutely.
Clearly somebody thought a pure band preference might be useful (to
never connect to a 2.4GHz AP if a 5 GHz AP is available, regardless of
their relative RSSI), but that's not what we're looking at here.
(That could probably also be achieved by setting the adjustment to
90dB, but whatever)
> At minimum, we would need to clearly document struct
> nl80211_bss_select_rssi_adjust, but even if we do, I'm not sure it
> really is ideal mechanism to move to now that I realized it is not an
> array, but a single band,delta pair.
We can move to an array easily in the future by extending the attribute
length and advertising the number of array entries that are supported,
if that's your biggest concern? I don't see it as being very useful
right now since I don't think we'll see offloaded roaming between 2.4/5
and 60 GHz anytime soon. This may change when we add more bands later,
I suppose.
> Arend:
> > > I am not saying it should be avoided. Just looking at it
> > > conceptually
> > > the scheduled scan request holds so-called matchsets that specify
> > > the
> > > constraints to determine whether a BSS was found that is worth
> > > notifying the host/user-space about. As such I would expect the
> > > relative RSSI attribute(s) to be part of the matchset. That way
> > > you
> > > can specify it together with the currently connected SSID in a
> > > single
> > > matchset.
> >
> > I think this makes a lot of sense.
>
> If we are talking only about roaming within an ESS (a single SSID),
> that would sound clear, but which relative RSSI rules would apply if
> there are match sets for both the currently connected SSID and
> another SSID that the candidate BSS is for?
Right, I see how this might become a problem. I generally see no issue
with supporting multiple matchsets with different SSIDs but all having
the "relative to connected BSS RSSI filter" (which would disregard the
SSID specified in the matchset), but this then would become a problem
when multiple matchsets are specified with *different* such RSSI
filters, e.g. one matchset would specify that you want a 5G preference
of 10dB, but the other would specify a preference of only 5dB.
Conceptually in this approach, that would be supported, but firmware
likely would not be able to express this, I suppose?
> > We already have NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, which asks to
> > be reporting only networks that have an *absolute* RSSI value above
> > the value of the attribute - a new attribute to make it relative to
> > the current network instead would make sense.
>
> When you say "current network", do you mean the current BSS? This
> gets complex when thinking about multiple SSIDs (which some call
> "networks") and there being NL80211_SCHED_SCAN_MATCH_ATTR_SSID and
> multiple match sets..
Yes, I did mean "current BSS". And yes - it does seem complex, see
above.
> > That would indeed be equivalent to NL80211_BSS_SELECT_ATTR_RSSI
> > then.
>
> NL80211_BSS_SELECT_ATTR_RSSI is a flag attribute.. BSS select
> mechanism does not provide an absolute RSSI value or threshold; it
> just seems to indicate use of RSSI-based selection mechanism without
> defining what exactly is better. There is
> NL80211_BSS_SELECT_ATTR_BAND_PREF that gives a preference to a
> specific band (without defining what that preference is) and then the
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST that can actually give a specific
> RSSI adjustment value (in dB?).
>
> > Now, if we consider this, NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI
> > actually is equivalent to NL80211_BSS_SELECT_ATTR_RSSI (a flag
> > attribute indicating whether or not RSSI-based selection/matching
> > is done) and NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI_5G_PREF is
> > equivalent to NL80211_BSS_SELECT_ATTR_RSSI_ADJUST, both need to be
> > given with the flag and affect operation.
>
> Hmm.. So you did notice it is a flag attribute.. So how would this
> match NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI which provides the
> threshold value for how many dB better the new BSS needs to be for it
> to be reported?
Yeah, umm, I think I got confused. There's no threshold in the
BSS_SELECT case, and maybe there doesn't need to be (at least not to
avoid host wakeups, though you'd want some hysteresis to avoid swapping
around too much).
> > So, how about we move these into NL80211_SCHED_SCAN_MATCH_ATTR_* as
> > suggested by Arend, and define them with the same content as the
> > corresponding NL80211_BSS_SELECT_ATTR_*?
>
> I think I'm missing something here.. Where would the threshold value
> (how much better new BSS needs to be) be stored? And do we really
> want something like the combination of
> NL80211_BSS_SELECT_ATTR_BAND_PREF and
> NL80211_BSS_SELECT_ATTR_RSSI_ADJUST which seems to be two different
> ways of doing band preference (the former without specifying delta
> and the latter with specific delta)? Or am I still not understanding
> how NL80211_BSS_SELECT_ATTR_* really works?
No, you're right, I missed the "better by" threshold.
I think between that (unless we add that, we could technically extend
flag attributes to allow them being an int as well, or add a new one)
and the fact that the device may not support different relative RSSI
matches in different matchsets, I'm almost convinced that adding new
attributes is better.
> The main concern I have for optional features with sched_scan is in
> whether the device ends up being woken up constantly if the driver
> does not understand a constraint that user space is trying to use to
> avoid being notified all the time.
Agree.
johannes
> Also I don't see the array issue. @relative_rssi_5g_pref with s8
> value seems same as @rssi_adjust with (band=5g, s8 value) packed
> together. Or am I missing something here.
Jouni is arguing that if you can specify which band to adjust, you
might want to adjust more than one band - and need an array of
(band,adjustment) rather than just a single such tuple.
But if we introduce this attribute with the tuple now, we can extend it
to an array of tuples later if we really need that.
johannes
On Tue, 2016-12-20 at 20:52 +0000, Malinen, Jouni wrote:
> > I think between that (unless we add that, we could technically
> > extend flag attributes to allow them being an int as well, or add a
> > new one) and the fact that the device may not support different
> > relative RSSI matches in different matchsets, I'm almost convinced
> > that adding new attributes is better.
>
> I'm not completely sure how to interpret all this and also the last
> email from Arend in this thread. Could either (or both :) of you
> providemore detailed suggestion on what exactly you would like us to
> change, if anything, in the attribute design now so that we can try
> to close on this?
I guess I'm thinking out loud too much :-)
Moving to the (band,adjustment) struct thing, like we have it in the
BSS select now, would be good I think - instead of the single value
that has the band hardcoded in the name. It's fairly useless with just
two bands since you can adjust one or the other to achieve the same
result, but for consistency it'd be good.
And then doing a match attribute we more or less discarded here, so
conceptually nothing else really changes.
johannes