2011-11-08 14:35:36

by Arik Nemtsov

[permalink] [raw]
Subject: [PATCH v3 1/3] nl80211: Add probe response offload attribute

Notify user-space about probe-response offloading support in the driver.

A wiphy flag is used to indicate support and a bitmap of protocols
determines which protocols are supported.

Signed-off-by: Guy Eilam <[email protected]>
Signed-off-by: Arik Nemtsov <[email protected]>
---
v1->3:
changed wording and added 80211u bit for supported offload protocols

include/linux/nl80211.h | 28 ++++++++++++++++++++++++++++
include/net/cfg80211.h | 5 +++++
net/wireless/nl80211.c | 5 +++++
3 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 8049bf7..e74afc4 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1109,6 +1109,11 @@ enum nl80211_commands {
* %NL80211_CMD_TDLS_MGMT. Otherwise %NL80211_CMD_TDLS_OPER should be
* used for asking the driver to perform a TDLS operation.
*
+ * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates support for probe
+ * response offloading by the driver/firmware.
+ * In addition this attribute holds a bitmap of the supported protocols
+ * for offloading using &enum nl80211_probe_resp_offload_support_attr.
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1337,6 +1342,8 @@ enum nl80211_attrs {
NL80211_ATTR_TDLS_SUPPORT,
NL80211_ATTR_TDLS_EXTERNAL_SETUP,

+ NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -2650,4 +2657,25 @@ enum nl80211_tdls_operation {
NL80211_TDLS_DISABLE_LINK,
};

+/**
+ * enum nl80211_probe_resp_offload_support_attr - definition of optional
+ * supported protocols for probe response offloading by the driver/FW.
+ * to be used with the %NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT
+ * attribute. Each enum value represents a bit in the bitmap of
+ * supported protocols. Typically a subset of probe-requests belonging
+ * to a supported protocol will be excluded from offload and uploaded
+ * to the host.
+ *
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS: Support for WPS ver. 1
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS2: Support for WPS ver. 2
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P: Support for P2P
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_80211U: Support for 802.11u
+ */
+enum nl80211_probe_resp_offload_support_attr {
+ NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS = 1<<0,
+ NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WPS2 = 1<<1,
+ NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P = 1<<2,
+ NL80211_PROBE_RESP_OFFLOAD_SUPPORT_80211U = 1<<3,
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 92cf1c2..7939495 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1693,6 +1693,7 @@ enum wiphy_flags {
WIPHY_FLAG_AP_UAPSD = BIT(14),
WIPHY_FLAG_SUPPORTS_TDLS = BIT(15),
WIPHY_FLAG_TDLS_EXTERNAL_SETUP = BIT(16),
+ WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD = BIT(17),
};

/**
@@ -1956,6 +1957,10 @@ struct wiphy {
u32 available_antennas_tx;
u32 available_antennas_rx;

+ /* bitmap of supported protocols for probe response offloading
+ * see enum nl80211_probe_resp_offload_support_attr */
+ u32 probe_resp_offload;
+
/* If multiple wiphys are registered and you're handed e.g.
* a regular netdev with assigned ieee80211_ptr, you won't
* know whether it points to a wiphy your driver has registered
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b3a476f..9800f8d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -758,6 +758,11 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
NLA_PUT_U32(msg, NL80211_ATTR_WIPHY_ANTENNA_AVAIL_RX,
dev->wiphy.available_antennas_rx);

+ if (dev->wiphy.flags & WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD)
+ NLA_PUT_U32(msg,
+ NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT,
+ dev->wiphy.probe_resp_offload);
+
if ((dev->wiphy.available_antennas_tx ||
dev->wiphy.available_antennas_rx) && dev->ops->get_antenna) {
u32 tx_ant = 0, rx_ant = 0;
--
1.7.5.4



2011-11-08 14:35:40

by Arik Nemtsov

[permalink] [raw]
Subject: [PATCH v3 3/3] mac80211: Save probe response data for bss

Allow setting a probe response template for an interface operating in
AP mode. Low level drivers are notified about changes in the probe
response template and are able to retrieve a copy of the current probe
response. This data can, for example, be uploaded to hardware as a
template.

Signed-off-by: Guy Eilam <[email protected]>
Signed-off-by: Arik Nemtsov <[email protected]>
---
v1->3:
save the probe response in mac80211 when configuring the new beacon,
not part of a BSS change.

include/net/mac80211.h | 15 +++++++++++++++
net/mac80211/cfg.c | 44 +++++++++++++++++++++++++++++++++++++++++---
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/iface.c | 6 +++++-
net/mac80211/tx.c | 31 +++++++++++++++++++++++++++++++
net/mac80211/util.c | 3 ++-
6 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 72eddd1..0f1d784 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -166,6 +166,7 @@ struct ieee80211_low_level_stats {
* that it is only ever disabled for station mode.
* @BSS_CHANGED_IDLE: Idle changed for this BSS/interface.
* @BSS_CHANGED_SSID: SSID changed for this BSS (AP mode)
+ * @BSS_CHANGED_AP_PROBE_RESP: Probe Response changed for this BSS (AP mode)
*/
enum ieee80211_bss_change {
BSS_CHANGED_ASSOC = 1<<0,
@@ -184,6 +185,7 @@ enum ieee80211_bss_change {
BSS_CHANGED_QOS = 1<<13,
BSS_CHANGED_IDLE = 1<<14,
BSS_CHANGED_SSID = 1<<15,
+ BSS_CHANGED_AP_PROBE_RESP = 1<<16,

/* when adding here, make sure to change ieee80211_reconfig */
};
@@ -2661,6 +2663,19 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
}

/**
+ * ieee80211_proberesp_get - retrieve a Probe Response template
+ * @hw: pointer obtained from ieee80211_alloc_hw().
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ *
+ * Creates a Probe Response template which can, for example, be uploaded to
+ * hardware. The destination address should be set by the caller.
+ *
+ * Can only be called in AP mode.
+ */
+struct sk_buff *ieee80211_proberesp_get(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif);
+
+/**
* ieee80211_pspoll_get - retrieve a PS Poll template
* @hw: pointer obtained from ieee80211_alloc_hw().
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a9ded52..a06e695 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -488,6 +488,37 @@ static void ieee80211_config_ap_ssid(struct ieee80211_sub_if_data *sdata,
(params->hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE);
}

+static int ieee80211_set_probe_resp(struct ieee80211_sub_if_data *sdata,
+ u8 *resp, size_t resp_len)
+{
+ struct sk_buff *new, *old;
+
+ if (resp_len == 0)
+ return -EINVAL;
+
+ old = sdata->u.ap.probe_resp;
+
+ if (!resp || !resp_len)
+ return -EINVAL;
+
+ new = dev_alloc_skb(resp_len);
+ if (!new) {
+ printk(KERN_DEBUG "%s: failed to allocate buffer for probe "
+ "response template\n", sdata->name);
+ return -ENOMEM;
+ }
+
+ memcpy(skb_put(new, resp_len), resp, resp_len);
+
+ rcu_assign_pointer(sdata->u.ap.probe_resp, new);
+ synchronize_rcu();
+
+ if (old)
+ dev_kfree_skb(old);
+
+ return 0;
+}
+
/*
* This handles both adding a beacon and setting new beacon info
*/
@@ -498,6 +529,7 @@ static int ieee80211_config_beacon(struct ieee80211_sub_if_data *sdata,
int new_head_len, new_tail_len;
int size;
int err = -EINVAL;
+ u32 changed = 0;

old = rtnl_dereference(sdata->u.ap.beacon);

@@ -581,11 +613,17 @@ static int ieee80211_config_beacon(struct ieee80211_sub_if_data *sdata,

kfree(old);

+ err = ieee80211_set_probe_resp(sdata, params->probe_resp,
+ params->probe_resp_len);
+ if (!err)
+ changed |= BSS_CHANGED_AP_PROBE_RESP;
+
ieee80211_config_ap_ssid(sdata, params);
+ changed |= BSS_CHANGED_BEACON_ENABLED |
+ BSS_CHANGED_BEACON |
+ BSS_CHANGED_SSID;

- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED |
- BSS_CHANGED_BEACON |
- BSS_CHANGED_SSID);
+ ieee80211_bss_info_change_notify(sdata, changed);
return 0;
}

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ea10a51..5a351cc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -228,6 +228,7 @@ struct beacon_data {

struct ieee80211_if_ap {
struct beacon_data __rcu *beacon;
+ struct sk_buff __rcu *probe_resp;

struct list_head vlans;

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index ef741e8..74d6f79 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -450,15 +450,19 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
struct ieee80211_sub_if_data *vlan, *tmpsdata;
struct beacon_data *old_beacon =
rtnl_dereference(sdata->u.ap.beacon);
+ struct sk_buff *old_probe_resp =
+ rtnl_dereference(sdata->u.ap.probe_resp);

/* sdata_running will return false, so this will disable */
ieee80211_bss_info_change_notify(sdata,
BSS_CHANGED_BEACON_ENABLED);

- /* remove beacon */
+ /* remove beacon and probe response */
rcu_assign_pointer(sdata->u.ap.beacon, NULL);
+ rcu_assign_pointer(sdata->u.ap.probe_resp, NULL);
synchronize_rcu();
kfree(old_beacon);
+ kfree(old_probe_resp);

/* down all dependent devices, that is VLANs */
list_for_each_entry_safe(vlan, tmpsdata, &sdata->u.ap.vlans,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 48bbb96..6b86f77 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2354,6 +2354,37 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_beacon_get_tim);

+struct sk_buff *ieee80211_proberesp_get(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif)
+{
+ struct ieee80211_if_ap *ap = NULL;
+ struct sk_buff *presp = NULL, *skb = NULL;
+ struct ieee80211_hdr *hdr;
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+ if (sdata->vif.type != NL80211_IFTYPE_AP)
+ return NULL;
+
+ rcu_read_lock();
+
+ ap = &sdata->u.ap;
+ presp = rcu_dereference(ap->probe_resp);
+ if (!presp)
+ goto out;
+
+ skb = skb_copy(presp, GFP_ATOMIC);
+ if (!skb)
+ goto out;
+
+ hdr = (struct ieee80211_hdr *) skb->data;
+ memset(hdr->addr1, 0, sizeof(hdr->addr1));
+
+out:
+ rcu_read_unlock();
+ return skb;
+}
+EXPORT_SYMBOL(ieee80211_proberesp_get);
+
struct sk_buff *ieee80211_pspoll_get(struct ieee80211_hw *hw,
struct ieee80211_vif *vif)
{
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 7439d26..d6d3ef4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1085,7 +1085,8 @@ int ieee80211_reconfig(struct ieee80211_local *local)
changed |= BSS_CHANGED_IBSS;
/* fall through */
case NL80211_IFTYPE_AP:
- changed |= BSS_CHANGED_SSID;
+ changed |= BSS_CHANGED_SSID |
+ BSS_CHANGED_AP_PROBE_RESP;
/* fall through */
case NL80211_IFTYPE_MESH_POINT:
changed |= BSS_CHANGED_BEACON |
--
1.7.5.4


2011-11-08 15:34:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mac80211: Save probe response data for bss

On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:

> + new = dev_alloc_skb(resp_len);
> + if (!new) {
> + printk(KERN_DEBUG "%s: failed to allocate buffer for probe "
> + "response template\n", sdata->name);

I'm pretty sure that will already warn so the message is probably not
very useful.

johannes


2011-11-08 16:05:09

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] nl80211: Pass probe response data to drivers

On Tue, Nov 8, 2011 at 17:32, Johannes Berg <[email protected]> wrote:
> On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
>> Pass probe-response data from usermode via beacon parameters.
>>
>> Signed-off-by: Guy Eilam <[email protected]>
>> Signed-off-by: Arik Nemtsov <[email protected]>
>> ---
>> v1->3:
>> Set the probe response data as part of the addset_beacon function, instead
>> of set_bss. This makes more sense since the probe response is always
>> updated with the beacon.
>>
>> include/linux/nl80211.h | ? ?4 ++++
>> ?include/net/cfg80211.h ?| ? ?4 ++++
>> ?net/wireless/nl80211.c ?| ? ?9 +++++++++
>> ?3 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
>> index e74afc4..3fa03b3 100644
>> --- a/include/linux/nl80211.h
>> +++ b/include/linux/nl80211.h
>> @@ -1114,6 +1114,8 @@ enum nl80211_commands {
>> ? * ? In addition this attribute holds a bitmap of the supported protocols
>> ? * ? for offloading using &enum nl80211_probe_resp_offload_support_attr.
>> ? *
>> + * @NL80211_ATTR_PROBE_RESP: Probe Response template data
>> + *
>
> That should say what kind of data is expected -- I think a full frame in
> this case? Should it really be a full frame though? Maybe ath6kl
> requires something else and we can make this somehow compatible with
> both?

I can be more verbose in the description, sure. The probe-resp IEs are
a subset of the full probe-resp template, so I guess we could give ath6kl a
start_pos, end_pos into the full probe_resp buffer.

But IMHO it's cleaner the way it is now.

2011-11-08 14:35:38

by Arik Nemtsov

[permalink] [raw]
Subject: [PATCH v3 2/3] nl80211: Pass probe response data to drivers

Pass probe-response data from usermode via beacon parameters.

Signed-off-by: Guy Eilam <[email protected]>
Signed-off-by: Arik Nemtsov <[email protected]>
---
v1->3:
Set the probe response data as part of the addset_beacon function, instead
of set_bss. This makes more sense since the probe response is always
updated with the beacon.

include/linux/nl80211.h | 4 ++++
include/net/cfg80211.h | 4 ++++
net/wireless/nl80211.c | 9 +++++++++
3 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index e74afc4..3fa03b3 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -1114,6 +1114,8 @@ enum nl80211_commands {
* In addition this attribute holds a bitmap of the supported protocols
* for offloading using &enum nl80211_probe_resp_offload_support_attr.
*
+ * @NL80211_ATTR_PROBE_RESP: Probe Response template data
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1344,6 +1346,8 @@ enum nl80211_attrs {

NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT,

+ NL80211_ATTR_PROBE_RESP,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7939495..97f3e85 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -391,6 +391,8 @@ struct cfg80211_crypto_settings {
* @assocresp_ies: extra information element(s) to add into (Re)Association
* Response frames or %NULL
* @assocresp_ies_len: length of assocresp_ies in octets
+ * @probe_resp_len: length of probe response template (@probe_resp)
+ * @probe_resp: probe response template (AP mode only)
*/
struct beacon_parameters {
u8 *head, *tail;
@@ -408,6 +410,8 @@ struct beacon_parameters {
size_t proberesp_ies_len;
const u8 *assocresp_ies;
size_t assocresp_ies_len;
+ int probe_resp_len;
+ u8 *probe_resp;
};

/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9800f8d..10d5b1f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -196,6 +196,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_TDLS_OPERATION] = { .type = NLA_U8 },
[NL80211_ATTR_TDLS_SUPPORT] = { .type = NLA_FLAG },
[NL80211_ATTR_TDLS_EXTERNAL_SETUP] = { .type = NLA_FLAG },
+ [NL80211_ATTR_PROBE_RESP] = { .type = NLA_BINARY,
+ .len = IEEE80211_MAX_DATA_LEN },
};

/* policy for the key attributes */
@@ -2160,6 +2162,13 @@ static int nl80211_addset_beacon(struct sk_buff *skb, struct genl_info *info)
nla_len(info->attrs[NL80211_ATTR_IE_ASSOC_RESP]);
}

+ if (info->attrs[NL80211_ATTR_PROBE_RESP]) {
+ params.probe_resp =
+ nla_data(info->attrs[NL80211_ATTR_PROBE_RESP]);
+ params.probe_resp_len =
+ nla_len(info->attrs[NL80211_ATTR_PROBE_RESP]);
+ }
+
err = call(&rdev->wiphy, dev, &params);
if (!err && params.interval)
wdev->beacon_interval = params.interval;
--
1.7.5.4


2011-11-08 16:07:30

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] mac80211: Save probe response data for bss

On Tue, Nov 8, 2011 at 17:34, Johannes Berg <[email protected]> wrote:
> On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
>
>> + ? ? new = dev_alloc_skb(resp_len);
>> + ? ? if (!new) {
>> + ? ? ? ? ? ? printk(KERN_DEBUG "%s: failed to allocate buffer for probe "
>> + ? ? ? ? ? ? ? ? ? ?"response template\n", sdata->name);
>
> I'm pretty sure that will already warn so the message is probably not
> very useful.

Sure. I'll remove it.

2011-11-08 15:58:54

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] nl80211: Add probe response offload attribute

On Tue, 2011-11-08 at 17:53 +0200, Arik Nemtsov wrote:
> On Tue, Nov 8, 2011 at 17:31, Johannes Berg <[email protected]> wrote:
> > On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
> >> Notify user-space about probe-response offloading support in the driver.
> >>
> >> A wiphy flag is used to indicate support and a bitmap of protocols
> >> determines which protocols are supported.
> >
> >> + * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates support for probe
> >> + * response offloading by the driver/firmware.
> >> + * In addition this attribute holds a bitmap of the supported protocols
> >> + * for offloading using &enum nl80211_probe_resp_offload_support_attr.
> >
> > I wonder if this should make it more clear that probe requests will be
> > replied to, and this is not optional?
>
> Actually wl12xx "supports" the legacy way as well. We can operate
> without wpa_s explicitly setting the probe-resp (even though we just
> piece it together in other ways).
> In this sense, it's not a mandatory feature. If wpa_s fails to
> generate it for some reason, we don't want it to fail the AP load. The
> AP will operate just fine.

It'll operate, not sure about "just fine" :-) The way I see it that's
mostly a backward compatibility feature, no?

> >> +++ b/include/net/cfg80211.h
> >> @@ -1693,6 +1693,7 @@ enum wiphy_flags {
> >> WIPHY_FLAG_AP_UAPSD = BIT(14),
> >> WIPHY_FLAG_SUPPORTS_TDLS = BIT(15),
> >> WIPHY_FLAG_TDLS_EXTERNAL_SETUP = BIT(16),
> >> + WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD = BIT(17),
> >
> > Ditto here, maybe WIPHY_FLAG_HAS_PROBE_RESP_OFFLOAD?
> >
> > As we discussed, ath6kl & similar full-mac drivers with AP SME in the
> > device should set this to advertise the probe protocol feature set they
> > support (by passing up), and even for wl12xx it's not optional.
>
> All this has to do with user-space semantics. Currently hostap always
> sets the probe-resp IEs, without checking offloading support.

Which is fine too, but I think ath6kl should set this value right?

johannes


2011-11-08 15:32:44

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] nl80211: Pass probe response data to drivers

On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
> Pass probe-response data from usermode via beacon parameters.
>
> Signed-off-by: Guy Eilam <[email protected]>
> Signed-off-by: Arik Nemtsov <[email protected]>
> ---
> v1->3:
> Set the probe response data as part of the addset_beacon function, instead
> of set_bss. This makes more sense since the probe response is always
> updated with the beacon.
>
> include/linux/nl80211.h | 4 ++++
> include/net/cfg80211.h | 4 ++++
> net/wireless/nl80211.c | 9 +++++++++
> 3 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index e74afc4..3fa03b3 100644
> --- a/include/linux/nl80211.h
> +++ b/include/linux/nl80211.h
> @@ -1114,6 +1114,8 @@ enum nl80211_commands {
> * In addition this attribute holds a bitmap of the supported protocols
> * for offloading using &enum nl80211_probe_resp_offload_support_attr.
> *
> + * @NL80211_ATTR_PROBE_RESP: Probe Response template data
> + *

That should say what kind of data is expected -- I think a full frame in
this case? Should it really be a full frame though? Maybe ath6kl
requires something else and we can make this somehow compatible with
both?

johannes


2011-11-08 16:10:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] nl80211: Pass probe response data to drivers

On Tue, 2011-11-08 at 18:04 +0200, Arik Nemtsov wrote:
> On Tue, Nov 8, 2011 at 17:32, Johannes Berg <[email protected]> wrote:
> > On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
> >> Pass probe-response data from usermode via beacon parameters.
> >>
> >> Signed-off-by: Guy Eilam <[email protected]>
> >> Signed-off-by: Arik Nemtsov <[email protected]>
> >> ---
> >> v1->3:
> >> Set the probe response data as part of the addset_beacon function, instead
> >> of set_bss. This makes more sense since the probe response is always
> >> updated with the beacon.
> >>
> >> include/linux/nl80211.h | 4 ++++
> >> include/net/cfg80211.h | 4 ++++
> >> net/wireless/nl80211.c | 9 +++++++++
> >> 3 files changed, 17 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> >> index e74afc4..3fa03b3 100644
> >> --- a/include/linux/nl80211.h
> >> +++ b/include/linux/nl80211.h
> >> @@ -1114,6 +1114,8 @@ enum nl80211_commands {
> >> * In addition this attribute holds a bitmap of the supported protocols
> >> * for offloading using &enum nl80211_probe_resp_offload_support_attr.
> >> *
> >> + * @NL80211_ATTR_PROBE_RESP: Probe Response template data
> >> + *
> >
> > That should say what kind of data is expected -- I think a full frame in
> > this case? Should it really be a full frame though? Maybe ath6kl
> > requires something else and we can make this somehow compatible with
> > both?
>
> I can be more verbose in the description, sure. The probe-resp IEs are
> a subset of the full probe-resp template, so I guess we could give ath6kl a
> start_pos, end_pos into the full probe_resp buffer.
>
> But IMHO it's cleaner the way it is now.

Agree, thanks for explaining (and on IRC too).

johannes


2011-11-08 15:31:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] nl80211: Add probe response offload attribute

On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
> Notify user-space about probe-response offloading support in the driver.
>
> A wiphy flag is used to indicate support and a bitmap of protocols
> determines which protocols are supported.

> + * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates support for probe
> + * response offloading by the driver/firmware.
> + * In addition this attribute holds a bitmap of the supported protocols
> + * for offloading using &enum nl80211_probe_resp_offload_support_attr.

I wonder if this should make it more clear that probe requests will be
replied to, and this is not optional?

> +++ b/include/net/cfg80211.h
> @@ -1693,6 +1693,7 @@ enum wiphy_flags {
> WIPHY_FLAG_AP_UAPSD = BIT(14),
> WIPHY_FLAG_SUPPORTS_TDLS = BIT(15),
> WIPHY_FLAG_TDLS_EXTERNAL_SETUP = BIT(16),
> + WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD = BIT(17),

Ditto here, maybe WIPHY_FLAG_HAS_PROBE_RESP_OFFLOAD?

As we discussed, ath6kl & similar full-mac drivers with AP SME in the
device should set this to advertise the probe protocol feature set they
support (by passing up), and even for wl12xx it's not optional.

johannes


2011-11-08 15:54:10

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] nl80211: Add probe response offload attribute

On Tue, Nov 8, 2011 at 17:31, Johannes Berg <[email protected]> wrote:
> On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
>> Notify user-space about probe-response offloading support in the driver.
>>
>> A wiphy flag is used to indicate support and a bitmap of protocols
>> determines which protocols are supported.
>
>> + * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates support for probe
>> + * ? response offloading by the driver/firmware.
>> + * ? In addition this attribute holds a bitmap of the supported protocols
>> + * ? for offloading using &enum nl80211_probe_resp_offload_support_attr.
>
> I wonder if this should make it more clear that probe requests will be
> replied to, and this is not optional?

Actually wl12xx "supports" the legacy way as well. We can operate
without wpa_s explicitly setting the probe-resp (even though we just
piece it together in other ways).
In this sense, it's not a mandatory feature. If wpa_s fails to
generate it for some reason, we don't want it to fail the AP load. The
AP will operate just fine.

>
>> +++ b/include/net/cfg80211.h
>> @@ -1693,6 +1693,7 @@ enum wiphy_flags {
>> ? ? ? WIPHY_FLAG_AP_UAPSD ? ? ? ? ? ? ? ? ? ? = BIT(14),
>> ? ? ? WIPHY_FLAG_SUPPORTS_TDLS ? ? ? ? ? ? ? ?= BIT(15),
>> ? ? ? WIPHY_FLAG_TDLS_EXTERNAL_SETUP ? ? ? ? ?= BIT(16),
>> + ? ? WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD ? = BIT(17),
>
> Ditto here, maybe WIPHY_FLAG_HAS_PROBE_RESP_OFFLOAD?
>
> As we discussed, ath6kl & similar full-mac drivers with AP SME in the
> device should set this to advertise the probe protocol feature set they
> support (by passing up), and even for wl12xx it's not optional.

All this has to do with user-space semantics. Currently hostap always
sets the probe-resp IEs, without checking offloading support.

Jouni/ath6kl people - are you ok with adding some checks before
sending the extra IEs to kernel, as ath6kl is using them?

Arik

2011-11-08 16:17:47

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] nl80211: Add probe response offload attribute

On Tue, Nov 8, 2011 at 17:58, Johannes Berg <[email protected]> wrote:
> On Tue, 2011-11-08 at 17:53 +0200, Arik Nemtsov wrote:
>> On Tue, Nov 8, 2011 at 17:31, Johannes Berg <[email protected]> wrote:
>> > On Tue, 2011-11-08 at 16:35 +0200, Arik Nemtsov wrote:
>> Actually wl12xx "supports" the legacy way as well. We can operate
>> without wpa_s explicitly setting the probe-resp (even though we just
>> piece it together in other ways).
>> In this sense, it's not a mandatory feature. If wpa_s fails to
>> generate it for some reason, we don't want it to fail the AP load. The
>> AP will operate just fine.
>
> It'll operate, not sure about "just fine" :-) The way I see it that's
> mostly a backward compatibility feature, no?

I guess so. I can make the wording more explicit there so as not to confuse.

>
>> >> +++ b/include/net/cfg80211.h
>> >> @@ -1693,6 +1693,7 @@ enum wiphy_flags {
>> >> ? ? ? WIPHY_FLAG_AP_UAPSD ? ? ? ? ? ? ? ? ? ? = BIT(14),
>> >> ? ? ? WIPHY_FLAG_SUPPORTS_TDLS ? ? ? ? ? ? ? ?= BIT(15),
>> >> ? ? ? WIPHY_FLAG_TDLS_EXTERNAL_SETUP ? ? ? ? ?= BIT(16),
>> >> + ? ? WIPHY_FLAG_SUPPORT_PROBE_RESP_OFFLOAD ? = BIT(17),
>> >
>> > Ditto here, maybe WIPHY_FLAG_HAS_PROBE_RESP_OFFLOAD?

sure.

>> >
>> > As we discussed, ath6kl & similar full-mac drivers with AP SME in the
>> > device should set this to advertise the probe protocol feature set they
>> > support (by passing up), and even for wl12xx it's not optional.
>>
>> All this has to do with user-space semantics. Currently hostap always
>> sets the probe-resp IEs, without checking offloading support.
>
> Which is fine too, but I think ath6kl should set this value right?

I'll add this to the kernel side for now, with no checks in hostap.

Arik