improve BSS selection

When noise floor measurements are not available, compute SNR
using default values for the noise floor. This helps steer us
towards 5 GHz BSSes in high signal strength environments.

In more detail...

Existing code prefers a 5 GHz BSS when the 5 GHz BSS's signal
strength is "close" to that of the 2.4 GHz BSS, or when both SNRs
are large. However, the mwifiex driver does not provide noise
floor measurements, so we can't compute SNRs.

Because mwifiex doesn't provide NF measurements, the "large SNR"
code wasn't effective. By using default values for the noise floor,
we can again compute SNRs, and decide that the SNR is high enough
that we shouldn't worry about the exact difference in SNR.

The default noise floor values (open for 2.4 GHz, and one for 5 GHz)
were chosen by measurement in a noisy environment, so they should be
conservative.

Note that while this patch is motivated by mwifiex, it affects
ath9k as well. Although ath9k provides noise floor measurements
in general, it will sometimes fail to provide a measurement for
one or more specific channels.

As a result of this patch, we'll always compare BSSes based on SNR
(either measured or estimated), rather than sometimes comparing
based on signal strength. ("Always" assumes that the
WPA_SCAN_LEVEL_DBM flag is set. It is for mwifiex and ath9k.)

While there:
- fix a whitespace issue (spaces -> tab)
- clean up existing comments
- update dump_scan_res to indicate whether the noise floor is
  measured, or default
- change "great signal" indicator in dump_scan_res from "*" to "~"

Signed-hostap: mukesh agrawal <quiche@chromium.org>
---
 wpa_supplicant/scan.c | 52 +++++++++++++++++++++++++++++++++++----------------
 1 file changed, 36 insertions(+), 16 deletions(-)

On Tue, 2014-04-08 at 17:54 -0700, quiche@chromium.org wrote:
> When noise floor measurements are not available, compute SNR
> using default values for the noise floor. This helps steer us
> towards 5 GHz BSSes in high signal strength environments.
> 
> In more detail...
> 
> Existing code prefers a 5 GHz BSS when the 5 GHz BSS's signal
> strength is "close" to that of the 2.4 GHz BSS, or when both SNRs
> are large. However, the mwifiex driver does not provide noise
> floor measurements, so we can't compute SNRs.
> 
> Because mwifiex doesn't provide NF measurements, the "large SNR"
> code wasn't effective. By using default values for the noise floor,
> we can again compute SNRs, and decide that the SNR is high enough
> that we shouldn't worry about the exact difference in SNR.

Note that mwifiex does have this information, it just doesn't expose it
right now.  It does receive an 'nf' with each packet, but apparently
only chooses to use that information when processing management packets.
It also simply calculates an RSSI from the NF and the SNR of the packet.

So you might possibly get better information out of the driver, if you
fixed the driver to provide the noise floor or got Marvell to do that
for you.

Dan

> The default noise floor values (open for 2.4 GHz, and one for 5 GHz)
> were chosen by measurement in a noisy environment, so they should be
> conservative.
> 
> Note that while this patch is motivated by mwifiex, it affects
> ath9k as well. Although ath9k provides noise floor measurements
> in general, it will sometimes fail to provide a measurement for
> one or more specific channels.
> 
> As a result of this patch, we'll always compare BSSes based on SNR
> (either measured or estimated), rather than sometimes comparing
> based on signal strength. ("Always" assumes that the
> WPA_SCAN_LEVEL_DBM flag is set. It is for mwifiex and ath9k.)
> 
> While there:
> - fix a whitespace issue (spaces -> tab)
> - clean up existing comments
> - update dump_scan_res to indicate whether the noise floor is
>   measured, or default
> - change "great signal" indicator in dump_scan_res from "*" to "~"
> 
> Signed-hostap: mukesh agrawal <quiche@chromium.org>
> ---
>  wpa_supplicant/scan.c | 52 +++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 36 insertions(+), 16 deletions(-)
> 
> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
> index 1bb065d..fb04528 100644
> --- a/wpa_supplicant/scan.c
> +++ b/wpa_supplicant/scan.c
> @@ -683,7 +683,7 @@ void wpa_supplicant_req_scan(struct wpa_supplicant *wpa_s, int sec, int usec)
>  		}
>  		if (ssid) {
>  			wpa_dbg(wpa_s, MSG_DEBUG, "Not rescheduling scan to "
> -			        "ensure that specific SSID scans occur");
> +				"ensure that specific SSID scans occur");
>  			return;
>  		}
>  	}
> @@ -1082,11 +1082,12 @@ struct wpabuf * wpa_scan_get_vendor_ie_multi_beacon(
>   */
>  #define GREAT_SNR 30
>  
> +#define IS_5GHZ(n) (n > 4000)
> +
>  /* Compare function for sorting scan results. Return >0 if @b is considered
>   * better. */
>  static int wpa_scan_result_compar(const void *a, const void *b)
>  {
> -#define IS_5GHZ(n) (n > 4000)
>  	struct wpa_scan_res **_wa = (void *) a;
>  	struct wpa_scan_res **_wb = (void *) b;
>  	struct wpa_scan_res *wa = *_wa;
> @@ -1113,18 +1114,18 @@ static int wpa_scan_result_compar(const void *a, const void *b)
>  	    (wb->caps & IEEE80211_CAP_PRIVACY) == 0)
>  		return -1;
>  
> -	if ((wa->flags & wb->flags & WPA_SCAN_LEVEL_DBM) &&
> -	    !((wa->flags | wb->flags) & WPA_SCAN_NOISE_INVALID)) {
> +	if (wa->flags & wb->flags & WPA_SCAN_LEVEL_DBM) {
>  		snr_a = MIN(wa->level - wa->noise, GREAT_SNR);
>  		snr_b = MIN(wb->level - wb->noise, GREAT_SNR);
>  	} else {
> -		/* Not suitable information to calculate SNR, so use level */
> +		/* Level is not in dBm, so we can't calculate
> +                 * SNR. Just use raw level (units unknown). */
>  		snr_a = wa->level;
>  		snr_b = wb->level;
>  	}
>  
> -	/* best/max rate preferred if SNR close enough */
> -        if ((snr_a && snr_b && abs(snr_b - snr_a) < 5) ||
> +	/* if SNR is close, decide by max rate or frequency band */
> +	if ((snr_a && snr_b && abs(snr_b - snr_a) < 5) ||
>  	    (wa->qual && wb->qual && abs(wb->qual - wa->qual) < 10)) {
>  		maxrate_a = wpa_scan_get_max_rate(wa);
>  		maxrate_b = wpa_scan_get_max_rate(wb);
> @@ -1134,15 +1135,12 @@ static int wpa_scan_result_compar(const void *a, const void *b)
>  			return IS_5GHZ(wa->freq) ? -1 : 1;
>  	}
>  
> -	/* use freq for channel preference */
> -
>  	/* all things being equal, use SNR; if SNRs are
>  	 * identical, use quality values since some drivers may only report
>  	 * that value and leave the signal level zero */
>  	if (snr_b == snr_a)
>  		return wb->qual - wa->qual;
>  	return snr_b - snr_a;
> -#undef IS_5GHZ
>  }
>  
> 
> @@ -1206,14 +1204,18 @@ static void dump_scan_res(struct wpa_scan_results *scan_res)
>  
>  	for (i = 0; i < scan_res->num; i++) {
>  		struct wpa_scan_res *r = scan_res->res[i];
> -		if ((r->flags & (WPA_SCAN_LEVEL_DBM | WPA_SCAN_NOISE_INVALID))
> -		    == WPA_SCAN_LEVEL_DBM) {
> +		if (r->flags & WPA_SCAN_LEVEL_DBM) {
>  			int snr = r->level - r->noise;
> +			int noise_valid = !(r->flags & WPA_SCAN_NOISE_INVALID);
>  			wpa_printf(MSG_EXCESSIVE, MACSTR " freq=%d qual=%d "
> -				   "noise=%d level=%d snr=%d%s flags=0x%x",
> -				   MAC2STR(r->bssid), r->freq, r->qual,
> -				   r->noise, r->level, snr,
> -				   snr >= GREAT_SNR ? "*" : "", r->flags);
> +				   "noise=%d%s level=%d snr=%d%s flags=0x%x",
> +				   MAC2STR(r->bssid),
> +				   r->freq,
> +				   r->qual,
> +				   r->noise, noise_valid ? " " : "*",
> +				   r->level,
> +				   snr, snr >= GREAT_SNR ? "~" : " ",
> +				   r->flags);
>  		} else {
>  			wpa_printf(MSG_EXCESSIVE, MACSTR " freq=%d qual=%d "
>  				   "noise=%d level=%d flags=0x%x",
> @@ -1225,6 +1227,14 @@ static void dump_scan_res(struct wpa_scan_results *scan_res)
>  }
>  
> 
> +/*
> + * Noise floor values to use when we have signal strength
> + * measurements, but no noise floor measurments. These values were
> + * measured in an office environment with many APs.
> + */
> +#define DEFAULT_NOISE_FLOOR_2GHZ (-89)
> +#define DEFAULT_NOISE_FLOOR_5GHZ (-92)
> +
>  /**
>   * wpa_supplicant_get_scan_results - Get scan results
>   * @wpa_s: Pointer to wpa_supplicant data
> @@ -1250,6 +1260,16 @@ wpa_supplicant_get_scan_results(struct wpa_supplicant *wpa_s,
>  		return NULL;
>  	}
>  
> +	for (i=0; i < scan_res->num; i++) {
> +		struct wpa_scan_res *scan_res_item = scan_res->res[i];
> +		if (scan_res_item->flags & WPA_SCAN_NOISE_INVALID) {
> +			scan_res_item->noise =
> +				(IS_5GHZ(scan_res_item->freq) ?
> +				 DEFAULT_NOISE_FLOOR_5GHZ :
> +				 DEFAULT_NOISE_FLOOR_2GHZ);
> +		}
> +	}
> +
>  #ifdef CONFIG_WPS
>  	if (wpas_wps_in_progress(wpa_s)) {
>  		wpa_dbg(wpa_s, MSG_DEBUG, "WPS: Order scan results with WPS "

On Tue, Apr 8, 2014 at 9:28 PM, Dan Williams <dcbw@redhat.com> wrote:

> Note that mwifiex does have this information, it just doesn't expose it
> right now.  It does receive an 'nf' with each packet, but apparently
> only chooses to use that information when processing management packets.
> It also simply calculates an RSSI from the NF and the SNR of the packet.
>

Fair point.

My perspective, though, is that the Linux kernel API doesn't require
drivers to
expose this information. So, concerns about mwifiex aside, we don't really
have
a good reason to expect that all drivers will provide noise floor
measurements.
(Note also that some drivers that normally provide measurements will
sometimes
fail to do so.)

Further, wpa_supplicant already has the plumbing to keep track of whether
or not
we received noise floor measurements. I'd like to use that plumbing to
improve
BSS selection in cases where either a) the driver doesn't provide the
measurements
at all, or b) the driver has some transient issue which prevents it from
providing
some measurements.

How's that sound?

On Tue, Apr 08, 2014 at 05:54:49PM -0700, quiche@chromium.org wrote:
> When noise floor measurements are not available, compute SNR
> using default values for the noise floor. This helps steer us
> towards 5 GHz BSSes in high signal strength environments.

Thanks, applied.