diff mbox series

[v3] driver_nl80211: report invalid signal and noise when info is unavailable

Message ID 20201103075416.14480-1-andrei.otcheretianski@intel.com
State Accepted
Headers show
Series [v3] driver_nl80211: report invalid signal and noise when info is unavailable | expand

Commit Message

Andrei Otcheretianski Nov. 3, 2020, 7:54 a.m. UTC 
From: Avraham Stern <avraham.stern@intel.com>

When the driver sends a CQM RSSI threshold event, wpa_supplicant
queries the driver for the signal and noise values. However, it
is possible that by that time the station has already disconnected
from the AP, so these values are no longer valid. In this case,
indicate that these values are invalid by setting them to
WPA_INVALID_NOISE.
Previously a value of 0 would be reported, which may be confusing as
this is a valid value.
Since nl80211_get_link_signal() and nl80211_get_link_noise() already
set invalid values for a case of failure, just use the value set by
these functions even if they fail.

Signed-off-by: Avraham Stern <avraham.stern@intel.com>
Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
---
 src/drivers/driver_nl80211_event.c | 26 ++++++++++++++++----------
 wpa_supplicant/bss.h               |  2 +-
 2 files changed, 17 insertions(+), 11 deletions(-)

Comments

Brian Norris Nov. 5, 2020, 9:21 p.m. UTC | #1 
On Mon, Nov 2, 2020 at 11:55 PM Andrei Otcheretianski
<andrei.otcheretianski@intel.com> wrote:
> From: Avraham Stern <avraham.stern@intel.com>
>
> When the driver sends a CQM RSSI threshold event, wpa_supplicant
> queries the driver for the signal and noise values. However, it
> is possible that by that time the station has already disconnected
> from the AP, so these values are no longer valid. In this case,
> indicate that these values are invalid by setting them to
> WPA_INVALID_NOISE.
> Previously a value of 0 would be reported, which may be confusing as
> this is a valid value.
> Since nl80211_get_link_signal() and nl80211_get_link_noise() already
> set invalid values for a case of failure, just use the value set by
> these functions even if they fail.
>
> Signed-off-by: Avraham Stern <avraham.stern@intel.com>
> Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

Thanks!

Reviewed-by: Brian Norris <briannorris@chromium.org>
Jouni Malinen Dec. 4, 2020, 2:57 p.m. UTC | #2 
On Tue, Nov 03, 2020 at 09:54:16AM +0200, Andrei Otcheretianski wrote:
> When the driver sends a CQM RSSI threshold event, wpa_supplicant
> queries the driver for the signal and noise values. However, it
> is possible that by that time the station has already disconnected
> from the AP, so these values are no longer valid. In this case,
> indicate that these values are invalid by setting them to
> WPA_INVALID_NOISE.
> Previously a value of 0 would be reported, which may be confusing as
> this is a valid value.
> Since nl80211_get_link_signal() and nl80211_get_link_noise() already
> set invalid values for a case of failure, just use the value set by
> these functions even if they fail.

Thanks, applied.
diff mbox series

Patch

diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c
index ce95e9cd39..31b2799535 100644
--- a/src/drivers/driver_nl80211_event.c
+++ b/src/drivers/driver_nl80211_event.c
@@ -1277,7 +1277,6 @@  static void nl80211_cqm_event(struct wpa_driver_nl80211_data *drv,
 	struct nlattr *cqm[NL80211_ATTR_CQM_MAX + 1];
 	enum nl80211_cqm_rssi_threshold_event event;
 	union wpa_event_data ed;
-	struct wpa_signal_info sig;
 	int res;
 
 	if (tb[NL80211_ATTR_CQM] == NULL ||
@@ -1344,20 +1343,27 @@  static void nl80211_cqm_event(struct wpa_driver_nl80211_data *drv,
 		return;
 	}
 
-	res = nl80211_get_link_signal(drv, &sig);
+	/*
+	 * nl80211_get_link_signal() and nl80211_get_link_noise() set default
+	 * values in case querying the driver fails.
+	 */
+	res = nl80211_get_link_signal(drv, &ed.signal_change);
 	if (res == 0) {
-		ed.signal_change.current_signal = sig.current_signal;
-		ed.signal_change.current_txrate = sig.current_txrate;
 		wpa_printf(MSG_DEBUG, "nl80211: Signal: %d dBm  txrate: %d",
-			   sig.current_signal, sig.current_txrate);
+			   ed.signal_change.current_signal,
+			   ed.signal_change.current_txrate);
+	} else {
+		wpa_printf(MSG_DEBUG,
+			   "nl80211: querying the driver for signal info failed");
 	}
 
-	res = nl80211_get_link_noise(drv, &sig);
-	if (res == 0) {
-		ed.signal_change.current_noise = sig.current_noise;
+	res = nl80211_get_link_noise(drv, &ed.signal_change);
+	if (res == 0)
 		wpa_printf(MSG_DEBUG, "nl80211: Noise: %d dBm",
-			   sig.current_noise);
-	}
+			   ed.signal_change.current_noise);
+	else
+		wpa_printf(MSG_DEBUG,
+			   "nl80211: querying the driver for noise info failed");
 
 	wpa_supplicant_event(drv->ctx, EVENT_SIGNAL_CHANGE, &ed);
 }
diff --git a/wpa_supplicant/bss.h b/wpa_supplicant/bss.h
index 0716761749..2cbe86cb5b 100644
--- a/wpa_supplicant/bss.h
+++ b/wpa_supplicant/bss.h
@@ -169,7 +169,7 @@  static inline int bss_is_pbss(struct wpa_bss *bss)
 
 static inline void wpa_bss_update_level(struct wpa_bss *bss, int new_level)
 {
-	if (bss != NULL && new_level < 0)
+	if (bss != NULL && new_level > -WPA_INVALID_NOISE && new_level < 0)
 		bss->level = new_level;
 }