RE: [PATCH 2/2] wpa_supplicant: fix the constant roaming problem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jouni,

We sent this patch about a year ago and it was unanswered.
As we still get complaints about constant roaming in the described scenario, I'd like to ask to you to take another look if possible.
We'd like to get any feedback, and if you think it may work  - we'll rebase and resubmit.

Thanks,
Andrei

> -----Original Message-----
> From: Hostap <hostap-bounces@xxxxxxxxxxxxxxxxxxx> On Behalf Of Andrei
> Otcheretianski
> Sent: Wednesday, September 05, 2018 20:45
> To: hostap@xxxxxxxxxxxxxxxxxxx
> Cc: Grumbach, Emmanuel <emmanuel.grumbach@xxxxxxxxx>
> Subject: [PATCH 2/2] wpa_supplicant: fix the constant roaming problem
> 
> From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
> 
> We saw that on certain platforms in certain places we keep switching between
> two APs and eventually get the same RSSI.
> Debugging showed that we have a very big difference between the two
> antennas.
> 
> Ant A can hear AP A very well (-60) but AP B very bad (-80) Ant B can hear AP B
> very well (-60) but AP A very bad (-80)
> 
> When the device associates to AP A, it'll learn to use Ant A.
> If the device uses one single antenna to receive the scan results, it may hear the
> AP it is currently associated to on the second antenna and get bad results.
> Because of that, the wpa_supplicant will roam to the other AP and the same
> scenario will repeat itself:
> 
> Association to AP A (Ant A reports -60).
> Scan on Ant A: AP A: -60, AP B: -80
> Scan on Ant B: AP A: -80, AP A: -60 ==> ROAM.
> 
> Association to AP B (Ant B reports -60)
> Scan on Ant A: AP A: -60, AP B: -80 ==> ROAM
> 
> Etc...
> 
> Fix this by querying the signal of the current AP using
> drv_signal_poll() instead of relying on the signal that we get from the scan
> results.
> 
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
> ---
>  wpa_supplicant/events.c           | 83 +++++++++++++++++++++++++++++++++----
> --
>  wpa_supplicant/scan.c             | 57 +++++++++++++--------------
>  wpa_supplicant/scan.h             |  4 ++
>  wpa_supplicant/wpa_supplicant_i.h |  9 +++++
>  4 files changed, 112 insertions(+), 41 deletions(-)
> 
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index
> 4cdca34..a814041 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -1617,6 +1617,33 @@ static void
> wpa_supplicant_rsn_preauth_scan_results(
>  }
> 
> 
> +static int wpas_get_snr_signal_info(const struct wpa_signal_info *si) {
> +	int noise = IS_5GHZ(si->frequency) ?
> +		DEFAULT_NOISE_FLOOR_5GHZ :
> +		DEFAULT_NOISE_FLOOR_2GHZ;
> +
> +	/*
> +	 * Since we take the average beacon signal, we can't use
> +	 * the current noise measurement (average vs. snapshot),
> +	 * so use the default values instead.
> +	 */
> +	return si->avg_beacon_signal - noise;
> +}
> +
> +
> +static unsigned int
> +wpas_get_est_throughput_from_bss_snr(const struct wpa_supplicant *wpa_s,
> +				     const struct wpa_bss *bss, int snr) {
> +	int rate = wpa_bss_get_max_rate(bss);
> +	const u8 *ies = (void *)(bss + 1);
> +	size_t ie_len = bss->ie_len ?: bss->beacon_ie_len;
> +
> +	return wpas_get_est_tpt(wpa_s, ies, ie_len, rate, snr); }
> +
> +
>  static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
>  				       struct wpa_bss *selected,
>  				       struct wpa_ssid *ssid)
> @@ -1625,7 +1652,9 @@ static int wpa_supplicant_need_to_roam(struct
> wpa_supplicant *wpa_s,  #ifndef CONFIG_NO_ROAMING
>  	int min_diff, diff;
>  	int to_5ghz;
> -	int cur_est, sel_est;
> +	int cur_level;
> +	unsigned int cur_est, sel_est;
> +	struct wpa_signal_info si;
>  #endif /* CONFIG_NO_ROAMING */
> 
>  	if (wpa_s->reassociate)
> @@ -1676,7 +1705,39 @@ static int wpa_supplicant_need_to_roam(struct
> wpa_supplicant *wpa_s,
>  		return 1;
>  	}
> 
> -	if (selected->est_throughput > current_bss->est_throughput + 5000) {
> +	cur_level = current_bss->level;
> +	cur_est = current_bss->est_throughput;
> +
> +	/*
> +	 * Try to poll the signal from the driver since this will allow to get
> +	 * more accurate values. In some cases, there can be big differences
> +	 * between the RSSI of the probe response of the AP we are associated
> +	 * to and the beacons we hear from the same AP after association. This
> +	 * can happen when there are two antennas that hear the AP very
> +	 * differently. If the driver chooses to hear the probe responses
> +	 * during the scan on the "bad" antenna because it wants to save power,
> +	 * but knows to choose the other antenna after association, we will
> +	 * hear our AP with a low RSSI as part of the scan, even we can hear
> +	 * it decently on the other antenna.
> +	 * To cope with this, ask the driver to teach us how it hears the AP.
> +	 * Also, the scan results may be a bit old, since we can very quickly
> +	 * get fresh information about our own AP, do that.
> +	 */
> +	if (!wpa_drv_signal_poll(wpa_s, &si)) {
> +		if (si.avg_beacon_signal) {
> +			int snr = wpas_get_snr_signal_info(&si);
> +
> +			cur_level = si.avg_beacon_signal;
> +			cur_est =
> wpas_get_est_throughput_from_bss_snr(wpa_s,
> +
> current_bss,
> +								       snr);
> +			wpa_dbg(wpa_s, MSG_DEBUG,
> +				"Using poll to get accurate level and est
> throughput: level=%d snr=%d est_throughput=%u",
> +				cur_level, snr, cur_est);
> +		}
> +	}
> +
> +	if (selected->est_throughput > cur_est + 5000) {
>  		wpa_dbg(wpa_s, MSG_DEBUG,
>  			"Allow reassociation - selected BSS has better
> estimated throughput");
>  		return 1;
> @@ -1684,30 +1745,28 @@ static int wpa_supplicant_need_to_roam(struct
> wpa_supplicant *wpa_s,
> 
>  	to_5ghz = selected->freq > 4000 && current_bss->freq < 4000;
> 
> -	if (current_bss->level < 0 &&
> -	    current_bss->level > selected->level + to_5ghz * 2) {
> +	if (cur_level < 0 && cur_level > selected->level + to_5ghz * 2) {
>  		wpa_dbg(wpa_s, MSG_DEBUG, "Skip roam - Current BSS has
> better "
>  			"signal level");
>  		return 0;
>  	}
> 
> -	if (current_bss->est_throughput > selected->est_throughput + 5000) {
> +	if (cur_est > selected->est_throughput + 5000) {
>  		wpa_dbg(wpa_s, MSG_DEBUG,
>  			"Skip roam - Current BSS has better estimated
> throughput");
>  		return 0;
>  	}
> 
> -	cur_est = current_bss->est_throughput;
>  	sel_est = selected->est_throughput;
>  	min_diff = 2;
> -	if (current_bss->level < 0) {
> -		if (current_bss->level < -85)
> +	if (cur_level < 0) {
> +		if (cur_level < -85)
>  			min_diff = 1;
> -		else if (current_bss->level < -80)
> +		else if (cur_level < -80)
>  			min_diff = 2;
> -		else if (current_bss->level < -75)
> +		else if (cur_level < -75)
>  			min_diff = 3;
> -		else if (current_bss->level < -70)
> +		else if (cur_level < -70)
>  			min_diff = 4;
>  		else
>  			min_diff = 5;
> @@ -1736,7 +1795,7 @@ static int wpa_supplicant_need_to_roam(struct
> wpa_supplicant *wpa_s,
>  		else
>  			min_diff = 0;
>  	}
> -	diff = abs(current_bss->level - selected->level);
> +	diff = abs(cur_level - selected->level);
>  	if (diff < min_diff) {
>  		wpa_dbg(wpa_s, MSG_DEBUG,
>  			"Skip roam - too small difference in signal level (%d <
> %d)", diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c index
> 56d53fb..030e7ac 100644
> --- a/wpa_supplicant/scan.c
> +++ b/wpa_supplicant/scan.c
> @@ -1869,8 +1869,6 @@ struct wpabuf *
> wpa_scan_get_vendor_ie_multi(const struct wpa_scan_res *res,
>   */
>  #define GREAT_SNR 25
> 
> -#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) @@ -2078,14
> +2076,6 @@ void filter_scan_res(struct wpa_supplicant *wpa_s,  }
> 
> 
> -/*
> - * Noise floor values to use when we have signal strength
> - * measurements, but no noise floor measurements. These values were
> - * measured in an office environment with many APs.
> - */
> -#define DEFAULT_NOISE_FLOOR_2GHZ (-89)
> -#define DEFAULT_NOISE_FLOOR_5GHZ (-92)
> -
>  void scan_snr(struct wpa_scan_res *res)  {
>  	if (res->flags & WPA_SCAN_NOISE_INVALID) { @@ -2169,21 +2159,13
> @@ static unsigned int max_vht80_rate(int snr)
>  	return 390000; /* VHT80 MCS9 */
>  }
> 
> -
> -void scan_est_throughput(struct wpa_supplicant *wpa_s,
> -			 struct wpa_scan_res *res)
> +unsigned int wpas_get_est_tpt(const struct wpa_supplicant *wpa_s,
> +			      const u8 *ies, size_t ies_len, int rate,
> +			      int snr)
>  {
>  	enum local_hw_capab capab = wpa_s->hw_capab;
> -	int rate; /* max legacy rate in 500 kb/s units */
> -	const u8 *ie;
>  	unsigned int est, tmp;
> -	int snr = res->snr;
> -
> -	if (res->est_throughput)
> -		return;
> -
> -	/* Get maximum legacy rate */
> -	rate = wpa_scan_get_max_rate(res);
> +	const u8 *ie;
> 
>  	/* Limit based on estimated SNR */
>  	if (rate > 1 * 2 && snr < 1)
> @@ -2209,7 +2191,7 @@ void scan_est_throughput(struct wpa_supplicant
> *wpa_s,
>  	est = rate * 500;
> 
>  	if (capab == CAPAB_HT || capab == CAPAB_HT40 || capab ==
> CAPAB_VHT) {
> -		ie = wpa_scan_get_ie(res, WLAN_EID_HT_CAP);
> +		ie = get_ie(ies, ies_len, WLAN_EID_HT_CAP);
>  		if (ie) {
>  			tmp = max_ht20_rate(snr);
>  			if (tmp > est)
> @@ -2218,7 +2200,7 @@ void scan_est_throughput(struct wpa_supplicant
> *wpa_s,
>  	}
> 
>  	if (capab == CAPAB_HT40 || capab == CAPAB_VHT) {
> -		ie = wpa_scan_get_ie(res, WLAN_EID_HT_OPERATION);
> +		ie = get_ie(ies, ies_len, WLAN_EID_HT_OPERATION);
>  		if (ie && ie[1] >= 2 &&
>  		    (ie[3] &
> HT_INFO_HT_PARAM_SECONDARY_CHNL_OFF_MASK)) {
>  			tmp = max_ht40_rate(snr);
> @@ -2229,13 +2211,13 @@ void scan_est_throughput(struct wpa_supplicant
> *wpa_s,
> 
>  	if (capab == CAPAB_VHT) {
>  		/* Use +1 to assume VHT is always faster than HT */
> -		ie = wpa_scan_get_ie(res, WLAN_EID_VHT_CAP);
> +		ie = get_ie(ies, ies_len, WLAN_EID_VHT_CAP);
>  		if (ie) {
>  			tmp = max_ht20_rate(snr) + 1;
>  			if (tmp > est)
>  				est = tmp;
> 
> -			ie = wpa_scan_get_ie(res,
> WLAN_EID_HT_OPERATION);
> +			ie = get_ie(ies, ies_len, WLAN_EID_HT_OPERATION);
>  			if (ie && ie[1] >= 2 &&
>  			    (ie[3] &
> 
> HT_INFO_HT_PARAM_SECONDARY_CHNL_OFF_MASK)) { @@ -2244,7 +2226,7
> @@ void scan_est_throughput(struct wpa_supplicant *wpa_s,
>  					est = tmp;
>  			}
> 
> -			ie = wpa_scan_get_ie(res,
> WLAN_EID_VHT_OPERATION);
> +			ie = get_ie(ies, ies_len, WLAN_EID_VHT_OPERATION);
>  			if (ie && ie[1] >= 1 &&
>  			    (ie[2] & VHT_OPMODE_CHANNEL_WIDTH_MASK)) {
>  				tmp = max_vht80_rate(snr) + 1;
> @@ -2254,9 +2236,26 @@ void scan_est_throughput(struct wpa_supplicant
> *wpa_s,
>  		}
>  	}
> 
> -	/* TODO: channel utilization and AP load (e.g., from AP Beacon) */
> +	return est;
> +}
> +
> +void scan_est_throughput(struct wpa_supplicant *wpa_s,
> +			 struct wpa_scan_res *res)
> +{
> +	int rate; /* max legacy rate in 500 kb/s units */
> +	int snr = res->snr;
> +	const u8 *ies = (void *)(res + 1);
> 
> -	res->est_throughput = est;
> +	if (res->est_throughput)
> +		return;
> +
> +	/* Get maximum legacy rate */
> +	rate = wpa_scan_get_max_rate(res);
> +
> +	res->est_throughput =
> +		wpas_get_est_tpt(wpa_s, ies, res->ie_len, rate, snr);
> +
> +	/* TODO: channel utilization and AP load (e.g., from AP Beacon) */
>  }
> 
> 
> diff --git a/wpa_supplicant/scan.h b/wpa_supplicant/scan.h index
> 2aa0a8b..204332a 100644
> --- a/wpa_supplicant/scan.h
> +++ b/wpa_supplicant/scan.h
> @@ -58,6 +58,10 @@ void filter_scan_res(struct wpa_supplicant *wpa_s,  void
> scan_snr(struct wpa_scan_res *res);  void scan_est_throughput(struct
> wpa_supplicant *wpa_s,
>  			 struct wpa_scan_res *res);
> +
> +unsigned int wpas_get_est_tpt(const struct wpa_supplicant *wpa_s,
> +			      const u8 *ies, size_t ies_len, int rate,
> +			      int snr);
>  void wpa_supplicant_set_default_scan_ies(struct wpa_supplicant *wpa_s);
> 
>  #endif /* SCAN_H */
> diff --git a/wpa_supplicant/wpa_supplicant_i.h
> b/wpa_supplicant/wpa_supplicant_i.h
> index 9b8d1fa..9255b2c 100644
> --- a/wpa_supplicant/wpa_supplicant_i.h
> +++ b/wpa_supplicant/wpa_supplicant_i.h
> @@ -1487,4 +1487,13 @@ int
> wpas_ctrl_iface_get_pref_freq_list_override(struct wpa_supplicant *wpa_s,  int
> wpa_is_fils_supported(struct wpa_supplicant *wpa_s);  int
> wpa_is_fils_sk_pfs_supported(struct wpa_supplicant *wpa_s);
> 
> +/*
> + * Noise floor values to use when we have signal strength
> + * measurements, but no noise floor measurements. These values were
> + * measured in an office environment with many APs.
> + */
> +#define DEFAULT_NOISE_FLOOR_2GHZ (-89)
> +#define DEFAULT_NOISE_FLOOR_5GHZ (-92)
> +#define IS_5GHZ(n) (n > 4000)
> +
>  #endif /* WPA_SUPPLICANT_I_H */
> --
> 2.7.4
> 
> 
> _______________________________________________
> Hostap mailing list
> Hostap@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/hostap

_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux