I've been looking at adding some of the missing configuration options to
allow AP mode in mac80211 to remain in sync with hostapd. The patch
below introduces a new cfg80211 command for setting BSS parameters and
adds three initial attributes for this command (CTS protection, short
preamble, short slot time). I would expect CTS protection and short
preamble to work as-is, but short slot time will need some more code to
notify the driver that it needs to change configuration. Based on a
quick test with hostapd, it looks like these parameters can be
successfully passed from there to mac80211.
Could you please take a quick look at whether this looks like a correct
way of introducing new functionality into cfg80211? I'll finish the
short slot time part in mac80211 and post an updated patch as a proper
submission later.
Index: wireless-testing/include/net/cfg80211.h
===================================================================
--- wireless-testing.orig/include/net/cfg80211.h
+++ wireless-testing/include/net/cfg80211.h
@@ -270,6 +270,23 @@ struct mpath_info {
u8 flags;
};
+/**
+ * struct bss_parameters - BSS parameters
+ *
+ * Used to change BSS parameters (mainly for AP mode).
+ *
+ * @use_cts_prot: Whether to use CTS protection
+ * (0 = no, 1 = yes, -1 = do not change)
+ * @use_short_preamble: Whether the use of short preambles is allowed
+ * (0 = no, 1 = yes, -1 = do not change)
+ * @use_short_slot_time: Whether the use of short slot time is allowed
+ * (0 = no, 1 = yes, -1 = do not change)
+ */
+struct bss_parameters {
+ int use_cts_prot;
+ int use_short_preamble;
+ int use_short_slot_time;
+};
/* from net/wireless.h */
struct wiphy;
@@ -322,6 +339,8 @@ struct wiphy;
* @change_station: Modify a given station.
*
* @set_mesh_cfg: set mesh parameters (by now, just mesh id)
+ *
+ * @change_bss: Modify parameters for a given BSS.
*/
struct cfg80211_ops {
int (*add_virtual_intf)(struct wiphy *wiphy, char *name,
@@ -377,6 +396,9 @@ struct cfg80211_ops {
int (*dump_mpath)(struct wiphy *wiphy, struct net_device *dev,
int idx, u8 *dst, u8 *next_hop,
struct mpath_info *pinfo);
+
+ int (*change_bss)(struct wiphy *wiphy, struct net_device *dev,
+ struct bss_parameters *params);
};
#endif /* __NET_CFG80211_H */
Index: wireless-testing/net/mac80211/cfg.c
===================================================================
--- wireless-testing.orig/net/mac80211/cfg.c
+++ wireless-testing/net/mac80211/cfg.c
@@ -1055,6 +1055,35 @@ static int ieee80211_dump_mpath(struct w
}
#endif
+static int ieee80211_change_bss(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct bss_parameters *params)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+ struct ieee80211_sub_if_data *sdata;
+
+ if (dev == local->mdev)
+ return -EOPNOTSUPP;
+
+ sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+ printk(KERN_DEBUG "JKM-%s: cts=%d preamble=%d slot=%d\n",
+ __func__, params->use_cts_prot, params->use_short_preamble,
+ params->use_short_slot_time);
+ if (params->use_cts_prot >= 0)
+ sdata->bss_conf.use_cts_prot = params->use_cts_prot;
+ if (params->use_short_preamble >= 0)
+ sdata->bss_conf.use_short_preamble =
+ params->use_short_preamble;
+#if 0 /* TODO: need to notify low-level driver */
+ if (params->use_short_slot_time >= 0)
+ sdata->bss_conf.use_short_slot_time =
+ params->use_short_slot_time;
+#endif
+
+ return 0;
+}
+
struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -1079,4 +1108,5 @@ struct cfg80211_ops mac80211_config_ops
.get_mpath = ieee80211_get_mpath,
.dump_mpath = ieee80211_dump_mpath,
#endif
+ .change_bss = ieee80211_change_bss,
};
Index: wireless-testing/include/linux/nl80211.h
===================================================================
--- wireless-testing.orig/include/linux/nl80211.h
+++ wireless-testing/include/linux/nl80211.h
@@ -89,6 +89,8 @@
* @NL80211_CMD_DEL_PATH: Remove a mesh path identified by %NL80211_ATTR_MAC
* or, if no MAC address given, all mesh paths, on the interface identified
* by %NL80211_ATTR_IFINDEX.
+ * @NL80211_CMD_SET_BSS: Set BSS attributes for BSS identified by
+ * %NL80211_ATTR_IFINDEX.
*
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
@@ -127,6 +129,8 @@ enum nl80211_commands {
NL80211_CMD_NEW_MPATH,
NL80211_CMD_DEL_MPATH,
+ NL80211_CMD_SET_BSS,
+
/* add commands here */
/* used to define NL80211_CMD_MAX below */
@@ -134,6 +138,11 @@ enum nl80211_commands {
NL80211_CMD_MAX = __NL80211_CMD_AFTER_LAST - 1
};
+/*
+ * Allow user space programs to use #ifdef on new commands by defining them
+ * here
+ */
+#define NL80211_CMD_SET_BSS NL80211_CMD_SET_BSS
/**
* enum nl80211_attrs - nl80211 netlink attributes
@@ -192,6 +201,12 @@ enum nl80211_commands {
* @NL80211_ATTR_MNTR_FLAGS: flags, nested element with NLA_FLAG attributes of
* &enum nl80211_mntr_flags.
*
+ * @NL80211_ATTR_BSS_CTS_PROT: whether CTS protection is enabled (u8, 0 or 1)
+ * @NL80211_ATTR_BSS_SHORT_PREAMBLE: whether short preamble is enabled
+ * (u8, 0 or 1)
+ * @NL80211_ATTR_BSS_SHORT_SLOT_TIME: whether short slot time enabled
+ * (u8, 0 or 1)
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -237,6 +252,10 @@ enum nl80211_attrs {
NL80211_ATTR_KEY_DEFAULT_MGMT,
+ NL80211_ATTR_BSS_CTS_PROT,
+ NL80211_ATTR_BSS_SHORT_PREAMBLE,
+ NL80211_ATTR_BSS_SHORT_SLOT_TIME,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
Index: wireless-testing/net/wireless/nl80211.c
===================================================================
--- wireless-testing.orig/net/wireless/nl80211.c
+++ wireless-testing/net/wireless/nl80211.c
@@ -89,6 +89,10 @@ static struct nla_policy nl80211_policy[
[NL80211_ATTR_MPATH_NEXT_HOP] = { .type = NLA_U32 },
[NL80211_ATTR_KEY_DEFAULT_MGMT] = { .type = NLA_FLAG },
+
+ [NL80211_ATTR_BSS_CTS_PROT] = { .type = NLA_U8 },
+ [NL80211_ATTR_BSS_SHORT_PREAMBLE] = { .type = NLA_U8 },
+ [NL80211_ATTR_BSS_SHORT_SLOT_TIME] = { .type = NLA_U8 },
};
/* message building helper */
@@ -1560,6 +1564,48 @@ static int nl80211_del_mpath(struct sk_b
return err;
}
+static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *drv;
+ int err;
+ struct net_device *dev;
+ struct bss_parameters params;
+
+ memset(¶ms, 0, sizeof(params));
+ /* default to not changing parameters */
+ params.use_cts_prot = -1;
+ params.use_short_preamble = -1;
+ params.use_short_slot_time = -1;
+
+ if (info->attrs[NL80211_ATTR_BSS_CTS_PROT])
+ params.use_cts_prot =
+ nla_get_u8(info->attrs[NL80211_ATTR_BSS_CTS_PROT]);
+ if (info->attrs[NL80211_ATTR_BSS_SHORT_PREAMBLE])
+ params.use_short_preamble =
+ nla_get_u8(info->attrs[NL80211_ATTR_BSS_SHORT_PREAMBLE]);
+ if (info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME])
+ params.use_short_slot_time =
+ nla_get_u8(info->attrs[NL80211_ATTR_BSS_SHORT_SLOT_TIME]);
+
+ err = get_drv_dev_by_info_ifindex(info->attrs, &drv, &dev);
+ if (err)
+ return err;
+
+ if (!drv->ops->change_bss) {
+ err = -EOPNOTSUPP;
+ goto out;
+ }
+
+ rtnl_lock();
+ err = drv->ops->change_bss(&drv->wiphy, dev, ¶ms);
+ rtnl_unlock();
+
+ out:
+ cfg80211_put_dev(drv);
+ dev_put(dev);
+ return err;
+}
+
static struct genl_ops nl80211_ops[] = {
{
.cmd = NL80211_CMD_GET_WIPHY,
@@ -1691,6 +1737,12 @@ static struct genl_ops nl80211_ops[] = {
.policy = nl80211_policy,
.flags = GENL_ADMIN_PERM,
},
+ {
+ .cmd = NL80211_CMD_SET_BSS,
+ .doit = nl80211_set_bss,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ },
};
/* multicast groups */
--
Jouni Malinen PGP id EFC895FA
On Thu, 2008-08-07 at 17:15 +0300, Jouni Malinen wrote:
> On Thu, Aug 07, 2008 at 03:48:26PM +0200, Johannes Berg wrote:
>
> > I used to have a patch to move the short-slot stuff in mac80211 to the
> > BSS info struct so that drivers that support different slot timing for
> > each frame can use it for different virtual BSSes. It no longer applies
> > though:
> >
> > http://johannes.sipsolutions.net/patches/kernel/all/2008-08-04-12%3a26/013-BROKEN-mac80211-bss-slot-time.patch
>
> I'll take a look at this. I hoped I would not need to go through all
> drivers to update the API ;-), but let's see.
Yeah, that was a sticky point... But if you want per-BSS information you
really have to because right now it's per-hw.
> > I think you may need to handle the case where dev is a VLAN? Or do we
> > want to have these settings per VLAN too? And should we reject them for
> > other modes than AP where the AP controls the settings?
>
> I was thinking about this a bit, but since ieee80211_sub_if_data had
> them shared for all types on interfaces and there might be some testing
> uses for these even outside AP mode, I did not bother adding extra
> validation here. At least the three parameters added here won't be
> per-VLAN since they are advertised in Beacon frames. Though, maybe the
> easiest option would be to limit this for IEEE80211_IF_TYPE_AP for now..
Ok, they can't be per VLAN then, true, but then I think we should limit
it to _AP just so nobody tries setting them without effect on other
devices or overrides, for _STA, what the AP said. For mesh I don't know
(adding Luis) and for IBSS I'm not sure either. I suspect you could use
it for IBSS too?
> > > + * @NL80211_CMD_SET_BSS: Set BSS attributes for BSS identified by
> > > + * %NL80211_ATTR_IFINDEX.
> >
> > Maybe we should have a way to retrieve the settings as well, and that
> > could even work in STA mode to see what the AP asked us to do.
>
> I did not see any normal use case for this, i.e., this would be more
> like debug information. CTS protection is already exported via debugfs
> and the other parameters could fit in there better, too.
True, there's little value in this information for regular use.
johannes
On Thu, Aug 07, 2008 at 04:25:10PM +0200, Johannes Berg wrote:
> > + NL80211_ATTR_BSS_SHORT_SLOT_TIME,
> Oh another idea I just had, I suspect we should use these to indicate
> support for the features too, on the wiphy info query result or
> something. Only tangentially related to this patch though.
Sounds reasonable. So far, I've mainly worked with hardware that
supports these, so I have just assumed that everything is fine and
haven't bothered dynamically trying to decide what to do in hostapd. Or
well, it was possible to configure this in hostapd.conf (though, it
looks like that was never actually merged into the open source version
completely, so the variable is there, but not a way to enable it; have
to fix that at some point, I guess..).
--
Jouni Malinen PGP id EFC895FA
On Thu, Aug 07, 2008 at 03:48:26PM +0200, Johannes Berg wrote:
> I used to have a patch to move the short-slot stuff in mac80211 to the
> BSS info struct so that drivers that support different slot timing for
> each frame can use it for different virtual BSSes. It no longer applies
> though:
>
> http://johannes.sipsolutions.net/patches/kernel/all/2008-08-04-12%3a26/013-BROKEN-mac80211-bss-slot-time.patch
I'll take a look at this. I hoped I would not need to go through all
drivers to update the API ;-), but let's see.
> I think you may need to handle the case where dev is a VLAN? Or do we
> want to have these settings per VLAN too? And should we reject them for
> other modes than AP where the AP controls the settings?
I was thinking about this a bit, but since ieee80211_sub_if_data had
them shared for all types on interfaces and there might be some testing
uses for these even outside AP mode, I did not bother adding extra
validation here. At least the three parameters added here won't be
per-VLAN since they are advertised in Beacon frames. Though, maybe the
easiest option would be to limit this for IEEE80211_IF_TYPE_AP for now..
> > + * @NL80211_CMD_SET_BSS: Set BSS attributes for BSS identified by
> > + * %NL80211_ATTR_IFINDEX.
>
> Maybe we should have a way to retrieve the settings as well, and that
> could even work in STA mode to see what the AP asked us to do.
I did not see any normal use case for this, i.e., this would be more
like debug information. CTS protection is already exported via debugfs
and the other parameters could fit in there better, too.
> Other than that, looks good to me.
Thanks!
--
Jouni Malinen PGP id EFC895FA
> + NL80211_ATTR_BSS_CTS_PROT,
> + NL80211_ATTR_BSS_SHORT_PREAMBLE,
> + NL80211_ATTR_BSS_SHORT_SLOT_TIME,
Oh another idea I just had, I suspect we should use these to indicate
support for the features too, on the wiphy info query result or
something. Only tangentially related to this patch though.
johannes
On Thu, Aug 07, 2008 at 04:46:28PM +0200, Johannes Berg wrote:
> On Thu, 2008-08-07 at 17:27 +0300, Jouni Malinen wrote:
> > Well, I don't actually need it per-BSS for hardware that I'm most likely
> > to work with ;-), but yeah, this is a per-BSS parameter, so in that
> > sense your patch is doing the correct thing.
>
> Yeah, ok, I guess I don't really care, do whichever way you want. It'd
> be a bit strange to set it with cfg80211 per BSS and have it show up on
> different BSSes, but that's about it, and a little strangeness hasn't
> hurt anyone ;)
It looks like it is easiest to make this a per-BSS parameter now and use
bss_info_changed() for it. However, I'm planning on leaving the other
parts of your patch out from the initial version. In other words, I will
not remove IEEE80211_CONF_SHORT_SLOT_TIME now (I just added a TODO note
to get it removed once drivers have been converted; mac80211 was never
actually setting this, anyway). This avoids the need to modify all
drivers with this change. In addition, I'm leaving out the client MLME
stuff for now, but the notification part will be available for it once
the MLME code is added.
--
Jouni Malinen PGP id EFC895FA
Thanks.
> Could you please take a quick look at whether this looks like a correct
> way of introducing new functionality into cfg80211? I'll finish the
> short slot time part in mac80211 and post an updated patch as a proper
> submission later.
I used to have a patch to move the short-slot stuff in mac80211 to the
BSS info struct so that drivers that support different slot timing for
each frame can use it for different virtual BSSes. It no longer applies
though:
http://johannes.sipsolutions.net/patches/kernel/all/2008-08-04-12%3a26/013-BROKEN-mac80211-bss-slot-time.patch
> +static int ieee80211_change_bss(struct wiphy *wiphy,
> + struct net_device *dev,
> + struct bss_parameters *params)
> +{
> + struct ieee80211_local *local = wiphy_priv(wiphy);
> + struct ieee80211_sub_if_data *sdata;
> +
> + if (dev == local->mdev)
> + return -EOPNOTSUPP;
> +
> + sdata = IEEE80211_DEV_TO_SUB_IF(dev);
I think you may need to handle the case where dev is a VLAN? Or do we
want to have these settings per VLAN too? And should we reject them for
other modes than AP where the AP controls the settings?
> + * @NL80211_CMD_SET_BSS: Set BSS attributes for BSS identified by
> + * %NL80211_ATTR_IFINDEX.
Maybe we should have a way to retrieve the settings as well, and that
could even work in STA mode to see what the AP asked us to do.
Other than that, looks good to me.
johannes
On Thu, 2008-08-07 at 17:27 +0300, Jouni Malinen wrote:
> > Yeah, that was a sticky point... But if you want per-BSS information you
> > really have to because right now it's per-hw.
>
> Well, I don't actually need it per-BSS for hardware that I'm most likely
> to work with ;-), but yeah, this is a per-BSS parameter, so in that
> sense your patch is doing the correct thing.
Yeah, ok, I guess I don't really care, do whichever way you want. It'd
be a bit strange to set it with cfg80211 per BSS and have it show up on
different BSSes, but that's about it, and a little strangeness hasn't
hurt anyone ;)
> > Ok, they can't be per VLAN then, true, but then I think we should limit
> > it to _AP just so nobody tries setting them without effect on other
> > devices or overrides, for _STA, what the AP said. For mesh I don't know
> > (adding Luis) and for IBSS I'm not sure either. I suspect you could use
> > it for IBSS too?
>
> I think I'll just make it _AP only for now and if someone ever really
> needs it for something else, they can enable it later.. It is possible
> that new attributes that would be more reasonable for other modes will
> be added in the future for _SET_BSS.
Sure, sounds fine.
johannes
On Thu, Aug 07, 2008 at 04:18:52PM +0200, Johannes Berg wrote:
> On Thu, 2008-08-07 at 17:15 +0300, Jouni Malinen wrote:
> > On Thu, Aug 07, 2008 at 03:48:26PM +0200, Johannes Berg wrote:
> > > I used to have a patch to move the short-slot stuff in mac80211 to the
> > > BSS info struct so that drivers that support different slot timing for
> > > each frame can use it for different virtual BSSes.
> Yeah, that was a sticky point... But if you want per-BSS information you
> really have to because right now it's per-hw.
Well, I don't actually need it per-BSS for hardware that I'm most likely
to work with ;-), but yeah, this is a per-BSS parameter, so in that
sense your patch is doing the correct thing.
> Ok, they can't be per VLAN then, true, but then I think we should limit
> it to _AP just so nobody tries setting them without effect on other
> devices or overrides, for _STA, what the AP said. For mesh I don't know
> (adding Luis) and for IBSS I'm not sure either. I suspect you could use
> it for IBSS too?
I think I'll just make it _AP only for now and if someone ever really
needs it for something else, they can enable it later.. It is possible
that new attributes that would be more reasonable for other modes will
be added in the future for _SET_BSS.
As far as IBSS is concerned, short slot time is disabled (802.11-2007,
19.4.4). Similarly, it is somewhat difficult to require the STAs in an
IBSS to control this type of disable-if-anyone-does-not-support
features. It is more likely to end up being
first-STA-to-create-IBSS-wins situation. Anyway, we do not have user
space IBSS control daemon anyway, so this is something that is not
needed yet.
--
Jouni Malinen PGP id EFC895FA
On Thu, 2008-08-07 at 17:32 +0300, Jouni Malinen wrote:
> On Thu, Aug 07, 2008 at 04:25:10PM +0200, Johannes Berg wrote:
>
> > > + NL80211_ATTR_BSS_SHORT_SLOT_TIME,
>
> > Oh another idea I just had, I suspect we should use these to indicate
> > support for the features too, on the wiphy info query result or
> > something. Only tangentially related to this patch though.
>
> Sounds reasonable. So far, I've mainly worked with hardware that
> supports these, so I have just assumed that everything is fine and
> haven't bothered dynamically trying to decide what to do in hostapd. Or
> well, it was possible to configure this in hostapd.conf (though, it
> looks like that was never actually merged into the open source version
> completely, so the variable is there, but not a way to enable it; have
> to fix that at some point, I guess..).
Ok, I think the only hardware we currently support that doesn't have
these doesn't support AP mode either, so it's probably not really worth
it now...
johannes