On Fri, Dec 06, 2019 at 02:27:47PM -0800, Matthew Wang wrote:
> 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.
...
Thanks for the reminder on this patch and the additional extension to it
in 5/5. I fixed couple of issues in these, cleaned up coding style a
bit, and split this patch 4/5 into pieces that make it easier to
understand what is happening here (it was hidden quite a bit due to the
refactoring included in the same patch 4/5 here).
The attached patches show the current pending set that I'm
considering to apply. I have not yet had a chance to test this properly,
but this looks much safer than I thought originally now that the real
change is clear (the attached patch 4/5 is clear).
--
Jouni Malinen PGP id EFC895FA
>From 98ea9d5d51133fa707bc81d061c7aa474d750a60 Mon Sep 17 00:00:00 2001
From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
Date: Fri, 6 Dec 2019 14:27:47 -0800
Subject: [PATCH 1/5] Use local variables for current BSS signal strength in
roaming
This is a step towards allowing these values to be determined based on
signal poll instead of scan results.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
---
wpa_supplicant/events.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 2bf35cb5898a..075bf6fcfb52 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -1680,7 +1680,8 @@ 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;
#endif /* CONFIG_NO_ROAMING */
if (wpa_s->reassociate)
@@ -1731,7 +1732,10 @@ 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;
+
+ if (selected->est_throughput > cur_est + 5000) {
wpa_dbg(wpa_s, MSG_DEBUG,
"Allow reassociation - selected BSS has better estimated throughput");
return 1;
@@ -1739,30 +1743,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;
@@ -1791,7 +1793,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)",
--
2.20.1
>From ef1a45f28a42d2c57c98435936de5ef0d48ede90 Mon Sep 17 00:00:00 2001
From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
Date: Fri, 6 Dec 2019 14:27:47 -0800
Subject: [PATCH 2/5] Move scan/roaming related defines to a header file
This is a step towards allowing these values to be used in both scan.c
and events.c.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
---
wpa_supplicant/scan.c | 22 ----------------------
wpa_supplicant/scan.h | 22 ++++++++++++++++++++++
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index 4d158a9c66da..fea410250a62 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -1956,20 +1956,6 @@ struct wpabuf * wpa_scan_get_vendor_ie_multi(const struct wpa_scan_res *res,
}
-/*
- * Channels with a great SNR can operate at full rate. What is a great SNR?
- * This doc https://supportforums.cisco.com/docs/DOC-12954 says, "the general
- * rule of thumb is that any SNR above 20 is good." This one
- * http://www.cisco.com/en/US/tech/tk722/tk809/technologies_q_and_a_item09186a00805e9a96.shtml#qa23
- * recommends 25 as a minimum SNR for 54 Mbps data rate. The estimates used in
- * scan_est_throughput() allow even smaller SNR values for the maximum rates
- * (21 for 54 Mbps, 22 for VHT80 MCS9, 24 for HT40 and HT20 MCS7). Use 25 as a
- * somewhat conservative value here.
- */
-#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)
@@ -2178,14 +2164,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) {
diff --git a/wpa_supplicant/scan.h b/wpa_supplicant/scan.h
index 58caa7818dce..5ad1ce9f1b30 100644
--- a/wpa_supplicant/scan.h
+++ b/wpa_supplicant/scan.h
@@ -9,6 +9,28 @@
#ifndef SCAN_H
#define SCAN_H
+/*
+ * 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)
+
+/*
+ * Channels with a great SNR can operate at full rate. What is a great SNR?
+ * This doc https://supportforums.cisco.com/docs/DOC-12954 says, "the general
+ * rule of thumb is that any SNR above 20 is good." This one
+ * http://www.cisco.com/en/US/tech/tk722/tk809/technologies_q_and_a_item09186a00805e9a96.shtml#qa23
+ * recommends 25 as a minimum SNR for 54 Mbps data rate. The estimates used in
+ * scan_est_throughput() allow even smaller SNR values for the maximum rates
+ * (21 for 54 Mbps, 22 for VHT80 MCS9, 24 for HT40 and HT20 MCS7). Use 25 as a
+ * somewhat conservative value here.
+ */
+#define GREAT_SNR 25
+
+#define IS_5GHZ(n) (n > 4000)
+
int wpa_supplicant_enabled_networks(struct wpa_supplicant *wpa_s);
void wpa_supplicant_req_scan(struct wpa_supplicant *wpa_s, int sec, int usec);
int wpa_supplicant_delayed_sched_scan(struct wpa_supplicant *wpa_s,
--
2.20.1
>From ad06ac0b04cc0a3efa8ec5b3e7814f2b4d864d88 Mon Sep 17 00:00:00 2001
From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
Date: Fri, 6 Dec 2019 14:27:47 -0800
Subject: [PATCH 3/5] Move throughput estimation into a helper function
This is a step towards allowing this functionality to update the scan
result -based values with the values from a signal poll for the current
BSS.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
---
wpa_supplicant/scan.c | 50 +++++++++++++++++++++++++++----------------
wpa_supplicant/scan.h | 3 +++
2 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
index fea410250a62..205653a67b63 100644
--- a/wpa_supplicant/scan.c
+++ b/wpa_supplicant/scan.c
@@ -2248,20 +2248,13 @@ static unsigned int max_vht80_rate(int snr)
}
-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)
@@ -2287,7 +2280,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)
@@ -2296,7 +2289,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);
@@ -2307,13 +2300,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)) {
@@ -2322,7 +2315,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;
@@ -2332,9 +2325,30 @@ 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 = (const void *) (res + 1);
+ size_t ie_len = res->ie_len;
+
+ if (res->est_throughput)
+ return;
+
+ /* Get maximum legacy rate */
+ rate = wpa_scan_get_max_rate(res);
- res->est_throughput = est;
+ if (!ie_len)
+ ie_len = res->beacon_ie_len;
+ 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 5ad1ce9f1b30..c9ce2cecf8ae 100644
--- a/wpa_supplicant/scan.h
+++ b/wpa_supplicant/scan.h
@@ -82,6 +82,9 @@ 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 */
--
2.20.1
>From 7e7b23e2290b1c0731f0ae04200ddef033334abb Mon Sep 17 00:00:00 2001
From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
Date: Fri, 6 Dec 2019 14:27:47 -0800
Subject: [PATCH 4/5] Update throughput estimate for the current BSS based on
signal poll
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...
Improve this by querying the signal level of the current AP using
drv_signal_poll() instead of relying on the signal level that we get
from the scan results. Also update the throughput estimate based on the
likely more accurate values for the current association.
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
---
wpa_supplicant/events.c | 56 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 075bf6fcfb52..6b7cf7c6fe7a 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -1672,6 +1672,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 = (const void *) (bss + 1);
+ size_t ie_len = bss->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)
@@ -1682,6 +1709,7 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
int to_5ghz;
int cur_level;
unsigned int cur_est, sel_est;
+ struct wpa_signal_info si;
#endif /* CONFIG_NO_ROAMING */
if (wpa_s->reassociate)
@@ -1735,6 +1763,34 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
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 frames of the AP we are
+ * associated with and the Beacon frames we hear from the same AP after
+ * association. This can happen, e.g., when there are two antennas that
+ * hear the AP very differently. If the driver chooses to hear the
+ * Probe Response frames 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 when 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 currently associated AP.
+ */
+ if (wpa_drv_signal_poll(wpa_s, &si) == 0 &&
+ 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 signal poll values for the current BSS: 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");
--
2.20.1
>From 69ccc557d89b21be8fccc6fe47df514a6ac79c91 Mon Sep 17 00:00:00 2001
From: Matthew Wang <matthewmwang@xxxxxxxxxxxx>
Date: Fri, 6 Dec 2019 14:27:48 -0800
Subject: [PATCH 5/5] wpa_supplicant: Fall back to avg_signal in roaming
decision
Some drivers (e.g. Marvell WiFi) don't report avg_beacon_signal, but
it's still useful to poll for the signal again when a roaming decision
needs to be made. Use si.avg_signal when si.avg_beacon_signal is not
available.
Signed-off-by: Matthew Wang <matthewmwang@xxxxxxxxxxxx>
---
wpa_supplicant/events.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 6b7cf7c6fe7a..cc6cb6716bbe 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -1672,9 +1672,9 @@ static void wpa_supplicant_rsn_preauth_scan_results(
}
-static int wpas_get_snr_signal_info(const struct wpa_signal_info *si)
+static int wpas_get_snr_signal_info(u32 frequency, int avg_signal)
{
- int noise = IS_5GHZ(si->frequency) ?
+ int noise = IS_5GHZ(frequency) ?
DEFAULT_NOISE_FLOOR_5GHZ :
DEFAULT_NOISE_FLOOR_2GHZ;
@@ -1683,7 +1683,7 @@ static int wpas_get_snr_signal_info(const struct wpa_signal_info *si)
* the current noise measurement (average vs. snapshot),
* so use the default values instead.
*/
- return si->avg_beacon_signal - noise;
+ return avg_signal - noise;
}
@@ -1779,10 +1779,13 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
* information about our currently associated AP.
*/
if (wpa_drv_signal_poll(wpa_s, &si) == 0 &&
- si.avg_beacon_signal) {
- int snr = wpas_get_snr_signal_info(&si);
+ (si.avg_beacon_signal || si.avg_signal)) {
+ int snr;
+
+ cur_level = si.avg_beacon_signal ? si.avg_beacon_signal :
+ si.avg_signal;
+ snr = wpas_get_snr_signal_info(si.frequency, cur_level);
- cur_level = si.avg_beacon_signal;
cur_est = wpas_get_est_throughput_from_bss_snr(wpa_s,
current_bss,
snr);
--
2.20.1
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
