[PATCH] rndis_wlan: don't report station signal

[PATCH] rndis_wlan: don't report station signal

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
---
 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,

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

[John W. Linville]

On Thu, Mar 15, 2012 at 03:13:25PM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> 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 <johannes.berg@intel.com>

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
linville@tuxdriver.com			might be all we have.  Be ready.

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

[Johannes Berg]

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 <johannes.berg@intel.com>
> > 
> > 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 <johannes.berg@intel.com>
> 
> 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

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

[John W. Linville]

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 <johannes.berg@intel.com>
> > > 
> > > 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 <johannes.berg@intel.com>
> > 
> > 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
linville@tuxdriver.com			might be all we have.  Be ready.

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

[Johannes Berg]

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 <johannes.berg@intel.com>
> > > > 
> > > > 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 <johannes.berg@intel.com>
> > > 
> > > 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

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

[John W. Linville]

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 <johannes.berg@intel.com>
> > > > > 
> > > > > 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 <johannes.berg@intel.com>
> > > > 
> > > > 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
linville@tuxdriver.com			might be all we have.  Be ready.

[PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[John W. Linville]

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 <linville@tuxdriver.com>
---
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

Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[Cristian Morales Vega]

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 <linville@tuxdriver.com> 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 <linville@tuxdriver.com>
> ---
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Cristian Morales Vega

Email cristian@samknows.com
Office +44 (0) 20 3111 4330
Direct Line:  +44 (0) 20 3111 4338
Web:  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

Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[Larry Finger]

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

Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[Johannes Berg]

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

[PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[John W. Linville]

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 <linville@tuxdriver.com>
---
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

Re: [PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info

[Johannes Berg]

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 <johannes@sipsolutions.net>

> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
> 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;
>  	}