We have beacon filtering (to reduce host wakeups) in our device,
but for some measurement/debug purposes we need to turn it off.
This adds API for it -- comments welcome.
johannes
Johannes Berg <[email protected]> writes:
> On Wed, 2013-07-17 at 07:10 +0300, Kalle Valo wrote:
>> Johannes Berg <[email protected]> writes:
>>
>> > We have beacon filtering (to reduce host wakeups) in our device,
>> > but for some measurement/debug purposes we need to turn it off.
>>
>> TBH I'm not really fond of this. I'm not really sure what's the use
>> case but first this sounded a like a factory test for me, not something
>> which a regular user would want to do.
Sure, that's good to do. I'm just worried that if we add a new command
to enable/disable each small feature we will have a lot of commands in
nl80211. But I guess that's not a problem as you are the maintainer
anyway :)
But I do see benefits from this, so I guess in the end this is good to
have. It would be nice if someone would add a similar command for BT
coexistance as well, that always seems to be a common source of
problems.
> Yeah, in a way that's true. FWIW, we could also connect it to testmode
> and not worry about it for upstream, but it seemed that others might
> want/need similar functionality.
>
>> Can't we connect this to power save? When disabling power save we could
>> also disable beacon filtering and would not need a separate command.
>
> I'm not so sure that's a good idea. While superficially beacon filtering
> is related to saving power, it's really a different thing - it's about
> CPU/host power while powersave is about device power (RX chains etc.)
> Connecting them, in particular where disabling beacon filtering isn't
> even supported by all devices, doesn't really seem like a good idea,
> particularly not for any tool that would require such functionality.
Makes sense.
--
Kalle Valo
From: Emmanuel Grumbach <[email protected]>
This new nl80211 command allow a user space application to
instruct the WiFi stack to let all the beacons through so
that measurements can be conducted on these beacons (i.e.
RSSI etc...)
Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/cfg80211.h | 12 ++++++++++++
include/uapi/linux/nl80211.h | 10 ++++++++++
net/wireless/nl80211.c | 32 +++++++++++++++++++++++++++++++-
net/wireless/rdev-ops.h | 13 +++++++++++++
net/wireless/trace.h | 17 +++++++++++++++++
5 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index aeaf6df..019e256 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2139,6 +2139,10 @@ struct cfg80211_update_ft_ies_params {
* @crit_proto_stop: Indicates critical protocol no longer needs increased link
* reliability. This operation can not fail.
* @set_coalesce: Set coalesce parameters.
+ *
+ * @beacon_measurement: Start / stops the beacon measurement. When enabled,
+ * beacons should be let through cfg80211 to proceed to measurements (e.g.
+ * RSSI analysis).
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2376,6 +2380,10 @@ struct cfg80211_ops {
struct wireless_dev *wdev);
int (*set_coalesce)(struct wiphy *wiphy,
struct cfg80211_coalesce *coalesce);
+
+ int (*beacon_measurement)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ bool state);
};
/*
@@ -2441,6 +2449,9 @@ struct cfg80211_ops {
* @WIPHY_FLAG_OFFCHAN_TX: Device supports direct off-channel TX.
* @WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL: Device supports remain-on-channel call.
* @WIPHY_FLAG_SUPPORTS_5_10_MHZ: Device supports 5 MHz and 10 MHz channels.
+ * @WIPHY_FLAG_SUPPORTS_BEACON_MEAS: Device supports beacon measurements. Should
+ * be set when the device can let all the beacon through up to cfg80211
+ * to proceed to measurements (i.e. RSSI measurements).
*/
enum wiphy_flags {
WIPHY_FLAG_CUSTOM_REGULATORY = BIT(0),
@@ -2465,6 +2476,7 @@ enum wiphy_flags {
WIPHY_FLAG_OFFCHAN_TX = BIT(20),
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL = BIT(21),
WIPHY_FLAG_SUPPORTS_5_10_MHZ = BIT(22),
+ WIPHY_FLAG_SUPPORTS_BEACON_MEAS = BIT(23),
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index eb68735..1a5edad 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -676,6 +676,9 @@
* @NL80211_CMD_GET_COALESCE: Get currently supported coalesce rules.
* @NL80211_CMD_SET_COALESCE: Configure coalesce rules or clear existing rules.
*
+ * @NL80211_CMD_BEACON_MEASUREMENT: Indicates the beacon measurement is
+ * started / stopped. Disassociation should stop the measurements.
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -841,6 +844,8 @@ enum nl80211_commands {
NL80211_CMD_GET_COALESCE,
NL80211_CMD_SET_COALESCE,
+ NL80211_CMD_BEACON_MEASUREMENT,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1469,6 +1474,9 @@ enum nl80211_commands {
*
* @NL80211_ATTR_COALESCE_RULE: Coalesce rule information.
*
+ * @NL80211_ATTR_BCON_MEAS_STATE: The state of the beacon measurements. This is
+ * a flag attribute.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1771,6 +1779,8 @@ enum nl80211_attrs {
NL80211_ATTR_COALESCE_RULE,
+ NL80211_ATTR_BCON_MEAS_STATE,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 03d4ef9..c2b43b6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -349,6 +349,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_IE_RIC] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_PEER_AID] = { .type = NLA_U16 },
+ [NL80211_ATTR_BCON_MEAS_STATE] = { .type = NLA_FLAG, },
};
/* policy for the key attributes */
@@ -8646,6 +8647,27 @@ static int nl80211_crit_protocol_stop(struct sk_buff *skb,
return 0;
}
+static int nl80211_beacon_measurement(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ bool state;
+
+
+ if (!rdev->ops->beacon_measurement ||
+ !(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_BEACON_MEAS))
+ return -EOPNOTSUPP;
+
+ /* Relevant for associated mgd interfaces only */
+ if (wdev->iftype != NL80211_IFTYPE_STATION || !wdev->current_bss)
+ return -EOPNOTSUPP;
+
+ state = info->attrs[NL80211_ATTR_BCON_MEAS_STATE];
+
+ return rdev_beacon_measurement(rdev, wdev, state);
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -9361,7 +9383,15 @@ static struct genl_ops nl80211_ops[] = {
.flags = GENL_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL,
- }
+ },
+ {
+ .cmd = NL80211_CMD_BEACON_MEASUREMENT,
+ .doit = nl80211_beacon_measurement,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};
static struct genl_multicast_group nl80211_mlme_mcgrp = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 9f15f0a..e5b0efb 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -923,4 +923,17 @@ static inline void rdev_crit_proto_stop(struct cfg80211_registered_device *rdev,
trace_rdev_return_void(&rdev->wiphy);
}
+static inline int
+rdev_beacon_measurement(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev, bool state)
+{
+ int ret;
+
+ trace_rdev_beacon_measurement(&rdev->wiphy, wdev, state);
+ ret = rdev->ops->beacon_measurement(&rdev->wiphy, wdev, state);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+
+ return ret;
+}
+
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 09af6eb..5f6e7f0 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1841,6 +1841,23 @@ TRACE_EVENT(rdev_crit_proto_stop,
WIPHY_PR_ARG, WDEV_PR_ARG)
);
+TRACE_EVENT(rdev_beacon_measurement,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev, bool state),
+ TP_ARGS(wiphy, wdev, state),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ __field(bool, state)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ __entry->state = state;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT " %s",
+ WIPHY_PR_ARG, WDEV_PR_ARG, BOOL_TO_STR(__entry->state))
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
1.8.0
On Wed, 2013-07-17 at 07:10 +0300, Kalle Valo wrote:
> Johannes Berg <[email protected]> writes:
>
> > We have beacon filtering (to reduce host wakeups) in our device,
> > but for some measurement/debug purposes we need to turn it off.
>
> TBH I'm not really fond of this. I'm not really sure what's the use
> case but first this sounded a like a factory test for me, not something
> which a regular user would want to do.
Yeah, in a way that's true. FWIW, we could also connect it to testmode
and not worry about it for upstream, but it seemed that others might
want/need similar functionality.
> Can't we connect this to power save? When disabling power save we could
> also disable beacon filtering and would not need a separate command.
I'm not so sure that's a good idea. While superficially beacon filtering
is related to saving power, it's really a different thing - it's about
CPU/host power while powersave is about device power (RX chains etc.)
Connecting them, in particular where disabling beacon filtering isn't
even supported by all devices, doesn't really seem like a good idea,
particularly not for any tool that would require such functionality.
johannes
Johannes Berg <[email protected]> writes:
> We have beacon filtering (to reduce host wakeups) in our device,
> but for some measurement/debug purposes we need to turn it off.
TBH I'm not really fond of this. I'm not really sure what's the use
case but first this sounded a like a factory test for me, not something
which a regular user would want to do.
Can't we connect this to power save? When disabling power save we could
also disable beacon filtering and would not need a separate command.
--
Kalle Valo
From: Emmanuel Grumbach <[email protected]>
This tells the low-level driver to let all the beacons
through and update the data needed to conduct the
measurements.
Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 7 +++++++
net/mac80211/cfg.c | 14 ++++++++++++++
net/mac80211/driver-ops.h | 20 ++++++++++++++++++++
net/mac80211/trace.h | 23 +++++++++++++++++++++++
4 files changed, 64 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3124036..fb8f6c6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2634,6 +2634,10 @@ enum ieee80211_roc_type {
* driver's resume function returned 1, as this is just like an "inline"
* hardware restart. This callback may sleep.
*
+ * @beacon_measurement: Start or stop beacon measurement. When started, all the
+ * beacons must be let through and data for measurement should be set in
+ * &ieee80211_rx_status (rssi etc...).
+ *
* @ipv6_addr_change: IPv6 address assignment on the given interface changed.
* Currently, this is only called for managed or P2P client interfaces.
* This callback is optional; it must not sleep.
@@ -2819,6 +2823,9 @@ struct ieee80211_ops {
void (*restart_complete)(struct ieee80211_hw *hw);
+ int (*beacon_measurement)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif, bool state);
+
#if IS_ENABLED(CONFIG_IPV6)
void (*ipv6_addr_change)(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b82fff6..34b57a9 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3407,6 +3407,19 @@ static int ieee80211_cfg_get_channel(struct wiphy *wiphy,
return ret;
}
+static int ieee80211_beacon_measurement(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ bool state)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ if (WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_STATION))
+ return -EINVAL;
+
+ return drv_beacon_measurement(local, sdata, state);
+}
+
#ifdef CONFIG_PM
static void ieee80211_set_wakeup(struct wiphy *wiphy, bool enabled)
{
@@ -3492,4 +3505,5 @@ struct cfg80211_ops mac80211_config_ops = {
.get_et_strings = ieee80211_get_et_strings,
.get_channel = ieee80211_cfg_get_channel,
.start_radar_detection = ieee80211_start_radar_detection,
+ .beacon_measurement = ieee80211_beacon_measurement,
};
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index b931c96..7fbe57f 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1060,6 +1060,26 @@ drv_set_default_unicast_key(struct ieee80211_local *local,
trace_drv_return_void(local);
}
+static inline int
+drv_beacon_measurement(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata, bool state)
+{
+ int ret = -EOPNOTSUPP;
+ check_sdata_in_driver(sdata);
+
+ trace_drv_beacon_measurement(local, sdata, state);
+
+ if (local->ops->beacon_measurement)
+ ret = local->ops->beacon_measurement(&local->hw, &sdata->vif,
+ state);
+ else
+ /* Driver advertises caps but doesn't implement the callback? */
+ WARN_ON_ONCE(1);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
#if IS_ENABLED(CONFIG_IPV6)
static inline void drv_ipv6_addr_change(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index c215fafd7..d41bbe2 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1887,6 +1887,29 @@ TRACE_EVENT(drv_set_default_unicast_key,
LOCAL_PR_ARG, VIF_PR_ARG, __entry->key_idx)
);
+TRACE_EVENT(drv_beacon_measurement,
+ TP_PROTO(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ bool state),
+
+ TP_ARGS(local, sdata, state),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ __field(bool, state)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ __entry->state = state;
+ ),
+
+ TP_printk(LOCAL_PR_FMT VIF_PR_FMT " state:%d",
+ LOCAL_PR_ARG, VIF_PR_ARG, __entry->state)
+);
+
TRACE_EVENT(api_radar_detected,
TP_PROTO(struct ieee80211_local *local),
--
1.8.0