[RFC] cfg80211: first stab at channel survey
This patch implements the NL80211_CMD_GET_SURVEY command and an get_survey()
ops that a driver can implement. The goal of this command is to allow a
driver to report back channel survey data (e.g. channel noise, channel
occupation) back to cfg80211 and thus via nl80211 to user-space.
In future, they will either be a survey-trigger command --- or the existing
scan-trigger command will be enhanced. However, the get_survey() operation
is usable even in absence of this: a driver can report the channel noise in
mBm with the implemented mechanism.
get_survey() is currently modelled like get_key(). I hope that's right.
struct survey_info is modelled like struct station_info. This allows different
drivers to fill in different fields.
Signed-off-by: Holger Schurig <[email protected]>
--- linux-wl.orig/include/linux/nl80211.h
+++ linux-wl/include/linux/nl80211.h
@@ -159,6 +159,8 @@
* NL80211_CMD_GET_SCAN and on the "scan" multicast group)
* @NL80211_CMD_SCAN_ABORTED: scan was aborted, for unspecified reasons,
* partial scan results may be available
+ * @NL80211_CMD_GET_SURVEY: get survey resuls, e.g. channel occupation
+ * or noise level
*
* @NL80211_CMD_REG_CHANGE: indicates to userspace the regulatory domain
* has been changed and provides details of the request information
@@ -341,6 +343,8 @@
NL80211_CMD_SET_WIPHY_NETNS,
+ NL80211_CMD_GET_SURVEY,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -584,6 +588,8 @@
* changed then the list changed and the dump should be repeated
* completely from scratch.
*
+ * @NL80211_CHANNEL_NOISE: Noise on channel in mBm
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -714,6 +720,8 @@
NL80211_ATTR_PID,
+ NL80211_ATTR_NOISE,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
--- linux-wl.orig/include/net/cfg80211.h
+++ linux-wl/include/net/cfg80211.h
@@ -232,6 +232,35 @@
u32 cipher;
};
+/** enum survey_info_flags - survey information flags
+ *
+ * Used by the driver to indicate which info in &struct survey_info
+ * it has filled in during the get_survey().
+ */
+enum survey_info_flags {
+ SURVEY_INFO_CHANNEL = 1<<0,
+ SURVEY_INFO_NOISE = 1<<1,
+};
+
+/**
+ * struct survey_info - channel survey response
+ *
+ * Used from the get_survey() callback to report back
+ * per-channel survey information.
+ *
+ * @filled: bitflag of flags from &enum survey_info_flags
+ * @noise: channel noise in mBm
+ * @channel: the channel this survey record reports
+ *
+ * This structure can later be expanded with things like
+ * channel duty cycles etc.
+ */
+struct survey_info {
+ u32 filled;
+ struct ieee80211_channel *channel;
+ s8 noise;
+};
+
/**
* struct beacon_parameters - beacon parameters
*
@@ -941,6 +970,8 @@
* @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
* functions to adjust rfkill hw state
*
+ * @get_survey: get site survey information about.
+ *
* @testmode_cmd: run a test mode command
*/
struct cfg80211_ops {
@@ -1060,6 +1091,10 @@
const u8 *peer,
const struct cfg80211_bitrate_mask *mask);
+ int (*get_survey)(struct wiphy *wiphy, struct net_device *netdev,
+ void *cookie,
+ void (*callback)(void *cookie, struct survey_info*));
+
/* some temporary stuff to finish wext */
int (*set_power_mgmt)(struct wiphy *wiphy, struct net_device *dev,
bool enabled, int timeout);
--- linux-wl.orig/net/wireless/nl80211.c
+++ linux-wl/net/wireless/nl80211.c
@@ -3238,6 +3238,155 @@
return err;
}
+struct get_survey_cookie {
+ struct sk_buff *msg;
+ int error;
+};
+
+static void get_survey_callback(void *c, struct survey_info *params)
+{
+ struct get_survey_cookie *cookie = c;
+
+ if (params->filled & SURVEY_INFO_CHANNEL)
+ if (nl80211_msg_put_channel(cookie->msg, params->channel))
+ goto nla_put_failure;
+
+ if (params->filled & SURVEY_INFO_NOISE)
+ NLA_PUT_U32(cookie->msg, NL80211_ATTR_NOISE,
+ params->noise);
+
+ nla_put_failure:
+ cookie->error = 1;
+}
+
+static int nl80211_get_survey(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev;
+ struct net_device *dev;
+ struct get_key_cookie cookie = {
+ .error = 0,
+ };
+ int err;
+ void *hdr;
+ struct sk_buff *msg;
+
+ rtnl_lock();
+
+ err = get_rdev_dev_by_info_ifindex(info, &rdev, &dev);
+ if (err)
+ goto unlock_rtnl;
+
+ if (!rdev->ops->get_survey) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
+ NL80211_CMD_NEW_KEY);
+
+ if (IS_ERR(hdr)) {
+ err = PTR_ERR(hdr);
+ goto free_msg;
+ }
+
+ cookie.msg = msg;
+
+ NLA_PUT_U32(msg, NL80211_ATTR_IFINDEX, dev->ifindex);
+
+ err = rdev->ops->get_survey(&rdev->wiphy, dev, &cookie,
+ get_survey_callback);
+
+ if (err)
+ goto free_msg;
+
+ if (cookie.error)
+ goto nla_put_failure;
+
+ genlmsg_end(msg, hdr);
+ err = genlmsg_reply(msg, info);
+ goto out;
+
+ nla_put_failure:
+ err = -ENOBUFS;
+ free_msg:
+ nlmsg_free(msg);
+ out:
+ cfg80211_unlock_rdev(rdev);
+ dev_put(dev);
+ unlock_rtnl:
+ rtnl_unlock();
+
+ return err;
+
+/*
+ if (!ifidx) {
+ err = nlmsg_parse(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize,
+ nl80211_fam.attrbuf, nl80211_fam.maxattr,
+ nl80211_policy);
+ if (err)
+ return err;
+
+ if (!nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX])
+ return -EINVAL;
+
+ ifidx = nla_get_u32(nl80211_fam.attrbuf[NL80211_ATTR_IFINDEX]);
+ if (!ifidx)
+ return -EINVAL;
+ cb->args[0] = ifidx;
+ }
+
+ dev = dev_get_by_index(sock_net(skb->sk), ifidx);
+ if (!dev)
+ return -ENODEV;
+
+ rdev = cfg80211_get_dev_from_ifindex(sock_net(skb->sk), ifidx);
+ if (IS_ERR(rdev)) {
+ err = PTR_ERR(rdev);
+ goto out_put_netdev;
+ }
+
+ wdev = dev->ieee80211_ptr;
+
+ wdev_lock(wdev);
+ //TODO
+
+ void *hdr;
+ int i;
+
+ ASSERT_WDEV_LOCK(wdev);
+
+ hdr = nl80211hdr_put(msg, pid, seq, flags,
+ NL80211_CMD_NEW_SCAN_RESULTS);
+ if (!hdr)
+ return -1;
+
+
+ return genlmsg_end(msg, hdr);
+
+ nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+
+
+ //TODO
+ wdev_unlock(wdev);
+
+ cb->args[1] = idx;
+ err = skb->len;
+ cfg80211_unlock_rdev(rdev);
+ out_put_netdev:
+ dev_put(dev);
+
+ return err;
+*/
+}
+
static bool nl80211_valid_auth_type(enum nl80211_auth_type auth_type)
{
return auth_type <= NL80211_AUTHTYPE_MAX;
@@ -4315,6 +4464,11 @@
.policy = nl80211_policy,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NL80211_CMD_GET_SURVEY,
+ .doit = nl80211_get_survey,
+ .policy = nl80211_policy,
+ },
};
static struct genl_multicast_group nl80211_mlme_mcgrp = {
.name = "mlme",
--
http://www.holgerschurig.de
On Tue, 2009-11-10 at 10:48 +0100, Holger Schurig wrote:
> +/** enum survey_info_flags - survey information flags
Nit: missing newline
> + *
> + * Used by the driver to indicate which info in &struct survey_info
> + * it has filled in during the get_survey().
> + */
> +enum survey_info_flags {
> + SURVEY_INFO_CHANNEL = 1<<0,
> + SURVEY_INFO_NOISE = 1<<1,
A response w/o a channel seems entirely useless, shouldn't we just
require a channel?
> + * @filled: bitflag of flags from &enum survey_info_flags
> + * @noise: channel noise in mBm
> + * @channel: the channel this survey record reports
> + *
> + * This structure can later be expanded with things like
> + * channel duty cycles etc.
> + */
> +struct survey_info {
> + u32 filled;
> + struct ieee80211_channel *channel;
> + s8 noise;
For mBm, an s8 won't be enough I think? -90 dBm == -9000 mBm?
> +static void get_survey_callback(void *c, struct survey_info *params)
> +{
> + struct get_survey_cookie *cookie = c;
> +
> + if (params->filled & SURVEY_INFO_CHANNEL)
> + if (nl80211_msg_put_channel(cookie->msg, params->channel))
> + goto nla_put_failure;
> +
> + if (params->filled & SURVEY_INFO_NOISE)
> + NLA_PUT_U32(cookie->msg, NL80211_ATTR_NOISE,
> + params->noise);
> +
> + nla_put_failure:
> + cookie->error = 1;
> +}
You need to construct a nested attribute here and put the values into it
so that the callback can be invoked multiple times, once for each
channel. OTOH, when there are many many channels that will overrun the
message size at some point and we'll pull our hair (like we're doing now
with the channel list in phy info) ... so I'd prefer you move this to a
dump-style model like dump_stations has.
Right now you're retrieving a single channel but can't even control
which one. Maybe that's useful functionality in addition to dump when
you _can_ ask for information on a specific channel, but that'd have to
pass in the frequency from userspace.
Retrieving all data like you've implemented (though I guess you forgot
the multiple channels case) should be a dump. Maybe that's sufficient
since there won't be huge amounts of data and userspace can just pick
out what it needs from the dump.
johannes
At Tue, 10 Nov 2009 10:48:01 +0100,
Holger Schurig wrote:
> @@ -584,6 +588,8 @@
> * changed then the list changed and the dump should be repeated
> * completely from scratch.
> *
> + * @NL80211_CHANNEL_NOISE: Noise on channel in mBm
> + *
> * @NL80211_ATTR_MAX: highest attribute number currently defined
> * @__NL80211_ATTR_AFTER_LAST: internal use
> */
> @@ -714,6 +720,8 @@
>
> NL80211_ATTR_PID,
>
> + NL80211_ATTR_NOISE,
> +
> /* add attributes here, update the policy in nl80211.c */
>
> __NL80211_ATTR_AFTER_LAST,
Oops, the comment in the kdoc doesn't match the enum.
--
http://www.holgerschurig.de
On Tue, 2009-11-10 at 12:19 +0100, Holger Schurig wrote:
> > > +enum survey_info_flags {
> > > + SURVEY_INFO_CHANNEL = 1<<0,
> > > + SURVEY_INFO_NOISE = 1<<1,
> >
> > A response w/o a channel seems entirely useless, shouldn't we just
> > require a channel?
>
> Hey, libertas could respond just the noise and no channel. But it
> could then also respond with the channel, it's there anyway.
But for the multi-channel use case, not having a channel will mean
userspace has no idea what the value means ... it's useless without a
channel I think.
> Controlling what should be returned should be done after either
> enhancing scan-trigger or writing a new survey-trigger command. So
> survey-dump would be very similar to scan-dump: return as much data as
> is available (and not stale).
Right.
> > Maybe that's useful functionality in addition to dump when you _can_
> > ask for information on a specific channel, but that'd have to pass
> > in the frequency from userspace.
>
> Would this then be useful for the scan-dump command as well?
Not sure .. would you want to get scan results for just a single
channel? But we can always add that later if we wanted to, I don't think
it makes a whole lot of sense for either of them.
> > Retrieving all data like you've implemented (though I guess you forgot
> > the multiple channels case) should be a dump.
>
> I did not forgot about the multi-channel case, I forgot about the
> nesting. /me's still learning nl80211 :-)
:)
johannes
On Tue, 2009-11-10 at 11:17 +0100, Johannes Berg wrote:
> Right now you're retrieving a single channel but can't even control
> which one. Maybe that's useful functionality in addition to dump when
> you _can_ ask for information on a specific channel, but that'd have to
> pass in the frequency from userspace.
>
> Retrieving all data like you've implemented (though I guess you forgot
> the multiple channels case) should be a dump. Maybe that's sufficient
> since there won't be huge amounts of data and userspace can just pick
> out what it needs from the dump.
And maybe the get_survey() driver call can actually pass in the channel
pointer ... then the drivers/mac80211 don't need to implement both
get_survey() and dump_survey() since cfg80211 knows the valid channel
list anyway. But that means iterating over the channel list all the
time, even if the driver can only return values for a single channel.
johannes
> Nit: missing newline
Done
> > +enum survey_info_flags {
> > + SURVEY_INFO_CHANNEL = 1<<0,
> > + SURVEY_INFO_NOISE = 1<<1,
>
> A response w/o a channel seems entirely useless, shouldn't we just
> require a channel?
Hey, libertas could respond just the noise and no channel. But it
could then also respond with the channel, it's there anyway.
> For mBm, an s8 won't be enough I think? -90 dBm == -9000 mBm?
Oh, sure.
> You need to construct a nested attribute here and put the values into it
> so that the callback can be invoked multiple times, once for each
> channel. OTOH, when there are many many channels that will overrun the
> message size at some point and we'll pull our hair (like we're doing now
> with the channel list in phy info) ... so I'd prefer you move this to a
> dump-style model like dump_stations has.
Okay.
Or maybe a little bit more nl80211_dump_scan.
> Right now you're retrieving a single channel but can't even control
> which one.
Idea was to retrieve as many channels as the drivers sends using the
callback.
Controlling what should be returned should be done after either
enhancing scan-trigger or writing a new survey-trigger command. So
survey-dump would be very similar to scan-dump: return as much data as
is available (and not stale).
And this could very well be only one channel. And in absence of
survey-trigger or an enhanced-scan trigger it will certainly only
report info about the current channel.
> Maybe that's useful functionality in addition to dump when you _can_
> ask for information on a specific channel, but that'd have to pass
> in the frequency from userspace.
Would this then be useful for the scan-dump command as well?
> Retrieving all data like you've implemented (though I guess you forgot
> the multiple channels case) should be a dump.
I did not forgot about the multi-channel case, I forgot about the
nesting. /me's still learning nl80211 :-)