2012-03-15 14:13:29

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] rndis_wlan: don't report station signal

From: Johannes Berg <[email protected]>

The station signal value is supposed to be in dBm
which the device can't give, so don't report it.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/rndis_wlan.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)

--- a/drivers/net/wireless/rndis_wlan.c 2012-03-10 09:17:00.000000000 +0100
+++ b/drivers/net/wireless/rndis_wlan.c 2012-03-15 15:11:52.000000000 +0100
@@ -2505,7 +2505,7 @@ static int rndis_set_default_key(struct
static void rndis_fill_station_info(struct usbnet *usbdev,
struct station_info *sinfo)
{
- __le32 linkspeed, rssi;
+ __le32 linkspeed;
int ret, len;

memset(sinfo, 0, sizeof(*sinfo));
@@ -2516,13 +2516,6 @@ static void rndis_fill_station_info(stru
sinfo->txrate.legacy = le32_to_cpu(linkspeed) / 1000;
sinfo->filled |= STATION_INFO_TX_BITRATE;
}
-
- len = sizeof(rssi);
- ret = rndis_query_oid(usbdev, OID_802_11_RSSI, &rssi, &len);
- if (ret == 0) {
- sinfo->signal = level_to_qual(le32_to_cpu(rssi));
- sinfo->filled |= STATION_INFO_SIGNAL;
- }
}

static int rndis_get_station(struct wiphy *wiphy, struct net_device *dev,




2012-03-15 16:02:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rndis_wlan: don't report station signal

On Thu, Mar 15, 2012 at 04:26:43PM +0100, Johannes Berg wrote:
> On Thu, 2012-03-15 at 11:03 -0400, John W. Linville wrote:
> > On Thu, Mar 15, 2012 at 03:47:14PM +0100, Johannes Berg wrote:
> > > On Thu, 2012-03-15 at 10:25 -0400, John W. Linville wrote:
> > > > On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> > > > > From: Johannes Berg <[email protected]>
> > > > >
> > > > > The station signal value is supposed to be in dBm
> > > > > which the device can't give, so don't report it.
> > > > >
> > > > > Signed-off-by: Johannes Berg <[email protected]>
> > > >
> > > > Won't this break functionality for users of this device?
> > >
> > > I don't know if it breaks functionality, but filling in values in the
> > > wrong units doesn't seem like a good plan.
> >
> > No, but if it isn't actually hurting anyting then I would rather
> > change a comment or two (and perhaps augment the nl80211 code) than
> > to simply break things for users.
> >
> > FWIW, cfg80211_wireless_stats has been able to cope with the
> > unspecified units since it was implemented in commit 8990646d.
> > Maybe that was a bug but it is a user-visible one that has been around
> > for years now...
>
> Interesting, I had no idea. I was more concerned with the actual nl80211
> exporting (nl80211_send_station), not wext. Seems like something like
> this patch would be sufficient then:
> http://p.sipsolutions.net/ca88e4afc69683ca.txt
>
> Of course, we could add nl80211 attributes for unspec in the station
> info too.

Yes, I came-up with something similar -- patch to follow...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2012-03-15 15:26:47

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] rndis_wlan: don't report station signal

On Thu, 2012-03-15 at 11:03 -0400, John W. Linville wrote:
> On Thu, Mar 15, 2012 at 03:47:14PM +0100, Johannes Berg wrote:
> > On Thu, 2012-03-15 at 10:25 -0400, John W. Linville wrote:
> > > On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> > > > From: Johannes Berg <[email protected]>
> > > >
> > > > The station signal value is supposed to be in dBm
> > > > which the device can't give, so don't report it.
> > > >
> > > > Signed-off-by: Johannes Berg <[email protected]>
> > >
> > > Won't this break functionality for users of this device?
> >
> > I don't know if it breaks functionality, but filling in values in the
> > wrong units doesn't seem like a good plan.
>
> No, but if it isn't actually hurting anyting then I would rather
> change a comment or two (and perhaps augment the nl80211 code) than
> to simply break things for users.
>
> FWIW, cfg80211_wireless_stats has been able to cope with the
> unspecified units since it was implemented in commit 8990646d.
> Maybe that was a bug but it is a user-visible one that has been around
> for years now...

Interesting, I had no idea. I was more concerned with the actual nl80211
exporting (nl80211_send_station), not wext. Seems like something like
this patch would be sufficient then:
http://p.sipsolutions.net/ca88e4afc69683ca.txt

Of course, we could add nl80211 attributes for unspec in the station
info too.

johannes


2012-03-15 17:33:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

On Thu, 2012-03-15 at 13:25 -0400, John W. Linville wrote:
> The station_info struct had demanded dBm signal values, but the
> cfg80211 wireless extensions implementation was also accepting
> "unspecified" (i.e. RSSI) unit values while the nl80211 code was
> completely unaware of them. Resolve this by formally allowing the
> "unspecified" units while making nl80211 ignore them.

Looks good, thanks.

Reviewed-by: Johannes Berg <[email protected]>

> Signed-off-by: John W. Linville <[email protected]>
> ---
> The switch statement could be an if, but used it because I copied it
> from elsewhere in the file and because we might (?) want to support a
> different attribute to report the unspecified unit values through
> nl80211?
>
> v2: Fix typo in commit message and add note about using dBm for drivers
> using CFG80211_SIGNAL_TYPE_MBM.
>
> include/net/cfg80211.h | 4 ++--
> net/wireless/nl80211.c | 29 +++++++++++++++++++----------
> 2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index a067d30..4263a62 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -605,8 +605,10 @@ struct sta_bss_parameters {
> * @llid: mesh local link id
> * @plid: mesh peer link id
> * @plink_state: mesh peer link state
> - * @signal: signal strength of last received packet in dBm
> - * @signal_avg: signal strength average in dBm
> + * @signal: the signal strength, type depends on the wiphy's signal_type
> + NOTE: For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_.
> + * @signal_avg: avg signal strength, type depends on the wiphy's signal_type
> + NOTE: For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_.
> * @txrate: current unicast bitrate from this station
> * @rxrate: current unicast bitrate to this station
> * @rx_packets: packets received from this station
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index afeea32..3105c77 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2349,7 +2349,9 @@ nla_put_failure:
> }
>
> static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
> - int flags, struct net_device *dev,
> + int flags,
> + struct cfg80211_registered_device *rdev,
> + struct net_device *dev,
> const u8 *mac_addr, struct station_info *sinfo)
> {
> void *hdr;
> @@ -2388,12 +2390,18 @@ static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
> if (sinfo->filled & STATION_INFO_PLINK_STATE)
> NLA_PUT_U8(msg, NL80211_STA_INFO_PLINK_STATE,
> sinfo->plink_state);
> - if (sinfo->filled & STATION_INFO_SIGNAL)
> - NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
> - sinfo->signal);
> - if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
> - NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
> - sinfo->signal_avg);
> + switch (rdev->wiphy.signal_type) {
> + case CFG80211_SIGNAL_TYPE_MBM:
> + if (sinfo->filled & STATION_INFO_SIGNAL)
> + NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
> + sinfo->signal);
> + if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
> + NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
> + sinfo->signal_avg);
> + break;
> + default:
> + break;
> + }
> if (sinfo->filled & STATION_INFO_TX_BITRATE) {
> if (!nl80211_put_sta_rate(msg, &sinfo->txrate,
> NL80211_STA_INFO_TX_BITRATE))
> @@ -2486,7 +2494,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
> if (nl80211_send_station(skb,
> NETLINK_CB(cb->skb).pid,
> cb->nlh->nlmsg_seq, NLM_F_MULTI,
> - netdev, mac_addr,
> + dev, netdev, mac_addr,
> &sinfo) < 0)
> goto out;
>
> @@ -2531,7 +2539,7 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
> return -ENOMEM;
>
> if (nl80211_send_station(msg, info->snd_pid, info->snd_seq, 0,
> - dev, mac_addr, &sinfo) < 0) {
> + rdev, dev, mac_addr, &sinfo) < 0) {
> nlmsg_free(msg);
> return -ENOBUFS;
> }
> @@ -7482,7 +7490,8 @@ void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
> if (!msg)
> return;
>
> - if (nl80211_send_station(msg, 0, 0, 0, dev, mac_addr, sinfo) < 0) {
> + if (nl80211_send_station(msg, 0, 0, 0,
> + rdev, dev, mac_addr, sinfo) < 0) {
> nlmsg_free(msg);
> return;
> }



2012-03-15 16:02:11

by John W. Linville

[permalink] [raw]
Subject: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

The station_info struct had demanded dBm signal values, but the
cfg80211 wireless extensions implementation was also accepting
"unspecified" (i.e. RSSI) unit values while the nl80211 code was
completely unaware of them. Resolve this by formalling allowing the
"unspecified" units while making nl80211 ignore them.

Signed-off-by: John W. Linville <[email protected]>
---
The switch statement could be an if, but used it because I copied it
from elsewhere in the file and because we might (?) want to support a
different attribute to report the unspecified unit values through
nl80211?

include/net/cfg80211.h | 4 ++--
net/wireless/nl80211.c | 29 +++++++++++++++++++----------
2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a067d30..4263a62 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -605,8 +605,8 @@ struct sta_bss_parameters {
* @llid: mesh local link id
* @plid: mesh peer link id
* @plink_state: mesh peer link state
- * @signal: signal strength of last received packet in dBm
- * @signal_avg: signal strength average in dBm
+ * @signal: the signal strength, type depends on the wiphy's signal_type
+ * @signal_avg: avg signal strength, type depends on the wiphy's signal_type
* @txrate: current unicast bitrate from this station
* @rxrate: current unicast bitrate to this station
* @rx_packets: packets received from this station
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index afeea32..3105c77 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2349,7 +2349,9 @@ nla_put_failure:
}

static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
- int flags, struct net_device *dev,
+ int flags,
+ struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
const u8 *mac_addr, struct station_info *sinfo)
{
void *hdr;
@@ -2388,12 +2390,18 @@ static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
if (sinfo->filled & STATION_INFO_PLINK_STATE)
NLA_PUT_U8(msg, NL80211_STA_INFO_PLINK_STATE,
sinfo->plink_state);
- if (sinfo->filled & STATION_INFO_SIGNAL)
- NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
- sinfo->signal);
- if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
- NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
- sinfo->signal_avg);
+ switch (rdev->wiphy.signal_type) {
+ case CFG80211_SIGNAL_TYPE_MBM:
+ if (sinfo->filled & STATION_INFO_SIGNAL)
+ NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
+ sinfo->signal);
+ if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
+ NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
+ sinfo->signal_avg);
+ break;
+ default:
+ break;
+ }
if (sinfo->filled & STATION_INFO_TX_BITRATE) {
if (!nl80211_put_sta_rate(msg, &sinfo->txrate,
NL80211_STA_INFO_TX_BITRATE))
@@ -2486,7 +2494,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
if (nl80211_send_station(skb,
NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
- netdev, mac_addr,
+ dev, netdev, mac_addr,
&sinfo) < 0)
goto out;

@@ -2531,7 +2539,7 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
return -ENOMEM;

if (nl80211_send_station(msg, info->snd_pid, info->snd_seq, 0,
- dev, mac_addr, &sinfo) < 0) {
+ rdev, dev, mac_addr, &sinfo) < 0) {
nlmsg_free(msg);
return -ENOBUFS;
}
@@ -7482,7 +7490,8 @@ void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
if (!msg)
return;

- if (nl80211_send_station(msg, 0, 0, 0, dev, mac_addr, sinfo) < 0) {
+ if (nl80211_send_station(msg, 0, 0, 0,
+ rdev, dev, mac_addr, sinfo) < 0) {
nlmsg_free(msg);
return;
}
--
1.7.4.4


2012-03-15 15:17:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rndis_wlan: don't report station signal

On Thu, Mar 15, 2012 at 03:47:14PM +0100, Johannes Berg wrote:
> On Thu, 2012-03-15 at 10:25 -0400, John W. Linville wrote:
> > On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> > > From: Johannes Berg <[email protected]>
> > >
> > > The station signal value is supposed to be in dBm
> > > which the device can't give, so don't report it.
> > >
> > > Signed-off-by: Johannes Berg <[email protected]>
> >
> > Won't this break functionality for users of this device?
>
> I don't know if it breaks functionality, but filling in values in the
> wrong units doesn't seem like a good plan.

No, but if it isn't actually hurting anyting then I would rather
change a comment or two (and perhaps augment the nl80211 code) than
to simply break things for users.

FWIW, cfg80211_wireless_stats has been able to cope with the
unspecified units since it was implemented in commit 8990646d.
Maybe that was a bug but it is a user-visible one that has been around
for years now...

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2012-03-15 17:28:10

by John W. Linville

[permalink] [raw]
Subject: [PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

The station_info struct had demanded dBm signal values, but the
cfg80211 wireless extensions implementation was also accepting
"unspecified" (i.e. RSSI) unit values while the nl80211 code was
completely unaware of them. Resolve this by formally allowing the
"unspecified" units while making nl80211 ignore them.

Signed-off-by: John W. Linville <[email protected]>
---
The switch statement could be an if, but used it because I copied it
from elsewhere in the file and because we might (?) want to support a
different attribute to report the unspecified unit values through
nl80211?

v2: Fix typo in commit message and add note about using dBm for drivers
using CFG80211_SIGNAL_TYPE_MBM.

include/net/cfg80211.h | 4 ++--
net/wireless/nl80211.c | 29 +++++++++++++++++++----------
2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a067d30..4263a62 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -605,8 +605,10 @@ struct sta_bss_parameters {
* @llid: mesh local link id
* @plid: mesh peer link id
* @plink_state: mesh peer link state
- * @signal: signal strength of last received packet in dBm
- * @signal_avg: signal strength average in dBm
+ * @signal: the signal strength, type depends on the wiphy's signal_type
+ NOTE: For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_.
+ * @signal_avg: avg signal strength, type depends on the wiphy's signal_type
+ NOTE: For CFG80211_SIGNAL_TYPE_MBM, value is expressed in _dBm_.
* @txrate: current unicast bitrate from this station
* @rxrate: current unicast bitrate to this station
* @rx_packets: packets received from this station
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index afeea32..3105c77 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2349,7 +2349,9 @@ nla_put_failure:
}

static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
- int flags, struct net_device *dev,
+ int flags,
+ struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
const u8 *mac_addr, struct station_info *sinfo)
{
void *hdr;
@@ -2388,12 +2390,18 @@ static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
if (sinfo->filled & STATION_INFO_PLINK_STATE)
NLA_PUT_U8(msg, NL80211_STA_INFO_PLINK_STATE,
sinfo->plink_state);
- if (sinfo->filled & STATION_INFO_SIGNAL)
- NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
- sinfo->signal);
- if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
- NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
- sinfo->signal_avg);
+ switch (rdev->wiphy.signal_type) {
+ case CFG80211_SIGNAL_TYPE_MBM:
+ if (sinfo->filled & STATION_INFO_SIGNAL)
+ NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
+ sinfo->signal);
+ if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
+ NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
+ sinfo->signal_avg);
+ break;
+ default:
+ break;
+ }
if (sinfo->filled & STATION_INFO_TX_BITRATE) {
if (!nl80211_put_sta_rate(msg, &sinfo->txrate,
NL80211_STA_INFO_TX_BITRATE))
@@ -2486,7 +2494,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
if (nl80211_send_station(skb,
NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
- netdev, mac_addr,
+ dev, netdev, mac_addr,
&sinfo) < 0)
goto out;

@@ -2531,7 +2539,7 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
return -ENOMEM;

if (nl80211_send_station(msg, info->snd_pid, info->snd_seq, 0,
- dev, mac_addr, &sinfo) < 0) {
+ rdev, dev, mac_addr, &sinfo) < 0) {
nlmsg_free(msg);
return -ENOBUFS;
}
@@ -7482,7 +7490,8 @@ void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
if (!msg)
return;

- if (nl80211_send_station(msg, 0, 0, 0, dev, mac_addr, sinfo) < 0) {
+ if (nl80211_send_station(msg, 0, 0, 0,
+ rdev, dev, mac_addr, sinfo) < 0) {
nlmsg_free(msg);
return;
}
--
1.7.4.4


2012-03-15 16:19:19

by Cristian Morales Vega

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

Would be a problem to increase the version field in nl80211_fam? Just
to know when I can trust the value (I could live without it).

On 15 March 2012 15:58, John W. Linville <[email protected]> wrote:
> The station_info struct had demanded dBm signal values, but the
> cfg80211 wireless extensions implementation was also accepting
> "unspecified" (i.e. RSSI) unit values while the nl80211 code was
> completely unaware of them. ?Resolve this by formalling allowing the
> "unspecified" units while making nl80211 ignore them.
>
> Signed-off-by: John W. Linville <[email protected]>
> ---
> The switch statement could be an if, but used it because I copied it
> from elsewhere in the file and because we might (?) want to support a
> different attribute to report the unspecified unit values through
> nl80211?
>
> ?include/net/cfg80211.h | ? ?4 ++--
> ?net/wireless/nl80211.c | ? 29 +++++++++++++++++++----------
> ?2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index a067d30..4263a62 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -605,8 +605,8 @@ struct sta_bss_parameters {
> ?* @llid: mesh local link id
> ?* @plid: mesh peer link id
> ?* @plink_state: mesh peer link state
> - * @signal: signal strength of last received packet in dBm
> - * @signal_avg: signal strength average in dBm
> + * @signal: the signal strength, type depends on the wiphy's signal_type
> + * @signal_avg: avg signal strength, type depends on the wiphy's signal_type
> ?* @txrate: current unicast bitrate from this station
> ?* @rxrate: current unicast bitrate to this station
> ?* @rx_packets: packets received from this station
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index afeea32..3105c77 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2349,7 +2349,9 @@ nla_put_failure:
> ?}
>
> ?static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int flags, struct net_device *dev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int flags,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct cfg80211_registered_device *rdev,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct net_device *dev,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const u8 *mac_addr, struct station_info *sinfo)
> ?{
> ? ? ? ?void *hdr;
> @@ -2388,12 +2390,18 @@ static int nl80211_send_station(struct sk_buff *msg, u32 pid, u32 seq,
> ? ? ? ?if (sinfo->filled & STATION_INFO_PLINK_STATE)
> ? ? ? ? ? ? ? ?NLA_PUT_U8(msg, NL80211_STA_INFO_PLINK_STATE,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?sinfo->plink_state);
> - ? ? ? if (sinfo->filled & STATION_INFO_SIGNAL)
> - ? ? ? ? ? ? ? NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?sinfo->signal);
> - ? ? ? if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
> - ? ? ? ? ? ? ? NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
> - ? ? ? ? ? ? ? ? ? ? ? ? ?sinfo->signal_avg);
> + ? ? ? switch (rdev->wiphy.signal_type) {
> + ? ? ? case CFG80211_SIGNAL_TYPE_MBM:
> + ? ? ? ? ? ? ? if (sinfo->filled & STATION_INFO_SIGNAL)
> + ? ? ? ? ? ? ? ? ? ? ? NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sinfo->signal);
> + ? ? ? ? ? ? ? if (sinfo->filled & STATION_INFO_SIGNAL_AVG)
> + ? ? ? ? ? ? ? ? ? ? ? NLA_PUT_U8(msg, NL80211_STA_INFO_SIGNAL_AVG,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sinfo->signal_avg);
> + ? ? ? ? ? ? ? break;
> + ? ? ? default:
> + ? ? ? ? ? ? ? break;
> + ? ? ? }
> ? ? ? ?if (sinfo->filled & STATION_INFO_TX_BITRATE) {
> ? ? ? ? ? ? ? ?if (!nl80211_put_sta_rate(msg, &sinfo->txrate,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?NL80211_STA_INFO_TX_BITRATE))
> @@ -2486,7 +2494,7 @@ static int nl80211_dump_station(struct sk_buff *skb,
> ? ? ? ? ? ? ? ?if (nl80211_send_station(skb,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?NETLINK_CB(cb->skb).pid,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cb->nlh->nlmsg_seq, NLM_F_MULTI,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? netdev, mac_addr,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev, netdev, mac_addr,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&sinfo) < 0)
> ? ? ? ? ? ? ? ? ? ? ? ?goto out;
>
> @@ -2531,7 +2539,7 @@ static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info)
> ? ? ? ? ? ? ? ?return -ENOMEM;
>
> ? ? ? ?if (nl80211_send_station(msg, info->snd_pid, info->snd_seq, 0,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dev, mac_addr, &sinfo) < 0) {
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rdev, dev, mac_addr, &sinfo) < 0) {
> ? ? ? ? ? ? ? ?nlmsg_free(msg);
> ? ? ? ? ? ? ? ?return -ENOBUFS;
> ? ? ? ?}
> @@ -7482,7 +7490,8 @@ void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
> ? ? ? ?if (!msg)
> ? ? ? ? ? ? ? ?return;
>
> - ? ? ? if (nl80211_send_station(msg, 0, 0, 0, dev, mac_addr, sinfo) < 0) {
> + ? ? ? if (nl80211_send_station(msg, 0, 0, 0,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rdev, dev, mac_addr, sinfo) < 0) {
> ? ? ? ? ? ? ? ?nlmsg_free(msg);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
> --
> 1.7.4.4
>
> --
> 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



--
Cristian Morales Vega

Email [email protected]
Office +44 (0) 20 3111 4330
Direct Line:? +44 (0) 20 3111 4338
Web:? http://www.samknows.com


This email is sent for and on behalf of SamKnows?Limited.

This email and any attachments are confidential,?legally privileged
and protected by copyright. If?you are not the intended recipient
dissemination?or copying of this email is prohibited. If you have
received this in error, please notify the sender by?replying by email
and then delete the email?completely from your system.

SamKnows Limited, Registered Number:?06510477, Registered Office: 25
Harley Street,?London, W1G 9BR. Registered in England and?Wales. Trade
Mark 2507103

2012-03-15 16:26:07

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

On 03/15/2012 10:58 AM, John W. Linville wrote:
> The station_info struct had demanded dBm signal values, but the
> cfg80211 wireless extensions implementation was also accepting
> "unspecified" (i.e. RSSI) unit values while the nl80211 code was
> completely unaware of them. Resolve this by formalling allowing the
formally ?

Larry

2012-03-15 16:33:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

On Thu, 2012-03-15 at 11:58 -0400, John W. Linville wrote:
> The station_info struct had demanded dBm signal values, but the
> cfg80211 wireless extensions implementation was also accepting
> "unspecified" (i.e. RSSI) unit values while the nl80211 code was
> completely unaware of them. Resolve this by formalling allowing the
> "unspecified" units while making nl80211 ignore them.

Looks fine, just a minor comment below.

> + * @signal: the signal strength, type depends on the wiphy's signal_type
> + * @signal_avg: avg signal strength, type depends on the wiphy's signal_type

This should be more specific since the BSS signal is "unspec" or "mBm"
where this now demands "unspec" or "dBm"... I suppose it could be made
more consistent at some point.

johannes


2012-03-15 14:32:14

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rndis_wlan: don't report station signal

On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> The station signal value is supposed to be in dBm
> which the device can't give, so don't report it.
>
> Signed-off-by: Johannes Berg <[email protected]>

Won't this break functionality for users of this device?

> ---
> drivers/net/wireless/rndis_wlan.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> --- a/drivers/net/wireless/rndis_wlan.c 2012-03-10 09:17:00.000000000 +0100
> +++ b/drivers/net/wireless/rndis_wlan.c 2012-03-15 15:11:52.000000000 +0100
> @@ -2505,7 +2505,7 @@ static int rndis_set_default_key(struct
> static void rndis_fill_station_info(struct usbnet *usbdev,
> struct station_info *sinfo)
> {
> - __le32 linkspeed, rssi;
> + __le32 linkspeed;
> int ret, len;
>
> memset(sinfo, 0, sizeof(*sinfo));
> @@ -2516,13 +2516,6 @@ static void rndis_fill_station_info(stru
> sinfo->txrate.legacy = le32_to_cpu(linkspeed) / 1000;
> sinfo->filled |= STATION_INFO_TX_BITRATE;
> }
> -
> - len = sizeof(rssi);
> - ret = rndis_query_oid(usbdev, OID_802_11_RSSI, &rssi, &len);
> - if (ret == 0) {
> - sinfo->signal = level_to_qual(le32_to_cpu(rssi));
> - sinfo->filled |= STATION_INFO_SIGNAL;
> - }
> }
>
> static int rndis_get_station(struct wiphy *wiphy, struct net_device *dev,
>
>
>

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2012-03-15 14:47:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] rndis_wlan: don't report station signal

On Thu, 2012-03-15 at 10:25 -0400, John W. Linville wrote:
> On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> > From: Johannes Berg <[email protected]>
> >
> > The station signal value is supposed to be in dBm
> > which the device can't give, so don't report it.
> >
> > Signed-off-by: Johannes Berg <[email protected]>
>
> Won't this break functionality for users of this device?

I don't know if it breaks functionality, but filling in values in the
wrong units doesn't seem like a good plan.

johannes