Re: [RFC] cfg80211: Add support for FILS shared key authentication offload

[Johannes Berg <johannes@sipsolutions.net>]

>  struct cfg80211_pmksa {
>  	const u8 *bssid;
>  	const u8 *pmkid;
> +	const u8 *pmk;
> +	size_t pmk_len;
> +	const u8 *ssid;
> +	size_t ssid_len;
> +	u16 cache_id;
>  };

So how does that work - from a flow perspective?

> +void cfg80211_connect_done(struct net_device *dev, const u8 *bssid,
> +			   struct cfg80211_bss *bss, const u8
> *req_ie,
> +			   size_t req_ie_len, const u8 *resp_ie,
> +			   size_t resp_ie_len, const u8 *fils_kek,
> +			   size_t fils_kek_len, bool
> update_erp_seq_num,
> +			   u16 fils_erp_seq_num, const u8 *pmk,
> size_t pmk_len,
> +			   const u8 *pmkid, int status, gfp_t gfp,
> +			   enum nl80211_timeout_reason
> timeout_reason);

Ouch. Please declare most of those (perhaps other than dev) as a
structure and pass a pointer here.

Also, why require the BSSID when BSS is set? Seems you could say one or
the other.

Also, what do you need the PMK for? If you have all of this stuff
offloaded, why does the host need to know the PMK?

> @@ -227,6 +227,13 @@ struct cfg80211_event {
>  			size_t req_ie_len;
>  			size_t resp_ie_len;
>  			struct cfg80211_bss *bss;
> +			const u8 *kek;
> +			size_t kek_len;
> +			bool update_erp_seq_num;
> +			u16 fils_erp_seq_num;
> +			const u8 *pmk;
> +			size_t pmk_len;
> +			const u8 *pmkid;
>  			int status; /* -1 = failed; 0..65535 =
> status code */
>  			enum nl80211_timeout_reason timeout_reason;
>  		} cr;

Could probably use the new struct from cfg80211.h that I asked to add
here then.


Other than that, most of the patch seems mechanical - I think it'd be
good to have a DOC: section or so somewhere (nl80211.h?) that explains
how this is intended to be used? I've already migrated the
documentation to the new rst/sphinx format so we can generate it nicely
and that would be a good addition - I'll probably do some more
additions too.

johannes