[PATCH] improved statistics for bcm43xx-softmac

[PATCH] improved statistics for bcm43xx-softmac

[Larry Finger]

This patch improves the statistics returned from bcm43xx_get_wireless_stats. The signal level comes
from smoothing the rssi value returned by the firmware. The quality value is a hack derived from the
smoothed rssi value and an assumed rssi_max of -25. If anyone has a better value, please let me
know. The noise value is still the one calculated from the clean-room formula. On my system, this is
roughly -65 dBm, which seems too high. I would appreciate getting any ideas on what other 
interface/driver combinations get for the noise value.

Signed-Off-By: Larry Finger <Larry.Finger@lwfinger.net>

==========================================================

diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
index af97755..c80fdcd 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -1581,17 +1581,8 @@ static void handle_irq_noise(struct bcm4
		else
			average -= 48;

-/* FIXME: This is wrong, but people want fancy stats. well... */
-bcm->stats.noise = average;
-		if (average > -65)
-			bcm->stats.link_quality = 0;
-		else if (average > -75)
-			bcm->stats.link_quality = 1;
-		else if (average > -85)
-			bcm->stats.link_quality = 2;
-		else
-			bcm->stats.link_quality = 3;
-//		dprintk(KERN_INFO PFX "Link Quality: %u (avg was %d)\n", bcm->stats.link_quality, average);
+/* FIXME: This matches the formula from the clean-room, but yields a value that is probably too
large. */
+		bcm->stats.noise = average;
  drop_calculation:
		bcm->noisecalc.calculation_running = 0;
		return;
diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
index 5c36e29..8224da1 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
@@ -47,6 +47,8 @@ #include "bcm43xx_phy.h"
  #define BCM43xx_WX_VERSION	18

  #define MAX_WX_STRING		80
+/* FIXME: the next line is a guess as to what the maximum value of rssi might be */
+#define RSSI_MAX		-25


  static int bcm43xx_wx_get_name(struct net_device *net_dev,
@@ -227,15 +229,15 @@ static int bcm43xx_wx_get_rangeparams(st

	range->max_qual.qual = 100;
	/* TODO: Real max RSSI */
-	range->max_qual.level = 3;
-	range->max_qual.noise = 100;
-	range->max_qual.updated = 7;
-
-	range->avg_qual.qual = 70;
-	range->avg_qual.level = 2;
-	range->avg_qual.noise = 40;
-	range->avg_qual.updated = 7;
-
+	range->max_qual.level = -100;
+	range->max_qual.noise = -100;
+	range->max_qual.updated = IW_QUAL_ALL_UPDATED;
+
+	range->avg_qual.qual = 50;
+	range->avg_qual.level = -40;
+	range->avg_qual.noise = -65;
+	range->avg_qual.updated = IW_QUAL_ALL_UPDATED;
+
	range->min_rts = BCM43xx_MIN_RTS_THRESHOLD;
	range->max_rts = BCM43xx_MAX_RTS_THRESHOLD;
	range->min_frag = MIN_FRAG_THRESHOLD;
@@ -827,6 +829,8 @@ static struct iw_statistics *bcm43xx_get
	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
	struct ieee80211softmac_device *mac = ieee80211_priv(net_dev);
	struct iw_statistics *wstats;
+	struct ieee80211_network *network = NULL;
+	static int tmp_level = 0;

	wstats = &bcm->stats.wstats;
	if (!mac->associated) {
@@ -849,11 +853,19 @@ static struct iw_statistics *bcm43xx_get
		return wstats;
	}
	/* fill in the real statistics when iface associated */
-	wstats->qual.qual = 100;     // TODO: get the real signal quality
-	wstats->qual.level = 3 - bcm->stats.link_quality;
+	list_for_each_entry(network, &mac->ieee->network_list, list) {
+		if (!memcmp(mac->associnfo.bssid, network->bssid, ETH_ALEN)) {
+			if (!tmp_level)		/* get initial value */
+				tmp_level = network->stats.rssi;
+			else			/* smooth results */
+				tmp_level = (7 * tmp_level + network->stats.rssi)/8;
+			break;
+		}
+	}
+	wstats->qual.level = tmp_level;
+	wstats->qual.qual = 100 + tmp_level - RSSI_MAX; // TODO: get the real signal quality
	wstats->qual.noise = bcm->stats.noise;
-	wstats->qual.updated = IW_QUAL_QUAL_UPDATED | IW_QUAL_LEVEL_UPDATED |
-		IW_QUAL_NOISE_UPDATED;
+	wstats->qual.updated = IW_QUAL_ALL_UPDATED;
	wstats->discard.code = bcm->ieee->ieee_stats.rx_discards_undecryptable;
	wstats->discard.retries = bcm->ieee->ieee_stats.tx_retry_limit_exceeded;
	wstats->discard.nwid = bcm->ieee->ieee_stats.tx_discards_wrong_sa;

==================

Re: [PATCH] improved statistics for bcm43xx-softmac

[Michael Buesch]

On Thursday 29 June 2006 06:08, Larry Finger wrote:
> This patch improves the statistics returned from bcm43xx_get_wireless_stats. The signal level comes
> from smoothing the rssi value returned by the firmware. The quality value is a hack derived from the
> smoothed rssi value and an assumed rssi_max of -25. If anyone has a better value, please let me
> know. The noise value is still the one calculated from the clean-room formula. On my system, this is
> roughly -65 dBm, which seems too high. I would appreciate getting any ideas on what other 
> interface/driver combinations get for the noise value.
> 
> Signed-Off-By: Larry Finger <Larry.Finger@lwfinger.net>
> 
> ==========================================================
> 
> diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> index af97755..c80fdcd 100644
> --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> @@ -1581,17 +1581,8 @@ static void handle_irq_noise(struct bcm4
> 		else
> 			average -= 48;
> 
> -/* FIXME: This is wrong, but people want fancy stats. well... */
> -bcm->stats.noise = average;
> -		if (average > -65)
> -			bcm->stats.link_quality = 0;
> -		else if (average > -75)
> -			bcm->stats.link_quality = 1;
> -		else if (average > -85)
> -			bcm->stats.link_quality = 2;
> -		else
> -			bcm->stats.link_quality = 3;
> -//		dprintk(KERN_INFO PFX "Link Quality: %u (avg was %d)\n", bcm->stats.link_quality, average);
> +/* FIXME: This matches the formula from the clean-room, but yields a value that is probably too
> large. */
> +		bcm->stats.noise = average;
>   drop_calculation:
> 		bcm->noisecalc.calculation_running = 0;
> 		return;
> diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> index 5c36e29..8224da1 100644
> --- a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> +++ b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> @@ -47,6 +47,8 @@ #include "bcm43xx_phy.h"
>   #define BCM43xx_WX_VERSION	18
> 
>   #define MAX_WX_STRING		80
> +/* FIXME: the next line is a guess as to what the maximum value of rssi might be */
> +#define RSSI_MAX		-25
> 
> 
>   static int bcm43xx_wx_get_name(struct net_device *net_dev,
> @@ -227,15 +229,15 @@ static int bcm43xx_wx_get_rangeparams(st
> 
> 	range->max_qual.qual = 100;
> 	/* TODO: Real max RSSI */
> -	range->max_qual.level = 3;
> -	range->max_qual.noise = 100;
> -	range->max_qual.updated = 7;
> -
> -	range->avg_qual.qual = 70;
> -	range->avg_qual.level = 2;
> -	range->avg_qual.noise = 40;
> -	range->avg_qual.updated = 7;
> -
> +	range->max_qual.level = -100;
> +	range->max_qual.noise = -100;
> +	range->max_qual.updated = IW_QUAL_ALL_UPDATED;
> +
> +	range->avg_qual.qual = 50;
> +	range->avg_qual.level = -40;
> +	range->avg_qual.noise = -65;
> +	range->avg_qual.updated = IW_QUAL_ALL_UPDATED;
> +
> 	range->min_rts = BCM43xx_MIN_RTS_THRESHOLD;
> 	range->max_rts = BCM43xx_MAX_RTS_THRESHOLD;
> 	range->min_frag = MIN_FRAG_THRESHOLD;
> @@ -827,6 +829,8 @@ static struct iw_statistics *bcm43xx_get
> 	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
> 	struct ieee80211softmac_device *mac = ieee80211_priv(net_dev);
> 	struct iw_statistics *wstats;
> +	struct ieee80211_network *network = NULL;
> +	static int tmp_level = 0;
> 
> 	wstats = &bcm->stats.wstats;
> 	if (!mac->associated) {
> @@ -849,11 +853,19 @@ static struct iw_statistics *bcm43xx_get
> 		return wstats;
> 	}
> 	/* fill in the real statistics when iface associated */
> -	wstats->qual.qual = 100;     // TODO: get the real signal quality
> -	wstats->qual.level = 3 - bcm->stats.link_quality;
> +	list_for_each_entry(network, &mac->ieee->network_list, list) {
> +		if (!memcmp(mac->associnfo.bssid, network->bssid, ETH_ALEN)) {
> +			if (!tmp_level)		/* get initial value */
> +				tmp_level = network->stats.rssi;
> +			else			/* smooth results */
> +				tmp_level = (7 * tmp_level + network->stats.rssi)/8;
> +			break;
> +		}
> +	}

You need to lock the ieee80211 lock here. Otherwise it might race and crash,
if some workqueue modifies the list inbetween.

> +	wstats->qual.level = tmp_level;
> +	wstats->qual.qual = 100 + tmp_level - RSSI_MAX; // TODO: get the real signal quality
> 	wstats->qual.noise = bcm->stats.noise;
> -	wstats->qual.updated = IW_QUAL_QUAL_UPDATED | IW_QUAL_LEVEL_UPDATED |
> -		IW_QUAL_NOISE_UPDATED;
> +	wstats->qual.updated = IW_QUAL_ALL_UPDATED;
> 	wstats->discard.code = bcm->ieee->ieee_stats.rx_discards_undecryptable;
> 	wstats->discard.retries = bcm->ieee->ieee_stats.tx_retry_limit_exceeded;
> 	wstats->discard.nwid = bcm->ieee->ieee_stats.tx_discards_wrong_sa;


-- 
Greetings Michael.

Re: [PATCH] improved statistics for bcm43xx-softmac

[Larry Finger]

Michael Buesch wrote:
> On Thursday 29 June 2006 06:08, Larry Finger wrote:
>> 	/* fill in the real statistics when iface associated */
>> -	wstats->qual.qual = 100;     // TODO: get the real signal quality
>> -	wstats->qual.level = 3 - bcm->stats.link_quality;
>> +	list_for_each_entry(network, &mac->ieee->network_list, list) {
>> +		if (!memcmp(mac->associnfo.bssid, network->bssid, ETH_ALEN)) {
>> +			if (!tmp_level)		/* get initial value */
>> +				tmp_level = network->stats.rssi;
>> +			else			/* smooth results */
>> +				tmp_level = (7 * tmp_level + network->stats.rssi)/8;
>> +			break;
>> +		}
>> +	}
> 
> You need to lock the ieee80211 lock here. Otherwise it might race and crash,
> if some workqueue modifies the list inbetween.

Thanks for the comment. Is there an easier way to get to network->stats?

Larry

Re: [PATCH] improved statistics for bcm43xx-softmac

[Dan Williams]

On Wed, 2006-06-28 at 23:08 -0500, Larry Finger wrote:
> This patch improves the statistics returned from bcm43xx_get_wireless_stats. The signal level comes
> from smoothing the rssi value returned by the firmware. The quality value is a hack derived from the
> smoothed rssi value and an assumed rssi_max of -25. If anyone has a better value, please let me
> know. The noise value is still the one calculated from the clean-room formula. On my system, this is
> roughly -65 dBm, which seems too high. I would appreciate getting any ideas on what other 
> interface/driver combinations get for the noise value.

Tentative NAK, this looks somewhat wrong...  are you _really_ getting
quality values as RSSI, or as dBm?

RSSI is usually a positive integer from 0 to some larger number.  It's
an arbitrary value and the maximum will be specific to the
chipset/radio/firmware.

dBm is usually bounded by (noise floor, 0), where noise floor is a
negative number around -100.

To me, it looks like you're trying to use dBm rather than RSSI here.  In
that case, you have to set range->max_qual.level to 0, because the upper
bound of dBm is indeed 0.  If you are using negative values _anywhere_
in your code for quality, you're almost certainly using dBm, not RSSI,
or something is very wrong.

Print out the value of 'average' from handle_irq_noise() both when close
and far away from the AP.  It looks to me like the final calculations
there come out in dBm given the fact that (a) average is negative, and
(b) that average is in the range of acceptable dBm numbers.  So:

1. set your stats->max_qual.level = 0
2. set bcm->stats.link_quality = average
3. Do link quality calculation on signal and noise; you can't just be
linear here though, since a noise floor of -100 with a signal of -90 is
in reality better than 10% signal; it's actually pretty OK.  Each -1 dBm
step near the noise floor counts for more % than a -1 dBm increase near
the best signal level.  You have to weight the bottom of the scale WRT
percentage to get values that make sense.

ipw2200 for example reports -53 dBm near the AP, with noise floor of -83
dBm in a fairly spectrum-crowded area.  That translates into what
ipw2200 reports as a 75% signal, which seems a bit off, but gives you an
idea.  You'll never get near 0 dBm, since there's already 10s of dBm
loss when the signal has to cross any air at all.

Dan

> Signed-Off-By: Larry Finger <Larry.Finger@lwfinger.net>
> 
> ==========================================================
> 
> diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> index af97755..c80fdcd 100644
> --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> @@ -1581,17 +1581,8 @@ static void handle_irq_noise(struct bcm4
> 		else
> 			average -= 48;
> 
> -/* FIXME: This is wrong, but people want fancy stats. well... */
> -bcm->stats.noise = average;
> -		if (average > -65)
> -			bcm->stats.link_quality = 0;
> -		else if (average > -75)
> -			bcm->stats.link_quality = 1;
> -		else if (average > -85)
> -			bcm->stats.link_quality = 2;
> -		else
> -			bcm->stats.link_quality = 3;
> -//		dprintk(KERN_INFO PFX "Link Quality: %u (avg was %d)\n", bcm->stats.link_quality, average);
> +/* FIXME: This matches the formula from the clean-room, but yields a value that is probably too
> large. */
> +		bcm->stats.noise = average;
>   drop_calculation:
> 		bcm->noisecalc.calculation_running = 0;
> 		return;
> diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> index 5c36e29..8224da1 100644
> --- a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> +++ b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
> @@ -47,6 +47,8 @@ #include "bcm43xx_phy.h"
>   #define BCM43xx_WX_VERSION	18
> 
>   #define MAX_WX_STRING		80
> +/* FIXME: the next line is a guess as to what the maximum value of rssi might be */
> +#define RSSI_MAX		-25
> 
> 
>   static int bcm43xx_wx_get_name(struct net_device *net_dev,
> @@ -227,15 +229,15 @@ static int bcm43xx_wx_get_rangeparams(st
> 
> 	range->max_qual.qual = 100;
> 	/* TODO: Real max RSSI */
> -	range->max_qual.level = 3;
> -	range->max_qual.noise = 100;
> -	range->max_qual.updated = 7;
> -
> -	range->avg_qual.qual = 70;
> -	range->avg_qual.level = 2;
> -	range->avg_qual.noise = 40;
> -	range->avg_qual.updated = 7;
> -
> +	range->max_qual.level = -100;
> +	range->max_qual.noise = -100;
> +	range->max_qual.updated = IW_QUAL_ALL_UPDATED;
> +
> +	range->avg_qual.qual = 50;
> +	range->avg_qual.level = -40;
> +	range->avg_qual.noise = -65;
> +	range->avg_qual.updated = IW_QUAL_ALL_UPDATED;
> +
> 	range->min_rts = BCM43xx_MIN_RTS_THRESHOLD;
> 	range->max_rts = BCM43xx_MAX_RTS_THRESHOLD;
> 	range->min_frag = MIN_FRAG_THRESHOLD;
> @@ -827,6 +829,8 @@ static struct iw_statistics *bcm43xx_get
> 	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
> 	struct ieee80211softmac_device *mac = ieee80211_priv(net_dev);
> 	struct iw_statistics *wstats;
> +	struct ieee80211_network *network = NULL;
> +	static int tmp_level = 0;
> 
> 	wstats = &bcm->stats.wstats;
> 	if (!mac->associated) {
> @@ -849,11 +853,19 @@ static struct iw_statistics *bcm43xx_get
> 		return wstats;
> 	}
> 	/* fill in the real statistics when iface associated */
> -	wstats->qual.qual = 100;     // TODO: get the real signal quality
> -	wstats->qual.level = 3 - bcm->stats.link_quality;
> +	list_for_each_entry(network, &mac->ieee->network_list, list) {
> +		if (!memcmp(mac->associnfo.bssid, network->bssid, ETH_ALEN)) {
> +			if (!tmp_level)		/* get initial value */
> +				tmp_level = network->stats.rssi;
> +			else			/* smooth results */
> +				tmp_level = (7 * tmp_level + network->stats.rssi)/8;
> +			break;
> +		}
> +	}
> +	wstats->qual.level = tmp_level;
> +	wstats->qual.qual = 100 + tmp_level - RSSI_MAX; // TODO: get the real signal quality
> 	wstats->qual.noise = bcm->stats.noise;
> -	wstats->qual.updated = IW_QUAL_QUAL_UPDATED | IW_QUAL_LEVEL_UPDATED |
> -		IW_QUAL_NOISE_UPDATED;
> +	wstats->qual.updated = IW_QUAL_ALL_UPDATED;
> 	wstats->discard.code = bcm->ieee->ieee_stats.rx_discards_undecryptable;
> 	wstats->discard.retries = bcm->ieee->ieee_stats.tx_retry_limit_exceeded;
> 	wstats->discard.nwid = bcm->ieee->ieee_stats.tx_discards_wrong_sa;
> 
> ==================
> 
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] improved statistics for bcm43xx-softmac

[Larry Finger]

Dan Williams wrote:

> To me, it looks like you're trying to use dBm rather than RSSI here.  In
> that case, you have to set range->max_qual.level to 0, because the upper
> bound of dBm is indeed 0.  If you are using negative values _anywhere_
> in your code for quality, you're almost certainly using dBm, not RSSI,
> or something is very wrong.
> Print out the value of 'average' from handle_irq_noise() both when close
> and far away from the AP.  It looks to me like the final calculations
> there come out in dBm given the fact that (a) average is negative, and
> (b) that average is in the range of acceptable dBm numbers.  So:
> 
> 1. set your stats->max_qual.level = 0
> 2. set bcm->stats.link_quality = average
> 3. Do link quality calculation on signal and noise; you can't just be
> linear here though, since a noise floor of -100 with a signal of -90 is
> in reality better than 10% signal; it's actually pretty OK.  Each -1 dBm
> step near the noise floor counts for more % than a -1 dBm increase near
> the best signal level.  You have to weight the bottom of the scale WRT
> percentage to get values that make sense.

Thanks for explaining how to get dBm into these fields. The comments in include/linux/wireless.h are 
confusing.

The quantity listed in stats.rssi has been processed into a dBm value in a routine called 
bcm43xx_rssi_postprocess, which I need to mention in the comments. The calculation is more black 
magic from the clean-room group's reverse engineering of the Mips driver. I changed the name of the 
constant from RSSI_MAX to RX_POWER_MAX, which I estimate to be -10 dBm (see below).

As I move away from the AP, I get the following results:

Dist.	"rssi"	"average"	Link Quality	Notes

.3 m	-14	-64		96/100		Nearly on top of AP
  2 m	-27	-62		83/100		Direct line of sight
15 m	-55	-65		55/100		Bounce down hallway and/or through wall
20 m	-66	-66		44/100		Like 15m point, but through plate glass window
						 - 35% packet loss with ping

The "Link Quality" above comes from 100 - "rssi" - RX_POWER_MAX, where RX_POWER_MAX is -10.

As "average" does not change, clearly the link quality cannot be derived from that value. It seems 
to me that the processed value in the stats.rssi field is not too bad an estimate of the qual.level 
(in dBm). The value from "average" is also not a good estimate of the noise, but it is the best I 
have at the moment. Clearly, the reported value of qual.qual (Link quality) needs to be adjusted

> ipw2200 for example reports -53 dBm near the AP, with noise floor of -83
> dBm in a fairly spectrum-crowded area.  That translates into what
> ipw2200 reports as a 75% signal, which seems a bit off, but gives you an
> idea.  You'll never get near 0 dBm, since there's already 10s of dBm
> loss when the signal has to cross any air at all.

I'm now getting a similar result with an 85% signal at 2 m. The main difference is that both signal 
and noise seem to be shifted up by about 20 dBm. As these are logarithmic quantities, S/N is 
preserved through this offset. My feeling is that these statistics have no absolute meaning, but are 
good as relative indicators. When the left-most bar in the Wireless Network Information applet in 
the KDE taskbar changes from green to yellow, I know that the signal is degraded.

Thanks for your comments. They made me dig a lot deeper into the numbers.

Larry