MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Wed, 26 Dec 2018 15:24:23 +0530
From:   vamsin@codeaurora.org
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org, jouni@codeaurora.org
Subject: Re: [PATCH] nl80211/cfg80211: Add support to specify band specific
 min rssi thresholds with sched scan
In-Reply-To: <49850508355f57fba6f9803416bf71c24766747b.camel@sipsolutions.net>
References: <1543566848-14819-1-git-send-email-vamsin@codeaurora.org>
 <49850508355f57fba6f9803416bf71c24766747b.camel@sipsolutions.net>
Message-ID: <a6b9ae694194e5b766b0045fe1e4fc76@codeaurora.org>
User-Agent: Roundcube Webmail/1.2.5
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

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