Re: [PATCH] mwifiex: don't do unbalanced free()'ing in cleanup_if()

[Dmitry Torokhov]

On Wed, Oct 26, 2016 at 04:43:54PM -0700, Brian Norris wrote:
> On Wed, Oct 26, 2016 at 04:35:54PM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 26, 2016 at 04:29:20PM -0700, Brian Norris wrote:
> 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
> > > b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > > index 8718950004f3..f04cf5a551b3 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> 
> > > @@ -2291,6 +2287,14 @@ static void mwifiex_recreate_adapter(struct 
> > > sdio_mmc_card *card)
> > >  
> > >   mwifiex_sdio_remove(func);
> > >  
> > > + /*
> > > +  * Normally, we would let the driver core take care of releasing these.
> > > +  * But we're not letting the driver core handle this one. See above
> > > +  * TODO.
> > > +  */
> > > + sdio_set_drvdata(func, NULL);
> > > + devm_kfree(>dev, card);
> > 
> > Ugh, this really messes the unwind order... I guess it is OK since it is
> > the only resource allocated with devm, but I'd be happier if we could
> > reuse existing "card" structure.
> 
> I'm really not interested in cleaning up the hacky reset function here
> (see the other TODOs here). I'm sure it's broken in other ways too. In
> its current "design" (if you can call it that) where we remove and
> re-probe the device, I'm not sure there's a way to get it to reuse the
> 'card'.

Ah, I see now... Nevermind then.

Thanks.

-- 
Dmitry

Re: [PATCH] mwifiex: don't do unbalanced free()'ing in cleanup_if()

[Brian Norris]

On Wed, Oct 26, 2016 at 04:35:54PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 26, 2016 at 04:29:20PM -0700, Brian Norris wrote:

> > diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
> > b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > index 8718950004f3..f04cf5a551b3 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c

> > @@ -2291,6 +2287,14 @@ static void mwifiex_recreate_adapter(struct 
> > sdio_mmc_card *card)
> >  
> > mwifiex_sdio_remove(func);
> >  
> > +   /*
> > +* Normally, we would let the driver core take care of releasing these.
> > +* But we're not letting the driver core handle this one. See above
> > +* TODO.
> > +*/
> > +   sdio_set_drvdata(func, NULL);
> > +   devm_kfree(>dev, card);
> 
> Ugh, this really messes the unwind order... I guess it is OK since it is
> the only resource allocated with devm, but I'd be happier if we could
> reuse existing "card" structure.

I'm really not interested in cleaning up the hacky reset function here
(see the other TODOs here). I'm sure it's broken in other ways too. In
its current "design" (if you can call it that) where we remove and
re-probe the device, I'm not sure there's a way to get it to reuse the
'card'.

If you insist on refactoring this to protect the potential future unwind
order (if we use devm more heavily), then I guess I'd have to go back to
manual k{zalloc,free}() for sdio.c.

Brian

Re: [PATCH] mwifiex: don't do unbalanced free()'ing in cleanup_if()

[Dmitry Torokhov]

On Wed, Oct 26, 2016 at 04:29:20PM -0700, Brian Norris wrote:
> The cleanup_if() callback is the inverse of init_if(). We allocate our
> 'card' interface structure in the probe() function, but we free it in
> cleanup_if(). That gives a few problems:
> (a) we leak this memory if probe() fails before we reach init_if()
> (b) we can't safely utilize 'card' after cleanup_if() -- namely, in
> remove() or suspend(), both of which might race with the cleanup
> paths in our asynchronous FW initialization path
> 
> Solution: just use devm_kzalloc(), which will free this structure
> properly when the device is removed -- and drop the set_drvdata(...,
> NULL), since the driver core does this for us. This also removes the
> temptation to use drvdata == NULL as a hack for checking if the device
> has been "cleaned up."
> 
> I *do* leave the set_drvdata(..., NULL) for the hacky SDIO
> mwifiex_recreate_adapter(), since the device core won't be able to clear
> that one for us.
> 
> Signed-off-by: Brian Norris 
> ---
>  drivers/net/wireless/marvell/mwifiex/pcie.c |  5 +
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 16 ++--
>  drivers/net/wireless/marvell/mwifiex/usb.c  |  7 +--
>  3 files changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 063c707844d3..3047c1ab944a 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -189,7 +189,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
>   pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
>pdev->vendor, pdev->device, pdev->revision);
>  
> - card = kzalloc(sizeof(struct pcie_service_card), GFP_KERNEL);
> + card = devm_kzalloc(>dev, sizeof(*card), GFP_KERNEL);
>   if (!card)
>   return -ENOMEM;
>  
> @@ -2815,7 +2815,6 @@ static int mwifiex_pcie_init(struct mwifiex_adapter 
> *adapter)
>  err_set_dma_mask:
>   pci_disable_device(pdev);
>  err_enable_dev:
> - pci_set_drvdata(pdev, NULL);
>   return ret;
>  }
>  
> @@ -2849,9 +2848,7 @@ static void mwifiex_pcie_cleanup(struct mwifiex_adapter 
> *adapter)
>   pci_disable_device(pdev);
>   pci_release_region(pdev, 2);
>   pci_release_region(pdev, 0);
> - pci_set_drvdata(pdev, NULL);
>   }
> - kfree(card);
>  }
>  
>  static int mwifiex_pcie_request_irq(struct mwifiex_adapter *adapter)
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
> b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 8718950004f3..f04cf5a551b3 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -152,7 +152,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
> sdio_device_id *id)
>   pr_debug("info: vendor=0x%4.04X device=0x%4.04X class=%d function=%d\n",
>func->vendor, func->device, func->class, func->num);
>  
> - card = kzalloc(sizeof(struct sdio_mmc_card), GFP_KERNEL);
> + card = devm_kzalloc(>dev, sizeof(*card), GFP_KERNEL);
>   if (!card)
>   return -ENOMEM;
>  
> @@ -185,7 +185,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
> sdio_device_id *id)
>  
>   if (ret) {
>   dev_err(>dev, "failed to enable function\n");
> - goto err_free;
> + return ret;
>   }
>  
>   /* device tree node parsing and platform specific configuration*/
> @@ -210,8 +210,6 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
> sdio_device_id *id)
>   sdio_claim_host(func);
>   sdio_disable_func(func);
>   sdio_release_host(func);
> -err_free:
> - kfree(card);
>  
>   return ret;
>  }
> @@ -2240,8 +2238,6 @@ static void mwifiex_cleanup_sdio(struct mwifiex_adapter 
> *adapter)
>   kfree(card->mpa_rx.len_arr);
>   kfree(card->mpa_tx.buf);
>   kfree(card->mpa_rx.buf);
> - sdio_set_drvdata(card->func, NULL);
> - kfree(card);
>  }
>  
>  /*
> @@ -2291,6 +2287,14 @@ static void mwifiex_recreate_adapter(struct 
> sdio_mmc_card *card)
>  
>   mwifiex_sdio_remove(func);
>  
> + /*
> +  * Normally, we would let the driver core take care of releasing these.
> +  * But we're not letting the driver core handle this one. See above
> +  * TODO.
> +  */
> + sdio_set_drvdata(func, NULL);
> + devm_kfree(>dev, card);

Ugh, this really messes the unwind order... I guess it is OK since it is
the only resource allocated with devm, but I'd be happier if we could
reuse existing "card" structure.

Thanks.

-- 
Dmitry

[PATCH] mwifiex: don't do unbalanced free()'ing in cleanup_if()

[Brian Norris]

The cleanup_if() callback is the inverse of init_if(). We allocate our
'card' interface structure in the probe() function, but we free it in
cleanup_if(). That gives a few problems:
(a) we leak this memory if probe() fails before we reach init_if()
(b) we can't safely utilize 'card' after cleanup_if() -- namely, in
remove() or suspend(), both of which might race with the cleanup
paths in our asynchronous FW initialization path

Solution: just use devm_kzalloc(), which will free this structure
properly when the device is removed -- and drop the set_drvdata(...,
NULL), since the driver core does this for us. This also removes the
temptation to use drvdata == NULL as a hack for checking if the device
has been "cleaned up."

I *do* leave the set_drvdata(..., NULL) for the hacky SDIO
mwifiex_recreate_adapter(), since the device core won't be able to clear
that one for us.

Signed-off-by: Brian Norris 
---
 drivers/net/wireless/marvell/mwifiex/pcie.c |  5 +
 drivers/net/wireless/marvell/mwifiex/sdio.c | 16 ++--
 drivers/net/wireless/marvell/mwifiex/usb.c  |  7 +--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 063c707844d3..3047c1ab944a 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -189,7 +189,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
 pdev->vendor, pdev->device, pdev->revision);
 
-   card = kzalloc(sizeof(struct pcie_service_card), GFP_KERNEL);
+   card = devm_kzalloc(>dev, sizeof(*card), GFP_KERNEL);
if (!card)
return -ENOMEM;
 
@@ -2815,7 +2815,6 @@ static int mwifiex_pcie_init(struct mwifiex_adapter 
*adapter)
 err_set_dma_mask:
pci_disable_device(pdev);
 err_enable_dev:
-   pci_set_drvdata(pdev, NULL);
return ret;
 }
 
@@ -2849,9 +2848,7 @@ static void mwifiex_pcie_cleanup(struct mwifiex_adapter 
*adapter)
pci_disable_device(pdev);
pci_release_region(pdev, 2);
pci_release_region(pdev, 0);
-   pci_set_drvdata(pdev, NULL);
}
-   kfree(card);
 }
 
 static int mwifiex_pcie_request_irq(struct mwifiex_adapter *adapter)
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index 8718950004f3..f04cf5a551b3 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -152,7 +152,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
pr_debug("info: vendor=0x%4.04X device=0x%4.04X class=%d function=%d\n",
 func->vendor, func->device, func->class, func->num);
 
-   card = kzalloc(sizeof(struct sdio_mmc_card), GFP_KERNEL);
+   card = devm_kzalloc(>dev, sizeof(*card), GFP_KERNEL);
if (!card)
return -ENOMEM;
 
@@ -185,7 +185,7 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
 
if (ret) {
dev_err(>dev, "failed to enable function\n");
-   goto err_free;
+   return ret;
}
 
/* device tree node parsing and platform specific configuration*/
@@ -210,8 +210,6 @@ mwifiex_sdio_probe(struct sdio_func *func, const struct 
sdio_device_id *id)
sdio_claim_host(func);
sdio_disable_func(func);
sdio_release_host(func);
-err_free:
-   kfree(card);
 
return ret;
 }
@@ -2240,8 +2238,6 @@ static void mwifiex_cleanup_sdio(struct mwifiex_adapter 
*adapter)
kfree(card->mpa_rx.len_arr);
kfree(card->mpa_tx.buf);
kfree(card->mpa_rx.buf);
-   sdio_set_drvdata(card->func, NULL);
-   kfree(card);
 }
 
 /*
@@ -2291,6 +2287,14 @@ static void mwifiex_recreate_adapter(struct 
sdio_mmc_card *card)
 
mwifiex_sdio_remove(func);
 
+   /*
+* Normally, we would let the driver core take care of releasing these.
+* But we're not letting the driver core handle this one. See above
+* TODO.
+*/
+   sdio_set_drvdata(func, NULL);
+   devm_kfree(>dev, card);
+
/* power cycle the adapter */
sdio_claim_host(func);
mmc_hw_reset(func->card->host);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c 
b/drivers/net/wireless/marvell/mwifiex/usb.c
index 73eb0846db21..57ed834ba296 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -382,7 +382,7 @@ static int mwifiex_usb_probe(struct usb_interface *intf,
struct usb_card_rec *card;
u16 id_vendor, id_product, bcd_device, bcd_usb;
 
-   card = kzalloc(sizeof(struct usb_card_rec), GFP_KERNEL);
+   card = devm_kzalloc(>dev, sizeof(*card), GFP_KERNEL);
if (!card)

[PATCH v2 8/9] mac80211: FILS AEAD protection for station mode association frames

[Jouni Malinen]

This adds support for encrypting (Re)Association Request frame and
decryption (Re)Association Response frame when using FILS in station
mode.

Signed-off-by: Jouni Malinen 
---
 net/mac80211/Makefile  |   1 +
 net/mac80211/aes_cmac.c|   8 +-
 net/mac80211/aes_cmac.h|   4 +
 net/mac80211/fils_aead.c   | 347 +
 net/mac80211/fils_aead.h   |  19 +++
 net/mac80211/ieee80211_i.h |   4 +
 net/mac80211/mlme.c|  27 
 7 files changed, 406 insertions(+), 4 deletions(-)
 create mode 100644 net/mac80211/fils_aead.c
 create mode 100644 net/mac80211/fils_aead.h

diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index f9137a8..0b202b3 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -19,6 +19,7 @@ mac80211-y := \
aes_gcm.o \
aes_cmac.o \
aes_gmac.o \
+   fils_aead.o \
cfg.o \
ethtool.o \
rx.o \
diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index bdf0790..d0bd5ff 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -23,7 +23,7 @@
 #define AAD_LEN 20
 
 
-static void gf_mulx(u8 *pad)
+void gf_mulx(u8 *pad)
 {
int i, carry;
 
@@ -35,9 +35,9 @@ static void gf_mulx(u8 *pad)
pad[AES_BLOCK_SIZE - 1] ^= 0x87;
 }
 
-static void aes_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
-   const u8 *addr[], const size_t *len, u8 *mac,
-   size_t mac_len)
+void aes_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
+const u8 *addr[], const size_t *len, u8 *mac,
+size_t mac_len)
 {
u8 cbc[AES_BLOCK_SIZE], pad[AES_BLOCK_SIZE];
const u8 *pos, *end;
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 3702041..c827e1d 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,6 +11,10 @@
 
 #include 
 
+void gf_mulx(u8 *pad);
+void aes_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
+const u8 *addr[], const size_t *len, u8 *mac,
+size_t mac_len);
 struct crypto_cipher *ieee80211_aes_cmac_key_setup(const u8 key[],
   size_t key_len);
 void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
new file mode 100644
index 000..afc7efc
--- /dev/null
+++ b/net/mac80211/fils_aead.c
@@ -0,0 +1,347 @@
+/*
+ * FILS AEAD for (Re)Association Request/Response frames
+ * Copyright 2016, Qualcomm Atheros, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+
+#include "ieee80211_i.h"
+#include "aes_cmac.h"
+#include "fils_aead.h"
+
+static int aes_s2v(struct crypto_cipher *tfm,
+  size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
+{
+   u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE];
+   size_t i;
+   const u8 *data[2];
+   size_t data_len[2], data_elems;
+
+   /* D = AES-CMAC(K, ) */
+   memset(tmp, 0, AES_BLOCK_SIZE);
+   data[0] = tmp;
+   data_len[0] = AES_BLOCK_SIZE;
+   aes_cmac_vector(tfm, 1, data, data_len, d, AES_BLOCK_SIZE);
+
+   for (i = 0; i < num_elem - 1; i++) {
+   /* D = dbl(D) xor AES_CMAC(K, Si) */
+   gf_mulx(d); /* dbl */
+   aes_cmac_vector(tfm, 1, [i], [i], tmp,
+   AES_BLOCK_SIZE);
+   crypto_xor(d, tmp, AES_BLOCK_SIZE);
+   }
+
+   if (len[i] >= AES_BLOCK_SIZE) {
+   /* len(Sn) >= 128 */
+   size_t j;
+   const u8 *pos;
+
+   /* T = Sn xorend D */
+
+   /* Use a temporary buffer to perform xorend on Sn (addr[i]) to
+* avoid modifying the const input argument.
+*/
+   data[0] = addr[i];
+   data_len[0] = len[i] - AES_BLOCK_SIZE;
+   pos = addr[i] + data_len[0];
+   for (j = 0; j < AES_BLOCK_SIZE; j++)
+   tmp[j] = pos[j] ^ d[j];
+   data[1] = tmp;
+   data_len[1] = AES_BLOCK_SIZE;
+   data_elems = 2;
+   } else {
+   /* len(Sn) < 128 */
+   /* T = dbl(D) xor pad(Sn) */
+   gf_mulx(d); /* dbl */
+   memset(tmp, 0, AES_BLOCK_SIZE);
+   memcpy(tmp, addr[i], len[i]);
+   tmp[len[i]] = 0x80;
+   crypto_xor(d, tmp, AES_BLOCK_SIZE);
+   data[0] = d;
+   data_len[0] = sizeof(d);
+   data_elems = 1;
+   }
+   /* V = AES-CMAC(K, T) */
+   aes_cmac_vector(tfm, data_elems, data, data_len, v, AES_BLOCK_SIZE);
+
+   return 0;
+}
+
+/* Note: addr[] and len[] 

[PATCH v2 9/9] mac80211: Claim Fast Initial Link Setup (FILS) STA support

[Jouni Malinen]

With the previous commits, initial FILS authentication/association
support is now functional in mac80211-based drivers for station role
(and FILS AP case is covered by user space in hostapd withotu requiring
mac80211 changes).

Signed-off-by: Jouni Malinen 
---
 net/mac80211/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 0d9163c..1822c77 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -549,6 +549,7 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t 
priv_data_len,
   NL80211_FEATURE_MAC_ON_CREATE |
   NL80211_FEATURE_USERSPACE_MPM |
   NL80211_FEATURE_FULL_AP_CLIENT_STATE;
+   wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_FILS_STA);
 
if (!ops->hw_scan)
wiphy->features |= NL80211_FEATURE_LOW_PRIORITY_SCAN |
-- 
1.9.1

[PATCH v2 5/9] cfg80211: Add Fast Initial Link Setup (FILS) auth algs

[Jouni Malinen]

This defines authentication algorithms for FILS (IEEE 802.11ai).

Signed-off-by: Jouni Malinen 
---
 include/linux/ieee80211.h|  3 +++
 include/uapi/linux/nl80211.h |  6 ++
 net/wireless/nl80211.c   | 21 +++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index d428adf..793a017 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1576,6 +1576,9 @@ struct ieee80211_vht_operation {
 #define WLAN_AUTH_SHARED_KEY 1
 #define WLAN_AUTH_FT 2
 #define WLAN_AUTH_SAE 3
+#define WLAN_AUTH_FILS_SK 4
+#define WLAN_AUTH_FILS_SK_PFS 5
+#define WLAN_AUTH_FILS_PK 6
 #define WLAN_AUTH_LEAP 128
 
 #define WLAN_AUTH_CHALLENGE_LEN 128
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f7e0791..dcf8f6f 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3667,6 +3667,9 @@ enum nl80211_bss_status {
  * @NL80211_AUTHTYPE_FT: Fast BSS Transition (IEEE 802.11r)
  * @NL80211_AUTHTYPE_NETWORK_EAP: Network EAP (some Cisco APs and mainly LEAP)
  * @NL80211_AUTHTYPE_SAE: Simultaneous authentication of equals
+ * @NL80211_AUTHTYPE_FILS_SK: Fast Initial Link Setup shared key
+ * @NL80211_AUTHTYPE_FILS_SK_PFS: Fast Initial Link Setup shared key with PFS
+ * @NL80211_AUTHTYPE_FILS_PK: Fast Initial Link Setup public key
  * @__NL80211_AUTHTYPE_NUM: internal
  * @NL80211_AUTHTYPE_MAX: maximum valid auth algorithm
  * @NL80211_AUTHTYPE_AUTOMATIC: determine automatically (if necessary by
@@ -3679,6 +3682,9 @@ enum nl80211_auth_type {
NL80211_AUTHTYPE_FT,
NL80211_AUTHTYPE_NETWORK_EAP,
NL80211_AUTHTYPE_SAE,
+   NL80211_AUTHTYPE_FILS_SK,
+   NL80211_AUTHTYPE_FILS_SK_PFS,
+   NL80211_AUTHTYPE_FILS_PK,
 
/* keep last */
__NL80211_AUTHTYPE_NUM,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 1a51bd4..893e321 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3777,12 +3777,23 @@ static bool nl80211_valid_auth_type(struct 
cfg80211_registered_device *rdev,
if (!(rdev->wiphy.features & NL80211_FEATURE_SAE) &&
auth_type == NL80211_AUTHTYPE_SAE)
return false;
+   if (!wiphy_ext_feature_isset(>wiphy,
+NL80211_EXT_FEATURE_FILS_STA) &&
+   (auth_type == NL80211_AUTHTYPE_FILS_SK ||
+auth_type == NL80211_AUTHTYPE_FILS_SK_PFS ||
+auth_type == NL80211_AUTHTYPE_FILS_PK))
+   return false;
return true;
case NL80211_CMD_CONNECT:
case NL80211_CMD_START_AP:
/* SAE not supported yet */
if (auth_type == NL80211_AUTHTYPE_SAE)
return false;
+   /* FILS not supported yet */
+   if (auth_type == NL80211_AUTHTYPE_FILS_SK ||
+   auth_type == NL80211_AUTHTYPE_FILS_SK_PFS ||
+   auth_type == NL80211_AUTHTYPE_FILS_PK)
+   return false;
return true;
default:
return false;
@@ -7809,12 +7820,18 @@ static int nl80211_authenticate(struct sk_buff *skb, 
struct genl_info *info)
if (!nl80211_valid_auth_type(rdev, auth_type, NL80211_CMD_AUTHENTICATE))
return -EINVAL;
 
-   if (auth_type == NL80211_AUTHTYPE_SAE &&
+   if ((auth_type == NL80211_AUTHTYPE_SAE ||
+auth_type == NL80211_AUTHTYPE_FILS_SK ||
+auth_type == NL80211_AUTHTYPE_FILS_SK_PFS ||
+auth_type == NL80211_AUTHTYPE_FILS_PK) &&
!info->attrs[NL80211_ATTR_AUTH_DATA])
return -EINVAL;
 
if (info->attrs[NL80211_ATTR_AUTH_DATA]) {
-   if (auth_type != NL80211_AUTHTYPE_SAE)
+   if (auth_type != NL80211_AUTHTYPE_SAE &&
+   auth_type != NL80211_AUTHTYPE_FILS_SK &&
+   auth_type != NL80211_AUTHTYPE_FILS_SK_PFS &&
+   auth_type != NL80211_AUTHTYPE_FILS_PK)
return -EINVAL;
auth_data = nla_data(info->attrs[NL80211_ATTR_AUTH_DATA]);
auth_data_len = nla_len(info->attrs[NL80211_ATTR_AUTH_DATA]);
-- 
1.9.1

[PATCH v2 0/9] cfg80211/mac80211: Fast Initial Link Setup (IEEE 802.11ai)

[Jouni Malinen]

This series adds support for using mac80211-based drivers with Fast
Initial Link Setup as defined in IEEE 802.11ai (to be published early
next year; no more technical changes are expected at this point). The
fils branch in git://w1.fi/hostap.git includes matching commits for
hostapd and wpa_supplicant to use this functionality and initial set of
mac80211_hwsim test cases for the functionality. Actually, most of the
commits are already in the master branch, i.e., only the changes
depending on the nl80211.h changes from this kernel patchset are waiting
in the fils branch.

This series covers only the FILS authentication/association
functionality from IEEE 802.11ai, i.e., the other changes like scanning
optimizations are not included.

v2: Updates to address comments from Johannes

Jouni Malinen (9):
  cfg80211: Rename SAE_DATA to more generic AUTH_DATA
  mac80211: Allow AUTH_DATA to be used for FILS
  cfg80211: Add feature flag for Fast Initial Link Setup (FILS) as STA
  cfg80211: Define IEEE P802.11ai (FILS) information elements
  cfg80211: Add Fast Initial Link Setup (FILS) auth algs
  cfg80211: Add KEK/nonces for FILS association frames
  mac80211: Add FILS auth alg mapping
  mac80211: FILS AEAD protection for station mode association frames
  mac80211: Claim Fast Initial Link Setup (FILS) STA support

 include/linux/ieee80211.h|  26 
 include/net/cfg80211.h   |  40 -
 include/uapi/linux/nl80211.h |  30 +++-
 net/mac80211/Makefile|   1 +
 net/mac80211/aes_cmac.c  |   8 +-
 net/mac80211/aes_cmac.h  |   4 +
 net/mac80211/fils_aead.c | 347 +++
 net/mac80211/fils_aead.h |  19 +++
 net/mac80211/ieee80211_i.h   |   4 +
 net/mac80211/main.c  |   1 +
 net/mac80211/mlme.c  |  57 ++-
 net/wireless/core.h  |   2 +-
 net/wireless/mlme.c  |   6 +-
 net/wireless/nl80211.c   |  51 +--
 14 files changed, 561 insertions(+), 35 deletions(-)
 create mode 100644 net/mac80211/fils_aead.c
 create mode 100644 net/mac80211/fils_aead.h

-- 
1.9.1

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Rajat Jain]

On Wed, Oct 26, 2016 at 2:08 PM, Brian Norris  wrote:
> On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
>> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
>> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
>> > >On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
>> > > wrote:
>> > >  On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
>> > >  Sorry, I just saw this... Why do we need devicetree data for
>> > >  discoverable bus (PCI)? How does the driver work on systems that do 
>> > > not
>> > >  use DT? Why do we need them to behave differently?
>> > >
>> > >There are a couple of out-of-band GPIO pins from Marvell chip that can
>> > >serve as wake-up pins (wake up the CPU when asserted). The Marvell 
>> > > chip
>> > >has to be told which GPIO pin is to be used as the wake-up pin. The 
>> > > pin to
>> > >be used is system / platform dependent. (On some systems it could be
>> > >GPIO13, on others it could be GPIO14 etc depending on how the marvell 
>> > > chip
>> > >is wired up to the CPU).
>>
>> So wakeup pin is not wired to PCIe WAKE?
>
> Not in our case.
>
>> > There's also calibration data. See "marvell,caldata*" and
>> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
>> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
>> > we're adding support for PCIe.
>>
>> How would it all work if I moved the PCIe module from one device to
>> another?
>
> These boards are soldered down, at least in the case I care about.

That is right. Since the out of band wake-up pin is not standard on
the PCIe connector - this feature is for soldered chips only.


>
> Brian

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Brian Norris]

On Wed, Oct 26, 2016 at 02:06:48PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> > On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> > >On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> > > wrote:
> > >  On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> > >  Sorry, I just saw this... Why do we need devicetree data for
> > >  discoverable bus (PCI)? How does the driver work on systems that do 
> > > not
> > >  use DT? Why do we need them to behave differently?
> > > 
> > >There are a couple of out-of-band GPIO pins from Marvell chip that can
> > >serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> > >has to be told which GPIO pin is to be used as the wake-up pin. The 
> > > pin to
> > >be used is system / platform dependent. (On some systems it could be
> > >GPIO13, on others it could be GPIO14 etc depending on how the marvell 
> > > chip
> > >is wired up to the CPU).
> 
> So wakeup pin is not wired to PCIe WAKE?

Not in our case.

> > There's also calibration data. See "marvell,caldata*" and
> > "marvell,wakeup-pin" properties. Currently only for SDIO, in
> > Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> > we're adding support for PCIe.
> 
> How would it all work if I moved the PCIe module from one device to
> another?

These boards are soldered down, at least in the case I care about.

Brian

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Dmitry Torokhov]

On Wed, Oct 26, 2016 at 01:56:34PM -0700, Brian Norris wrote:
> On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
> >On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> > wrote:
> >  On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> >  Sorry, I just saw this... Why do we need devicetree data for
> >  discoverable bus (PCI)? How does the driver work on systems that do not
> >  use DT? Why do we need them to behave differently?
> > 
> >There are a couple of out-of-band GPIO pins from Marvell chip that can
> >serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
> >has to be told which GPIO pin is to be used as the wake-up pin. The pin 
> > to
> >be used is system / platform dependent. (On some systems it could be
> >GPIO13, on others it could be GPIO14 etc depending on how the marvell 
> > chip
> >is wired up to the CPU).

So wakeup pin is not wired to PCIe WAKE?

> There's also calibration data. See "marvell,caldata*" and
> "marvell,wakeup-pin" properties. Currently only for SDIO, in
> Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
> we're adding support for PCIe.

How would it all work if I moved the PCIe module from one device to
another?

Thanks.

-- 
Dmitry

Re: [PATCH 7/8] mac80211: FILS AEAD protection for station mode association frames

[Malinen, Jouni]

On Wed, Oct 26, 2016 at 07:49:59AM +0200, Johannes Berg wrote:
> > +static u8 *fils_find_session(u8 *pos, u8 *end)

> Hmm. I think we should try to write this in terms of cfg80211_find_ie,
> or perhaps cfg80211_find_ie_match, maybe we need to extend those but
> this won't be the only one using the 255 escape.
> 
> Perhaps just making the eid passed there be a u16, and extending the
> EID definitions appropriately?

Will do that based on our discussion on the details (a new wrapper
function).

> > +   if (!session) {
> > +   sdata_info(sdata,

> Should we really print the message this prominently? It seems like an
> error case that we may not want to log at all, in case somebody tries
> to flood us with such frames?
...
> Likewise here. Perhaps make these mlme_dbg() or so instead?

These are pretty useful messages for debugging at least now that FILS is
still so new.. Once it gets more mature and has documented
interoperability between independent implementations, we may consider
removing the messages since they would not really show up during normal
operations and would not help much an actual end user. Anyway, I'll
replace them with mlme_dbg() for now.

> > +   if (req->fils_nonces)
> > +   assoc_data_len += 2 * FILS_NONCE_LEN;
> 
> Is this really correct? It seems like a bit of an artifact of initially
> having had the nonces in a variable part of the struct?
> 
> Or perhaps they have to go into the frame in some place that I missed?
> Please add a comment if so.

Ah, looks like I forgot that there and req->fils_kek_len for that
matter. I had initially thought of adding these as dynamically allocated
components at the end of the buffer, but after seeing how req->ie[] was
used, gave up on that extra complexity and simply used fixed size arrays
since both the KEK and FILS nonces do have a known fixed size and we can
easily "waste" that memory in struct ieee80211_mgd_assoc_data for
non-FILS cases without causing any noticeable impact.

> Also, you're never checking req->fils_nonces_set, so you can get rid of
> that.

Indeed. I'll make the cfg80211 patch enforce that both the KEK and
nonces are set since they are both needed for all FILS cases and remove
fils_nonces_set from mac80211.

-- 
Jouni MalinenPGP id EFC895FA

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Brian Norris]

On Wed, Oct 26, 2016 at 01:51:48PM -0700, Rajat Jain wrote:
>On Wed, Oct 26, 2016 at 1:46 PM, Dmitry Torokhov
> wrote:
>  On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
>  Sorry, I just saw this... Why do we need devicetree data for
>  discoverable bus (PCI)? How does the driver work on systems that do not
>  use DT? Why do we need them to behave differently?
> 
>There are a couple of out-of-band GPIO pins from Marvell chip that can
>serve as wake-up pins (wake up the CPU when asserted). The Marvell chip
>has to be told which GPIO pin is to be used as the wake-up pin. The pin to
>be used is system / platform dependent. (On some systems it could be
>GPIO13, on others it could be GPIO14 etc depending on how the marvell chip
>is wired up to the CPU).

There's also calibration data. See "marvell,caldata*" and
"marvell,wakeup-pin" properties. Currently only for SDIO, in
Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt, but
we're adding support for PCIe.

Brian

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Dmitry Torokhov]

On Wed, Oct 26, 2016 at 01:17:36PM -0700, Brian Norris wrote:
> Hi Rajat,
> 
> On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> > From: Xinming Hu 
> > 
> > This patch derives device tree node from pcie bus layer framework, and
> > fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> > Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> > marvell-8xxx.txt) to accommodate PCIe changes.
> > 
> > Signed-off-by: Xinming Hu 
> > Signed-off-by: Amitkumar Karwar 
> > Signed-off-by: Rajat Jain 
> > Reviewed-by: Brian Norris 
> > ---
> > v2: Included vendor and product IDs in compatible strings for PCIe
> > chipsets(Rob Herring)
> > v3: Patch is created using -M option so that it will only include diff of
> > original and renamed files(Rob Herring)
> > Resend v3: Resending the patch because I missed to include device tree 
> > mailing
> > while sending v3.
> > v4: Fix error handling, also move-on even if no device tree node is present.
> > v5: Update commit log to include memory leak, return -EINVAL instead of -1.
> 
> I've been working on reworking some bugfixes for this driver, and I
> noticed we have some problems w.r.t. memory leaks, and the "memory leak"
> fix is not actually a fix. See below.

Sorry, I just saw this... Why do we need devicetree data for
discoverable bus (PCI)? How does the driver work on systems that do not
use DT? Why do we need them to behave differently?

Thanks.

-- 
Dmitry

Re: compex wle900vx (ath10k) problem on 4.4.24 / armv7

[Oliver Zemann]

I (naively) went through pci/pm git log and found the following was
applied on 4.7-rc2 (i.e. prior to 4.7 release):

 commit 006d44e49a259b39947366728d65a873a19aadc0
 Author: Mika Westerberg 
 Date:   Thu Jun 2 11:17:15 2016 +0300

 PCI: Add runtime PM support for PCIe ports

From reading the commit log it seems to me like it could be it.

ath10k tries to wake up the device during probing before it starts
talking to it and it does so through MMIO/PCI config space. If it's
not mapped properly then driver will not be able to wake it up and
will timeout waiting for it.

Can you try cherry-picking it into your 4.4.24 and see if it helps?



Thanks for the help!

Until now I did not compile a kernel for the board.
I will give it a try and come back with the results ...


Best regards,
Matthias

Any news on that? I am in contact with the support of SolidRun 
(clearfog). Unfortunately they will stay at 4.4.x - no upgrade to 4.8 
nor 4.9 planned. Unfortunately the commit (006d44e) is incompatible to 
4.4. because pci_dev does not know bridge_d3

Re: [PATCH v6] mwifiex: parse device tree node for PCIe

[Brian Norris]

Hi Rajat,

On Fri, Oct 21, 2016 at 02:21:09PM -0700, Rajat Jain wrote:
> From: Xinming Hu 
> 
> This patch derives device tree node from pcie bus layer framework, and
> fixes a minor memory leak in mwifiex_pcie_probe() (in failure path).
> Device tree bindings file has been renamed(marvell-sd8xxx.txt ->
> marvell-8xxx.txt) to accommodate PCIe changes.
> 
> Signed-off-by: Xinming Hu 
> Signed-off-by: Amitkumar Karwar 
> Signed-off-by: Rajat Jain 
> Reviewed-by: Brian Norris 
> ---
> v2: Included vendor and product IDs in compatible strings for PCIe
> chipsets(Rob Herring)
> v3: Patch is created using -M option so that it will only include diff of
> original and renamed files(Rob Herring)
> Resend v3: Resending the patch because I missed to include device tree mailing
> while sending v3.
> v4: Fix error handling, also move-on even if no device tree node is present.
> v5: Update commit log to include memory leak, return -EINVAL instead of -1.

I've been working on reworking some bugfixes for this driver, and I
noticed we have some problems w.r.t. memory leaks, and the "memory leak"
fix is not actually a fix. See below.

> v6: Remove an unnecessary error print, fix typo in commit log
> 
>  .../{marvell-sd8xxx.txt => marvell-8xxx.txt}   |  8 +++--
>  drivers/net/wireless/marvell/mwifiex/pcie.c| 36 
> +++---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c |  3 +-
>  3 files changed, 39 insertions(+), 8 deletions(-)
>  rename Documentation/devicetree/bindings/net/wireless/{marvell-sd8xxx.txt => 
> marvell-8xxx.txt} (91%)
> 
> diff --git 
> a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt 
> b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> similarity index 91%
> rename from Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> rename to Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> index c421aba..dfe5f8e 100644
> --- a/Documentation/devicetree/bindings/net/wireless/marvell-sd8xxx.txt
> +++ b/Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt
> @@ -1,8 +1,8 @@
> -Marvell 8897/8997 (sd8897/sd8997) SDIO devices
> +Marvell 8897/8997 (sd8897/sd8997/pcie8997) SDIO/PCIE devices
>  --
>  
> -This node provides properties for controlling the marvell sdio wireless 
> device.
> -The node is expected to be specified as a child node to the SDIO controller 
> that
> +This node provides properties for controlling the marvell sdio/pcie wireless 
> device.
> +The node is expected to be specified as a child node to the SDIO/PCIE 
> controller that
>  connects the device to the system.
>  
>  Required properties:
> @@ -10,6 +10,8 @@ Required properties:
>- compatible : should be one of the following:
>   * "marvell,sd8897"
>   * "marvell,sd8997"
> + * "pci11ab,2b42"
> + * "pci1b4b,2b42"
>  
>  Optional properties:
>  
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c 
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 3c3c4f1..f7c84d3 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -37,6 +37,22 @@ static struct mwifiex_if_ops pcie_ops;
>  
>  static struct semaphore add_remove_card_sem;
>  
> +static const struct of_device_id mwifiex_pcie_of_match_table[] = {
> + { .compatible = "pci11ab,2b42" },
> + { .compatible = "pci1b4b,2b42" },
> + { }
> +};
> +
> +static int mwifiex_pcie_probe_of(struct device *dev)
> +{
> + if (!of_match_node(mwifiex_pcie_of_match_table, dev->of_node)) {
> + dev_err(dev, "required compatible string missing\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
>  static int
>  mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct sk_buff *skb,
>  size_t size, int flags)
> @@ -178,6 +194,7 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
>   const struct pci_device_id *ent)
>  {
>   struct pcie_service_card *card;
> + int ret;
>  
>   pr_debug("info: vendor=0x%4.04X device=0x%4.04X rev=%d\n",
>pdev->vendor, pdev->device, pdev->revision);
> @@ -199,13 +216,24 @@ static int mwifiex_pcie_probe(struct pci_dev *pdev,
>   card->pcie.can_ext_scan = data->can_ext_scan;
>   }
>  
> - if (mwifiex_add_card(card, _remove_card_sem, _ops,
> -  MWIFIEX_PCIE)) {
> - pr_err("%s failed\n", __func__);
> - return -1;
> + /* device tree node parsing and platform specific configuration*/
> + if (pdev->dev.of_node) {
> + ret = mwifiex_pcie_probe_of(>dev);
> + if (ret)
> + goto err_free;
>   }
>  
> + ret = mwifiex_add_card(card, _remove_card_sem, _ops,
> +MWIFIEX_PCIE);
> + if (ret) {
> + 

RE: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Amitkumar Karwar]

Hi Dmitry,

> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Wednesday, October 26, 2016 10:06 PM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@vger.kernel.org; Cathy Luo; Nishant
> Sarmukadam
> Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing'
> in shutdown_drv
> 
> Hi Amit,
> 
> On Wed, Oct 26, 2016 at 03:23:08PM +, Amitkumar Karwar wrote:
> >
> > This race won't occur. At this point of time(i.e while calling
> mwifiex_shutdown_drv() in deinit), following things are completed. We
> don't expect mwifiex_main_process() to be scheduled.
> > 1) Connection to peer device is terminated at the beginning of
> teardown thread. So we don't receive any Tx data from kernel.
> > 2) Last command SHUTDOWN is exchanged with firmware. So there won't be
> any activity/interrupt from firmware.
> > 3) Interrupts are disabled.
> > 4) "adapter->surprise_removed" flag is set. It will skip
> mwifiex_main_process() calls.
> >
> > ---
> > static void mwifiex_main_work_queue(struct work_struct *work) {
> > struct mwifiex_adapter *adapter =
> > container_of(work, struct mwifiex_adapter, main_work);
> >
> > if (adapter->surprise_removed)
> > return;
> > mwifiex_main_process(adapter); }
> > --
> > 5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and
> destroy workqueue.
> 
> OK, but if interrupts are disabled and you ensure that work is flushed
> or completed before you call mwifiex_shutdown_drv() then I do not
> understand why you need all of this at all? Why do you need to check
> status in mwifiex_shutdown_drv() and why do you want
> mwifiex_main_process() to call mwifiex_shutdown_drv() in certain cases?
> Can you simply remove all this stuff?
> 

I agree. This code is there for long time. I will prepare a patch for this 
cleanup work.

Regards,
Amitkumar

Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Dmitry Torokhov]

Hi Amit,

On Wed, Oct 26, 2016 at 03:23:08PM +, Amitkumar Karwar wrote:
> 
> This race won't occur. At this point of time(i.e while calling 
> mwifiex_shutdown_drv() in deinit), following things are completed. We don't 
> expect mwifiex_main_process() to be scheduled.
> 1) Connection to peer device is terminated at the beginning of teardown 
> thread. So we don't receive any Tx data from kernel.
> 2) Last command SHUTDOWN is exchanged with firmware. So there won't be any 
> activity/interrupt from firmware.
> 3) Interrupts are disabled.
> 4) "adapter->surprise_removed" flag is set. It will skip 
> mwifiex_main_process() calls.
> 
> ---
> static void mwifiex_main_work_queue(struct work_struct *work)
> {
> struct mwifiex_adapter *adapter =
> container_of(work, struct mwifiex_adapter, main_work);
> 
> if (adapter->surprise_removed)
> return;
> mwifiex_main_process(adapter);
> }
> --
> 5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and destroy 
> workqueue.

OK, but if interrupts are disabled and you ensure that work is flushed
or completed before you call mwifiex_shutdown_drv() then I do not
understand why you need all of this at all? Why do you need to check
status in mwifiex_shutdown_drv() and why do you want
mwifiex_main_process() to call mwifiex_shutdown_drv() in certain cases?
Can you simply remove all this stuff?

Thanks.

-- 
Dmitry

Re: [PATCH 2/8] mac80211: Allow AUTH_DATA to be used for FILS

[Johannes Berg]

> This is admittedly a bit strange design with that special case needed
> for SAE. If we were to design the SAE case now in combination with
> FILS, I guess this would be quite different (e.g., separate
> attributes for Authentication transaction sequence number and Status
> code). Unlike the mesh use case with SAE, FILS is only between an AP
> and a station and as such, there would not really be a case where the
> station would send an Authentication frame with non-zero Status code.
> 
> A future amendment might define a new authentication algorithm that
> ends up using more than a single Authentication frame exchange. In
> such a case, we would actually have need for Authentication
> transaction sequence number even though FILS doesn't need it.
> 
> I think I'd rather maintain a consistent attribute design for all
> authentication algorithms and leave this as-is now. Another option
> would be to not apply the rename SAE attributes patch and define
> something new as a more generic solution, but I'm not sure there is
> sufficient justification for the added complexity since we cannot
> really get rid of the current SAE design any time soon.

Yes, fair point.

Maybe you can clarify the nl80211 attribute documentation wrt. this? It
just states that it starts with the Authentication transaction sequence
field, but afaict that's not true, it also has the status code field,
which is also ignored here.

johannes

Re: [PATCH] rtl8xxxu: Add D-Link DWA-131 rev E1

[Jes Sorensen]

Barry Day  writes:
> This is a rtl8192eu dongle and has been tested
>
> Signed-off-by: Barry Day 
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks, I'll add it to my queue, unless Kalle grabs it first. Glad to
know it's working for you!

Jes

> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c 
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index 04141e5..d426836 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6009,7 +6009,7 @@ static int rtl8xxxu_probe(struct usb_interface 
> *interface,
>   untested = 0;
>   break;
>   case 0x2001:
> - if (id->idProduct == 0x3308)
> + if (id->idProduct == 0x3308 || id->idProduct == 0x3319)
>   untested = 0;
>   break;
>   case 0x2357:
> @@ -6188,6 +6188,8 @@ static struct usb_device_id dev_table[] = {
>   .driver_info = (unsigned long)_fops},
>  {USB_DEVICE_AND_INTERFACE_INFO(USB_VENDOR_ID_REALTEK, 0x818b, 0xff, 0xff, 
> 0xff),
>   .driver_info = (unsigned long)_fops},
> +{USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x3319, 0xff, 0xff, 0xff),
> + .driver_info = (unsigned long)_fops},
>  /* Tested by Myckel Habets */
>  {USB_DEVICE_AND_INTERFACE_INFO(0x2357, 0x0109, 0xff, 0xff, 0xff),
>   .driver_info = (unsigned long)_fops},

Re: [PATCH 2/8] mac80211: Allow AUTH_DATA to be used for FILS

[Malinen, Jouni]

On Wed, Oct 26, 2016 at 07:30:00AM +0200, Johannes Berg wrote:
> 
> >     if (req->auth_data_len >= 4) {
> > -   __le16 *pos = (__le16 *) req->auth_data;
> > -   auth_data->sae_trans = le16_to_cpu(pos[0]);
> > -   auth_data->sae_status = le16_to_cpu(pos[1]);
> > +   if (req->auth_type == NL80211_AUTHTYPE_SAE) {
> > +   __le16 *pos = (__le16 *) req->auth_data;
> > +   auth_data->sae_trans = le16_to_cpu(pos[0]);
> > +   auth_data->sae_status = le16_to_cpu(pos[1]);
> > +   }
> >     memcpy(auth_data->data, req->auth_data + 4,
> >        req->auth_data_len - 4);
> >     auth_data->data_len += req->auth_data_len - 4;
> 
> Hmm. Do we really want to still skip the first four bytes of the data
> userspace passed? That seems a bit strange to me. The docs in nl80211.h
> do say it that way now, but should we really include a dummy
> Authentication transaction sequence number field?

This is admittedly a bit strange design with that special case needed
for SAE. If we were to design the SAE case now in combination with FILS,
I guess this would be quite different (e.g., separate attributes for
Authentication transaction sequence number and Status code). Unlike the
mesh use case with SAE, FILS is only between an AP and a station and as
such, there would not really be a case where the station would send an
Authentication frame with non-zero Status code.

A future amendment might define a new authentication algorithm that ends
up using more than a single Authentication frame exchange. In such a
case, we would actually have need for Authentication transaction
sequence number even though FILS doesn't need it.

I think I'd rather maintain a consistent attribute design for all
authentication algorithms and leave this as-is now. Another option would
be to not apply the rename SAE attributes patch and define something new
as a more generic solution, but I'm not sure there is sufficient
justification for the added complexity since we cannot really get rid of
the current SAE design any time soon.

-- 
Jouni MalinenPGP id EFC895FA

RE: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing' in shutdown_drv

[Amitkumar Karwar]

Hi Dmitry,

> From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> Sent: Tuesday, October 25, 2016 10:05 PM
> To: Amitkumar Karwar
> Cc: Brian Norris; linux-wireless@vger.kernel.org; Cathy Luo; Nishant
> Sarmukadam
> Subject: Re: [PATCH 2/5] mwifiex: use spinlock for 'mwifiex_processing'
> in shutdown_drv
> 
> On Tue, Oct 25, 2016 at 04:11:14PM +, Amitkumar Karwar wrote:
> > Hi Dmitry,
> >
> > > From: Dmitry Torokhov [mailto:dmitry.torok...@gmail.com]
> > > Sent: Tuesday, October 25, 2016 5:28 AM
> > > To: Brian Norris
> > > Cc: Amitkumar Karwar; linux-wireless@vger.kernel.org; Cathy Luo;
> > > Nishant Sarmukadam
> > > Subject: Re: [PATCH 2/5] mwifiex: use spinlock for
> 'mwifiex_processing'
> > > in shutdown_drv
> > >
> > > On Mon, Oct 24, 2016 at 12:19:15PM -0700, Brian Norris wrote:
> > > > On Mon, Oct 24, 2016 at 07:51:29PM +0530, Amitkumar Karwar wrote:
> > > > > This variable is guarded by spinlock at all other places. This
> > > > > patch takes care of missing spinlock usage in
> mwifiex_shutdown_drv().
> > > > >
> > > > > Signed-off-by: Amitkumar Karwar 
> > > > > ---
> > > > >  drivers/net/wireless/marvell/mwifiex/init.c | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > b/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > index 82839d9..8e5e424 100644
> > > > > --- a/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > +++ b/drivers/net/wireless/marvell/mwifiex/init.c
> > > > > @@ -670,11 +670,14 @@ mwifiex_shutdown_drv(struct
> > > > > mwifiex_adapter
> > > > > *adapter)
> > > > >
> > > > >   adapter->hw_status = MWIFIEX_HW_STATUS_CLOSING;
> > > > >   /* wait for mwifiex_process to complete */
> > > > > + spin_lock_irqsave(>main_proc_lock, flags);
> > > > >   if (adapter->mwifiex_processing) {
> > > >
> > > > I'm not quite sure why we have this check in the first place; if
> > > > the main process is still running, then don't we have bigger
> > > > problems here anyway? But this is definitely the "right" way to
> check this flag, so:
> > >
> > > No, this is definitely not right way to check it. What exactly does
> > > this spinlock protect? It seems that the intent is to make sure we
> > > do not call mwifiex_shutdown_drv() while mwifiex_main_process() is
> executing.
> > > It even says above that we are "waiting" for it (but we do not, we
> > > may bail out or we may not, depends on luck).
> > >
> > > To implement this properly you not only need to take a lock and
> > > check the flag, but keep the lock until mwifiex_shutdown_drv() is
> > > done, or use some other way to let mwifiex_main_process() know it
> > > should not access the adapter while mwifiex_shutdown_drv() is
> running.
> > >
> > > NACK.
> > >
> >
> > As I explained in other email, here is the sequence.
> > 1) We find mwifiex_processing is true in mwifiex_shitdown_drv(). "-
> EINPROGRESS" is returned by the function and hw_status state is changed
> to MWIFIEX_HW_STATUS_CLOSING.
> > 2) We wait until main_process is completed.
> >
> > if (mwifiex_shutdown_drv(adapter) == -EINPROGRESS)
> > wait_event_interruptible(adapter->init_wait_q,
> >
> > adapter->init_wait_q_woken);
> >
> > 3) We have following check at the end of main_process()
> >
> > exit_main_proc:
> > if (adapter->hw_status == MWIFIEX_HW_STATUS_CLOSING)
> > mwifiex_shutdown_drv(adapter);
> >
> > 4) Here at the end of mwifiex_shutdown_drv(), we are setting
> > "adapter->init_wait_q_woken" and waking up the thread in point (2)
> 
> 1. We do not find mwifiex_processing to be true
> 2. We proceed to try and shut down the driver
> 3. Interrupt is raised in the meantime, kicking work item
> 4. mwifiex_main_process() sees that adapter->hw_status is
> MWIFIEX_HW_STATUS_CLOSING and jumps to exit_main_proc
> 5.mwifiex_main_process() calls into mwifiex_shutdown_drv() that is now
> racing with another copy of the same.

This race won't occur. At this point of time(i.e while calling 
mwifiex_shutdown_drv() in deinit), following things are completed. We don't 
expect mwifiex_main_process() to be scheduled.
1) Connection to peer device is terminated at the beginning of teardown thread. 
So we don't receive any Tx data from kernel.
2) Last command SHUTDOWN is exchanged with firmware. So there won't be any 
activity/interrupt from firmware.
3) Interrupts are disabled.
4) "adapter->surprise_removed" flag is set. It will skip mwifiex_main_process() 
calls.

---
static void mwifiex_main_work_queue(struct work_struct *work)
{
struct mwifiex_adapter *adapter =
container_of(work, struct mwifiex_adapter, main_work);

if (adapter->surprise_removed)
return;
mwifiex_main_process(adapter);
}
--
5) We have "mwifiex_terminate_workqueue(adapter)" call to flush and destroy 
workqueue.

> 
> It seems to me that mwifiex_main_process() is 

Re: [ath9k-devel] [NOT FOR MERGE] ath9k: work around key cache corruption

[Ferry Huberts]

do a grep in the kernel: dual license bsd/gpl

On 26/10/16 16:05, Kalle Valo wrote:

Antonio Quartulli  writes:


From: Antonio Quartulli 

This patch was crafted long time ago to work around a key cache
corruption problem on ath9k chipsets.

The workaround consists in periodically triggering a worker that
uploads all the keys to the HW cache. The worker is triggered also
when the vif detects 21 undecryptable packets.


This patch is based on top compat-wireless-2015-10-26.


I was asked to release this code to the public and, since it is
GPL, I am now doing it.


GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
sentence above, but it's possible to interpret it so that this patch is
not under ISC license, which is problematic.



--
Ferry Huberts

Re: [NOT FOR MERGE] ath9k: work around key cache corruption

[Kalle Valo]

Antonio Quartulli  writes:

> From: Antonio Quartulli 
>
> This patch was crafted long time ago to work around a key cache
> corruption problem on ath9k chipsets.
>
> The workaround consists in periodically triggering a worker that
> uploads all the keys to the HW cache. The worker is triggered also
> when the vif detects 21 undecryptable packets.
>
>
> This patch is based on top compat-wireless-2015-10-26.
>
>
> I was asked to release this code to the public and, since it is
> GPL, I am now doing it.

GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
sentence above, but it's possible to interpret it so that this patch is
not under ISC license, which is problematic.

-- 
Kalle Valo

Re: [NOT FOR MERGE] ath9k: work around key cache corruption

[Antonio Quartulli]

On Wed, Oct 26, 2016 at 05:05:14PM +0300, Kalle Valo wrote:
> Antonio Quartulli  writes:
> 
> > From: Antonio Quartulli 
> >
> > This patch was crafted long time ago to work around a key cache
> > corruption problem on ath9k chipsets.
> >
> > The workaround consists in periodically triggering a worker that
> > uploads all the keys to the HW cache. The worker is triggered also
> > when the vif detects 21 undecryptable packets.
> >
> >
> > This patch is based on top compat-wireless-2015-10-26.
> >
> >
> > I was asked to release this code to the public and, since it is
> > GPL, I am now doing it.
> 
> GPL? AFAICS ath9k is under ISC. I'm not sure what you mean with the
> sentence above, but it's possible to interpret it so that this patch is
> not under ISC license, which is problematic.

Honestly, my sentence was just a way to say "it makes sense to release this
patch to the public".

If it needs to be ISC in order to be merged, then it can be released under ISC.

I don't want to enter a legal case :) I just to make the patch public

Cheers,

-- 
Antonio Quartulli


signature.asc
Description: Digital signature

Re: [PATCH v2 2/2] mac80211: passively scan DFS channels if requested

[Simon Wunderlich]

On Wednesday, October 26, 2016 2:58:56 PM CEST Johannes Berg wrote:
> > (NOTE: going off-channel while operating is a different topic).
> 
> Why do you think it's different?
> 
> Seems exactly the same to me, since you come back to the channel and
> start sending without any new checking?

There are two ways to leave a channel:

1.) Leave the operating channel "permanently". Then the ETSI 301 893 says:
"If no radar was detected on an Operating Channel but the RLAN stops operating 
on that channel, then the channel becomes an Available Channel." (4.7.1.4 
Master Devices (c))

Then we can select a new operating channel and re-start the process.

2.) Off-Channel CAC: This is an optional feature. Simply spoken, we keep the 
channel operating, but try to do CAC-checking on a different channel by 
shortly switching to that new channel from time to time. The new channel must 
be checked for a longer time than the standard CAC time (6 minutes for non-
weather radar channels in ETSI).

The main difference is that we keep the current channel as "operating channel" 
and are also required to be able to detect radars with the same probability.

Also note that this feature is not implemented in Linux as far as I know, and 
personally I haven't seen it in the wild yet.

From 4.7.2.3.1:

"Off-Channel CAC is defined as an optional mechanism by which an RLAN device 
monitors channel(s), different from the Operating Channel(s), for the presence 
of radar signals. The Off-Channel CAC may be used in addition to the Channel 
Availability Check defined in clause 4.7.2.2, for identifying Available 
Channels.

Off-Channel CAC is performed by a number of non-continuous checks spread over 
a period in time. This period, which is required to determine the presence of 
radar signals, is defined as the Off-Channel CAC Time.

If no radars have been detected in a channel, then that channel becomes an 
Available Channel."

Cheers,
 Simon

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH][RFC] nl80211/mac80211: Rounded RSSI reporting

[Johannes Berg]

On Fri, 2016-10-21 at 19:03 +, Zaborowski, Andrew wrote:

> > The problem is that you really want this to be offloaded to the
> > device, *especially* if you care about low power usage, because you
> > absolutely don't want to be processing each beacon (which is
> > typically what we derive the data from today) in the host CPU - you
> > want those to be filtered by the device so you don't wake up the
> > host CPU every ~102ms.
> 
> Right, that would be a separate optimisation but it probably won't
> arrive until there's an API that the drivers can implement, so I
> think this is a prerequisite.

Huh? No, beacon filtering is implemented by a lot of drivers today.

> The userspace switches count too and are the main motivation for this
> patch.  Eventually, as you say, things will depend on specific
> drivers and you will want to optimise whatever you can assuming the
> hardware you're constrained to.

Yes and no. I think we can probably define a reasonable subset that
you'd expect more devices to implement. I don't really see the
requirement to do the "banding" that you did here offloaded - doing it
in mac80211 won't help much, and won't work in cases where you have
beacon filtering already anyway.

Drivers like iwlwifi would be able to implement the banding in
software, since they get a notification on every change above a
configurable threshold, but other drivers like one I looked at actually
do have a low and high threshold today, and program them to be the same
value with our current CQM API definitions.

> It seems like any mechanism you choose can be implemented on top of
> hardware that supports a low and a high threshold by setting the next
> values while handling the event related to the previous threshold
> crossing event.  If the thresholds are specified by a middle value
> and a hysteresis or distance, that would work equally well.
> The API I added in the patch also allows offloading to such hardware.

Well, ok, technically the API can be implemented on top of drivers with
low/high thresholds, by doing the configuration according to the
current range you're in.

I would argue though that it makes more sense to expose a simpler
capability (e.g. two instead of the current single threshold) and do
the reprogramming from higher layers. That ends up being more flexible,
since you can then, for example, also have ranges that aren't all
identical - which makes some sense because above a certain level you
don't really care at all.

> In short a nl80211 change would be needed regardless of the mechanism
> chosen.

Agree. I'm just not convinced that the banding mechanism you propose is
the most reasonable choice for new API.

johannes

[RFC] cfg80211: support 4-way-handshake offload with PSK and 802.1X

[Johannes Berg]

From: Avraham Stern 

TODO:
 * add a separate capability flag? and explain how the offload
   is supposed to work in 802.1X, EAPOL-Key messages are going
   to be processed by the supplicant, but then the 4-way-HS is
   done by the device after getting the PMK in the PMKSA cache
   entry - explain that mechanism
 * does anyone still want EAP-LEAP 16-byte PMK?

Signed-off-by: Avraham Stern 
Signed-off-by: Johannes Berg 
---
 include/linux/ieee80211.h|  3 +++
 include/net/cfg80211.h   |  8 +++-
 include/uapi/linux/nl80211.h | 14 +-
 net/wireless/nl80211.c   | 15 +++
 4 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a80516fd65c8..40206b2a6e6d 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2326,6 +2326,9 @@ enum ieee80211_sa_query_action {
 
 #define WLAN_PMKID_LEN 16
 
+#define WLAN_PMK_LEN   32
+#define WLAN_PMK_LEN_SUITE_B_192   48
+
 #define WLAN_OUI_WFA   0x506f9a
 #define WLAN_OUI_TYPE_WFA_P2P  9
 #define WLAN_OUI_MICROSOFT 0x0050f2
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d1ffbc3a8e55..3bb407c57177 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -615,6 +615,7 @@ struct survey_info {
  * @wep_keys: static WEP keys, if not NULL points to an array of
  * CFG80211_MAX_WEP_KEYS WEP keys
  * @wep_tx_key: key index (0..3) of the default TX static WEP key
+ * @psk: PSK (for devices supporting 4-way-handshake offload, 32 bytes)
  */
 struct cfg80211_crypto_settings {
u32 wpa_versions;
@@ -628,6 +629,7 @@ struct cfg80211_crypto_settings {
bool control_port_no_encrypt;
struct key_params *wep_keys;
int wep_tx_key;
+   const u8 *psk;
 };
 
 /**
@@ -2064,11 +2066,15 @@ enum wiphy_params_flags {
  * caching.
  *
  * @bssid: The AP's BSSID.
- * @pmkid: The PMK material itself.
+ * @pmkid: The PMK identifier.
+ * @pmk: The PMK material itself.
+ * @pmk_len: The PMK length in bytes.
  */
 struct cfg80211_pmksa {
const u8 *bssid;
const u8 *pmkid;
+   const u8 *pmk;
+   u8 pmk_len;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1362d24957b5..40b003cc07bd 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -371,7 +371,8 @@
  * NL80211_CMD_GET_SURVEY and on the "scan" multicast group)
  *
  * @NL80211_CMD_SET_PMKSA: Add a PMKSA cache entry, using %NL80211_ATTR_MAC
- * (for the BSSID) and %NL80211_ATTR_PMKID.
+ * (for the BSSID) and %NL80211_ATTR_PMKID. Optionally, %NL80211_ATTR_PMK
+ * can be used to specify the PMK.
  * @NL80211_CMD_DEL_PMKSA: Delete a PMKSA cache entry, using %NL80211_ATTR_MAC
  * (for the BSSID) and %NL80211_ATTR_PMKID.
  * @NL80211_CMD_FLUSH_PMKSA: Flush all PMKSA cache entries.
@@ -1937,6 +1938,11 @@ enum nl80211_commands {
  * @NL80211_ATTR_NAN_MATCH: used to report a match. This is a nested attribute.
  * See  nl80211_nan_match_attributes.
  *
+ * @NL80211_ATTR_PMK: PMK for offloaded 4-Way Handshake. Relevant with
+ * %NL80211_CMD_CONNECT (for WPA/WPA2-PSK networks) when PSK is used, or
+ * with %NL80211_CMD_SET_PMKSA when 802.1X authentication is used and for
+ * PMKSA caching.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2336,6 +2342,8 @@ enum nl80211_attrs {
NL80211_ATTR_NAN_FUNC,
NL80211_ATTR_NAN_MATCH,
 
+   NL80211_ATTR_PMK,
+
/* add attributes here, update the policy in nl80211.c */
 
__NL80211_ATTR_AFTER_LAST,
@@ -4638,6 +4646,9 @@ enum nl80211_feature_flags {
  * configuration (AP/mesh) with HT rates.
  * @NL80211_EXT_FEATURE_BEACON_RATE_VHT: Driver supports beacon rate
  * configuration (AP/mesh) with VHT rates.
+ * @NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA: Device supports
+ * doing 4-way handshake in station mode (PSK is passed as part
+ * of the connect command).
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4652,6 +4663,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_BEACON_RATE_LEGACY,
NL80211_EXT_FEATURE_BEACON_RATE_HT,
NL80211_EXT_FEATURE_BEACON_RATE_VHT,
+   NL80211_EXT_FEATURE_4WAY_HANDSHAKE_OFFLOAD_STA,
 
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b0440de82171..6720c7bf3ed1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -414,6 +414,7 @@ static const struct nla_policy 
nl80211_policy[NUM_NL80211_ATTR] = {

Re: [PATCH][RFC] nl80211/mac80211: Rounded RSSI reporting

[Johannes Berg]

On Fri, 2016-10-21 at 09:20 -0500, Denis Kenzior wrote:

> > It's actually not clear to me that this is really how it should be.
> > There's a point to be made that taking a more holistic "link
> > quality" would be a better choice. That's related, but maybe can be
> > a separate discussion.

> Can you elaborate on this 'link quality' idea?

Well, I didn't really want to - getting 3 system folks into a room will
result in 4 different ways of doing it - but you can take into account
not just the RSSI, but also the bitrate you can reasonably use on the
channel/with the AP, the noise you can perhaps detect (if you can), the
amount of packet loss or retransmissions you experience, etc.

I think that some systems (Android, maybe Windows) already do something
more complex than pure RSSI indicators, but I don't really know for
sure.

> > Yes, this would be ideal.
> > 
> > [...]

see my other email

> This sounds really brittle.  Furthermore, we also need a facility to 
> know when signal strength is getting low to trigger roaming
> logic.  This would mean sharing CQM facility between roaming & signal
> strength notifications.  As you wrote above, things become quite
> impractical.

This would likely go through the supplicant anyway, so it could manage
proper range overlaps etc. for this.

It does seem brittle if we just have a single value, but if we add
low/high thresholds (with hysteresis) then I think we can do this, and
gain more flexibility in the process. But let's discuss more details
over in the other email I just sent :)

johannes

Re: [PATCH v2 2/2] mac80211: passively scan DFS channels if requested

[Johannes Berg]

On Mon, 2016-10-24 at 16:53 +0200, Simon Wunderlich wrote:
> [snip]
> 
> Again, no explicit "on installation" here, but there is also nothing
> saying  that we can not check/operate on other channels in the
> meantime. 

Yeah, ok.

> (NOTE: going off-channel while operating is a different topic).

Why do you think it's different?

Seems exactly the same to me, since you come back to the channel and
start sending without any new checking?

johannes

[PATCH] nl80211: use nla_parse_nested() instead of nla_parse()

[Johannes Berg]

From: Johannes Berg 

It's just an inline doing the same thing, but the code
is nicer with it.

Signed-off-by: Johannes Berg 
---
 net/wireless/nl80211.c | 85 ++
 1 file changed, 37 insertions(+), 48 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b0440de82171..ad49a4c43e01 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2318,10 +2318,9 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct 
genl_info *info)
nla_for_each_nested(nl_txq_params,
info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS],
rem_txq_params) {
-   result = nla_parse(tb, NL80211_TXQ_ATTR_MAX,
-  nla_data(nl_txq_params),
-  nla_len(nl_txq_params),
-  txq_params_policy);
+   result = nla_parse_nested(tb, NL80211_TXQ_ATTR_MAX,
+ nl_txq_params,
+ txq_params_policy);
if (result)
return result;
result = parse_txq_params(tb, _params);
@@ -3571,8 +3570,8 @@ static int nl80211_parse_tx_bitrate_mask(struct genl_info 
*info,
sband = rdev->wiphy.bands[band];
if (sband == NULL)
return -EINVAL;
-   err = nla_parse(tb, NL80211_TXRATE_MAX, nla_data(tx_rates),
-   nla_len(tx_rates), nl80211_txattr_policy);
+   err = nla_parse_nested(tb, NL80211_TXRATE_MAX, tx_rates,
+  nl80211_txattr_policy);
if (err)
return err;
if (tb[NL80211_TXRATE_LEGACY]) {
@@ -6328,9 +6327,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct 
genl_info *info)
 
nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
rem_reg_rules) {
-   r = nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,
- nla_data(nl_reg_rule), nla_len(nl_reg_rule),
- reg_rule_policy);
+   r = nla_parse_nested(tb, NL80211_REG_RULE_ATTR_MAX,
+nl_reg_rule, reg_rule_policy);
if (r)
goto bad_reg;
r = parse_reg_rule(tb, >reg_rules[rule_idx]);
@@ -6397,8 +6395,8 @@ static int parse_bss_select(struct nlattr *nla, struct 
wiphy *wiphy,
if (!nla_ok(nest, nla_len(nest)))
return -EINVAL;
 
-   err = nla_parse(attr, NL80211_BSS_SELECT_ATTR_MAX, nla_data(nest),
-   nla_len(nest), nl80211_bss_select_policy);
+   err = nla_parse_nested(attr, NL80211_BSS_SELECT_ATTR_MAX, nest,
+  nl80211_bss_select_policy);
if (err)
return err;
 
@@ -6788,9 +6786,8 @@ nl80211_parse_sched_scan_plans(struct wiphy *wiphy, int 
n_plans,
if (WARN_ON(i >= n_plans))
return -EINVAL;
 
-   err = nla_parse(plan, NL80211_SCHED_SCAN_PLAN_MAX,
-   nla_data(attr), nla_len(attr),
-   nl80211_plan_policy);
+   err = nla_parse_nested(plan, NL80211_SCHED_SCAN_PLAN_MAX,
+  attr, nl80211_plan_policy);
if (err)
return err;
 
@@ -6879,9 +6876,9 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct 
wireless_dev *wdev,
tmp) {
struct nlattr *rssi;
 
-   err = nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
-   nla_data(attr), nla_len(attr),
-   nl80211_match_policy);
+   err = nla_parse_nested(tb,
+  
NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
+  attr, nl80211_match_policy);
if (err)
return ERR_PTR(err);
/* add other standalone attributes here */
@@ -7052,9 +7049,9 @@ nl80211_parse_sched_scan(struct wiphy *wiphy, struct 
wireless_dev *wdev,
tmp) {
struct nlattr *ssid, *rssi;
 
-   err = nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
-   nla_data(attr), nla_len(attr),
-   nl80211_match_policy);
+   err = nla_parse_nested(tb,
+  

Re: [PATCH] cfg80211: add key management offload feature

[Johannes Berg]

Getting back to this ... as I was preparing my patch.

> @@ -3687,6 +3692,9 @@ enum nl80211_key_attributes {
>   NL80211_KEY_DEFAULT_MGMT,
>   NL80211_KEY_TYPE,
>   NL80211_KEY_DEFAULT_TYPES,
> + NL80211_KEY_REPLAY_CTR,
> + NL80211_KEY_KCK,
> + NL80211_KEY_KEK,

You made those key attributes, but ...
 
>    nla_put(msg, NL80211_ATTR_RESP_IE, resp_ie_len,
> resp_ie)))
>   goto nla_put_failure;
>  
> + if (wiphy_ext_feature_isset(>wiphy,
> + NL80211_EXT_FEATURE_KEY_MGMT_OFF
> LOAD) &&
> + (nla_put_u8(msg, NL80211_ATTR_AUTHORIZED, authorized) ||
> + (key_replay_ctr && nla_put(msg, NL80211_KEY_REPLAY_CTR,
> +  NL80211_REPLAY_CTR_LEN, key_replay_ctr)) ||
> + (key_kck &&
> +  nla_put(msg, NL80211_KEY_KCK, NL80211_KCK_LEN,
> key_kck)) ||
> + (key_kek &&
> +  nla_put(msg, NL80211_KEY_KEK, NL80211_KEK_LEN,
> key_kek
> + goto nla_put_failure;

Used them at a top level here! That can't possibly have worked.

Anyway, I checked and we can transport these without adding new
attributes, but adding the NL80211_ATTR_REKEY_DATA attribute with its
nested KEK, KCK and REPLAY_CTR.

That leaves the authorized attribute, I guess nesting a whole bunch of
station info etc. doesn't make a lot of sense.

I also fail to see how the data is actually configured down, since you
just pass it through. I'll send our patch for configuring the PMK/PSK
via the PMKSA cache separately in a few minutes.

johannes

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann  writes:

> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann  writes:
>> 
>> > A bugfix added a sanity check around the assignment and use of the
>> > 'is_11d' variable, which looks correct to me, but as the function is
>> > rather complex already, this confuses the compiler to the point where
>> > it can no longer figure out if the variable is always initialized
>> > correctly:
>> >
>> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
>> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used 
>> > uninitialized in this function [-Werror=maybe-uninitialized]
>> >
>> > This adds an initialization for the newly introduced case in which
>> > the variable should not really be used, in order to make the warning
>> > go away.
>> >
>> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
>> > Cc: Hante Meuleman 
>> > Cc: Arend van Spriel 
>> > Cc: Kalle Valo 
>> > Signed-off-by: Arnd Bergmann 
>> 
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.

Ok, I'll take it. I'm planning to push this to 4.9.

-- 
Kalle Valo

Re: [PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Johannes Berg]

On Wed, 2016-10-26 at 11:52 +0200, Thomas Graf wrote:
> On 10/26/16 at 10:59am, Johannes Berg wrote:
> > 
> > 
> > > 
> > >  /**
> > > + * nla_memdup - duplicate attribute memory (kmemdup)
> > > + * @src: netlink attribute to duplicate from
> > > + * @gfp: GFP mask
> > 
> > Actually, is there any point in passing a GFP mask? None of the
> > current
> > users need it, and it seems fairly unlikely to be needed since this
> > is
> > typically used on the netlink input path, where you surely
> > shouldn't
> > need GFP_ATOMIC or so?
> 
> I'm fine either way. I didn't want to make assumptions which need
> later changes. It's not hurting either and the function prototype
> is very small.

Yeah, I don't really care much - just wondered.

johannes

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Arnd Bergmann]

On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
> Arnd Bergmann  writes:
> 
> > A bugfix added a sanity check around the assignment and use of the
> > 'is_11d' variable, which looks correct to me, but as the function is
> > rather complex already, this confuses the compiler to the point where
> > it can no longer figure out if the variable is always initialized
> > correctly:
> >
> > brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> > brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used 
> > uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > This adds an initialization for the newly introduced case in which
> > the variable should not really be used, in order to make the warning
> > go away.
> >
> > Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> > Cc: Hante Meuleman 
> > Cc: Arend van Spriel 
> > Cc: Kalle Valo 
> > Signed-off-by: Arnd Bergmann 
> 
> Via which tree are you planning to submit this? Should I take it?

I'd prefer if you can take it and forward it along with your other
bugfixes. I'll try to take care of the ones that nobody else
picked up.

Arnd

Re: [PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Thomas Graf]

On 10/26/16 at 10:59am, Johannes Berg wrote:
> 
> >  /**
> > + * nla_memdup - duplicate attribute memory (kmemdup)
> > + * @src: netlink attribute to duplicate from
> > + * @gfp: GFP mask
> 
> Actually, is there any point in passing a GFP mask? None of the current
> users need it, and it seems fairly unlikely to be needed since this is
> typically used on the netlink input path, where you surely shouldn't
> need GFP_ATOMIC or so?

I'm fine either way. I didn't want to make assumptions which need
later changes. It's not hurting either and the function prototype
is very small.

[PATCH] nl80211: move unsplit command advertising to a separate function

[Johannes Berg]

From: Johannes Berg 

When we split the wiphy dump because it got too large, I added a
comment and asked that all new command advertising be done only
for userspace clients capable of receiving split data, in order
to not break older ones (which can't use the new commands anyway)

This mostly worked, and we haven't added many new commands, but
I occasionally get patches that modify the wrong place.

Make this easier to detect and understand by splitting out the
old commands to a separate function that makes it more clear it
should never be modified again.

Signed-off-by: Johannes Berg 
---
 net/wireless/nl80211.c | 142 +++--
 1 file changed, 79 insertions(+), 63 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 46cd48993ce9..b0440de82171 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1328,6 +1328,82 @@ nl80211_send_mgmt_stypes(struct sk_buff *msg,
return 0;
 }
 
+#define CMD(op, n) \
+do {   \
+   if (rdev->ops->op) {\
+   i++;\
+   if (nla_put_u32(msg, i, NL80211_CMD_ ## n)) \
+   goto nla_put_failure;   \
+   }   \
+   } while (0)
+
+static int nl80211_add_commands_unsplit(struct cfg80211_registered_device 
*rdev,
+   struct sk_buff *msg)
+{
+   int i = 0;
+
+   /*
+* do *NOT* add anything into this function, new things need to be
+* advertised only to new versions of userspace that can deal with
+* the split (and they can't possibly care about new features...
+*/
+   CMD(add_virtual_intf, NEW_INTERFACE);
+   CMD(change_virtual_intf, SET_INTERFACE);
+   CMD(add_key, NEW_KEY);
+   CMD(start_ap, START_AP);
+   CMD(add_station, NEW_STATION);
+   CMD(add_mpath, NEW_MPATH);
+   CMD(update_mesh_config, SET_MESH_CONFIG);
+   CMD(change_bss, SET_BSS);
+   CMD(auth, AUTHENTICATE);
+   CMD(assoc, ASSOCIATE);
+   CMD(deauth, DEAUTHENTICATE);
+   CMD(disassoc, DISASSOCIATE);
+   CMD(join_ibss, JOIN_IBSS);
+   CMD(join_mesh, JOIN_MESH);
+   CMD(set_pmksa, SET_PMKSA);
+   CMD(del_pmksa, DEL_PMKSA);
+   CMD(flush_pmksa, FLUSH_PMKSA);
+   if (rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL)
+   CMD(remain_on_channel, REMAIN_ON_CHANNEL);
+   CMD(set_bitrate_mask, SET_TX_BITRATE_MASK);
+   CMD(mgmt_tx, FRAME);
+   CMD(mgmt_tx_cancel_wait, FRAME_WAIT_CANCEL);
+   if (rdev->wiphy.flags & WIPHY_FLAG_NETNS_OK) {
+   i++;
+   if (nla_put_u32(msg, i, NL80211_CMD_SET_WIPHY_NETNS))
+   goto nla_put_failure;
+   }
+   if (rdev->ops->set_monitor_channel || rdev->ops->start_ap ||
+   rdev->ops->join_mesh) {
+   i++;
+   if (nla_put_u32(msg, i, NL80211_CMD_SET_CHANNEL))
+   goto nla_put_failure;
+   }
+   CMD(set_wds_peer, SET_WDS_PEER);
+   if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS) {
+   CMD(tdls_mgmt, TDLS_MGMT);
+   CMD(tdls_oper, TDLS_OPER);
+   }
+   if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
+   CMD(sched_scan_start, START_SCHED_SCAN);
+   CMD(probe_client, PROBE_CLIENT);
+   CMD(set_noack_map, SET_NOACK_MAP);
+   if (rdev->wiphy.flags & WIPHY_FLAG_REPORTS_OBSS) {
+   i++;
+   if (nla_put_u32(msg, i, NL80211_CMD_REGISTER_BEACONS))
+   goto nla_put_failure;
+   }
+   CMD(start_p2p_device, START_P2P_DEVICE);
+   CMD(set_mcast_rate, SET_MCAST_RATE);
+#ifdef CONFIG_NL80211_TESTMODE
+   CMD(testmode_cmd, TESTMODE);
+#endif
+   return i;
+ nla_put_failure:
+   return -ENOBUFS;
+}
+
 struct nl80211_dump_wiphy_state {
s64 filter_wiphy;
long start;
@@ -1555,68 +1631,9 @@ static int nl80211_send_wiphy(struct 
cfg80211_registered_device *rdev,
if (!nl_cmds)
goto nla_put_failure;
 
-   i = 0;
-#define CMD(op, n) \
-do {   \
-   if (rdev->ops->op) {\
-   i++;\
-   if (nla_put_u32(msg, i, NL80211_CMD_ ## n)) \
-   goto nla_put_failure;   \
-   }   \
-   } while 

[PATCH] wireless: only ask about WDS if EXPERT is selected

[Johannes Berg]

From: Johannes Berg 

There won't be any users of this that really need it and
aren't already experts in how the kernel functions, so
require setting EXPERT to reach this setting.

Signed-off-by: Johannes Berg 
---
 drivers/net/wireless/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/Kconfig b/drivers/net/wireless/Kconfig
index fa9c4fa9477e..8f5a3f4a43f2 100644
--- a/drivers/net/wireless/Kconfig
+++ b/drivers/net/wireless/Kconfig
@@ -18,7 +18,7 @@ menuconfig WLAN
 if WLAN
 
 config WIRELESS_WDS
-   bool "mac80211-based legacy WDS support"
+   bool "mac80211-based legacy WDS support" if EXPERT
help
  This option enables the deprecated WDS support, the newer
  mac80211-based 4-addr AP/client support supersedes it with
-- 
2.8.1

Re: [PATCH 8/8] mac80211: Claim Fast Initial Link Setup (FILS) support

[Johannes Berg]

On Wed, 2016-10-26 at 09:23 +, Malinen, Jouni wrote:
> On Wed, Oct 26, 2016 at 07:50:36AM +0200, Johannes Berg wrote:
> > 
> > On Wed, 2016-10-26 at 01:44 +0300, Jouni Malinen wrote:
> > > 
> > > With the previous commits, initial FILS support is now functional
> > > in
> > > mac80211-based drivers for both AP and stations roles.
> > 
> > That's a bit misleading, I guess AP role is handled entirely in
> > hostapd? You documented the extended feature bit to explicitly mean
> > station role only :)
> 
> Yeah.. In case of mac80211, there was not really changes needed in
> the kernel side for AP mode. That may be different with non-mac80211
> drivers, though, since they might be easier to handle with the AES-
> SIV operations for associations frames handled within the driver.

Right.

> Would you prefer to split that NL80211_EXT_FEATURE_FILS into two
> separate values (_STA and _AP) 

I think having it called _STA may be a little clearer, but I think I'm
OK with it the way it is (documented as station) as well.

> and have mac80211 advertise both? 

I wouldn't do that, there's nothing that makes it have that capability.

> Or just add a _STA only case for now and see what we need to do with
> NL80211_ATTR_DEVICE_AP_SME cases separately once such a thing is in
> in functional state?

Yeah, we should do that.

Really all I thought you should do was reword the commit message to
make it clear that the flag only implied the station case, and that the
AP case needed no changes.

Perhaps renaming the flag to ..._STA will make that a bit clearer.

johannes

Re: [PATCH 8/8] mac80211: Claim Fast Initial Link Setup (FILS) support

[Malinen, Jouni]

On Wed, Oct 26, 2016 at 07:50:36AM +0200, Johannes Berg wrote:
> On Wed, 2016-10-26 at 01:44 +0300, Jouni Malinen wrote:
> > With the previous commits, initial FILS support is now functional in
> > mac80211-based drivers for both AP and stations roles.
> 
> That's a bit misleading, I guess AP role is handled entirely in
> hostapd? You documented the extended feature bit to explicitly mean
> station role only :)

Yeah.. In case of mac80211, there was not really changes needed in the
kernel side for AP mode. That may be different with non-mac80211
drivers, though, since they might be easier to handle with the AES-SIV
operations for associations frames handled within the driver.

Would you prefer to split that NL80211_EXT_FEATURE_FILS into two
separate values (_STA and _AP) and have mac80211 advertise both? Or just
add a _STA only case for now and see what we need to do with
NL80211_ATTR_DEVICE_AP_SME cases separately once such a thing is in in
functional state?

-- 
Jouni MalinenPGP id EFC895FA

Re: [PATCH 5/8] cfg80211: Add KEK/nonces for FILS association frames

[Malinen, Jouni]

On Wed, Oct 26, 2016 at 07:36:27AM +0200, Johannes Berg wrote:
> > +++ b/net/wireless/nl80211.c
> > +   [NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
> > +   .len = FILS_MAX_KEK_LEN },
> > +   [NL80211_ATTR_FILS_NONCES] = { .type = NLA_BINARY,
> > +      .len = 2 * FILS_NONCE_LEN },

> If you remove the type = NLA_BINARY and just leave the type zero, then
> you'll get *minimum* length validation, rather than limiting the
> maximum length. That seems more appropriate for the nonces?

FILS_KEK is variable length, but FILS_NONCES should be exactly 2 *
FILS_NONCE_LEN. I didn't remember the minimum length case here, but yes,
that sounds reasonable for FILS_NONCES.

> > +   if (info->attrs[NL80211_ATTR_FILS_NONCES]) {
> > +   if (nla_len(info->attrs[NL80211_ATTR_FILS_NONCES])
> > !=
> > +   2 * FILS_NONCE_LEN)
> > +   return -EINVAL;
> 
> You're validating the *exact* length here, which unfortunately nlattr
> doesn't support right now, but perhaps we can live with checking that
> it's at least that many bytes, and using only 2*nonces? We do that for
> most other attributes (like MAC addresses).

This was because of the .len above ending up allowing shorter values..
Since we do that minimum length check only for number of other
attributes, I guess we can do it here as well and ignore whatever else
the user space might have added incorrectly.

> Or do we expect to extend this to more than 2 nonces in the future, at
> which point we'll need the length?

No, this should remain exactly two nonces and each of those having
exactly 16 octets.

-- 
Jouni MalinenPGP id EFC895FA

Re: [PATCH] NFC: nfcmrvl: Include unaligned.h instead of access_ok.h

[Tobias Klauser]

On 2016-10-26 at 11:00:12 +0200, Tobias Klauser  wrote:
> Including linux/unaligned/access_ok.h causes the allmodconfig build on
> ia64 (and maybe others) to fail with the following warnings:
> 
> include/linux/unaligned/access_ok.h:7:19: error: redefinition of 
> 'get_unaligned_le16'
> include/linux/unaligned/access_ok.h:12:19: error: redefinition of 
> 'get_unaligned_le32'
> include/linux/unaligned/access_ok.h:17:19: error: redefinition of 
> 'get_unaligned_le64'
> include/linux/unaligned/access_ok.h:22:19: error: redefinition of 
> 'get_unaligned_be16'
> include/linux/unaligned/access_ok.h:27:19: error: redefinition of 
> 'get_unaligned_be32'
> include/linux/unaligned/access_ok.h:32:19: error: redefinition of 
> 'get_unaligned_be64'
> include/linux/unaligned/access_ok.h:37:20: error: redefinition of 
> 'put_unaligned_le16'
> include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
> 'put_unaligned_le32'
> include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
> 'put_unaligned_le64'
> include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
> 'put_unaligned_be16'
> include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
> 'put_unaligned_be32'
> include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
> 'put_unaligned_be64'
> 
> Fix these by including asm/unaligned.h instead and leave it up to the
> architecture to decide how to implement unaligned accesses.
> 
> Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support")
> Reported-by: kbuild test robot 
> Link: https://lkml.org/lkml/2016/10/22/247
> Cc: Vincent Cuissard 
> Signed-off-by: Tobias Klauser 

There are two other instances of the same issue in the NFC subsystem,
namely in drivers/nfc/nxp-nci/firmware.c and drivers/nfc/nxp-nci/i2c.c

Guenter Roeck already sent a patch for these on 2015-08-01. It would be
nice if it could be applied as well.

[1] https://patchwork.kernel.org/patch/6922341/

Thanks

Re: [PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Daniel Borkmann]

On 10/26/2016 10:53 AM, Thomas Graf wrote:

Wrap several common instances of:
kmemdup(nla_data(attr), nla_len(attr), GFP_KERNEL);

Signed-off-by: Thomas Graf 


Thanks!

Acked-by: Daniel Borkmann 

Re: [PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Johannes Berg]

>  /**
> + * nla_memdup - duplicate attribute memory (kmemdup)
> + * @src: netlink attribute to duplicate from
> + * @gfp: GFP mask

Actually, is there any point in passing a GFP mask? None of the current
users need it, and it seems fairly unlikely to be needed since this is
typically used on the netlink input path, where you surely shouldn't
need GFP_ATOMIC or so?

johannes

[PATCH] NFC: nfcmrvl: Include unaligned.h instead of access_ok.h

[Tobias Klauser]

Including linux/unaligned/access_ok.h causes the allmodconfig build on
ia64 (and maybe others) to fail with the following warnings:

include/linux/unaligned/access_ok.h:7:19: error: redefinition of 
'get_unaligned_le16'
include/linux/unaligned/access_ok.h:12:19: error: redefinition of 
'get_unaligned_le32'
include/linux/unaligned/access_ok.h:17:19: error: redefinition of 
'get_unaligned_le64'
include/linux/unaligned/access_ok.h:22:19: error: redefinition of 
'get_unaligned_be16'
include/linux/unaligned/access_ok.h:27:19: error: redefinition of 
'get_unaligned_be32'
include/linux/unaligned/access_ok.h:32:19: error: redefinition of 
'get_unaligned_be64'
include/linux/unaligned/access_ok.h:37:20: error: redefinition of 
'put_unaligned_le16'
include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
'put_unaligned_le32'
include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
'put_unaligned_le64'
include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
'put_unaligned_be16'
include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
'put_unaligned_be32'
include/linux/unaligned/access_ok.h:42:20: error: redefinition of 
'put_unaligned_be64'

Fix these by including asm/unaligned.h instead and leave it up to the
architecture to decide how to implement unaligned accesses.

Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support")
Reported-by: kbuild test robot 
Link: https://lkml.org/lkml/2016/10/22/247
Cc: Vincent Cuissard 
Signed-off-by: Tobias Klauser 
---
 drivers/nfc/nfcmrvl/fw_dnld.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nfc/nfcmrvl/fw_dnld.c b/drivers/nfc/nfcmrvl/fw_dnld.c
index f8dcdf4b24f6..441c1b0ec7b5 100644
--- a/drivers/nfc/nfcmrvl/fw_dnld.c
+++ b/drivers/nfc/nfcmrvl/fw_dnld.c
@@ -17,7 +17,7 @@
  */
 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.0

Re: [PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Johannes Berg]

On Wed, 2016-10-26 at 10:53 +0200, Thomas Graf wrote:
> Wrap several common instances of:
>   kmemdup(nla_data(attr), nla_len(attr), GFP_KERNEL);

Makes sense

Acked-by: Johannes Berg 

johannes

Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw

[Johannes Berg]

On Wed, 2016-10-26 at 11:36 +0300, Jani Nikula wrote:
> On Wed, 26 Oct 2016, Johannes Berg  wrote:
> > 
> > On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote:
> > > 
> > > It's easier to manage the kernel-doc for the fields when they
> > > documentation is next to the field.
> > 
> > Ok, actually, this doesn't apply. Perhaps I'll look into scripting
> > this
> > kind of conversion.
> 
> No problem; I'll leave it up to you.

I'll probably end up doing it manually though - no chance I can extend
the parser to output an spatch (doing the comment placement with spatch
actually works, but getting there...)

[PATCH net-next] netlink: Add nla_memdup() to wrap kmemdup() use on nlattr

[Thomas Graf]

Wrap several common instances of:
kmemdup(nla_data(attr), nla_len(attr), GFP_KERNEL);

Signed-off-by: Thomas Graf 
---
 include/net/netlink.h  | 10 ++
 net/sched/act_bpf.c|  4 +---
 net/sched/cls_bpf.c|  4 +---
 net/wireless/nl80211.c |  3 +--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 254a0fc..a34f53a 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1191,6 +1191,16 @@ static inline struct in6_addr nla_get_in6_addr(const 
struct nlattr *nla)
 }
 
 /**
+ * nla_memdup - duplicate attribute memory (kmemdup)
+ * @src: netlink attribute to duplicate from
+ * @gfp: GFP mask
+ */
+static inline void *nla_memdup(const struct nlattr *src, gfp_t gfp)
+{
+   return kmemdup(nla_data(src), nla_len(src), gfp);
+}
+
+/**
  * nla_nest_start - Start a new level of nested attributes
  * @skb: socket buffer to add attributes to
  * @attrtype: attribute type of container
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 1d39600..9ff06cf 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -226,9 +226,7 @@ static int tcf_bpf_init_from_efd(struct nlattr **tb, struct 
tcf_bpf_cfg *cfg)
return PTR_ERR(fp);
 
if (tb[TCA_ACT_BPF_NAME]) {
-   name = kmemdup(nla_data(tb[TCA_ACT_BPF_NAME]),
-  nla_len(tb[TCA_ACT_BPF_NAME]),
-  GFP_KERNEL);
+   name = nla_memdup(tb[TCA_ACT_BPF_NAME], GFP_KERNEL);
if (!name) {
bpf_prog_put(fp);
return -ENOMEM;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index bb1d5a4..52dc85a 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -369,9 +369,7 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct 
cls_bpf_prog *prog,
return PTR_ERR(fp);
 
if (tb[TCA_BPF_NAME]) {
-   name = kmemdup(nla_data(tb[TCA_BPF_NAME]),
-  nla_len(tb[TCA_BPF_NAME]),
-  GFP_KERNEL);
+   name = nla_memdup(tb[TCA_BPF_NAME], GFP_KERNEL);
if (!name) {
bpf_prog_put(fp);
return -ENOMEM;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c510810..7bfe1e0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10638,8 +10638,7 @@ static int handle_nan_filter(struct nlattr *attr_filter,
 
i = 0;
nla_for_each_nested(attr, attr_filter, rem) {
-   filter[i].filter = kmemdup(nla_data(attr), nla_len(attr),
-  GFP_KERNEL);
+   filter[i].filter = nla_memdup(attr, GFP_KERNEL);
filter[i].len = nla_len(attr);
i++;
}
-- 
2.7.4

Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw

[Jani Nikula]

On Wed, 26 Oct 2016, Johannes Berg  wrote:
> On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote:
>> It's easier to manage the kernel-doc for the fields when they
>> documentation is next to the field.
>
> Ok, actually, this doesn't apply. Perhaps I'll look into scripting this
> kind of conversion.

No problem; I'll leave it up to you.

BR,
Jani.

>
> johannes

-- 
Jani Nikula, Intel Open Source Technology Center

pull-request: iwlwifi-next 2016-10-25-2

[Luca Coelho]

Hi Kalle,

Here's an updated pull-request, replacing 2016-10-25, with one commit
message reworded, as you requested.

I sent v2 of that patch to the linux-wireless mailing list.

Let me know if everything's fine (or not). :)

Luca.


The following changes since commit 15b95a15950238eff4d7f24be1716086eea67835:

  Merge ath-next from 
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git (2016-09-28 
16:37:33 +0300)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next.git 
tags/iwlwifi-next-for-kalle-2016-10-25-2

for you to fetch changes up to 7e62a699aafbd97928f19a8356d719b71b0e151c:

  iwlwifi: mvm: use dev_coredumpsg() (2016-10-26 11:04:15 +0300)


* Finalize and enable dynamic queue allocation;
* Use dev_coredumpmsg() to prevent locking the driver;
* Small fix to pass the AID to the FW;
* Use FW PS decisions with multi-queue;


Aviya Erenfeld (1):
  iwlwifi: mvm: use dev_coredumpsg()

Emmanuel Grumbach (1):
  iwlwifi: mvm: tell the firmware about the AID of the peer

Johannes Berg (1):
  iwlwifi: mvm: use firmware station PM notification for AP_LINK_PS

Liad Kaufman (5):
  iwlwifi: mvm: update txq metadata to current owner
  iwlwifi: mvm: fix reserved txq freeing
  iwlwifi: mvm: support MONITOR vif in DQA mode
  iwlwifi: mvm: fix dqa deferred frames marking
  iwlwifi: mvm: enable dynamic queue allocation mode

Sara Sharon (1):
  iwlwifi: mvm: assign cab queue to the correct station

Sharon Dvir (1):
  iwlwifi: pcie: give a meaningful name to interrupt request

 drivers/net/wireless/intel/iwlwifi/iwl-fw-file.h|   2 ++
 drivers/net/wireless/intel/iwlwifi/mvm/fw-api-rx.h  |  26 
 drivers/net/wireless/intel/iwlwifi/mvm/fw-api-sta.h |   9 ---
 drivers/net/wireless/intel/iwlwifi/mvm/fw-api.h |   1 +
 drivers/net/wireless/intel/iwlwifi/mvm/fw-dbg.c | 100 
+--
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c   |  24 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c   |  86 
+---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h|   6 ++---
 drivers/net/wireless/intel/iwlwifi/mvm/ops.c|   3 +++
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c|  37 
++--
 drivers/net/wireless/intel/iwlwifi/mvm/sta.h|   1 +
 drivers/net/wireless/intel/iwlwifi/pcie/trans.c |  29 
+-
 12 files changed, 249 insertions(+), 75 deletions(-)

signature.asc
Description: This is a digitally signed message part

[PATCH v2] iwlwifi: mvm: enable dynamic queue allocation mode

[Luca Coelho]

From: Liad Kaufman 

New firmwares support dynamic queue allocation (DQA), which enables
on-demand allocation of queues per RA/TID, instead of allocating them
statically per vif.  This allows an AP to send, for instance, BE
traffic to STA2 even if it also needs to send traffic to a sleeping
STA1, without being blocked by the sleeping station.

The implementation in the driver is now ready, so we can enable this
feature by default when running firmwares that support it.

Signed-off-by: Liad Kaufman 
[reworded the commit message]
Signed-off-by: Luca Coelho 
---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h 
b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index 726ba48..cde8c6c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -,9 +,8 @@ static inline bool iwl_mvm_is_d0i3_supported(struct 
iwl_mvm *mvm)
 
 static inline bool iwl_mvm_is_dqa_supported(struct iwl_mvm *mvm)
 {
-   /* Make sure DQA isn't allowed in driver until feature is complete */
-   return false && fw_has_capa(>fw->ucode_capa,
-   IWL_UCODE_TLV_CAPA_DQA_SUPPORT);
+   return fw_has_capa(>fw->ucode_capa,
+  IWL_UCODE_TLV_CAPA_DQA_SUPPORT);
 }
 
 static inline bool iwl_mvm_enter_d0i3_on_suspend(struct iwl_mvm *mvm)
-- 
2.9.3

Re: [PATCH 3/7] iwlwifi: mvm: fix d3_test with unified D0/D3 images

[Luca Coelho]

On Wed, 2016-10-26 at 09:59 +0300, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > On Wed, 2016-10-26 at 09:19 +0300, Kalle Valo wrote:
> > > Luca Coelho  writes:
> > > 
> > > > From: Luca Coelho 
> > > > 
> > > > When a unified D0/D3 image is used, we don't restart the FW in the
> > > > D0->D3->D0 transitions.  Therefore, the d3_test functionality should
> > > > not call ieee8021_restart_hw() when the resuming either.
> > > > 
> > > > Fixes: commit 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on 
> > > > suspend")
> > > 
> > > The word "commit" is not needed in the Fixes line, but no need to change
> > > that now.
> > 
> > Oh, I didn't know that.  Thanks for pointing out.  I'll keep it in mind
> > for future patches.
> 
> Related to this Documentation/SubmittingPatches has a handy tip:
> 
> --
> The following ``git config`` settings can be used to add a pretty format for
> outputting the above style in the ``git log`` or ``git show`` commands::
> 
> [core]
> abbrev = 12
> [pretty]
> fixes = Fixes: %h (\"%s\")
> --
> 
> And then you can do just:
> 
> $ git show --format=fixes 23ae61282b88 | head -1
> Fixes: 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on suspend")
> $
> 
> Perfect for a lazy person like me :)

Wow, really perfect for me too! Thanks! I should read SubmittingPatches
more often. :)

--
Luca.

Re: [PATCH 09/10] iwlwifi: mvm: operate in dqa mode

[Valo, Kalle]

Kalle Valo  writes:

>> Since we've been working on this for so long and already use the term
>> DQA broadly, we thought it wouldn't be necessary to explain more when
>> we are finally enabling it by default.  But of course I can change that
>> if you prefer.
>
> I guessed I could find more information from the history and I know this
> is obvious to your team, but it's not obvious to everyone. The commit
> log should always answer the questions "Why?" and this isn't answering
> that. For example, I need this information when sending pull requests to
> Dave and I'm sure lots of other people find it useful as well,
> especially when enabling a new feature.
>
> So I'm not asking for a long essay, something like this would be
> adequate:
>
> "Run DQA flows by default, as long as the FW supports it. It's currently
> supported on 1234, 3456 and 7654, maybe more in the future. DQA improves
> latency when X is used or throughput when Y is disabled. On the downside
> it sometimes slows down throughput when using Z but that's still
> accetable as it's so rarely used."

Oh, I forgot that the acronym should be also spelled out in the commit
log.

-- 
Kalle Valo

Re: [PATCH 09/10] iwlwifi: mvm: operate in dqa mode

[Kalle Valo]

"Coelho, Luciano"  writes:

> Hi Kalle,
>
> On Wed, 2016-10-26 at 09:32 +0300, Kalle Valo wrote:
>> Luca Coelho  writes:
>> 
>> > From: Liad Kaufman 
>> > 
>> > Run DQA flows by default, as long as the FW supports it.
>> > 
>> > Signed-off-by: Liad Kaufman 
>> > Signed-off-by: Luca Coelho 
>> 
>> What's this DQA mode? A short summary would have been nice why it's
>> useful.
>
> DQA is Dynamic Queue Allocation.  We have been working on this for
> quite a while now (since v4.7) and have many patches mentioning that
> are already merged.  The first patch is this:
>
> commit 24afba7690e4 ("iwlwifi: mvm: support bss dynamic alloc/dealloc
> of queues")

Yeah, but that's commited over six month ago.

> Its commit message has some explanation and we also have a DOC section
> in our driver that explains it in further details:
>
> https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/sta.h#n82

And that's too detailed. I was hoping more like an executive summary in
few sentences.

> Since we've been working on this for so long and already use the term
> DQA broadly, we thought it wouldn't be necessary to explain more when
> we are finally enabling it by default.  But of course I can change that
> if you prefer.

I guessed I could find more information from the history and I know this
is obvious to your team, but it's not obvious to everyone. The commit
log should always answer the questions "Why?" and this isn't answering
that. For example, I need this information when sending pull requests to
Dave and I'm sure lots of other people find it useful as well,
especially when enabling a new feature.

So I'm not asking for a long essay, something like this would be
adequate:

"Run DQA flows by default, as long as the FW supports it. It's currently
supported on 1234, 3456 and 7654, maybe more in the future. DQA improves
latency when X is used or throughput when Y is disabled. On the downside
it sometimes slows down throughput when using Z but that's still
accetable as it's so rarely used."

But no need to change anything for this commit, but just keep in mind
for the future.

-- 
Kalle Valo

Re: [PATCH 3/7] iwlwifi: mvm: fix d3_test with unified D0/D3 images

[Kalle Valo]

Luca Coelho  writes:

> On Wed, 2016-10-26 at 09:19 +0300, Kalle Valo wrote:
>> Luca Coelho  writes:
>> 
>> > From: Luca Coelho 
>> > 
>> > When a unified D0/D3 image is used, we don't restart the FW in the
>> > D0->D3->D0 transitions.  Therefore, the d3_test functionality should
>> > not call ieee8021_restart_hw() when the resuming either.
>> > 
>> > Fixes: commit 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on 
>> > suspend")
>> 
>> The word "commit" is not needed in the Fixes line, but no need to change
>> that now.
>
> Oh, I didn't know that.  Thanks for pointing out.  I'll keep it in mind
> for future patches.

Related to this Documentation/SubmittingPatches has a handy tip:

--
The following ``git config`` settings can be used to add a pretty format for
outputting the above style in the ``git log`` or ``git show`` commands::

[core]
abbrev = 12
[pretty]
fixes = Fixes: %h (\"%s\")
--

And then you can do just:

$ git show --format=fixes 23ae61282b88 | head -1
Fixes: 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on suspend")
$

Perfect for a lazy person like me :)

-- 
Kalle Valo

Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap

[Kalle Valo]

Arnd Bergmann  writes:

> A bugfix added a sanity check around the assignment and use of the
> 'is_11d' variable, which looks correct to me, but as the function is
> rather complex already, this confuses the compiler to the point where
> it can no longer figure out if the variable is always initialized
> correctly:
>
> brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
> brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used 
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> This adds an initialization for the newly introduced case in which
> the variable should not really be used, in order to make the warning
> go away.
>
> Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
> Cc: Hante Meuleman 
> Cc: Arend van Spriel 
> Cc: Kalle Valo 
> Signed-off-by: Arnd Bergmann 

Via which tree are you planning to submit this? Should I take it?

-- 
Kalle Valo

Re: [PATCH 1/3] rsi: Fix memory leak in module unload

[Kalle Valo]

Prameela Rani Garnepudi  writes:

> On 10/13/2016 07:56 PM, Kalle Valo wrote: 
>> Kalle Valo  writes: 
>> 
>>> Prameela Rani Garnepudi  writes: 
>>> 
 Observed crash when module is unloaded if CONFIG_RSI_DEBUGSFS is not set. 
 Fix: Debugfs entry removal moved inside CONFIG_RSI_DEBUGSFS flag in 
   function rsi_mac80211_detach() 
 Memory leak found and fixed for below structures in function
> rsi_mac80211_detach() 
 * channel list for each supported band 
 * rsi debugfs info 
 
 Signed-off-by: Prameela Rani Garnepudi  
>>> BTW, I don't see patch 2 in patchwork nor in linux-wireless, but I do 
>>> see it on my inbox. Either it's slow or there was a problem with the 
>>> delivery. 
>> It was just slow, I see it now: 
>> 
>> https://patchwork.kernel.org/patch/9374987/ 
>> 
> Hi, 
> Can you please review this patch. 

wireless-drivers-next is not open yet as I was on vacation last week,
hoping to open it today.

And instead of mailing me please check the status from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#checking_state_of_patches_from_patchwork

If you see lots of patches pending submitted around the same time as
yours it's a pretty good hint that I just haven't been able to go
through my queue:

https://patchwork.kernel.org/project/linux-wireless/list/?state=*=25621

-- 
Kalle Valo

Re: [PATCH 3/7] iwlwifi: mvm: fix d3_test with unified D0/D3 images

[Luca Coelho]

On Wed, 2016-10-26 at 09:19 +0300, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > From: Luca Coelho 
> > 
> > When a unified D0/D3 image is used, we don't restart the FW in the
> > D0->D3->D0 transitions.  Therefore, the d3_test functionality should
> > not call ieee8021_restart_hw() when the resuming either.
> > 
> > Fixes: commit 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on 
> > suspend")
> 
> The word "commit" is not needed in the Fixes line, but no need to change
> that now.

Oh, I didn't know that.  Thanks for pointing out.  I'll keep it in mind
for future patches.

--
Luca.

Re: [PATCH 09/10] iwlwifi: mvm: operate in dqa mode

[Coelho, Luciano]

Hi Kalle,

On Wed, 2016-10-26 at 09:32 +0300, Kalle Valo wrote:
> Luca Coelho  writes:
> 
> > From: Liad Kaufman 
> > 
> > Run DQA flows by default, as long as the FW supports it.
> > 
> > Signed-off-by: Liad Kaufman 
> > Signed-off-by: Luca Coelho 
> 
> What's this DQA mode? A short summary would have been nice why it's
> useful.

DQA is Dynamic Queue Allocation.  We have been working on this for
quite a while now (since v4.7) and have many patches mentioning that
are already merged.  The first patch is this:

commit 24afba7690e4 ("iwlwifi: mvm: support bss dynamic alloc/dealloc
of queues")

Its commit message has some explanation and we also have a DOC section
in our driver that explains it in further details:

https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/sta.h#n82

Since we've been working on this for so long and already use the term
DQA broadly, we thought it wouldn't be necessary to explain more when
we are finally enabling it by default.  But of course I can change that
if you prefer.

--
Cheers,
Luca.

Re: [PATCH 09/10] iwlwifi: mvm: operate in dqa mode

[Kalle Valo]

Luca Coelho  writes:

> From: Liad Kaufman 
>
> Run DQA flows by default, as long as the FW supports it.
>
> Signed-off-by: Liad Kaufman 
> Signed-off-by: Luca Coelho 

What's this DQA mode? A short summary would have been nice why it's
useful.

-- 
Kalle Valo

Re: [PATCH 8/8] mac80211: Claim Fast Initial Link Setup (FILS) support

[Johannes Berg]

On Wed, 2016-10-26 at 01:44 +0300, Jouni Malinen wrote:
> With the previous commits, initial FILS support is now functional in
> mac80211-based drivers for both AP and stations roles.

That's a bit misleading, I guess AP role is handled entirely in
hostapd? You documented the extended feature bit to explicitly mean
station role only :)

johannes

Re: [PATCH 3/7] iwlwifi: mvm: fix d3_test with unified D0/D3 images

[Kalle Valo]

Luca Coelho  writes:

> From: Luca Coelho 
>
> When a unified D0/D3 image is used, we don't restart the FW in the
> D0->D3->D0 transitions.  Therefore, the d3_test functionality should
> not call ieee8021_restart_hw() when the resuming either.
>
> Fixes: commit 23ae61282b88 ("iwlwifi: mvm: Do not switch to D3 image on 
> suspend")

The word "commit" is not needed in the Fixes line, but no need to change
that now.

-- 
Kalle Valo

Re: [PATCH 2/2] mac80211: use inline kernel-doc for struct ieee80211_hw

[Johannes Berg]

On Fri, 2016-10-21 at 15:57 +0300, Jani Nikula wrote:
> It's easier to manage the kernel-doc for the fields when they
> documentation is next to the field.

Ok, actually, this doesn't apply. Perhaps I'll look into scripting this
kind of conversion.

johannes

Re: [PATCH 1/2] mac80211: fix some sphinx warnings

[Johannes Berg]

Applied.

I'll take this into the RCs so we don't have warnings there, and the
other patch into -next.

johannes

[PATCH] cfg80211: process events caused by suspend before suspending

[Johannes Berg]

From: Johannes Berg 

When suspending without WoWLAN, cfg80211 will ask drivers to
disconnect. Even when the driver does this synchronously, and
immediately returns with a notification, cfg80211 schedules
the handling thereof to a workqueue, and may then call back
into the driver when the driver was already suspended/ing.

Fix this by processing all events caused by cfg80211_leave_all()
directly after that function returns. The driver still needs to
do the right thing here and wait for the firmware response, but
that is - at least - true for mwifiex where this occurred.

Reported-by: Amitkumar Karwar 
Tested-by: Amitkumar Karwar 
Signed-off-by: Johannes Berg 
---
 net/wireless/sysfs.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/wireless/sysfs.c b/net/wireless/sysfs.c
index 0082f4b01795..14b3f007826d 100644
--- a/net/wireless/sysfs.c
+++ b/net/wireless/sysfs.c
@@ -104,13 +104,16 @@ static int wiphy_suspend(struct device *dev)
 
rtnl_lock();
if (rdev->wiphy.registered) {
-   if (!rdev->wiphy.wowlan_config)
+   if (!rdev->wiphy.wowlan_config) {
cfg80211_leave_all(rdev);
+   cfg80211_process_rdev_events(rdev);
+   }
if (rdev->ops->suspend)
ret = rdev_suspend(rdev, rdev->wiphy.wowlan_config);
if (ret == 1) {
/* Driver refuse to configure wowlan */
cfg80211_leave_all(rdev);
+   cfg80211_process_rdev_events(rdev);
ret = rdev_suspend(rdev, NULL);
}
}
-- 
2.8.1