2018-11-30 08:34:31

by Vamsi Krishna

[permalink] [raw]
Subject: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

Earlier, we have provision to specify a single minimum rssi threshold
min_rssi_thold to drivers which is band agnostic. Add support to configure
drivers with different minimum rssi thresholds for different bands used for
scan results filtering. User space tools shall use NL80211_BAND_* variables
defined in enum nl80211_band as attribute types and the rssi thresholds as
values to form each of the band specific attributes. Such attributes shall
be nested in NL80211_ATTR_SCHED_SCAN_MIN_RSSI attribute.

Also, use new attributes and variables to move away from the earlier design
which has the confusion created by match_set with
NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attribute alone to
be considered as minimum rssi threshold for all matchsets that doesn't have
NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attribute. With this commit, for the
matchsets which doesn't have NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attribute
alone to will use @band_specific_min_rssi_thold as minimum rssi threshold.

For the matchsets that user space configures with some rssi threshold, the
rssi threshold that is configured within matchset will have higher
precedence compared to the band specific rssi thresholds configured with
%NL80211_ATTR_SCHED_SCAN_MIN_RSSI attribute. In other words, if the rssi
value configured with a specific match set(match_set[x].rssi_thold) is not
equal to %NL80211_SCAN_RSSI_THOLD_OFF then match_set[x].rssi_thold value
shall be used scan results filtering that belong to the match set.
Otherwise, if match_set[x].rssi_thold is equals to
%NL80211_SCAN_RSSI_THOLD_OFF then the rssi threshold values configured with
@band_specific_min_rssi_thold shall be used for filtering scan results that
belong to such match set.

The drivers with capability to filter scan results with different rssi
thresholds for different bands shall indicate the support to user space by
setting %NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD in
wiphy->ext_features. The drivers that support this feature shall use rssi
values from @band_specific_min_rssi_thold for scan results filtering and
shall not use min_rssi_thold anymore.

This commit adds support to specify band specific minimum rssi thresholds
applicable for all scan results but not different band specific rssi
thresholds for individual matchsets. This support can be added later when
needed.

Signed-off-by: vamsi krishna <[email protected]>

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 1fa41b7..c032375 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1848,7 +1848,10 @@ struct cfg80211_bss_select_adjust {
* @scan_start: start time of the scheduled scan
* @channels: channels to scan
* @min_rssi_thold: for drivers only supporting a single threshold, this
- * contains the minimum over all matchsets
+ * contains the minimum over all matchsets. This parameter is obsolete for
+ * the drivers that support scan results filtering with band specific rssi
+ * thresholds feature associated with
+ * %NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD capability.
* @mac_addr: MAC address used with randomisation
* @mac_addr_mask: MAC address mask used with randomisation, bits that
* are 0 in the mask should be randomised, bits that are 1 should
@@ -1875,6 +1878,12 @@ struct cfg80211_bss_select_adjust {
* using @relative_rssi. If delta is a negative number, the BSSs that
* belong to the specified band will be penalized by delta dB in relative
* comparisions.
+ * @band_specific_min_rssi_thold: Minimum rssi threshold for each band to be
+ * applied to filter out scan results received. Drivers that support band
+ * specific rssi based filtering and sets the corresponding capability bit
+ * %NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD will use these
+ * rssi thresholds for filtering scan results and will ignore rssi value
+ * provided in @min_rssi_thold.
*/
struct cfg80211_sched_scan_request {
u64 reqid;
@@ -1908,6 +1917,7 @@ struct cfg80211_sched_scan_request {
u32 owner_nlportid;
bool nl_owner_dead;
struct list_head list;
+ s32 band_specific_min_rssi_thold[NUM_NL80211_BANDS];

/* keep last */
struct ieee80211_channel *channels[0];
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6d610ba..0d4dd14 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2254,6 +2254,22 @@ enum nl80211_commands {
* @NL80211_ATTR_FTM_RESPONDER_STATS: Nested attribute with FTM responder
* statistics, see &enum nl80211_ftm_responder_stats.
*
+ * @NL80211_ATTR_SCHED_SCAN_MIN_RSSI: Nested attribute that carries the band
+ * specific minimum rssi thresholds for the bands defined in enum
+ * nl80211_band. The minimum rssi threshold value(s32) specific to a band
+ * shall be encapsulated in attribute with type value equals to one of the
+ * NL80211_BAND_* defined in enum nl80211_band. For example, the minimum
+ * rssi threshold value for 2.4GHz band shall be encapsulated within an
+ * attribute of type NL80211_BAND_2GHz. And one or more of such attributes
+ * will be nested within this attribute. If min rssi sub attribute is not
+ * included for any band, then %NL80211_SCAN_RSSI_THOLD_OFF will be used
+ * as minimum rssi threshold for scan result filtering.
+ * If a scan result received in a particular band has lesser rssi than the
+ * rssi threshold specified for that band, then the scan result shall be
+ * filtered out. The rssi thresholds specified in matchsets along with ssid
+ * or bssid attribute will have higher precedence than the thresholds
+ * mentioned in this attribute while checking rssi.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2699,6 +2715,8 @@ enum nl80211_attrs {

NL80211_ATTR_FTM_RESPONDER_STATS,

+ NL80211_ATTR_SCHED_SCAN_MIN_RSSI,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -5257,6 +5275,9 @@ enum nl80211_feature_flags {
* able to rekey an in-use key correctly. Userspace must not rekey PTK keys
* if this flag is not set. Ignoring this can leak clear text packets and/or
* freeze the connection.
+ * @NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD: The driver supports
+ * filtering scan results with band specific rssi thresholds that are
+ * specified within %NL80211_ATTR_SCHED_SCAN_MIN_RSSI.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5297,6 +5318,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_SCAN_MIN_PREQ_CONTENT,
NL80211_EXT_FEATURE_CAN_REPLACE_PTK0,
NL80211_EXT_FEATURE_ENABLE_FTM_RESPONDER,
+ NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD,

/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 744b585..4fc0bfc 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -497,6 +497,7 @@ static int validate_ie_attr(const struct nlattr *attr,
.type = NLA_NESTED,
.validation_data = nl80211_ftm_responder_policy,
},
+ [NL80211_ATTR_SCHED_SCAN_MIN_RSSI] = { .type = NLA_NESTED },
};

/* policy for the key attributes */
@@ -7344,6 +7345,37 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
return 0;
}

+static int
+nl80211_parse_sched_scan_min_rssi(struct wiphy *wiphy,
+ struct cfg80211_sched_scan_request *request,
+ struct nlattr **attrs)
+{
+ struct nlattr *attr;
+ int i, tmp;
+
+ if (!attrs[NL80211_ATTR_SCHED_SCAN_MIN_RSSI]) {
+ for (i = 0; i < NUM_NL80211_BANDS; i++)
+ request->band_specific_min_rssi_thold[i] =
+ request->min_rssi_thold;
+ return 0;
+ }
+
+ for (i = 0; i < NUM_NL80211_BANDS; i++)
+ request->band_specific_min_rssi_thold[i] =
+ NL80211_SCAN_RSSI_THOLD_OFF;
+
+ nla_for_each_nested(attr, attrs[NL80211_ATTR_SCHED_SCAN_MIN_RSSI],
+ tmp) {
+ enum nl80211_band band = nla_type(attr);
+
+ if (band < 0 || band >= NUM_NL80211_BANDS)
+ return -EINVAL;
+
+ request->band_specific_min_rssi_thold[band] = nla_get_s32(attr);
+ }
+ return 0;
+}
+
static struct cfg80211_sched_scan_request *
nl80211_parse_sched_scan(struct wiphy *wiphy, struct wireless_dev *wdev,
struct nlattr **attrs, int max_match_sets)
@@ -7460,6 +7492,11 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
attrs[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST]))
return ERR_PTR(-EINVAL);

+ if (!wiphy_ext_feature_isset(wiphy,
+ NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD) &&
+ attrs[NL80211_ATTR_SCHED_SCAN_MIN_RSSI])
+ return ERR_PTR(-EINVAL);
+
request = kzalloc(sizeof(*request)
+ sizeof(*request->ssids) * n_ssids
+ sizeof(*request->match_sets) * n_match_sets
@@ -7636,6 +7673,10 @@ static int nl80211_abort_scan(struct sk_buff *skb, struct genl_info *info)
request->min_rssi_thold = NL80211_SCAN_RSSI_THOLD_OFF;
}

+ err = nl80211_parse_sched_scan_min_rssi(wiphy, request, attrs);
+ if (err)
+ goto out_free;
+
if (ie_len) {
request->ie_len = ie_len;
memcpy((void *)request->ie,
--
1.9.1



2018-12-18 13:09:40

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

On Fri, 2018-11-30 at 14:04 +0530, vamsi krishna wrote:
>
> The drivers with capability to filter scan results with different rssi
> thresholds for different bands shall indicate the support to user space by
> setting %NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD in
> wiphy->ext_features. The drivers that support this feature shall use rssi
> values from @band_specific_min_rssi_thold for scan results filtering and
> shall not use min_rssi_thold anymore.

Is there much point in this?

It seems to me that userspace will not really do anything different if
it knows what's supported - it's probably easier for it (or cfg80211?)
to just set the global min_rssi to the minimum over all bands, and treat
this as we do most things in scheduled scan - an optimisation that
doesn't really need a feature advertisement?

I think that would simplify things somewhat.

> + * @NL80211_ATTR_SCHED_SCAN_MIN_RSSI: Nested attribute that carries the band
> + * specific minimum rssi thresholds for the bands defined in enum
> + * nl80211_band. The minimum rssi threshold value(s32) specific to a band
> + * shall be encapsulated in attribute with type value equals to one of the
> + * NL80211_BAND_* defined in enum nl80211_band. For example, the minimum
> + * rssi threshold value for 2.4GHz band shall be encapsulated within an
> + * attribute of type NL80211_BAND_2GHz. And one or more of such attributes
> + * will be nested within this attribute. If min rssi sub attribute is not
> + * included for any band, then %NL80211_SCAN_RSSI_THOLD_OFF will be used
> + * as minimum rssi threshold for scan result filtering.
> + * If a scan result received in a particular band has lesser rssi than the
> + * rssi threshold specified for that band, then the scan result shall be
> + * filtered out. The rssi thresholds specified in matchsets along with ssid
> + * or bssid attribute will have higher precedence than the thresholds
> + * mentioned in this attribute while checking rssi.

It seems that this should be with the existing
NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, not in this level namespace.

johannes


2018-12-26 09:54:26

by Vamsi Krishna

[permalink] [raw]
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

On 2018-12-18 18:39, Johannes Berg wrote:
> On Fri, 2018-11-30 at 14:04 +0530, vamsi krishna wrote:
>>
>> The drivers with capability to filter scan results with different rssi
>> thresholds for different bands shall indicate the support to user
>> space by
>> setting %NL80211_EXT_FEATURE_SCHED_SCAN_BAND_SPECIFIC_RSSI_THOLD in
>> wiphy->ext_features. The drivers that support this feature shall use
>> rssi
>> values from @band_specific_min_rssi_thold for scan results filtering
>> and
>> shall not use min_rssi_thold anymore.
>
> Is there much point in this?
>
> It seems to me that userspace will not really do anything different if
> it knows what's supported - it's probably easier for it (or cfg80211?)
> to just set the global min_rssi to the minimum over all bands, and
> treat
> this as we do most things in scheduled scan - an optimisation that
> doesn't really need a feature advertisement?
>
> I think that would simplify things somewhat.

I think the driver capability advertisement will provide more
flexibility to users in few cases, e.g., earlier, say if userspace is
programming -65dBm as min_rssi_thold common for all bands and with the
new implementation it may choose to configure -60dBm for 5GHz band and
-70dBm for 2.4GHz band for better user experience and power saving(with
filtering out unwanted 5GHz bssids). If the user configures -60dBm and
-70dBm thresholds for the old drivers without band specific threshold
support, it could result in unnecessary wakeups for 5GHz bssids with
rssi between -65dBm and -70dBm (compared to the old case of configuring
single threshold of -65dBm). So, I think it is better to keep the
capability advertisement.

>
>> + * @NL80211_ATTR_SCHED_SCAN_MIN_RSSI: Nested attribute that carries
>> the band
>> + * specific minimum rssi thresholds for the bands defined in enum
>> + * nl80211_band. The minimum rssi threshold value(s32) specific to a
>> band
>> + * shall be encapsulated in attribute with type value equals to one
>> of the
>> + * NL80211_BAND_* defined in enum nl80211_band. For example, the
>> minimum
>> + * rssi threshold value for 2.4GHz band shall be encapsulated within
>> an
>> + * attribute of type NL80211_BAND_2GHz. And one or more of such
>> attributes
>> + * will be nested within this attribute. If min rssi sub attribute is
>> not
>> + * included for any band, then %NL80211_SCAN_RSSI_THOLD_OFF will be
>> used
>> + * as minimum rssi threshold for scan result filtering.
>> + * If a scan result received in a particular band has lesser rssi
>> than the
>> + * rssi threshold specified for that band, then the scan result shall
>> be
>> + * filtered out. The rssi thresholds specified in matchsets along
>> with ssid
>> + * or bssid attribute will have higher precedence than the thresholds
>> + * mentioned in this attribute while checking rssi.
>
> It seems that this should be with the existing
> NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, not in this level namespace.

The band specific rssi thresholds that are being configured are global
across all matchsets whereas NL80211_SCHED_SCAN_MATCH_ATTR_* attributes
are mostly specific to each matchset. Hence I choose to define
attributes in higher level namespace. In future, whenever we want to
adding support for rssi thresholds per band and per matchset, we can
define attributes within NL80211_SCHED_SCAN_MATCH_ATTR_* namespace
level.

Thanks & Regards,
Vamsi

2019-01-15 13:25:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

Hi,

> > Is there much point in this?
> >
> > It seems to me that userspace will not really do anything different if
> > it knows what's supported - it's probably easier for it (or cfg80211?)
> > to just set the global min_rssi to the minimum over all bands, and
> > treat
> > this as we do most things in scheduled scan - an optimisation that
> > doesn't really need a feature advertisement?
> >
> > I think that would simplify things somewhat.
>
> I think the driver capability advertisement will provide more
> flexibility to users in few cases, e.g., earlier, say if userspace is
> programming -65dBm as min_rssi_thold common for all bands and with the
> new implementation it may choose to configure -60dBm for 5GHz band and
> -70dBm for 2.4GHz band for better user experience and power saving(with
> filtering out unwanted 5GHz bssids). If the user configures -60dBm and
> -70dBm thresholds for the old drivers without band specific threshold
> support, it could result in unnecessary wakeups for 5GHz bssids with
> rssi between -65dBm and -70dBm (compared to the old case of configuring
> single threshold of -65dBm). So, I think it is better to keep the
> capability advertisement.

Ok, fair enough.

> > It seems that this should be with the existing
> > NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, not in this level namespace.
>
> The band specific rssi thresholds that are being configured are global
> across all matchsets whereas NL80211_SCHED_SCAN_MATCH_ATTR_* attributes
> are mostly specific to each matchset. Hence I choose to define
> attributes in higher level namespace. In future, whenever we want to
> adding support for rssi thresholds per band and per matchset, we can
> define attributes within NL80211_SCHED_SCAN_MATCH_ATTR_* namespace
> level.

But why do we want global ones now?

johannes


2019-01-22 11:30:26

by Vamsi Krishna

[permalink] [raw]
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

On 2019-01-15 18:55, Johannes Berg wrote:

>
>> > It seems that this should be with the existing
>> > NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, not in this level namespace.
>>
>> The band specific rssi thresholds that are being configured are global
>> across all matchsets whereas NL80211_SCHED_SCAN_MATCH_ATTR_*
>> attributes
>> are mostly specific to each matchset. Hence I choose to define
>> attributes in higher level namespace. In future, whenever we want to
>> adding support for rssi thresholds per band and per matchset, we can
>> define attributes within NL80211_SCHED_SCAN_MATCH_ATTR_* namespace
>> level.
>
> But why do we want global ones now?
>

The global thresholds were there earlier as well. Earlier, we were using
a matchset with only rssi attribute without any ssid/bssid attribute
within the matchset. I thought having global attributes for specifying
global thresholds is better way going forward and avoids unnecessary
confusion. Mostly, same thresholds will be used for all ssids/bssids in
general rather than thresholds specific to ssid/bssid. However, I
couldn't see any disadvantage of using global attributes for this case,
please let me know if you see any disadvantage/concern with this.

Thanks,
Vamsi

2019-01-25 20:44:38

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific min rssi thresholds with sched scan

On Tue, 2019-01-22 at 17:00 +0530, [email protected] wrote:

> > > > It seems that this should be with the existing
> > > > NL80211_SCHED_SCAN_MATCH_ATTR_RSSI, not in this level namespace.
> > >
> > > The band specific rssi thresholds that are being configured are global
> > > across all matchsets whereas NL80211_SCHED_SCAN_MATCH_ATTR_*
> > > attributes
> > > are mostly specific to each matchset. Hence I choose to define
> > > attributes in higher level namespace. In future, whenever we want to
> > > adding support for rssi thresholds per band and per matchset, we can
> > > define attributes within NL80211_SCHED_SCAN_MATCH_ATTR_* namespace
> > > level.
> >
> > But why do we want global ones now?
> >
>
> The global thresholds were there earlier as well. Earlier, we were using
> a matchset with only rssi attribute without any ssid/bssid attribute
> within the matchset. I thought having global attributes for specifying
> global thresholds is better way going forward and avoids unnecessary
> confusion. Mostly, same thresholds will be used for all ssids/bssids in
> general rather than thresholds specific to ssid/bssid. However, I
> couldn't see any disadvantage of using global attributes for this case,
> please let me know if you see any disadvantage/concern with this.

I just think it's more complex code, overall.

To me we now have sort of a fork in the road

global RSSI ---> per-matchset RSSI
\
\--> global band-specific RSSI

so it seems somebody will want to introduce per-matchset band-specific
RSSI eventually, and that just makes it even more complex?

What's the downside of going to per-matchset band-specific RSSI?

johannes