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

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

[Andrey Borzenkov]

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

Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
From: Andrey Borzenkov <arvidjaar@mail.ru>

/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 <arvidjaar@mail.ru>

---

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


diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 92d6b94..24c67bc 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -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);

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

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Pavel Roskin]

On Sat, 2008-11-15 at 18:31 +0300, Andrey Borzenkov wrote:
> Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
> From: Andrey Borzenkov <arvidjaar@mail.ru>
> 
> /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.

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

> +	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.

-- 
Regards,
Pavel Roskin

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Andrey Borzenkov]

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

On Wednesday 19 November 2008, Pavel Roskin wrote:
> On Sat, 2008-11-15 at 18:31 +0300, Andrey Borzenkov wrote:
> > Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
> > From: Andrey Borzenkov <arvidjaar@mail.ru>
> > 
> > /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.
> 

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.

> Receiving 1 mW out of 200 mW is not impossible in some antenna
> configurations even with consumer devices.
> 
> > Before:
> > {pts/1}% cat /sys/class/net/eth1/wireless/level
> > 203
> > {pts/1}% cat /sys/class/net/eth1/wireless/noise
> > 166
> 
> > +	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.

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

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[John W. Linville]

On Wed, Nov 19, 2008 at 10:45:21PM +0300, Andrey Borzenkov wrote:
> On Wednesday 19 November 2008, Pavel Roskin wrote:
> > On Sat, 2008-11-15 at 18:31 +0300, Andrey Borzenkov wrote:
> > > Subject: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs
> > > From: Andrey Borzenkov <arvidjaar@mail.ru>
> > > 
> > > /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.
> > 
> 
> 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.
> 
> > Receiving 1 mW out of 200 mW is not impossible in some antenna
> > configurations even with consumer devices.
> > 
> > > Before:
> > > {pts/1}% cat /sys/class/net/eth1/wireless/level
> > > 203
> > > {pts/1}% cat /sys/class/net/eth1/wireless/noise
> > > 166
> > 
> > > +	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.

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

John
-- 
John W. Linville		Linux should be at the core
linville@tuxdriver.com			of your literate lifestyle.

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Pavel Roskin]

On Wed, 2008-11-19 at 15:08 -0500, John W. Linville wrote:

> > 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.

> 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.

-- 
Regards,
Pavel Roskin

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Johannes Berg]

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

On Wed, 2008-11-19 at 16:51 -0500, Pavel Roskin wrote:
> On Wed, 2008-11-19 at 15:08 -0500, John W. Linville wrote:
> 
> > > 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.
> 
> > 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

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

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Holger Schurig]

> 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.
-------------------------------------------

Re: [PATCH] wireless: sysfs was displaying different values for level and noise than procfs

[Pavel Roskin]

On Thu, 2008-11-20 at 07:57 +0100, Johannes Berg wrote:

> 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.

-- 
Regards,
Pavel Roskin