2010-03-12 11:37:16

by Juuso Oikarinen

[permalink] [raw]
Subject: [RFC PATCH 0/2] mac80211: cfg80211: Roam trigger support

To implement good performance WLAN roaming, it is not sufficient to start
scanning for other available AP's only after the currently serving association
is lost.

The entity controlling the roaming will need to get indication of a
deteriorating WLAN connection in order to start preparing for roaming already
before the serving association is lost. This way, it can roam to a better AP
perhaps even before the serving association becomes too bad in quality.

These patches propose an implementation facilitating this using a simple RSSI
threshold and hysteresis approach.

These patches add a nl80211 interface for simply enabling and disabling a
roam trigger, and configuring a RSSI threshold and hysteresis value to
facilitate the triggering.

For the triggering, these patches currently rely on HW support, host based
triggering is not implemented, but could be added later if needed.

These patches are currently still under development, and mostly untested.

Thanks in advance for your comments and suggestions.

Juuso Oikarinen (2):
cfg80211: Add roaming trigger support to nl80211
mac80211: Add support for roam trigger configuration

include/linux/nl80211.h | 21 ++++++++
include/net/cfg80211.h | 33 ++++++++++++
include/net/mac80211.h | 28 +++++++++++
net/mac80211/cfg.c | 29 +++++++++++
net/mac80211/mlme.c | 9 +++
net/wireless/core.c | 12 +++++
net/wireless/mlme.c | 25 +++++++++
net/wireless/nl80211.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.h | 5 ++
9 files changed, 287 insertions(+), 0 deletions(-)



2010-03-15 06:25:12

by Juuso Oikarinen

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] cfg80211: Add roaming trigger support to nl80211

On Fri, 2010-03-12 at 13:00 +0100, ext Johannes Berg wrote:
> On Fri, 2010-03-12 at 13:34 +0200, Juuso Oikarinen wrote:
> > Add support for basic configuration of a roaming RSSI trigger to the nl80211
> > interface, and basic support for notifying about trigger events.
>
> Cool.
>
> > + NL80211_ATTR_RTRIG_RSSI_STATE,
> > + NL80211_ATTR_RTRIG_RSSI_THOLD,
> > + NL80211_ATTR_RTRIG_RSSI_HYST,
> > + NL80211_ATTR_RTRIG_LEVEL,
>
> IMHO we should have one NL80211_ATTR_ROAM_TRIGGER_CONFIG or something
> like that which nests NL80211_RTRIG_ATTR_* from a separate attribute
> namespace (note the transposition to tell which namespace they belong
> to) so it's easier to understand when extended.
>
> > +enum nl80211_rtrig_rssi_state {
> > + NL80211_RTRIG_RSSI_DISABLED,
> > + NL80211_RTRIG_RSSI_ENABLED,
> > +};
>
> Shouldn't _STATE be implicitly enabled when you give the required
> attributes? Missing docstrings too.
>
> Also overall I'm not sure this should be called roam_trigger at all. It
> will eventually trigger roaming in userspace, but I think the actual
> feature it implements in the kernel/device side is better called
> something like "connection quality monitoring".

These are good comments. I'll try to clean up accordingly.

> > +/**
> > + * enum ieee80211_roam_trigger_level - supported frequency bands
>
> ??
>
> > + * @IEEE80211_ROAM_TRIGGER_LOW: current RSSI level is below the threshold
> > + * @IEEE80211_ROAM_TRIGGER_HIGH: current RSSI level is above the threshold
> > + */
>
> > +
> > +enum ieee80211_roam_trigger_level {
> > + IEEE80211_ROAM_TRIGGER_LEVEL_LOW,
> > + IEEE80211_ROAM_TRIGGER_LEVEL_HIGH,
> > +};
>
> You should just reuse the nl80211 enum.
>
> > diff --git a/net/wireless/core.c b/net/wireless/core.c
> > index 7fdb940..c8156d6 100644
> > --- a/net/wireless/core.c
> > +++ b/net/wireless/core.c
> > @@ -713,6 +713,18 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
> > wdev->ps = false;
> > }
> >
> > + wdev->rtrig_rssi = false;
> > + wdev->rtrig_rssi_thold = -75;
> > + wdev->rtrig_rssi_hyst = 2;
> > + if (rdev->ops->set_roam_trigger)
> > + if (rdev->ops->set_roam_trigger(wdev->wiphy, dev,
> > + wdev->rtrig_rssi,
> > + wdev->rtrig_rssi_thold,
> > + wdev->rtrig_rssi_hyst)) {
> > + /* assume this means it's off */
> > + wdev->rtrig_rssi = false;
> > + }
>
> I'm not sure I agree this should be enabled by default ... it needs
> software listening to it anyway so that software can just enable it.
>
> Also, you don't need to keep track of rtrig_rssi etc. in the wdev since
> you never read those variables again, unless you want to add code to
> export them again when the interface configuration is dumped out.

It's not actually being "enabled" here - actually it's being explicitly
disabled. But you're right, that probably is not necessary either, so
revisit this and come up with a RFCv2

> Either way, it's a good start, thanks for doing this work.

Thanks for the feedback.

-Juuso

> johannes
>
> --
> 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



2010-03-12 11:37:44

by Juuso Oikarinen

[permalink] [raw]
Subject: [RFC PATCH 2/2] mac80211: Add support for roam trigger configuration

Add support for the set_roam_trigger op. This op function configures the
requested rssi roam trigger state, and associated threshold and hysteresis
values to the hardware, if the hardware supports
IEEE80211_HW_SUPPORTS_ROAM_TRIGGER.

For unsupporting hardware, currently -EOPNOTSUPP is returned, so the mac80211
is currently not doing roam trigger monitoring on the host. This could be added
later, if needed.

Signed-off-by: Juuso Oikarinen <[email protected]>
---
include/net/mac80211.h | 28 ++++++++++++++++++++++++++++
net/mac80211/cfg.c | 29 +++++++++++++++++++++++++++++
net/mac80211/mlme.c | 9 +++++++++
3 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 936bc41..f2be0c8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -599,6 +599,7 @@ enum ieee80211_conf_flags {
* @IEEE80211_CONF_CHANGE_RETRY_LIMITS: retry limits changed
* @IEEE80211_CONF_CHANGE_IDLE: Idle flag changed
* @IEEE80211_CONF_CHANGE_SMPS: Spatial multiplexing powersave mode changed
+ * @IEEE80211_CONF_CHANGE_ROAM_TRIGGER: roaming trigger config changed
*/
enum ieee80211_conf_changed {
IEEE80211_CONF_CHANGE_SMPS = BIT(1),
@@ -609,6 +610,7 @@ enum ieee80211_conf_changed {
IEEE80211_CONF_CHANGE_CHANNEL = BIT(6),
IEEE80211_CONF_CHANGE_RETRY_LIMITS = BIT(7),
IEEE80211_CONF_CHANGE_IDLE = BIT(8),
+ IEEE80211_CONF_CHANGE_ROAM_TRIGGER = BIT(9),
};

/**
@@ -662,6 +664,10 @@ enum ieee80211_smps_mode {
* frame, called "dot11ShortRetryLimit" in 802.11, but actually means the
* number of transmissions not the number of retries
*
+ * @rtrig_rssi: Current state of the RSSI level based roam trigger
+ * @rtrig_rssi_thold: Roam trigger RSSI threshold
+ * @rtrig_rssi_hyst: Roam trigger RSSI hysteresis
+ *
* @smps_mode: spatial multiplexing powersave mode; note that
* %IEEE80211_SMPS_STATIC is used when the device is not
* configured for an HT channel
@@ -676,6 +682,10 @@ struct ieee80211_conf {

u8 long_frame_max_tx_count, short_frame_max_tx_count;

+ bool rtrig_rssi;
+ s8 rtrig_rssi_thold;
+ u8 rtrig_rssi_hyst;
+
struct ieee80211_channel *channel;
enum nl80211_channel_type channel_type;
enum ieee80211_smps_mode smps_mode;
@@ -954,6 +964,10 @@ enum ieee80211_tkip_key_type {
* Hardware can provide ack status reports of Tx frames to
* the stack.
*
+ * @IEEE80211_HW_SUPPORTS_ROAM_TRIGGER:
+ * Hardware can provide roaming indications based on configured
+ * RSSI threshold and hysteresis values.
+ *
*/
enum ieee80211_hw_flags {
IEEE80211_HW_HAS_RATE_CONTROL = 1<<0,
@@ -975,6 +989,7 @@ enum ieee80211_hw_flags {
IEEE80211_HW_SUPPORTS_DYNAMIC_SMPS = 1<<16,
IEEE80211_HW_SUPPORTS_UAPSD = 1<<17,
IEEE80211_HW_REPORTS_TX_ACK_STATUS = 1<<18,
+ IEEE80211_HW_SUPPORTS_ROAM_TRIGGER = 1<<19,
};

/**
@@ -2370,6 +2385,19 @@ void ieee80211_sta_block_awake(struct ieee80211_hw *hw,
*/
void ieee80211_beacon_loss(struct ieee80211_vif *vif);

+/**
+ * ieee80211_roam_trigger - inform roaming RSSI threshold triggered
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @level: the current level of the RSSI value
+ *
+ * When the HW supports IEEE80211_HW_SUPPORTS_ROAM_TRIGGER is set, and
+ * a roaming RSSI threshold has been configured and enabled, the driver will
+ * inform whenever RSSI goes above or below the threshold with this function.
+ */
+void ieee80211_roam_trigger(struct ieee80211_vif *vif,
+ enum ieee80211_roam_trigger_level level);
+
/* Rate control API */

/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b7116ef..079fcf6 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1402,6 +1402,34 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
return 0;
}

+static int ieee80211_set_roam_trigger(struct wiphy *wiphy,
+ struct net_device *dev,
+ bool rssi_enabled, s8 rssi_thold,
+ u8 rssi_hyst)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_conf *conf = &local->hw.conf;
+
+ if (sdata->vif.type != NL80211_IFTYPE_STATION)
+ return -EOPNOTSUPP;
+
+ if (!(local->hw.flags & IEEE80211_HW_SUPPORTS_ROAM_TRIGGER))
+ return -EOPNOTSUPP;
+
+ if (rssi_enabled == conf->rtrig_rssi &&
+ rssi_thold == conf->rtrig_rssi_thold &&
+ rssi_hyst == conf->rtrig_rssi_hyst)
+ return 0;
+
+ conf->rtrig_rssi = rssi_enabled;
+ conf->rtrig_rssi_thold = rssi_thold;
+ conf->rtrig_rssi_hyst = rssi_hyst;
+
+ ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_ROAM_TRIGGER);
+ return 0;
+}
+
static int ieee80211_set_bitrate_mask(struct wiphy *wiphy,
struct net_device *dev,
const u8 *addr,
@@ -1506,4 +1534,5 @@ struct cfg80211_ops mac80211_config_ops = {
.remain_on_channel = ieee80211_remain_on_channel,
.cancel_remain_on_channel = ieee80211_cancel_remain_on_channel,
.action = ieee80211_action,
+ .set_roam_trigger = ieee80211_set_roam_trigger,
};
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index be5f723..7a72913 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2135,3 +2135,12 @@ int ieee80211_mgd_action(struct ieee80211_sub_if_data *sdata,
*cookie = (unsigned long) skb;
return 0;
}
+
+void ieee80211_roam_trigger(struct ieee80211_vif *vif,
+ enum ieee80211_roam_trigger_level level)
+{
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+ cfg80211_roam_trigger(sdata->dev, level, GFP_KERNEL);
+}
+EXPORT_SYMBOL(ieee80211_roam_trigger);
--
1.6.3.3


2010-03-12 11:37:30

by Juuso Oikarinen

[permalink] [raw]
Subject: [RFC PATCH 1/2] cfg80211: Add roaming trigger support to nl80211

Add support for basic configuration of a roaming RSSI trigger to the nl80211
interface, and basic support for notifying about trigger events.

Via this interface a user-space connection manager may configure and receive
pre-warning events of deteriorating WLAN connection quality, and start
preparing for roaming in advance, before the connection is already lost.

An example usage of such a trigger is starting scanning for nearby AP's in
an attempt to find one with better connection quality, and associate to it
before the connection characteristics of the existing connection become too bad
or the association is even lost, leading in a prolonged delay in connectivity.

The interface currently supports only RSSI, but it could be later extended
to include other parameters, such as signal-to-noise ratio, if need for that
arises.

Signed-off-by: Juuso Oikarinen <[email protected]>
---
include/linux/nl80211.h | 21 ++++++++
include/net/cfg80211.h | 33 ++++++++++++
net/wireless/core.c | 12 +++++
net/wireless/mlme.c | 25 +++++++++
net/wireless/nl80211.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.h | 5 ++
6 files changed, 221 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 28ba20f..8154c10 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -323,6 +323,10 @@
* the TX command and %NL80211_ATTR_FRAME includes the contents of the
* frame. %NL80211_ATTR_ACK flag is included if the recipient acknowledged
* the frame.
+ * @NL80211_CMD_SET_ROAM_TRIGGER: Roaming RSSI trigger level configuration and
+ * notification. This command is used both as a command (to configure
+ * a trigger level) and as an event (to indicate the configured level was
+ * reached.)
*
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
@@ -419,6 +423,8 @@ enum nl80211_commands {
NL80211_CMD_SET_POWER_SAVE,
NL80211_CMD_GET_POWER_SAVE,

+ NL80211_CMD_SET_ROAM_TRIGGER,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -842,6 +848,11 @@ enum nl80211_attrs {

NL80211_ATTR_PS_STATE,

+ NL80211_ATTR_RTRIG_RSSI_STATE,
+ NL80211_ATTR_RTRIG_RSSI_THOLD,
+ NL80211_ATTR_RTRIG_RSSI_HYST,
+ NL80211_ATTR_RTRIG_LEVEL,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -1583,4 +1594,14 @@ enum nl80211_ps_state {
NL80211_PS_ENABLED,
};

+enum nl80211_rtrig_rssi_state {
+ NL80211_RTRIG_RSSI_DISABLED,
+ NL80211_RTRIG_RSSI_ENABLED,
+};
+
+enum nl80211_rtrig_level {
+ NL80211_RTRIG_HIGH,
+ NL80211_RTRIG_LOW,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 3d134a1..3d10578 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1007,6 +1007,7 @@ struct cfg80211_pmksa {
* RSN IE. It allows for faster roaming between WPA2 BSSIDs.
* @del_pmksa: Delete a cached PMKID.
* @flush_pmksa: Flush all cached PMKIDs.
+ * @set_roam_trigger: Configure roaming trigger notification RSSI threshold.
*
*/
struct cfg80211_ops {
@@ -1152,6 +1153,10 @@ struct cfg80211_ops {

int (*set_power_mgmt)(struct wiphy *wiphy, struct net_device *dev,
bool enabled, int timeout);
+
+ int (*set_roam_trigger)(struct wiphy *wiphy, struct net_device *dev,
+ bool rssi_enabled, s8 rssi_thold,
+ u8 rssi_hyst);
};

/*
@@ -1491,6 +1496,10 @@ struct wireless_dev {
bool ps;
int ps_timeout;

+ bool rtrig_rssi;
+ s8 rtrig_rssi_thold;
+ u8 rtrig_rssi_hyst;
+
#ifdef CONFIG_CFG80211_WEXT
/* wext data */
struct {
@@ -2337,4 +2346,28 @@ bool cfg80211_rx_action(struct net_device *dev, int freq, const u8 *buf,
void cfg80211_action_tx_status(struct net_device *dev, u64 cookie,
const u8 *buf, size_t len, bool ack, gfp_t gfp);

+
+/**
+ * enum ieee80211_roam_trigger_level - supported frequency bands
+ *
+ * @IEEE80211_ROAM_TRIGGER_LOW: current RSSI level is below the threshold
+ * @IEEE80211_ROAM_TRIGGER_HIGH: current RSSI level is above the threshold
+ */
+
+enum ieee80211_roam_trigger_level {
+ IEEE80211_ROAM_TRIGGER_LEVEL_LOW,
+ IEEE80211_ROAM_TRIGGER_LEVEL_HIGH,
+};
+
+/**
+ * cfg80211_roam_trigger - notification of a roaming trigger event
+ * @dev: network device
+ * @level: current level of the RSSI
+ * @gfp: context flags
+ *
+ * This function is called when a configured roaming trigger event occurs.
+ */
+void cfg80211_roam_trigger(struct net_device *dev,
+ enum ieee80211_roam_trigger_level level, gfp_t gfp);
+
#endif /* __NET_CFG80211_H */
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 7fdb940..c8156d6 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -713,6 +713,18 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
wdev->ps = false;
}

+ wdev->rtrig_rssi = false;
+ wdev->rtrig_rssi_thold = -75;
+ wdev->rtrig_rssi_hyst = 2;
+ if (rdev->ops->set_roam_trigger)
+ if (rdev->ops->set_roam_trigger(wdev->wiphy, dev,
+ wdev->rtrig_rssi,
+ wdev->rtrig_rssi_thold,
+ wdev->rtrig_rssi_hyst)) {
+ /* assume this means it's off */
+ wdev->rtrig_rssi = false;
+ }
+
if (!dev->ethtool_ops)
dev->ethtool_ops = &cfg80211_ethtool_ops;

diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 62bc885..cba6365 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -894,3 +894,28 @@ void cfg80211_action_tx_status(struct net_device *dev, u64 cookie,
nl80211_send_action_tx_status(rdev, dev, cookie, buf, len, ack, gfp);
}
EXPORT_SYMBOL(cfg80211_action_tx_status);
+
+void cfg80211_roam_trigger(struct net_device *dev,
+ enum ieee80211_roam_trigger_level level, gfp_t gfp)
+{
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+ enum nl80211_rtrig_level nllevel;
+
+ switch (level) {
+ case IEEE80211_ROAM_TRIGGER_LEVEL_LOW:
+ nllevel = NL80211_RTRIG_LOW;
+ break;
+ case IEEE80211_ROAM_TRIGGER_LEVEL_HIGH:
+ nllevel = NL80211_RTRIG_HIGH;
+ break;
+ default:
+ WARN_ON(1);
+ return;
+ }
+
+ /* Indicate roaming trigger event to user space */
+ nl80211_send_roam_trigger(rdev, dev, nllevel, gfp);
+}
+EXPORT_SYMBOL(cfg80211_roam_trigger);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e447db0..6f5bbd3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -149,6 +149,10 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_FRAME_MATCH] = { .type = NLA_BINARY, },
[NL80211_ATTR_PS_STATE] = { .type = NLA_U32 },
+ [NL80211_ATTR_RTRIG_RSSI_STATE] = { .type = NLA_U32 },
+ [NL80211_ATTR_RTRIG_RSSI_THOLD] = { .type = NLA_U8 },
+ [NL80211_ATTR_RTRIG_RSSI_HYST] = { .type = NLA_U8 },
+ [NL80211_ATTR_RTRIG_LEVEL] = { .type = NLA_U32 },
};

/* policy for the attributes */
@@ -4778,6 +4782,85 @@ unlock_rtnl:
return err;
}

+static int nl80211_set_roam_trigger(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ struct wireless_dev *wdev;
+ struct net_device *dev;
+ u8 rssi_state;
+ u8 rssi_thold;
+ u8 rssi_hyst;
+
+ bool state;
+ int err;
+
+ if (!info->attrs[NL80211_ATTR_RTRIG_RSSI_STATE] &&
+ !info->attrs[NL80211_ATTR_RTRIG_RSSI_THOLD] &&
+ !info->attrs[NL80211_ATTR_RTRIG_RSSI_HYST]) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ rssi_state = nla_get_u32(info->attrs[NL80211_ATTR_RTRIG_RSSI_STATE]);
+ if (rssi_state != NL80211_RTRIG_RSSI_DISABLED &&
+ rssi_state != NL80211_RTRIG_RSSI_ENABLED) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ rssi_thold = nla_get_u8(info->attrs[NL80211_ATTR_RTRIG_RSSI_THOLD]);
+ if (rssi_thold > 100) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ rssi_hyst = nla_get_u8(info->attrs[NL80211_ATTR_RTRIG_RSSI_HYST]);
+ if (rssi_hyst > 10) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ rtnl_lock();
+
+ err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
+ if (err)
+ goto unlock_rdev;
+
+ wdev = dev->ieee80211_ptr;
+
+ if (!rdev->ops->set_roam_trigger) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ state = (rssi_state == NL80211_RTRIG_RSSI_ENABLED) ? true : false;
+
+ if (state == wdev->rtrig_rssi &&
+ wdev->rtrig_rssi_thold == rssi_thold &&
+ wdev->rtrig_rssi_hyst == -rssi_hyst)
+ goto unlock_rdev;
+
+ wdev->rtrig_rssi = state;
+ wdev->rtrig_rssi_thold = rssi_thold;
+ wdev->rtrig_rssi_hyst = rssi_hyst;
+
+ if (rdev->ops->set_roam_trigger(wdev->wiphy, dev,
+ wdev->rtrig_rssi,
+ wdev->rtrig_rssi_thold,
+ wdev->rtrig_rssi_hyst)) {
+ /* assume this means it's off */
+ wdev->rtrig_rssi = false;
+ }
+
+unlock_rdev:
+ cfg80211_unlock_rdev(rdev);
+ dev_put(dev);
+ rtnl_unlock();
+
+out:
+ return err;
+}
+
static struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_GET_WIPHY,
@@ -5082,6 +5165,12 @@ static struct genl_ops nl80211_ops[] = {
.policy = nl80211_policy,
/* can be retrieved by unprivileged users */
},
+ {
+ .cmd = NL80211_CMD_SET_ROAM_TRIGGER,
+ .doit = nl80211_set_roam_trigger,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ },
};

static struct genl_multicast_group nl80211_mlme_mcgrp = {
@@ -5832,6 +5921,42 @@ void nl80211_send_action_tx_status(struct cfg80211_registered_device *rdev,
nlmsg_free(msg);
}

+void nl80211_send_roam_trigger(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev,
+ enum nl80211_rtrig_level level,
+ gfp_t gfp)
+{
+ struct sk_buff *msg;
+ void *hdr;
+
+ msg = nlmsg_new(NLMSG_GOODSIZE, gfp);
+ if (!msg)
+ return;
+
+ hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_SET_ROAM_TRIGGER);
+ if (!hdr) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ NLA_PUT_U32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx);
+ NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex);
+ NLA_PUT_U32(msg, NL80211_ATTR_RTRIG_LEVEL, level);
+
+ if (genlmsg_end(msg, hdr) < 0) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_mlme_mcgrp.id, gfp);
+ return;
+
+ nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ nlmsg_free(msg);
+}
+
static int nl80211_netlink_notify(struct notifier_block * nb,
unsigned long state,
void *_notify)
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 4ca5111..12c9069 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -82,4 +82,9 @@ void nl80211_send_action_tx_status(struct cfg80211_registered_device *rdev,
const u8 *buf, size_t len, bool ack,
gfp_t gfp);

+void nl80211_send_roam_trigger(struct cfg80211_registered_device *rdev,
+ struct net_device *netdev,
+ enum nl80211_rtrig_level level,
+ gfp_t gfp);
+
#endif /* __NET_WIRELESS_NL80211_H */
--
1.6.3.3


2010-03-12 12:00:22

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC PATCH 1/2] cfg80211: Add roaming trigger support to nl80211

On Fri, 2010-03-12 at 13:34 +0200, Juuso Oikarinen wrote:
> Add support for basic configuration of a roaming RSSI trigger to the nl80211
> interface, and basic support for notifying about trigger events.

Cool.

> + NL80211_ATTR_RTRIG_RSSI_STATE,
> + NL80211_ATTR_RTRIG_RSSI_THOLD,
> + NL80211_ATTR_RTRIG_RSSI_HYST,
> + NL80211_ATTR_RTRIG_LEVEL,

IMHO we should have one NL80211_ATTR_ROAM_TRIGGER_CONFIG or something
like that which nests NL80211_RTRIG_ATTR_* from a separate attribute
namespace (note the transposition to tell which namespace they belong
to) so it's easier to understand when extended.

> +enum nl80211_rtrig_rssi_state {
> + NL80211_RTRIG_RSSI_DISABLED,
> + NL80211_RTRIG_RSSI_ENABLED,
> +};

Shouldn't _STATE be implicitly enabled when you give the required
attributes? Missing docstrings too.

Also overall I'm not sure this should be called roam_trigger at all. It
will eventually trigger roaming in userspace, but I think the actual
feature it implements in the kernel/device side is better called
something like "connection quality monitoring".

> +/**
> + * enum ieee80211_roam_trigger_level - supported frequency bands

??

> + * @IEEE80211_ROAM_TRIGGER_LOW: current RSSI level is below the threshold
> + * @IEEE80211_ROAM_TRIGGER_HIGH: current RSSI level is above the threshold
> + */

> +
> +enum ieee80211_roam_trigger_level {
> + IEEE80211_ROAM_TRIGGER_LEVEL_LOW,
> + IEEE80211_ROAM_TRIGGER_LEVEL_HIGH,
> +};

You should just reuse the nl80211 enum.

> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 7fdb940..c8156d6 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -713,6 +713,18 @@ static int cfg80211_netdev_notifier_call(struct notifier_block * nb,
> wdev->ps = false;
> }
>
> + wdev->rtrig_rssi = false;
> + wdev->rtrig_rssi_thold = -75;
> + wdev->rtrig_rssi_hyst = 2;
> + if (rdev->ops->set_roam_trigger)
> + if (rdev->ops->set_roam_trigger(wdev->wiphy, dev,
> + wdev->rtrig_rssi,
> + wdev->rtrig_rssi_thold,
> + wdev->rtrig_rssi_hyst)) {
> + /* assume this means it's off */
> + wdev->rtrig_rssi = false;
> + }

I'm not sure I agree this should be enabled by default ... it needs
software listening to it anyway so that software can just enable it.

Also, you don't need to keep track of rtrig_rssi etc. in the wdev since
you never read those variables again, unless you want to add code to
export them again when the interface configuration is dumped out.

Either way, it's a good start, thanks for doing this work.

johannes