Signal quality strange since commit 708c57cf1709fb95

Signal quality strange since commit 708c57cf1709fb95

[Larry Finger]

I admit that I didn't follow the discussion of reporting signal quality very
closely; however since commit 708c57cf1709fb95, all of my wireless devices (b43,
p54usb, rtl8187) are reporting link quality as XX/70 rather than XX/100. I doubt
that this was an intended consequence of that patch. If the SIOCGIWRANGE ioctl
is now more correct, what changes are needed in the drivers to restore the old
behavior? Having written the qual calculation for 2 drivers, I understand the
uselessness of this number, but I also know the uproar such a change will cause
in the user community.

Larry

Re: Signal quality strange since commit 708c57cf1709fb95

[Dan Williams]

On Sat, 2009-02-21 at 09:09 -0600, Larry Finger wrote:
> I admit that I didn't follow the discussion of reporting signal quality very
> closely; however since commit 708c57cf1709fb95, all of my wireless devices (b43,
> p54usb, rtl8187) are reporting link quality as XX/70 rather than XX/100. I doubt
> that this was an intended consequence of that patch. If the SIOCGIWRANGE ioctl
> is now more correct, what changes are needed in the drivers to restore the old
> behavior? Having written the qual calculation for 2 drivers, I understand the
> uselessness of this number, but I also know the uproar such a change will cause
> in the user community.

Intended.  It doesn't matter what the /X number is; it's simply the
upper bound of what the # before the / will be.  Quality is in the range
of [0 ... max_qual.qual].  You should be able to divide quality /
max_qual.qual and get a reasonable number to use for pretty UI display.

Many drivers used to use something other than 100 for max quality, and
many still do:

airo.c:		range->max_qual.qual = airo_get_max_quality(&cap_rid);
hostap/hostap_ioctl.c:		range->max_qual.qual = 70; /* what is correct max? This was not
hostap/hostap_ioctl.c:		range->max_qual.qual = 92; /* 0 .. 92 */
netwave_cs.c:	range->max_qual.qual = 255;
orinoco/wext.c:		range->max_qual.qual = 0x8b - 0x2f;
wavelan.c:	range->max_qual.qual = MMR_SGNL_QUAL;
wavelan_cs.c:	range->max_qual.qual = MMR_SGNL_QUAL;
zd1201.c:	range->max_qual.qual = 128;

Dan

Re: Signal quality strange since commit 708c57cf1709fb95

[Larry Finger]

Dan Williams wrote:
> On Sat, 2009-02-21 at 09:09 -0600, Larry Finger wrote:
>> I admit that I didn't follow the discussion of reporting signal quality very
>> closely; however since commit 708c57cf1709fb95, all of my wireless devices (b43,
>> p54usb, rtl8187) are reporting link quality as XX/70 rather than XX/100. I doubt
>> that this was an intended consequence of that patch. If the SIOCGIWRANGE ioctl
>> is now more correct, what changes are needed in the drivers to restore the old
>> behavior? Having written the qual calculation for 2 drivers, I understand the
>> uselessness of this number, but I also know the uproar such a change will cause
>> in the user community.
> 
> Intended.  It doesn't matter what the /X number is; it's simply the
> upper bound of what the # before the / will be.  Quality is in the range
> of [0 ... max_qual.qual].  You should be able to divide quality /
> max_qual.qual and get a reasonable number to use for pretty UI display.
> 
> Many drivers used to use something other than 100 for max quality, and
> many still do:
> 
> airo.c:		range->max_qual.qual = airo_get_max_quality(&cap_rid);
> hostap/hostap_ioctl.c:		range->max_qual.qual = 70; /* what is correct max? This was not
> hostap/hostap_ioctl.c:		range->max_qual.qual = 92; /* 0 .. 92 */
> netwave_cs.c:	range->max_qual.qual = 255;
> orinoco/wext.c:		range->max_qual.qual = 0x8b - 0x2f;
> wavelan.c:	range->max_qual.qual = MMR_SGNL_QUAL;
> wavelan_cs.c:	range->max_qual.qual = MMR_SGNL_QUAL;
> zd1201.c:	range->max_qual.qual = 128;

I know how it works with drivers that implement their own ioctl calls for the
WEXT interface; however, the 3 that I mentioned all use mac80211 and get their
WEXT support through cfg80211.

Larry

Re: Signal quality strange since commit 708c57cf1709fb95

[Kalle Valo]

Larry Finger <Larry.Finger@lwfinger.net> writes:

> I know how it works with drivers that implement their own ioctl
> calls for the WEXT interface; however, the 3 that I mentioned all
> use mac80211 and get their WEXT support through cfg80211.

Yes, something is wrong now. I noticed the same problem (/70 in signal
strength) when I was implementing rssi support for at76c50x-usb.

-- 
Kalle Valo

Re: Signal quality strange since commit 708c57cf1709fb95

[Dan Williams]

On Sun, 2009-02-22 at 08:39 +0200, Kalle Valo wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
> 
> > I know how it works with drivers that implement their own ioctl
> > calls for the WEXT interface; however, the 3 that I mentioned all
> > use mac80211 and get their WEXT support through cfg80211.
> 
> Yes, something is wrong now. I noticed the same problem (/70 in signal
> strength) when I was implementing rssi support for at76c50x-usb.

The /70 *by itself* is not a problem.  There could be a problem with the
quality calculation that cfg80211/mac80211 are doing with the reported
signal strength from the hardware-specific driver.

Dan

Re: Signal quality strange since commit 708c57cf1709fb95

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

On Sun, 2009-02-22 at 08:39 +0200, Kalle Valo wrote:
> Larry Finger <Larry.Finger@lwfinger.net> writes:
> 
> > I know how it works with drivers that implement their own ioctl
> > calls for the WEXT interface; however, the 3 that I mentioned all
> > use mac80211 and get their WEXT support through cfg80211.
> 
> Yes, something is wrong now. I noticed the same problem (/70 in signal
> strength) when I was implementing rssi support for at76c50x-usb.

Ok, so what exactly is the problem here?

I intentionally changed it to be /70 rather than /100 so I didn't have
to do *100/70 in kernel space, and tools should cope with that (and
users too); is there a problem with how the XX (XX/70) gets calculated?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Signal quality strange since commit 708c57cf1709fb95

[Larry Finger]

Johannes Berg wrote:
> On Sun, 2009-02-22 at 08:39 +0200, Kalle Valo wrote:
>> Larry Finger <Larry.Finger@lwfinger.net> writes:
>>
>>> I know how it works with drivers that implement their own ioctl
>>> calls for the WEXT interface; however, the 3 that I mentioned all
>>> use mac80211 and get their WEXT support through cfg80211.
>> Yes, something is wrong now. I noticed the same problem (/70 in signal
>> strength) when I was implementing rssi support for at76c50x-usb.
> 
> Ok, so what exactly is the problem here?
> 
> I intentionally changed it to be /70 rather than /100 so I didn't have
> to do *100/70 in kernel space, and tools should cope with that (and
> users too); is there a problem with how the XX (XX/70) gets calculated?

No, I just wondered if it was intentional that it was changed to /70. It appears
that it was, therefore, at least 3 drivers must be changed as they are
calculating on the basis of /100. For example, my b43 is showing an iwconfig
Link Quality of 93/70 while the KNM applet is showing 50%.

Larry

Re: Signal quality strange since commit 708c57cf1709fb95

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On Mon, 2009-02-23 at 22:18 -0600, Larry Finger wrote:

> No, I just wondered if it was intentional that it was changed to /70. It appears
> that it was, therefore, at least 3 drivers must be changed as they are
> calculating on the basis of /100. For example, my b43 is showing an iwconfig
> Link Quality of 93/70 while the KNM applet is showing 50%.

Well that _is_ a bug, since this shouldn't have required driver changes!
I'll take a look into why that might be happening.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Signal quality strange since commit 708c57cf1709fb95

[Larry Finger]

Johannes Berg wrote:
> On Mon, 2009-02-23 at 22:18 -0600, Larry Finger wrote:
> 
>> No, I just wondered if it was intentional that it was changed to /70. It appears
>> that it was, therefore, at least 3 drivers must be changed as they are
>> calculating on the basis of /100. For example, my b43 is showing an iwconfig
>> Link Quality of 93/70 while the KNM applet is showing 50%.
> 
> Well that _is_ a bug, since this shouldn't have required driver changes!
> I'll take a look into why that might be happening.

Drivers b43, b43legacy, rtl8187, and p54usb all assume a Link Quality scaled to
100, which was the behavior before the commit in the subject. I don't quite
understand why as the old code had this fragment:

-       if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM) {
-               /*
-                * cfg80211 assumes -110 to -40 dBm and clamps to that range
-                * for qual.qual, so tell userspace this is what we give it
-                * but take into account that we have to start from 0.
-                */
-               range->max_qual.qual = 70;
-               range->avg_qual.qual = 35;
-       } else {
-               /*
-                * cfg80211 just uses the level value for qual too, and it
-                * requires the level value to be 0 .. 100.
-                */
-               range->max_qual.qual = 100;
-               range->avg_qual.qual = 50;
-       }

That should have set max_qual.qual to 70 as all the drivers set
IEEE80211_HW_SIGNAL_DBM. I don't know why, but the code took the other branch,
which seems to have been some other bug.

Larry

Re: Signal quality strange since commit 708c57cf1709fb95

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On Tue, 2009-02-24 at 01:07 -0600, Larry Finger wrote:
> Johannes Berg wrote:
> > On Mon, 2009-02-23 at 22:18 -0600, Larry Finger wrote:
> > 
> >> No, I just wondered if it was intentional that it was changed to /70. It appears
> >> that it was, therefore, at least 3 drivers must be changed as they are
> >> calculating on the basis of /100. For example, my b43 is showing an iwconfig
> >> Link Quality of 93/70 while the KNM applet is showing 50%.
> > 
> > Well that _is_ a bug, since this shouldn't have required driver changes!
> > I'll take a look into why that might be happening.
> 
> Drivers b43, b43legacy, rtl8187, and p54usb all assume a Link Quality scaled to
> 100, which was the behavior before the commit in the subject. I don't quite
> understand why as the old code had this fragment:

Ahh. I see now, you're talking wireless_stats and I'm thinking scan
results. Yeah, I see what's going on, I'll fix it tomorrow.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Signal quality strange since commit 708c57cf1709fb95

[Johannes Berg]

On Mon, 2009-02-23 at 23:13 -0800, Johannes Berg wrote:

> > >> No, I just wondered if it was intentional that it was changed to /70. It appears
> > >> that it was, therefore, at least 3 drivers must be changed as they are
> > >> calculating on the basis of /100. For example, my b43 is showing an iwconfig
> > >> Link Quality of 93/70 while the KNM applet is showing 50%.
> > > 
> > > Well that _is_ a bug, since this shouldn't have required driver changes!
> > > I'll take a look into why that might be happening.
> > 
> > Drivers b43, b43legacy, rtl8187, and p54usb all assume a Link Quality scaled to
> > 100, which was the behavior before the commit in the subject. I don't quite
> > understand why as the old code had this fragment:
> 
> Ahh. I see now, you're talking wireless_stats and I'm thinking scan
> results. Yeah, I see what's going on, I'll fix it tomorrow.

Ok so tomorrow wasn't it, but I think I've fixed it. I didn't test it
because I'm sitting on an airplane from Portland to DC right now.

Since I'll be gone for a week once you read this email, I would
appreciate if you could submit this patch if it works (or if it doesn't
and you can fix it), adding appropriate tested-by tags, and maybe adding
some more of the information that you had to the commit log.

johannes


From: Johannes Berg <johannes@sipsolutions.net>

Fix mac80211 quality repoorting for wireless stats.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Reported-by: Larry Finger <larry.finger@lwfinger.net>
---
 net/mac80211/wext.c |   58 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 19 deletions(-)

--- wireless-testing.orig/net/mac80211/wext.c	2009-02-27 18:28:24.000000000 +0100
+++ wireless-testing/net/mac80211/wext.c	2009-02-27 18:45:17.000000000 +0100
@@ -886,21 +886,6 @@ static int ieee80211_ioctl_siwauth(struc
 	return ret;
 }
 
-static u8 ieee80211_get_wstats_flags(struct ieee80211_local *local)
-{
-	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;
-	if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
-		wstats_flags |= IW_QUAL_DBM;
-
-	return wstats_flags;
-}
-
 /* Get wireless statistics.  Called by /proc/net/wireless and by SIOCGIWSTATS */
 static struct iw_statistics *ieee80211_get_wireless_stats(struct net_device *dev)
 {
@@ -922,10 +907,45 @@ static struct iw_statistics *ieee80211_g
 		wstats->qual.noise = 0;
 		wstats->qual.updated = IW_QUAL_ALL_INVALID;
 	} else {
-		wstats->qual.level = sta->last_signal;
-		wstats->qual.qual = sta->last_qual;
-		wstats->qual.noise = sta->last_noise;
-		wstats->qual.updated = ieee80211_get_wstats_flags(local);
+		wstats->qual.updated = 0;
+		/*
+		 * mirror what cfg80211 does for iwrange/scan results,
+		 * otherwise userspace gets confused.
+		 */
+		if (local->hw.flags & (IEEE80211_HW_SIGNAL_UNSPEC |
+				       IEEE80211_HW_SIGNAL_DBM)) {
+			wstats->qual.updated |= IW_QUAL_LEVEL_UPDATED;
+			wstats->qual.updated |= IW_QUAL_QUAL_UPDATED;
+		} else {
+			wstats->qual.updated |= IW_QUAL_LEVEL_INVALID;
+			wstats->qual.updated |= IW_QUAL_QUAL_INVALID;
+		}
+
+		if (local->hw.flags & IEEE80211_HW_SIGNAL_UNSPEC) {
+			wstats->qual.level = sta->last_signal;
+			wstats->qual.qual = sta->last_signal;
+		} else if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM) {
+			int sig = sta->last_signal;
+
+			wstats->qual.updated |= IW_QUAL_DBM;
+			wstats->qual.level = sig;
+			if (sig < -110)
+				sig = -110;
+			else if (sig > -40)
+				sig = -40;
+			wstats->qual.qual = sig + 110;
+		}
+
+		if (local->hw.flags & IEEE80211_HW_NOISE_DBM) {
+			/*
+			 * This assumes that if driver reports noise, it also
+			 * reports signal in dBm.
+			 */
+			wstats->qual.noise = sta->last_noise;
+			wstats->qual.updated |= IW_QUAL_NOISE_UPDATED;
+		} else {
+			wstats->qual.updated |= IW_QUAL_NOISE_INVALID;
+		}
 	}
 
 	rcu_read_unlock();

Re: Signal quality strange since commit 708c57cf1709fb95

[Larry Finger]

Johannes Berg wrote:
> 
> Ok so tomorrow wasn't it, but I think I've fixed it. I didn't test it
> because I'm sitting on an airplane from Portland to DC right now.
> 
> Since I'll be gone for a week once you read this email, I would
> appreciate if you could submit this patch if it works (or if it doesn't
> and you can fix it), adding appropriate tested-by tags, and maybe adding
> some more of the information that you had to the commit log.

I'll test it and submit the patch. You will have crossed my longitude about
halfway through you flight. I hope it was a good one.

Larry

Re: Signal quality strange since commit 708c57cf1709fb95

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On Fri, 2009-02-27 at 15:24 -0600, Larry Finger wrote:

> I'll test it and submit the patch. You will have crossed my longitude about
> halfway through you flight. I hope it was a good one.

Thanks. Yeah, flight was good. Now boarding to FRA. I might check email
once more in FRA, but after that you won't hear from me for a week, most
likely.

Go wild in wireless! :P

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Signal quality strange since commit 708c57cf1709fb95

[Dan Williams]

On Sat, 2009-02-21 at 14:04 -0600, Larry Finger wrote:
> Dan Williams wrote:
> > On Sat, 2009-02-21 at 09:09 -0600, Larry Finger wrote:
> >> I admit that I didn't follow the discussion of reporting signal quality very
> >> closely; however since commit 708c57cf1709fb95, all of my wireless devices (b43,
> >> p54usb, rtl8187) are reporting link quality as XX/70 rather than XX/100. I doubt
> >> that this was an intended consequence of that patch. If the SIOCGIWRANGE ioctl
> >> is now more correct, what changes are needed in the drivers to restore the old
> >> behavior? Having written the qual calculation for 2 drivers, I understand the
> >> uselessness of this number, but I also know the uproar such a change will cause
> >> in the user community.
> > 
> > Intended.  It doesn't matter what the /X number is; it's simply the
> > upper bound of what the # before the / will be.  Quality is in the range
> > of [0 ... max_qual.qual].  You should be able to divide quality /
> > max_qual.qual and get a reasonable number to use for pretty UI display.
> > 
> > Many drivers used to use something other than 100 for max quality, and
> > many still do:
> > 
> > airo.c:		range->max_qual.qual = airo_get_max_quality(&cap_rid);
> > hostap/hostap_ioctl.c:		range->max_qual.qual = 70; /* what is correct max? This was not
> > hostap/hostap_ioctl.c:		range->max_qual.qual = 92; /* 0 .. 92 */
> > netwave_cs.c:	range->max_qual.qual = 255;
> > orinoco/wext.c:		range->max_qual.qual = 0x8b - 0x2f;
> > wavelan.c:	range->max_qual.qual = MMR_SGNL_QUAL;
> > wavelan_cs.c:	range->max_qual.qual = MMR_SGNL_QUAL;
> > zd1201.c:	range->max_qual.qual = 128;
> 
> I know how it works with drivers that implement their own ioctl calls for the
> WEXT interface; however, the 3 that I mentioned all use mac80211 and get their
> WEXT support through cfg80211.

It's not really relevant what mac80211/cfg80211 uses for max_qual.qual.
What's relevant is that the calculation that the driver/stack makes for
quality is appropriate for the max_qual.qual.  Again, it doesn't matter
what the /XX number is, as long as you can take YY/XX and get something
between 0 and 100 (inclusive) that changes on a linear scale and is
usable for a generic "signal strength %" measure.

Dan