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(-)
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);
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 <[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.
> 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
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
> 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.
-------------------------------------------
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
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
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 <[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.
> >
>
> 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
[email protected] of your literate lifestyle.
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 <[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.
>
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.