[PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Gautam (Gautam Kumar) Shukla]

For offloading 4 way handshake to driver, currently we don't have any member  of struct cfg80211_connect_params to pass PSK from supplicant to driver. I have added psk for the same and added rest of the code needed in nl80211.h and nl80211.c to parse and make it available to driver.
>From supplicant, we already have psk member field in assoc_params to use .

Tested on x86 linux.

Signed-off-by: Gautam kumar shukla <gautams@broadcom.com>
---
include/net/cfg80211.h       | 2 ++
 include/uapi/linux/nl80211.h | 8 +++++++-
 net/wireless/nl80211.c       | 4 ++++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index a2ddcf2..6f744e0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1758,6 +1758,7 @@ struct cfg80211_ibss_params {
  *    allowed to ignore this @bssid_hint if it has knowledge of a better BSS
  *    to use.
  * @ssid: SSID
+ * @psk:preshared key for WPA2-PSK connection or %NULL if not specified
  * @ssid_len: Length of ssid in octets
  * @auth_type: Authentication type (algorithm)
  * @ie: IEs for association request
@@ -1783,6 +1784,7 @@ struct cfg80211_connect_params {
     const u8 *bssid;
     const u8 *bssid_hint;
     const u8 *ssid;
+    const u8 *psk;
     size_t ssid_len;
     enum nl80211_auth_type auth_type;
     const u8 *ie;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 4b28dc0..b01d5dd 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -421,7 +421,7 @@
  *    %NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT,
  *    %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
  *    %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and
- *    %NL80211_ATTR_WIPHY_FREQ_HINT.
+ *    %NL80211_ATTR_WIPHY_FREQ_HINT, and %NL80211_ATTR_PSK.
  *    If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are
  *    restrictions on BSS selection, i.e., they effectively prevent roaming
  *    within the ESS. %NL80211_ATTR_MAC_HINT and %NL80211_ATTR_WIPHY_FREQ_HINT @@ -1638,6 +1638,10 @@ enum nl80211_commands {
  * @NL80211_ATTR_SMPS_MODE: SMPS mode to use (ap mode). see
  *    &enum nl80211_smps_mode.
  *
+ *    @NL80211_ATTR_PSK: a PSK value (u8 attribute).This is 32-octet 
+ (256-bit)
+ *    PSK.
+ *
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1990,6 +1994,8 @@ enum nl80211_attrs {
 
     NL80211_ATTR_SMPS_MODE,
 
+    NL80211_ATTR_PSK,
+
     /* add attributes here, update the policy in nl80211.c */
 
     __NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 5839c85..91c24b1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -395,6 +395,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
     [NL80211_ATTR_USER_PRIO] = { .type = NLA_U8 },
     [NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
     [NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
+    [NL80211_ATTR_PSK] = { .type = NLA_BINARY, .len = 32 },
 };
 
 /* policy for the key attributes */
@@ -7310,6 +7311,9 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
         connect.flags |= ASSOC_REQ_USE_RRM;
     }
 
+    if (info->attrs[NL80211_ATTR_PSK])
+        connect.psk = nla_data(info->attrs[NL80211_ATTR_PSK]);
+
     wdev_lock(dev->ieee80211_ptr);
     err = cfg80211_connect(rdev, dev, &connect, connkeys, NULL);
     wdev_unlock(dev->ieee80211_ptr);
--
1.9.1

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Johannes Berg]

On Tue, 2014-11-11 at 05:56 +0000, Gautam (Gautam Kumar) Shukla wrote:
> For offloading 4 way handshake to driver, currently we don't have any
> member  of struct cfg80211_connect_params to pass PSK from supplicant
> to driver. I have added psk for the same and added rest of the code
> needed in nl80211.h and nl80211.c to parse and make it available to
> driver.
> From supplicant, we already have psk member field in assoc_params to
> use .
> 
> Tested on x86 linux.

Your commit message needs serious work.

Also, there's a competing approach from QCA that's far more suited.

In either case though, I'm going to ask which driver is going to use
this and when it's going to land in mainline.

johannes

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Arend van Spriel]

On 11-11-14 10:29, Johannes Berg wrote:
> On Tue, 2014-11-11 at 05:56 +0000, Gautam (Gautam Kumar) Shukla wrote:
>> For offloading 4 way handshake to driver, currently we don't have any
>> member  of struct cfg80211_connect_params to pass PSK from supplicant
>> to driver. I have added psk for the same and added rest of the code
>> needed in nl80211.h and nl80211.c to parse and make it available to
>> driver.
>> From supplicant, we already have psk member field in assoc_params to
>> use .
>>
>> Tested on x86 linux.
> 
> Your commit message needs serious work.
> 
> Also, there's a competing approach from QCA that's far more suited.

I probably was not paying attention to it, but would you have a
reference to this.

> In either case though, I'm going to ask which driver is going to use
> this and when it's going to land in mainline.

I had the same question ;-)

> johannes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Johannes Berg]

On Tue, 2014-11-11 at 10:54 +0100, Arend van Spriel wrote:

> > Also, there's a competing approach from QCA that's far more suited.
> 
> I probably was not paying attention to it, but would you have a
> reference to this.

... digs around in email ...

http://mid.gmane.org/1343907187-6686-1-git-send-email-qca_vkondrat@qca.qualcomm.com

Anyway, looking back at that, it wasn't really all that different, just
a bit more complete maybe.

johannes

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Arend van Spriel]

On 11-11-14 11:03, Johannes Berg wrote:
> On Tue, 2014-11-11 at 10:54 +0100, Arend van Spriel wrote:
> 
>>> Also, there's a competing approach from QCA that's far more suited.
>>
>> I probably was not paying attention to it, but would you have a
>> reference to this.
> 
> ... digs around in email ...
> 
> http://mid.gmane.org/1343907187-6686-1-git-send-email-qca_vkondrat@qca.qualcomm.com
> 
> Anyway, looking back at that, it wasn't really all that different, just
> a bit more complete maybe.

Read through the whole thread. It seems some comments from you needed to
be addressed and Vladimir wanted to evaluate it. So that was the end of
the thread.

What did pop up is the wiphy flags vs. nl80211 feature flags. When that
comes up it looks like 'potAtoes, potaetoes' to me.

So is there are clear design rule for when to use which flag. For me the
wiphy object represents the device/firmware and 4-way handshake offload
support is determined by what the device/firmware supports.

Regards,
Arend

> johannes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Johannes Berg]

On Tue, 2014-11-11 at 11:35 +0100, Arend van Spriel wrote:

> What did pop up is the wiphy flags vs. nl80211 feature flags. When that
> comes up it looks like 'potAtoes, potaetoes' to me.
> 
> So is there are clear design rule for when to use which flag. For me the
> wiphy object represents the device/firmware and 4-way handshake offload
> support is determined by what the device/firmware supports.

There are three types of flags:

 * wiphy flag attributes - deprecated as far as I'm concerned
 * wiphy nl80211 feature flags - much easier to use in kernel (and
userspace)
 * nl80211 protocol flags - only one exists
                            (NL80211_PROTOCOL_FEATURE_SPLIT_WIPHY_DUMP)

johannes

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Arend van Spriel]

On 11-11-14 11:38, Johannes Berg wrote:
> On Tue, 2014-11-11 at 11:35 +0100, Arend van Spriel wrote:
> 
>> What did pop up is the wiphy flags vs. nl80211 feature flags. When that
>> comes up it looks like 'potAtoes, potaetoes' to me.
>>
>> So is there are clear design rule for when to use which flag. For me the
>> wiphy object represents the device/firmware and 4-way handshake offload
>> support is determined by what the device/firmware supports.
> 
> There are three types of flags:
> 
>  * wiphy flag attributes - deprecated as far as I'm concerned

Ok. deprecated is clear enough ;-)

>  * wiphy nl80211 feature flags - much easier to use in kernel (and
> userspace)
>  * nl80211 protocol flags - only one exists
>                             (NL80211_PROTOCOL_FEATURE_SPLIT_WIPHY_DUMP)

Thanks,
Arend

RE: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Gautam (Gautam Kumar) Shukla]

VGhhbmtzIGEgbG90ICBKb2hhbm5lcyBhbmQgQXJlbmQgLCBmb3IgeW91ciBjb21tZW50cyBhbmQg
cmV2aWV3IC4NCkl0IHdpbGwgcmVhbGx5IGhlbHAgbWUgaW4gbXkgbmV4dCBwYXRjaCBzdWJtaXNz
aW9uIC4NCg0KSnVzdCB3YW50ZWQgdG8ga25vdyB3aGVuIFFDQSBwYXRjaCB3aWxsIGdldCBpbiBt
YWluIGxpbmUgYXMgaXQgd2FzICBsYXN0IHVwZGF0ZWQgb24gIDIzIGF1ZyAyMDEyLg0KDQoNClRo
YW5rcyBhbmQgcmVnYXJkcw0KDQotLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KRnJvbTogQXJl
bmQgdmFuIFNwcmllbCBbbWFpbHRvOmFyZW5kQGJyb2FkY29tLmNvbV0gDQpTZW50OiBUdWVzZGF5
LCBOb3ZlbWJlciAxMSwgMjAxNCAzOjI0IFBNDQpUbzogSm9oYW5uZXMgQmVyZzsgR2F1dGFtIChH
YXV0YW0gS3VtYXIpIFNodWtsYQ0KQ2M6IGxpbnZpbGxlQHR1eGRyaXZlci5jb207IGxpbnV4LXdp
cmVsZXNzQHZnZXIua2VybmVsLm9yZzsgZGF2ZW1AZGF2ZW1sb2Z0Lm5ldDsgbGludXgtYXBpQHZn
ZXIua2VybmVsLm9yZzsgbGludXgta2VybmVsQHZnZXIua2VybmVsLm9yZzsgbmV0ZGV2QHZnZXIu
a2VybmVsLm9yZzsgSml0aHUgSmFuY2U7IFNyZWVuYXRoIFMNClN1YmplY3Q6IFJlOiBbUEFUQ0gg
MS8xXSBsaW51eC13aXJlbGVzczogQWRkZWQgcHNrIGluIHN0cnVjdCBjZmc4MDIxMV9jb25uZWN0
X3BhcmFtcyBuZWVkZWQgZm9yIG9mZmxvYWRpbmcgNHdheSBoYW5kc2hha2UgdG8gZHJpdmVyDQoN
Ck9uIDExLTExLTE0IDEwOjI5LCBKb2hhbm5lcyBCZXJnIHdyb3RlOg0KPiBPbiBUdWUsIDIwMTQt
MTEtMTEgYXQgMDU6NTYgKzAwMDAsIEdhdXRhbSAoR2F1dGFtIEt1bWFyKSBTaHVrbGEgd3JvdGU6
DQo+PiBGb3Igb2ZmbG9hZGluZyA0IHdheSBoYW5kc2hha2UgdG8gZHJpdmVyLCBjdXJyZW50bHkg
d2UgZG9uJ3QgaGF2ZSBhbnkgDQo+PiBtZW1iZXIgIG9mIHN0cnVjdCBjZmc4MDIxMV9jb25uZWN0
X3BhcmFtcyB0byBwYXNzIFBTSyBmcm9tIHN1cHBsaWNhbnQgDQo+PiB0byBkcml2ZXIuIEkgaGF2
ZSBhZGRlZCBwc2sgZm9yIHRoZSBzYW1lIGFuZCBhZGRlZCByZXN0IG9mIHRoZSBjb2RlIA0KPj4g
bmVlZGVkIGluIG5sODAyMTEuaCBhbmQgbmw4MDIxMS5jIHRvIHBhcnNlIGFuZCBtYWtlIGl0IGF2
YWlsYWJsZSB0byANCj4+IGRyaXZlci4NCj4+IEZyb20gc3VwcGxpY2FudCwgd2UgYWxyZWFkeSBo
YXZlIHBzayBtZW1iZXIgZmllbGQgaW4gYXNzb2NfcGFyYW1zIHRvIA0KPj4gdXNlIC4NCj4+DQo+
PiBUZXN0ZWQgb24geDg2IGxpbnV4Lg0KPiANCj4gWW91ciBjb21taXQgbWVzc2FnZSBuZWVkcyBz
ZXJpb3VzIHdvcmsuDQo+IA0KPiBBbHNvLCB0aGVyZSdzIGEgY29tcGV0aW5nIGFwcHJvYWNoIGZy
b20gUUNBIHRoYXQncyBmYXIgbW9yZSBzdWl0ZWQuDQoNCkkgcHJvYmFibHkgd2FzIG5vdCBwYXlp
bmcgYXR0ZW50aW9uIHRvIGl0LCBidXQgd291bGQgeW91IGhhdmUgYSByZWZlcmVuY2UgdG8gdGhp
cy4NCg0KPiBJbiBlaXRoZXIgY2FzZSB0aG91Z2gsIEknbSBnb2luZyB0byBhc2sgd2hpY2ggZHJp
dmVyIGlzIGdvaW5nIHRvIHVzZSANCj4gdGhpcyBhbmQgd2hlbiBpdCdzIGdvaW5nIHRvIGxhbmQg
aW4gbWFpbmxpbmUuDQoNCkkgaGFkIHRoZSBzYW1lIHF1ZXN0aW9uIDstKQ0KDQo+IGpvaGFubmVz
DQo+IA0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGlu
ZSAidW5zdWJzY3JpYmUgDQo+IGxpbnV4LXdpcmVsZXNzIiBpbiB0aGUgYm9keSBvZiBhIG1lc3Nh
Z2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZyANCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBh
dCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQo+IA0KDQo=

Re: [PATCH 1/1] linux-wireless: Added psk in struct cfg80211_connect_params needed for offloading 4way handshake to driver

[Arend van Spriel]

On 11-11-14 06:56, Gautam (Gautam Kumar) Shukla wrote:
>

Hi Gautam,

Good to see more upstream contributions, but it might be useful to have
a driver implementation as well in this series. Maybe we can take a shot
with brcmfmac for obvious reasons. Would you happen to have
wpa_supplicant changes as well?

I added some inline comments below.

Regards,
Arend

> For offloading 4 way handshake to driver, currently we don't have any member  of struct cfg80211_connect_params to pass PSK from supplicant to driver. I have added psk for the same and added rest of the code needed in nl80211.h and nl80211.c to parse and make it available to driver.
> From supplicant, we already have psk member field in assoc_params to use .

In the commit message you should not describe what you did, but what
problem you are trying to solve and/or what functional change the patch
provides.

> Tested on x86 linux.
> 
> Signed-off-by: Gautam kumar shukla <gautams@broadcom.com>
> ---
> include/net/cfg80211.h       | 2 ++
>  include/uapi/linux/nl80211.h | 8 +++++++-
>  net/wireless/nl80211.c       | 4 ++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index a2ddcf2..6f744e0 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -1758,6 +1758,7 @@ struct cfg80211_ibss_params {
>   *    allowed to ignore this @bssid_hint if it has knowledge of a better BSS
>   *    to use.
>   * @ssid: SSID
> + * @psk:preshared key for WPA2-PSK connection or %NULL if not specified

add space after the colon sign.

>   * @ssid_len: Length of ssid in octets
>   * @auth_type: Authentication type (algorithm)
>   * @ie: IEs for association request
> @@ -1783,6 +1784,7 @@ struct cfg80211_connect_params {
>      const u8 *bssid;
>      const u8 *bssid_hint;
>      const u8 *ssid;
> +    const u8 *psk;
>      size_t ssid_len;
>      enum nl80211_auth_type auth_type;
>      const u8 *ie;
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 4b28dc0..b01d5dd 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -421,7 +421,7 @@
>   *    %NL80211_ATTR_MAC, %NL80211_ATTR_WIPHY_FREQ, %NL80211_ATTR_CONTROL_PORT,
>   *    %NL80211_ATTR_CONTROL_PORT_ETHERTYPE,
>   *    %NL80211_ATTR_CONTROL_PORT_NO_ENCRYPT, %NL80211_ATTR_MAC_HINT, and
> - *    %NL80211_ATTR_WIPHY_FREQ_HINT.
> + *    %NL80211_ATTR_WIPHY_FREQ_HINT, and %NL80211_ATTR_PSK.
>   *    If included, %NL80211_ATTR_MAC and %NL80211_ATTR_WIPHY_FREQ are
>   *    restrictions on BSS selection, i.e., they effectively prevent roaming
>   *    within the ESS. %NL80211_ATTR_MAC_HINT and %NL80211_ATTR_WIPHY_FREQ_HINT @@ -1638,6 +1638,10 @@ enum nl80211_commands {
>   * @NL80211_ATTR_SMPS_MODE: SMPS mode to use (ap mode). see
>   *    &enum nl80211_smps_mode.
>   *
> + *    @NL80211_ATTR_PSK: a PSK value (u8 attribute).This is 32-octet 
> + (256-bit)
> + *    PSK.
> + *

Some indentation gone haywire here. I would remove '(u8 attribute)'. The
mention of 32-octet seems sufficient to me.

>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
>   */
> @@ -1990,6 +1994,8 @@ enum nl80211_attrs {
>  
>      NL80211_ATTR_SMPS_MODE,
>  
> +    NL80211_ATTR_PSK,
> +
>      /* add attributes here, update the policy in nl80211.c */
>  
>      __NL80211_ATTR_AFTER_LAST,
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 5839c85..91c24b1 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -395,6 +395,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
>      [NL80211_ATTR_USER_PRIO] = { .type = NLA_U8 },
>      [NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
>      [NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
> +    [NL80211_ATTR_PSK] = { .type = NLA_BINARY, .len = 32 },
>  };
>  
>  /* policy for the key attributes */
> @@ -7310,6 +7311,9 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
>          connect.flags |= ASSOC_REQ_USE_RRM;
>      }
>  
> +    if (info->attrs[NL80211_ATTR_PSK])
> +        connect.psk = nla_data(info->attrs[NL80211_ATTR_PSK]);
> +
>      wdev_lock(dev->ieee80211_ptr);
>      err = cfg80211_connect(rdev, dev, &connect, connkeys, NULL);
>      wdev_unlock(dev->ieee80211_ptr);
> --
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>