[PATCH v2 1/3] nl80211: Add probe response offload attribute

[PATCH v2 1/3] nl80211: Add probe response offload attribute

[Guy Eilam]

Notify the userspace of the probe response offloading
support by the driver.

Signed-off-by: Guy Eilam <guy@wizery.com>
---
v2:
use struct wiphy instead of a function pointer
this change made the previous second PATCH:
"Get the probe response offloading support from the driver" irrelevant
changed WPS to WSC

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

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 9d797f2..c1f0a3d 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 the support
+ *	of 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,
@@ -2648,4 +2655,21 @@ 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/firmware
+ *	to be used with the %NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT
+ *	attribute. Each enum value represents a bit in the bitmap of
+ *	supported protocols.
+ *
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC: Support for WSC ver. 1
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC2: Support for WSC ver. 2
+ * @NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P: Support for P2P
+ */
+enum nl80211_probe_resp_offload_support_attr {
+	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC =	1<<0,
+	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC2 =	1<<1,
+	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P =	1<<2,
+};
+
 #endif /* __LINUX_NL80211_H */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 74f4f85..c9f69a6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1690,6 +1690,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),
 };
 
 /**
@@ -1953,6 +1954,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 edf655a..71d36ed 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -759,6 +759,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.4.1

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Johannes Berg]

On Sat, 2011-10-22 at 21:51 +0200, Guy Eilam wrote:

> +/**
> + * enum nl80211_probe_resp_offload_support_attr - definition of optional
> + *	supported protocols for probe response offloading by the driver/firmware
> + *	to be used with the %NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT
> + *	attribute. Each enum value represents a bit in the bitmap of
> + *	supported protocols.

Oops I should have pointed that out before -- that's not valid
kernel-doc.

Also, maybe this should say how it'll be supported (i.e. by passing the
frame up for userspace to reply to, I think?)

johannes

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Kalle Valo]

Guy Eilam <guy@wizery.com> writes:

> Notify the userspace of the probe response offloading
> support by the driver.

[...]

> +enum nl80211_probe_resp_offload_support_attr {
> +	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC =	1<<0,
> +	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC2 =	1<<1,
> +	NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P =	1<<2,
> +};

Maybe BIT() macro (like below) would be better for consistency?

> @@ -1690,6 +1690,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),
>  };

-- 
Kalle Valo

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Johannes Berg]

> > +enum nl80211_probe_resp_offload_support_attr {
> > +    NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC =    1<<0,
> > +    NL80211_PROBE_RESP_OFFLOAD_SUPPORT_WSC2 =    1<<1,
> > +    NL80211_PROBE_RESP_OFFLOAD_SUPPORT_P2P =    1<<2,
> > +};
> 
> Maybe BIT() macro (like below) would be better for consistency?

No, this header is userspace visible, so BIT isn't easily available.

johannes

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Luciano Coelho]

On Sat, 2011-10-22 at 21:51 +0200, Guy Eilam wrote: 
> Notify the userspace of the probe response offloading
> support by the driver.
> 
> Signed-off-by: Guy Eilam <guy@wizery.com>
> ---
> v2:
> use struct wiphy instead of a function pointer
> this change made the previous second PATCH:
> "Get the probe response offloading support from the driver" irrelevant
> changed WPS to WSC
> 
>  include/linux/nl80211.h |   24 ++++++++++++++++++++++++
>  include/net/cfg80211.h  |    5 +++++
>  net/wireless/nl80211.c  |    5 +++++
>  3 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
> index 9d797f2..c1f0a3d 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 the support
> + *	of 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,
> +

I'm not sure I understand why we need this.  Why aren't the flags
themselves enough?

Johannes wrote, on a separate thread:
> Oh, and probably a regular WIPHY flag that indicates whether the
> attribute should be added at all so that it can also be 0 but present
> (presence with 0 value indicates something other than not present).

What would be the meaning when the WIPHY flag is set but the attributes
are all 0? Wouldn't it mean that we don't support probe_resp offload at
all? Or would it mean that we support probe_resp offloading in normal
cases (ie. not WCS nor P2P)? If the latter is the case, why not add a
bit in the attributes to indicate that "normal" probe_resp offloading is
supported? I think this would be cleaner because there wouldn't be any
implicit semantics.


-- 
Cheers,
Luca.

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Johannes Berg]

On Mon, 2011-10-31 at 12:04 +0200, Luciano Coelho wrote:

> > + * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates the support
> > + *	of 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'm not sure I understand why we need this.  Why aren't the flags
> themselves enough?

Not sure I understand the question?

> Johannes wrote, on a separate thread:
> > Oh, and probably a regular WIPHY flag that indicates whether the
> > attribute should be added at all so that it can also be 0 but present
> > (presence with 0 value indicates something other than not present).
> 
> What would be the meaning when the WIPHY flag is set but the attributes
> are all 0? Wouldn't it mean that we don't support probe_resp offload at
> all? Or would it mean that we support probe_resp offloading in normal
> cases (ie. not WCS nor P2P)? If the latter is the case, why not add a
> bit in the attributes to indicate that "normal" probe_resp offloading is
> supported? I think this would be cleaner because there wouldn't be any
> implicit semantics.

It would mean we don't support probe response offload at all, and as a
consequence would be more or less equivalent to having 0xfffffff as the
flags mask (everything is "supported").

Adding a flag for "normal" doesn't make sense: you can only ever reply
to "normal" probe requests, you need to send any "special" ones up to
the host. So each bit in this bitfield essentially says "I support this
by passing it to the host" -- a bit for "normal" doesn't make sense in
that context.

johannes

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Luciano Coelho]

On Wed, 2011-11-02 at 11:13 +0100, Johannes Berg wrote: 
> On Mon, 2011-10-31 at 12:04 +0200, Luciano Coelho wrote:
> 
> > > + * @NL80211_ATTR_PROBE_RESP_OFFLOAD_SUPPORT: Indicates the support
> > > + *	of 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'm not sure I understand why we need this.  Why aren't the flags
> > themselves enough?
> 
> Not sure I understand the question?

It was what I meant (and you answered) below. :)


> > Johannes wrote, on a separate thread:
> > > Oh, and probably a regular WIPHY flag that indicates whether the
> > > attribute should be added at all so that it can also be 0 but present
> > > (presence with 0 value indicates something other than not present).
> > 
> > What would be the meaning when the WIPHY flag is set but the attributes
> > are all 0? Wouldn't it mean that we don't support probe_resp offload at
> > all? Or would it mean that we support probe_resp offloading in normal
> > cases (ie. not WCS nor P2P)? If the latter is the case, why not add a
> > bit in the attributes to indicate that "normal" probe_resp offloading is
> > supported? I think this would be cleaner because there wouldn't be any
> > implicit semantics.
> 
> It would mean we don't support probe response offload at all, and as a
> consequence would be more or less equivalent to having 0xfffffff as the
> flags mask (everything is "supported").
> 
> Adding a flag for "normal" doesn't make sense: you can only ever reply
> to "normal" probe requests, you need to send any "special" ones up to
> the host. So each bit in this bitfield essentially says "I support this
> by passing it to the host" -- a bit for "normal" doesn't make sense in
> that context.

Okay, I get it now.  So the flags are actually telling which types of
probe responses the hardware can pass up to the host when probe_resp
offload is in use.

My confusion was because I thought the flags were actually telling which
type of probe_resps the HW could offload.

Thanks for clarifying.

-- 
Cheers,
Luca.

Re: [PATCH v2 1/3] nl80211: Add probe response offload attribute

[Arik Nemtsov]

On Sat, Oct 22, 2011 at 21:51, Guy Eilam <guy@wizery.com> wrote:
> Notify the userspace of the probe response offloading
> support by the driver.
>
> Signed-off-by: Guy Eilam <guy@wizery.com>
> ---
> v2:
> use struct wiphy instead of a function pointer
> this change made the previous second PATCH:
> "Get the probe response offloading support from the driver" irrelevant
> changed WPS to WSC

I'll revert the WSC wording to WPS in the v3 of the probe-resp
patches. Seems more appropriate in light of the new defunct AP_SME
attribute, and the naming conventions for this in hostap

Arik