Re: [PATCH v2] brcmfmac: Don't increase 8021x_cnt for dropped packets

[Per Förlin]

2016-04-07 1:57 GMT+02:00 Per Förlin :
> 2016-04-06 23:50 GMT+02:00 Arend Van Spriel :
>>
>>
>> On 6-4-2016 17:56, Per Förlin wrote:
>>> Hi,
>>>
>>> Thanks for getting back to me,
>>>
>>> * Chip: chip 43340 rev 2 pmurev 20
>>> * I'm running kernel 4.1 (backports) for the wireless parts
>>> * I have not to setup my environment for the latest kernel yet.
>>>   It's on my todo list.
>>>
>>> Looking at the code you pointed out I realize my fix i misplaced.
>>> It's strange that I don't end up with a negative count since
>>> the counter would be decreased twice, with my patch in place.
>>> I took a closer look at the execution flow and made the following
>>> observation:
>>>
>>> The eh->h_proto has changed when reaching the txfinalize() function.
>>> This only happens in case of packet drop.
>>>
>>> Simplified execution flow.
>>>
>>> brcmf_fws_process_skb():
>>>   # At this point ntohs(eh->h_proto) == 0x888E (ETH_P_PAE)
>>>   rc = brcmf_proto_txdata() ->
>>> brcmf_proto_bcdc_txdata() ->
>>>   brcmf_sdio_bus_txdata()
>>>   # At this point it has changed to: ntohs(eh->h_proto) == 0x8939 (?)
>>
>> How did you check this? The skb->data pointer is moved here [1] as we
>> need to prepend a protocol header. So probably you did not map 'eh'
>> variable over the correct data portion.
> I'm refering to the case when rc < 0.
> The next step will be a call to brcmf_txfinalize()
>
> In the following code:
> void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success)
> {
> struct ethhdr *eh;
> u16 type;
>
> eh = (struct ethhdr *)(txp->data);
> type = ntohs(eh->h_proto);
> 
> if (type == ETH_P_PAE) {
> atomic_dec(>pend_8021x_cnt);
>
> At this point "type" is 0x8939.
>
It looks like the header offset is pushed but not pulled back again in
case of error.
I have not tested this code yet but please let me know what you think.

+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -329,8 +329,12 @@ static int
 brcmf_proto_bcdc_txdata(struct brcmf_pub *drvr, int ifidx, u8 offset,
struct sk_buff *pktbuf)
 {
+   int res;
brcmf_proto_bcdc_hdrpush(drvr, ifidx, offset, pktbuf);
-   return brcmf_bus_txdata(drvr->bus_if, pktbuf);
+   res = brcmf_bus_txdata(drvr->bus_if, pktbuf);
+   if (res < 0)
+   brcmf_proto_bcdc_hdrpull(drvr, ifidx, offset, pktbuf);
+   return res;


> BR
> Per
>
>>
>> Regards,
>> Arend
>>
>> [1]
>> http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c#L2708
>>
>>>   if (rc < 0)
>>> # When executing this function the cnt will not be decreased due to
>>> # eh->h_proto being changed.
>>> brcmf_txfinalize()
>>>
>>> I need to continue my investigation to find out why the h_proto got changed.
>>> I assume this is not an expected behavior?
>>>
>>> BR
>>> Per
>>>
>>>
>>> 2016-04-06 10:22 GMT+02:00 Arend Van Spriel :
 On 5-4-2016 22:01, per.for...@gmail.com wrote:
> From: Per Forlin 
>
> The pend_8021x_cnt gets into a state where it's not being decreased.
> When the brcmf_netdev_wait_pend8021x is called it will timeout
> because there are no pending packets to be consumed. There is no
> easy way of getting out of this state once it has happened.
> Preventing the counter from being increased in case of dropped
> packets seem like a reasonable solution.
>
> Log showing overflow txq and increased cnt:
>
> // Extra debug prints
> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2
>
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>
> // Extra debug prints
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4
>
> WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
>   (warn_slowpath_common)
>   (warn_slowpath_null)
>   (brcmf_netdev_wait_pend8021x [brcmfmac])
>   (send_key_to_dongle [brcmfmac])
>   (brcmf_cfg80211_del_key [brcmfmac])
>   (nl80211_del_key [cfg80211])
>   (genl_rcv_msg)
>   (netlink_rcv_skb)
>   (genl_rcv)

Re: [PATCH v2] brcmfmac: Don't increase 8021x_cnt for dropped packets

[Per Förlin]

2016-04-06 23:50 GMT+02:00 Arend Van Spriel :
>
>
> On 6-4-2016 17:56, Per Förlin wrote:
>> Hi,
>>
>> Thanks for getting back to me,
>>
>> * Chip: chip 43340 rev 2 pmurev 20
>> * I'm running kernel 4.1 (backports) for the wireless parts
>> * I have not to setup my environment for the latest kernel yet.
>>   It's on my todo list.
>>
>> Looking at the code you pointed out I realize my fix i misplaced.
>> It's strange that I don't end up with a negative count since
>> the counter would be decreased twice, with my patch in place.
>> I took a closer look at the execution flow and made the following
>> observation:
>>
>> The eh->h_proto has changed when reaching the txfinalize() function.
>> This only happens in case of packet drop.
>>
>> Simplified execution flow.
>>
>> brcmf_fws_process_skb():
>>   # At this point ntohs(eh->h_proto) == 0x888E (ETH_P_PAE)
>>   rc = brcmf_proto_txdata() ->
>> brcmf_proto_bcdc_txdata() ->
>>   brcmf_sdio_bus_txdata()
>>   # At this point it has changed to: ntohs(eh->h_proto) == 0x8939 (?)
>
> How did you check this? The skb->data pointer is moved here [1] as we
> need to prepend a protocol header. So probably you did not map 'eh'
> variable over the correct data portion.
I'm refering to the case when rc < 0.
The next step will be a call to brcmf_txfinalize()

In the following code:
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success)
{
struct ethhdr *eh;
u16 type;

eh = (struct ethhdr *)(txp->data);
type = ntohs(eh->h_proto);

if (type == ETH_P_PAE) {
atomic_dec(>pend_8021x_cnt);

At this point "type" is 0x8939.

BR
Per

>
> Regards,
> Arend
>
> [1]
> http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c#L2708
>
>>   if (rc < 0)
>> # When executing this function the cnt will not be decreased due to
>> # eh->h_proto being changed.
>> brcmf_txfinalize()
>>
>> I need to continue my investigation to find out why the h_proto got changed.
>> I assume this is not an expected behavior?
>>
>> BR
>> Per
>>
>>
>> 2016-04-06 10:22 GMT+02:00 Arend Van Spriel :
>>> On 5-4-2016 22:01, per.for...@gmail.com wrote:
 From: Per Forlin 

 The pend_8021x_cnt gets into a state where it's not being decreased.
 When the brcmf_netdev_wait_pend8021x is called it will timeout
 because there are no pending packets to be consumed. There is no
 easy way of getting out of this state once it has happened.
 Preventing the counter from being increased in case of dropped
 packets seem like a reasonable solution.

 Log showing overflow txq and increased cnt:

 // Extra debug prints
 brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
 brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
 brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2

 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
 brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!

 // Extra debug prints
 brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
 brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
 brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4

 WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
   (warn_slowpath_common)
   (warn_slowpath_null)
   (brcmf_netdev_wait_pend8021x [brcmfmac])
   (send_key_to_dongle [brcmfmac])
   (brcmf_cfg80211_del_key [brcmfmac])
   (nl80211_del_key [cfg80211])
   (genl_rcv_msg)
   (netlink_rcv_skb)
   (genl_rcv)
   (netlink_unicast)
   (netlink_sendmsg)
   (sock_sendmsg)
   (___sys_sendmsg)
   (__sys_sendmsg)
   (SyS_sendmsg)

 Signed-off-by: Per Forlin 
 ---
 I came across this bug in an older kernel but it seems relevant
 for the latest kernel as well. I'm not familiar with the code
 but I can reproduce the issue and verify a fix for it.
 With this patch I no longer get stuck with a pend8021_cnt > 0.

 Change log:
  v2 - Add variable to know whether the counter is increased or not
>>>
>>> Sorry for the late response. Can you elaborate where you are seeing this.
>>>
>>> What kind of chipset are you using?
>>> What kernel version are you running?
>>>
>>> The function brcmf_fws_process_skb() should call brcmf_txfinalize() in
>>> case of an error and it does in latest kernel. There the 

[PATCH 5/7] staging: wilc1000: change handle_set_operation_mode's return type to void

[Chaehyun Lim]

When handle_set_operation_mode is called in hostIFthread that is a
kernel thread, it is not checked return type of this function. This
patch changes return type to void and removes braces if statement due to
have a single statement.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 4fd429e..ecf4ddc 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -328,8 +328,8 @@ static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
netdev_err(vif->ndev, "Failed to set driver handler\n");
 }
 
-static s32 handle_set_operation_mode(struct wilc_vif *vif,
-struct op_mode *hif_op_mode)
+static void handle_set_operation_mode(struct wilc_vif *vif,
+ struct op_mode *hif_op_mode)
 {
s32 result = 0;
struct wid wid;
@@ -345,12 +345,8 @@ static s32 handle_set_operation_mode(struct wilc_vif *vif,
if ((hif_op_mode->mode) == IDLE_MODE)
complete(_driver_comp);
 
-   if (result) {
+   if (result)
netdev_err(vif->ndev, "Failed to set driver handler\n");
-   return -EINVAL;
-   }
-
-   return result;
 }
 
 static s32 handle_set_ip_address(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
-- 
2.8.0

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

[PATCH 7/7] staging: wilc1000: rename result in handle_set_operation_mode

[Chaehyun Lim]

This patch renames result to ret that is used to get return value from
wilc_send_config_pkt. Some handle_*() functions are used as result,
others are used as ret. It will be changed as ret in all handle_*()
functions to match variable name.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 52eddc4..ac620fb 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -331,7 +331,7 @@ static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
 static void handle_set_operation_mode(struct wilc_vif *vif,
  struct op_mode *hif_op_mode)
 {
-   int result = 0;
+   int ret = 0;
struct wid wid;
 
wid.id = (u16)WID_SET_OPERATION_MODE;
@@ -339,13 +339,13 @@ static void handle_set_operation_mode(struct wilc_vif 
*vif,
wid.val = (s8 *)_op_mode->mode;
wid.size = sizeof(u32);
 
-   result = wilc_send_config_pkt(vif, SET_CFG, , 1,
- wilc_get_vif_idx(vif));
+   ret = wilc_send_config_pkt(vif, SET_CFG, , 1,
+  wilc_get_vif_idx(vif));
 
if ((hif_op_mode->mode) == IDLE_MODE)
complete(_driver_comp);
 
-   if (result)
+   if (ret)
netdev_err(vif->ndev, "Failed to set driver handler\n");
 }
 
-- 
2.8.0

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

[PATCH 6/7] staging: wilc1000: change data type of result in handle_set_operation_mode

[Chaehyun Lim]

This patch changes data type of result variable from s32 to int. result
is used to get return value from wilc_send_config_pkt that has return
type of int.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index ecf4ddc..52eddc4 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -331,7 +331,7 @@ static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
 static void handle_set_operation_mode(struct wilc_vif *vif,
  struct op_mode *hif_op_mode)
 {
-   s32 result = 0;
+   int result = 0;
struct wid wid;
 
wid.id = (u16)WID_SET_OPERATION_MODE;
-- 
2.8.0

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

[PATCH 3/7] staging: wilc1000: change data type of result in handle_set_wfi_drv_handler

[Chaehyun Lim]

This patch changes data type of result variable from s32 to int.
result is used to get return value from wilc_send_config_pkt that has
return type of int.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index ba472d5..33735fd 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -310,7 +310,7 @@ static void handle_set_channel(struct wilc_vif *vif,
 static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
   struct drv_handler *hif_drv_handler)
 {
-   s32 result = 0;
+   int result = 0;
struct wid wid;
 
wid.id = (u16)WID_SET_DRV_HANDLER;
-- 
2.8.0

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

[PATCH 4/7] staging: wilc1000: rename result in handle_set_wfi_drv_handler

[Chaehyun Lim]

This patch renames result to ret that is used to get return value from
wilc_send_config_pkt. Some handle_*() function are used as result,
others are used as ret. It will be changed as ret in all handle_*()
function to match variable name.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 33735fd..4fd429e 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -310,7 +310,7 @@ static void handle_set_channel(struct wilc_vif *vif,
 static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
   struct drv_handler *hif_drv_handler)
 {
-   int result = 0;
+   int ret = 0;
struct wid wid;
 
wid.id = (u16)WID_SET_DRV_HANDLER;
@@ -318,13 +318,13 @@ static void handle_set_wfi_drv_handler(struct wilc_vif 
*vif,
wid.val = (s8 *)hif_drv_handler;
wid.size = sizeof(*hif_drv_handler);
 
-   result = wilc_send_config_pkt(vif, SET_CFG, , 1,
- hif_drv_handler->handler);
+   ret = wilc_send_config_pkt(vif, SET_CFG, , 1,
+  hif_drv_handler->handler);
 
if (!hif_drv_handler->handler)
complete(_driver_comp);
 
-   if (result)
+   if (ret)
netdev_err(vif->ndev, "Failed to set driver handler\n");
 }
 
-- 
2.8.0

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

[PATCH 1/7] staging: wilc1000: change return type of ret variable in handle_get_tx_pwr

[Chaehyun Lim]

This patch changes return type of ret variable from s32 to int.
ret has return value from wilc_send_config_pkt that has return type of
int.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index eb86fd4..09e4ca3 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2468,7 +2468,7 @@ static void handle_set_tx_pwr(struct wilc_vif *vif, u8 
tx_pwr)
 
 static void handle_get_tx_pwr(struct wilc_vif *vif, u8 *tx_pwr)
 {
-   s32 ret = 0;
+   int ret = 0;
struct wid wid;
 
wid.id = (u16)WID_TX_POWER;
-- 
2.8.0

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

[PATCH 2/7] staging: wilc1000: change handle_set_wfi_drv_handler's return type to void

[Chaehyun Lim]

When handle_set_wfi_drv_handler is called in hostIFthread that is a
kernel thread, it is not checked return type of this function. This
patch changes return type to void and removes braces if statement due to
have a single statement.

Signed-off-by: Chaehyun Lim 
---
 drivers/staging/wilc1000/host_interface.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 09e4ca3..ba472d5 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -307,8 +307,8 @@ static void handle_set_channel(struct wilc_vif *vif,
netdev_err(vif->ndev, "Failed to set channel\n");
 }
 
-static s32 handle_set_wfi_drv_handler(struct wilc_vif *vif,
- struct drv_handler *hif_drv_handler)
+static void handle_set_wfi_drv_handler(struct wilc_vif *vif,
+  struct drv_handler *hif_drv_handler)
 {
s32 result = 0;
struct wid wid;
@@ -324,12 +324,8 @@ static s32 handle_set_wfi_drv_handler(struct wilc_vif *vif,
if (!hif_drv_handler->handler)
complete(_driver_comp);
 
-   if (result) {
+   if (result)
netdev_err(vif->ndev, "Failed to set driver handler\n");
-   return -EINVAL;
-   }
-
-   return result;
 }
 
 static s32 handle_set_operation_mode(struct wilc_vif *vif,
-- 
2.8.0

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

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Matthew Garrett]

On Wed, Apr 06, 2016 at 11:30:29PM +0200, Lukas Wunner wrote:

> Matthew's grub patch puts the wireless card into D3hot, which does not
> stop the interrupts. I have tested this. (It may stop the DMA, I cannot
> verify that as it doesn't occur on my machine for lack of an access point.)

Ah, interesting. One thing to note (and which I've just noticed) is that 
it looks like this fixup is only called when you boot with the "linux" 
command and not with the "linuxefi" command. If you're using Ubuntu, 
"linux" may fall back to "linuxefi" without executing this fixup. So we 
need to add it to the kernel EFI stub anyway.

-- 
Matthew Garrett | mj...@srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] brcmfmac: Don't increase 8021x_cnt for dropped packets

[Arend Van Spriel]

On 6-4-2016 17:56, Per Förlin wrote:
> Hi,
> 
> Thanks for getting back to me,
> 
> * Chip: chip 43340 rev 2 pmurev 20
> * I'm running kernel 4.1 (backports) for the wireless parts
> * I have not to setup my environment for the latest kernel yet.
>   It's on my todo list.
> 
> Looking at the code you pointed out I realize my fix i misplaced.
> It's strange that I don't end up with a negative count since
> the counter would be decreased twice, with my patch in place.
> I took a closer look at the execution flow and made the following
> observation:
> 
> The eh->h_proto has changed when reaching the txfinalize() function.
> This only happens in case of packet drop.
> 
> Simplified execution flow.
> 
> brcmf_fws_process_skb():
>   # At this point ntohs(eh->h_proto) == 0x888E (ETH_P_PAE)
>   rc = brcmf_proto_txdata() ->
> brcmf_proto_bcdc_txdata() ->
>   brcmf_sdio_bus_txdata()
>   # At this point it has changed to: ntohs(eh->h_proto) == 0x8939 (?)

How did you check this? The skb->data pointer is moved here [1] as we
need to prepend a protocol header. So probably you did not map 'eh'
variable over the correct data portion.

Regards,
Arend

[1]
http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c#L2708

>   if (rc < 0)
> # When executing this function the cnt will not be decreased due to
> # eh->h_proto being changed.
> brcmf_txfinalize()
> 
> I need to continue my investigation to find out why the h_proto got changed.
> I assume this is not an expected behavior?
> 
> BR
> Per
> 
> 
> 2016-04-06 10:22 GMT+02:00 Arend Van Spriel :
>> On 5-4-2016 22:01, per.for...@gmail.com wrote:
>>> From: Per Forlin 
>>>
>>> The pend_8021x_cnt gets into a state where it's not being decreased.
>>> When the brcmf_netdev_wait_pend8021x is called it will timeout
>>> because there are no pending packets to be consumed. There is no
>>> easy way of getting out of this state once it has happened.
>>> Preventing the counter from being increased in case of dropped
>>> packets seem like a reasonable solution.
>>>
>>> Log showing overflow txq and increased cnt:
>>>
>>> // Extra debug prints
>>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
>>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
>>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2
>>>
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>>
>>> // Extra debug prints
>>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
>>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
>>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4
>>>
>>> WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
>>>   (warn_slowpath_common)
>>>   (warn_slowpath_null)
>>>   (brcmf_netdev_wait_pend8021x [brcmfmac])
>>>   (send_key_to_dongle [brcmfmac])
>>>   (brcmf_cfg80211_del_key [brcmfmac])
>>>   (nl80211_del_key [cfg80211])
>>>   (genl_rcv_msg)
>>>   (netlink_rcv_skb)
>>>   (genl_rcv)
>>>   (netlink_unicast)
>>>   (netlink_sendmsg)
>>>   (sock_sendmsg)
>>>   (___sys_sendmsg)
>>>   (__sys_sendmsg)
>>>   (SyS_sendmsg)
>>>
>>> Signed-off-by: Per Forlin 
>>> ---
>>> I came across this bug in an older kernel but it seems relevant
>>> for the latest kernel as well. I'm not familiar with the code
>>> but I can reproduce the issue and verify a fix for it.
>>> With this patch I no longer get stuck with a pend8021_cnt > 0.
>>>
>>> Change log:
>>>  v2 - Add variable to know whether the counter is increased or not
>>
>> Sorry for the late response. Can you elaborate where you are seeing this.
>>
>> What kind of chipset are you using?
>> What kernel version are you running?
>>
>> The function brcmf_fws_process_skb() should call brcmf_txfinalize() in
>> case of an error and it does in latest kernel. There the count is
>> decreased. We had an issue mapping to the right ifp as reported by
>> Rafał, but that has also been fixed recently. So main question here:
>>
>> Do you see the issue in the latest kernel?
>>
>> Regards,
>> Arend
>>
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 7 ++-
>>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> index ed9998b..de80ad8 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>>> +++ 

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Lukas Wunner]

Hi,

On Wed, Apr 06, 2016 at 05:17:24PM +0200, Michael Büsch wrote:
> On Wed, 6 Apr 2016 08:31:51 -0500 Bjorn Helgaas  wrote:
>
> > Even for interrupts, it's only a 90% solution because we do still get
> > a few interrupts before the quirk runs.  There may not be enough to
> > trigger the "IRQ nobody cared" check,
> 
> If no driver requested the shared interrupt yet, it should be disabled
> in the interrupt controller. So the interrupts do not reach the CPU.
> The interrupt storm (on the CPU) starts as soon as some driver
> that shares the interrupt with b43 requests and thus enables the
> interrupt. And that always happens after the PCI fixup. Thus this is
> safe.

Yes, that is correct. I only see 67 interrupts for IRQ 17 with the
PCI quirk compiled in, all of which seem to come from the initialization
of pciehp, mmc and the hda sound card (which share the IRQ with b43 on my
machine).

Thanks,

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

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Lukas Wunner]

[+cc Matt Fleming, linux-efi]

Hi Bjorn,

On Tue, Apr 05, 2016 at 02:40:15PM -0500, Bjorn Helgaas wrote:
> On Tue, Mar 29, 2016 at 08:20:30PM +0200, Lukas Wunner wrote:
> > Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > on boot until they are reset, causing spurious interrupts if the IRQ is
> > shared. Apparently the EFI bootloader enables the device and does not
> > disable it before passing control to the OS. The bootloader contains a
> > driver for the wireless card which allows it to phone home to Cupertino.
> > This is used for Internet Recovery (download and install OS X images)
> > and probably also for Back to My Mac (remote access, RFC 6281) and to
> > discover stolen hardware.
> > 
> > The issue is most pronounced on 2011 and 2012 MacBook Pros where the IRQ
> > is shared with 3 other devices (Light Ridge Thunderbolt controller, SDXC
> > reader, HDA card on discrete GPU). As soon as an interrupt handler is
> > installed for one of these devices, the ensuing storm of spurious IRQs
> > causes the kernel to disable the IRQ and switch to polling. This lasts
> > until the b43 driver loads and resets the device.
> > 
> > Loading the b43 driver first is not always an option, in particular with
> > the Light Ridge Thunderbolt controller: The PCI hotplug IRQ handler gets
> > installed early on because it is built in, unlike b43 which is usually
> > a module.
> > 
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
> > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632
> > Tested-by: Lukas Wunner  [MacBookPro9,1]
> > Signed-off-by: Lukas Wunner 
> > ---
> >  drivers/bcma/bcma_private.h |  2 --
> >  drivers/pci/quirks.c| 27 +++
> >  include/linux/bcma/bcma.h   |  1 +
> >  3 files changed, 28 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
> > index eda0909..f642c42 100644
> > --- a/drivers/bcma/bcma_private.h
> > +++ b/drivers/bcma/bcma_private.h
> > @@ -8,8 +8,6 @@
> >  #include 
> >  #include 
> >  
> > -#define BCMA_CORE_SIZE 0x1000
> > -
> >  #define bcma_err(bus, fmt, ...) \
> > pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
> >  #define bcma_warn(bus, fmt, ...) \
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 8e67802..d4fb5ee 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -25,6 +25,8 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include/* isa_dma_bridge_buggy */
> >  #include "pci.h"
> >  
> > @@ -3282,6 +3284,31 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL, 
> > 0x156d,
> >quirk_apple_wait_for_thunderbolt);
> >  #endif
> >  
> > +/*
> > + * Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > + * on boot until they are reset, causing spurious interrupts if the IRQ is
> > + * shared. Apparently the EFI bootloader enables the device to phone home
> > + * to Cupertino and does not disable it before passing control to the OS.
> > + */
> > +static void quirk_apple_b43_reset(struct pci_dev *dev)
> > +{
> > +   void __iomem *mmio;
> > +
> > +   if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc.") || !dev->bus->self ||
> > +   pci_pcie_type(dev->bus->self) != PCI_EXP_TYPE_ROOT_PORT)
> > +   return;
> > +
> > +   mmio = pci_iomap(dev, 0, 0);
> > +   if (!mmio) {
> > +   pr_err("b43 quirk: Cannot iomap device, IRQ storm ahead\n");
> > +   return;
> > +   }
> > +   iowrite32(BCMA_RESET_CTL_RESET,
> > + mmio + (1 * BCMA_CORE_SIZE) + BCMA_RESET_CTL);
> > +   pci_iounmap(dev, mmio);
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=111781 and
> https://mjg59.dreamwidth.org/11235.html describe a sort of similar
> issue, but with DMA.  An interrupt from the device is probably to
> signal a DMA completion, but these problem reports only mention the
> "IRQ nobody cared" issue; I don't see anything about memory
> corruption.
> 
> If this resets the device, I guess that should prevent future DMA as
> well as future interrupts.  Would pci_reset_function() do the same
> thing in a more generic way?

Matthew's grub patch puts the wireless card into D3hot, which does not
stop the interrupts. I have tested this. (It may stop the DMA, I cannot
verify that as it doesn't occur on my machine for lack of an access point.)

Calling pci_reset_function() does stop the interrupts. More specifically,
clearing the command register in pci_dev_save_and_disable() does. However
the b43/bcma driver subsequently locks up the machine on module load.
The closed source broadcom-sta driver loads fine. (I'm not even sure it's
worth fixing this anomalous use case in b43/bcma.)

It should be noted that clearing the command register 

ietf proposed standards for dscp to 802.11e mappings

[Dave Taht]

If anyone cares

https://datatracker.ietf.org/doc/draft-szigeti-tsvwg-ieee-802-11/?include_text=1

these will be discussed on the tsvwg mailing list.

https://www.ietf.org/mailman/listinfo/tsvwg

There are several things in here I object to - no discussion of
multicast, and arbitrarily dropping CS6 and CS7 on APs, but I figure
that will be sorted out eventually.

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

RE: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)

[Machani, Yaniv]

On Wed, Apr 06, 2016 at 22:07:39, Kalle Valo wrote:
> 
> > More than that, wl1251 family is not officially supported via the 
> > mainline Linux.
> 
> I guess you mean not officially supported by TI? Because wl1251 driver 
> has been in mainline for ages and reportedly working.
>   
Correct.

Yaniv 
> --
> Kalle Valo


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

Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)

[Kalle Valo]

"Machani, Yaniv"  writes:

> More than that, wl1251 family is not officially supported via the
> mainline Linux.

I guess you mean not officially supported by TI? Because wl1251 driver
has been in mainline for ages and reportedly working.

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

Re: [v2] mwifiex: advertise low priority scan feature

[Kalle Valo]

> From: Amitkumar Karwar 
> 
> Low priority scan handling code which delays or aborts scan
> operation based on Tx traffic is removed recently. The reason
> is firmware already takes care of it in our new feature scan
> channel gap. Hence we should advertise low priority scan
> support to cfg80211.
> 
> This patch fixes a problem in which OBSS scan request from
> wpa_supplicant was being rejected by cfg80211.
> 
> Signed-off-by: Amitkumar Karwar 
> Signed-off-by: Wei-Ning Huang 
> Tested-by: Wei-Ning Huang 
> Acked-by: Amitkumar Karwar 

Thanks, applied to wireless-drivers-next.git.

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

Re: brcmfmac: sdio: remove unused variable retry_limit

[Kalle Valo]

> From: Colin Ian King 
> 
> retry_limit has never been used during the life of this driver, so
> we may as well remove it as it is redundant.
> 
> Signed-off-by: Colin Ian King 
> Reviewed-by: Julian Calaby 

Thanks, applied to wireless-drivers-next.git.

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

Re: [v6] rt2x00usb: Use usb anchor to manage URB

[Kalle Valo]

> With current driver, it is observed that a URB is not
> completed while the USB disconnect is initiated. Due to
> that, the URB completion handler is trying to access
> the resource which was freed as a part of USB disconnect.
> Managing the URBs with anchor will make sure that all
> the URBs are handled gracefully before device gets
> disconnected.
> 
> Signed-off-by: Vishal Thanki 
> Acked-by: Stanislaw Gruszka 

Thanks, applied to wireless-drivers-next.git.

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

Re: wl12xx: remove redundant null check on wl->scan.ssid

[Kalle Valo]

> From: Colin Ian King 
> 
> ssid is an array of u8, so it can never be null, so the null check on
> wl->scan.ssid is redundant and can be removed.
> 
> Signed-off-by: Colin Ian King 

Thanks, applied to wireless-drivers-next.git.

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

Re: rtlwifi: btcoexist: Convert BTC_PRINTK to btc__dbg

[Kalle Valo]

> Use a more common logging style.
> 
> Miscellanea:
> 
> o Add specific logging macros for ALGORITHM and INTERFACE types
> o Output the messages at KERN_DEBUG
> o Coalesce formats
> o Align arguments
> o Whitespace style adjustments for only these changes
> 
> Signed-off-by: Joe Perches 

Thanks, applied to wireless-drivers-next.git. There were some conflicts but
3-way merge was able to fix them. Please double check still.

Applying: rtlwifi: btcoexist: Convert BTC_PRINTK to btc__dbg
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
Auto-merging drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c
Auto-merging drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c

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

Re: [1/9] rtlwifi: Fix Smatch warnings

[Kalle Valo]

> Smatch reports the following:
> 
>   CHECK   drivers/net/wireless/realtek/rtlwifi/pci.c
> drivers/net/wireless/realtek/rtlwifi/pci.c:366 rtl_pci_check_buddy_priv() 
> error: we previously assumed 'tpriv' could be null (see line 368)
> drivers/net/wireless/realtek/rtlwifi/pci.c:1216 _rtl_pci_init_struct() warn: 
> inconsistent indenting
> 
> Signed-off-by: Larry Finger 

Thanks, 9 patches applied to wireless-drivers-next.git:

37c52934c668 rtlwifi: Fix Smatch warnings
2e074fab347e rtlwifi: btcoexist: Fix Smatch warning
844026f609fc rtlwifi: rtl8188ee: Fix Smatch warnings
de8a9a6eeb57 rtlwifi: rtl8192c-common: Fix Smatch warning
05d9e1bba43b rtlwifi: rtl8192ee: Fix Smatch warning
c42ceccec170 rtlwifi: rtl8192se: Fix Smatch warning
154fb486df3d rtlwifi: rtl8723ae: Fix Smatch warning
b3c4201bce5e rtlwifi: rtl8723be: Fix Smatch warnings
1e812458206e rtlwifi: rtl8821ae: Fix Smatch warnings

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

Re: [1/2] rtlwifi: rtl8723be: Add antenna select module parameter

[Kalle Valo]

> A number of new laptops have been delivered with only a single antenna.
> In principle, this is OK; however, a problem arises when the on-board
> EEPROM is programmed to use the other antenna connection. The option
> of opening the computer and moving the connector is not always possible
> as it will void the warranty in some cases. In addition, this solution
> breaks the Windows driver when the box dual boots Linux and Windows.
> 
> A fix involving a new module parameter has been developed.  This commit
> adds the new parameter and implements the changes needed for the driver.
> 
> Signed-off-by: Larry Finger 
> Cc: Stable  [V4.0+]

Thanks, 2 patches applied to wireless-drivers-next.git:

c18d8f509571 rtlwifi: rtl8723be: Add antenna select module parameter
baa170229095 rtlwifi: btcoexist: Implement antenna selection

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

Re: brcmfmac: uninitialized "ret" variable

[Kalle Valo]

> There is an error path where "ret" isn't initialized.
> 
> Signed-off-by: Dan Carpenter 

Thanks, applied to wireless-drivers-next.git.

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

Re: [PATCH] mwifiex: bridged packets cause wmm_tx_pending counter to go negative

[Kalle Valo]

Marty Faltesek  writes:

> When a packet is queued from the bridge, wmm_tx_pending is not
> incremented, but when the packet is dequeued the counter is decremented.

Signed-off-by line is missing, please resend.

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

Re: [PATCH] mwifiex: fw download does not release sdio bus during failure

[Kalle Valo]

Marty Faltesek  writes:

> ---
>  drivers/net/wireless/marvell/mwifiex/sdio.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Signed-off-by line is missing, please resend.

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

Re: [trivial] mwifiex: Spelling s/minmum/minimum/, s/bandwidth/bandwith/

[Kalle Valo]

> Signed-off-by: Geert Uytterhoeven 

Thanks, applied to wireless-drivers-next.git.

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

Re: [PATCH] mwifiex: transmit packet stats incorrect.

[Kalle Valo]

Marty Faltesek  writes:

> tx_packets counter is incremented for aggregated packets, when it had
> already been incremented for the aggregated packet's constituent
> parts. Removing the extra count.

Because Signed-off-by line is missing I can't take this. Please resend.

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

Re: [PATCH] ath9k: fix rng high cpu load

[Kalle Valo]

miaoq...@codeaurora.org writes:

> From: Miaoqing Pan 
>
> If no valid ADC randomness output, ath9k rng will continuously
> reading ADC, which will cause high CPU load. So increase the
> delay to wait for ADC ready.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=114261
> Signed-off-by: Miaoqing Pan 

Applied, thanks.

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

Re: pull request: iwlwifi-next 2016-03-30

[Kalle Valo]

"Grumbach, Emmanuel"  writes:

> This is a pull request for 4.7. Lots of work here and more to come when
> dependencies on mac80211 will be resolved.
> Let me know if you have issues!
>
> Thanks.
>
> The following changes since commit 1200b6809dfd9d73bc4c7db76d288c35fa4b2ebe:
>
>   Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next 
> (2016-03-19 10:05:34 -0700)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next.git 
> tags/iwlwifi-next-for-kalle-2016-03-30

Pulled, thanks.

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

Re: [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Dave Taht]

On Wed, Apr 6, 2016 at 12:21 AM, Johannes Berg
 wrote:
> [removing other lists since they spam me with moderation bounces]

I have added your email address be accepted to the codel,
make-wifi-fast lists. My apologies for the bounces.

The people on those lists generally do not have the time to tackle the
volume of traffic on linux-wireless.

>> The hope had been the original codel.h would have been reusable,
>> which is not the case at present.
>
> So what's the strategy for making it happen?

Strategy? to meander towards a result that gives low latency to all
stations, no matter their bandwidth, on several chipsets.

The holy grail from my viewpoint is to get airtime fairness, better
mac utilization, slow stations not starving fast ones, more stations
servicable, and so on, and my focus has generally been on having an
architecture that applied equally to APs and clients. Getting clients
alone to have a queuing latency reduction of these orders of magnitude
on uploads at low rates would be a huge win, but not the holy grail.

It was really nice to have michal's proof of concept(s) show up and
show fq_codel-like benefits at both low and high speeds on wifi, but
it is clear more architectural rework is required to fit the theory
into the reality.

> Unless there is one, I
> don't see the point in making the code more complicated than it already
> has to be anyway.

+1.

Next steps were - get toke's and my testbeds up - avery/tim/myself to
keep hammering at the ath9k - michal exploring dql - jonathon poking
at it with cake-like ideas - and anyone else that cares to join in on
finally fixing bufferbloat on wifi.

and maybe put together a videoconference in 2-3 weeks or so with where
we are stuck at (felix will be off vacation, too, I think). There are
still multiple points where we all talk past each other.

Me, for example, am overly fixated on having a per station queue to
start with (which in the case of a client is two stations - one
multicast/mgtmt frames and regular traffic) and not dealing with tids
until much later in the process. Unfortunately it seems the hook is
very late in the process.
>
> johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Make-wifi-fast] [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Jonathan Morton]

> On 6 Apr, 2016, at 10:16, Michal Kazior  wrote:
> 
> When a driver asks mac80211 to dequeue given txq it implies a
> destination station as well. This is important because 802.11
> aggregation can be performed only on groups of packets going to a
> single station on a single tid.
> 
> Cake - as I understand it - doesn't really *guarantee* maintaining
> this. Keep in mind you can run with hundreds of stations connected.
> 
> You don't really want to burden drivers with sorting this grouping up
> themselves (and hence coerce them into introducing another level of
> intermediate queues, bis).

Well, no.  Cake isn’t designed to maintain per-station queues explicitly, 
though it does have support for stochastic fairness between hosts.  It is also 
blissfuly unaware of the requirements of wifi aggregation, largely because the 
standard qdisc interface is likewise ignorant.  I’m therefore not suggesting 
that you use Cake as-is.

What I’m pointing at instead is the set-associative hash, which could easily be 
tweaked to put greater emphasis on avoiding putting multiple stations’ traffic 
in one queue, while maintaining the performance benefits of a fixed queue pool 
indexed by a hash table, and an extended operating region in which flow 
isolation is maintained.  You can then have a linked-list of queues assigned to 
a particular station, so that when a packet for a particular station is 
requested, you can easily locate one.

I hadn’t appreciated, though, that the TXQ struct was station-specific.  This 
wasn’t obvious from the code fragments posted, so it looked like packets that 
incurred hash collisions would be dumped into a single overflow queue.

 - Jonathan Morton

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

Re: [PATCH v2] brcmfmac: Don't increase 8021x_cnt for dropped packets

[Per Förlin]

Hi,

Thanks for getting back to me,

* Chip: chip 43340 rev 2 pmurev 20
* I'm running kernel 4.1 (backports) for the wireless parts
* I have not to setup my environment for the latest kernel yet.
  It's on my todo list.

Looking at the code you pointed out I realize my fix i misplaced.
It's strange that I don't end up with a negative count since
the counter would be decreased twice, with my patch in place.
I took a closer look at the execution flow and made the following
observation:

The eh->h_proto has changed when reaching the txfinalize() function.
This only happens in case of packet drop.

Simplified execution flow.

brcmf_fws_process_skb():
  # At this point ntohs(eh->h_proto) == 0x888E (ETH_P_PAE)
  rc = brcmf_proto_txdata() ->
brcmf_proto_bcdc_txdata() ->
  brcmf_sdio_bus_txdata()
  # At this point it has changed to: ntohs(eh->h_proto) == 0x8939 (?)
  if (rc < 0)
# When executing this function the cnt will not be decreased due to
# eh->h_proto being changed.
brcmf_txfinalize()

I need to continue my investigation to find out why the h_proto got changed.
I assume this is not an expected behavior?

BR
Per


2016-04-06 10:22 GMT+02:00 Arend Van Spriel :
> On 5-4-2016 22:01, per.for...@gmail.com wrote:
>> From: Per Forlin 
>>
>> The pend_8021x_cnt gets into a state where it's not being decreased.
>> When the brcmf_netdev_wait_pend8021x is called it will timeout
>> because there are no pending packets to be consumed. There is no
>> easy way of getting out of this state once it has happened.
>> Preventing the counter from being increased in case of dropped
>> packets seem like a reasonable solution.
>>
>> Log showing overflow txq and increased cnt:
>>
>> // Extra debug prints
>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2
>>
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>
>> // Extra debug prints
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4
>>
>> WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
>>   (warn_slowpath_common)
>>   (warn_slowpath_null)
>>   (brcmf_netdev_wait_pend8021x [brcmfmac])
>>   (send_key_to_dongle [brcmfmac])
>>   (brcmf_cfg80211_del_key [brcmfmac])
>>   (nl80211_del_key [cfg80211])
>>   (genl_rcv_msg)
>>   (netlink_rcv_skb)
>>   (genl_rcv)
>>   (netlink_unicast)
>>   (netlink_sendmsg)
>>   (sock_sendmsg)
>>   (___sys_sendmsg)
>>   (__sys_sendmsg)
>>   (SyS_sendmsg)
>>
>> Signed-off-by: Per Forlin 
>> ---
>> I came across this bug in an older kernel but it seems relevant
>> for the latest kernel as well. I'm not familiar with the code
>> but I can reproduce the issue and verify a fix for it.
>> With this patch I no longer get stuck with a pend8021_cnt > 0.
>>
>> Change log:
>>  v2 - Add variable to know whether the counter is increased or not
>
> Sorry for the late response. Can you elaborate where you are seeing this.
>
> What kind of chipset are you using?
> What kernel version are you running?
>
> The function brcmf_fws_process_skb() should call brcmf_txfinalize() in
> case of an error and it does in latest kernel. There the count is
> decreased. We had an issue mapping to the right ifp as reported by
> Rafał, but that has also been fixed recently. So main question here:
>
> Do you see the issue in the latest kernel?
>
> Regards,
> Arend
>
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> index ed9998b..de80ad8 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> @@ -196,6 +196,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct 
>> sk_buff *skb,
>>   struct brcmf_if *ifp = netdev_priv(ndev);
>>   struct brcmf_pub *drvr = ifp->drvr;
>>   struct ethhdr *eh = (struct ethhdr *)(skb->data);
>> + bool pend_8021x_cnt_increased = false;
>>
>>   brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>>
>> @@ -233,14 +234,18 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct 
>> sk_buff 

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Michael Büsch]

On Wed, 6 Apr 2016 08:31:51 -0500
Bjorn Helgaas  wrote:

> Even for interrupts, it's only a 90% solution because we do still get
> a few interrupts before the quirk runs.  There may not be enough to
> trigger the "IRQ nobody cared" check,

If no driver requested the shared interrupt yet, it should be disabled
in the interrupt controller. So the interrupts do not reach the CPU.
The interrupt storm (on the CPU) starts as soon as some driver
that shares the interrupt with b43 requests and thus enables the
interrupt. And that always happens after the PCI fixup. Thus this is
safe.

-- 
Michael


pgp8pqESvQDtE.pgp
Description: OpenPGP digital signature

[PATCH 6/8] ath10k: move htt_op_version to struct ath10k_fw_file

[Kalle Valo]

Preparation for testmode.c to use ath10k_core_fetch_board_data_api_n().

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |   12 ++--
 drivers/net/wireless/ath/ath10k/core.h |1 +
 drivers/net/wireless/ath/ath10k/debug.c|2 +-
 drivers/net/wireless/ath/ath10k/htt.c  |2 +-
 drivers/net/wireless/ath/ath10k/htt.h  |1 -
 drivers/net/wireless/ath/ath10k/mac.c  |2 +-
 drivers/net/wireless/ath/ath10k/testmode.c |1 +
 7 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 6d995219a497..4b1fc4b99205 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1117,10 +1117,10 @@ static int ath10k_core_fetch_firmware_api_n(struct 
ath10k *ar, const char *name,
 
version = (__le32 *)data;
 
-   ar->htt.op_version = le32_to_cpup(version);
+   fw_file->htt_op_version = le32_to_cpup(version);
 
ath10k_dbg(ar, ATH10K_DBG_BOOT, "found fw ie htt op 
version %d\n",
-  ar->htt.op_version);
+  fw_file->htt_op_version);
break;
case ATH10K_FW_IE_FW_CODE_SWAP_IMAGE:
ath10k_dbg(ar, ATH10K_DBG_BOOT,
@@ -1569,18 +1569,18 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
/* Backwards compatibility for firmwares without
 * ATH10K_FW_IE_HTT_OP_VERSION.
 */
-   if (ar->htt.op_version == ATH10K_FW_HTT_OP_VERSION_UNSET) {
+   if (fw_file->htt_op_version == ATH10K_FW_HTT_OP_VERSION_UNSET) {
switch (fw_file->wmi_op_version) {
case ATH10K_FW_WMI_OP_VERSION_MAIN:
-   ar->htt.op_version = ATH10K_FW_HTT_OP_VERSION_MAIN;
+   fw_file->htt_op_version = ATH10K_FW_HTT_OP_VERSION_MAIN;
break;
case ATH10K_FW_WMI_OP_VERSION_10_1:
case ATH10K_FW_WMI_OP_VERSION_10_2:
case ATH10K_FW_WMI_OP_VERSION_10_2_4:
-   ar->htt.op_version = ATH10K_FW_HTT_OP_VERSION_10_1;
+   fw_file->htt_op_version = ATH10K_FW_HTT_OP_VERSION_10_1;
break;
case ATH10K_FW_WMI_OP_VERSION_TLV:
-   ar->htt.op_version = ATH10K_FW_HTT_OP_VERSION_TLV;
+   fw_file->htt_op_version = ATH10K_FW_HTT_OP_VERSION_TLV;
break;
case ATH10K_FW_WMI_OP_VERSION_10_4:
case ATH10K_FW_WMI_OP_VERSION_UNSET:
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 640733d6d7cc..b779e059a9c3 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -634,6 +634,7 @@ struct ath10k_fw_file {
DECLARE_BITMAP(fw_features, ATH10K_FW_FEATURE_COUNT);
 
enum ath10k_fw_wmi_op_version wmi_op_version;
+   enum ath10k_fw_htt_op_version htt_op_version;
 
const void *firmware_data;
size_t firmware_len;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index f9e9a83eb7d0..bcf6da2ffe95 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -179,7 +179,7 @@ void ath10k_debug_print_boot_info(struct ath10k *ar)
ar->htt.target_version_major,
ar->htt.target_version_minor,
ar->normal_mode_fw.fw_file.wmi_op_version,
-   ar->htt.op_version,
+   ar->normal_mode_fw.fw_file.htt_op_version,
ath10k_cal_mode_str(ar->cal_mode),
ar->max_num_stations,
test_bit(ATH10K_FLAG_RAW_MODE, >dev_flags),
diff --git a/drivers/net/wireless/ath/ath10k/htt.c 
b/drivers/net/wireless/ath/ath10k/htt.c
index ee79512b1fcc..130cd9502021 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -183,7 +183,7 @@ int ath10k_htt_init(struct ath10k *ar)
8 + /* llc snap */
2; /* ip4 dscp or ip6 priority */
 
-   switch (ar->htt.op_version) {
+   switch (ar->running_fw->fw_file.htt_op_version) {
case ATH10K_FW_HTT_OP_VERSION_10_4:
ar->htt.t2h_msg_types = htt_10_4_t2h_msg_types;
ar->htt.t2h_msg_types_max = HTT_10_4_T2H_NUM_MSGS;
diff --git a/drivers/net/wireless/ath/ath10k/htt.h 
b/drivers/net/wireless/ath/ath10k/htt.h
index ee7c8f8f8073..911c535d0863 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1562,7 +1562,6 @@ struct ath10k_htt {
u8 target_version_major;
u8 target_version_minor;
struct completion 

[PATCH 5/8] ath10k: move wmi_op_version to struct ath10k_fw_file

[Kalle Valo]

Preparation for testmode.c to use ath10k_core_fetch_board_data_api_n().

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |   20 ++--
 drivers/net/wireless/ath/ath10k/core.h |5 ++---
 drivers/net/wireless/ath/ath10k/debug.c|2 +-
 drivers/net/wireless/ath/ath10k/mac.c  |2 +-
 drivers/net/wireless/ath/ath10k/testmode.c |   17 ++---
 drivers/net/wireless/ath/ath10k/wmi.c  |4 ++--
 6 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index bb8c8b2f785b..6d995219a497 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1106,10 +1106,10 @@ static int ath10k_core_fetch_firmware_api_n(struct 
ath10k *ar, const char *name,
 
version = (__le32 *)data;
 
-   ar->wmi.op_version = le32_to_cpup(version);
+   fw_file->wmi_op_version = le32_to_cpup(version);
 
ath10k_dbg(ar, ATH10K_DBG_BOOT, "found fw ie wmi op 
version %d\n",
-  ar->wmi.op_version);
+  fw_file->wmi_op_version);
break;
case ATH10K_FW_IE_HTT_OP_VERSION:
if (ie_len != sizeof(u32))
@@ -1438,9 +1438,9 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
return -EINVAL;
}
 
-   if (ar->wmi.op_version >= ATH10K_FW_WMI_OP_VERSION_MAX) {
+   if (fw_file->wmi_op_version >= ATH10K_FW_WMI_OP_VERSION_MAX) {
ath10k_err(ar, "unsupported WMI OP version (max %d): %d\n",
-  ATH10K_FW_WMI_OP_VERSION_MAX, ar->wmi.op_version);
+  ATH10K_FW_WMI_OP_VERSION_MAX, 
fw_file->wmi_op_version);
return -EINVAL;
}
 
@@ -1496,19 +1496,19 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
/* Backwards compatibility for firmwares without
 * ATH10K_FW_IE_WMI_OP_VERSION.
 */
-   if (ar->wmi.op_version == ATH10K_FW_WMI_OP_VERSION_UNSET) {
+   if (fw_file->wmi_op_version == ATH10K_FW_WMI_OP_VERSION_UNSET) {
if (test_bit(ATH10K_FW_FEATURE_WMI_10X, fw_file->fw_features)) {
if (test_bit(ATH10K_FW_FEATURE_WMI_10_2,
 fw_file->fw_features))
-   ar->wmi.op_version = 
ATH10K_FW_WMI_OP_VERSION_10_2;
+   fw_file->wmi_op_version = 
ATH10K_FW_WMI_OP_VERSION_10_2;
else
-   ar->wmi.op_version = 
ATH10K_FW_WMI_OP_VERSION_10_1;
+   fw_file->wmi_op_version = 
ATH10K_FW_WMI_OP_VERSION_10_1;
} else {
-   ar->wmi.op_version = ATH10K_FW_WMI_OP_VERSION_MAIN;
+   fw_file->wmi_op_version = ATH10K_FW_WMI_OP_VERSION_MAIN;
}
}
 
-   switch (ar->wmi.op_version) {
+   switch (fw_file->wmi_op_version) {
case ATH10K_FW_WMI_OP_VERSION_MAIN:
ar->max_num_peers = TARGET_NUM_PEERS;
ar->max_num_stations = TARGET_NUM_STATIONS;
@@ -1570,7 +1570,7 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
 * ATH10K_FW_IE_HTT_OP_VERSION.
 */
if (ar->htt.op_version == ATH10K_FW_HTT_OP_VERSION_UNSET) {
-   switch (ar->wmi.op_version) {
+   switch (fw_file->wmi_op_version) {
case ATH10K_FW_WMI_OP_VERSION_MAIN:
ar->htt.op_version = ATH10K_FW_HTT_OP_VERSION_MAIN;
break;
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 649fb940eb9f..640733d6d7cc 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -139,7 +139,6 @@ struct ath10k_mem_chunk {
 };
 
 struct ath10k_wmi {
-   enum ath10k_fw_wmi_op_version op_version;
enum ath10k_htc_ep_id eid;
struct completion service_ready;
struct completion unified_ready;
@@ -634,6 +633,8 @@ struct ath10k_fw_file {
 
DECLARE_BITMAP(fw_features, ATH10K_FW_FEATURE_COUNT);
 
+   enum ath10k_fw_wmi_op_version wmi_op_version;
+
const void *firmware_data;
size_t firmware_len;
 
@@ -894,8 +895,6 @@ struct ath10k {
struct {
/* protected by conf_mutex */
struct ath10k_fw_components utf_mode_fw;
-   enum ath10k_fw_wmi_op_version orig_wmi_op_version;
-   enum ath10k_fw_wmi_op_version op_version;
 
/* protected by data_lock */
bool utf_monitor;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index 07b15ffce220..f9e9a83eb7d0 100644

[PATCH 7/8] ath10k: switch testmode to use ath10k_core_fetch_firmware_api_n()

[Kalle Valo]

Now that all firmware-N.bin related are within struct ath10k_fw_file we can
switch to use ath10k_core_fetch_firmware_api_n() and delete almost identical
ath10k_tm_fetch_utf_firmware_api_2().

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |4 -
 drivers/net/wireless/ath/ath10k/core.h |2 
 drivers/net/wireless/ath/ath10k/testmode.c |  124 
 3 files changed, 6 insertions(+), 124 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 4b1fc4b99205..d0cecab5aaec 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -976,8 +976,8 @@ success:
return 0;
 }
 
-static int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char 
*name,
-   struct ath10k_fw_file *fw_file)
+int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char *name,
+struct ath10k_fw_file *fw_file)
 {
size_t magic_len, len, ie_len;
int ie_id, i, index, bit, ret;
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index b779e059a9c3..c18043fd0093 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -932,6 +932,8 @@ void ath10k_core_destroy(struct ath10k *ar);
 void ath10k_core_get_fw_features_str(struct ath10k *ar,
 char *buf,
 size_t max_len);
+int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, const char *name,
+struct ath10k_fw_file *fw_file);
 
 int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
  const struct ath10k_fw_components *fw_components);
diff --git a/drivers/net/wireless/ath/ath10k/testmode.c 
b/drivers/net/wireless/ath/ath10k/testmode.c
index 2d9a3f56213f..81fe266f7990 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -139,127 +139,6 @@ static int ath10k_tm_cmd_get_version(struct ath10k *ar, 
struct nlattr *tb[])
return cfg80211_testmode_reply(skb);
 }
 
-static int ath10k_tm_fetch_utf_firmware_api_2(struct ath10k *ar,
- struct ath10k_fw_file *fw_file)
-{
-   size_t len, magic_len, ie_len;
-   struct ath10k_fw_ie *hdr;
-   char filename[100];
-   __le32 *version;
-   const u8 *data;
-   int ie_id, ret;
-
-   snprintf(filename, sizeof(filename), "%s/%s",
-ar->hw_params.fw.dir, ATH10K_FW_UTF_API2_FILE);
-
-   /* load utf firmware image */
-   ret = request_firmware(_file->firmware, filename, ar->dev);
-   if (ret) {
-   ath10k_warn(ar, "failed to retrieve utf firmware '%s': %d\n",
-   filename, ret);
-   return ret;
-   }
-
-   data = fw_file->firmware->data;
-   len = fw_file->firmware->size;
-
-   /* FIXME: call release_firmware() in error cases */
-
-   /* magic also includes the null byte, check that as well */
-   magic_len = strlen(ATH10K_FIRMWARE_MAGIC) + 1;
-
-   if (len < magic_len) {
-   ath10k_err(ar, "utf firmware file is too small to contain 
magic\n");
-   ret = -EINVAL;
-   goto err;
-   }
-
-   if (memcmp(data, ATH10K_FIRMWARE_MAGIC, magic_len) != 0) {
-   ath10k_err(ar, "invalid firmware magic\n");
-   ret = -EINVAL;
-   goto err;
-   }
-
-   /* jump over the padding */
-   magic_len = ALIGN(magic_len, 4);
-
-   len -= magic_len;
-   data += magic_len;
-
-   /* loop elements */
-   while (len > sizeof(struct ath10k_fw_ie)) {
-   hdr = (struct ath10k_fw_ie *)data;
-
-   ie_id = le32_to_cpu(hdr->id);
-   ie_len = le32_to_cpu(hdr->len);
-
-   len -= sizeof(*hdr);
-   data += sizeof(*hdr);
-
-   if (len < ie_len) {
-   ath10k_err(ar, "invalid length for FW IE %d (%zu < 
%zu)\n",
-  ie_id, len, ie_len);
-   ret = -EINVAL;
-   goto err;
-   }
-
-   switch (ie_id) {
-   case ATH10K_FW_IE_FW_VERSION:
-   if (ie_len > sizeof(fw_file->fw_version) - 1)
-   break;
-
-   memcpy(fw_file->fw_version, data, ie_len);
-   fw_file->fw_version[ie_len] = '\0';
-
-   ath10k_dbg(ar, ATH10K_DBG_TESTMODE,
-  "testmode found fw utf version %s\n",
-  fw_file->fw_version);
-   break;
-   case ATH10K_FW_IE_TIMESTAMP:
-   /* ignore timestamp, 

[PATCH 8/8] ath10k: remove enum ath10k_swap_code_seg_bin_type

[Kalle Valo]

It's not needed for anything so just get rid of it.

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |3 +--
 drivers/net/wireless/ath/ath10k/swap.c |   22 ++
 drivers/net/wireless/ath/ath10k/swap.h |9 +
 3 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index d0cecab5aaec..55f3a80b2884 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -647,8 +647,7 @@ static int ath10k_download_fw(struct ath10k *ar)
data = ar->running_fw->fw_file.firmware_data;
data_len = ar->running_fw->fw_file.firmware_len;
 
-   ret = ath10k_swap_code_seg_configure(ar,
-ATH10K_SWAP_CODE_SEG_BIN_TYPE_FW);
+   ret = ath10k_swap_code_seg_configure(ar);
if (ret) {
ath10k_err(ar, "failed to configure fw code swap: %d\n",
   ret);
diff --git a/drivers/net/wireless/ath/ath10k/swap.c 
b/drivers/net/wireless/ath/ath10k/swap.c
index 8d6dc86799f5..af447a60143b 100644
--- a/drivers/net/wireless/ath/ath10k/swap.c
+++ b/drivers/net/wireless/ath/ath10k/swap.c
@@ -134,27 +134,17 @@ ath10k_swap_code_seg_alloc(struct ath10k *ar, size_t 
swap_bin_len)
return seg_info;
 }
 
-int ath10k_swap_code_seg_configure(struct ath10k *ar,
-  enum ath10k_swap_code_seg_bin_type type)
+int ath10k_swap_code_seg_configure(struct ath10k *ar)
 {
int ret;
struct ath10k_swap_code_seg_info *seg_info = NULL;
 
-   switch (type) {
-   case ATH10K_SWAP_CODE_SEG_BIN_TYPE_FW:
-   if (!ar->swap.firmware_swap_code_seg_info)
-   return 0;
-
-   ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot found firmware code swap 
binary\n");
-   seg_info = ar->swap.firmware_swap_code_seg_info;
-   break;
-   default:
-   case ATH10K_SWAP_CODE_SEG_BIN_TYPE_OTP:
-   case ATH10K_SWAP_CODE_SEG_BIN_TYPE_UTF:
-   ath10k_warn(ar, "ignoring unknown code swap binary type %d\n",
-   type);
+   if (!ar->swap.firmware_swap_code_seg_info)
return 0;
-   }
+
+   ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot found firmware code swap 
binary\n");
+
+   seg_info = ar->swap.firmware_swap_code_seg_info;
 
ret = ath10k_bmi_write_memory(ar, seg_info->target_addr,
  _info->seg_hw_info,
diff --git a/drivers/net/wireless/ath/ath10k/swap.h 
b/drivers/net/wireless/ath/ath10k/swap.h
index 5c89952dd20f..36991c7b07a0 100644
--- a/drivers/net/wireless/ath/ath10k/swap.h
+++ b/drivers/net/wireless/ath/ath10k/swap.h
@@ -39,12 +39,6 @@ union ath10k_swap_code_seg_item {
struct ath10k_swap_code_seg_tail tail;
 } __packed;
 
-enum ath10k_swap_code_seg_bin_type {
-ATH10K_SWAP_CODE_SEG_BIN_TYPE_OTP,
-ATH10K_SWAP_CODE_SEG_BIN_TYPE_FW,
-ATH10K_SWAP_CODE_SEG_BIN_TYPE_UTF,
-};
-
 struct ath10k_swap_code_seg_hw_info {
/* Swap binary image size */
__le32 swap_size;
@@ -64,8 +58,7 @@ struct ath10k_swap_code_seg_info {
dma_addr_t paddr[ATH10K_SWAP_CODE_SEG_NUM_SUPPORTED];
 };
 
-int ath10k_swap_code_seg_configure(struct ath10k *ar,
-  enum ath10k_swap_code_seg_bin_type type);
+int ath10k_swap_code_seg_configure(struct ath10k *ar);
 void ath10k_swap_code_seg_release(struct ath10k *ar);
 int ath10k_swap_code_seg_init(struct ath10k *ar);
 

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

[PATCH 3/8] ath10k: move fw_version inside struct ath10k_fw_file

[Kalle Valo]

Preparation for testmode.c to use ath10k_core_fetch_board_data_api_n().

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |   13 +
 drivers/net/wireless/ath/ath10k/core.h |3 ++-
 drivers/net/wireless/ath/ath10k/testmode.c |   12 ++--
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index c4a36661cb17..9f783d35d9f5 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1039,15 +1039,15 @@ static int ath10k_core_fetch_firmware_api_n(struct 
ath10k *ar, const char *name,
 
switch (ie_id) {
case ATH10K_FW_IE_FW_VERSION:
-   if (ie_len > sizeof(ar->hw->wiphy->fw_version) - 1)
+   if (ie_len > sizeof(fw_file->fw_version) - 1)
break;
 
-   memcpy(ar->hw->wiphy->fw_version, data, ie_len);
-   ar->hw->wiphy->fw_version[ie_len] = '\0';
+   memcpy(fw_file->fw_version, data, ie_len);
+   fw_file->fw_version[ie_len] = '\0';
 
ath10k_dbg(ar, ATH10K_DBG_BOOT,
   "found fw version %s\n",
-   ar->hw->wiphy->fw_version);
+   fw_file->fw_version);
break;
case ATH10K_FW_IE_TIMESTAMP:
if (ie_len != sizeof(u32))
@@ -1867,6 +1867,11 @@ static int ath10k_core_probe_fw(struct ath10k *ar)
goto err_power_down;
}
 
+   BUILD_BUG_ON(sizeof(ar->hw->wiphy->fw_version) !=
+sizeof(ar->normal_mode_fw.fw_file.fw_version));
+   memcpy(ar->hw->wiphy->fw_version, ar->normal_mode_fw.fw_file.fw_version,
+  sizeof(ar->hw->wiphy->fw_version));
+
ath10k_debug_print_hwfw_info(ar);
 
ret = ath10k_core_pre_cal_download(ar);
diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index ff3d92d7dc8d..3626069a4fd6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -630,6 +630,8 @@ enum ath10k_tx_pause_reason {
 struct ath10k_fw_file {
const struct firmware *firmware;
 
+   char fw_version[ETHTOOL_FWVERS_LEN];
+
const void *firmware_data;
size_t firmware_len;
 
@@ -892,7 +894,6 @@ struct ath10k {
struct {
/* protected by conf_mutex */
struct ath10k_fw_components utf_mode_fw;
-   char utf_version[32];
DECLARE_BITMAP(orig_fw_features, ATH10K_FW_FEATURE_COUNT);
enum ath10k_fw_wmi_op_version orig_wmi_op_version;
enum ath10k_fw_wmi_op_version op_version;
diff --git a/drivers/net/wireless/ath/ath10k/testmode.c 
b/drivers/net/wireless/ath/ath10k/testmode.c
index e80aa4eeed05..64e3d5f0d3ac 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -205,15 +205,15 @@ static int ath10k_tm_fetch_utf_firmware_api_2(struct 
ath10k *ar,
 
switch (ie_id) {
case ATH10K_FW_IE_FW_VERSION:
-   if (ie_len > sizeof(ar->testmode.utf_version) - 1)
+   if (ie_len > sizeof(fw_file->fw_version) - 1)
break;
 
-   memcpy(ar->testmode.utf_version, data, ie_len);
-   ar->testmode.utf_version[ie_len] = '\0';
+   memcpy(fw_file->fw_version, data, ie_len);
+   fw_file->fw_version[ie_len] = '\0';
 
ath10k_dbg(ar, ATH10K_DBG_TESTMODE,
   "testmode found fw utf version %s\n",
-  ar->testmode.utf_version);
+  fw_file->fw_version);
break;
case ATH10K_FW_IE_TIMESTAMP:
/* ignore timestamp, but don't warn about it either */
@@ -388,8 +388,8 @@ static int ath10k_tm_cmd_utf_start(struct ath10k *ar, 
struct nlattr *tb[])
 
ar->state = ATH10K_STATE_UTF;
 
-   if (strlen(ar->testmode.utf_version) > 0)
-   ver = ar->testmode.utf_version;
+   if (strlen(ar->testmode.utf_mode_fw.fw_file.fw_version) > 0)
+   ver = ar->testmode.utf_mode_fw.fw_file.fw_version;
else
ver = "API 1";
 

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

[PATCH 2/8] ath10k: refactor firmware images to struct ath10k_fw_components

[Kalle Valo]

To make it easier to share ath10k_core_fetch_board_data_api_n() with testmode.c
refactor all firmware components to struct ath10k_fw_components. This structure
will hold firmware related files, for example firmware-N.bin and board-N.bin.

For firmware-N.bin create a new struct ath10k_fw_file which contains the actual
firmware image as well as the parsed data from the image.

Modify ath10k_core_start() to take struct ath10k_fw_components() as an argument
which makes it possible in following patches to drop some ugly hacks from
testmode.c.

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |  176 +++-
 drivers/net/wireless/ath/ath10k/core.h |   45 +--
 drivers/net/wireless/ath/ath10k/debug.c|   28 +++-
 drivers/net/wireless/ath/ath10k/mac.c  |3 
 drivers/net/wireless/ath/ath10k/swap.c |   21 ++-
 drivers/net/wireless/ath/ath10k/testmode.c |   57 ++---
 6 files changed, 193 insertions(+), 137 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index a04e911bd7e9..c4a36661cb17 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -538,7 +538,8 @@ static int ath10k_core_get_board_id_from_otp(struct ath10k 
*ar)
 
address = ar->hw_params.patch_load_addr;
 
-   if (!ar->otp_data || !ar->otp_len) {
+   if (!ar->normal_mode_fw.fw_file.otp_data ||
+   !ar->normal_mode_fw.fw_file.otp_len) {
ath10k_warn(ar,
"failed to retrieve board id because of invalid 
otp\n");
return -ENODATA;
@@ -546,9 +547,11 @@ static int ath10k_core_get_board_id_from_otp(struct ath10k 
*ar)
 
ath10k_dbg(ar, ATH10K_DBG_BOOT,
   "boot upload otp to 0x%x len %zd for board id\n",
-  address, ar->otp_len);
+  address, ar->normal_mode_fw.fw_file.otp_len);
 
-   ret = ath10k_bmi_fast_download(ar, address, ar->otp_data, ar->otp_len);
+   ret = ath10k_bmi_fast_download(ar, address,
+  ar->normal_mode_fw.fw_file.otp_data,
+  ar->normal_mode_fw.fw_file.otp_len);
if (ret) {
ath10k_err(ar, "could not write otp for board id check: %d\n",
   ret);
@@ -586,7 +589,9 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
u32 bmi_otp_exe_param = ar->hw_params.otp_exe_param;
int ret;
 
-   ret = ath10k_download_board_data(ar, ar->board_data, ar->board_len);
+   ret = ath10k_download_board_data(ar,
+ar->running_fw->board_data,
+ar->running_fw->board_len);
if (ret) {
ath10k_err(ar, "failed to download board data: %d\n", ret);
return ret;
@@ -594,16 +599,20 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
 
/* OTP is optional */
 
-   if (!ar->otp_data || !ar->otp_len) {
+   if (!ar->running_fw->fw_file.otp_data ||
+   !ar->running_fw->fw_file.otp_len) {
ath10k_warn(ar, "Not running otp, calibration will be incorrect 
(otp-data %p otp_len %zd)!\n",
-   ar->otp_data, ar->otp_len);
+   ar->running_fw->fw_file.otp_data,
+   ar->running_fw->fw_file.otp_len);
return 0;
}
 
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot upload otp to 0x%x len %zd\n",
-  address, ar->otp_len);
+  address, ar->running_fw->fw_file.otp_len);
 
-   ret = ath10k_bmi_fast_download(ar, address, ar->otp_data, ar->otp_len);
+   ret = ath10k_bmi_fast_download(ar, address,
+  ar->running_fw->fw_file.otp_data,
+  ar->running_fw->fw_file.otp_len);
if (ret) {
ath10k_err(ar, "could not write otp (%d)\n", ret);
return ret;
@@ -627,46 +636,33 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
return 0;
 }
 
-static int ath10k_download_fw(struct ath10k *ar, enum ath10k_firmware_mode 
mode)
+static int ath10k_download_fw(struct ath10k *ar)
 {
u32 address, data_len;
-   const char *mode_name;
const void *data;
int ret;
 
address = ar->hw_params.patch_load_addr;
 
-   switch (mode) {
-   case ATH10K_FIRMWARE_MODE_NORMAL:
-   data = ar->firmware_data;
-   data_len = ar->firmware_len;
-   mode_name = "normal";
-   ret = ath10k_swap_code_seg_configure(ar,
-
ATH10K_SWAP_CODE_SEG_BIN_TYPE_FW);
-   if (ret) {
-   ath10k_err(ar, "failed to configure fw code swap: %d\n",
-  ret);
- 

[PATCH 4/8] ath10k: move fw_features to struct ath10k_fw_file

[Kalle Valo]

Preparation for testmode.c to use ath10k_core_fetch_board_data_api_n().

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |   28 +++-
 drivers/net/wireless/ath/ath10k/core.h |5 ++---
 drivers/net/wireless/ath/ath10k/htt_rx.c   |2 +-
 drivers/net/wireless/ath/ath10k/htt_tx.c   |9 ++---
 drivers/net/wireless/ath/ath10k/mac.c  |   14 --
 drivers/net/wireless/ath/ath10k/testmode.c |   14 --
 drivers/net/wireless/ath/ath10k/wmi.c  |5 +++--
 drivers/net/wireless/ath/ath10k/wow.c  |7 ---
 8 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index 9f783d35d9f5..bb8c8b2f785b 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -261,7 +261,7 @@ void ath10k_core_get_fw_features_str(struct ath10k *ar,
int i;
 
for (i = 0; i < ATH10K_FW_FEATURE_COUNT; i++) {
-   if (test_bit(i, ar->fw_features)) {
+   if (test_bit(i, ar->normal_mode_fw.fw_file.fw_features)) {
if (len > 0)
len += scnprintf(buf + len, buf_len - len, ",");
 
@@ -627,7 +627,7 @@ static int ath10k_download_and_run_otp(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot otp execute result %d\n", result);
 
if (!(skip_otp || test_bit(ATH10K_FW_FEATURE_IGNORE_OTP_RESULT,
-  ar->fw_features)) &&
+  ar->running_fw->fw_file.fw_features)) &&
result != 0) {
ath10k_err(ar, "otp calibration failed: %d", result);
return -EINVAL;
@@ -1074,13 +1074,13 @@ static int ath10k_core_fetch_firmware_api_n(struct 
ath10k *ar, const char *name,
ath10k_dbg(ar, ATH10K_DBG_BOOT,
   "Enabling feature bit: %i\n",
   i);
-   __set_bit(i, ar->fw_features);
+   __set_bit(i, fw_file->fw_features);
}
}
 
ath10k_dbg_dump(ar, ATH10K_DBG_BOOT, "features", "",
-   ar->fw_features,
-   sizeof(ar->fw_features));
+   ar->running_fw->fw_file.fw_features,
+   sizeof(fw_file->fw_features));
break;
case ATH10K_FW_IE_FW_IMAGE:
ath10k_dbg(ar, ATH10K_DBG_BOOT,
@@ -1430,8 +1430,10 @@ static void ath10k_core_restart(struct work_struct *work)
 
 static int ath10k_core_init_firmware_features(struct ath10k *ar)
 {
-   if (test_bit(ATH10K_FW_FEATURE_WMI_10_2, ar->fw_features) &&
-   !test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
+   struct ath10k_fw_file *fw_file = >normal_mode_fw.fw_file;
+
+   if (test_bit(ATH10K_FW_FEATURE_WMI_10_2, fw_file->fw_features) &&
+   !test_bit(ATH10K_FW_FEATURE_WMI_10X, fw_file->fw_features)) {
ath10k_err(ar, "feature bits corrupted: 10.2 feature requires 
10.x feature to be set as well");
return -EINVAL;
}
@@ -1450,7 +1452,7 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
break;
case ATH10K_CRYPT_MODE_SW:
if (!test_bit(ATH10K_FW_FEATURE_RAW_MODE_SUPPORT,
- ar->fw_features)) {
+ fw_file->fw_features)) {
ath10k_err(ar, "cryptmode > 0 requires raw mode support 
from firmware");
return -EINVAL;
}
@@ -1469,7 +1471,7 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
 
if (rawmode) {
if (!test_bit(ATH10K_FW_FEATURE_RAW_MODE_SUPPORT,
- ar->fw_features)) {
+ fw_file->fw_features)) {
ath10k_err(ar, "rawmode = 1 requires support from 
firmware");
return -EINVAL;
}
@@ -1495,9 +1497,9 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
 * ATH10K_FW_IE_WMI_OP_VERSION.
 */
if (ar->wmi.op_version == ATH10K_FW_WMI_OP_VERSION_UNSET) {
-   if (test_bit(ATH10K_FW_FEATURE_WMI_10X, ar->fw_features)) {
+   if (test_bit(ATH10K_FW_FEATURE_WMI_10X, fw_file->fw_features)) {
if (test_bit(ATH10K_FW_FEATURE_WMI_10_2,
-ar->fw_features))
+fw_file->fw_features))
ar->wmi.op_version = 
ATH10K_FW_WMI_OP_VERSION_10_2;
 

[PATCH 1/8] ath10k: remove deprecated firmware API 1 support

[Kalle Valo]

This has ben deprecated years ago, I haven't heard anyone using it since and
most likely it won't even work anymore. So just remove all of it.

Signed-off-by: Kalle Valo 
---
 drivers/net/wireless/ath/ath10k/core.c |   73 
 drivers/net/wireless/ath/ath10k/core.h |3 -
 drivers/net/wireless/ath/ath10k/hw.h   |2 -
 drivers/net/wireless/ath/ath10k/pci.c  |1 
 drivers/net/wireless/ath/ath10k/wmi.c  |4 --
 5 files changed, 83 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index b2c7fe3d30a4..a04e911bd7e9 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -63,8 +63,6 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] 
= {
.cal_data_len = 2116,
.fw = {
.dir = QCA988X_HW_2_0_FW_DIR,
-   .fw = QCA988X_HW_2_0_FW_FILE,
-   .otp = QCA988X_HW_2_0_OTP_FILE,
.board = QCA988X_HW_2_0_BOARD_DATA_FILE,
.board_size = QCA988X_BOARD_DATA_SZ,
.board_ext_size = QCA988X_BOARD_EXT_DATA_SZ,
@@ -82,8 +80,6 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] 
= {
.cal_data_len = 8124,
.fw = {
.dir = QCA6174_HW_2_1_FW_DIR,
-   .fw = QCA6174_HW_2_1_FW_FILE,
-   .otp = QCA6174_HW_2_1_OTP_FILE,
.board = QCA6174_HW_2_1_BOARD_DATA_FILE,
.board_size = QCA6174_BOARD_DATA_SZ,
.board_ext_size = QCA6174_BOARD_EXT_DATA_SZ,
@@ -102,8 +98,6 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] 
= {
.cal_data_len = 8124,
.fw = {
.dir = QCA6174_HW_2_1_FW_DIR,
-   .fw = QCA6174_HW_2_1_FW_FILE,
-   .otp = QCA6174_HW_2_1_OTP_FILE,
.board = QCA6174_HW_2_1_BOARD_DATA_FILE,
.board_size = QCA6174_BOARD_DATA_SZ,
.board_ext_size = QCA6174_BOARD_EXT_DATA_SZ,
@@ -122,8 +116,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.cal_data_len = 8124,
.fw = {
.dir = QCA6174_HW_3_0_FW_DIR,
-   .fw = QCA6174_HW_3_0_FW_FILE,
-   .otp = QCA6174_HW_3_0_OTP_FILE,
.board = QCA6174_HW_3_0_BOARD_DATA_FILE,
.board_size = QCA6174_BOARD_DATA_SZ,
.board_ext_size = QCA6174_BOARD_EXT_DATA_SZ,
@@ -143,8 +135,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.fw = {
/* uses same binaries as hw3.0 */
.dir = QCA6174_HW_3_0_FW_DIR,
-   .fw = QCA6174_HW_3_0_FW_FILE,
-   .otp = QCA6174_HW_3_0_OTP_FILE,
.board = QCA6174_HW_3_0_BOARD_DATA_FILE,
.board_size = QCA6174_BOARD_DATA_SZ,
.board_ext_size = QCA6174_BOARD_EXT_DATA_SZ,
@@ -167,8 +157,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.cal_data_len = 12064,
.fw = {
.dir = QCA99X0_HW_2_0_FW_DIR,
-   .fw = QCA99X0_HW_2_0_FW_FILE,
-   .otp = QCA99X0_HW_2_0_OTP_FILE,
.board = QCA99X0_HW_2_0_BOARD_DATA_FILE,
.board_size = QCA99X0_BOARD_DATA_SZ,
.board_ext_size = QCA99X0_BOARD_EXT_DATA_SZ,
@@ -186,8 +174,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.cal_data_len = 8124,
.fw = {
.dir = QCA9377_HW_1_0_FW_DIR,
-   .fw = QCA9377_HW_1_0_FW_FILE,
-   .otp = QCA9377_HW_1_0_OTP_FILE,
.board = QCA9377_HW_1_0_BOARD_DATA_FILE,
.board_size = QCA9377_BOARD_DATA_SZ,
.board_ext_size = QCA9377_BOARD_EXT_DATA_SZ,
@@ -205,8 +191,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.cal_data_len = 8124,
.fw = {
.dir = QCA9377_HW_1_0_FW_DIR,
-   .fw = QCA9377_HW_1_0_FW_FILE,
-   .otp = QCA9377_HW_1_0_OTP_FILE,
.board = QCA9377_HW_1_0_BOARD_DATA_FILE,
.board_size = QCA9377_BOARD_DATA_SZ,
.board_ext_size = QCA9377_BOARD_EXT_DATA_SZ,
@@ -229,8 +213,6 @@ static const struct ath10k_hw_params 
ath10k_hw_params_list[] = {
.cal_data_len = 12064,
.fw = {
.dir = 

[PATCH] cfg80211: Improve Connect/Associate command documentation

[Jouni Malinen]

The roaming cases for the Connect command were not fully covered and
neither Connect nor Associate command uses of the prev_bssid parameter
were very clear. Add details to describe how the prev_bssid argument is
supposed to be used and when the driver should use association or
reassociation.

Signed-off-by: Jouni Malinen 
---
 include/net/cfg80211.h   | 26 +++---
 include/uapi/linux/nl80211.h | 16 +---
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b39277e..5ec2036 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1750,7 +1750,12 @@ enum cfg80211_assoc_req_flags {
  * @ie_len: Length of ie buffer in octets
  * @use_mfp: Use management frame protection (IEEE 802.11w) in this association
  * @crypto: crypto settings
- * @prev_bssid: previous BSSID, if not %NULL use reassociate frame
+ * @prev_bssid: previous BSSID, if not %NULL use reassociate frame. This is 
used
+ * to indicate a request to reassociate within the ESS instead of a request
+ * do the initial association with the ESS. When included, this is set to
+ * the BSSID of the current association, i.e., to the value that is
+ * included in the Current AP address field of the Reassociation Request
+ * frame.
  * @flags:  See  cfg80211_assoc_req_flags
  * @ht_capa:  HT Capabilities over-rides.  Values set in ht_capa_mask
  * will be used in ht_capa.  Un-supported values will be ignored.
@@ -1925,7 +1930,12 @@ struct cfg80211_bss_selection {
  * @pbss: if set, connect to a PCP instead of AP. Valid for DMG
  * networks.
  * @bss_select: criteria to be used for BSS selection.
- * @prev_bssid: previous BSSID, if not %NULL use reassociate frame
+ * @prev_bssid: previous BSSID, if not %NULL use reassociate frame. This is 
used
+ * to indicate a request to reassociate within the ESS instead of a request
+ * do the initial association with the ESS. When included, this is set to
+ * the BSSID of the current association, i.e., to the value that is
+ * included in the Current AP address field of the Reassociation Request
+ * frame.
  */
 struct cfg80211_connect_params {
struct ieee80211_channel *channel;
@@ -2377,7 +2387,17 @@ struct cfg80211_qos_map {
  * @connect: Connect to the ESS with the specified parameters. When connected,
  * call cfg80211_connect_result() with status code %WLAN_STATUS_SUCCESS.
  * If the connection fails for some reason, call cfg80211_connect_result()
- * with the status from the AP.
+ * with the status from the AP. The driver is allowed to roam to other
+ * BSSes within the ESS when the other BSS matches the connect parameters.
+ * When such roaming is initiated by the driver, the driver is expected to
+ * verify that the target matches the configured security parameters and
+ * to use Reassociation Request frame instead of Association Request frame.
+ * The connect function can also be used to request the driver to perform
+ * a specific roam when connected to an ESS. In that case, the prev_bssid
+ * parameter is set to the BSSID of the currently associated BSS as an
+ * indication of requesting reassociation. In both the driver-initiated and
+ * new connect() call initiated roaming cases, the result of roaming is
+ * indicated with a call to cfg80211_roamed() or cfg80211_roamed_bss().
  * (invoked with the wireless_dev mutex held)
  * @disconnect: Disconnect from the BSS/ESS.
  * (invoked with the wireless_dev mutex held)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6da52d7..b460628 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -429,7 +429,11 @@
  * @NL80211_CMD_ASSOCIATE: association request and notification; like
  * NL80211_CMD_AUTHENTICATE but for Association and Reassociation
  * (similar to MLME-ASSOCIATE.request, MLME-REASSOCIATE.request,
- * MLME-ASSOCIATE.confirm or MLME-REASSOCIATE.confirm primitives).
+ * MLME-ASSOCIATE.confirm or MLME-REASSOCIATE.confirm primitives). The
+ * %NL80211_ATTR_PREV_BSSID attribute is used to specify whether the
+ * request is for the initial association to an ESS (that attribute not
+ * included) or for reassociation within the ESS (that attribute is
+ * included).
  * @NL80211_CMD_DEAUTHENTICATE: deauthentication request and notification; like
  * NL80211_CMD_AUTHENTICATE but for Deauthentication frames (similar to
  * MLME-DEAUTHENTICATION.request and MLME-DEAUTHENTICATE.indication
@@ -479,6 +483,9 @@
  * set of BSSID,frequency parameters is used (i.e., either the enforcing
  * %NL80211_ATTR_MAC,%NL80211_ATTR_WIPHY_FREQ or the less strict
  * %NL80211_ATTR_MAC_HINT and %NL80211_ATTR_WIPHY_FREQ_HINT).
+ * %NL80211_ATTR_PREV_BSSID can be used to request a reassociation within
+ * the ESS in case the 

[PATCH 2/2] Check protocol number in nl80211 netlink socket release notification handler.

[Dmitry Ivanov]

This patch corrects the problem where non-privileged user can create
netlink socket with the same port_id as used by hostapd but different
protocol number. Upon close() or process termination, a notification is
sent  to nl80211 subsystem which will destroy virtual wireless network
interfaces created by hostapd like it just died but in fact hostapd is
still running. This is possible because port_id is unique within particular
protocol number only.

Fixes: 026331c4d9b5 ("cfg80211/mac80211: allow registering for and sending 
action frames")
Signed-off-by: Dmitry Ivanov 
---
 net/wireless/nl80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 98c9242..056a730 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -13216,7 +13216,7 @@ static int nl80211_netlink_notify(struct notifier_block 
* nb,
struct wireless_dev *wdev;
struct cfg80211_beacon_registration *reg, *tmp;
 
-   if (state != NETLINK_URELEASE)
+   if (state != NETLINK_URELEASE || notify->protocol != NETLINK_GENERIC)
return NOTIFY_DONE;
 
rcu_read_lock();
-- 
2.1.4

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

[PATCH 1/2] Do not send netlink socket release notification when socket is not bound

[Dmitry Ivanov]

This patch corrects the problem where non-privileged user may create
netlink socket with port_id equal to port_id used by hostapd to create
virtual wireless network interfaces. Call to bind() will fail for such
socket, but release notification sent on close() or process termination
to nl80211 subsystem will destroy virtual network interfaces while
hostapd is still running.

Signed-off-by: Dmitry Ivanov 
---
 net/netlink/af_netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 215fc08..330ebd6 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -688,7 +688,7 @@ static int netlink_release(struct socket *sock)
 
skb_queue_purge(>sk_write_queue);
 
-   if (nlk->portid) {
+   if (nlk->portid && nlk->bound) {
struct netlink_notify n = {
.net = sock_net(sk),
.protocol = sk->sk_protocol,
-- 
2.1.4

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

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Bjorn Helgaas]

On Tue, Apr 05, 2016 at 09:49:45PM +0200, Michael Büsch wrote:
> On Tue, 5 Apr 2016 14:40:15 -0500
> Bjorn Helgaas  wrote:
> 
> > [+cc Matthew]
> > 
> > Hi Lukas,
> > 
> > On Tue, Mar 29, 2016 at 08:20:30PM +0200, Lukas Wunner wrote:
> > > Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ storm
> > > on boot until they are reset, causing spurious interrupts if the IRQ is
> > > shared. Apparently the EFI bootloader enables the device and does not
> > > disable it before passing control to the OS. The bootloader contains a
> > > driver for the wireless card which allows it to phone home to Cupertino.
> > > This is used for Internet Recovery (download and install OS X images)
> > > and probably also for Back to My Mac (remote access, RFC 6281) and to
> > > discover stolen hardware.
> > > 
> > > The issue is most pronounced on 2011 and 2012 MacBook Pros where the IRQ
> > > is shared with 3 other devices (Light Ridge Thunderbolt controller, SDXC
> > > reader, HDA card on discrete GPU). As soon as an interrupt handler is
> > > installed for one of these devices, the ensuing storm of spurious IRQs
> > > causes the kernel to disable the IRQ and switch to polling. This lasts
> > > until the b43 driver loads and resets the device.
> > > 
> > > Loading the b43 driver first is not always an option, in particular with
> > > the Light Ridge Thunderbolt controller: The PCI hotplug IRQ handler gets
> > > installed early on because it is built in, unlike b43 which is usually
> > > a module.
> > > 
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=79301
> > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=895951
> > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1009819
> > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1149632
> > > Tested-by: Lukas Wunner  [MacBookPro9,1]
> > > Signed-off-by: Lukas Wunner 
> > > ---
> > >  drivers/bcma/bcma_private.h |  2 --
> > >  drivers/pci/quirks.c| 27 +++
> > >  include/linux/bcma/bcma.h   |  1 +
> > >  3 files changed, 28 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/bcma/bcma_private.h b/drivers/bcma/bcma_private.h
> > > index eda0909..f642c42 100644
> > > --- a/drivers/bcma/bcma_private.h
> > > +++ b/drivers/bcma/bcma_private.h
> > > @@ -8,8 +8,6 @@
> > >  #include 
> > >  #include 
> > >  
> > > -#define BCMA_CORE_SIZE   0x1000
> > > -
> > >  #define bcma_err(bus, fmt, ...) \
> > >   pr_err("bus%d: " fmt, (bus)->num, ##__VA_ARGS__)
> > >  #define bcma_warn(bus, fmt, ...) \
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index 8e67802..d4fb5ee 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -25,6 +25,8 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#include 
> > >  #include  /* isa_dma_bridge_buggy */
> > >  #include "pci.h"
> > >  
> > > @@ -3282,6 +3284,31 @@ 
> > > DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL, 0x156d,
> > >  quirk_apple_wait_for_thunderbolt);
> > >  #endif
> > >  
> > > +/*
> > > + * Broadcom 4331 wireless cards built into Apple Macs unleash an IRQ 
> > > storm
> > > + * on boot until they are reset, causing spurious interrupts if the IRQ 
> > > is
> > > + * shared. Apparently the EFI bootloader enables the device to phone home
> > > + * to Cupertino and does not disable it before passing control to the OS.
> > > + */
> > > +static void quirk_apple_b43_reset(struct pci_dev *dev)
> > > +{
> > > + void __iomem *mmio;
> > > +
> > > + if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc.") || !dev->bus->self ||
> > > + pci_pcie_type(dev->bus->self) != PCI_EXP_TYPE_ROOT_PORT)
> > > + return;
> > > +
> > > + mmio = pci_iomap(dev, 0, 0);
> > > + if (!mmio) {
> > > + pr_err("b43 quirk: Cannot iomap device, IRQ storm ahead\n");
> > > + return;
> > > + }
> > > + iowrite32(BCMA_RESET_CTL_RESET,
> > > +   mmio + (1 * BCMA_CORE_SIZE) + BCMA_RESET_CTL);
> > > + pci_iounmap(dev, mmio);  
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=111781 and
> > https://mjg59.dreamwidth.org/11235.html describe a sort of similar
> > issue, but with DMA.  An interrupt from the device is probably to
> > signal a DMA completion, but these problem reports only mention the
> > "IRQ nobody cared" issue; I don't see anything about memory
> > corruption.
> > 
> > If this resets the device, I guess that should prevent future DMA as
> > well as future interrupts.  Would pci_reset_function() do the same
> > thing in a more generic way?
> > 
> > I'm a little bit hesitant to put a quirk like this in Linux because
> > it's only a 90% solution.  If the only problem is the interrupts, it's
> > probably OK since a few extra interrupts doesn't hurt anything.  But
> > if there is also DMA that might corrupt something, a 90% solution just
> > makes it harder to debug for the remaining 10%.
> 
> This 

Re: pull-request: mac80211-next 2016-04-06

[Johannes Berg]

On Wed, 2016-04-06 at 15:25 +0200, Johannes Berg wrote:
> Hi Dave,
> 
> For the 4.6 cycle, there's of course much more. The few things that
> 

Err, -next, so that's 4.7.

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

pull-request: mac80211-next 2016-04-06

[Johannes Berg]

Hi Dave,

For the 4.6 cycle, there's of course much more. The few things that
stand out are in the tag message below.

There's a pending patchset to add NAN (Neighbor Awareness Networking,
a WFA protocol) API support, but the API isn't ready yet. I might
have that later in time for 4.6.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 05cf8077e54b20dddb756eaa26f3aeb5c38dd3cf:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-04-01 
20:03:33 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git 
tags/mac80211-next-for-davem-2016-04-06

for you to fetch changes up to 4ce2bd9c4c1dfb416206ff1ad5283f6d24af4031:

  cfg80211: Allow reassociation to be requested with internal SME (2016-04-06 
15:09:28 +0200)


For the 4.6 cycle, we have a number of changes:
 * Bob's mesh mode rhashtable conversion, this includes
   the rhashtable API change for allocation flags
 * BSSID scan, connect() command reassoc support (Jouni)
 * fast (optimised data only) and support for RSS in mac80211 (myself)
 * various smaller changes


Akira Moroo (1):
  cfg80211: fix kernel-doc struct name

Arend van Spriel (1):
  nl80211: add feature for BSS selection support

Avraham Stern (1):
  ieee80211: support parsing Fine Timing Measurement action frame

Ayala Beker (2):
  cfg80211: allow userspace to specify client P2P PS support
  mac80211: track and tell driver about GO client P2P PS abilities

Bob Copeland (14):
  mac80211: mesh: move path tables into if_mesh
  mac80211: mesh: don't hash sdata in mpath tables
  mac80211: mesh: factor out common mesh path allocation code
  mac80211: mesh: embed known gates list in struct mesh_path
  rhashtable: accept GFP flags in rhashtable_walk_init
  mac80211: mesh: convert path table to rhashtable
  mac80211: mesh: fix crash in mesh_path_timer
  mac80211: mesh: handle failed alloc for rmc cache
  mac80211: mesh: use hlist for rmc cache
  mac80211: mesh: embed gates hlist head directly
  mac80211: mesh: reorder structure members
  mac80211: mesh: fix mesh path kerneldoc
  mac80211: mesh: fix cleanup for mesh pathtable
  mac80211: mesh: flush paths outside of plink lock

Emmanuel Grumbach (1):
  mac80211: Set global RRM capability

Felix Fietkau (4):
  mac80211: do not pass injected frames without a valid rate to the driver
  mac80211: minstrel_ht: improve sample rate skip logic
  mac80211: add A-MSDU tx support
  mac80211: minstrel_ht: set A-MSDU tx limits based on selected 
max_prob_rate

Johannes Berg (17):
  wext: unregister_pernet_subsys() on notifier registration failure
  mac80211: allow drivers to report CLOCK_BOOTTIME for scan results
  mac80211: remove sta_info debugfs sub-struct
  mac80211: don't start dynamic PS timer if not needed
  mac80211: clean up station flags debugfs
  mac80211: fix cipher scheme function name
  mac80211: avoid useless memory write on each frame RX
  mac80211: allow passing transmitter station on RX
  mac80211: count MSDUs in A-MSDU properly
  mac80211: move semicolon out of CALL_RXH macro
  mac80211: move averaged values out of rx_stats
  mac80211: remove rx_stats.last_rx update after sta alloc
  mac80211: add separate last_ack variable
  mac80211: fix last RX rate data consistency
  mac80211: fix RX u64 stats consistency on 32-bit platforms
  mac80211: add fast-rx path
  mac80211: enable collecting station statistics per-CPU

Jouni Malinen (5):
  cfg80211: Allow a scan request for a specific BSSID
  mac80211: Support a scan request for a specific BSSID
  mac80211_hwsim: Support a hw scan request for a specific BSSID
  cfg80211: Add option to specify previous BSSID for Connect command
  cfg80211: Allow reassociation to be requested with internal SME

João Paulo Rechi Vita (1):
  rfkill: Use switch to demux userspace operations

Lorenzo Bianconi (1):
  mac80211: parse VHT info in injected frames

Mohammed Shafi Shajakhan (1):
  mac80211: Remove unused variable in per STA debugfs struct

Sara Sharon (4):
  mac80211: allow not sending MIC up from driver for HW crypto
  mac80211: synchronize driver rx queues before removing a station
  mac80211: add NETIF_F_RXCSUM to features white list
  mac80211: enable starting BA session with custom timeout

Sven Eckelmann (2):
  mac80211: document only injected *_RADIOTAP_* flags
  mac80211: fix parsing of 40Mhz in injected radiotap header

 Documentation/networking/mac80211-injection.txt   |  17 +-
 drivers/net/wireless/ath/ath10k/htt_rx.c  |   2 +-
 drivers/net/wireless/ath/wcn36xx/txrx.c   |   2 +-
 drivers/net/wireless/intel/iwlwifi/dvm/rx.c  

pull-request: mac80211 2016-04-06

[Johannes Berg]

Hi Dave,

First set of fixes for 4.6. Nothing really stands out.

Let me know if there's any problem.

Thanks,
johannes



The following changes since commit 05cf8077e54b20dddb756eaa26f3aeb5c38dd3cf:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2016-04-01 
20:03:33 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git 
tags/mac80211-for-davem-2016-04-06

for you to fetch changes up to b4201cc4fc6e1c57d6d306b1f787865043d60129:

  mac80211: fix "warning: ‘target_metric’ may be used uninitialized" 
(2016-04-06 15:10:25 +0200)


For the current RC series, we have the following fixes:
 * TDLS fixes from Arik and Ilan
 * rhashtable fixes from Ben and myself
 * documentation fixes from Luis
 * U-APSD fixes from Emmanuel
 * a TXQ fix from Felix
 * and a compiler warning suppression from Jeff


Arik Nemtsov (3):
  mac80211: TDLS: always downgrade invalid chandefs
  mac80211: TDLS: change BW calculation for WIDER_BW peers
  mac80211: recalc min_def chanctx even when chandef is identical

Ben Greear (1):
  mac80211: ensure no limits on station rhashtable

Emmanuel Grumbach (2):
  mac80211: don't send deferred frames outside the SP
  mac80211: close the SP when we enqueue frames during the SP

Felix Fietkau (1):
  mac80211: fix AP buffered multicast frames with queue control and txq

Ilan Peer (1):
  mac80211: Fix BW upgrade for TDLS peers

Jeff Mahoney (1):
  mac80211: fix "warning: ‘target_metric’ may be used uninitialized"

Johannes Berg (1):
  mac80211: properly deal with station hashtable insert errors

Luis de Bethencourt (2):
  mac80211: add doc for RX_FLAG_DUP_VALIDATED flag
  mac80211: remove description of dropped member

 include/net/mac80211.h |  2 ++
 net/mac80211/chan.c|  4 +++-
 net/mac80211/ieee80211_i.h |  4 
 net/mac80211/mesh_hwmp.c   |  2 +-
 net/mac80211/sta_info.c| 14 +-
 net/mac80211/sta_info.h|  1 -
 net/mac80211/tdls.c| 43 +++
 net/mac80211/tx.c  | 13 +
 net/mac80211/vht.c | 30 +-
 9 files changed, 88 insertions(+), 25 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: fix "warning: ‘target_metric’ may be used uninitialized"

[Johannes Berg]

On Mon, 2016-04-04 at 14:15 -0400, Jeff Mahoney wrote:
> This fixes:
> 
> net/mac80211/mesh_hwmp.c:603:26: warning: ‘target_metric’ may be used
> uninitialized in this function
> 
> target_metric is only consumed when reply = true so no bug exists
> here,
> but gcc doesn't notice that.  Initializing to 0 clears the warning.
> 

Applied.

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

Re: [PATCH 2/2] cfg80211: Allow reassociation to be requested with internal SME

[Johannes Berg]

On Tue, 2016-03-29 at 13:53 +0300, Jouni Malinen wrote:
> If the user space issues a NL80211_CMD_CONNECT with
> NL80211_ATTR_PREV_BSSID when there is already a connection, allow
> this
> to proceed as a reassociation instead of rejecting the new connect
> command with EALREADY.
> 
> Signed-off-by: Jouni Malinen 
> ---
>  net/wireless/nl80211.c |  3 ++-
>  net/wireless/sme.c | 11 +--
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index a98665a..773b20f 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -8117,7 +8117,8 @@ static int nl80211_connect(struct sk_buff *skb,
> struct genl_info *info)
>   }
>  
>   wdev_lock(dev->ieee80211_ptr);
> - err = cfg80211_connect(rdev, dev, , connkeys, NULL);
> + err = cfg80211_connect(rdev, dev, , connkeys,
> +    connect.prev_bssid);
>   wdev_unlock(dev->ieee80211_ptr);
>   if (err)
>   kzfree(connkeys);
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 65882d2..ce32492 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -492,8 +492,15 @@ static int cfg80211_sme_connect(struct
> wireless_dev *wdev,
>   if (!rdev->ops->auth || !rdev->ops->assoc)
>   return -EOPNOTSUPP;
>  
> - if (wdev->current_bss)
> - return -EALREADY;
> + if (wdev->current_bss) {
> + if (!prev_bssid)
> + return -EALREADY;
> + cfg80211_unhold_bss(wdev->current_bss);
> + cfg80211_put_bss(wdev->wiphy, >current_bss-
> >pub);
> + wdev->current_bss = NULL;
> +
> + cfg80211_sme_free(wdev);
> + }
> 
Since we know the previous BSSID, I've added a check here for it.

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

RE: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)

[Machani, Yaniv]

On Wed, Apr 06, 2016 at 15:12:43, Pali Rohár wrote:
> > > > For other TI wilink chips there are -ap.bin firmware files 
> > > > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode.
> > > > But for
> > > > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is 
> > > > missing.
> > > >
> > > > Do you have any idea what happened with AP firmware for ti
> > > > wilink4 wl1251 wifi chip? Or where can be found? Guys from TI, 
> > > > can you help?
> > > >
> > > > I see that STA firmware was added into linux-firmware tree in 
> > > > year
> > > > 2013 by this pull request [2].
> > > >
> > > > [1] -
> > > > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firm
> > > > w
> > > > are .g
> > > > it/tree/ti-connectivity
> > > >
> > > > [2] -
> > > > http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382
> > >
> > > Hi! Anybody has some idea about that AP firmware?
> >
> > Hi,
> > wl1251 does not support AP mode, so there is no firmware for it in 
> > the tree.
> >
> > Regards,
> > Yaniv
> 
> Hi Yaniv! I read on some TI whitepaper, that wl1251 hardware supports 
> some Soft-AP mode. So I expect that either special FW is needed for it 
> or somehow it is possible to use current released. Do you have any 
> information about it?
> 
> --
> Pali Rohár
> pali.ro...@gmail.com

Hi Pali,
This must be some typo, the device does not support Soft-AP.
More than that, wl1251 family is not officially supported via the mainline 
Linux.
For Soft-AP, and other new features based on Linux you should use WiLink8 chip 
family.

Regards,
Yaniv

Re: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)

[Pali Rohár]

On Wednesday 06 April 2016 13:30:22 Machani, Yaniv wrote:
> On Mon, Apr 04, 2016 at 15:39:44, Pali Rohár wrote:
> > > In linux-firmware repository [1] is missing AP firmware for TI
> > > wl1251 chip. There is only STA firmware wl1251-fw.bin which
> > > supports managed and ad-hoc modes.
> > > 
> > > For other TI wilink chips there are -ap.bin firmware files
> > > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode.
> > > But for
> > > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is
> > > missing.
> > > 
> > > Do you have any idea what happened with AP firmware for ti
> > > wilink4 wl1251 wifi chip? Or where can be found? Guys from TI,
> > > can you help?
> > > 
> > > I see that STA firmware was added into linux-firmware tree in
> > > year 2013 by this pull request [2].
> > > 
> > > [1] -
> > > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmw
> > > are .g
> > > it/tree/ti-connectivity
> > > 
> > > [2] -
> > > http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382
> > 
> > Hi! Anybody has some idea about that AP firmware?
> 
> Hi,
> wl1251 does not support AP mode, so there is no firmware for it in
> the tree.
> 
> Regards,
> Yaniv

Hi Yaniv! I read on some TI whitepaper, that wl1251 hardware supports 
some Soft-AP mode. So I expect that either special FW is needed for it 
or somehow it is possible to use current released. Do you have any 
information about it?

-- 
Pali Rohár
pali.ro...@gmail.com


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

RE: AP firmware for TI wl1251 wifi chip (wl1251-fw-ap.bin)

[Machani, Yaniv]

On Mon, Apr 04, 2016 at 15:39:44, Pali Rohár wrote:

> > In linux-firmware repository [1] is missing AP firmware for TI 
> > wl1251 chip. There is only STA firmware wl1251-fw.bin which supports 
> > managed and ad-hoc modes.
> >
> > For other TI wilink chips there are -ap.bin firmware files 
> > (wl1271-fw-ap.bin and wl128x-fw-ap.bin) which support AP mode. But 
> > for
> > wl1251 firmware file with guessed name "wl1251-fw-ap.bin" is missing.
> >
> > Do you have any idea what happened with AP firmware for ti wilink4
> > wl1251 wifi chip? Or where can be found? Guys from TI, can you help?
> >
> > I see that STA firmware was added into linux-firmware tree in year
> > 2013 by this pull request [2].
> >
> > [1] -
> > https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware
> > .g
> > it/tree/ti-connectivity
> >
> > [2] - 
> > http://thread.gmane.org/gmane.linux.kernel/1566500/focus=1571382
> >
> 
> Hi! Anybody has some idea about that AP firmware?
> 

Hi,
wl1251 does not support AP mode, so there is no firmware for it in the tree.

Regards,
Yaniv

> --
> Pali Rohár
> pali.ro...@gmail.com

Re: [PATCH] PCI: Add Broadcom 4331 reset quirk to prevent IRQ storm

[Andrew Worsley]

That patch appears to be the grub 1 equivalent  to  grub2
git://git.savannah.gnu.org/grub.git rev 9d34bb8 which puts the network
device into D3 power state.

I am running grub2 with that patch and it doesn't fix my irq 17 problem.

Can we not fix this in grub2 via something like Lukas original patch
or disable any DMA transfers before the kernel starts?

Andrew
On 6 April 2016 at 05:59, Matthew Garrett  wrote:
> On Tue, Apr 05, 2016 at 02:40:15PM -0500, Bjorn Helgaas wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=111781 and
>> https://mjg59.dreamwidth.org/11235.html describe a sort of similar
>> issue, but with DMA.  An interrupt from the device is probably to
>> signal a DMA completion, but these problem reports only mention the
>> "IRQ nobody cared" issue; I don't see anything about memory
>> corruption.
>
> I "fixed" this with
> https://github.com/mjg59/grub-fedora/commit/21fcd6d79b7601e4b20ad70c5408adff2dabbc1d
> - doing the same in the kernel EFI stub would probably be the best way
> to handle it. This way you're guaranteed to stop DMA before the kernel
> reclaims boot services memory, which guarantees you won't have any
> corruption.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCHv3 RESEND 06/11] cfg80211: Provide an API to report NAN function termination

[Otcheretianski, Andrei]

> -Original Message-
> From: Malinen, Jouni [mailto:jo...@qca.qualcomm.com]
> Sent: Wednesday, April 6, 2016 12:40 PM
> To: Grumbach, Emmanuel 
> Cc: johan...@sipsolutions.net; linux-wireless@vger.kernel.org;
> Otcheretianski, Andrei 
> Subject: Re: [PATCHv3 RESEND 06/11] cfg80211: Provide an API to report
> NAN function termination
> 
> On Tue, Mar 29, 2016 at 12:35:04PM +0300, Emmanuel Grumbach wrote:
> > Provide a function that reports NAN DE function termination. The
> > function may be terminated due to one of the following reasons: user
> > request, ttl expiration or failure.
> 
> > diff --git a/include/uapi/linux/nl80211.h
> > b/include/uapi/linux/nl80211.h
> 
> > +/**
> > + * enum nl80211_nan_func_term_reason - NAN functions termination
> > +reason
> > + *
> > + * Defines termination reasons of a NAN function
> > + *
> > + * @NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST: requested
> by user
> > + * @NL80211_NAN_FUNC_TERM_REASON_TTL_EXPIRED: timeout
> > + * @NL80211_NAN_FUNC_TERM_REASON_ERROR: errored  */ enum
> > +nl80211_nan_func_term_reason {
> > +   NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST,
> > +   NL80211_NAN_FUNC_TERM_REASON_TTL_EXPIRED,
> > +   NL80211_NAN_FUNC_TERM_REASON_ERROR,
> > +};
> 
> Can you please add
> NL80211_NAN_FUNC_TERM_REASON_VENDOR_SPECIFIC? That would help
> vendors to implement and send vendor specific reason codes if any.
> 

The termination reasons are defined in the spec. Also adding a single enum 
value will not help much if the vendor has multiple error codes.
So I think this info should be in a different attribute, if needed.

Andrei.

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

RE: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Otcheretianski, Andrei]

> -Original Message-
> From: Malinen, Jouni [mailto:jo...@qca.qualcomm.com]
> Sent: Wednesday, April 6, 2016 12:34 PM
> To: Grumbach, Emmanuel 
> Cc: johan...@sipsolutions.net; linux-wireless@vger.kernel.org;
> Otcheretianski, Andrei 
> Subject: Re: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN
> commands
> 
> On Tue, Mar 29, 2016 at 12:34:59PM +0300, Emmanuel Grumbach wrote:
> > Define several attributes that may be configured by user space when
> > starting NAN functionality (master preference and dual band operation)
> 
> > diff --git a/include/uapi/linux/nl80211.h
> > b/include/uapi/linux/nl80211.h @@ -826,6 +826,16 @@
> > + * @NL80211_CMD_START_NAN: Start NAN operation, identified by its
> > + * %NL80211_ATTR_WDEV interface. This interface must have been
> previously
> > + * created with %NL80211_CMD_NEW_INTERFACE. After it has been
> started, the
> > + * NAN interface will create or join a cluster. This command must have a
> > + * valid %NL80211_ATTR_NAN_MASTER_PREF attribute and optional
> > + * %NL80211_ATTR_NAN_DUAL attributes.
> 
> 
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> 
> > +static int nl80211_start_nan(struct sk_buff *skb, struct genl_info
> > +*info)
> 
> > +   if (!info->attrs[NL80211_ATTR_NAN_MASTER_PREF])
> > +   return -EINVAL;
> 
> Why NL80211_ATTR_NAN_MASTER_PREF is mandatory? The spec suggests
> to assume master preference value of 128 if not provided. Please see quoted
> text from NAN 1.0 specification:
> "A NAN Device which out-of-the-box will have a Master Preference greater
> than or equal to 128 with the intention of being a NAN Master Device"
> 
I think that this quote just defines "NAN infrastructure device".
I don't see where the spec specifies that 128 should be used as a default value.

Andrei

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

Re: [PATCH] ath10k: Fix return value for btcoex and peer stats debugfs

[Mohammed Shafi Shajakhan]

Hello Joe,

On Tue, Apr 05, 2016 at 10:28:56AM -0700, Joe Perches wrote:
> On Tue, 2016-04-05 at 20:58 +0530, Mohammed Shafi Shajakhan wrote:
> > From: Mohammed Shafi Shajakhan 
> > 
> > Return value is incorrect for btcoex and peer stats debugfs
> > 'write' entries if the user provides a value that matches with
> > the already available debugfs entry, this results in the debugfs
> > entry getting stuck and the operation has to be terminated manually.
> > Fix this by returning the appropriate return 'count' as we do it for
> > other debugfs entries like pktlog etc
> 
> Not your code, but some of the xor uses are odd at best.

[shafi] ok.

> 
> > diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
> > b/drivers/net/wireless/ath/ath10k/debug.c
> []
> > @@ -2122,7 +2122,7 @@ static ssize_t ath10k_write_btcoex(struct file *file,
> >     struct ath10k *ar = file->private_data;
> >     char buf[32];
> >     size_t buf_size;
> > -   int ret = 0;
> > +   int ret;
> >     bool val;
> >  
> >     buf_size = min(count, (sizeof(buf) - 1));
> > @@ -2142,8 +2142,10 @@ static ssize_t ath10k_write_btcoex(struct file *file,
> >     goto exit;
> >     }
> >  
> > -   if (!(test_bit(ATH10K_FLAG_BTCOEX, >dev_flags) ^ val))
> > +   if (!(test_bit(ATH10K_FLAG_BTCOEX, >dev_flags) ^ val)) {
> 
> xor on an int and a bool.

[shafi] Should we change the 'val' to int, but this code seems to be working
test_bit seems to do this :
return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));

do we see a problem, it should work right  (because they are bitwise
'ANDING' with 1UL)

> 
> > @@ -2189,7 +2191,7 @@ static ssize_t ath10k_write_peer_stats(struct file 
> > *file,
> >     struct ath10k *ar = file->private_data;
> >     char buf[32];
> >     size_t buf_size;
> > -   int ret = 0;
> > +   int ret;
> >     bool val;
> >  
> >     buf_size = min(count, (sizeof(buf) - 1));
> > @@ -2209,8 +2211,10 @@ static ssize_t ath10k_write_peer_stats(struct file 
> > *file,
> >     goto exit;
> >     }
> >  
> > -   if (!(test_bit(ATH10K_FLAG_PEER_STATS, >dev_flags) ^ val))
> > +   if (!(test_bit(ATH10K_FLAG_PEER_STATS, >dev_flags) ^ val)) {
> 
> here too

[shafi] same thing here ? is there a better code , kindly suggest

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

RE: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Grumbach, Emmanuel]

> 
> On Tue, Mar 29, 2016 at 12:34:59PM +0300, Emmanuel Grumbach wrote:
> > This allows user space to start/stop NAN interface.
> > A NAN interface is like P2P device in a few aspects: it doesn't have a
> > netdev associated to it.
> > Add the new interface type and prevent operations that can't be
> > executed on NAN interface like scan.
> 
> What is the need for this new NAN interface from the view point of
> supporting NAN discovery?

I guess you can ask the same question for P2P device. They are both
very similar. No traffic, discovery only. So all the reasons to have a P2P
device interface apply here as well.

> Are you planning to use the same iface type
> eventually for supporting data traffic over NAN interface as well?

We don't really know yet. The plan is to have another interface type.
Probably one instance per Nan Data Path or the interface type.

> 
> > + * @start_nan: Start the NAN interface.
> > + * @stop_nan: Stop the NAN interface.
> 
> >  struct cfg80211_ops {
> > +   int (*start_nan)(struct wiphy *wiphy, struct wireless_dev
> *wdev,
> > +struct cfg80211_nan_conf *conf);
> > +   void(*stop_nan)(struct wiphy *wiphy, struct wireless_dev
> *wdev);
> 
> And similarly from 4/11:
> + int (*nan_change_conf)(struct wiphy *wiphy,
> +struct wireless_dev *wdev,
> +struct cfg80211_nan_conf *conf,
> +u32 changes);
> 
> There is no matching cookie (such as transaction id) in these requests to the
> driver, are you expecting synchronous response from the driver for these
> requests? Or would some kind of event after the operation has been
> completed be possible?

Yes - these are expected to be synchronous since they don't involve network
transactions. Does your solution require this operation to be asynchronous? I
find a bit weird to make a local operation asynchronous though.

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

Re: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Malinen, Jouni]

On Tue, Mar 29, 2016 at 12:34:59PM +0300, Emmanuel Grumbach wrote:
> This allows user space to start/stop NAN interface.
> A NAN interface is like P2P device in a few aspects: it
> doesn't have a netdev associated to it.
> Add the new interface type and prevent operations that
> can't be executed on NAN interface like scan.

What is the need for this new NAN interface from the view point of
supporting NAN discovery? Are you planning to use the same iface type
eventually for supporting data traffic over NAN interface as well?

> + * @start_nan: Start the NAN interface.
> + * @stop_nan: Stop the NAN interface.

>  struct cfg80211_ops {
> + int (*start_nan)(struct wiphy *wiphy, struct wireless_dev *wdev,
> +  struct cfg80211_nan_conf *conf);
> + void(*stop_nan)(struct wiphy *wiphy, struct wireless_dev *wdev);

And similarly from 4/11:
+   int (*nan_change_conf)(struct wiphy *wiphy,
+  struct wireless_dev *wdev,
+  struct cfg80211_nan_conf *conf,
+  u32 changes);

There is no matching cookie (such as transaction id) in these requests
to the driver, are you expecting synchronous response from the driver
for these requests? Or would some kind of event after the operation has
been completed be possible?

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

RE: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Grumbach, Emmanuel]

 
> On Tue, Mar 29, 2016 at 12:34:59PM +0300, Emmanuel Grumbach wrote:
> > Define several attributes that may be configured by user space when
> > starting NAN functionality (master preference and dual band operation)
> 
> > diff --git a/include/uapi/linux/nl80211.h
> > b/include/uapi/linux/nl80211.h @@ -826,6 +826,16 @@
> > + * @NL80211_CMD_START_NAN: Start NAN operation, identified by its
> > + * %NL80211_ATTR_WDEV interface. This interface must have been
> previously
> > + * created with %NL80211_CMD_NEW_INTERFACE. After it has been
> started, the
> > + * NAN interface will create or join a cluster. This command must have a
> > + * valid %NL80211_ATTR_NAN_MASTER_PREF attribute and optional
> > + * %NL80211_ATTR_NAN_DUAL attributes.
> 
> 
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> 
> > +static int nl80211_start_nan(struct sk_buff *skb, struct genl_info
> > +*info)
> 
> > +   if (!info->attrs[NL80211_ATTR_NAN_MASTER_PREF])
> > +   return -EINVAL;
> 
> Why NL80211_ATTR_NAN_MASTER_PREF is mandatory? The spec suggests
> to assume master preference value of 128 if not provided. Please see quoted
> text from NAN 1.0 specification:
> "A NAN Device which out-of-the-box will have a Master Preference greater
> than or equal to 128 with the intention of being a NAN Master Device"
> 

Yes, but you can still have this default being defined in user space. While I 
agree that
the kernel could accept the command even if that attribute is not present, this 
code,
by itself, doesn't mean that it does not comply with the spec. It just defines 
a different
partition of the roles.

I read the spec again, and I fail to see how you deduce from this that 128 
should be the default.
Different devices will have different master preferences. Infrastructure 
devices will have >=128
which hints that they want to be master (consume more power) most probably 
because they
are powered. Other devices that would prefer to avoid being master should set 
their out of the
box master preference to <128.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Johannes Berg]

> Why NL80211_ATTR_NAN_MASTER_PREF is mandatory? The spec suggests to
> assume master preference value of 128 if not provided. Please see
> quoted text from NAN 1.0 specification:
> "A NAN Device which out-of-the-box will have a Master Preference
> greater than or equal to 128 with the intention of being a NAN Master
> Device"

I guess the question would be at which level of API this default would
be set? I'm perfectly happy setting it at the nl80211 level, but I
don't expect applications to (be able to) use the nl80211 API, and some
sort of management application could just as well set the default and
always provide the attribute.

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

Re: [PATCHv3 RESEND 06/11] cfg80211: Provide an API to report NAN function termination

[Malinen, Jouni]

On Tue, Mar 29, 2016 at 12:35:04PM +0300, Emmanuel Grumbach wrote:
> Provide a function that reports NAN DE function termination. The function
> may be terminated due to one of the following reasons: user request,
> ttl expiration or failure.

> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h

> +/**
> + * enum nl80211_nan_func_term_reason - NAN functions termination reason
> + *
> + * Defines termination reasons of a NAN function
> + *
> + * @NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST: requested by user
> + * @NL80211_NAN_FUNC_TERM_REASON_TTL_EXPIRED: timeout
> + * @NL80211_NAN_FUNC_TERM_REASON_ERROR: errored
> + */
> +enum nl80211_nan_func_term_reason {
> + NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST,
> + NL80211_NAN_FUNC_TERM_REASON_TTL_EXPIRED,
> + NL80211_NAN_FUNC_TERM_REASON_ERROR,
> +};

Can you please add NL80211_NAN_FUNC_TERM_REASON_VENDOR_SPECIFIC? That
would help vendors to implement and send vendor specific reason codes if
any.

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

Re: [PATCHv3 RESEND 05/11] cfg80211: provide a function to report a match for NAN

[Johannes Berg]

On Wed, 2016-04-06 at 09:38 +, Malinen, Jouni wrote:
> 
> > +void cfg80211_nan_match(struct wireless_dev *wdev,
> > +   struct cfg80211_nan_match_params *match,
> > gfp_t gfp);
> Looks like this function is common for publish, subscribe and follow
> up. If so, can we please have separate functions for publish,
> subscribe and followup to match the spec primitives?

Is it really worth exporting three functions just for the sake of that?
Perhaps better if we provide inline wrappers though that set the value
correctly?

Would you extend this request also to the nl80211 command value?

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

Re: [PATCHv3 RESEND 05/11] cfg80211: provide a function to report a match for NAN

[Malinen, Jouni]

On Tue, Mar 29, 2016 at 12:35:03PM +0300, Emmanuel Grumbach wrote:
> Provide a function the driver can call to report a match.
> This will send the event to the user space.

> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h

> +/**
> + * struct cfg80211_nan_match_params - NAN match parameters
> + * @type: the type of the function that triggered a match. If it is
> + *%NL80211_NAN_FUNC_SUBSCRIBE it means that we replied to a subscriber.
> + *If it is %NL80211_NAN_FUNC_PUBLISH, it means that we got a discovery
> + *result.
> + *If it is %NL80211_NAN_FUNC_FOLLOW_UP, we received a follow up.

> +/**
> + * cfg80211_nan_match - report a match for a NAN function.
> + * @wdev: the wireless device reporting the match
> + * @match: match notification parameters
> + * @gfp: allocation flags
> + *
> + * This function reports that the a NAN function had a match. This
> + * can be a subscribe that had a match or a solicited publish that
> + * was sent. It can also be a follow up that was received.
> + */
> +void cfg80211_nan_match(struct wireless_dev *wdev,
> + struct cfg80211_nan_match_params *match, gfp_t gfp);

Looks like this function is common for publish, subscribe and follow up.
If so, can we please have separate functions for publish, subscribe and
followup to match the spec primitives?

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

Re: NETLINK_URELEASE non-bound socket problem (was: [PATCH] Fix local DoS in cfg80211 subsystem)

[Johannes Berg]

On Wed, 2016-04-06 at 11:20 +0300, Dmitrijs Ivanovs wrote:
> Hi Johannes!
> 
> I will prepare patch which does not send NETLINK_URELEASE for unbound
> sockets as you suggest. But I think protocol check in nl80211 is
> still needed because port_id is unique per-protocol.
> 

Yes, good point. Can you please send that as a separate patch? That one
should have a

Fixes: 026331c4d9b5 ("cfg80211/mac80211: allow registering for and sending 
action frames")

tag. I'll apply this one right away, but the other one should probably
go through Dave's tree.

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

Re: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Malinen, Jouni]

On Tue, Mar 29, 2016 at 12:34:59PM +0300, Emmanuel Grumbach wrote:
> Define several attributes that may be configured by user space
> when starting NAN functionality (master preference and dual
> band operation)

> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> @@ -826,6 +826,16 @@
> + * @NL80211_CMD_START_NAN: Start NAN operation, identified by its
> + *   %NL80211_ATTR_WDEV interface. This interface must have been previously
> + *   created with %NL80211_CMD_NEW_INTERFACE. After it has been started, the
> + *   NAN interface will create or join a cluster. This command must have a
> + *   valid %NL80211_ATTR_NAN_MASTER_PREF attribute and optional
> + *   %NL80211_ATTR_NAN_DUAL attributes.


> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c

> +static int nl80211_start_nan(struct sk_buff *skb, struct genl_info *info)

> + if (!info->attrs[NL80211_ATTR_NAN_MASTER_PREF])
> + return -EINVAL;

Why NL80211_ATTR_NAN_MASTER_PREF is mandatory? The spec suggests to
assume master preference value of 128 if not provided. Please see quoted
text from NAN 1.0 specification:
"A NAN Device which out-of-the-box will have a Master Preference greater
than or equal to 128 with the intention of being a NAN Master Device"

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

[PATCH] NETLINK_URELEASE non-bound socket problem

[Dmitrijs Ivanovs]

Currently, any non-privileged user can create netlink socket
with port_id equal to port_id used by hostapd to create wireless network
interfaces on-the-fly when more than one BSS is configured. When such socket
is closed, nl80211 will receive socket release notification and such virtual
interfaces will be removed while hostapd is still running.

This patch introduces two additional checks to correct the problem:

1) Do not send netlink socket release notification when socket is not
bound.
2) Check protocol number in nl80211 netlink socket release notification
handler.

Signed-off-by: Dmitry Ivanov 
---
 net/netlink/af_netlink.c | 2 +-
 net/wireless/nl80211.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 215fc08..330ebd6 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -688,7 +688,7 @@ static int netlink_release(struct socket *sock)

 skb_queue_purge(>sk_write_queue);

-if (nlk->portid) {
+if (nlk->portid && nlk->bound) {
 struct netlink_notify n = {
 .net = sock_net(sk),
 .protocol = sk->sk_protocol,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 98c9242..056a730 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -13216,7 +13216,7 @@ static int nl80211_netlink_notify(struct
notifier_block * nb,
 struct wireless_dev *wdev;
 struct cfg80211_beacon_registration *reg, *tmp;

-if (state != NETLINK_URELEASE)
+if (state != NETLINK_URELEASE || notify->protocol != NETLINK_GENERIC)
 return NOTIFY_DONE;

 rcu_read_lock();
-- 
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 RESEND 10/11] mac80211: Add API to report nan function match

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> 
> +void ieee80211_nan_func_match(struct ieee80211_vif *vif,
> +   struct cfg80211_nan_match_params
> *match,
> +   gfp_t gfp)
> +{
> + struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> + struct ieee80211_nan_func *func;
> + struct wireless_dev *wdev;
> +
> + if (WARN_ON(vif->type != NL80211_IFTYPE_NAN))
> + return;
> +
> + spin_lock_bh(>u.nan.func_lock);
> +
> + func = ieee80211_find_nan_func(sdata, match->inst_id);
> + if (WARN_ON(!func)) {
> + spin_unlock_bh(>u.nan.func_lock);
> + return;
> + }
> + match->cookie = func->func.cookie;
> 
Ok maybe this requires a spinlock - due to how drivers want to call it
- but could also use RCU? Particularly with IDR that shouldn't be
difficult.

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

Re: iwlwifi monitor mode: No data frame captured on 5 Ghz

[Gucea Doru]

On Wed, Apr 6, 2016 at 12:39 AM, Reinoud Koornstra
 wrote:
> On Tue, Apr 5, 2016 at 7:25 AM, Gucea Doru  wrote:
>> On Tue, Apr 5, 2016 at 1:00 PM, Grumbach, Emmanuel
>>  wrote:

 Hello,

 I am trying to capture packets that are exchanged between an AP and a
 smartphone on the 5Ghz frequency. For generating traffic, I upload UDP
 traffic from a laptop PC towards the smartphone using iperf.

 The problem is that I can see _only_ the control frames like RTS/CTS, Block
 ACK, while the data packets are not captured. I uploaded the Wireshark
 capture files at [1] (located inside the folders whose name starts with 
 5Ghz).
>>>
>>> Most likely the packets on A band have a VHT preamble and your SKU is 11N 
>>> only.
>>
>> My card, Intel 7260 [1]  supports 802.11 ac. So it should also support
>> VHT, right? Is there any interface in user-space for checking after
>> VHT?
>>
>> However, I noticed a "failure" message in dmesg:
>> [4.030428] Intel(R) Wireless WiFi driver for Linux, in-tree:
>> [4.030570] iwlwifi :04:00.0: irq 37 for MSI/MSI-X
>> [4.030760] iwlwifi :04:00.0: Direct firmware load for
>> iwlwifi-7260-10.ucode failed with error -2
>> [4.035509] iwlwifi :04:00.0: loaded firmware version
>> 25.228.9.0 op_mode iwlmvm
>> [4.454772] iwlwifi :04:00.0: Detected Intel(R) Dual Band
>> Wireless N 7260, REV=0x144
>> [4.454825] iwlwifi :04:00.0: L1 Disabled - LTR Enabled
>> [4.455055] iwlwifi :04:00.0: L1 Disabled - LTR Enabled
>> [   15.049933] iwlwifi :04:00.0: L1 Disabled - LTR Enabled
>> [   15.050269] iwlwifi :04:00.0: L1 Disabled - LTR Enabled
>
> The ucode failure isn't a problem, you'll likely find that another
> firmware does load.
> I've got also a 7620, but mine does support 11ac, so mine does support VHT.
>
> [9.820637] iwlwifi :04:00.0: Detected Intel(R) Dual Band
> Wireless AC 7260, REV=0x144
>
> I also got a firmware load failure, but that's ok
>
> [8.399101] iwlwifi :04:00.0: Direct firmware load for
> iwlwifi-7260-17.ucode failed with error -2
>
> Cause here it does load the one it needs:
>
> [9.110486] iwlwifi :04:00.0: loaded firmware version
> 16.242414.0 op_mode iwlmvm
>
> I did fetch recent firmware by git to be up to date.
> I also added some instrumentation to get to see the unsupported splx
> structure and in my case it's just because of the package.count isn't
> what it should be.
> This isn't a problem as well really.
>
> Mar 14 00:32:36 router-dev kernel: [8.273274] iwlwifi
> :04:00.0: Unsupported splx structure
> Mar 14 00:32:36 router-dev kernel: [8.273276] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: splx->type is 0x04
> Mar 14 00:32:36 router-dev kernel: [8.273277] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: splx->package.count is 4
> Mar 14 00:32:36 router-dev kernel: [8.273277] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: splx->package.elements[0].type is 0x01
> Mar 14 00:32:36 router-dev kernel: [8.273278] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: splx->package.elements[0].integer.value is 0
> Mar 14 00:32:36 router-dev kernel: [8.273279] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: limits->type is 0x04
> Mar 14 00:32:36 router-dev kernel: [8.273280] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: limits->package.count is 3
> Mar 14 00:32:36 router-dev kernel: [8.273281] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: limits->package.elements[0].type is 0x01
> Mar 14 00:32:36 router-dev kernel: [8.273281] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: limits->package.elements[1].type is 0x01
> Mar 14 00:32:36 router-dev kernel: [8.273282] iwlwifi
> :04:00.0: drivers/net/wireless/iwlwifi/pcie/drv.c,
> splx_get_pwr_limit: WiFi power is not limited,
> domain_type->integer.value is 0
>
> I compiled wpa_supplicant and it connects without trouble to several
> access points.
> I mostly do it this way, because I maintain the mtk driver I use in
> the access point here so I get to see extensive info from both sides
> when things go pearshape. :)
>

Thanks a lot for your support: as Emmanuel pointed out the problem
comes from my Wi-Fi flavour card.

>>
>> Also, the maximum bit rate reported by iwconfig is 150 Mb/s, so my
>> assumption is that the card can't enter into the 802.11 ac mode, it
>> just stays into 802.11n.
>>
>> doru@doru-N551JK:~$ iwconfig wlan0
>> wlan0 IEEE 802.11abgn  ESSID:"5_mptcp"
>>   Mode:Managed  Frequency:5.24 GHz  Access Point: C4:6E:1F:4B:10:A2
>>   Bit Rate=150 Mb/s   

Re: [PATCHv3 RESEND 09/11] mac80211: Implement add_nan_func and rm_nan_func

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:

> + * @rm_nan_func: Remove a nan function. The driver must call
> + * ieee80211_nan_func_terminated() with
> + * NL80211_NAN_FUNC_TERM_REASON_USER_REQUEST reason code upon
> removal.

bad indentation.

Also here: nan -> NAN.

> + /* Only set max_nan_de_entries as available to honor the
> device's
> +  * limitations
> +  */
> + bitmap_set(sdata->u.nan.func_ids, 1,
> +    sdata->local->hw.max_nan_de_entries);

That doesn't make a lot of sense to me. What are you trying to do?

> + inst_id = find_first_bit(sdata->u.nan.func_ids,
> +  IEEE80211_MAX_NAN_INSTANCE_ID + 1);
> + if (inst_id == IEEE80211_MAX_NAN_INSTANCE_ID + 1)
> + return -ENOBUFS;

Wouldn't you use max_nan_de_entries here instead of
MAX_NAN_INSTANCE_ID, after validating that the former is actually
smaller than or equal to the latter?

Also, the logic says that the variable should be called
"available_func_ids", although I'd prefer "used_func_ids" after
adjusting the logic according to the comments above.

> + cfg80211_clone_nan_func_members(>func, nan_func);

This is quite obviously missing error checking, but see the discussion
in the patch that introduced it.

> + spin_lock_bh(>u.nan.func_lock);
> + clear_bit(inst_id, sdata->u.nan.func_ids);
> + list_add(>list, >u.nan.functions_list);
> + spin_unlock_bh(>u.nan.func_lock);
> +
> + ret = drv_add_nan_func(sdata->local, sdata, nan_func);
> + if (ret) {
> + spin_lock_bh(>u.nan.func_lock);
> + set_bit(inst_id, sdata->u.nan.func_ids);
> + list_del(>list);
> + spin_unlock_bh(>u.nan.func_lock);
> +
> + cfg80211_free_nan_func_members(>func);
> + kfree(func);
> + }
> +
> + return ret;
> +}

> +static struct ieee80211_nan_func *
> +ieee80211_find_nan_func(struct ieee80211_sub_if_data *sdata, u8
> instance_id)
> +{
> + struct ieee80211_nan_func *func;
> +
> + lockdep_assert_held(>u.nan.func_lock);
> +
> + list_for_each_entry(func, >u.nan.functions_list,
> list) {
> + if (func->func.instance_id == instance_id)
> + return func;
> + }
> +
> + return NULL;
> +}

Arguably though, this whole thing just screams "idr" [1] and then you
don't even need the list_head in the cfg80211 struct.

[1] include/linux/idr.h

> +static struct ieee80211_nan_func *
> +ieee80211_find_nan_func_by_cookie(struct ieee80211_sub_if_data
> *sdata,
> +   u64 cookie)
> +{
> + struct ieee80211_nan_func *func;
> +
> + lockdep_assert_held(>u.nan.func_lock);
> +
> + list_for_each_entry(func, >u.nan.functions_list,
> list) {
> + if (func->func.cookie == cookie)
> + return func;
> + }
> +
> + return NULL;
> +}

Although this might be more difficult then, but you always
have idr_for_each_entry().

> + case NL80211_IFTYPE_NAN:
> + /* clean all the functions */
> + spin_lock_bh(>u.nan.func_lock);
> + list_for_each_entry_safe(func, tmp_func,
> +  
> >u.nan.functions_list, list) {
> + list_del(>list);
> + cfg80211_free_nan_func_members(>func);
> + kfree(func);
> + }
> + spin_unlock_bh(>u.nan.func_lock);
> + break;
>   case NL80211_IFTYPE_P2P_DEVICE:
>   /* relies on synchronize_rcu() below */
>   RCU_INIT_POINTER(local->p2p_sdata, NULL);
>   /* fall through */
> - case NL80211_IFTYPE_NAN:

any particular reason you're moving the case label?

>   default:
>   cancel_work_sync(>work);
>   /*
> @@ -1453,9 +1464,15 @@ static void ieee80211_setup_sdata(struct
> ieee80211_sub_if_data *sdata,
>   case NL80211_IFTYPE_WDS:
>   sdata->vif.bss_conf.bssid = NULL;
>   break;
> + case NL80211_IFTYPE_NAN:
> + bitmap_zero(sdata->u.nan.func_ids,
> + IEEE80211_MAX_NAN_INSTANCE_ID + 1);
> + INIT_LIST_HEAD(>u.nan.functions_list);
> + spin_lock_init(>u.nan.func_lock);
> + sdata->vif.bss_conf.bssid = sdata->vif.addr;
> + break;
>   case NL80211_IFTYPE_AP_VLAN:
>   case NL80211_IFTYPE_P2P_DEVICE:
> - case NL80211_IFTYPE_NAN:

also here?

> + if (!local->hw.max_nan_de_entries)
> + local->hw.max_nan_de_entries =
> IEEE80211_MAX_NAN_INSTANCE_ID;

Need a max check also, I guess?

> +/* TODO: record more fields */

...
but isn't cfg80211 recording them anyway?

> +static int ieee80211_reconfig_nan(struct ieee80211_sub_if_data
> *sdata)
> +{
> + struct ieee80211_nan_func *func, *ftmp;
> + LIST_HEAD(tmp_list);
> + int res;
> +
> + res = drv_start_nan(sdata->local, sdata,
> + 

Re: [PATCHv3 RESEND 08/11] mac80211: implement nan_change_conf

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> 
>   * @start_nan: join an existing nan cluster, or create a new one.
>   * @stop_nan: leave the nan cluster.
> + * @nan_change_conf: change nan configuration. The data in
> cfg80211_nan_conf
> + *   contains full new configuration and changes specify which
> parameters

@changes I guess? at least you should be consistent, even if there's no
perfectly correct syntax here.

Also please specify where the change flags come from.

I'm also not sure that the description is actually correct? How can
both "contains [the] full new configuration" and "changes speicfy which
parameters" be correct? You have a full new configuration but still
want to indicate the changes?

> + *   are changed with respect to the last nan config.
> 

Also, more nan vs. NAN, I'm sure I didn't comment on them all.

> + int (*nan_change_conf)(struct ieee80211_hw *hw,
> +    struct ieee80211_vif *vif,
> +    struct cfg80211_nan_conf *conf, u8
> changes);

An earlier patch in your series used u32, why u8 here?

> + memcpy(>u.nan.nan_conf, conf, sizeof(sdata-
> >u.nan.nan_conf));

why not use struct assignment?

sdata->u.nan.conf = conf;

> + memcpy(>u.nan.nan_conf, _conf,
> +    sizeof(sdata->u.nan.nan_conf));

ditto

> +/**
> + * struct ieee80211_if_nan - NAN state
> + *
> + * @nan_conf: current nan configuration
> + */
> +struct ieee80211_if_nan {
> + struct cfg80211_nan_conf nan_conf;
> +};

There's no point in calling it nan_conf since it's within a nan struct
and then later called "nan.nan_conf"...

> + __field(u32, changes)

You're not being very consistent with the type of the "changes"
parameter :)

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

Re: [PATCHv3 RESEND 07/11] cfg80211: add utility functions to clone and free nan_func

[Johannes Berg]

> +int cfg80211_clone_nan_func_members(struct cfg80211_nan_func *f1,
> + const struct cfg80211_nan_func *f2)
> +{
> + memcpy(f1, f2, sizeof(*f1));

That's pretty weird. And the only user of this (in a later patch) is
first allocating the f1. I think this function should be changed to
also allocate the entire struct cfg80211_nan_func, with all the
contents.

Yes, mac80211 is actually allocating a bit more - but only a list_head.
I think I'd prefer just putting a "struct list_head list" into the
cfg80211 struct "for driver use" and getting rid of all of these
contortions.


With some care, I'm pretty sure you could even get away with a single
allocation that's big enough to cover everything, so that you don't
need to have cfg80211_free_nan_func_members() exported but can simply
kfree() the pointer returned from this function.

That's basically doing 

size = sizeof(*dst);
size += src->serv_spec_info_len;
size += src->srf_bf_len;
size += src->srf_num_macs * size(...);
size += 
size += 

and then using pointers to the single block.

The only problem might be if that can get really large, but I think it
would make memory management simpler.

In fact, it'd be *really* nice if that could also be done when parsing
this from nl80211.


Additionally, and alternatively to exporting this function from
cfg80211, why don't you change the rules around the memory?
If nl80211_nan_add_func() doesn't put the func on the stack, but
allocates it, then rdev_add_nan_func() can be forced to take ownership
of the pointer. That way, there's no need to even copy the data at all.
Yes, that'd have to be documented (particularly whether or not it also
takes ownership in error cases, it probably should), but overall I'm
pretty sure it'd simplify things.

Even if we *don't* get away with putting everything into a single
allocation in nl80211 - that might be too complicated - we'd only have
to export the free function and would never copy around things.

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

Re: [PATCHv3 RESEND 06/11] cfg80211: Provide an API to report NAN function termination

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> 
> @@ -2246,6 +2248,7 @@ enum nl80211_attrs {
>   NL80211_ATTR_NAN_FUNC,
>   NL80211_ATTR_NAN_FUNC_INST_ID,
>   NL80211_ATTR_NAN_MATCH,
> + NL80211_ATTR_NAN_FUNC_TERM_REASON,

You can add this as a nested function attribute. If we remove the
FUNC_INST_ID attribute we have to do the nesting anyway.

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

Re: [PATCHv3 RESEND 05/11] cfg80211: provide a function to report a match for NAN

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> 
> +/**
> + * enum nl80211_nan_match_attributes - NAN match attributes
> + * @__NL80211_NAN_MATCH_INVALID: invalid
> + * @NL80211_NAN_MATCH_FUNC_TYPE:  nl80211_nan_function_type
> (u8). This is
> + *   the type of the function which had a match.

> + * @NL80211_NAN_MATCH_INSTANCE_ID: The instance ID of the local
> function that
> + *   had a match. This is a u8.

It would make sense to report this using the proper
NL80211_ATTR_NAN_FUNC_INST_ID.

In a previous email I just said you can remove that, so then you'd have to do 
some nesting here, and then you can easily also use the FUNC_TYPE from the 
function attributes.

Having two sets of identical attributes is quite odd, IMHO.

> + * @NL80211_NAN_MATCH_MAC: The MAC address of the peer. This
> attribute is
> + *   binary.

This is debatable, I guess, you could also use the top-level attribute.

> +nla_put_failure:
> + genlmsg_cancel(msg, hdr);
> + nlmsg_free(msg);

No need to cancel before free :)

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

Re: [PATCHv3 RESEND 03/11] cfg80211: add add_nan_func / rm_nan_func

[Johannes Berg]

> + NL80211_ATTR_NAN_FUNC_INST_ID,

I don't see why this has to be a top-level attribute.

> +enum nl80211_nan_func_attributes {
> + __NL80211_NAN_FUNC_INVALID,
> + NL80211_NAN_FUNC_TYPE,

You can easily move it into here, and 

> + /* propagate the instance id and cookie to userspace  */
> + if (WARN_ON(nla_put_u8(msg, NL80211_ATTR_NAN_FUNC_INST_ID,
> +    func.instance_id) ||
> + nla_put_u64(msg, NL80211_ATTR_COOKIE,
> func.cookie))) {

use the appropriate nesting here.

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

Re: [PATCHv3 RESEND 04/11] cfg80211: allow the user space to change current NAN configuration

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> Some NAN configuration paramaters 

parameters

>  /**
> + * enum cfg80211_nan_conf_changes - indicates changed fields in nan
> configuration

again, NAN please

> + *
> + * @CFG80211_NAN_CONF_CHANGED_PREF: master preference
> + * @CFG80211_NAN_CONF_CHANGED_DUAL: dual band operation
> + */
> +enum cfg80211_nan_conf_changes {
> + CFG80211_NAN_CONF_CHANGED_PREF = BIT(0),
> + CFG80211_NAN_CONF_CHANGED_DUAL = BIT(1),
> +};

I also think this enum should be declared next to struct
cfg80211_nan_conf, not far later in the file.

> + * @nan_change_conf: changes NAN configuration. The changed
> parameters must
> + *   be specified in @changes. All other parameters must be
> ignored.

This is misleading. "The changed parameters are specified by the bitmap
@changes (using  cfg80211_nan_conf_changes); all other parameters
must be ignored" or so. You should always write this for driver
authors.

>   * @NL80211_ATTR_NAN_MASTER_PREF: the master preference to be used
> by
> - *   _CMD_START_NAN. Its type is u8 and it can't be 0.
> + *   _CMD_START_NAN and optionally with
> + *   _CMD_CHANGE_NAN_CONFIG. Its type is u8 and it
> can't be 0.

I didn't notice this before, but generally the & is wrong. That will
try to link to that as a struct/enum/union name, which clearly isn't
the case here. You can use @, perhaps, or just % for a constant.

>   *   Also, values 1 and 255 are reserved for certification
> purposes and
>   *   should not be used during a normal device operation.
>   * @NL80211_ATTR_NAN_DUAL: NAN dual band operation config (see
>   *nl80211_nan_dual_band_conf). This attribute is used 

Here the & is correct.

> - *   _CMD_START_NAN.
> + *   _CMD_START_NAN and optionally with
> + *   _CMD_CHANGE_NAN_CONFIG.

Here it's wrong again.

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

Re: [PATCHv3 RESEND 03/11] cfg80211: add add_nan_func / rm_nan_func

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:

> + * @cookie: user defined cookie (will be returned with
> notifications)

Didn't we change it to not be user defined?

> + * @NL80211_NAN_FUNC_TTL: number of DWs this function should stay
> active. 0 is
> + *   equivalent to no TTL at all. This is a u32.

I think it should rather be "if the attribute is not specified, no TTL
is used", with 0 being an invalid value.

> + * @NL80211_NAN_FUNC_RX_MATCH_FILTER: Receive Matching filter. This
> is a nested
> + *   attribute. It is a list of binary values.

That probably needs to say what kind if "binary values"?

> + * @NL80211_NAN_FUNC_TX_MATCH_FILTER: Transmit Matching filter. This
> is a
> + *   nested attribute. It is a list of binary values.

Ditto.

> + * @NL80211_NAN_SRF_INCLUDE: true if the include bit of the SRF set.
> + *   This is a flag.
> + * @NL80211_NAN_SRF_TYPE_BF: true if the SRF is a Bloom Filter SRF.
> If false
> + *   the SRF will be _ATTR_MAC_ADDRS. This is a flag.

"true" and "false" aren't really states for a flag, it can be
"specified" or "not specified", or maybe "present" and "not present".

> + * @NL80211_NAN_SRF_BF: Bloom Filter. Relevant and mandatory if
> + *   _NAN_SRF_TYPE_BF is true. This attribute is
> binary.

Likewise.
However, why do you even need two attributes? It doesn't seem relevant
to specify the NAN_SRF_BF attribute when NAN_SRF_TYPE_BF isn't set?

> + * @NL80211_NAN_SRF_BF_IDX: index of the Bloom Filter. Relevant and
> + *   mandatory if _NAN_SRF_TYPE_BF is true. This is a
> u8.

Likewise - what do you need the SRF_TYPE_BF flag for at all?

> + * @NL80211_NAN_SRF_MAC_ADDRS: list of MAC addresses for the SRF.
> Relevant and
> + *   mandatory if _NAN_SRF_TYPE_BF is false. This is a
> nested
> + *   attribute. Each nested attribute is a MAC address.

And this is obviously the opposite - so both SRF_MAC_ADDRS and
SRF_BR/BF_IDX can't be specified together. No need for the flag?


> + if (tx) {
> + func->num_tx_filters = (u8)n_entries;
> + func->tx_filters = filter;
> + } else {
> + func->num_rx_filters = (u8)n_entries;
> + func->rx_filters = filter;

No need for those casts.

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

Re: [PATCHv3 RESEND 02/11] mac80211: add boilerplate code for start / stop NAN

[Johannes Berg]

On Tue, 2016-03-29 at 12:35 +0300, Emmanuel Grumbach wrote:
> This codes doens't do much besides allowing to start and
> stop the vif.

Few typos? :)

> + * @start_nan: join an existing nan cluster, or create a new one.
> + * @stop_nan: leave the nan cluster.

NAN

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

Re: [PATCHv3 RESEND 01/11] cfg80211: add start / stop NAN commands

[Johannes Berg]

>  /**
> + * struct cfg80211_nan_conf - nan configuration
> + *
> + * This struct defines nan configuration parameters

I think you should consistently capitalize "NAN" in comments.

> + * @start_nan: Start the NAN interface.
> + * @stop_nan: Stop the NAN interface.

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

Re: [PATCH v2] brcmfmac: Don't increase 8021x_cnt for dropped packets

[Arend Van Spriel]

On 5-4-2016 22:01, per.for...@gmail.com wrote:
> From: Per Forlin 
> 
> The pend_8021x_cnt gets into a state where it's not being decreased.
> When the brcmf_netdev_wait_pend8021x is called it will timeout
> because there are no pending packets to be consumed. There is no
> easy way of getting out of this state once it has happened.
> Preventing the counter from being increased in case of dropped
> packets seem like a reasonable solution.
> 
> Log showing overflow txq and increased cnt:
> 
> // Extra debug prints
> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2
> 
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
> 
> // Extra debug prints
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4
> 
> WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
>   (warn_slowpath_common)
>   (warn_slowpath_null)
>   (brcmf_netdev_wait_pend8021x [brcmfmac])
>   (send_key_to_dongle [brcmfmac])
>   (brcmf_cfg80211_del_key [brcmfmac])
>   (nl80211_del_key [cfg80211])
>   (genl_rcv_msg)
>   (netlink_rcv_skb)
>   (genl_rcv)
>   (netlink_unicast)
>   (netlink_sendmsg)
>   (sock_sendmsg)
>   (___sys_sendmsg)
>   (__sys_sendmsg)
>   (SyS_sendmsg)
> 
> Signed-off-by: Per Forlin 
> ---
> I came across this bug in an older kernel but it seems relevant
> for the latest kernel as well. I'm not familiar with the code
> but I can reproduce the issue and verify a fix for it.
> With this patch I no longer get stuck with a pend8021_cnt > 0.
> 
> Change log:
>  v2 - Add variable to know whether the counter is increased or not

Sorry for the late response. Can you elaborate where you are seeing this.

What kind of chipset are you using?
What kernel version are you running?

The function brcmf_fws_process_skb() should call brcmf_txfinalize() in
case of an error and it does in latest kernel. There the count is
decreased. We had an issue mapping to the right ifp as reported by
Rafał, but that has also been fixed recently. So main question here:

Do you see the issue in the latest kernel?

Regards,
Arend

>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> index ed9998b..de80ad8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
> @@ -196,6 +196,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff 
> *skb,
>   struct brcmf_if *ifp = netdev_priv(ndev);
>   struct brcmf_pub *drvr = ifp->drvr;
>   struct ethhdr *eh = (struct ethhdr *)(skb->data);
> + bool pend_8021x_cnt_increased = false;
>  
>   brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>  
> @@ -233,14 +234,18 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct 
> sk_buff *skb,
>   goto done;
>   }
>  
> - if (eh->h_proto == htons(ETH_P_PAE))
> + if (eh->h_proto == htons(ETH_P_PAE)) {
>   atomic_inc(>pend_8021x_cnt);
> + pend_8021x_cnt_increased = true;
> + }
>  
>   ret = brcmf_fws_process_skb(ifp, skb);
>  
>  done:
>   if (ret) {
>   ifp->stats.tx_dropped++;
> + if (pend_8021x_cnt_increased)
> + atomic_dec(>pend_8021x_cnt);
>   } else {
>   ifp->stats.tx_packets++;
>   ifp->stats.tx_bytes += skb->len;
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: NETLINK_URELEASE non-bound socket problem (was: [PATCH] Fix local DoS in cfg80211 subsystem)

[Dmitrijs Ivanovs]

Hi Johannes!

I will prepare patch which does not send NETLINK_URELEASE for unbound
sockets as you suggest. But I think protocol check in nl80211 is still
needed because port_id is unique per-protocol.

On Tue, Apr 5, 2016 at 12:56 PM, Johannes Berg
 wrote:
> Hi Dmitrijs,
>
> Thanks for reporting this problem.
>
>> The patch below corrects this problem in kernel space.
>
> I don't think that this is correct, there are four more users of
> NETLINK_URELEASE (nfnetlink, NFC), and afaict all of them have the same
> bug as nl80211.
>
> Rather than fix all of them, I think we should simply not report
> NETLINK_URELEASE for netlink sockets that weren't bound; if any user
> comes up that requires them later we could add a new event instead.
>
> I can't find what commit introduced this code, it goes back before git
> history, so I don't have the commit log. Maybe it was done for
> nfnetlink log/queue? Certainly both nl80211 and NFC are much newer.
>
>> Also, it is
>> recommended to ensure that user-space applications are not using
>> user-supplied port_id for netlink sockets (which is default in
>> libnl-tiny for example).
>
> This I think we should remove from the commit log - it's misleading and
> there's no point.
>
> johannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: debugfs var for the default aggregation timeout.

[Johannes Berg]

On Tue, 2016-04-05 at 19:46 -0400, Avery Pennarun wrote:

> This test was with backports-20150525 on ath9k.  (We have newer
> versions in the queue, but they haven't rolled out to our customers
> yet.  Anyway, earlier in this thread, I was able to trigger the race
> condition on much newer backports.  Unfortunately the current fix
> makes my reproducible test case go away, but I don't know any reason
> to assume the race condition is fixed.)

Well, we know that the timeout is likely unrelated to the issue (other
than not triggering the broken code path that frequently), so you can
revert the timeout change for the test case.

> While we're here, unfortunately it turns out that just observing the
> agg_status file can cause crashes (though not very often... except
> for a few unlucky customers), probably due to a different race
> condition.
> Any suggestions about this one?  Stack trace attached below.  (I
> think the stack trace suggests a mac80211 problem?)
> 

That has to be a mac80211 problem, yeah.
(Side note: I'm a bit surprised this is a 32-bit system?)

Looks like we use RCU protection to get the data. Can I get the
mac80211.ko binary (with debug data) corresponding to the crash below?

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

RE: [PATCH] wlcore: spi: add wl18xx support

[Reizer, Eyal]

Ping on this patch

> -Original Message-
> From: Eyal Reizer [mailto:eyalrei...@gmail.com]
> Sent: Wednesday, March 30, 2016 4:07 PM
> To: kv...@codeaurora.org; linux-wireless@vger.kernel.org;
> net...@vger.kernel.org; linux-ker...@vger.kernel.org
> Cc: Reizer, Eyal
> Subject: [PATCH] wlcore: spi: add wl18xx support
> 
> Add support for using with both wl12xx and wl18xx.
> 
> - all wilink family needs special init command for entering wspi mode.
>   extra clock cycles should be sent after the spi init command while the
>   cs pin is high.
> - switch to controling the cs pin from the spi driver for achieveing the
>   above.
> - the selected cs gpio is read from the spi device-tree node using the
>   cs-gpios field and setup as a gpio.
> - See the example below for specifying the cs gpio using the cs-gpios entry
> 
>  {
>   status = "okay";
>   pinctrl-names = "default";
>   pinctrl-0 = <_pins>;
>   cs-gpios = < 5 0>;
>   #address-cells = <1>;
>   #size-cells = <0>;
>   wlcore: wlcore@0 {
>   compatible = "ti,wl1835";
>   vwlan-supply = <_en_reg>;
>   spi-max-frequency = <4800>;
>   reg = <0>;  /* chip select 0 on spi0, ie spi0.0 */
>   interrupt-parent = <>;
>   interrupts = <27 IRQ_TYPE_EDGE_RISING>;
>   };
> };
> 
> Signed-off-by: Eyal Reizer 
> ---
>  drivers/net/wireless/ti/wlcore/spi.c |  176
> ++
>  1 file changed, 157 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c
> b/drivers/net/wireless/ti/wlcore/spi.c
> index 96d9c9d..6c5a369 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include "wlcore.h"
>  #include "wl12xx_80211.h"
> @@ -70,16 +71,30 @@
>  #define WSPI_MAX_CHUNK_SIZE4092
> 
>  /*
> - * only support SPI for 12xx - this code should be reworked when 18xx
> - * support is introduced
> + * wl18xx driver aggregation buffer size is (13 * PAGE_SIZE) compared
> + to
> + * (4 * PAGE_SIZE) for wl12xx, so use the larger buffer needed for
> + wl18xx
>   */
> -#define SPI_AGGR_BUFFER_SIZE (4 * PAGE_SIZE)
> +#define SPI_AGGR_BUFFER_SIZE (13 * PAGE_SIZE)
> 
>  /* Maximum number of SPI write chunks */  #define
> WSPI_MAX_NUM_OF_CHUNKS \
>   ((SPI_AGGR_BUFFER_SIZE / WSPI_MAX_CHUNK_SIZE) + 1)
> 
> 
> +struct wilink_familiy_data {
> + char name[8];
> +};
> +
> +const struct wilink_familiy_data *wilink_data;
> +
> +static const struct wilink_familiy_data wl18xx_data = {
> + .name = "wl18xx",
> +};
> +
> +static const struct wilink_familiy_data wl12xx_data = {
> + .name = "wl12xx",
> +};
> +
>  struct wl12xx_spi_glue {
>   struct device *dev;
>   struct platform_device *core;
> @@ -120,6 +135,8 @@ static void wl12xx_spi_init(struct device *child)
>   struct spi_transfer t;
>   struct spi_message m;
>   u8 *cmd = kzalloc(WSPI_INIT_CMD_LEN, GFP_KERNEL);
> + struct spi_device *spi = (struct spi_device *)glue->dev;
> + struct spi_master *master = spi->master;
> 
>   if (!cmd) {
>   dev_err(child->parent,
> @@ -127,6 +144,15 @@ static void wl12xx_spi_init(struct device *child)
>   return;
>   }
> 
> + if (!master->cs_gpios) {
> + dev_err(child->parent,
> + "spi chip select pin missing in platform data!\n");
> + return;
> + }
> +
> + /* Drive CS line low */
> + gpio_direction_output(master->cs_gpios[0], 0);
> +
>   memset(, 0, sizeof(t));
>   spi_message_init();
> 
> @@ -163,6 +189,26 @@ static void wl12xx_spi_init(struct device *child)
>   spi_message_add_tail(, );
> 
>   spi_sync(to_spi_device(glue->dev), );
> +
> + /* Send extra clocks with CS high. this is required by the wilink
> +  * family in order for successfully enter WSPI mode
> +  */
> + gpio_direction_output(master->cs_gpios[0], 1);
> +
> + memset(, 0, sizeof(m));
> + spi_message_init();
> +
> + cmd[0] = 0xff;
> + cmd[1] = 0xff;
> + cmd[2] = 0xff;
> + cmd[3] = 0xff;
> + swab32s((u32 *)cmd);
> +
> + t.tx_buf = cmd;
> + t.len = 4;
> + spi_message_add_tail(, );
> + spi_sync(to_spi_device(glue->dev), );
> +
>   kfree(cmd);
>  }
> 
> @@ -213,6 +259,16 @@ static int __must_check
> wl12xx_spi_raw_read(struct device *child, int addr,
>   u32 *busy_buf;
>   u32 *cmd;
>   u32 chunk_len;
> + struct spi_device *spi = (struct spi_device *)glue->dev;
> + struct spi_master *master = spi->master;
> +
> + if (!master->cs_gpios) {
> + dev_err(child->parent,
> + "spi chip select pin missing in platform data!\n");
> + return -EINVAL;
> + }
> + /* Drive CS line low */
> + gpio_direction_output(master->cs_gpios[0], 0);
> 
>   while (len > 0) {
> 

Re: [PATCH 4/7] wil6210: print debug message when transmitting while disconnected

[Joe Perches]

On Tue, 2016-04-05 at 14:24 +0300, Maya Erez w
> Network stack can try to transmit data while AP / STA is
> disconnected.
> Change this print-out to debug level as this should not be
> handled as error.

Should probably say something about adding ratelimited
logging functions

> diff --git a/drivers/net/wireless/ath/wil6210/debug.c 
> b/drivers/net/wireless/ath/wil6210/debug.c
[]
> @@ -49,6 +49,23 @@ void __wil_err_ratelimited(struct wil6210_priv *wil, const 
> char *fmt, ...)
>   }
>  }
>  
> +void __wil_dbg_ratelimited(struct wil6210_priv *wil, const char *fmt, ...)
> +{
> + if (net_ratelimit()) {

Inverting the test would reduce indentation.

> + struct net_device *ndev = wil_to_ndev(wil);
> + struct va_format vaf = {
> + .fmt = fmt,
> + };
> + va_list args;
> +
> + va_start(args, fmt);
> + vaf.va = 
> + netdev_dbg(ndev, "%pV", );
> + trace_wil6210_log_dbg();
> + va_end(args);
> + }
> +}

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

Re: [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Johannes Berg]

[removing other lists since they spam me with moderation bounces]

> The hope had been the original codel.h would have been reusable,
> which is not the case at present.

So what's the strategy for making it happen? Unless there is one, I
don't see the point in making the code more complicated than it already
has to be anyway.

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

Re: [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Johannes Berg]

On Wed, 2016-04-06 at 07:35 +0200, Michal Kazior wrote:

> I just wanted to follow the suggested/implied usage of codel code and
> keep modifications to a minimum. I could very well just assimilate it
> if you wish.

I don't really feel all that strongly about it, but I also don't see
the point. It makes it harder to look for the code though, and that
seems fairly pointless.

Btw, just realized that there's also __u32 in there which you should
probably remove and use just u32. Also don't #include 


> This follows net/sched/sch_fq_codel.h. I can put up a comment to
> explain what it's supposed to do?
> 

Ok, fair enough.

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

Re: [PATCH 1/7] wil6210: add function name to wil log macros

[Joe Perches]

On Tue, 2016-04-05 at 14:24 +0300, Maya Erez wrote:
> Add __func__ to all wil log macros for easier debugging.

I think this is unnecessary and merely bloats code size.
For all the _dbg calls, dynamic debug can add function
names if desired.

If really desired, I suggest changing the logging
functions to use "%ps and __builtin_return_address(0)

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

Re: [Make-wifi-fast] [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Michal Kazior]

On 6 April 2016 at 08:03, Jonathan Morton  wrote:
>
>> On 6 Apr, 2016, at 08:35, Michal Kazior  wrote:
>>
>> Packets can be destined to different stations/txqs. At enqueue time I
>> do a partial hash of a packet to get an "index" which I then use to
>> address a txq_flow from per-radio list (out of 4096 of them). You can
>> end up with a situtation like this:
>> - packet A hashing to X destined to txq P which is VI
>> - packet B hashing to X destined to txq Q which is BK
>>
>> You can't use the same txq_flow for both A and B because you want to
>> maintain packets per txqs more than you want to maintain them per flow
>> (you don't want to queue BK traffic onto VI or vice versa as an
>> artifact, do you? ;). When a txq_flow doesn't have a txqi it is bound.
>> Later, if a collision happens (i.e. resulting txq_flow has non-NULL
>> txqi) the "embedded" per-txq flow is used:
>>
>>  struct txq_info {
>> - struct sk_buff_head queue;
>> + struct txq_flow flow; // <--- this
>>
>> When txq_flow becomes empty its txqi is reset.
>>
>> The embedded flow is otherwise treated like any other flow, i.e. it
>> can be linked to old_flows and new_flows.
>
> This smells like a very fragile and complex solution to the collision 
> problem.  You may want to look at how Cake solves it.
>
> I use a separate pool of flows per traffic class (essentially, VO/VI/BE/BK), 
> and there is also a set-associative hash to take care of the birthday 
> problem.  The latter has an order-of-magnitude effect on the general flow 
> collision rate once you get into the tens of flows, for very little CPU cost.

When a driver asks mac80211 to dequeue given txq it implies a
destination station as well. This is important because 802.11
aggregation can be performed only on groups of packets going to a
single station on a single tid.

Cake - as I understand it - doesn't really *guarantee* maintaining
this. Keep in mind you can run with hundreds of stations connected.

You don't really want to burden drivers with sorting this grouping up
themselves (and hence coerce them into introducing another level of
intermediate queues, bis).

Without the per-txq fallback flow (regardless of using fq_codel-like
scheme or cake-like scheme in mac80211) you'll need to modify
codel_dequeue() itself to compensate and re-queue/skip frames not
belonging to requested txq.

I'm not sure it's worth it for initial fair-queuing implementation.


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

Re: [Make-wifi-fast] [PATCHv2 1/2] mac80211: implement fair queuing per txq

[Jonathan Morton]

> On 6 Apr, 2016, at 08:35, Michal Kazior  wrote:
> 
> Packets can be destined to different stations/txqs. At enqueue time I
> do a partial hash of a packet to get an "index" which I then use to
> address a txq_flow from per-radio list (out of 4096 of them). You can
> end up with a situtation like this:
> - packet A hashing to X destined to txq P which is VI
> - packet B hashing to X destined to txq Q which is BK
> 
> You can't use the same txq_flow for both A and B because you want to
> maintain packets per txqs more than you want to maintain them per flow
> (you don't want to queue BK traffic onto VI or vice versa as an
> artifact, do you? ;). When a txq_flow doesn't have a txqi it is bound.
> Later, if a collision happens (i.e. resulting txq_flow has non-NULL
> txqi) the "embedded" per-txq flow is used:
> 
>  struct txq_info {
> - struct sk_buff_head queue;
> + struct txq_flow flow; // <--- this
> 
> When txq_flow becomes empty its txqi is reset.
> 
> The embedded flow is otherwise treated like any other flow, i.e. it
> can be linked to old_flows and new_flows.

This smells like a very fragile and complex solution to the collision problem.  
You may want to look at how Cake solves it.

I use a separate pool of flows per traffic class (essentially, VO/VI/BE/BK), 
and there is also a set-associative hash to take care of the birthday problem.  
The latter has an order-of-magnitude effect on the general flow collision rate 
once you get into the tens of flows, for very little CPU cost.

 - Jonathan Morton

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