Re: Missing link quality with wireless-testing

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2009-02-18 at 07:18 -0500, Dan Williams wrote:

[snipped explanation]

> Anyone think all this stuff really, really sucks?  Yes!!!  So lets just
> have drivers/stack provide a few sane values that userspace really
> doesn't have to go through this shit to calculate...
> 
> <rant ends>
> 
> NM is probably fine here with qual == 0 because I doubt the GIWRANGE
> handler is returning a valid max_qual.qual > 0 anymore with Johannes'
> patch.  Could be wrong though.

We're actually getting _two_ things wrong now and nobody ever noticed.
So much for "but someone might care about the values wext returns". But
there actually is a problem.

First, we're reporting max_qual.qual != 0 still, this should be changed.
wpa_supplicant is still reporting quality values, so the invalid flags
aren't ever set. This needs to change in mac80211 (fixing the immediate
regression) and wpa_supplicant.

HOWEVER. Since all our qual.qual values are 0 though, your code _should_
cope fine, because of this (from NM):

        /* If the quality percent was 0 or doesn't exist, then try to use signal levels instead */
        if ((percent < 1) && (level_percent >= 0))
                percent = level_percent;

Now, why doesn't it?

Because we don't report proper max_qual _level_ values. Something
clearly nobody cared about because NM was using qual.qual and not
qual.level. Here's the relevant snippet from mac80211:

        /* cfg80211 requires this, and enforces 0..100 */
        if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC)
                range->max_qual.level = 100;
        else if  (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
                range->max_qual.level = -110;
        else
                range->max_qual.level = 0;

Note the dBm branch -- clearly wrong. Did anyone care? Clearly not.


Therefore, here's what I'm going to do:
 1) always set IW_QUAL_QUAL_INVALID, even in max_qual (fixes bug #1)
 2) set max_qual.level to 0 for dBm instead of -110 (fixes bug #2)

patch below.

johannes

Subject: mac80211: fix wext max_qual report

mac80211 is reporting a max_qual.level of -110 for dBm while it
should be reporting 0. It is also reporting a valid max_qual.qual
although we no longer set qual.qual, which trips up NetworkManager
due to a wpa_supplicant bug. But we don't need to report that our
max_qual.qual value is valid.

Fix the max_qual.level to be 0 for dBm and 100 for non-dBm and
remove max_qual.qual. max_qual.noise = -110 also seems wrong,
so fix that while at it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/mac80211/wext.c |   28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

--- wireless-testing.orig/net/mac80211/wext.c	2009-02-18 14:24:56.000000000 +0100
+++ wireless-testing/net/mac80211/wext.c	2009-02-18 14:29:07.000000000 +0100
@@ -148,11 +148,19 @@ static u8 ieee80211_get_wstats_flags(str
 {
 	u8 wstats_flags = 0;
 
-	wstats_flags |= local->hw.flags & (IEEE80211_HW_SIGNAL_UNSPEC |
-					   IEEE80211_HW_SIGNAL_DBM) ?
-				IW_QUAL_QUAL_UPDATED : IW_QUAL_QUAL_INVALID;
-	wstats_flags |= local->hw.flags & IEEE80211_HW_NOISE_DBM ?
-				IW_QUAL_NOISE_UPDATED : IW_QUAL_NOISE_INVALID;
+	wstats_flags |= IW_QUAL_QUAL_INVALID;
+
+	if (local->hw.flags & (IEEE80211_HW_SIGNAL_UNSPEC |
+			       IEEE80211_HW_SIGNAL_DBM))
+		wstats_flags |= IW_QUAL_LEVEL_UPDATED;
+	else
+		wstats_flags |= IW_QUAL_LEVEL_INVALID;
+
+	if (local->hw.flags & IEEE80211_HW_NOISE_DBM)
+		wstats_flags |= IW_QUAL_NOISE_UPDATED;
+	else
+		wstats_flags |= IW_QUAL_NOISE_INVALID;
+
 	if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
 		wstats_flags |= IW_QUAL_DBM;
 
@@ -191,19 +199,11 @@ static int ieee80211_ioctl_giwrange(stru
 	if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC)
 		range->max_qual.level = 100;
 	else if  (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
-		range->max_qual.level = -110;
-	else
 		range->max_qual.level = 0;
 
-	if (local->hw.flags & IEEE80211_HW_NOISE_DBM)
-		range->max_qual.noise = -110;
-	else
-		range->max_qual.noise = 0;
-
-	range->max_qual.qual = 100;
+	range->max_qual.noise = 0;
 	range->max_qual.updated = ieee80211_get_wstats_flags(local);
 
-	range->avg_qual.qual = 50;
 	/* not always true but better than nothing */
 	range->avg_qual.level = range->max_qual.level / 2;
 	range->avg_qual.noise = range->max_qual.noise / 2;