wireless: sysfs was displaying different values for level and noise than procfs

PATCH In review Andrey Borzenkov 5 participants 8 messages Nov 15, 2008 – Nov 20, 2008 Networking

Share & tools

Andrey Borzenkov

author

2008-11-15 15:31 UTC #0 lore raw

PATCH 1/1 wireless: sysfs was displaying different values for level and noise than procfs

Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
From: Andrey Borzenkov <[email protected]>

/proc/net/wireless asjusts display of signal and noise level depending on whether units are percentage or dBm. Use the same format in sysfs. This makes it easy to know unit - below zero is dBm, above zero is percent.

Before:
{pts/1}% cat /sys/class/net/eth1/wireless/level
203
{pts/1}% cat /sys/class/net/eth1/wireless/noise
166

After:
{pts/1}% cat /sys/class/net/eth1/wireless/level
-48
{pts/1}% cat /sys/class/net/eth1/wireless/noise
-91

for the following iwconfig output:
eth1 IEEE 802.11b ESSID:"Home, sweet home" Nickname:"cooker"

	[...]
          Link Quality=54/92  Signal level=-48 dBm  Noise level=-91 dBm

Signed-off-by Andrey Borzenkov <[email protected]>

---

 net/core/net-sysfs.c |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

net/core/net-sysfs.c +15 -2

index 92d6b94..24c67bc 100644
@@ -389,10 +389,23 @@ static ssize_t show_iw_##name(struct device *d,				\
 }									\
 static DEVICE_ATTR(name, S_IRUGO, show_iw_##name, NULL)
 
+#define WIRELESS_SHOW_LEVEL(name, field, format_string)			\
+static ssize_t format_iw_##name(const struct iw_statistics *iw, char *buf) \
+{									\
+	return sprintf(buf, format_string, (__s32)iw->field -		\
+			((iw->qual.updated & IW_QUAL_DBM) ? 0x100 : 0)); \
+}									\
+static ssize_t show_iw_##name(struct device *d,				\
+			      struct device_attribute *attr, char *buf)	\
+{									\
+	return wireless_show(d, buf, format_iw_##name);			\
+}									\
+static DEVICE_ATTR(name, S_IRUGO, show_iw_##name, NULL)
+
 WIRELESS_SHOW(status, status, fmt_hex);
 WIRELESS_SHOW(link, qual.qual, fmt_dec);
-WIRELESS_SHOW(level, qual.level, fmt_dec);
-WIRELESS_SHOW(noise, qual.noise, fmt_dec);
+WIRELESS_SHOW_LEVEL(level, qual.level, fmt_dec);
+WIRELESS_SHOW_LEVEL(noise, qual.noise, fmt_dec);
 WIRELESS_SHOW(nwid, discard.nwid, fmt_dec);
 WIRELESS_SHOW(crypt, discard.code, fmt_dec);
 WIRELESS_SHOW(fragment, discard.fragment, fmt_dec);

Pavel Roskin

2008-11-19 19:13 UTC #1 lore raw

Quoted 6 lines · Andrey Borzenkov

Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
From: Andrey Borzenkov <[email protected]>
/proc/net/wireless asjusts display of signal and noise level depending on whether units are percentage or dBm. Use the same format in sysfs. This makes it easy to know unit - below zero is dBm, above zero is percent.

I've seen received power up to 3 dBm. The AP was using an amplifier (it's specialized hardware), and it's possible that the card on the station side was exaggerating the signal. However, the data we report should not need any guesswork to be interpreted. It may be parsed by userspace software.

Receiving 1 mW out of 200 mW is not impossible in some antenna configurations even with consumer devices.

net/core/net-sysfs.c static ssize_t show_iw_##name(struct device *d, \ · line 396

Quoted 8 lines

Before:
{pts/1}% cat /sys/class/net/eth1/wireless/level
203
{pts/1}% cat /sys/class/net/eth1/wireless/noise
166
net/core/net-sysfs.c +2 -0
+	return sprintf(buf, format_string, (__s32)iw->field -		\
+			((iw->qual.updated & IW_QUAL_DBM) ? 0x100 : 0)); \

I think non-dBm data should be shown in a special format: "203/255" where 255 is the maximal value, and the dBm data should be shown as
numbers: "-48". This way, we encourage use of dBm data by making it easier to parse.

Signature · 2 lines

Regards,
Pavel Roskin

Andrey Borzenkov

author

2008-11-19 19:45 UTC #2 lore raw

Quoted 8 lines · Pavel Roskin

Quoted 6 lines · Andrey Borzenkov
Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
From: Andrey Borzenkov <[email protected]>
/proc/net/wireless asjusts display of signal and noise level depending on whether units are percentage or dBm. Use the same format in sysfs. This makes it easy to know unit - below zero is dBm, above zero is percent.

Quoted 6 lines

I've seen received power up to 3 dBm. The AP was using an amplifier (it's specialized hardware), and it's possible that the card on the station side was exaggerating the signal. However, the data we report should not need any guesswork to be interpreted. It may be parsed by userspace software.

But positive dBm is not even possible to represnt right now. It is u8 and is displayed as ((s32)level - 256); so it is *always* negative.

Quoted 9 lines

Receiving 1 mW out of 200 mW is not impossible in some antenna configurations even with consumer devices.
Quoted 5 lines
Before:
{pts/1}% cat /sys/class/net/eth1/wireless/level
203
{pts/1}% cat /sys/class/net/eth1/wireless/noise
166

Quoted 8 lines

net/core/net-sysfs.c static ssize_t show_iw_##name(struct device *d, \ · line 396
Quoted 2 lines
net/core/net-sysfs.c +2 -0
+	return sprintf(buf, format_string, (__s32)iw->field -		\
+			((iw->qual.updated & IW_QUAL_DBM) ? 0x100 : 0)); \
I think non-dBm data should be shown in a special format: "203/255" where 255 is the maximal value, and the dBm data should be shown as
numbers: "-48". This way, we encourage use of dBm data by making it easier to parse.

Well, I do not have any strong opinion regarding this; I just noticed discrepancy between procfs and sysfs for what effectively is the same raw value.

John W. Linville

2008-11-19 20:16 UTC #3 lore raw

Quoted 37 lines · Andrey Borzenkov

Quoted 8 lines · Pavel Roskin
Quoted 6 lines · Andrey Borzenkov
Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
From: Andrey Borzenkov <[email protected]>
/proc/net/wireless asjusts display of signal and noise level depending on whether units are percentage or dBm. Use the same format in sysfs. This makes it easy to know unit - below zero is dBm, above zero is percent.
Quoted 6 lines
I've seen received power up to 3 dBm. The AP was using an amplifier (it's specialized hardware), and it's possible that the card on the station side was exaggerating the signal. However, the data we report should not need any guesswork to be interpreted. It may be parsed by userspace software.
But positive dBm is not even possible to represnt right now. It is u8 and is displayed as ((s32)level - 256); so it is *always* negative.
Quoted 9 lines
Receiving 1 mW out of 200 mW is not impossible in some antenna configurations even with consumer devices.
Quoted 5 lines
Before:
{pts/1}% cat /sys/class/net/eth1/wireless/level
203
{pts/1}% cat /sys/class/net/eth1/wireless/noise
166
Quoted 8 lines
net/core/net-sysfs.c static ssize_t show_iw_##name(struct device *d, \ · line 396
Quoted 2 lines
net/core/net-sysfs.c +2 -0
+	return sprintf(buf, format_string, (__s32)iw->field -		\
+			((iw->qual.updated & IW_QUAL_DBM) ? 0x100 : 0)); \
I think non-dBm data should be shown in a special format: "203/255" where 255 is the maximal value, and the dBm data should be shown as
numbers: "-48". This way, we encourage use of dBm data by making it easier to parse.

Quoted 3 lines

Well, I do not have any strong opinion regarding this; I just noticed discrepancy between procfs and sysfs for what effectively is the same raw value.

And what is your perspective regarding potential 'userland ABI' issues of either this patch or of Pavel's suggestion?

John

Signature · 2 lines

John W. Linville		Linux should be at the core
[email protected]			of your literate lifestyle.

Pavel Roskin

2008-11-19 21:52 UTC #4 lore raw

Quoted 3 lines · John W. Linville

Quoted 3 lines
But positive dBm is not even possible to represnt right now. It is u8 and is displayed as ((s32)level - 256); so it is *always* negative.

I would force dBm to be between -192 and 63. That would cover everything even remotely possible.

Quoted 2 lines

And what is your perspective regarding potential 'userland ABI' issues of either this patch or of Pavel's suggestion?

In my opinion, discrepancy between procfs and sysfs is permissible. If the sysfs interface has been wrong, we should be able to change the format. As for the procfs interface, it could be deprecated.

Signature · 2 lines

Regards,
Pavel Roskin

Johannes Berg

2008-11-20 06:58 UTC #5 lore raw

Quoted 9 lines · Pavel Roskin

Quoted 3 lines · John W. Linville
Quoted 3 lines
But positive dBm is not even possible to represnt right now. It is u8 and is displayed as ((s32)level - 256); so it is *always* negative.
I would force dBm to be between -192 and 63. That would cover everything even remotely possible.

Quoted 6 lines

Quoted 2 lines
And what is your perspective regarding potential 'userland ABI' issues of either this patch or of Pavel's suggestion?
In my opinion, discrepancy between procfs and sysfs is permissible. If the sysfs interface has been wrong, we should be able to change the format. As for the procfs interface, it could be deprecated.

Actually, I was hoping to deprecate the sysfs interface, there are tons of apps relying on procfs unfortunately and very few, if any, relying on sysfs.

johannes

Pavel Roskin

2008-11-20 16:15 UTC #6 lore raw

Quoted 3 lines · Johannes Berg

Actually, I was hoping to deprecate the sysfs interface, there are tons of apps relying on procfs unfortunately and very few, if any, relying on sysfs.

OK, then I would not bother changing it at all.

Signature · 2 lines

Regards,
Pavel Roskin

Holger Schurig

2008-11-20 12:08 UTC #7 lore raw

In reply to Pavel Roskin

Quoted 1 line

As for the procfs interface, it could be deprecated.

Sysfs (at least /sys/class/net/*/wireless) is already deprecated, see the help text of WIRELESS_EXT_SYSFS:

-------------------------------------------
defined at net/wireless/Kconfig:64

This option enables the deprecated wireless statistics

                        ^^^^^^^^^^

files in /sys/class/net/*/wireless/. The same information is available via the ioctls as well.

Say Y if you have programs using it, like old versions of hal.
-------------------------------------------