[PATCH v2] mwifiex: report wakeup for wowlan

2016-09-27 Thread Rajat Jain
Enable notifying wakeup source to the PM core in case of
a wake on wireless LAN event.

Signed-off-by: Wei-Ning Huang 
Signed-off-by: Rajat Jain 
Tested-by: Wei-Ning Huang 
Reviewed-by: Eric Caruso 
---
v2: Fix the commit log

 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 drivers/net/wireless/marvell/mwifiex/sdio.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561..a5f63e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -89,6 +89,9 @@ static irqreturn_t mwifiex_wake_irq_wifi(int irq, void *priv)
disable_irq_nosync(irq);
}
 
+   /* Notify PM core we are wakeup source */
+   pm_wakeup_event(cfg->dev, 0);
+
return IRQ_HANDLED;
 }
 
@@ -112,6 +115,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
  GFP_KERNEL);
cfg = card->plt_wake_cfg;
if (cfg && card->plt_of_node) {
+   cfg->dev = dev;
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
dev_dbg(dev,
@@ -130,6 +134,10 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
struct sdio_mmc_card *card)
}
}
 
+   ret = device_init_wakeup(dev, true);
+   if (ret)
+   dev_err(dev, "fail to init wakeup for mwifiex");
+
return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h 
b/drivers/net/wireless/marvell/mwifiex/sdio.h
index db837f1..07cdd23 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -155,6 +155,7 @@
 } while (0)
 
 struct mwifiex_plt_wake_cfg {
+   struct device *dev;
int irq_wifi;
bool wake_by_wifi;
 };
-- 
2.8.0.rc3.226.g39d4020



[PATCH] mwifiex: report wakeup for wowlan

2016-09-27 Thread Rajat Jain
Enable notifying wakeup source to the PM core. This allow darkresume to
correctly track wakeup source and mark mwifiex_plt as 'automatic' wakeup
source.

Signed-off-by: Wei-Ning Huang 
Signed-off-by: Rajat Jain 
Tested-by: Wei-Ning Huang 
Reviewed-by: Eric Caruso 
---
 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 drivers/net/wireless/marvell/mwifiex/sdio.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561..a5f63e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -89,6 +89,9 @@ static irqreturn_t mwifiex_wake_irq_wifi(int irq, void *priv)
disable_irq_nosync(irq);
}
 
+   /* Notify PM core we are wakeup source */
+   pm_wakeup_event(cfg->dev, 0);
+
return IRQ_HANDLED;
 }
 
@@ -112,6 +115,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
  GFP_KERNEL);
cfg = card->plt_wake_cfg;
if (cfg && card->plt_of_node) {
+   cfg->dev = dev;
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
dev_dbg(dev,
@@ -130,6 +134,10 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
struct sdio_mmc_card *card)
}
}
 
+   ret = device_init_wakeup(dev, true);
+   if (ret)
+   dev_err(dev, "fail to init wakeup for mwifiex");
+
return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h 
b/drivers/net/wireless/marvell/mwifiex/sdio.h
index db837f1..07cdd23 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -155,6 +155,7 @@
 } while (0)
 
 struct mwifiex_plt_wake_cfg {
+   struct device *dev;
int irq_wifi;
bool wake_by_wifi;
 };
-- 
2.8.0.rc3.226.g39d4020



Re: rfkill guidance

2016-09-27 Thread Jonathan Richardson
+linux-wireless

On 16-09-27 12:25 AM, Johannes Berg wrote:
> Hi,
> 
> You should probably Cc the linux-wireless list, at least :)
> 
> But anyway.
> 
>> The driver enables/disables one or more regulators controlling a
>> bluetooth radio using the rfkill_ops set_block function. It's
>> different in that there is no bluetooth driver in the kernel - it's
>> controlled with some firmware that gets uploaded to a chip. The only
>> kernel usage is to power on/off the regulator(s).
> 
> Interesting, don't you have to talk to the BT somehow? So the BT can't
> ever be used from the kernel?

Not from the kernel, a user space app interacts with the BT chip and
kernel. More below.

> 
> Is rfkill really even the right concept here then? I mean, from the
> kernel's POV it doesn't even really have to know there's BT behind
> that, if there's no relation to BT in the kernel? Then again, the
> *user* might need to know?

> Would you be able to explain the overall system function of this a bit
> more?

Sure, I'll tell you what I can. A user space app handles interaction
between the kernel and BT radio. When the app is started, it enables the
regulator (via the rfkill driver and userspace sysfs interface) to power
up the BT chip. The app sends the firmware via a uart to the chip. The
chip loads the firmware then goes into a lower power mode. When the app
needs to talk to the chip it toggles a gpio (device wake) before sending
messages. If the kernel is in a low power mode and the BT chip receives
a message that requires handling by the app, then a host wake gpio is
driven to wake up the host (kernel).

In addition to the regulator(s) controlled by the rfkill driver I didn't
mention the two gpio's previously. The existing rfkill driver does
nothing with them. It just exports the global gpio # to sysfs which the
app can read to control via the gpio sysfs interface. These could be
hard coded in a startup script or something, they aren't that important
but it does handle the conversion to the global gpio# from 1 of 3 gpio
controllers on a Cygnus SOC. I'm not sure how to handle that, since the
gpio's vary across board variants.

I'm not sure rfkill is the right concept either but the app does use the
rfkill sysfs interface to control the regulators which in turn powers a
BT radio. The driver could not use the rfkill interface and provide its
own but would essentially be doing the same thing as rfkill-regulator.c.

> 
>> The driver is basically a wrapper around controlling regulators.
>> There's already rfkill-regulator.c but it has no DT support and only
>> controls 1 regulator. I was considering modifying it until I saw this
>> thread where Mark objected and it looks like the submitter abandoned
>> the patchset (http://www.serverphorums.com/read.php?12,870025).
> 
> I don't think that's a "killer" thread for this feature really. Yes,
> people have (quite possibly legitimate) concerns about that, but we
> should discuss that.
> 
>> I couldn't see any way to enable/disable regulators from sysfs so it
>> looks like the regulator kernel API needs to be used from some
>> driver.
> 
> Indeed, looks like Documentation/ABI/testing/sysfs-class-regulator is
> all read-only.

There's a regulator driver named userspace-consumer.c that allows
enable/disable from userspace, but it doesn't support DT either.

> 
>> Do you think adding DT support and multiple regulators to the
>> existing driver is the best way to add support for this? Our driver
>> basically duplicates this and I don't think can be upstreamed the way
>> it is.
> 
> Hard to say, really. Like I said, I'm not really _quite_ sure rfkill is
> even the right concept, since you're essentially toggling some
> hardware, but then again it *does* end up being a BT device.

Looking forward to more feedback. I see the options as either rfkill or
some regulator controller that allows enabling/disabling via sysfs.

> 
> It also looks like nobody is actually using rfkill-regulator anywhere
> (in the upstream kernel), so we could just port that entirely to DT,
> which would be nice.
> 
> johannes
> 

Thanks,
Jon



Re:

2016-09-27 Thread Rajat Jain
Please ignore, not sure why this landed without a subject.

On Tue, Sep 27, 2016 at 9:50 AM, Rajat Jain  wrote:
> From: Wei-Ning Huang 
>
> Date: Thu, 17 Mar 2016 11:43:16 +0800
> Subject: [PATCH] mwifiex: report wakeup for wowlan
>
> Enable notifying wakeup source to the PM core. This allow darkresume to
> correctly track wakeup source and mark mwifiex_plt as 'automatic' wakeup
> source.
>
> Signed-off-by: Wei-Ning Huang 
> Signed-off-by: Rajat Jain 
> Tested-by: Wei-Ning Huang 
> Reviewed-by: Eric Caruso 
> ---
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
>  drivers/net/wireless/marvell/mwifiex/sdio.h | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
> b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index d3e1561..a5f63e4 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -89,6 +89,9 @@ static irqreturn_t mwifiex_wake_irq_wifi(int irq, void 
> *priv)
> disable_irq_nosync(irq);
> }
>
> +   /* Notify PM core we are wakeup source */
> +   pm_wakeup_event(cfg->dev, 0);
> +
> return IRQ_HANDLED;
>  }
>
> @@ -112,6 +115,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
> struct sdio_mmc_card *card)
>   GFP_KERNEL);
> cfg = card->plt_wake_cfg;
> if (cfg && card->plt_of_node) {
> +   cfg->dev = dev;
> cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
> if (!cfg->irq_wifi) {
> dev_dbg(dev,
> @@ -130,6 +134,10 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
> struct sdio_mmc_card *card)
> }
> }
>
> +   ret = device_init_wakeup(dev, true);
> +   if (ret)
> +   dev_err(dev, "fail to init wakeup for mwifiex");
> +
> return 0;
>  }
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h 
> b/drivers/net/wireless/marvell/mwifiex/sdio.h
> index db837f1..07cdd23 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.h
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
> @@ -155,6 +155,7 @@
>  } while (0)
>
>  struct mwifiex_plt_wake_cfg {
> +   struct device *dev;
> int irq_wifi;
> bool wake_by_wifi;
>  };
> --
> 2.8.0.rc3.226.g39d4020
>


[no subject]

2016-09-27 Thread Rajat Jain
From: Wei-Ning Huang 

Date: Thu, 17 Mar 2016 11:43:16 +0800
Subject: [PATCH] mwifiex: report wakeup for wowlan

Enable notifying wakeup source to the PM core. This allow darkresume to
correctly track wakeup source and mark mwifiex_plt as 'automatic' wakeup
source.

Signed-off-by: Wei-Ning Huang 
Signed-off-by: Rajat Jain 
Tested-by: Wei-Ning Huang 
Reviewed-by: Eric Caruso 
---
 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 drivers/net/wireless/marvell/mwifiex/sdio.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561..a5f63e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -89,6 +89,9 @@ static irqreturn_t mwifiex_wake_irq_wifi(int irq, void *priv)
disable_irq_nosync(irq);
}
 
+   /* Notify PM core we are wakeup source */
+   pm_wakeup_event(cfg->dev, 0);
+
return IRQ_HANDLED;
 }
 
@@ -112,6 +115,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
  GFP_KERNEL);
cfg = card->plt_wake_cfg;
if (cfg && card->plt_of_node) {
+   cfg->dev = dev;
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
dev_dbg(dev,
@@ -130,6 +134,10 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
struct sdio_mmc_card *card)
}
}
 
+   ret = device_init_wakeup(dev, true);
+   if (ret)
+   dev_err(dev, "fail to init wakeup for mwifiex");
+
return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h 
b/drivers/net/wireless/marvell/mwifiex/sdio.h
index db837f1..07cdd23 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -155,6 +155,7 @@
 } while (0)
 
 struct mwifiex_plt_wake_cfg {
+   struct device *dev;
int irq_wifi;
bool wake_by_wifi;
 };
-- 
2.8.0.rc3.226.g39d4020



[PATCH] mwifiex: report wakeup for wowlan

2016-09-27 Thread Rajat Jain
Enable notifying wakeup source to the PM core. This allow darkresume to
correctly track wakeup source and mark mwifiex_plt as 'automatic' wakeup
source.

Signed-off-by: Wei-Ning Huang 
Signed-off-by: Rajat Jain 
Tested-by: Wei-Ning Huang 
Reviewed-by: Eric Caruso 
---
 drivers/net/wireless/marvell/mwifiex/sdio.c | 8 
 drivers/net/wireless/marvell/mwifiex/sdio.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c 
b/drivers/net/wireless/marvell/mwifiex/sdio.c
index d3e1561..a5f63e4 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.c
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
@@ -89,6 +89,9 @@ static irqreturn_t mwifiex_wake_irq_wifi(int irq, void *priv)
disable_irq_nosync(irq);
}
 
+   /* Notify PM core we are wakeup source */
+   pm_wakeup_event(cfg->dev, 0);
+
return IRQ_HANDLED;
 }
 
@@ -112,6 +115,7 @@ static int mwifiex_sdio_probe_of(struct device *dev, struct 
sdio_mmc_card *card)
  GFP_KERNEL);
cfg = card->plt_wake_cfg;
if (cfg && card->plt_of_node) {
+   cfg->dev = dev;
cfg->irq_wifi = irq_of_parse_and_map(card->plt_of_node, 0);
if (!cfg->irq_wifi) {
dev_dbg(dev,
@@ -130,6 +134,10 @@ static int mwifiex_sdio_probe_of(struct device *dev, 
struct sdio_mmc_card *card)
}
}
 
+   ret = device_init_wakeup(dev, true);
+   if (ret)
+   dev_err(dev, "fail to init wakeup for mwifiex");
+
return 0;
 }
 
diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.h 
b/drivers/net/wireless/marvell/mwifiex/sdio.h
index db837f1..07cdd23 100644
--- a/drivers/net/wireless/marvell/mwifiex/sdio.h
+++ b/drivers/net/wireless/marvell/mwifiex/sdio.h
@@ -155,6 +155,7 @@
 } while (0)
 
 struct mwifiex_plt_wake_cfg {
+   struct device *dev;
int irq_wifi;
bool wake_by_wifi;
 };
-- 
2.8.0.rc3.226.g39d4020



[PATCH] hostap: Mark the Host AP driver obsolete

2016-09-27 Thread Jouni Malinen
This is old code for old hardware and it is not really accurate to claim
this to be maintained anymore. Change the status to "Obsolete" to make
it clearer that minor cleanup and other unnecessary changes from
automated tools is not necessarily beneficial and has larger risk of
breaking something without being quickly noticed due to lack of testing.

In addition, remove the old mailing list that does not work anymore and
point the web-page to a more accurate URL.

Signed-off-by: Jouni Malinen 
---
 MAINTAINERS | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index efd69c76..20de5f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5593,10 +5593,9 @@ F:   
Documentation/devicetree/bindings/scsi/hisilicon-sas.txt
 
 HOST AP DRIVER
 M: Jouni Malinen 
-L: hos...@shmoo.com (subscribers-only)
 L: linux-wireless@vger.kernel.org
-W: http://hostap.epitest.fi/
-S: Maintained
+W: http://w1.fi/hostap-driver.html
+S: Obsolete
 F: drivers/net/wireless/intersil/hostap/
 
 HP COMPAQ TC1100 TABLET WMI EXTRAS DRIVER
-- 
1.9.1


-- 
Jouni MalinenPGP id EFC895FA


Re: brcmfmac: replace WARNING on timeout with a simple error message

2016-09-27 Thread Kalle Valo
Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> Even with timeout increased to 950 ms we get WARNINGs from time to time.
> It mostly happens on A-MPDU stalls (e.g. when station goes out of
> range). It may take up to 5-10 secods for the firmware to recover and
> for that time it doesn't process packets.
> 
> It's still useful to have a message on time out as it may indicate some
> firmware problem and incorrect key update. Raising a WARNING however
> wasn't really that necessary, it doesn't point to any driver bug anymore
> and backtrace wasn't much useful.
> 
> Signed-off-by: Rafał Miłecki 
> Acked-by: Arend van Spriel 

Patch applied to wireless-drivers-next.git, thanks.

2f0e56fa37cc brcmfmac: replace WARNING on timeout with a simple error message

-- 
https://patchwork.kernel.org/patch/9351657/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: [1/2] rtlwifi: Add HAL_DEF_WOWLAN case to *_get_hw() routines

2016-09-27 Thread Kalle Valo
Larry Finger  wrote:
> Only rtl8821ae implements WOWLAN; however, the other drivers may receive
> a call requesting information about this mode. The other drivers need to
> ignore the request rather than logging that the default branch of the
> switch statement has been reached.
> 
> Reported by: Jean Delvare 
> Signed-off-by: Larry Finger 

2 patches applied to wireless-drivers-next.git, thanks.

1cc49a5b5466 rtlwifi: Add HAL_DEF_WOWLAN case to *_get_hw() routines
8334ffdc8290 rtlwifi: Add explicit values to hw_variables enum

-- 
https://patchwork.kernel.org/patch/9349217/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: rtlwifi: Add switch variable to 'switch case not processed' messages

2016-09-27 Thread Kalle Valo
Joe Perches  wrote:
> Help along debugging by showing what switch/case variable is not
> being processed in these messages.
> 
> Signed-off-by: Joe Perches 
> Acked-by: Larry Finger 

Patch applied to wireless-drivers-next.git, thanks.

ad5748893b27 rtlwifi: Add switch variable to 'switch case not processed' 
messages

-- 
https://patchwork.kernel.org/patch/9348573/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: pull-request: iwlwifi-next 2016-09-26

2016-09-27 Thread Kalle Valo
Luca Coelho  writes:

> Hi Kalle,
>
> One more small pull-request intended for 4.9.  It seems that I'm
> actually catching up with our internal tree! :) Nothing major here,
> mostly clean-ups and small bug fixes and improvements.
>
> Let me know if everything's fine (or not). :)
>
> Luca.
>
>
> The following changes since commit 0979a913f879ea39504200d83fb9f275a555a58d:
>
>   iwlwifi: pcie: use LIST_HEAD() macro (2016-09-19 11:29:35 +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-09-26

Pulled, thanks.

-- 
Kalle Valo


RE: [PATCH v8] cfg80211: Provision to allow the support for different beacon intervals

2016-09-27 Thread Undekari, Sunil Dutt
Hi Johannes,

>> 2. Since "diff_beacon_int_gcd_min"  is only specified / advertised in 
>> the interface combinations , our logic was to get the minimal 
>> "diff_beacon_int_gcd_min" of all the matching combinations and later
>> compare with the new beacon interval. (API 
>> "cfg80211_iter_combinations" has to be invoked to get the 
>> corresponding "diff_beacon_int_gcd_min")

>Yeah, but you get the diff_beacon_int_gcd_min for all combinations that allow 
>the *current* combination of interfaces, not the combinations that allow the 
>new *new* combination of interfaces, no?

>I mean, consider the case that you have a single AP interface, with beacon 
>interval 300, and you're adding another AP, with beacon interval 150, and the 
>following allowed combinations:

 >* ap = 1
 > mesh = 1
 >  diff_beacon_int_gcd_min = 100
 > * ap = 2
>  diff_beacon_int_gcd_min = 50

> Wouldn't you prevent that, or something?

> Or say you have

> * ap = 1
>   mesh = 1
>   diff_beacon_int_gcd_min = 100
>  * ap = 2
>   diff_beacon_int_gcd_min = 200

> Probably doesn't really make sense, but now if you have an existing AP 
> interface, you would think the min is 100, when really while adding another 
> AP it should be 200.

In PATCH v8 , cfg80211_validate_beacon_int -> cfg80211_iter_combinations 
carries the argument iftype_num , which also considers the "new interface" that 
is getting added. 
Thus , in the example you have quoted above , the iftype_num shall represent 2 
( AP + AP ) , and thus the min_gcd obtained out of cfg80211_iter_combinations 
(cfg80211_get_beacon_int_min_gcd) shall be 50 for the example 1 and 200 for the 
example 2 . 
Thus , considering the two examples mentioned above , the second AP should 
succeed for "example 1" vs failure for "example 2" with patch V8 , isn't ?  

>> If I understand this correctly , are you saying that this new argument 
>> to cfg80211_iter_combinations shall be the GCD of all the existing 
>> beacon intervals and would be used to match with the corresponding 
>> "diff_beacon_int_gcd_min" of the interface combinations in 
>> "cfg80211_iter_combinations".

>Yes, that's what I was thinking.

>> If yes , this GCD argument does not represent , if the beacon 
>> intervals of all the existing interfaces differ or not , isn’t ? If 
>> the "diff_beacon_int_gcd_min"  of the all the matching interface 
>> combinations is 0 , such differed beacon intervals would pass the 
>> check , contradicting the existing logic ( not allowing the differed 
>> beacon intervals), isn't ?

>Oh, well, ok - if all existing and new beacon intervals are the same we'd have 
>to do the lookup without the existing beacon interval GCD and still compare to 
>the min separately.

Let me reiterate our problem here with this approach.

The current behavior of the kernel is to not allow the configuration of 
different beacon intervals and our expectation is that this new patch should be 
backward compatible with the existing behavior.
Now , if we go with this approach of "introducing a new argument to 
cfg80211_iter_combinations which shall be the GCD of all the existing 
combinations to check against the respective "diff_beacon_int_gcd_min"" ,  
consider the case ( same one which is mentioned above ) that we have a single 
AP interface ( beacon interval = 300 ) , and a new AP is getting added ( beacon 
interval = 150 ),  with the following allowed combinations:

1) * ap = 2
diff_beacon_int_gcd_min = 0 ( rather not specified ) 
2)  * ap = 2
  diff_beacon_int_gcd_min = 100

The GCD calculated is 150 . cfg80211_iter_combinations shall return success for 
both the scenarios ( 1 and 2 ) (The intention here is to compare with only the 
nonzero " diff_beacon_int_gcd_min" )

This success from "cfg80211_iter_combinations" does not represent if the GCD 
passed is compared against a 0 / non zero "diff_beacon_int_gcd_min" , isn't ?

Thus , we are trying to solve this problem , by getting the 
"diff_beacon_int_gcd_min" for the respective interface combination , before 
comparing it with the calculated GCD. 

Please correct us.

Regards,
Sunil

 



Re: staging: wilc1000: kernel Oops while opening the device

2016-09-27 Thread Greg KH
On Mon, Sep 26, 2016 at 05:17:37AM +, aditya.shan...@microchip.com wrote:
> On Friday 23 September 2016 08:37 PM, Greg KH wrote:
> > On Fri, Sep 23, 2016 at 01:17:08PM +, aditya.shan...@microchip.com 
> > wrote:
> >> On Thursday 22 September 2016 06:04 PM, Ganesh Krishna - I00112 wrote:
>  -Original Message-
>  From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] 
>  Sent: Monday, September 19, 2016 7:12 PM
>  To: Nicolas Ferre
>  Cc: linux-wireless@vger.kernel.org; Aditya Shankar - I16078; Ganesh 
>  Krishna - I00112; lui...@osg.samsung.com
>  Subject: Re: staging: wilc1000: kernel Oops while opening the device
> 
>  On Mon, Sep 19, 2016 at 02:06:02PM +0200, Nicolas Ferre wrote:
> > Hi all,
> >
> > While using the wilc1000 driver with latest 4.8-rc7, I have 
> > difficulties to open the device and actually use it as I have this 
> > kernel Oops right after the loading of the firmware 
> > (wilc1003_firmware.bin).
> >
> > If I revert back the driver to its 
> > b9811891a9f60ca9c314dcab3244c65930c4cf37 state, it works okay. I did 
> > this because I tend to think that it might be related to the latest 
> > move on this driver to "completion" or "work queues".
> > It seems to be a regression from 4.7.
> 
>  Ick, not good at all.
> 
>  Any chance you can run 'git bisect' to see if you can find the offending 
>  patch?
> 
>  I thought the maintainers were testing this driver over time :(
> >>>
>  thanks,
> 
>  greg k-h
> >>>
> >>> Ick indeed. Maintainers are on it full time. Thank you very much for the 
> >>> flag. While I am only reporting activity, hope to have results soon.
> >>> Regards,
> >>> ganesh k 
> >>>
> >> Hi All,
> >>
> >> bisecting changes on the wilc1000 host driver, I find that this problem 
> >> seems to occur after the below change. 
> >> As of now, I only know the where and not the why. So I will continue to 
> >> figure out why this happens.
> >>
> >> commit 2518ac59eb27ed003c5a97e8f9588adafdfe4a8a
> >> Author: Binoy Jayan 
> >> Date:   Thu Jun 23 11:11:51 2016 +0530
> >>
> >> staging: wilc1000: Replace kthread with workqueue for host interface
> >> 
> >> Deconstruct the kthread / message_queue logic, replacing it with
> >> create_singlethread_workqueue() / queue_work() setup, by adding a
> >> 'struct work_struct' to 'struct host_if_msg'. The current kthread
> >> hostIFthread() is converted to a work queue helper with the name
> >> 'host_if_work'.
> > 
> > So if you revert this, it works?
> > 
> > If so, let us know and I will gladly revert it, as breaking things is
> > not ok.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> Unfortunately no. We will have to revert this commit and the one submitted 
> after this. 
> The commit a5c84b2 after 2518ac5 is heavily reliant on its parent 2518ac5.
> 
> Note that reverting 2518ac5 throws up a couple of minor merge conflicts which 
> can be
> resolved by dropping changes in HEAD and retaining the parent commit of 
> 2518ac5.
> With this, the crash is not seen anymore.

Can you send me a patch, or series of patches, that does all of this so
I know I get it correct?

thanks,

greg k-h


Re: [v2] RANDOM: ATH9K RNG delivers zero bits of entropy

2016-09-27 Thread Stephan Mueller
Am Dienstag, 27. September 2016, 16:44:16 CEST schrieb Kalle Valo:

Hi Kalle,

> Stephan Mueller  wrote:
> > The ATH9K driver implements an RNG which is completely bypassing the
> > standard Linux HW generator logic.
> > 
> > The RNG may or may not deliver entropy. Considering the conservative
> > approach in treating entropy with respect to non-auditable sources, this
> > patch changes the delivered entropy value to zero. The RNG still feeds
> > data into the input_pool but it is assumed to have no entropy.
> > 
> > When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> > the entropy estimation considering that a user can change that value at
> > boot and runtime.
> > 
> > Reviewed-by: Jason Cooper 
> > Signed-off-by: Stephan Mueller 
> 
> Based on the discussion I'm dropping this patch. But the discussion was
> hard to follow so please let me know if I misunderstood.

I guess the rejection is appropriate, but something needs to be done: 
add_hwgenerator_randomness should not be used in this scenario.
> 
> Patch set to Rejected.



Ciao
Stephan


Re: [1/2] ath9k: change entropy formula for easier understanding

2016-09-27 Thread Kalle Valo
miaoqing pan  wrote:
> From: Miaoqing Pan 
> 
> The quality of ADC entropy is 10 bits of min-entropy for
> a 32-bit value, change '(((x) * 8 * 320) >> 10)' to
> '(((x) * 8 * 10) >> 5)' for easier understanding.
> 
> Signed-off-by: Miaoqing Pan 

I need some help here, it this patch ok to take or should I drop it? The
discussion is available from the patchwork link below if someone needs to
refresh the memory.

-- 
https://patchwork.kernel.org/patch/9270463/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: wil6200

2016-09-27 Thread Lior David
On 9/26/2016 9:05 PM, Vikram wrote:
> Hi Lior,
> 
> On 26 September 2016 at 18:19, Lior David  wrote:
>> On 9/25/2016 6:15 PM, Vikram wrote:
>>> Hi,
>>>
>>> Could you please let me know as to how to change the mode of wil6200
>>> chip from WBE to WiFi?
>>>
>>> Regards,
>>> Vikram
>>>
>> What is the exact device you are using? Older devices have firmware that only
>> supports WBE mode.
>>
>> Thanks,
>> Lior
> 
> I'm using Dell Wireless DW1601 7Gbps WiGig 802.11ad 802.11n 2*2 WiFi
> Half Mini Card abgn+ad 2*2:2
Unfortunately it looks like this is an old chip which does not support WIFI
mode. There are 2 main problems:
1. The chip has its firmware burned into flash and cannot be upgraded, unlike
newer chips where firmware can be loaded into memory (so wil6210 driver can use
the firmware file from /lib/firmware).
2. Based on the product description this card was specifically used to connect
to the Dell wireless dock which used WBE, so it has a firmware with only WBE
support.

Thanks,
Lior

> 
> Regards,
> Vikram
> 


Re: [v2] RANDOM: ATH9K RNG delivers zero bits of entropy

2016-09-27 Thread Kalle Valo
Stephan Mueller  wrote:
> The ATH9K driver implements an RNG which is completely bypassing the
> standard Linux HW generator logic.
> 
> The RNG may or may not deliver entropy. Considering the conservative
> approach in treating entropy with respect to non-auditable sources, this
> patch changes the delivered entropy value to zero. The RNG still feeds
> data into the input_pool but it is assumed to have no entropy.
> 
> When the ATH9K RNG changes to use the HW RNG framework, it may re-enable
> the entropy estimation considering that a user can change that value at
> boot and runtime.
> 
> Reviewed-by: Jason Cooper 
> Signed-off-by: Stephan Mueller 

Based on the discussion I'm dropping this patch. But the discussion was
hard to follow so please let me know if I misunderstood.

Patch set to Rejected.

-- 
https://patchwork.kernel.org/patch/9266265/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: [PATCH v4] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

2016-09-27 Thread Shajakhan, Mohammed Shafi (Mohammed Shafi)
apologies please ignore this , wrong patch due to confusing v4 tag
 
please review,

[PATCH] ath10k: Cleanup calling ath10k_htt_rx_h_unchain


From: Shajakhan, Mohammed Shafi (Mohammed Shafi)
Sent: 27 September 2016 18:31
To: ath...@lists.infradead.org
Cc: moham...@codeaurora.org; linux-wireless@vger.kernel.org; Shajakhan, 
Mohammed Shafi (Mohammed Shafi)
Subject: [PATCH v4] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

From: Mohammed Shafi Shajakhan 

'ath10k_htt_rx_h_unchain' is need to be called only if the return
value from 'ath10k_htt_rx_amsdu_pop' is 1('chained msdu's'), this
change makes it more explicit and avoids doing a skb_peek, fetching
rx descriptor pointer, checking rx msdu decap format for the case of
ret = 0 (unchained msdus). Found this change during code walk through,
not sure if this addresses any issue.

Signed-off-by: Mohammed Shafi Shajakhan 
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7ae9349..e51dace 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1459,8 +1459,7 @@ static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
 }

 static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
-   struct sk_buff_head *amsdu,
-   bool chained)
+   struct sk_buff_head *amsdu)
 {
struct sk_buff *first;
struct htt_rx_desc *rxd;
@@ -1471,9 +1470,6 @@ static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
decap = MS(__le32_to_cpu(rxd->msdu_start.common.info1),
   RX_MSDU_START_INFO1_DECAP_FORMAT);

-   if (!chained)
-   return;
-
/* FIXME: Current unchaining logic can only handle simple case of raw
 * msdu chaining. If decapping is other than raw the chaining may be
 * more complex and this isn't handled by the current code. Don't even
@@ -1550,7 +1546,11 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt 
*htt)
}

ath10k_htt_rx_h_ppdu(ar, , rx_status, 0x);
-   ath10k_htt_rx_h_unchain(ar, , ret > 0);
+
+   /* only for ret = 1 indicates chained msdus */
+   if (ret > 0)
+   ath10k_htt_rx_h_unchain(ar, );
+
ath10k_htt_rx_h_filter(ar, , rx_status);
ath10k_htt_rx_h_mpdu(ar, , rx_status);
ath10k_htt_rx_h_deliver(ar, , rx_status);
--
1.9.1



[PATCH] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

2016-09-27 Thread Mohammed Shafi Shajakhan
From: Mohammed Shafi Shajakhan 

'ath10k_htt_rx_h_unchain' is need to be called only if the return
value from 'ath10k_htt_rx_amsdu_pop' is 1('chained msdu's'), this
change makes it more explicit and avoids doing a skb_peek, fetching
rx descriptor pointer, checking rx msdu decap format for the case of
ret = 0 (unchained msdus). Found this change during code walk through,
not sure if this addresses any issue.

Signed-off-by: Mohammed Shafi Shajakhan 
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7ae9349..e51dace 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1459,8 +1459,7 @@ static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
 }
 
 static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
-   struct sk_buff_head *amsdu,
-   bool chained)
+   struct sk_buff_head *amsdu)
 {
struct sk_buff *first;
struct htt_rx_desc *rxd;
@@ -1471,9 +1470,6 @@ static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
decap = MS(__le32_to_cpu(rxd->msdu_start.common.info1),
   RX_MSDU_START_INFO1_DECAP_FORMAT);
 
-   if (!chained)
-   return;
-
/* FIXME: Current unchaining logic can only handle simple case of raw
 * msdu chaining. If decapping is other than raw the chaining may be
 * more complex and this isn't handled by the current code. Don't even
@@ -1550,7 +1546,11 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt 
*htt)
}
 
ath10k_htt_rx_h_ppdu(ar, , rx_status, 0x);
-   ath10k_htt_rx_h_unchain(ar, , ret > 0);
+
+   /* only for ret = 1 indicates chained msdus */
+   if (ret > 0)
+   ath10k_htt_rx_h_unchain(ar, );
+
ath10k_htt_rx_h_filter(ar, , rx_status);
ath10k_htt_rx_h_mpdu(ar, , rx_status);
ath10k_htt_rx_h_deliver(ar, , rx_status);
-- 
1.9.1



Re: [PATCH v8] cfg80211: Provision to allow the support for different beacon intervals

2016-09-27 Thread Johannes Berg
Hi,

Sorry for the long delay.

> > In order to validate a new beacon interval, you're first looking up
> > the min GCD value of all the combinations that allow the *current*
> > scenario, but doing that matching without the right # of channels
> > or radar detect parameters? And then you're trying to match that to
> > the new beacon interval?

> Yes . Please allow us to explain the rationale for doing so. 

> 1. The intention here is to ensure that the beacon interval
> configured for any single interface is greater than the
> "diff_beacon_int_gcd_min"  specified in the respective
> combinations.  

Sure, ok, so far so good - not that I think it would really be
necessary to validate it this way, but ok.

> 2. Since "diff_beacon_int_gcd_min"  is only specified / advertised in
> the interface combinations , our logic was to get the minimal
> "diff_beacon_int_gcd_min" of all the matching combinations and later 
> compare with the new beacon interval. (API
> "cfg80211_iter_combinations" has to be invoked to get the
> corresponding "diff_beacon_int_gcd_min")

Yeah, but you get the diff_beacon_int_gcd_min for all combinations that
allow the *current* combination of interfaces, not the combinations
that allow the new *new* combination of interfaces, no?

I mean, consider the case that you have a single AP interface, with
beacon interval 300, and you're adding another AP, with beacon interval
150, and the following allowed combinations:

 * ap = 1
   mesh = 1
   diff_beacon_int_gcd_min = 100
 * ap = 2
   diff_beacon_int_gcd_min = 50

Wouldn't you prevent that, or something?

Or say you have

 * ap = 1
   mesh = 1
   diff_beacon_int_gcd_min = 100
 * ap = 2
   diff_beacon_int_gcd_min = 200

Probably doesn't really make sense, but now if you have an existing AP
interface, you would think the min is 100, when really while adding
another AP it should be 200.

> 3. If the beacon interval configured needs to be ensured to be
> greater than the "diff_beacon_int_gcd_min" configured for both the
> "single interface" and "interface combination" , we have resorted to 
>  "2"  (get the minimal of "diff_beacon_int_gcd_min" among all the
> matched interface combinations and then compare it with the
> configured beacon interval). 

Yeah, I think you're just matching the interface combinations wrong; I
think it needs to take into account the new interfaces and the existing
beacon intervals.

> > If the driver specified diff_beacon_int_gcd_min, then don't do
> > anything in cfg80211_validate_beacon_int(), other than perhaps a
> > minimal range check against the minimum of all
> > >diff_beacon_int_gcd_min values for all combinations.
> > That new argument could be made the GCD of all existing beaconing
> > interfaces (or 0 if no such exists), since that's sufficient for
> > checking against a new min_gcd.

> If I understand this correctly , are you saying that this new
> argument to cfg80211_iter_combinations shall be the GCD of all the
> existing beacon intervals and would be used to match with the
> corresponding "diff_beacon_int_gcd_min" of the interface combinations
> in "cfg80211_iter_combinations". 

Yes, that's what I was thinking.

> If yes , this GCD argument does not represent , if the beacon
> intervals of all the existing interfaces differ or not , isn’t ? If
> the "diff_beacon_int_gcd_min"  of the all the matching interface
> combinations is 0 , such differed beacon intervals would pass the
> check , contradicting the existing logic ( not allowing the differed
> beacon intervals), isn't ? 

Oh, well, ok - if all existing and new beacon intervals are the same
we'd have to do the lookup without the existing beacon interval GCD and
still compare to the min separately.

> Thus, wouldn't it be a better option to first get the
> "diff_beacon_int_gcd_min" advertised by the respective interface
> combinations and then later compare it with the configured /
> calculated beacon interval's GCD . 

As above, I think you might find a combination that no longer applies
after the new interface is added, thus causing a situation that isn't
actually covered by the allowed combinations.

> Also , not quite sure how would your comment " matching without the
> right # of channels or radar detect parameters" get addressed with
> your new proposal ( adding a new argument to
> "cfg80211_iter_combinations" ) 

That's just addressed by not doing the "get min min_gcd" first step,
where you used it wrong, afaict.

johannes


[PATCH v4] ath10k: Cleanup calling ath10k_htt_rx_h_unchain

2016-09-27 Thread Mohammed Shafi Shajakhan
From: Mohammed Shafi Shajakhan 

'ath10k_htt_rx_h_unchain' is need to be called only if the return
value from 'ath10k_htt_rx_amsdu_pop' is 1('chained msdu's'), this
change makes it more explicit and avoids doing a skb_peek, fetching
rx descriptor pointer, checking rx msdu decap format for the case of
ret = 0 (unchained msdus). Found this change during code walk through,
not sure if this addresses any issue.

Signed-off-by: Mohammed Shafi Shajakhan 
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 7ae9349..e51dace 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1459,8 +1459,7 @@ static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
 }
 
 static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
-   struct sk_buff_head *amsdu,
-   bool chained)
+   struct sk_buff_head *amsdu)
 {
struct sk_buff *first;
struct htt_rx_desc *rxd;
@@ -1471,9 +1470,6 @@ static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
decap = MS(__le32_to_cpu(rxd->msdu_start.common.info1),
   RX_MSDU_START_INFO1_DECAP_FORMAT);
 
-   if (!chained)
-   return;
-
/* FIXME: Current unchaining logic can only handle simple case of raw
 * msdu chaining. If decapping is other than raw the chaining may be
 * more complex and this isn't handled by the current code. Don't even
@@ -1550,7 +1546,11 @@ static int ath10k_htt_rx_handle_amsdu(struct ath10k_htt 
*htt)
}
 
ath10k_htt_rx_h_ppdu(ar, , rx_status, 0x);
-   ath10k_htt_rx_h_unchain(ar, , ret > 0);
+
+   /* only for ret = 1 indicates chained msdus */
+   if (ret > 0)
+   ath10k_htt_rx_h_unchain(ar, );
+
ath10k_htt_rx_h_filter(ar, , rx_status);
ath10k_htt_rx_h_mpdu(ar, , rx_status);
ath10k_htt_rx_h_deliver(ar, , rx_status);
-- 
1.9.1



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

2016-09-27 Thread Johannes Berg
>  #define WLAN_CIPHER_SUITE_SMS4   0x00147201
> +#define WLAN_CIPHER_SUITE_PMK   0x00147202
> +#define WLAN_CIPHER_SUITE_PMK_R00x00147203
> +#define WLAN_CIPHER_SUITE_PMK_R0_NAME   0x00147204

Err, what? No, things can't work that way. This is the Chinese
company's OUI, you can't just assign it to PMK stuff.

> + * @NL80211_ATTR_AUTHORIZED: flag attribute, if set indicates that the
> + *  connection is authorized.
> + *
>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -2267,6 +2270,8 @@ enum nl80211_attrs {
>  
>   NL80211_ATTR_MESH_PEER_AID,
>  
> + NL80211_ATTR_AUTHORIZED,

This already exists, no?

NL80211_STA_FLAG_AUTHORIZED should be more or less equivalent, if you
do it per station (or just for the AP in case of managed connection)

>   /* add attributes here, update the policy in nl80211.c */
>  
>   __NL80211_ATTR_AFTER_LAST,
> @@ -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,

I don't think we should conflate the (P)MK and *TK concepts in nl80211,
they're both keys, but they're completely separate in terms of expected
usage.


Ilan and I looked at this, considering 4-way-HS offload after 1X
authentication, and think that the more natural API would be to add all
the necessary data to the PMKSA cache entry. Thus, a PMKSA cache entry
for a device that does 4-way-handshake offloading would include the PMK
(or perhaps MSK?), and for FT it would also including the PMK-R0,
PMKR0Name (and possibly the MDID, or can it be derived?)


However, I'm wondering what exactly the offloads here do. Jouni, could
you also chime in with the QCA (vendor command) design?

In particular, with key management offloaded, it's not clear to me what
exactly the roles of the device and host are here. I'm considering that
the device would handle the 4-way and 2-way handshakes, but then you
wouldn't need the KEK/KCK/ReplayCounter in the host, so there wouldn't
be much point in giving them to it.
But if the device doesn't do that, what exactly *does* it do?


Thanks,
johannes


Re: [v2,02/21] ath10k: fix typo in logging message

2016-09-27 Thread Kalle Valo
Ben Greear  wrote:
> From: Ben Greear 
> 
> Signed-off-by: Ben Greear 

3 patches applied to ath-next branch of ath.git, thanks.

aa66ba0c31c6 ath10k: fix typo in logging message
15138fdf327d ath10k: document cycle count related counters
30d2049b3277 ath10k: support up to 64 vdevs

-- 
https://patchwork.kernel.org/patch/9073591/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: ath10k: Fix rfc1042 header retrieval in QCA4019 with eth decap mode

2016-09-27 Thread Kalle Valo
Vasanthakumar Thiagarajan  wrote:
> Chipset from QCA99X0 onwards (QCA99X0, QCA9984, QCA4019 & future)
> rx_hdr_status is not padded to align in 4-byte boundary. Define a
> new hw_params field to handle different alignment behaviour between
> different hw. This patch fixes improper retrieval of rfc1042 header
> with QCA4019. This patch along with "ath10k: Properly remove padding
> from the start of rx payload" will fix traffic failure in ethernet
> decap mode for QCA4019.
> 
> Signed-off-by: Vasanthakumar Thiagarajan 

Patch applied to ath-next branch of ath.git, thanks.

2f38c3c01de9 ath10k: fix rfc1042 header retrieval in QCA4019 with eth decap mode

-- 
https://patchwork.kernel.org/patch/9249751/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: [1/3] ath10k: use devm_clk_get() instead of clk_get()

2016-09-27 Thread Kalle Valo
Masahiro Yamada  wrote:
> Use the managed variant of clk_get() to simplify the failure path
> and the .remove callback.
> 
> Signed-off-by: Masahiro Yamada 

3 patches applied to ath-next branch of ath.git, thanks.

828662753d60 ath10k: use devm_clk_get() instead of clk_get()
c5d8a34675d9 ath10k: use devm_reset_control_get() instead of reset_control_get()
65901a9e7058 ath10k: do not check if reset is NULL

-- 
https://patchwork.kernel.org/patch/9316579/

Documentation about submitting wireless patches and checking status
from patchwork:

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



[PATCH V2 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Rafał Miłecki
From: Rafał Miłecki 

Flowrings contain skbs waiting for transmission that were passed to us
by netif. It means we checked every one of them looking for 802.1x
Ethernet type. When deleting flowring we have to use freeing function
that will check for 802.1x type as well.

Freeing skbs without a proper check was leading to counter not being
properly decreased. This was triggering a WARNING every time
brcmf_netdev_wait_pend8021x was called.

Signed-off-by: Rafał Miłecki 
Acked-by: Arend van Spriel 
Cc: sta...@vger.kernel.org # 4.5+
---
V2: Add Cc for stable 4.5+. It doesn't apply cleanly to 4.4 and is not
possible for 4.3- due to missing brcmf_get_ifp.
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
index b16b367..d0b738d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
@@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring 
*flow, u16 flowid,
 
 void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
 {
+   struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
struct brcmf_flowring_ring *ring;
+   struct brcmf_if *ifp;
u16 hash_idx;
+   u8 ifidx;
struct sk_buff *skb;
 
ring = flow->rings[flowid];
if (!ring)
return;
+
+   ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+   ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
+
brcmf_flowring_block(flow, flowid, false);
hash_idx = ring->hash_id;
flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;
@@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 
flowid)
 
skb = skb_dequeue(>skblist);
while (skb) {
-   brcmu_pkt_buf_free_skb(skb);
+   brcmf_txfinalize(ifp, skb, false);
skb = skb_dequeue(>skblist);
}
 
-- 
2.9.3



Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Kalle Valo
Rafał Miłecki  writes:

> Kalle: this isn't important enough for 4.8 as it's too late for that.
>
> I'd like to get it for 4.9 however, as this fixes bug that could lead
> to WARNING on every add_key/del_key call. We was struggling with these
> WARNINGs for some time and this fixes one of two problems causing them.
>>>
>>> Ok, I'll queue this for 4.9.
>>>
 Please mark it for stable as well.
>>>
>>> I can add that. Any ideas how old releases stable releases should this
>>> go to?
>>
>> I was analyzing this.
>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>> Increase nr of supported flowrings.")
>> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>>
>> That said I suggest 4.5+. Any objections?
>
> Let me see if patchwork with pick Cc tag as it does for others.
>
> Cc: sta...@vger.kernel.org # 4.5+

An excellent idea but no luck:

Signed-off-by: Rafa? Mi?ecki 
Acked-by: Arend van Spriel 

I'll add this to my patchwork wishlist though, I think it would be a
really useful feature to have.

(The question marks are because of my buggy copy paste, ignore those)

-- 
Kalle Valo


Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Rafał Miłecki
On 27 September 2016 at 14:04, Arend Van Spriel
 wrote:
> On 27-9-2016 13:58, Rafał Miłecki wrote:
>> On 27 September 2016 at 13:44, Rafał Miłecki  wrote:
>>> On 27 September 2016 at 13:27, Kalle Valo  wrote:
 Arend Van Spriel  writes:

> On 27-9-2016 11:14, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>>
>> Flowrings contain skbs waiting for transmission that were passed to us
>> by netif. It means we checked every one of them looking for 802.1x
>> Ethernet type. When deleting flowring we have to use freeing function
>> that will check for 802.1x type as well.
>>
>> Freeing skbs without a proper check was leading to counter not being
>> properly decreased. This was triggering a WARNING every time
>> brcmf_netdev_wait_pend8021x was called.
>
> Acked-by: Arend van Spriel 
>> Signed-off-by: Rafał Miłecki 
>> ---
>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>
>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>> to WARNING on every add_key/del_key call. We was struggling with these
>> WARNINGs for some time and this fixes one of two problems causing them.

 Ok, I'll queue this for 4.9.

> Please mark it for stable as well.

 I can add that. Any ideas how old releases stable releases should this
 go to?
>>>
>>> I was analyzing this.
>>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>>> Increase nr of supported flowrings.")
>>> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>>>
>>> That said I suggest 4.5+. Any objections?
>
> No objections. Just a tip. I tend to look at kernel.org main page to see
> the stable and long-term kernel listed. So 4.7+ and 4.5+ have same
> meaning as 4.5 and 4.6 are not stable/long-term kernels.

Some projects may work on their own stable kernels, e.g. Ubuntu, see:
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

That's why I don't always look strictly at upstream stable releases only.

-- 
Rafał


Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 13:58, Rafał Miłecki wrote:
> On 27 September 2016 at 13:44, Rafał Miłecki  wrote:
>> On 27 September 2016 at 13:27, Kalle Valo  wrote:
>>> Arend Van Spriel  writes:
>>>
 On 27-9-2016 11:14, Rafał Miłecki wrote:
> From: Rafał Miłecki 
>
> Flowrings contain skbs waiting for transmission that were passed to us
> by netif. It means we checked every one of them looking for 802.1x
> Ethernet type. When deleting flowring we have to use freeing function
> that will check for 802.1x type as well.
>
> Freeing skbs without a proper check was leading to counter not being
> properly decreased. This was triggering a WARNING every time
> brcmf_netdev_wait_pend8021x was called.

 Acked-by: Arend van Spriel 
> Signed-off-by: Rafał Miłecki 
> ---
> Kalle: this isn't important enough for 4.8 as it's too late for that.
>
> I'd like to get it for 4.9 however, as this fixes bug that could lead
> to WARNING on every add_key/del_key call. We was struggling with these
> WARNINGs for some time and this fixes one of two problems causing them.
>>>
>>> Ok, I'll queue this for 4.9.
>>>
 Please mark it for stable as well.
>>>
>>> I can add that. Any ideas how old releases stable releases should this
>>> go to?
>>
>> I was analyzing this.
>> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
>> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
>> Increase nr of supported flowrings.")
>> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>>
>> That said I suggest 4.5+. Any objections?

No objections. Just a tip. I tend to look at kernel.org main page to see
the stable and long-term kernel listed. So 4.7+ and 4.5+ have same
meaning as 4.5 and 4.6 are not stable/long-term kernels.

Regards,
Arend

> Let me see if patchwork with pick Cc tag as it does for others.
> 
> Cc: sta...@vger.kernel.org # 4.5+
> 
> This may be worth backporting to 4.4 as well (as it's longterm), but
> I'll do it separately due to patch not applying cleanly.


Re: ath10k: Spelling and miscellaneous neatening

2016-09-27 Thread Kalle Valo
Joe Perches  wrote:
> Correct some trivial comment typos.
> Remove unnecessary parentheses in a long line.
> Convert a return; before the end of a void function definition to just ;
> 
> Signed-off-by: Joe Perches 
> Reviewed-by: Julian Calaby 

Patch applied to ath-next branch of ath.git, thanks.

e13dbead976d ath10k: spelling and miscellaneous neatening

-- 
https://patchwork.kernel.org/patch/9304171/

Documentation about submitting wireless patches and checking status
from patchwork:

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



Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Rafał Miłecki
On 27 September 2016 at 13:44, Rafał Miłecki  wrote:
> On 27 September 2016 at 13:27, Kalle Valo  wrote:
>> Arend Van Spriel  writes:
>>
>>> On 27-9-2016 11:14, Rafał Miłecki wrote:
 From: Rafał Miłecki 

 Flowrings contain skbs waiting for transmission that were passed to us
 by netif. It means we checked every one of them looking for 802.1x
 Ethernet type. When deleting flowring we have to use freeing function
 that will check for 802.1x type as well.

 Freeing skbs without a proper check was leading to counter not being
 properly decreased. This was triggering a WARNING every time
 brcmf_netdev_wait_pend8021x was called.
>>>
>>> Acked-by: Arend van Spriel 
 Signed-off-by: Rafał Miłecki 
 ---
 Kalle: this isn't important enough for 4.8 as it's too late for that.

 I'd like to get it for 4.9 however, as this fixes bug that could lead
 to WARNING on every add_key/del_key call. We was struggling with these
 WARNINGs for some time and this fixes one of two problems causing them.
>>
>> Ok, I'll queue this for 4.9.
>>
>>> Please mark it for stable as well.
>>
>> I can add that. Any ideas how old releases stable releases should this
>> go to?
>
> I was analyzing this.
> 1) This patch uses brcmf_get_ifp which is available in 4.4+ only.
> 2) It applies cleanly to 4.5+ only due to 32f90caa7debd ("brcmfmac:
> Increase nr of supported flowrings.")
> 3) 4.4 would also require applying to the patch without broadcom/ subdir
>
> That said I suggest 4.5+. Any objections?

Let me see if patchwork with pick Cc tag as it does for others.

Cc: sta...@vger.kernel.org # 4.5+

This may be worth backporting to 4.4 as well (as it's longterm), but
I'll do it separately due to patch not applying cleanly.

-- 
Rafał


Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 13:27, Kalle Valo wrote:
> Arend Van Spriel  writes:
> 
>> On 27-9-2016 11:14, Rafał Miłecki wrote:
>>> From: Rafał Miłecki 
>>>
>>> Flowrings contain skbs waiting for transmission that were passed to us
>>> by netif. It means we checked every one of them looking for 802.1x
>>> Ethernet type. When deleting flowring we have to use freeing function
>>> that will check for 802.1x type as well.
>>>
>>> Freeing skbs without a proper check was leading to counter not being
>>> properly decreased. This was triggering a WARNING every time
>>> brcmf_netdev_wait_pend8021x was called.
>>
>> Acked-by: Arend van Spriel 
>>> Signed-off-by: Rafał Miłecki 
>>> ---
>>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>>>
>>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>>> to WARNING on every add_key/del_key call. We was struggling with these
>>> WARNINGs for some time and this fixes one of two problems causing them.
> 
> Ok, I'll queue this for 4.9.
> 
>> Please mark it for stable as well.
> 
> I can add that. Any ideas how old releases stable releases should this
> go to?

Not sure if the vendor directory move causes issues as stable can not
fallback to three-way merge. I assumed it would so my last stable tag
was only for 4.7 and I took care of older kernels at later time with
backported patch. I can do that for this one as well.

Regards,
Arend


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

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 12:56, Amitkumar Karwar wrote:
> From: lihz 

Also the mailing list is no longer at shmoo.com. Should be:
hos...@lists.infradead.org

Regards,
Arend


Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Kalle Valo
Arend Van Spriel  writes:

> On 27-9-2016 11:14, Rafał Miłecki wrote:
>> From: Rafał Miłecki 
>> 
>> Flowrings contain skbs waiting for transmission that were passed to us
>> by netif. It means we checked every one of them looking for 802.1x
>> Ethernet type. When deleting flowring we have to use freeing function
>> that will check for 802.1x type as well.
>> 
>> Freeing skbs without a proper check was leading to counter not being
>> properly decreased. This was triggering a WARNING every time
>> brcmf_netdev_wait_pend8021x was called.
>
> Acked-by: Arend van Spriel 
>> Signed-off-by: Rafał Miłecki 
>> ---
>> Kalle: this isn't important enough for 4.8 as it's too late for that.
>> 
>> I'd like to get it for 4.9 however, as this fixes bug that could lead
>> to WARNING on every add_key/del_key call. We was struggling with these
>> WARNINGs for some time and this fixes one of two problems causing them.

Ok, I'll queue this for 4.9.

> Please mark it for stable as well.

I can add that. Any ideas how old releases stable releases should this
go to?

-- 
Kalle Valo


Re: [PATCH] brcmfmac: replace WARNING on timeout with a simple error message

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 12:12, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> Even with timeout increased to 950 ms we get WARNINGs from time to time.
> It mostly happens on A-MPDU stalls (e.g. when station goes out of
> range). It may take up to 5-10 secods for the firmware to recover and
> for that time it doesn't process packets.
> 
> It's still useful to have a message on time out as it may indicate some
> firmware problem and incorrect key update. Raising a WARNING however
> wasn't really that necessary, it doesn't point to any driver bug anymore
> and backtrace wasn't much useful.

Indeed the interesting part would be in another context. So:

Acked-by: Arend van Spriel 
> Signed-off-by: Rafał Miłecki 
> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index 6d046ba..9e6f60a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -1161,7 +1161,8 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp)
>!brcmf_get_pend_8021x_cnt(ifp),
>MAX_WAIT_FOR_8021X_TX);
>  
> - WARN_ON(!err);
> + if (!err)
> + brcmf_err("Timed out waiting for no pending 802.1x packets\n");
>  
>   return !err;
>  }
> 


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

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 12:56, Amitkumar Karwar wrote:
> From: lihz 

minor thing. Could you use another prefix iso 'nl80211:'. That has
different expectation for me at least, ie. changes in nl80211 api, but
this patch is for hostap repo so 'hostap:' or 'wpa_supp:' would be
better fit here.

Regards,
Arend

> Currently this feature is available under CONFIG_DRIVER_NL80211_QCA
> flag. It makes use of vendor command approach.
> 
> This patch along with a kernel patch is an attempt to make the
> feature generic. psk is downloaded through standard set_key path
> There is an extra handling in ROAM event from driver.
> 
> Signed-off-by: Huazeng Li 
> Signed-off-by: Amitkumar Karwar 
> ---
>  src/common/defs.h  |  4 +++-
>  src/common/ieee802_11_defs.h   |  3 +++
>  src/drivers/driver_nl80211.c   | 40 
> +-
>  src/drivers/driver_nl80211_capa.c  |  4 
>  src/drivers/driver_nl80211_event.c | 17 
>  src/drivers/nl80211_copy.h |  8 
>  src/rsn_supp/wpa_ft.c  |  4 
>  7 files changed, 57 insertions(+), 23 deletions(-)
> 
> diff --git a/src/common/defs.h b/src/common/defs.h
> index 4f56794..e9e9692 100644
> --- a/src/common/defs.h
> +++ b/src/common/defs.h
> @@ -148,7 +148,9 @@ enum wpa_alg {
>   WPA_ALG_CCMP_256,
>   WPA_ALG_BIP_GMAC_128,
>   WPA_ALG_BIP_GMAC_256,
> - WPA_ALG_BIP_CMAC_256
> + WPA_ALG_BIP_CMAC_256,
> + WPA_ALG_PMK_R0,
> + WPA_ALG_PMK_R0_NAME,
>  };
>  
>  /**
> diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
> index 02d2ad7..632374a 100644
> --- a/src/common/ieee802_11_defs.h
> +++ b/src/common/ieee802_11_defs.h
> @@ -1376,6 +1376,9 @@ enum plink_action_field {
>  #define WLAN_CIPHER_SUITE_BIP_CMAC_256   0x000FAC0D
>  
>  #define WLAN_CIPHER_SUITE_SMS4   0x00147201
> +#define WLAN_CIPHER_SUITE_PMK0x00147202
> +#define WLAN_CIPHER_SUITE_PMK_R0 0x00147203
> +#define WLAN_CIPHER_SUITE_PMK_R0_NAME0x00147204
>  
>  #define WLAN_CIPHER_SUITE_CKIP   0x00409600
>  #define WLAN_CIPHER_SUITE_CKIP_CMIC  0x00409601
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 1210d43..7024b8a 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -2675,21 +2675,34 @@ static int wpa_driver_nl80211_set_key(const char 
> *ifname, struct i802_bss *bss,
>   }
>  #endif /* CONFIG_TDLS */
>  
> -#ifdef CONFIG_DRIVER_NL80211_QCA
> - if (alg == WPA_ALG_PMK &&
> - (drv->capa.flags & WPA_DRIVER_FLAGS_KEY_MGMT_OFFLOAD)) {
> - wpa_printf(MSG_DEBUG, "%s: calling issue_key_mgmt_set_key",
> -__func__);
> - ret = issue_key_mgmt_set_key(drv, key, key_len);
> - return ret;
> +
> + if ((alg == WPA_ALG_PMK || alg == WPA_ALG_PMK_R0 ||
> +  alg == WPA_ALG_PMK_R0_NAME) &&
> +  (drv->capa.flags & WPA_DRIVER_FLAGS_KEY_MGMT_OFFLOAD)) {
> + u32 suite;
> +
> + if (alg == WPA_ALG_PMK_R0)
> + suite = WLAN_CIPHER_SUITE_PMK_R0;
> + else if (alg == WPA_ALG_PMK_R0_NAME)
> + suite = WLAN_CIPHER_SUITE_PMK_R0_NAME;
> + else if (alg == WPA_ALG_PMK)
> + suite = WLAN_CIPHER_SUITE_PMK;
> + if (!suite)
> + goto fail;
> + msg = nl80211_ifindex_msg(drv, ifindex, 0, NL80211_CMD_NEW_KEY);
> + if (!msg ||
> + nla_put(msg, NL80211_ATTR_KEY_DATA, key_len, key) ||
> + nla_put_u32(msg, NL80211_ATTR_KEY_CIPHER, suite))
> + goto fail;
> + wpa_hexdump_key(MSG_DEBUG, "nl80211: KEY_DATA", key, key_len);
>   }
> -#endif /* CONFIG_DRIVER_NL80211_QCA */
>  
>   if (alg == WPA_ALG_NONE) {
>   msg = nl80211_ifindex_msg(drv, ifindex, 0, NL80211_CMD_DEL_KEY);
>   if (!msg)
>   return -ENOBUFS;
> - } else {
> + } else if (alg != WPA_ALG_PMK && alg != WPA_ALG_PMK_R0 &&
> +alg != WPA_ALG_PMK_R0_NAME) {
>   u32 suite;
>  
>   suite = wpa_alg_to_cipher_suite(alg, key_len);
> @@ -5137,6 +5150,15 @@ static int wpa_driver_nl80211_associate(
>  
>   if (wpa_driver_nl80211_set_mode(priv, nlmode) < 0)
>   return -1;
> + if (params->req_key_mgmt_offload && params->psk &&
> + (params->key_mgmt_suite == WPA_KEY_MGMT_PSK ||
> +  params->key_mgmt_suite == WPA_KEY_MGMT_PSK_SHA256 ||
> +  params->key_mgmt_suite == WPA_KEY_MGMT_FT_PSK)) {
> + wpa_driver_nl80211_set_key(bss->ifname, bss,
> +WPA_ALG_PMK,
> +NULL, 0, 1, NULL, 0,
> +  

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

2016-09-27 Thread Kalle Valo
Amitkumar Karwar  writes:

> From: lihz 

A minor thing, but the from header in both of the patches don't have the
full name and the git log would look ugly. It should be something like
this:

From: Huazeng Li 

-- 
Kalle Valo


[PATCH] nl80211: add key management offload feature

2016-09-27 Thread Amitkumar Karwar
From: lihz 

Currently this feature is available under CONFIG_DRIVER_NL80211_QCA
flag. It makes use of vendor command approach.

This patch along with a kernel patch is an attempt to make the
feature generic. psk is downloaded through standard set_key path
There is an extra handling in ROAM event from driver.

Signed-off-by: Huazeng Li 
Signed-off-by: Amitkumar Karwar 
---
 src/common/defs.h  |  4 +++-
 src/common/ieee802_11_defs.h   |  3 +++
 src/drivers/driver_nl80211.c   | 40 +-
 src/drivers/driver_nl80211_capa.c  |  4 
 src/drivers/driver_nl80211_event.c | 17 
 src/drivers/nl80211_copy.h |  8 
 src/rsn_supp/wpa_ft.c  |  4 
 7 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/src/common/defs.h b/src/common/defs.h
index 4f56794..e9e9692 100644
--- a/src/common/defs.h
+++ b/src/common/defs.h
@@ -148,7 +148,9 @@ enum wpa_alg {
WPA_ALG_CCMP_256,
WPA_ALG_BIP_GMAC_128,
WPA_ALG_BIP_GMAC_256,
-   WPA_ALG_BIP_CMAC_256
+   WPA_ALG_BIP_CMAC_256,
+   WPA_ALG_PMK_R0,
+   WPA_ALG_PMK_R0_NAME,
 };
 
 /**
diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
index 02d2ad7..632374a 100644
--- a/src/common/ieee802_11_defs.h
+++ b/src/common/ieee802_11_defs.h
@@ -1376,6 +1376,9 @@ enum plink_action_field {
 #define WLAN_CIPHER_SUITE_BIP_CMAC_256 0x000FAC0D
 
 #define WLAN_CIPHER_SUITE_SMS4 0x00147201
+#define WLAN_CIPHER_SUITE_PMK  0x00147202
+#define WLAN_CIPHER_SUITE_PMK_R0   0x00147203
+#define WLAN_CIPHER_SUITE_PMK_R0_NAME  0x00147204
 
 #define WLAN_CIPHER_SUITE_CKIP 0x00409600
 #define WLAN_CIPHER_SUITE_CKIP_CMIC0x00409601
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 1210d43..7024b8a 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -2675,21 +2675,34 @@ static int wpa_driver_nl80211_set_key(const char 
*ifname, struct i802_bss *bss,
}
 #endif /* CONFIG_TDLS */
 
-#ifdef CONFIG_DRIVER_NL80211_QCA
-   if (alg == WPA_ALG_PMK &&
-   (drv->capa.flags & WPA_DRIVER_FLAGS_KEY_MGMT_OFFLOAD)) {
-   wpa_printf(MSG_DEBUG, "%s: calling issue_key_mgmt_set_key",
-  __func__);
-   ret = issue_key_mgmt_set_key(drv, key, key_len);
-   return ret;
+
+   if ((alg == WPA_ALG_PMK || alg == WPA_ALG_PMK_R0 ||
+alg == WPA_ALG_PMK_R0_NAME) &&
+(drv->capa.flags & WPA_DRIVER_FLAGS_KEY_MGMT_OFFLOAD)) {
+   u32 suite;
+
+   if (alg == WPA_ALG_PMK_R0)
+   suite = WLAN_CIPHER_SUITE_PMK_R0;
+   else if (alg == WPA_ALG_PMK_R0_NAME)
+   suite = WLAN_CIPHER_SUITE_PMK_R0_NAME;
+   else if (alg == WPA_ALG_PMK)
+   suite = WLAN_CIPHER_SUITE_PMK;
+   if (!suite)
+   goto fail;
+   msg = nl80211_ifindex_msg(drv, ifindex, 0, NL80211_CMD_NEW_KEY);
+   if (!msg ||
+   nla_put(msg, NL80211_ATTR_KEY_DATA, key_len, key) ||
+   nla_put_u32(msg, NL80211_ATTR_KEY_CIPHER, suite))
+   goto fail;
+   wpa_hexdump_key(MSG_DEBUG, "nl80211: KEY_DATA", key, key_len);
}
-#endif /* CONFIG_DRIVER_NL80211_QCA */
 
if (alg == WPA_ALG_NONE) {
msg = nl80211_ifindex_msg(drv, ifindex, 0, NL80211_CMD_DEL_KEY);
if (!msg)
return -ENOBUFS;
-   } else {
+   } else if (alg != WPA_ALG_PMK && alg != WPA_ALG_PMK_R0 &&
+  alg != WPA_ALG_PMK_R0_NAME) {
u32 suite;
 
suite = wpa_alg_to_cipher_suite(alg, key_len);
@@ -5137,6 +5150,15 @@ static int wpa_driver_nl80211_associate(
 
if (wpa_driver_nl80211_set_mode(priv, nlmode) < 0)
return -1;
+   if (params->req_key_mgmt_offload && params->psk &&
+   (params->key_mgmt_suite == WPA_KEY_MGMT_PSK ||
+params->key_mgmt_suite == WPA_KEY_MGMT_PSK_SHA256 ||
+params->key_mgmt_suite == WPA_KEY_MGMT_FT_PSK)) {
+   wpa_driver_nl80211_set_key(bss->ifname, bss,
+  WPA_ALG_PMK,
+  NULL, 0, 1, NULL, 0,
+  params->psk, 32);
+   }
return wpa_driver_nl80211_connect(drv, params);
}
 
diff --git a/src/drivers/driver_nl80211_capa.c 
b/src/drivers/driver_nl80211_capa.c
index 6adc3f6..26bd7bd 100644
--- a/src/drivers/driver_nl80211_capa.c
+++ b/src/drivers/driver_nl80211_capa.c
@@ -362,6 +362,10 @@ static void wiphy_info_ext_feature_flags(struct 
wiphy_info_data *info,
 
if 

[PATCH] cfg80211: add key management offload feature

2016-09-27 Thread Amitkumar Karwar
From: lihz 

This patch adds key management offload feature. It needs to be
advertised through NL80211_EXT_FEATURE_KEY_MGMT_OFFLOAD flag.
Existing cfg80211_roamed API has been extended to report keys
for roaming offload.

Signed-off-by: Huazeng Li 
Signed-off-by: Amitkumar Karwar 
---
 drivers/net/wireless/ath/ath6kl/cfg80211.c |  3 ++-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c |  3 ++-
 drivers/net/wireless/rndis_wlan.c  |  3 ++-
 drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c  |  2 +-
 drivers/staging/wlan-ng/cfg80211.c |  2 +-
 include/linux/ieee80211.h  |  3 +++
 include/net/cfg80211.h |  8 +--
 include/uapi/linux/nl80211.h   | 11 +
 net/wireless/core.h|  8 ++-
 net/wireless/nl80211.c | 19 ++--
 net/wireless/nl80211.h |  4 +++-
 net/wireless/sme.c | 26 +-
 net/wireless/util.c|  4 +++-
 13 files changed, 79 insertions(+), 17 deletions(-)
 mode change 100644 => 100755 net/wireless/nl80211.c

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c 
b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index b7fe0af..9511f73 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -809,7 +809,8 @@ void ath6kl_cfg80211_connect_event(struct ath6kl_vif *vif, 
u16 channel,
} else if (vif->sme_state == SME_CONNECTED) {
/* inform roam event to cfg80211 */
cfg80211_roamed_bss(vif->ndev, bss, assoc_req_ie, assoc_req_len,
-   assoc_resp_ie, assoc_resp_len, GFP_KERNEL);
+   assoc_resp_ie, assoc_resp_len, GFP_KERNEL,
+   NULL, NULL, NULL, 0);
}
 }
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 748eaa6..5934b77 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5450,7 +5450,8 @@ done:
kfree(buf);
cfg80211_roamed(ndev, notify_channel, (u8 *)profile->bssid,
conn_info->req_ie, conn_info->req_ie_len,
-   conn_info->resp_ie, conn_info->resp_ie_len, GFP_KERNEL);
+   conn_info->resp_ie, conn_info->resp_ie_len, GFP_KERNEL,
+   NULL, NULL, NULL, 0);
brcmf_dbg(CONN, "Report roaming result\n");
 
set_bit(BRCMF_VIF_STATUS_CONNECTED, >vif->sme_state);
diff --git a/drivers/net/wireless/rndis_wlan.c 
b/drivers/net/wireless/rndis_wlan.c
index 603c904..ad9535f 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2838,7 +2838,8 @@ static void rndis_wlan_do_link_up_work(struct usbnet 
*usbdev)
cfg80211_roamed(usbdev->net,
get_current_channel(usbdev, NULL),
bssid, req_ie, req_ie_len,
-   resp_ie, resp_ie_len, GFP_KERNEL);
+   resp_ie, resp_ie_len, GFP_KERNEL,
+   NULL, NULL, NULL, 0);
} else if (priv->infra_mode == NDIS_80211_INFRA_ADHOC)
cfg80211_ibss_joined(usbdev->net, bssid,
 get_current_channel(usbdev, NULL),
diff --git a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c 
b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
index d0ba377..e74216a 100644
--- a/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723au/os_dep/ioctl_cfg80211.c
@@ -341,7 +341,7 @@ void rtw_cfg80211_indicate_connect(struct rtw_adapter 
*padapter)
sizeof(struct ieee80211_hdr_3addr) + 6,
pmlmepriv->assoc_rsp_len -
sizeof(struct ieee80211_hdr_3addr) - 6,
-   GFP_ATOMIC);
+   GFP_ATOMIC, NULL, NULL, NULL, 0);
} else {
cfg80211_connect_result(padapter->pnetdev,
cur_network->network.MacAddress,
diff --git a/drivers/staging/wlan-ng/cfg80211.c 
b/drivers/staging/wlan-ng/cfg80211.c
index f46dfe6..178d955 100644
--- a/drivers/staging/wlan-ng/cfg80211.c
+++ b/drivers/staging/wlan-ng/cfg80211.c
@@ -722,7 +722,7 @@ void prism2_disconnected(wlandevice_t *wlandev)
 void prism2_roamed(wlandevice_t *wlandev)
 {
cfg80211_roamed(wlandev->netdev, NULL, wlandev->bssid,
-   NULL, 0, NULL, 0, GFP_KERNEL);
+   NULL, 0, NULL, 0, GFP_KERNEL, NULL, NULL, 

[PATCH] brcmfmac: replace WARNING on timeout with a simple error message

2016-09-27 Thread Rafał Miłecki
From: Rafał Miłecki 

Even with timeout increased to 950 ms we get WARNINGs from time to time.
It mostly happens on A-MPDU stalls (e.g. when station goes out of
range). It may take up to 5-10 secods for the firmware to recover and
for that time it doesn't process packets.

It's still useful to have a message on time out as it may indicate some
firmware problem and incorrect key update. Raising a WARNING however
wasn't really that necessary, it doesn't point to any driver bug anymore
and backtrace wasn't much useful.

Signed-off-by: Rafał Miłecki 
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 6d046ba..9e6f60a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1161,7 +1161,8 @@ int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp)
 !brcmf_get_pend_8021x_cnt(ifp),
 MAX_WAIT_FOR_8021X_TX);
 
-   WARN_ON(!err);
+   if (!err)
+   brcmf_err("Timed out waiting for no pending 802.1x packets\n");
 
return !err;
 }
-- 
2.9.3



Re: [PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Arend Van Spriel
On 27-9-2016 11:14, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> Flowrings contain skbs waiting for transmission that were passed to us
> by netif. It means we checked every one of them looking for 802.1x
> Ethernet type. When deleting flowring we have to use freeing function
> that will check for 802.1x type as well.
> 
> Freeing skbs without a proper check was leading to counter not being
> properly decreased. This was triggering a WARNING every time
> brcmf_netdev_wait_pend8021x was called.

Acked-by: Arend van Spriel 
> Signed-off-by: Rafał Miłecki 
> ---
> Kalle: this isn't important enough for 4.8 as it's too late for that.
> 
> I'd like to get it for 4.9 however, as this fixes bug that could lead
> to WARNING on every add_key/del_key call. We was struggling with these
> WARNINGs for some time and this fixes one of two problems causing them.

Please mark it for stable as well.

> ---
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> index b16b367..d0b738d 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
> @@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring 
> *flow, u16 flowid,
>  
>  void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
>  {
> + struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
>   struct brcmf_flowring_ring *ring;
> + struct brcmf_if *ifp;
>   u16 hash_idx;
> + u8 ifidx;
>   struct sk_buff *skb;
>  
>   ring = flow->rings[flowid];
>   if (!ring)
>   return;
> +
> + ifidx = brcmf_flowring_ifidx_get(flow, flowid);
> + ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
> +
>   brcmf_flowring_block(flow, flowid, false);
>   hash_idx = ring->hash_id;
>   flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;

I am not very familiar with flowring code, but I suppose this is just
initializing the entry for later use, right?

> @@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, 
> u16 flowid)
>  
>   skb = skb_dequeue(>skblist);
>   while (skb) {
> - brcmu_pkt_buf_free_skb(skb);
> + brcmf_txfinalize(ifp, skb, false);
>   skb = skb_dequeue(>skblist);
>   }
>  
> 


Re: [PATCH] brcmfmac: implement more accurate skb tracking

2016-09-27 Thread Arend Van Spriel
On 26-9-2016 14:38, Rafał Miłecki wrote:
> On 26 September 2016 at 14:13, Rafał Miłecki  wrote:
>> On 26 September 2016 at 13:46, Arend Van Spriel
>>  wrote:
>>> On 26-9-2016 12:23, Rafał Miłecki wrote:
 From: Rafał Miłecki 

 We need to track 802.1x packets to know if there are any pending ones
 for transmission. This is required for performing key update in the
 firmware.
>>>
>>> The problem we are trying to solve is a pretty old one. The problem is
>>> that wpa_supplicant uses two separate code paths: EAPOL messaging
>>> through data path and key configuration though nl80211.
>>
>> Can I find it described/reported somewhere?
>>
>>
 Unfortunately our old tracking code wasn't very accurate. It was
 treating skb as pending as soon as it was passed by the netif. Actual
 handling packet to the firmware was happening later as brcmfmac
 internally queues them and uses its own worker(s).
>>>
>>> That does not seem right. As soon as we get a 1x packet we need to wait
>>> with key configuration regardless whether it is still in the driver or
>>> handed over to firmware already.
>>
>> OK, thanks.
> 
> Actually, it's not OK. I was trying to report/describe/discuss this
> problem for over a week. I couldn't get much of answer from you.
> 
> I had to come with a patch I worked on for quite some time. Only then
> you decided to react and reply with a reason for a nack. I see this
> patch may be wrong (but it's still hard to know what's going wrong
> without a proper hostapd bug report). I'd expect you to somehow work &
> communicate with open source community.

We do or at least make an honest attempt, but there is more on our plate
so responses may be delayed. It also does not help when you get anal and
preachy when we do respond. Also not OK. In this case the delay is
caused because I had to pick up the thread(s) as Hante is on vacation
(he needed a break :-p ). However, you started sending patches so I
decided to look at and respond to those. Sorry if you felt like we left
you hanging to dry.

Regards,
Arend


[PATCH 4.9] brcmfmac: use correct skb freeing helper when deleting flowring

2016-09-27 Thread Rafał Miłecki
From: Rafał Miłecki 

Flowrings contain skbs waiting for transmission that were passed to us
by netif. It means we checked every one of them looking for 802.1x
Ethernet type. When deleting flowring we have to use freeing function
that will check for 802.1x type as well.

Freeing skbs without a proper check was leading to counter not being
properly decreased. This was triggering a WARNING every time
brcmf_netdev_wait_pend8021x was called.

Signed-off-by: Rafał Miłecki 
---
Kalle: this isn't important enough for 4.8 as it's too late for that.

I'd like to get it for 4.9 however, as this fixes bug that could lead
to WARNING on every add_key/del_key call. We was struggling with these
WARNINGs for some time and this fixes one of two problems causing them.
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
index b16b367..d0b738d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/flowring.c
@@ -234,13 +234,20 @@ static void brcmf_flowring_block(struct brcmf_flowring 
*flow, u16 flowid,
 
 void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 flowid)
 {
+   struct brcmf_bus *bus_if = dev_get_drvdata(flow->dev);
struct brcmf_flowring_ring *ring;
+   struct brcmf_if *ifp;
u16 hash_idx;
+   u8 ifidx;
struct sk_buff *skb;
 
ring = flow->rings[flowid];
if (!ring)
return;
+
+   ifidx = brcmf_flowring_ifidx_get(flow, flowid);
+   ifp = brcmf_get_ifp(bus_if->drvr, ifidx);
+
brcmf_flowring_block(flow, flowid, false);
hash_idx = ring->hash_id;
flow->hash[hash_idx].ifidx = BRCMF_FLOWRING_INVALID_IFIDX;
@@ -249,7 +256,7 @@ void brcmf_flowring_delete(struct brcmf_flowring *flow, u16 
flowid)
 
skb = skb_dequeue(>skblist);
while (skb) {
-   brcmu_pkt_buf_free_skb(skb);
+   brcmf_txfinalize(ifp, skb, false);
skb = skb_dequeue(>skblist);
}
 
-- 
2.9.3



Re: [PATCH] brcmfmac: implement more accurate skb tracking

2016-09-27 Thread Arend Van Spriel
On 26-9-2016 16:59, Dan Williams wrote:
> On Mon, 2016-09-26 at 14:13 +0200, Rafał Miłecki wrote:
>> On 26 September 2016 at 13:46, Arend Van Spriel
>>  wrote:
>>>
>>> On 26-9-2016 12:23, Rafał Miłecki wrote:

 From: Rafał Miłecki 

 We need to track 802.1x packets to know if there are any pending
 ones
 for transmission. This is required for performing key update in
 the
 firmware.
>>>
>>> The problem we are trying to solve is a pretty old one. The problem
>>> is
>>> that wpa_supplicant uses two separate code paths: EAPOL messaging
>>> through data path and key configuration though nl80211.
>>
>> Can I find it described/reported somewhere?
> 
> If I understand the issue correctly, you can find all this in the
> supplicant code.  Once the supplicant has done whatever it wants to do
> with the data frames that just happen to be EAPOL it then sends the
> keys down to the driver with nl80211.

Indeed. EAPOL packets are simply data packets as far as the 802.11 stack
is concerned. The arrival of those in the driver is not predictable
hence we hold off the key configuration until those have been passed
over to firmware.

> But it sounds like, instead of sniffing EAPOL frames in the driver skb
> tracking and sniffing ETH_P_PAE, you should probably implement support
> for NL80211_CMD_CRIT_PROTOCOL_START/NL80211_CMD_CRIT_PROTOCOL_STOP and
> key off the passed-in NL80211_CRIT_PROTO_EAPOL.  At least at the
> beginning of connection setup only EAPOL packets will be allowed
> anyway.
> 
> It doesn't seem like the supplicant uses NL80211_CRIT_PROTO_EAPOL yet,
> but that should also be fixed in the supplicant itself.  You should
> probably get some comments from Jouni on how he'd like to see all this
> work.  But generally the less specific sniffing of frames in drivers,
> likely the better.

Indeed. That was the main motivation to introduce the CRIT_PROTO api. If
I recall correctly it was considered the task of the network manager to
issue the START/STOP. Recently noticed the use of CRIT_PROTO_DHCP on
some target system, which we already support in brcmfmac. From your
response I guess you consider CRIT_PROTO_EAPOL to be issued by the
supplicant.

Regards,
Arend

> Dan
> 
>>
>>>

 Unfortunately our old tracking code wasn't very accurate. It was
 treating skb as pending as soon as it was passed by the netif.
 Actual
 handling packet to the firmware was happening later as brcmfmac
 internally queues them and uses its own worker(s).
>>>
>>> That does not seem right. As soon as we get a 1x packet we need to
>>> wait
>>> with key configuration regardless whether it is still in the driver
>>> or
>>> handed over to firmware already.
>>
>> OK, thanks.


bcmdhd: Strange Power Save messages

2016-09-27 Thread Gucea Doru
Hello,

I am analyzing the Power Save algorithm used on a pair of Nexus 5
devices. The devices use the bcmdhd Broadcom driver and are Wi-Fi
Direct connected.

My test is very simple: I send ping packets from the P2P client
towards the P2P GO. Before sending a ping packet the P2P client enters
PS mode (it sends a Null frame with the PWR MGT bit set). A few
miliseconds later, the P2P client sends the ping request message. Once
the ping request is ACKed the P2P client exits the PS mode (it sends a
Null frame with the PWR MGT bit unset) and the P2P GO decides to send
him the ping reply.

What is the decision triggering the exit from the PS mode immediately
after the ping request? I am asking this because 802.11 PS legacy
specifies that the client should wait for a beacon with TIM set in
order to wake up: in my case, there is no beacon between the ping
request message and the Null frame that announces the exit from the PS
mode.

For reference, the Wireshark trace can be found at [1].
SSID: DIRECT-35-Android_Intel
WPA pass: JYdrhZp3

[1] 
https://drive.google.com/file/d/0B5SBH08PU_ChQk95LWpzekh3VU0/view?usp=sharing

Thank you,
Doru


Re: [PATCHv2] mac80211: check A-MSDU inner frame source address on AP interfaces

2016-09-27 Thread michael-dev

Am 27.09.2016 10:01, schrieb Johannes Berg:

...

This leaves "eth_80211" uninitialized if has_80211_header is false.


@@ -768,6 +768,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff
*skb, struct sk_buff_head *list,
    subframe_len = sizeof(struct ethhdr) + len;
    padding = (4 - subframe_len) & 0x3;
 
+   if (unlikely(has_80211_header &&
+    (iftype == NL80211_IFTYPE_AP ||
+     iftype == NL80211_IFTYPE_AP_VLAN) &&
+    !ether_addr_equal(eth_80211.h_source,
eth.h_source)
+      ))
+   goto purge;


And this then compares against uninitialized data, so this won't work.


but it only compares against eth_80211 if has_80211_header is true due 
to order of evaluation, which in turn implies eth_80211 is initialized, 
right?


michael


Re: [PATCHv2] mac80211: check A-MSDU inner frame source address on AP interfaces

2016-09-27 Thread Johannes Berg
On Tue, 2016-09-27 at 10:53 +0200, michael-dev wrote:
> Am 27.09.2016 10:01, schrieb Johannes Berg:
> > 
> > ...
> > 
> > This leaves "eth_80211" uninitialized if has_80211_header is false.
> > 
> > > 
> > > @@ -768,6 +768,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff
> > > *skb, struct sk_buff_head *list,
> > >   subframe_len = sizeof(struct ethhdr) + len;
> > >   padding = (4 - subframe_len) & 0x3;
> > >  
> > > + if (unlikely(has_80211_header &&
> > > +  (iftype == NL80211_IFTYPE_AP ||
> > > +   iftype == NL80211_IFTYPE_AP_VLAN)
> > > &&
> > > > > > +    !ether_addr_equal(eth_80211.h_source,
> > > eth.h_source)
> > > +    ))
> > > + goto purge;
> > 
> > And this then compares against uninitialized data, so this won't
> > work.
> 
> but it only compares against eth_80211 if has_80211_header is true
> due to order of evaluation, which in turn implies eth_80211 is
> initialized, right?
> 

Oh, right, I missed that, sorry.

Nevertheless, it seems it would be better to allow the other users (not
mac80211) that have has_80211_header=false to still have the check?

johannes


Re: [PATCHv2] mac80211: check A-MSDU inner frame source address on AP interfaces

2016-09-27 Thread Johannes Berg
Huh. I know this bug, I thought we fixed it a long time ago. Oops.


> - struct ethhdr eth;
> + struct ethhdr eth, eth_80211;
>   bool reuse_frag = skb->head_frag && !skb_has_frag_list(skb);
>   bool reuse_skb = false;
>   bool last = false;
>  
>   if (has_80211_header) {
> - err = __ieee80211_data_to_8023(skb, , addr,
> iftype);
> + err = __ieee80211_data_to_8023(skb, _80211,
> addr, iftype);
>   if (err)
>   goto out;
>   }

This leaves "eth_80211" uninitialized if has_80211_header is false.

> @@ -768,6 +768,13 @@ void ieee80211_amsdu_to_8023s(struct sk_buff
> *skb, struct sk_buff_head *list,
>   subframe_len = sizeof(struct ethhdr) + len;
>   padding = (4 - subframe_len) & 0x3;
>  
> + if (unlikely(has_80211_header &&
> +  (iftype == NL80211_IFTYPE_AP ||
> +   iftype == NL80211_IFTYPE_AP_VLAN) &&
> +  !ether_addr_equal(eth_80211.h_source,
> eth.h_source)
> +    ))
> + goto purge;

And this then compares against uninitialized data, so this won't work.

I'd suggest removing the "has_80211_header" argument entirely, and
replacing it with a "const u8 *sa" argument, but that complicates
mac80211 significantly since all the checks
in __ieee80211_data_to_8023() would have to be replicated.

Maybe we can still do this, and say that it must be NULL when an 802.11
header is present, and be the SA when not. However, mwifiex doesn't
seem to be able to easily provide the SA (at least I don't see it,
perhaps it can), so that we'd have to allow some kind of ERR_PTR() or
something for that special case ... Actually it'd be better to just fix
mwifiex though :)

The staging driver using this (rtl8712) can easily provide the SA,
afaict.

johannes