2007-04-08 06:07:26

by Larry Finger

[permalink] [raw]
Subject: [PATCH] mac80211: Report correct wireless statistics

In mac80211 the 'qual' and 'level' values are interchanged. The patch also
places the 'qual' value on a 0 - 100 scale and calculated using the formula
contained in iwlib.

Signed-off-by: Larry Finger <[email protected]>
---

John and Michael,

This fix is meant for the wireless-dev and mb git trees.

Larry

ieee80211_ioctl.c | 16 +++++++++++-----
ieee80211_sta.c | 9 +++++++--
2 files changed, 18 insertions(+), 7 deletions(-)

Index: wireless-mb/net/mac80211/ieee80211_sta.c
===================================================================
--- wireless-mb.orig/net/mac80211/ieee80211_sta.c
+++ wireless-mb/net/mac80211/ieee80211_sta.c
@@ -2822,8 +2822,10 @@ ieee80211_sta_scan_result(struct net_dev

memset(&iwe, 0, sizeof(iwe));
iwe.cmd = IWEVQUAL;
- iwe.u.qual.qual = bss->signal;
- iwe.u.qual.level = bss->rssi;
+ iwe.u.qual.level = bss->signal;
+ if (unlikely(local->hw.max_rssi == 0))
+ local->hw.max_rssi = 100;
+ iwe.u.qual.qual = (100 * bss->rssi) / local->hw.max_rssi;
iwe.u.qual.noise = bss->noise;
iwe.u.qual.updated = local->wstats_flags;
current_ev = iwe_stream_add_event(current_ev, end_buf, &iwe,
Index: wireless-mb/net/mac80211/ieee80211_ioctl.c
===================================================================
--- wireless-mb.orig/net/mac80211/ieee80211_ioctl.c
+++ wireless-mb/net/mac80211/ieee80211_ioctl.c
@@ -1593,12 +1593,12 @@ static int ieee80211_ioctl_giwrange(stru
range->min_frag = 256;
range->max_frag = 2346;

- range->max_qual.qual = local->hw.max_signal;
- range->max_qual.level = local->hw.max_rssi;
+ range->max_qual.qual = 100;
+ range->max_qual.level = local->hw.max_signal;
range->max_qual.noise = local->hw.max_noise;
range->max_qual.updated = local->wstats_flags;

- range->avg_qual.qual = local->hw.max_signal/2;
+ range->avg_qual.qual = 50;
range->avg_qual.level = 0;
range->avg_qual.noise = 0;
range->avg_qual.updated = local->wstats_flags;
@@ -3087,8 +3087,10 @@ static struct iw_statistics *ieee80211_g
wstats->qual.noise = 0;
wstats->qual.updated = IW_QUAL_ALL_INVALID;
} else {
- wstats->qual.level = sta->last_rssi;
- wstats->qual.qual = sta->last_signal;
+ if (unlikely(local->hw.max_rssi == 0))
+ local->hw.max_rssi = 100;
+ wstats->qual.qual = (100 * sta->last_rssi) / local->hw.max_rssi;
+ wstats->qual.level = sta->last_signal;
wstats->qual.noise = sta->last_noise;
wstats->qual.updated = local->wstats_flags;
sta_info_put(sta);


2007-04-09 03:53:58

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> On Sunday 08 April 2007 20:02, Larry Finger wrote:
>> Link Quality=219/60 Signal level=-200 dBm Noise level=-69 dBm
>>
> Try the attached patch without your "bcm43xx-mac80211: Fix error in
> initiallizing max RSSI and max signal" patch.
>
> -Michael Wu

Why would I want to do this? If the community agrees on anything, it is that the signal is given in
dBm (i.e. a negative number) and that the rssi is a positive number. The firmware in the bcm43xx
chips return a quantity that looks like an rssi with a received packet, and bcm43xx_rssi_postprocess
turns that into a quantity that looks like dBm. Your patch reverses those designations and mixes up
the two quantities. Again I ask "Why"?

Larry


2007-04-09 12:04:10

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Mon, 2007-04-09 at 00:43 -0400, Michael Wu wrote:
> On Sunday 08 April 2007 23:54, Larry Finger wrote:
> > Why would I want to do this?
> Did it fix the output?
>
> > If the community agrees on anything, it is
> > that the signal is given in dBm (i.e. a negative number) and that the rssi
> > is a positive number.
> Nope. dBm doesn't have to be negative, though it often is since most wireless
> hardware isn't that powerful. RSSI is simply a number that's bigger for
> stronger signals. It could be dBm, but it doesn't have to be. If you want a
> stronger definition of RSSI, look at RCPI.
>
> > The firmware in the bcm43xx chips return a quantity
> > that looks like an rssi with a received packet, and
> > bcm43xx_rssi_postprocess turns that into a quantity that looks like dBm.
> > Your patch reverses those designations and mixes up the two quantities.
> > Again I ask "Why"?
> >
> Because of the naming/use of the statistics in mac80211 and WE. Signal ends up
> getting assigned to (struct iw_quality).qual, which is actually just an
> arbitrary link quality indicator, not dBm. Anything you care about can be put
> there. (r)ssi gets assigned to (struct iw_quality).level, which is RSSI. WE
> allows that and noise to be specified in either arbitrary units or dBm or
> RCPI.
>
> Yes, I did reverse your conventions, but it makes more sense this way. (R)SSI
> is always valid to assign to (struct iw_quality).level and signal ((struct
> iw_quality).qual) is quite arbitrary and cannot be specified in specific
> units.
>
> Signal should be probably be renamed to qual to make it more clear that it is
> arbitrary.

In WE, qual is arbitrary within a few limits:

a) qual _must_ change on a linear scale
b) a valid max_qual.qual must be set
c) qual must fall within the bounds of [0, max_qual.qual] inclusive

If you report 'level' in dBm, you must set the IW_QUAL_DBM flag.
Otherwise, 'level' _may_ be assumed to be RSSI. If 'level' is dBm,
max_qual.level must be 0. If 'level' is RSSI, max_qual.level must be
greater than 0, and level must fall within the bounds of [0,
max_qual.level] inclusive. Replace 'level' with 'noise' here for the
rules for noise.

I don't particularly care if level/noise is RSSI _as long as_ you give
the max RSSI for your part. Different radio parts have different max
RSSI values, and if you're writing a driver you sure better know them or
figure some reasonable ones out by experimentation. RSSI is entirely
vendor defined and does _not_ conform to any rules. Therefore we need
the max RSSI to get usable signal strength reports from your part.

I know that 0 dBm isn't actually the upper bound, but in practice most
people aren't going to get parts that go above that. 0 dBm should be
considered a _limitation_ of WEXT that we obviously fix with
cfg80211/nl80211 when we bring some sanity to signal strength reporting.

Again, if you report level in RSSI, you must provide the max RSSI for
your part in max_qual.level.

Dan



2007-04-09 00:01:54

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> On Sunday 08 April 2007 19:32, Larry Finger wrote:
>> Michael Wu wrote:
>>> Is this before or after your "bcm43xx-mac80211: Fix error in
>>> initiallizing max RSSI and max signal" patch?
>>>
>> This is after that patch.
>>
> So what do the statistics look like without that patch?

Link Quality=219/60 Signal level=-200 dBm Noise level=-69 dBm

Larry

2007-04-09 15:48:38

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Dan Williams wrote:
> On Mon, 2007-04-09 at 00:43 -0400, Michael Wu wrote:
>> Yes, I did reverse your conventions, but it makes more sense this way. (R)SSI
>> is always valid to assign to (struct iw_quality).level and signal ((struct
>> iw_quality).qual) is quite arbitrary and cannot be specified in specific
>> units.
>>
>> Signal should be probably be renamed to qual to make it more clear that it is
>> arbitrary.
>
> In WE, qual is arbitrary within a few limits:
>
> a) qual _must_ change on a linear scale
> b) a valid max_qual.qual must be set
> c) qual must fall within the bounds of [0, max_qual.qual] inclusive

Note that the quantity supplied by the bcm43xx firmware (called jssi in the code) varies linearly
with the strength of the signal, whereas the quantity derived from it in bcm43xx_rssi_postprocess is
quasi logarithmic. The statement in a) above would argue against reversal of my conventions. What
happens in the other drivers that use mac80211?
>
> If you report 'level' in dBm, you must set the IW_QUAL_DBM flag.
> Otherwise, 'level' _may_ be assumed to be RSSI. If 'level' is dBm,
> max_qual.level must be 0. If 'level' is RSSI, max_qual.level must be
> greater than 0, and level must fall within the bounds of [0,
> max_qual.level] inclusive. Replace 'level' with 'noise' here for the
> rules for noise.
>
> I don't particularly care if level/noise is RSSI _as long as_ you give
> the max RSSI for your part. Different radio parts have different max
> RSSI values, and if you're writing a driver you sure better know them or
> figure some reasonable ones out by experimentation. RSSI is entirely
> vendor defined and does _not_ conform to any rules. Therefore we need
> the max RSSI to get usable signal strength reports from your part.
>
> I know that 0 dBm isn't actually the upper bound, but in practice most
> people aren't going to get parts that go above that. 0 dBm should be
> considered a _limitation_ of WEXT that we obviously fix with
> cfg80211/nl80211 when we bring some sanity to signal strength reporting.

If the IW_QUAL_DBM flag is set, the "maximum" for that quantity is actually interpreted as a minimum
and the range is interpreted as [maximum..0] using s8 arithmetic.

> Again, if you report level in RSSI, you must provide the max RSSI for
> your part in max_qual.level.

Agreed.

Larry


2007-04-09 00:34:28

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Sunday 08 April 2007 20:02, Larry Finger wrote:
> Link Quality=219/60 Signal level=-200 dBm Noise level=-69 dBm
>
Try the attached patch without your "bcm43xx-mac80211: Fix error in
initiallizing max RSSI and max signal" patch.

-Michael Wu


Attachments:
(No filename) (0.00 B)
(No filename) (189.00 B)
Download all attachments

2007-04-09 23:04:52

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Monday 09 April 2007 17:12, Larry Finger wrote:
> The code is internally consistent. Now it is only the usage of the (r)ssi
> name to describe the logarithmic numbers that is confusing. Would you
> entertain a patch that changed (r)ssi to level in the structs and drivers
> to reduce the confusion? I have such a patch that works with
> bcm43xx-mac80211. In addition, all other drivers compile.
>
Absolutely - it is confusing since dBm can also be specified in addition to
RSSI. Renaming signal to qual or something along those lines would also be
useful IMHO, but if you're okay with it as it stands, I don't mind it.

-Michael Wu


Attachments:
(No filename) (637.00 B)
(No filename) (189.00 B)
Download all attachments

2007-04-10 00:58:07

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
>>
> Absolutely - it is confusing since dBm can also be specified in addition to
> RSSI. Renaming signal to qual or something along those lines would also be
> useful IMHO, but if you're okay with it as it stands, I don't mind it.

I changed signal to qual and (r)ssi to level. Once these changes were made, iwconfig gives me the
following:

Link Quality=57/100 Signal level=-37 dBm Noise level=-70 dBm

The patch is pretty large and touches a lot of files, but if one is to be able to bisect around it,
it has to be a single commit. As the patch entitled "[PATCH] bcm43xx-mac80211: Fix error in
initiallizing max RSSI and max signal" has been accepted into Michael Buesch's tree, I have assumed
that it is already included.

Larry
---

Index: wireless-dev/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_main.c
+++ wireless-dev/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_main.c
@@ -3760,7 +3760,7 @@ static int bcm43xx_wireless_init(struct
IEEE80211_HW_MONITOR_DURING_OPER |
IEEE80211_HW_DEVICE_HIDES_WEP |
IEEE80211_HW_WEP_INCLUDE_IV;
- hw->max_signal = -110;
+ hw->max_qual = 100;
hw->max_rssi = BCM43xx_RX_MAX_SSI;
hw->max_noise = -110;
hw->queues = 1;
Index: wireless-dev/include/net/mac80211.h
===================================================================
--- wireless-dev.orig/include/net/mac80211.h
+++ wireless-dev/include/net/mac80211.h
@@ -227,8 +227,8 @@ struct ieee80211_rx_status {
int freq; /* receive frequency in Mhz */
int channel;
int phymode;
- int ssi;
- int signal; /* used as qual in statistics reporting */
+ int level; /* used in statistics reporting */
+ int qual; /* used in statistics reporting */
int noise;
int antenna;
int rate;
@@ -540,7 +540,7 @@ struct ieee80211_hw {
/* Maximum values for various statistics.
* Leave at 0 to indicate no support. Use negative numbers for dBm. */
s8 max_rssi;
- s8 max_signal;
+ s8 max_qual;
s8 max_noise;

/* Number of available hardware TX queues for data packets.
Index: wireless-dev/net/mac80211/ieee80211.c
===================================================================
--- wireless-dev.orig/net/mac80211/ieee80211.c
+++ wireless-dev/net/mac80211/ieee80211.c
@@ -2705,7 +2705,7 @@ ieee80211_fill_frame_info(struct ieee802
fi->antenna = htonl(status->antenna);
fi->priority = 0xffffffff; /* no clue */
fi->ssi_type = htonl(ieee80211_ssi_raw);
- fi->ssi_signal = htonl(status->ssi);
+ fi->ssi_signal = htonl(status->level);
fi->ssi_noise = 0x00000000;
fi->encoding = 0;
} else {
@@ -3352,10 +3352,10 @@ ieee80211_rx_h_sta_process(struct ieee80

sta->rx_fragments++;
sta->rx_bytes += rx->skb->len;
- sta->last_rssi = (sta->last_rssi * 15 +
- rx->u.rx.status->ssi) / 16;
- sta->last_signal = (sta->last_signal * 15 +
- rx->u.rx.status->signal) / 16;
+ sta->last_level = (sta->last_level * 15 +
+ rx->u.rx.status->level) / 16;
+ sta->last_qual = (sta->last_qual * 15 +
+ rx->u.rx.status->qual) / 16;
sta->last_noise = (sta->last_noise * 15 +
rx->u.rx.status->noise) / 16;

@@ -4643,7 +4643,7 @@ int ieee80211_register_hw(struct ieee802

local->wstats_flags |= local->hw.max_rssi ?
IW_QUAL_LEVEL_UPDATED : IW_QUAL_LEVEL_INVALID;
- local->wstats_flags |= local->hw.max_signal ?
+ local->wstats_flags |= local->hw.max_qual ?
IW_QUAL_QUAL_UPDATED : IW_QUAL_QUAL_INVALID;
local->wstats_flags |= local->hw.max_noise ?
IW_QUAL_NOISE_UPDATED : IW_QUAL_NOISE_INVALID;
Index: wireless-dev/net/mac80211/ieee80211_sta.c
===================================================================
--- wireless-dev.orig/net/mac80211/ieee80211_sta.c
+++ wireless-dev/net/mac80211/ieee80211_sta.c
@@ -1192,8 +1192,8 @@ static void ieee80211_rx_mgmt_assoc_resp
}
bss = ieee80211_rx_bss_get(dev, ifsta->bssid);
if (bss) {
- sta->last_rssi = bss->rssi;
- sta->last_signal = bss->signal;
+ sta->last_level = bss->level;
+ sta->last_qual = bss->qual;
sta->last_noise = bss->noise;
ieee80211_rx_bss_put(dev, bss);
}
@@ -1582,8 +1582,8 @@ static void ieee80211_rx_bss_info(struct
}
bss->timestamp = timestamp;
bss->last_update = jiffies;
- bss->rssi = rx_status->ssi;
- bss->signal = rx_status->signal;
+ bss->level = rx_status->level;
+ bss->qual = rx_status->qual;
bss->noise = rx_status->noise;
if (!beacon)
bss->probe_resp++;
@@ -2060,7 +2060,7 @@ static int ieee80211_sta_config_auth(str
struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_sta_bss *bss, *selected = NULL;
- int top_rssi = 0, freq;
+ int top_level = 0, freq;

if (!ifsta->auto_channel_sel && !ifsta->auto_bssid_sel &&
!ifsta->auto_ssid_sel) {
@@ -2090,9 +2090,9 @@ static int ieee80211_sta_config_auth(str
!ieee80211_sta_match_ssid(ifsta, bss->ssid, bss->ssid_len))
continue;

- if (top_rssi < bss->rssi) {
+ if (top_level < bss->level) {
selected = bss;
- top_rssi = bss->rssi;
+ top_level = bss->level;
}
}
if (selected)
@@ -2822,8 +2822,8 @@ ieee80211_sta_scan_result(struct net_dev

memset(&iwe, 0, sizeof(iwe));
iwe.cmd = IWEVQUAL;
- iwe.u.qual.qual = bss->signal;
- iwe.u.qual.level = bss->rssi;
+ iwe.u.qual.qual = bss->qual;
+ iwe.u.qual.level = bss->level;
iwe.u.qual.noise = bss->noise;
iwe.u.qual.updated = local->wstats_flags;
current_ev = iwe_stream_add_event(current_ev, end_buf, &iwe,
@@ -2908,7 +2908,7 @@ ieee80211_sta_scan_result(struct net_dev

memset(&iwe, 0, sizeof(iwe));
iwe.cmd = IWEVCUSTOM;
- sprintf(buf, "rssi=%d", bss->rssi);
+ sprintf(buf, "level=%d", bss->level);
iwe.u.data.length = strlen(buf);
current_ev = iwe_stream_add_point(current_ev, end_buf, &iwe,
buf);
Index: wireless-dev/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.c
+++ wireless-dev/drivers/net/wireless/mac80211/bcm43xx/bcm43xx_xmit.c
@@ -460,7 +460,7 @@ void bcm43xx_rx(struct bcm43xx_wldev *de
u16 phystat0, phystat3, chanstat, mactime;
u32 macstat;
u16 chanid;
- u8 jssi;
+ u8 qual;
int padding;

memset(&status, 0, sizeof(status));
@@ -468,7 +468,7 @@ void bcm43xx_rx(struct bcm43xx_wldev *de
/* Get metadata about the frame from the header. */
phystat0 = le16_to_cpu(rxhdr->phy_status0);
phystat3 = le16_to_cpu(rxhdr->phy_status3);
- jssi = rxhdr->jssi;
+ qual = rxhdr->jssi;
macstat = le32_to_cpu(rxhdr->mac_status);
mactime = le16_to_cpu(rxhdr->mac_time);
chanstat = le16_to_cpu(rxhdr->channel);
@@ -529,12 +529,12 @@ void bcm43xx_rx(struct bcm43xx_wldev *de
}
}

- status.signal = bcm43xx_rssi_postprocess(dev, jssi,
+ status.level = bcm43xx_rssi_postprocess(dev, qual,
(phystat0 & BCM43xx_RX_PHYST0_OFDM),
(phystat0 & BCM43xx_RX_PHYST0_GAINCTL),
(phystat3 & BCM43xx_RX_PHYST3_TRSTATE));
status.noise = dev->stats.link_noise;
- status.ssi = jssi;
+ status.qual = qual;
if (phystat0 & BCM43xx_RX_PHYST0_OFDM)
status.rate = bcm43xx_plcp_get_bitrate_ofdm(plcp);
else
Index: wireless-dev/net/mac80211/debugfs_sta.c
===================================================================
--- wireless-dev.orig/net/mac80211/debugfs_sta.c
+++ wireless-dev/net/mac80211/debugfs_sta.c
@@ -76,8 +76,8 @@ STA_FILE(txrate, txrate, RATE);
STA_FILE(last_txrate, last_txrate, RATE);
STA_FILE(tx_retry_failed, tx_retry_failed, LU);
STA_FILE(tx_retry_count, tx_retry_count, LU);
-STA_FILE(last_rssi, last_rssi, D);
-STA_FILE(last_signal, last_signal, D);
+STA_FILE(last_level, last_level, D);
+STA_FILE(last_qual, last_qual, D);
STA_FILE(last_noise, last_noise, D);
STA_FILE(channel_use, channel_use, D);
STA_FILE(wep_weak_iv_count, wep_weak_iv_count, D);
Index: wireless-dev/net/mac80211/ieee80211_i.h
===================================================================
--- wireless-dev.orig/net/mac80211/ieee80211_i.h
+++ wireless-dev/net/mac80211/ieee80211_i.h
@@ -82,7 +82,7 @@ struct ieee80211_sta_bss {
int hw_mode;
int channel;
int freq;
- int rssi, signal, noise;
+ int level, qual, noise;
u8 *wpa_ie;
size_t wpa_ie_len;
u8 *rsn_ie;
Index: wireless-dev/net/mac80211/ieee80211_ioctl.c
===================================================================
--- wireless-dev.orig/net/mac80211/ieee80211_ioctl.c
+++ wireless-dev/net/mac80211/ieee80211_ioctl.c
@@ -445,7 +445,7 @@ static int ieee80211_ioctl_get_info_sta(
skb_queue_len(&sta->ps_tx_buf);
param->u.get_info_sta.tx_retry_failed = sta->tx_retry_failed;
param->u.get_info_sta.tx_retry_count = sta->tx_retry_count;
- param->u.get_info_sta.last_rssi = sta->last_rssi;
+ param->u.get_info_sta.last_rssi = sta->last_level;
param->u.get_info_sta.last_ack_rssi = sta->last_ack_rssi[2];

sta_info_put(sta);
@@ -1593,12 +1593,12 @@ static int ieee80211_ioctl_giwrange(stru
range->min_frag = 256;
range->max_frag = 2346;

- range->max_qual.qual = local->hw.max_signal;
+ range->max_qual.qual = local->hw.max_qual;
range->max_qual.level = local->hw.max_rssi;
range->max_qual.noise = local->hw.max_noise;
range->max_qual.updated = local->wstats_flags;

- range->avg_qual.qual = local->hw.max_signal/2;
+ range->avg_qual.qual = local->hw.max_qual/2;
range->avg_qual.level = 0;
range->avg_qual.noise = 0;
range->avg_qual.updated = local->wstats_flags;
@@ -3084,8 +3084,8 @@ static struct iw_statistics *ieee80211_g
wstats->qual.noise = 0;
wstats->qual.updated = IW_QUAL_ALL_INVALID;
} else {
- wstats->qual.level = sta->last_rssi;
- wstats->qual.qual = sta->last_signal;
+ wstats->qual.level = sta->last_level;
+ wstats->qual.qual = sta->last_qual;
wstats->qual.noise = sta->last_noise;
wstats->qual.updated = local->wstats_flags;
sta_info_put(sta);
Index: wireless-dev/net/mac80211/sta_info.h
===================================================================
--- wireless-dev.orig/net/mac80211/sta_info.h
+++ wireless-dev/net/mac80211/sta_info.h
@@ -82,8 +82,8 @@ struct sta_info {
unsigned long rx_fragments; /* number of received MPDUs */
unsigned long rx_dropped; /* number of dropped MPDUs from this STA */

- int last_rssi; /* RSSI of last received frame from this STA */
- int last_signal; /* signal of last received frame from this STA */
+ int last_level; /* level of last received frame from this STA */
+ int last_qual; /* qual of last received frame from this STA */
int last_noise; /* noise of last received frame from this STA */
int last_ack_rssi[3]; /* RSSI of last received ACKs from this STA */
unsigned long last_ack;
Index: wireless-dev/drivers/net/wireless/mac80211/adm8211/adm8211.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/adm8211/adm8211.c
+++ wireless-dev/drivers/net/wireless/mac80211/adm8211/adm8211.c
@@ -540,9 +540,9 @@ static void adm8211_interrupt_rci(struct
struct ieee80211_rx_status rx_status = {0};

if (priv->revid < ADM8211_REV_CA)
- rx_status.ssi = rssi;
+ rx_status.level = rssi;
else
- rx_status.ssi = 100 - rssi;
+ rx_status.level = 100 - rssi;

if (rate <= 4)
rx_status.rate = rate_tbl[rate];
Index: wireless-dev/drivers/net/wireless/mac80211/p54/prism54common.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/p54/prism54common.c
+++ wireless-dev/drivers/net/wireless/mac80211/p54/prism54common.c
@@ -282,7 +282,7 @@ static void p54_rx_data(struct ieee80211
struct ieee80211_rx_status rx_status = {0};
u16 freq = le16_to_cpu(hdr->freq);

- rx_status.ssi = hdr->rssi; /* TODO: check this */
+ rx_status.level = hdr->rssi; /* TODO: check this */
rx_status.rate = min(hdr->rate + 1, 12); /* TODO: check this */
rx_status.channel = freq == 2484 ? 14 : (freq - 2407)/5;
rx_status.freq = freq;
Index: wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2500pci.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rt2x00/rt2500pci.c
+++ wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2500pci.c
@@ -1742,7 +1742,7 @@ static void rt2500pci_rxdone(struct work
* Update link statistics
*/
rt2x00_update_link_rssi(&rt2x00dev->link,
- rt2x00dev->rx_status.ssi);
+ rt2x00dev->rx_status.level);

skip_entry:
rt2x00_set_field32(&word0, RXD_W0_OWNER_NIC, 1);
Index: wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2500usb.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rt2x00/rt2500usb.c
+++ wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2500usb.c
@@ -1702,7 +1702,7 @@ static void rt2500usb_interrupt_rxdone(s
/*
* Update link statistics
*/
- rt2x00_update_link_rssi(&rt2x00dev->link, rt2x00dev->rx_status.ssi);
+ rt2x00_update_link_rssi(&rt2x00dev->link, rt2x00dev->rx_status.level);

skip_entry:
if (!GET_FLAG(ring->rt2x00dev, DEVICE_ENABLED_RADIO))
Index: wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2x00dev.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rt2x00/rt2x00dev.c
+++ wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt2x00dev.c
@@ -157,7 +157,7 @@ void rt2x00lib_update_rx_stats(struct rt
}

rt2x00dev->rx_status.rate = val;
- rt2x00dev->rx_status.ssi = rssi;
+ rt2x00dev->rx_status.level = rssi;
rt2x00dev->rx_status.noise = rt2x00_get_link_noise(&rt2x00dev->link);
}
EXPORT_SYMBOL_GPL(rt2x00lib_update_rx_stats);
Index: wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt61pci.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rt2x00/rt61pci.c
+++ wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt61pci.c
@@ -2047,7 +2047,7 @@ static void rt61pci_rxdone(struct work_s
* Update link statistics
*/
rt2x00_update_link_rssi(&rt2x00dev->link,
- rt2x00dev->rx_status.ssi);
+ rt2x00dev->rx_status.level);

skip_entry:
rt2x00_set_field32(&word0, RXD_W0_OWNER_NIC, 1);
Index: wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt73usb.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rt2x00/rt73usb.c
+++ wireless-dev/drivers/net/wireless/mac80211/rt2x00/rt73usb.c
@@ -1858,7 +1858,7 @@ static void rt73usb_interrupt_rxdone(str
/*
* Update link statistics
*/
- rt2x00_update_link_rssi(&rt2x00dev->link, rt2x00dev->rx_status.ssi);
+ rt2x00_update_link_rssi(&rt2x00dev->link, rt2x00dev->rx_status.level);

skip_entry:
if (!GET_FLAG(ring->rt2x00dev, DEVICE_ENABLED_RADIO))
Index: wireless-dev/drivers/net/wireless/mac80211/rtl818x/rtl8187_dev.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/rtl818x/rtl8187_dev.c
+++ wireless-dev/drivers/net/wireless/mac80211/rtl818x/rtl8187_dev.c
@@ -161,8 +161,8 @@ static void rtl8187_rx_cb(struct urb *ur
}

rx_status.antenna = (hdr->signal >> 7) & 1;
- rx_status.signal = 64 - min(hdr->noise, (u8)64);
- rx_status.ssi = signal;
+ rx_status.qual = 64 - min(hdr->noise, (u8)64);
+ rx_status.level = signal;
rx_status.rate = priv->rates[rate].rate;
rx_status.freq = dev->conf.freq;
rx_status.channel = dev->conf.channel;
@@ -609,7 +609,7 @@ static int __devinit rtl8187_probe(struc
dev->extra_tx_headroom = sizeof(struct rtl8187_tx_hdr);
dev->queues = 1;
dev->max_rssi = 65;
- dev->max_signal = 64;
+ dev->max_qual = 64;

for (i = 0; i < 2; i++)
if ((err = ieee80211_register_hwmode(dev, &priv->modes[i])))
Index: wireless-dev/drivers/net/wireless/mac80211/zd1211rw/zd_mac.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/mac80211/zd1211rw/zd_mac.c
+++ wireless-dev/drivers/net/wireless/mac80211/zd1211rw/zd_mac.c
@@ -407,8 +407,8 @@ static int fill_rx_stats(struct ieee8021
stats->channel = _zd_chip_get_channel(&mac->chip);
stats->freq = zd_channels[stats->channel - 1].freq;
stats->phymode = MODE_IEEE80211G;
- stats->ssi = zd_rx_strength_percent(status->signal_strength);
- stats->signal = zd_rx_qual_percent(buffer,
+ stats->level = zd_rx_strength_percent(status->signal_strength);
+ stats->qual = zd_rx_qual_percent(buffer,
length - sizeof(struct rx_status),
status);
stats->rate = zd_rx_rate(buffer, status);
@@ -453,7 +453,7 @@ static int filter_ack(struct ieee80211_h
if (control) {
memcpy(&status.control, control, sizeof(status.control));
status.flags = IEEE80211_TX_STATUS_ACK;
- status.ack_signal = stats->ssi;
+ status.ack_signal = stats->level;
ieee80211_tx_status_irqsafe(dev, skb, &status);
kfree(control);
} else
@@ -631,7 +631,7 @@ struct ieee80211_hw *zd_mac_alloc(struct
dev->flags = IEEE80211_HW_RX_INCLUDES_FCS |
IEEE80211_HW_WEP_INCLUDE_IV;
dev->max_rssi = 100;
- dev->max_signal = 100;
+ dev->max_qual = 100;

dev->queues = 1;
dev->extra_tx_headroom = sizeof(struct zd_ctrlset);

----




2007-04-09 04:46:05

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Sunday 08 April 2007 23:54, Larry Finger wrote:
> Why would I want to do this?
Did it fix the output?

> If the community agrees on anything, it is
> that the signal is given in dBm (i.e. a negative number) and that the rssi
> is a positive number.
Nope. dBm doesn't have to be negative, though it often is since most wireless
hardware isn't that powerful. RSSI is simply a number that's bigger for
stronger signals. It could be dBm, but it doesn't have to be. If you want a
stronger definition of RSSI, look at RCPI.

> The firmware in the bcm43xx chips return a quantity
> that looks like an rssi with a received packet, and
> bcm43xx_rssi_postprocess turns that into a quantity that looks like dBm.
> Your patch reverses those designations and mixes up the two quantities.
> Again I ask "Why"?
>
Because of the naming/use of the statistics in mac80211 and WE. Signal ends up
getting assigned to (struct iw_quality).qual, which is actually just an
arbitrary link quality indicator, not dBm. Anything you care about can be put
there. (r)ssi gets assigned to (struct iw_quality).level, which is RSSI. WE
allows that and noise to be specified in either arbitrary units or dBm or
RCPI.

Yes, I did reverse your conventions, but it makes more sense this way. (R)SSI
is always valid to assign to (struct iw_quality).level and signal ((struct
iw_quality).qual) is quite arbitrary and cannot be specified in specific
units.

Signal should be probably be renamed to qual to make it more clear that it is
arbitrary.

-Michael Wu


Attachments:
(No filename) (1.50 kB)
(No filename) (189.00 B)
Download all attachments

2007-04-08 15:38:01

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Sunday 08 April 2007 01:04, Larry Finger wrote:
> In mac80211 the 'qual' and 'level' values are interchanged. The patch also
> places the 'qual' value on a 0 - 100 scale and calculated using the formula
> contained in iwlib.
>
NACK. Qual (arbitrary signal quality) and level (RSSI) values are correct and
userspace is responsible for scaling any statistics values.

-Michael Wu


Attachments:
(No filename) (382.00 B)
(No filename) (189.00 B)
Download all attachments

2007-04-08 23:31:41

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> On Sunday 08 April 2007 18:26, Larry Finger wrote:
>> Link Quality=216/146 Signal level=-197 dBm Noise level=-63 dBm
>>
> Is this before or after your "bcm43xx-mac80211: Fix error in initiallizing max
> RSSI and max signal" patch?
>
> -Michael Wu


This is after that patch.

Larry

2007-04-08 23:43:46

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Sunday 08 April 2007 19:32, Larry Finger wrote:
> Michael Wu wrote:
> > Is this before or after your "bcm43xx-mac80211: Fix error in
> > initiallizing max RSSI and max signal" patch?
> >
> This is after that patch.
>
So what do the statistics look like without that patch?

-Michael Wu


Attachments:
(No filename) (289.00 B)
(No filename) (189.00 B)
Download all attachments

2007-04-09 17:18:58

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Monday 09 April 2007 11:49, Larry Finger wrote:
> > In WE, qual is arbitrary within a few limits:
> >
> > a) qual _must_ change on a linear scale
> > b) a valid max_qual.qual must be set
> > c) qual must fall within the bounds of [0, max_qual.qual] inclusive
>
> Note that the quantity supplied by the bcm43xx firmware (called jssi in the
> code) varies linearly with the strength of the signal, whereas the quantity
> derived from it in bcm43xx_rssi_postprocess is quasi logarithmic. The
> statement in a) above would argue against reversal of my conventions. What
> happens in the other drivers that use mac80211?
>
rt2x00 does not use qual. All other drivers in tree that use qual
(zd1211rw-mac80211, rtl8187) use the current convention.

I don't see anything in those 4 points that would argue against the current
conventions in mac80211. In fact, it sounds like you are arguing for it since
jssi, which you said is linear, goes to qual (after my patch is applied)
which wants a linear number.

-Michael Wu


Attachments:
(No filename) (0.99 kB)
(No filename) (189.00 B)
Download all attachments

2007-04-09 21:11:17

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> rt2x00 does not use qual. All other drivers in tree that use qual
> (zd1211rw-mac80211, rtl8187) use the current convention.
>
> I don't see anything in those 4 points that would argue against the current
> conventions in mac80211. In fact, it sounds like you are arguing for it since
> jssi, which you said is linear, goes to qual (after my patch is applied)
> which wants a linear number.

The code is internally consistent. Now it is only the usage of the (r)ssi name to describe the
logarithmic numbers that is confusing. Would you entertain a patch that changed (r)ssi to level in
the structs and drivers to reduce the confusion? I have such a patch that works with
bcm43xx-mac80211. In addition, all other drivers compile.

Larry

2007-04-09 12:18:30

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Mon, 2007-04-09 at 08:07 -0400, Dan Williams wrote:
> On Mon, 2007-04-09 at 00:43 -0400, Michael Wu wrote:
> > On Sunday 08 April 2007 23:54, Larry Finger wrote:
> > > Why would I want to do this?
> > Did it fix the output?
> >
> > > If the community agrees on anything, it is
> > > that the signal is given in dBm (i.e. a negative number) and that the rssi
> > > is a positive number.
> > Nope. dBm doesn't have to be negative, though it often is since most wireless
> > hardware isn't that powerful. RSSI is simply a number that's bigger for
> > stronger signals. It could be dBm, but it doesn't have to be. If you want a
> > stronger definition of RSSI, look at RCPI.
> >
> > > The firmware in the bcm43xx chips return a quantity
> > > that looks like an rssi with a received packet, and
> > > bcm43xx_rssi_postprocess turns that into a quantity that looks like dBm.
> > > Your patch reverses those designations and mixes up the two quantities.
> > > Again I ask "Why"?
> > >
> > Because of the naming/use of the statistics in mac80211 and WE. Signal ends up
> > getting assigned to (struct iw_quality).qual, which is actually just an
> > arbitrary link quality indicator, not dBm. Anything you care about can be put
> > there. (r)ssi gets assigned to (struct iw_quality).level, which is RSSI. WE
> > allows that and noise to be specified in either arbitrary units or dBm or
> > RCPI.
> >
> > Yes, I did reverse your conventions, but it makes more sense this way. (R)SSI
> > is always valid to assign to (struct iw_quality).level and signal ((struct
> > iw_quality).qual) is quite arbitrary and cannot be specified in specific
> > units.
> >
> > Signal should be probably be renamed to qual to make it more clear that it is
> > arbitrary.
>
> In WE, qual is arbitrary within a few limits:
>
> a) qual _must_ change on a linear scale
> b) a valid max_qual.qual must be set
> c) qual must fall within the bounds of [0, max_qual.qual] inclusive

d) max_qual.qual must be greater than 0

> If you report 'level' in dBm, you must set the IW_QUAL_DBM flag.
> Otherwise, 'level' _may_ be assumed to be RSSI. If 'level' is dBm,
> max_qual.level must be 0. If 'level' is RSSI, max_qual.level must be
> greater than 0, and level must fall within the bounds of [0,
> max_qual.level] inclusive. Replace 'level' with 'noise' here for the
> rules for noise.
>
> I don't particularly care if level/noise is RSSI _as long as_ you give
> the max RSSI for your part. Different radio parts have different max
> RSSI values, and if you're writing a driver you sure better know them or
> figure some reasonable ones out by experimentation. RSSI is entirely
> vendor defined and does _not_ conform to any rules. Therefore we need
> the max RSSI to get usable signal strength reports from your part.

RSSI doesn't conform to any rules _except_ that has a lower bound of 0
and an upper bound of the max for your part. WE happens to provide only
a u8 for the value, so if your vendor defines RSSI as > 255 you must
somehow stuff RSSI into a u8.

14.2.3.2 RXVECTOR RSSI
The receive signal strength indicator (RSSI) is an optional
parameter that has a value of 0 through RSSI Max.

Every vendor has a different max; Symbol's max is (at least used to be)
31. Cisco aironet's max is 100. Atheros' max appears to be 60.
Therefore, you must specify your max RSSI level in max_qual.level if you
use RSSI so that userspace tools can actually use your RSSI to produce
meaningful numbers.

Dan

> I know that 0 dBm isn't actually the upper bound, but in practice most
> people aren't going to get parts that go above that. 0 dBm should be
> considered a _limitation_ of WEXT that we obviously fix with
> cfg80211/nl80211 when we bring some sanity to signal strength reporting.
>
> Again, if you report level in RSSI, you must provide the max RSSI for
> your part in max_qual.level.
>
> Dan
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2007-04-08 07:48:17

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

What if rssi is in DBM i.e. negative number.

According definition it should be positive but then there is that code
in ieee80211.c

int ieee80211_register_hw(struct ieee80211_hw *hw).
...
if (local->hw.max_rssi < 0 || local->hw.max_noise < 0)

local->wstats_flags |= IW_QUAL_DBM;



On 4/8/07, Larry Finger <[email protected]> wrote:
> In mac80211 the 'qual' and 'level' values are interchanged. The patch also
> places the 'qual' value on a 0 - 100 scale and calculated using the formula
> contained in iwlib.
>
> Signed-off-by: Larry Finger <[email protected]>
> ---
>
> John and Michael,
>
> This fix is meant for the wireless-dev and mb git trees.
>
> Larry
>
> ieee80211_ioctl.c | 16 +++++++++++-----
> ieee80211_sta.c | 9 +++++++--
> 2 files changed, 18 insertions(+), 7 deletions(-)
>
> Index: wireless-mb/net/mac80211/ieee80211_sta.c
> ===================================================================
> --- wireless-mb.orig/net/mac80211/ieee80211_sta.c
> +++ wireless-mb/net/mac80211/ieee80211_sta.c
> @@ -2822,8 +2822,10 @@ ieee80211_sta_scan_result(struct net_dev
>
> memset(&iwe, 0, sizeof(iwe));
> iwe.cmd = IWEVQUAL;
> - iwe.u.qual.qual = bss->signal;
> - iwe.u.qual.level = bss->rssi;
> + iwe.u.qual.level = bss->signal;
> + if (unlikely(local->hw.max_rssi == 0))
> + local->hw.max_rssi = 100;
> + iwe.u.qual.qual = (100 * bss->rssi) / local->hw.max_rssi;
> iwe.u.qual.noise = bss->noise;
> iwe.u.qual.updated = local->wstats_flags;
> current_ev = iwe_stream_add_event(current_ev, end_buf, &iwe,
> Index: wireless-mb/net/mac80211/ieee80211_ioctl.c
> ===================================================================
> --- wireless-mb.orig/net/mac80211/ieee80211_ioctl.c
> +++ wireless-mb/net/mac80211/ieee80211_ioctl.c
> @@ -1593,12 +1593,12 @@ static int ieee80211_ioctl_giwrange(stru
> range->min_frag = 256;
> range->max_frag = 2346;
>
> - range->max_qual.qual = local->hw.max_signal;
> - range->max_qual.level = local->hw.max_rssi;
> + range->max_qual.qual = 100;
> + range->max_qual.level = local->hw.max_signal;
> range->max_qual.noise = local->hw.max_noise;
> range->max_qual.updated = local->wstats_flags;
>
> - range->avg_qual.qual = local->hw.max_signal/2;
> + range->avg_qual.qual = 50;
> range->avg_qual.level = 0;
> range->avg_qual.noise = 0;
> range->avg_qual.updated = local->wstats_flags;
> @@ -3087,8 +3087,10 @@ static struct iw_statistics *ieee80211_g
> wstats->qual.noise = 0;
> wstats->qual.updated = IW_QUAL_ALL_INVALID;
> } else {
> - wstats->qual.level = sta->last_rssi;
> - wstats->qual.qual = sta->last_signal;
> + if (unlikely(local->hw.max_rssi == 0))
> + local->hw.max_rssi = 100;
> + wstats->qual.qual = (100 * sta->last_rssi) / local->hw.max_rssi;
> + wstats->qual.level = sta->last_signal;
> wstats->qual.noise = sta->last_noise;
> wstats->qual.updated = local->wstats_flags;
> sta_info_put(sta);
> -
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2007-04-08 23:04:51

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Sunday 08 April 2007 18:26, Larry Finger wrote:
> Link Quality=216/146 Signal level=-197 dBm Noise level=-63 dBm
>
Is this before or after your "bcm43xx-mac80211: Fix error in initiallizing max
RSSI and max signal" patch?

-Michael Wu


Attachments:
(No filename) (241.00 B)
(No filename) (189.00 B)
Download all attachments

2007-04-08 22:25:29

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> On Sunday 08 April 2007 01:04, Larry Finger wrote:
>> In mac80211 the 'qual' and 'level' values are interchanged. The patch also
>> places the 'qual' value on a 0 - 100 scale and calculated using the formula
>> contained in iwlib.
>>
> NACK. Qual (arbitrary signal quality) and level (RSSI) values are correct and
> userspace is responsible for scaling any statistics values.

I disagree. For the wireless extensions output, the maximum values are set implicitly in the
iw_range struct. In addition, the level is always in dBm (a negative number), whereas the RSSI is an
arbitrary positive number. Because the maximum value is driver dependent, it must be specified when
the driver initializes the MAC layer.

In any case, the current code is broken. Without my patch, an iwconfig command with bcm43xx-mac80211
produces the line:

Link Quality=216/146 Signal level=-197 dBm Noise level=-63 dBm

We could argue about the Link Quality, although the x/146 looks pretty strange. The Signal level of
-197 dBm is impossible. With my patch, the same version of bcm43xx gets the line

Link Quality=90/100 Signal level=-36 dBm Noise level=-69 dBm

That Signal level matches what comes from both bcm43xx-softmac and the Windows driver running under
ndiswrapper.

Larry



2007-04-09 05:05:33

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

Michael Wu wrote:
> On Sunday 08 April 2007 23:54, Larry Finger wrote:
>> Why would I want to do this?
> Did it fix the output?

With that patch as the only one applied to wireless-dev, I get

Link Quality=69/60 Signal level=-37 dBm Noise level=-70 dBm

Larry


2007-04-13 23:21:18

by Michael Wu

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Report correct wireless statistics

On Monday 09 April 2007 20:59, Larry Finger wrote:
> I changed signal to qual and (r)ssi to level. Once these changes were made,
> iwconfig gives me the following:
>
> Link Quality=57/100 Signal level=-37 dBm Noise level=-70 dBm
>
> The patch is pretty large and touches a lot of files, but if one is to be
> able to bisect around it, it has to be a single commit. As the patch
> entitled "[PATCH] bcm43xx-mac80211: Fix error in initiallizing max RSSI and
> max signal" has been accepted into Michael Buesch's tree, I have assumed
> that it is already included.
>
I would prefer having the bcm43xx-mac80211 fix and the renaming separated into
two patches.

Also, why wasn't max_rssi changed to max_level? I'm not so sure about doing
s/rssi/level/, however. s/rssi/signal/ sounds better to me. (which makes it
sound similar to your other patch, except rssi is eliminated because one
variable needs to do RSSI/dBm and the other needs to be more general than
just another RSSI statistic)

The change from "rssi=%d" to "level=%d" looks wrong to me. If any userspace
app is parsing that info, that breaks it. Then again, that information is
actually redundant since we report IWEVQUAL earlier so it should just be
removed. I'll post a patch.

Thanks,
-Michael Wu


Attachments:
(No filename) (1.24 kB)
(No filename) (189.00 B)
Download all attachments