From: Kevin Lund <kglund@xxxxxxxxxx>
Within wpas_connection_failed(), the 'count' value of wpa_blacklist is
erroneously used as a tally of the number times the device has failed
to associate to a given bssid without making a successful connection.
This is not accurate becuase there are a variety of ways a BSS can be
added to the blacklist beyond failed association such as interference
or deauthentication. This 'count' is lost whenever the blacklist is
cleared, so the wpa_supplicant stores an additional value
'extra_blacklist_count' which helps persist the 'count' through clears.
These count values are used to determine how long to wait to rescan
after a failed connection attempt.
While this logic was already slightly wrong, it would have been
completely broken by the upcoming change which adds time-based
blacklisting functionality. With the upcoming change, 'count' values
are not cleared on association, and thus do not necessarily even
approximate the "consecutive connection failures" which they were being
used for.
This change seeks to remove this unnecessary overloading of the
blacklist 'count' by directly tracking consecutive connection failures
within the wpa_supplicant struct, independent of the blacklist. This new
'consecutive_conn_failures' is iterated with every connection failure
and cleared when any successful connection is made. This change also
removes the now unused 'extra_blacklist_count' value.
Signed-off-by: Kevin Lund <kglund@xxxxxxxxxx>
Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
---
v2:
- rebased (previous versions didn't apply to tip-of-git)
- stripped downstream stuff from commit message
- sent via @chromium.org, since @google.com has DMARC policies that
aren't compatible with mailing lists that add footers (that is, mail
gets marked as spam by various mail providers, including Google...)
wpa_supplicant/blacklist.c | 5 -----
wpa_supplicant/ctrl_iface.c | 3 ++-
wpa_supplicant/wpa_supplicant.c | 27 +++++++++++----------------
wpa_supplicant/wpa_supplicant_i.h | 13 ++-----------
wpa_supplicant/wps_supplicant.c | 2 +-
5 files changed, 16 insertions(+), 34 deletions(-)
diff --git a/wpa_supplicant/blacklist.c b/wpa_supplicant/blacklist.c
index e53dc38b3ec8..2e01e7feaa39 100644
--- a/wpa_supplicant/blacklist.c
+++ b/wpa_supplicant/blacklist.c
@@ -123,19 +123,14 @@ int wpa_blacklist_del(struct wpa_supplicant *wpa_s, const u8 *bssid)
void wpa_blacklist_clear(struct wpa_supplicant *wpa_s)
{
struct wpa_blacklist *e, *prev;
- int max_count = 0;
e = wpa_s->blacklist;
wpa_s->blacklist = NULL;
while (e) {
- if (e->count > max_count)
- max_count = e->count;
prev = e;
e = e->next;
wpa_printf(MSG_DEBUG, "Removed BSSID " MACSTR " from "
"blacklist (clear)", MAC2STR(prev->bssid));
os_free(prev);
}
-
- wpa_s->extra_blacklist_count += max_count;
}
diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
index e727dc5bbae7..bdc65ca422ec 100644
--- a/wpa_supplicant/ctrl_iface.c
+++ b/wpa_supplicant/ctrl_iface.c
@@ -8309,9 +8309,10 @@ static void wpa_supplicant_ctrl_iface_flush(struct wpa_supplicant *wpa_s)
wpa_s->set_sta_uapsd = 0;
wpa_s->sta_uapsd = 0;
+ wpa_s->consecutive_conn_failures = 0;
+
wpa_drv_radio_disable(wpa_s, 0);
wpa_blacklist_clear(wpa_s);
- wpa_s->extra_blacklist_count = 0;
wpa_supplicant_ctrl_iface_remove_network(wpa_s, "all");
wpa_supplicant_ctrl_iface_remove_cred(wpa_s, "all");
wpa_config_flush_blobs(wpa_s->conf);
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index f5a4c360b6a4..42f6a1d41b45 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -979,7 +979,7 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
#endif /* CONFIG_CTRL_IFACE || !CONFIG_NO_STDOUT_DEBUG */
wpas_clear_temp_disabled(wpa_s, ssid, 1);
wpa_blacklist_clear(wpa_s);
- wpa_s->extra_blacklist_count = 0;
+ wpa_s->consecutive_conn_failures = 0;
wpa_s->new_connection = 0;
wpa_drv_set_operstate(wpa_s, 1);
#ifndef IEEE8021X_EAPOL
@@ -7229,10 +7229,6 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
/*
* Add the failed BSSID into the blacklist and speed up next scan
* attempt if there could be other APs that could accept association.
- * The current blacklist count indicates how many times we have tried
- * connecting to this AP and multiple attempts mean that other APs are
- * either not available or has already been tried, so that we can start
- * increasing the delay here to avoid constant scanning.
*/
count = wpa_blacklist_add(wpa_s, bssid);
if (count == 1 && wpa_s->current_bss) {
@@ -7257,19 +7253,19 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
}
}
- /*
- * Add previous failure count in case the temporary blacklist was
- * cleared due to no other BSSes being available.
- */
- count += wpa_s->extra_blacklist_count;
+ wpa_s->consecutive_conn_failures++;
- if (count > 3 && wpa_s->current_ssid) {
+ if (wpa_s->consecutive_conn_failures > 3 && wpa_s->current_ssid) {
wpa_printf(MSG_DEBUG, "Continuous association failures - "
"consider temporary network disabling");
wpas_auth_failed(wpa_s, "CONN_FAILED");
}
-
- switch (count) {
+ /*
+ * Multiple consecutive connection failures mean that other APs are
+ * either not available or have already been tried, so we can start
+ * increasing the delay here to avoid constant scanning.
+ */
+ switch (wpa_s->consecutive_conn_failures) {
case 1:
timeout = 100;
break;
@@ -7287,8 +7283,8 @@ void wpas_connection_failed(struct wpa_supplicant *wpa_s, const u8 *bssid)
break;
}
- wpa_dbg(wpa_s, MSG_DEBUG, "Blacklist count %d --> request scan in %d "
- "ms", count, timeout);
+ wpa_dbg(wpa_s, MSG_DEBUG, "Consecutive connection failures: %d --> request "
+ "scan in %d ms", wpa_s->consecutive_conn_failures, timeout);
/*
* TODO: if more than one possible AP is available in scan results,
@@ -7652,7 +7648,6 @@ void wpas_request_connection(struct wpa_supplicant *wpa_s)
wpa_s->normal_scans = 0;
wpa_s->scan_req = NORMAL_SCAN_REQ;
wpa_supplicant_reinit_autoscan(wpa_s);
- wpa_s->extra_blacklist_count = 0;
wpa_s->disconnected = 0;
wpa_s->reassociate = 1;
wpa_s->last_owe_group = 0;
diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
index 28867f04ec23..d0de2bf8e233 100644
--- a/wpa_supplicant/wpa_supplicant_i.h
+++ b/wpa_supplicant/wpa_supplicant_i.h
@@ -643,17 +643,8 @@ struct wpa_supplicant {
struct wpa_blacklist *blacklist;
- /**
- * extra_blacklist_count - Sum of blacklist counts after last connection
- *
- * This variable is used to maintain a count of temporary blacklisting
- * failures (maximum number for any BSS) over blacklist clear
- * operations. This is needed for figuring out whether there has been
- * failures prior to the last blacklist clear operation which happens
- * whenever no other not-blacklisted BSS candidates are available. This
- * gets cleared whenever a connection has been established successfully.
- */
- int extra_blacklist_count;
+ /* Number of connection failures since last successful connection */
+ unsigned int consecutive_conn_failures;
/**
* scan_req - Type of the scan request
diff --git a/wpa_supplicant/wps_supplicant.c b/wpa_supplicant/wps_supplicant.c
index b7680f0888e2..613fe2b6a3b6 100644
--- a/wpa_supplicant/wps_supplicant.c
+++ b/wpa_supplicant/wps_supplicant.c
@@ -720,7 +720,7 @@ static void wpa_supplicant_wps_event_success(struct wpa_supplicant *wpa_s)
wpas_notify_wps_event_success(wpa_s);
if (wpa_s->current_ssid)
wpas_clear_temp_disabled(wpa_s, wpa_s->current_ssid, 1);
- wpa_s->extra_blacklist_count = 0;
+ wpa_s->consecutive_conn_failures = 0;
/*
* Enable the networks disabled during wpas_wps_reassoc after 10
--
2.27.0.rc2.251.g90737beb825-goog
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
