* Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info
2012-03-15 15:58 ` [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info John W. Linville
@ 2012-03-15 16:19 ` Cristian Morales Vega
2012-03-15 16:26 ` Larry Finger
` (2 subsequent siblings)
3 siblings, 0 replies; 12+ messages in thread
From: Cristian Morales Vega @ 2012-03-15 16:19 UTC (permalink / raw)
To: linux-wireless
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info
2012-03-15 15:58 ` [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info John W. Linville
2012-03-15 16:19 ` Cristian Morales Vega
@ 2012-03-15 16:26 ` Larry Finger
2012-03-15 16:33 ` Johannes Berg
2012-03-15 17:25 ` [PATCH v2] " John W. Linville
3 siblings, 0 replies; 12+ messages in thread
From: Larry Finger @ 2012-03-15 16:26 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, johannes
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info
2012-03-15 15:58 ` [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info John W. Linville
2012-03-15 16:19 ` Cristian Morales Vega
2012-03-15 16:26 ` Larry Finger
@ 2012-03-15 16:33 ` Johannes Berg
2012-03-15 17:25 ` [PATCH v2] " John W. Linville
3 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2012-03-15 16:33 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info
2012-03-15 15:58 ` [PATCH] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info John W. Linville
` (2 preceding siblings ...)
2012-03-15 16:33 ` Johannes Berg
@ 2012-03-15 17:25 ` John W. Linville
2012-03-15 17:33 ` Johannes Berg
3 siblings, 1 reply; 12+ messages in thread
From: John W. Linville @ 2012-03-15 17:25 UTC (permalink / raw)
To: linux-wireless
Cc: johannes, Cristian Morales Vega, Larry Finger, 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2] cfg80211: allow CFG80211_SIGNAL_TYPE_UNSPEC in station_info
2012-03-15 17:25 ` [PATCH v2] " John W. Linville
@ 2012-03-15 17:33 ` Johannes Berg
0 siblings, 0 replies; 12+ messages in thread
From: Johannes Berg @ 2012-03-15 17:33 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, Cristian Morales Vega, Larry Finger
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;
> }
^ permalink raw reply [flat|nested] 12+ messages in thread