Re: [PATCH v1] cfg80211: Support for 4-way handshake offloading for WPA2-Personal

[Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>]

On Monday, August 20, 2012 01:20:29 PM Johannes Berg wrote:
> On Thu, 2012-08-02 at 14:33 +0300, Vladimir Kondratiev wrote:
> > + * @NL80211_ATTR_4WAY_HANDSHAKE_OFFLOAD_STA: This is a flag that
> > indicates to + *	WPA supplicant that 4-way handshake has been offloaded
> > to the firmware + *	in the STA mode.
> > + *	In this case, WPA supplicant will provide driver with PSK for
> > + *	connect()
> > + *
> > + * @NL80211_ATTR_4WAY_HANDSHAKE_OFFLOAD_AP: This is a flag that indicates
> > to + *	WPA supplicant that 4-way handshake has been offloaded to the
> > firmware + *	in the AP mode.
> > + *	In this case, WPA supplicant will provide driver with PSK for
> > + *	start_ap()
> 
> I think you should use the nl80211 driver flags, not extra attributes
> here.

I thought I need to use same approach as for other flags. All other use bit in 
wiphy.flags and NL80211_ATTR_xxx. And reason is simple - in the NL80211, it is 
enum, while for the driver one need to reserve a bit. This is how other flags 
are dealt with:

	if ((dev->wiphy.flags & WIPHY_FLAG_IBSS_RSN) &&
	    nla_put_flag(msg, NL80211_ATTR_SUPPORT_IBSS_RSN))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_MESH_AUTH) &&
	    nla_put_flag(msg, NL80211_ATTR_SUPPORT_MESH_AUTH))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_AP_UAPSD) &&
	    nla_put_flag(msg, NL80211_ATTR_SUPPORT_AP_UAPSD))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_SUPPORTS_FW_ROAM) &&
	    nla_put_flag(msg, NL80211_ATTR_ROAM_SUPPORT))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS) &&
	    nla_put_flag(msg, NL80211_ATTR_TDLS_SUPPORT))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_TDLS_EXTERNAL_SETUP) &&
	    nla_put_flag(msg, NL80211_ATTR_TDLS_EXTERNAL_SETUP))
		goto nla_put_failure;
	if ((dev->wiphy.flags & WIPHY_FLAG_4WAY_HANDSHAKE_OFFLOAD_STA) &&
	    nla_put_flag(msg, NL80211_ATTR_4WAY_HANDSHAKE_OFFLOAD_STA))
		goto nla_put_failure;

> 
> And the descriptions (and flag checking!) should make it clear that it
> is only supported with the connect() API, not with the auth/assoc APIs.
> 
> > + * @WIPHY_FLAG_4WAY_HANDSHAKE_OFFLOAD: Device supports 4-way handshake
> > offload.
> Use nl80211 flag directly. Also, docs are wrong:
> > +	WIPHY_FLAG_4WAY_HANDSHAKE_OFFLOAD_STA	= BIT(22),
> > +	WIPHY_FLAG_4WAY_HANDSHAKE_OFFLOAD_AP	= BIT(23),
> 
> Please also add validation that these flags are specified only when the
> correct APIs (connect/start_ap) are supported.

Agree, will add both doc and validation.

I want to do some more work exploring how this offload serves in real life, 
will submit when (if?) it will be ready.

Thanks, Vladimir.