On Wed, 2008-11-26 at 10:38 +0200, Jouni Malinen wrote:
> On Tue, Nov 25, 2008 at 09:05:33PM +0200, Jouni Malinen wrote:
> > This patch adds new NL80211_CMD_SET_WIPHY attributes
> > NL80211_ATTR_WIPHY_FREQ and NL80211_ATTR_WIPHY_SEC_CHAN_OFFSET to allow
> > userspace to set the operating channel (e.g., hostapd for AP mode).
>
> This version addresses the feedback I received by adding regulatory
> enforcement in net/wireless/nl80211.c and mechanism for indicating
> non-HT vs. HT20. In addition, the ht_enable/sec_chan_offset values are
> now handled similarly to local->oper_channel to avoid changing
> parameters for scan.
Thanks. A few other things that I just thought of:
> @@ -26,8 +26,9 @@
> * @NL80211_CMD_GET_WIPHY: request information about a wiphy or dump request
> * to get a list of all present wiphys.
> * @NL80211_CMD_SET_WIPHY: set wiphy parameters, needs %NL80211_ATTR_WIPHY or
> - * %NL80211_ATTR_IFINDEX; can be used to set %NL80211_ATTR_WIPHY_NAME
> - * and/or %NL80211_ATTR_WIPHY_TXQ_PARAMS.
> + * %NL80211_ATTR_IFINDEX; can be used to set %NL80211_ATTR_WIPHY_NAME,
> + * %NL80211_ATTR_WIPHY_TXQ_PARAMS, %NL80211_ATTR_WIPHY_FREQ, and/or
> + * %NL80211_ATTR_WIPHY_SEC_CHAN_OFFSET.
> * @NL80211_CMD_NEW_WIPHY: Newly created wiphy, response to get request
> * or rename notification. Has attributes %NL80211_ATTR_WIPHY and
> * %NL80211_ATTR_WIPHY_NAME.
> @@ -180,6 +181,12 @@
> * /sys/class/ieee80211/<phyname>/index
> * @NL80211_ATTR_WIPHY_NAME: wiphy name (used for renaming)
> * @NL80211_ATTR_WIPHY_TXQ_PARAMS: a nested array of TX queue parameters
> + * @NL80211_ATTR_WIPHY_FREQ: frequency of the selected channel in MHz
> + * @NL80211_ATTR_WIPHY_SEC_CHAN_OFFSET: included with NL80211_ATTR_WIPHY_FREQ
> + * if HT20 or HT40 are allowed (i.e., 802.11n disabled if not included):
It's also possible to explicitly give the _NO_HT value too.
> + * NL80211_SEC_CHAN_DISABLED = HT20 only
> + * NL80211_SEC_CHAN_BELOW = secondary channel is below the primary channel
> + * NL80211_SEC_CHAN_ABOVE = secondary channel is above the primary channel
> + *
> + * @set_channel: Set channel (freq = frequency in MHz, sec_chan_offset:
> + * 0 = HT40 disabled; 1 = HT40 enabled, secondary channel above primary;
> + * -1 = HT40 enabled, secondary channel below primary)
> */
> struct cfg80211_ops {
> int (*add_virtual_intf)(struct wiphy *wiphy, char *name,
> @@ -513,6 +517,9 @@
>
> int (*set_txq_params)(struct wiphy *wiphy,
> struct ieee80211_txq_params *params);
> +
> + int (*set_channel)(struct wiphy *wiphy, int freq, bool ht_enabled,
> + int sec_chan_offset);
I wonder if we shouldn't simply pass in the enum nl80211_sec_chan_offset
value here, that seems easier to understand when you're looking at the
whole thing from userspace down to a possible non-mac80211 driver. Not
that I think that'll ever happen, but still? It might even make sense to
use that value within mac80211's driver API at some point, instead of
the enabled / +/-1 thing.
> +static int ieee80211_set_channel(struct wiphy *wiphy, int freq,
> + bool ht_enabled, int sec_chan_offset)
> +{
> + struct ieee80211_local *local = wiphy_priv(wiphy);
> + struct ieee80211_channel *chan;
> +
> + chan = ieee80211_get_channel(local->hw.wiphy, freq);
> + if (!chan)
> + return -EINVAL;
> +
> + local->oper_channel = chan;
> + local->oper_ht_enabled = ht_enabled;
> + local->oper_sec_chan_offset = sec_chan_offset;
And you could even store the enum here, and translate it only in
_hw_config.
> + [NL80211_ATTR_WIPHY_FREQ] = { .type = NLA_U32 },
> + [NL80211_ATTR_WIPHY_SEC_CHAN_OFFSET] = { .type = NLA_U8 },
I think it's more "normal" to use a u32 for an enum although I don't
think we'll ever add anything, so I guess it doesn't actually matter.
> + freq = nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]);
> + chan = ieee80211_get_channel(&rdev->wiphy, freq);
> + result = rdev->ops->set_channel(&rdev->wiphy, freq, ht_enabled,
> + sec_chan_offset);
would it make sense to pass in 'chan'? mac80211 will do the lookup
again, and if a driver doesn't it can just use chan->center_freq.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
