Email change

[Michal Kazior]

Hi everyone,

I'm parting ways with Tieto today so my email is going  to become
defunct. I'll be reachable via: kazikcz at gmail dot com.


Michał

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

[Michal Kazior]

On 1 February 2017 at 09:33, Pali Rohár  wrote:
> On Tuesday 31 January 2017 07:59:18 Tony Lindgren wrote:
>> * Kalle Valo  [170130 22:36]:
[...]
>> > * before distro updates linux-firmware create yours own deb/rpm/whatever
>> >   package "wl1251-firmware" which installs your flavor of nvs file (or
>> >   the user fallback helper if more dynamic functionality is preferred)
>>
>> And that won't work when using the same file system on other machines.
>>
>> Think NFSroot for example. At least I'm using the same NFSroot across
>> about 15 different machines including one n900 macro board with smc91x
>> Ethernet.
>
> Exactly problem which we already discussed in previous emails. You
> cannot serve one file (loaded by direct request_firmware) when your
> rootfs is readonly, e.g. comes via NFS shared for more devices...

You can extract the nvs blob, put it in tmpfs and bind-mount (or
symlink) it to /lib/firmware/ via modprobe install hook (or init
scripts).


Michał

Re: [RFC] ath10k: silence firmware file probing warnings

[Michal Kazior]

On 20 January 2017 at 13:51, Kalle Valo <kv...@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kaz...@tieto.com> wrote:
>> Firmware files are versioned to prevent older
>> driver instances to load unsupported firmware
>> blobs. This is reflected with a fallback logic
>> which attempts to load several firmware files.
>>
>> This however produced a lot of unnecessary
>> warnings sometimes confusing users and leading
>> them to rename firmware files making things even
>> more confusing.
>>
>> Hence use request_firmware_direct() which does not
>> produce extra warnings. This shouldn't really
>> break anything because most modern systems don't
>> rely on udev/hotplug helpers to load firmware
>> files anymore.
>>
>> Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
>
> This ended into a rather long discussion, see the full thread from the 
> patchwork link
> below, but I'll try to summarise it here:
>
> * Nobody stepped up and mentioned that they need/use the user fallback helper 
> with ath10k.
>
> * Felix confirmed that LEDE creates the calibration file before loading ath10k
>   so this should not break LEDE.
>
> * This also fixes a 60 second delay per _each_ unexistent firmware/calibration
>   file with distros which have CONFIG_FW_LOADER_USER_HELPER_FALLBACK enabled,
>   RHEL being a notable example. Using ath10k with firmware-2.bin this might
>   end up into a five minute delay in boot.
>
> * Luis is working on new drvdata interface for kernel, but that's not merged 
> yet.
>
> Based on this I think the right approach is to apply this patch. Any concerns?
>
> While writing this I started to suspect is it just by accident that
> request_firmware_direct() does not print any error messages and
> request_firmware() again does print those? Let's hope nobody decides to change
> that.  And at least Luis' drvdata interface has a documented 'optional' flag,
> so we can always switch to using that (once it's merged):
>
> * struct drvdata_req_params - driver data request parameters
> * @optional: if true it is not a hard requirement by the caller that this
> *   file be present. An error will not be recorded if the file is not
> *   found.
>
> Michal, do you mind if I'll add more info to the commit log and submit this 
> RFC
> as a proper patch? It still seems to apply and work just fine.

I don't mind :)


Michał

[PATCH v2] mac80211: prevent skb/txq mismatch

[Michal Kazior]

Station structure is considered as not uploaded
(to driver) until drv_sta_state() finishes. This
call is however done after the structure is
attached to mac80211 internal lists and hashes.
This means mac80211 can lookup (and use) station
structure before it is uploaded to a driver.

If this happens (structure exists, but
sta->uploaded is false) fast_tx path can still be
taken. Deep in the fastpath call the sta->uploaded
is checked against to derive "pubsta" argument for
ieee80211_get_txq(). If sta->uploaded is false
(and sta is actually non-NULL) ieee80211_get_txq()
effectively downgraded to vif->txq.

At first glance this may look innocent but coerces
mac80211 into a state that is almost guaranteed
(codel may drop offending skb) to crash because a
station-oriented skb gets queued up on
vif-oriented txq. The ieee80211_tx_dequeue() ends
up looking at info->control.flags and tries to use
txq->sta which in the fail case is NULL.

It's probably pointless to pretend one can
downgrade skb from sta-txq to vif-txq.

Since downgrading unicast traffic to vif->txq must
not be done there's no txq to put a frame on if
sta->uploaded is false. Therefore the code is made
to fall back to regular tx() op path if the
described condition is hit.

Only drivers using wake_tx_queue were affected.

Example crash dump before fix:

 Unable to handle kernel paging request at virtual address e26c
 PC is at ieee80211_tx_dequeue+0x204/0x690 [mac80211]
 [] (ieee80211_tx_dequeue [mac80211]) from
 [] (ath10k_mac_tx_push_txq+0x54/0x1c0 [ath10k_core])
 [] (ath10k_mac_tx_push_txq [ath10k_core]) from
 [] (ath10k_htt_txrx_compl_task+0xd78/0x11d0 [ath10k_core])
 [] (ath10k_htt_txrx_compl_task [ath10k_core])
 [] (ath10k_pci_napi_poll+0x54/0xe8 [ath10k_pci])
 [] (ath10k_pci_napi_poll [ath10k_pci]) from
 [] (net_rx_action+0xac/0x160)

Reported-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v2:
 * move sta->uploaded check to if (sta) {} block [johannes]
 * explain how the change fixes the bug [johannes]

 net/mac80211/tx.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 4dea18be385c..2076d62207ee 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1244,7 +1244,7 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
 
 static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
  struct ieee80211_vif *vif,
- struct ieee80211_sta *pubsta,
+ struct sta_info *sta,
  struct sk_buff *skb)
 {
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
@@ -1258,10 +1258,13 @@ static struct txq_info *ieee80211_get_txq(struct 
ieee80211_local *local,
if (!ieee80211_is_data(hdr->frame_control))
return NULL;
 
-   if (pubsta) {
+   if (sta) {
u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
 
-   txq = pubsta->txq[tid];
+   if (!sta->uploaded)
+   return NULL;
+
+   txq = sta->sta.txq[tid];
} else if (vif) {
txq = vif->txq;
}
@@ -1504,23 +1507,17 @@ static bool ieee80211_queue_skb(struct ieee80211_local 
*local,
struct fq *fq = >fq;
struct ieee80211_vif *vif;
struct txq_info *txqi;
-   struct ieee80211_sta *pubsta;
 
if (!local->ops->wake_tx_queue ||
sdata->vif.type == NL80211_IFTYPE_MONITOR)
return false;
 
-   if (sta && sta->uploaded)
-   pubsta = >sta;
-   else
-   pubsta = NULL;
-
if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
sdata = container_of(sdata->bss,
 struct ieee80211_sub_if_data, u.ap);
 
vif = >vif;
-   txqi = ieee80211_get_txq(local, vif, pubsta, skb);
+   txqi = ieee80211_get_txq(local, vif, sta, skb);
 
if (!txqi)
return false;
-- 
2.1.4

Re: [PATCH] mac80211: prevent skb/txq mismatch

[Michal Kazior]

On 13 January 2017 at 09:16, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Thu, 2017-01-12 at 15:28 +0100, Michal Kazior wrote:
>> Station structure is considered as not uploaded
>> (to driver) until drv_sta_state() finishes. This
>> call is however done after the structure is
>> attached to mac80211 internal lists and hashes.
>> This means mac80211 can lookup (and use) station
>> structure before it is uploaded to a driver.
>>
>> If this happens (structure exists, but
>> sta->uploaded is false) fast_tx path can still be
>> taken. Deep in the fastpath call the sta->uploaded
>> is checked against to derive "pubsta" argument for
>> ieee80211_get_txq(). If sta->uploaded is false
>> (and sta is actually non-NULL) ieee80211_get_txq()
>> effectively downgraded to vif->txq.
>>
>> At first glance this may look innocent but coerces
>> mac80211 into a state that is almost guaranteed
>> (codel may drop offending skb) to crash because a
>> station-oriented skb gets queued up on
>> vif-oriented txq. The ieee80211_tx_dequeue() ends
>> up looking at info->control.flags and tries to use
>> txq->sta which in the fail case is NULL.
>>
>> It's probably pointless to pretend one can
>> downgrade skb from sta-txq to vif-txq.
>
> Ok. I understand things until this point, more or less.
>
> What I don't understand - and you haven't really described - is how the
> changes fix it? Could you resend with a paragraph added that explains
> that?

"Since downgrading unicast traffic to vif->txq must not be done
there's no txq to put a frame on if sta->uploaded is false. Therefore
the code is made to fall back to regular tx() op path if the described
condition is hit. " -- is this sufficient?


> Also, you're adding a test:
>
>>   if (sta && !sta->uploaded)
>
> but couldn't do move that into the existing "if (sta)" block?
> Everything before that only ever returns NULL anyway.

Good point. It makes more sense to put the sta->uploaded check in if
(sta) block. I'll move it.


Michał

Re: [PATCH] ath10k: prevent sta pointer rcu violation

[Michal Kazior]

On 13 January 2017 at 08:24, Johannes Berg  wrote:
>
>> Unless you then continue to use that sta pointer after you release
>> data_lock.
>
> Ouch, ok. That's rather strangely hidden though.
>
>> Consider this:
>>
>> >  CPU0 CPU1
>> > 1   synchronize_net()
>> > 2drv_sta_state()
>> > 3  htt_fetch_ind(pid,tid) called
>> > 4  rcu_read_lock()
>> > 5  get(data_lock)
>> > 6  txq=peers[pid]->sta->txq[tid]
>> > 7  put(data_lock)
>> > 8get(data_lock)
>> > 9 peer->sta=0
>> > 10   put(data_lock)
>> > 11 kfree(sta)
>> > 12 ieee80211_tx_dequeue(txq)
>>
>> Even though there's no code like (9) per se you can think of it as
>> anything that tries to "remove" the peer--sta association
>> (ath10k_peer
>> is removed implicitly via wmi peer delete command and waiting for htt
>> event completion).
>>
>> Holding data_lock for the entire duration of handling fetch
>> indication isn't really good for performance so it's better to fix
>> RCU handling.
>
> Yeah, I see it now - also the comment where this happens. You probably
> should mark some things __rcu though and actually use RCU primitives,
> so the code is actually understandable :)

Good point. I'll do that in another patch.


Michał

Re: [PATCH] ath10k: prevent sta pointer rcu violation

[Michal Kazior]

On 12 January 2017 at 16:46, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Thu, 2017-01-12 at 16:14 +0100, Michal Kazior wrote:
>> Station pointers are RCU protected so driver must
>> be extra careful if it tries to store them
>> internally for later use outside of the RCU
>> section it obtained it in.
>>
>> It was possible for station teardown to race with
>> some htt events. The possible outcome could be a
>> use-after-free and a crash.
>>
>> Only peer-flow-control capable firmware was
>> affected (so hardware-wise qca99x0 and qca4019).
>>
>> This could be done in sta_state() itself via
>> explicit synchronize_net() call but there's
>> already a convenient sta_pre_rcu_remove() op that
>> can be hooked up to avoid extra rcu stall.
>
> I don't think this makes sense. You're not using RCU-protected pointers
> to the stations yourself, all accesses to them are locked under the
> >data_lock. As a consequence, you can't have any need for waiting
> for a grace period. Since you also remove the pointer (under lock) when
> the station gets removed, I don't think RCU can be the problem?

Unless you then continue to use that sta pointer after you release
data_lock. Consider this:

>  CPU0 CPU1
> 1   synchronize_net()
> 2drv_sta_state()
> 3  htt_fetch_ind(pid,tid) called
> 4  rcu_read_lock()
> 5  get(data_lock)
> 6  txq=peers[pid]->sta->txq[tid]
> 7  put(data_lock)
> 8get(data_lock)
> 9 peer->sta=0
> 10   put(data_lock)
> 11 kfree(sta)
> 12 ieee80211_tx_dequeue(txq)

Even though there's no code like (9) per se you can think of it as
anything that tries to "remove" the peer--sta association (ath10k_peer
is removed implicitly via wmi peer delete command and waiting for htt
event completion).

Holding data_lock for the entire duration of handling fetch indication
isn't really good for performance so it's better to fix RCU handling.


Michał

[PATCH] ath10k: prevent sta pointer rcu violation

[Michal Kazior]

Station pointers are RCU protected so driver must
be extra careful if it tries to store them
internally for later use outside of the RCU
section it obtained it in.

It was possible for station teardown to race with
some htt events. The possible outcome could be a
use-after-free and a crash.

Only peer-flow-control capable firmware was
affected (so hardware-wise qca99x0 and qca4019).

This could be done in sta_state() itself via
explicit synchronize_net() call but there's
already a convenient sta_pre_rcu_remove() op that
can be hooked up to avoid extra rcu stall.

The peer->sta pointer itself can't be set to
NULL/ERR_PTR because it is later used in
sta_state() for extra sanity checks.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/mac.c  | 18 ++
 2 files changed, 19 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index c7664d6569fa..1ab589296dff 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -314,6 +314,7 @@ struct ath10k_peer {
struct ieee80211_vif *vif;
struct ieee80211_sta *sta;
 
+   bool removed;
int vdev_id;
u8 addr[ETH_ALEN];
DECLARE_BITMAP(peer_ids, ATH10K_MAX_NUM_PEER_IDS);
diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index d1b7edba5e49..aa91f55b35a4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3774,6 +3774,9 @@ struct ieee80211_txq *ath10k_mac_txq_lookup(struct ath10k 
*ar,
if (!peer)
return NULL;
 
+   if (peer->removed)
+   return NULL;
+
if (peer->sta)
return peer->sta->txq[tid];
else if (peer->vif)
@@ -7476,6 +7479,20 @@ ath10k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
return 0;
 }
 
+static void ath10k_mac_op_sta_pre_rcu_remove(struct ieee80211_hw *hw,
+struct ieee80211_vif *vif,
+struct ieee80211_sta *sta)
+{
+   struct ath10k *ar;
+   struct ath10k_peer *peer;
+
+   ar = hw->priv;
+
+   list_for_each_entry(peer, >peers, list)
+   if (peer->sta == sta)
+   peer->removed = true;
+}
+
 static const struct ieee80211_ops ath10k_ops = {
.tx = ath10k_mac_op_tx,
.wake_tx_queue  = ath10k_mac_op_wake_tx_queue,
@@ -7516,6 +7533,7 @@ static const struct ieee80211_ops ath10k_ops = {
.assign_vif_chanctx = ath10k_mac_op_assign_vif_chanctx,
.unassign_vif_chanctx   = ath10k_mac_op_unassign_vif_chanctx,
.switch_vif_chanctx = ath10k_mac_op_switch_vif_chanctx,
+   .sta_pre_rcu_remove = ath10k_mac_op_sta_pre_rcu_remove,
 
CFG80211_TESTMODE_CMD(ath10k_tm_cmd)
 
-- 
2.1.4

[PATCH] mac80211: prevent skb/txq mismatch

[Michal Kazior]

Station structure is considered as not uploaded
(to driver) until drv_sta_state() finishes. This
call is however done after the structure is
attached to mac80211 internal lists and hashes.
This means mac80211 can lookup (and use) station
structure before it is uploaded to a driver.

If this happens (structure exists, but
sta->uploaded is false) fast_tx path can still be
taken. Deep in the fastpath call the sta->uploaded
is checked against to derive "pubsta" argument for
ieee80211_get_txq(). If sta->uploaded is false
(and sta is actually non-NULL) ieee80211_get_txq()
effectively downgraded to vif->txq.

At first glance this may look innocent but coerces
mac80211 into a state that is almost guaranteed
(codel may drop offending skb) to crash because a
station-oriented skb gets queued up on
vif-oriented txq. The ieee80211_tx_dequeue() ends
up looking at info->control.flags and tries to use
txq->sta which in the fail case is NULL.

It's probably pointless to pretend one can
downgrade skb from sta-txq to vif-txq.

Only drivers using wake_tx_queue were affected.

Example crash dump before fix:

 Unable to handle kernel paging request at virtual address e26c
 PC is at ieee80211_tx_dequeue+0x204/0x690 [mac80211]
 [] (ieee80211_tx_dequeue [mac80211]) from
 [] (ath10k_mac_tx_push_txq+0x54/0x1c0 [ath10k_core])
 [] (ath10k_mac_tx_push_txq [ath10k_core]) from
 [] (ath10k_htt_txrx_compl_task+0xd78/0x11d0 [ath10k_core])
 [] (ath10k_htt_txrx_compl_task [ath10k_core])
 [] (ath10k_pci_napi_poll+0x54/0xe8 [ath10k_pci])
 [] (ath10k_pci_napi_poll [ath10k_pci]) from
 [] (net_rx_action+0xac/0x160)

Reported-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 net/mac80211/tx.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 4dea18be385c..c77fcf83d004 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1244,13 +1244,16 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data 
*sdata,
 
 static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
  struct ieee80211_vif *vif,
- struct ieee80211_sta *pubsta,
+ struct sta_info *sta,
  struct sk_buff *skb)
 {
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_txq *txq = NULL;
 
+   if (sta && !sta->uploaded)
+   return NULL;
+
if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
(info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
return NULL;
@@ -1258,10 +1261,10 @@ static struct txq_info *ieee80211_get_txq(struct 
ieee80211_local *local,
if (!ieee80211_is_data(hdr->frame_control))
return NULL;
 
-   if (pubsta) {
+   if (sta) {
u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
 
-   txq = pubsta->txq[tid];
+   txq = sta->sta.txq[tid];
} else if (vif) {
txq = vif->txq;
}
@@ -1504,23 +1507,17 @@ static bool ieee80211_queue_skb(struct ieee80211_local 
*local,
struct fq *fq = >fq;
struct ieee80211_vif *vif;
struct txq_info *txqi;
-   struct ieee80211_sta *pubsta;
 
if (!local->ops->wake_tx_queue ||
sdata->vif.type == NL80211_IFTYPE_MONITOR)
return false;
 
-   if (sta && sta->uploaded)
-   pubsta = >sta;
-   else
-   pubsta = NULL;
-
if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
sdata = container_of(sdata->bss,
 struct ieee80211_sub_if_data, u.ap);
 
vif = >vif;
-   txqi = ieee80211_get_txq(local, vif, pubsta, skb);
+   txqi = ieee80211_get_txq(local, vif, sta, skb);
 
if (!txqi)
return false;
-- 
2.1.4

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Michal Kazior]

On 13 December 2016 at 19:37, Erik Stromdahl <erik.stromd...@gmail.com> wrote:
>
>
> On 12/13/2016 06:26 PM, Valo, Kalle wrote:
>> Michal Kazior <michal.kaz...@tieto.com> writes:
>>
>>> On 13 December 2016 at 14:44, Valo, Kalle <kv...@qca.qualcomm.com> wrote:
>>>> Erik Stromdahl <erik.stromd...@gmail.com> writes:
>>>>
>>>>> Code refactorization:
>>>>>
>>>>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>>>>> to ath10k_htc_control_rx_complete.
>>>>>
>>>>> This eases the implementation of SDIO/mbox significantly since
>>>>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>>>>> hif layer.
>>>>>
>>>>> Since the ath10k_htc_control_rx_complete already is present
>>>>> (only containing a warning message) there is no reason for not
>>>>> using it (instead of having a special case for ep 0 in
>>>>> ath10k_htc_rx_completion_handler).
>>>>>
>>>>> Signed-off-by: Erik Stromdahl <erik.stromd...@gmail.com>
>>>>
>>>> I tested this on QCA988X PCI board just to see if there are any
>>>> regressions. It crashes immediately during module load, every time, and
>>>> bisected that the crashing starts on this patch:
>>>>
>>>> [ 1239.715325] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 
>>>> irq_mode 0 reset_mode 0
>>>> [ 1239.885125] ath10k_pci :02:00.0: Direct firmware load for 
>>>> ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
>>>> [ 1239.885260] ath10k_pci :02:00.0: Direct firmware load for 
>>>> ath10k/cal-pci-:02:00.0.bin failed with error -2
>>>> [ 1239.885687] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c 
>>>> chip_id 0x043202ff sub :
>>>> [ 1239.885699] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 
>>>> 1 dfs 1 testmode 1
>>>> [ 1239.885899] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
>>>> features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
>>>> [ 1239.941836] ath10k_pci :02:00.0: Direct firmware load for 
>>>> ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
>>>> [ 1239.941993] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
>>>> bebc7c08
>>>> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   
>>>> (null)
>>>> [ 1241.136738] IP: [<  (null)>]   (null)
>>>> [ 1241.136759] *pdpt =  *pde = f0002a55f0002a55 [ 
>>>> 1241.136781]
>>>> [ 1241.136793] Oops: 0010 [#1] SMP
>>>>
>>>> What's odd is that when I added some printks on my own and enabled both
>>>> boot and htc debug levels it doesn't crash anymore. After everything
>>>> works normally after that, I can start AP mode and connect to it. Is it
>>>> a race somewhere?
>>>
>>> Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
>>> is set in htc_wait_target() [changed patch 4, but still too late].
>>>
>>> ep_rx_complete must be set prior to calling hif_start(). You probably
>>> crash on end of ath10k_htc_rx_completion_handler() when trying to call
>>> ep->ep_ops.ep_rx_complete(ar, skb).
>>
>> Yeah, just checked and ep->ep_ops.ep_rx_complete is NULL at the end of
>> ath10k_htc_rx_completion_handler().
>>
> It is indeed correct as Michal points out, there is a risk that the
> first HTC control message (typically an HTC ready message) is received
> before the HTC control endpoint is connected.
>
> I have experienced a similar race with my SDIO implementation as well.
> In this case I did solve the issue by enabling HIF target interrupts
> after the HTC control endpoint was connected. I am not sure however if
> this is the most elegant way to solve this problem.
>
> My SDIO target won't send the HTC ready message before this is done.
> The fix essentially consists of moving the ..._irg_enable call from
> hif_start into another hif op.

It makes more sense to move ep_rx_complete setup/assignment before
hif_start(). This assignment should be done very early as there is
nothing to change/override for this endpoint during operation, is
there? It's known what it needs to store very early on.


> I have made a few updates since I submitted the original RFC and created
> a repo on github:
>
> https://github.com/erstrom/linux-ath
>
> I have a bunch of branches that are all based on the tags on the ath master.
>
> As of this moment the latest version is:
>
> ath-201612131156-ath10k-sdio
>
> This branch contains the original RFC patches plus some addons/fixes.
>
> In the above mentioned branch there are a few commits related to this
> race condition. Perhaps you can have a look at them?
>
> The commits are:
> 821672913328cf737c9616786dc28d2e4e8a4a90

I would avoid if(bus==xx) checks.


> dd7fcf0a1f78e68876d14f90c12bd37f3a700ad7
> 7434b7b40875bd08a3a48a437ba50afed7754931
>
> Perhaps this approach can work with PCIe as well?

I think I did contemplate the unmask/start distinction at some point
but I didn't go with it for some reason I can't recall now.


Michał

Re: [RFC v2 05/11] ath10k: htc: refactorization

[Michal Kazior]

On 13 December 2016 at 14:44, Valo, Kalle  wrote:
> Erik Stromdahl  writes:
>
>> Code refactorization:
>>
>> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
>> to ath10k_htc_control_rx_complete.
>>
>> This eases the implementation of SDIO/mbox significantly since
>> the ep_rx_complete cb is invoked directly from the SDIO/mbox
>> hif layer.
>>
>> Since the ath10k_htc_control_rx_complete already is present
>> (only containing a warning message) there is no reason for not
>> using it (instead of having a special case for ep 0 in
>> ath10k_htc_rx_completion_handler).
>>
>> Signed-off-by: Erik Stromdahl 
>
> I tested this on QCA988X PCI board just to see if there are any
> regressions. It crashes immediately during module load, every time, and
> bisected that the crashing starts on this patch:
>
> [ 1239.715325] ath10k_pci :02:00.0: pci irq msi oper_irq_mode 2 irq_mode 
> 0 reset_mode 0
> [ 1239.885125] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/pre-cal-pci-:02:00.0.bin failed with error -2
> [ 1239.885260] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/cal-pci-:02:00.0.bin failed with error -2
> [ 1239.885687] ath10k_pci :02:00.0: qca988x hw2.0 target 0x4100016c 
> chip_id 0x043202ff sub :
> [ 1239.885699] ath10k_pci :02:00.0: kconfig debug 1 debugfs 1 tracing 1 
> dfs 1 testmode 1
> [ 1239.885899] ath10k_pci :02:00.0: firmware ver 10.2.4.70.59-2 api 5 
> features no-p2p,raw-mode,mfp,allows-mesh-bcast crc32 4159f498
> [ 1239.941836] ath10k_pci :02:00.0: Direct firmware load for 
> ath10k/QCA988X/hw2.0/board-2.bin failed with error -2
> [ 1239.941993] ath10k_pci :02:00.0: board_file api 1 bmi_id N/A crc32 
> bebc7c08
> [ 1241.136693] BUG: unable to handle kernel NULL pointer dereference at   
> (null)
> [ 1241.136738] IP: [<  (null)>]   (null)
> [ 1241.136759] *pdpt =  *pde = f0002a55f0002a55 [ 1241.136781]
> [ 1241.136793] Oops: 0010 [#1] SMP
>
> What's odd is that when I added some printks on my own and enabled both
> boot and htc debug levels it doesn't crash anymore. After everything
> works normally after that, I can start AP mode and connect to it. Is it
> a race somewhere?

Yes. htc_wait_target() is called after hif_start(). The ep_rx_complete
is set in htc_wait_target() [changed patch 4, but still too late].

ep_rx_complete must be set prior to calling hif_start(). You probably
crash on end of ath10k_htc_rx_completion_handler() when trying to call
ep->ep_ops.ep_rx_complete(ar, skb).


Michał

Re: [PATCH 2/2] mac80211: put upper bound on txqi queue length.

[Michal Kazior]

On 5 December 2016 at 14:56, Johannes Berg  wrote:
> On Tue, 2016-11-29 at 10:05 -0800, gree...@candelatech.com wrote:
>> From: Ben Greear 
>>
>> This fixes OOM when using pktgen to drive a wifi station at more than
>> the station can transmit.  pktgen uses ndo_start_xmit instead of
>> going
>> through the queue layer, so it will not back off when the queues are
>> stopped, and would thus cause packets to be added to the txqi->queue
>> until the system goes OOM and crashes.
>>
>> Signed-off-by: Ben Greear 
>> ---
>>
>> This is against 4.7.10+, not sure if it is actually needed in latest
>> kernel.
>
> One would hope that fq_tin_enqueue() does something like that, but
> anyway the patch doesn't apply so I'm dropping it.

fq_tin_enqueue() drops "fat" flow head packet upon reaching packet
count limit (8192) or memory limit (4 or 16 mbytes depending on vht
availability) whichever is hit first.


Michał

Re: [PATCH 1/3] ath10k: Ensure there are no stale ar->txqs entries.

[Michal Kazior]

On 2 December 2016 at 01:24, Ben Greear <gree...@candelatech.com> wrote:
> On 12/01/2016 02:52 PM, Ben Greear wrote:
>>
>> On 08/19/2016 06:34 AM, Ben Greear wrote:
>>>
>>>
>>>
>>> On 08/18/2016 11:59 PM, Michal Kazior wrote:
>>>>
>>>> On 19 August 2016 at 03:26,  <gree...@candelatech.com> wrote:
>>>>>
>>>>> From: Ben Greear <gree...@candelatech.com>
>>>>>
>>>>> I was seeing kernel crashes due to accessing freed memory
>>>>> while debugging a 9984 firmware that was crashing often.
>>>>>
>>>>> This patch fixes the crashes.  I am not certain if there
>>>>> is a better way or not.
>
>
> Michal, thanks for the help on IRC.  I added this logic:
>
> static void ieee80211_drv_tx(struct ieee80211_local *local,
>  struct ieee80211_vif *vif,
>  struct ieee80211_sta *pubsta,
>  struct sk_buff *skb)
> {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> struct ieee80211_tx_control control = {
> .sta = pubsta,
> };
> struct ieee80211_txq *txq = NULL;
> struct txq_info *txqi;
> u8 ac;
>
> if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
> (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
> goto tx_normal;
>
> if (!ieee80211_is_data(hdr->frame_control))
> goto tx_normal;
>
> if (unlikely(!ieee80211_sdata_running(sdata))) {
> WARN_ON_ONCE(1);
> goto delete_and_return;
> }
>
> ...
>
> if (atomic_read(>txqs_len[ac]) >=
> (local->hw.txq_ac_max_pending * 2)) {
> /* Must be that something is not paying attention to
>  * max-pending, like pktgen, so just drop this frame.
>  */
> delete_and_return:
> ieee80211_free_txskb(>hw, skb);
> return;
> }
>
>
> But, I still see the txq entries on the ar->txqs list in the
> ath10k_mac_txq_init
> after firmware crash in my test case.  Is this the test you were suggesting?

Yes.

Now that I think about it mac80211 doesn't call anything in driver
during hw_restart that would unref txqs. This means you'll have them
still linked when add_interface/sta_state is called, no?

This means that either:
 (a) txq (re-)init should be smarter in ath10k
 (b) txqs should be purged during hw_restart in ath10k


Michał

Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure

[Michal Kazior]

On 2 December 2016 at 03:29,   wrote:
> From: Ben Greear 
>
> This appears to fix a problem where ath10k firmware would crash,
> mac80211 would start re-adding interfaces to the driver, but the
> iterate-active-interfaces logic would then try to use the half-built
> interfaces.  With a bit of extra debug to catch the problem, the
> ath10k crash looks like this:
>
> ath10k_pci :05:00.0: Initializing arvif: 8801ce97e320 on vif: 
> 8801ce97e1d8
>
> [the print that happens after arvif->ar is assigned is not shown, so code did 
> not make it that far before
>  the tx-beacon-nowait method was called]
>
> tx-beacon-nowait:  arvif: 8801ce97e320  ar:   (null)
[...]
>
> Signed-off-by: Ben Greear 
> ---
>  net/mac80211/util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 863f2c1..abe1f64 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -705,7 +705,7 @@ static void __iterate_interfaces(struct ieee80211_local 
> *local,
> break;
> }
> if (!(iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL) &&
> -   active_only && !(sdata->flags & 
> IEEE80211_SDATA_IN_DRIVER))
> +   (active_only && (local->in_reconfig || !(sdata->flags & 
> IEEE80211_SDATA_IN_DRIVER
> continue;

Doesn't this effectivelly prevent you from iterating over interfaces
completely during reconfig? As you bring up interfaces you might
need/want to iterate over others to re-adjust your own state.

I'd argue there should be another flag, IEEE80211_SDATA_RESUMING, used
with sdata->flags for resuming so that once it is re-added to the
driver it can be cleared (and therefore properly iterated over).


Michał

Re: [PATCH 3/5] ath10k: Remove unused wmi_p2p_noa_descriptor 'noa' in wmi-tlv

[Michal Kazior]

On 24 November 2016 at 09:01, Kirtika Ruchandani
<kirtika.ruchand...@gmail.com> wrote:
> Commit ca996ec56608 (ath10k: implement wmi-tlv backend)
> introduced ath10k_wmi_tlv_op_gen_vdev_start() where
> 'struct wmi_p2p_noa_descriptor *noa' is defined and set but not used.
> Compiling with W=1 gives the following warning, fix it.
> drivers/net/wireless/ath/ath10k/wmi-tlv.c: In function 
> ‘ath10k_wmi_tlv_op_gen_vdev_start’:
> drivers/net/wireless/ath/ath10k/wmi-tlv.c:1647:33: warning: variable ‘noa’ 
> set but not used [-Wunused-but-set-variable]
>
> Fixes: ca996ec56608 ("ath10k: implement wmi-tlv backend")
> Cc: Michal Kazior <michal.kaz...@tieto.com>
> Cc: Kalle Valo <kv...@qca.qualcomm.com>
> Signed-off-by: Kirtika Ruchandani <kirt...@chromium.org>
> ---
>  drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> index e64f593..0e4bd29 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -1644,7 +1644,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
>  {
> struct wmi_tlv_vdev_start_cmd *cmd;
> struct wmi_channel *ch;
> -   struct wmi_p2p_noa_descriptor *noa;
> struct wmi_tlv *tlv;
> struct sk_buff *skb;
> size_t len;
> @@ -1702,7 +1701,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
> tlv = ptr;
> tlv->tag = __cpu_to_le16(WMI_TLV_TAG_ARRAY_STRUCT);
> tlv->len = 0;
> -   noa = (void *)tlv->value;
>
> /* Note: This is a nested TLV containing:
>  * [wmi_tlv][wmi_p2p_noa_descriptor][wmi_tlv]..

I would rather keep this one as it serves as documentation. Would
"(void) noa;" be enough satisfy the compiler?


Michał

Re: wl1251 & mac address & calibration data

[Michal Kazior]

On 22 November 2016 at 16:31, Pali Rohár <pali.ro...@gmail.com> wrote:
> On Tuesday 22 November 2016 16:22:57 Michal Kazior wrote:
>> On 21 November 2016 at 16:51, Pali Rohár <pali.ro...@gmail.com> wrote:
>> > On Friday 11 November 2016 18:20:50 Pali Rohár wrote:
>> >> Hi! I will open discussion about mac address and calibration data for
>> >> wl1251 wireless chip again...
>> >>
>> >> Problem: Mac address & calibration data for wl1251 chip on Nokia N900
>> >> are stored on second nand partition (mtd1) in special proprietary format
>> >> which is used only for Nokia N900 (probably on N8x0 and N9 too).
>> >> Wireless driver wl1251.ko cannot work without mac address and
>> >> calibration data.
>>
>> Same problem applies to some ath9k/ath10k supported routers. Some even
>> carry mac address as implicit offset from ethernet mac address. As far
>> as I understand OpenWRT cooks cal blobs on first boot prior to loading
>> modules.
>
> So... wl1251 on Nokia N900 is not alone and this problem is there for
> more drivers and devices. Which means we should come up with some
> generic solution.

This isn't particularly a problem for ath9k/ath10k.

Let me give you more background on ath10k.

ath10k devices can come with caldata and macaddr stored in their
OTP/EEPROM. In that case a generic "template" board file is used.
Userspace doesn't need to do anything special.

Some vendors however decide to use flash partition to store caldata.
In that case ath10k expects userspace to prepare cal-$bus-$devname.bin
files, each for a different radio (you can have multiple radios on a
system).

Now translating this for wl1251 I would expect it should also use
something like wl1251-nvs-sdio-0x0001.bin for devices like N900 that
have caldata on flash partition (instead of the generic
wl1251-nvs.bin). I'm not sure if wl1251-nvs.bin is something
comparable to (the generic) board.bin ath10k has though. Maybe the
entire idea behind wl1251-nvs.bin is flawed as it's supposed to be
device specific and is oblivious to possibility of having multiple
wl1251 radios on one system (probably sane assumption from practical
standpoint but still).


>> >> Absence of mac address cause that driver generates random mac address at
>> >> every kernel boot which has couple of problems (unstable identifier of
>> >> wireless device due to udev permanent storage rules; unpredictable
>> >> behaviour for dhcp mac address assignment, mac address filtering, ...).
>> >>
>> >> Currently there is no way to set (permanent) mac address for network
>> >> interface from userspace. And it does not make sense to implement in
>> >> linux kernel large parser for proprietary format of second nand
>> >> partition where is mac address stored only for one device -- Nokia N900.
>> >>
>> >> Driver wl1251.ko loads calibration data via request_firmware() for file
>> >> wl1251-nvs.bin. There are some "example" calibration file in linux-
>> >> firmware repository, but it is not suitable for normal usage as real
>> >> calibration data are per-device specific.
>>
>> You could hook up a script that cooks up the cal/mac file via
>> modprobe's install hook, no?
>
> Via modprobe hook I can either pass custom module parameter or call any
> other system (shell) commands.
>
> As wl1251.ko does not accept mac_address as module parameter, such
> modprobe hook does not help -- as there is absolutely no way from
> userspace to set or change (permanent) mac address.

Quoting modprobe.d manual:

>   install modulename command...
>   This command instructs modprobe to run your
>   command instead of inserting the module in the
>   kernel as normal. The command can be any shell
>   command: this allows you to do any kind of
>   complex processing you might wish. [...]

You can hook up a script that cooks up wl1251-nvs.bin (caldata,
macaddr) and then insmod the actual wl1251.ko module. Or you can just
cook up the nvs on first device boot and store it in /lib/firmware
(possibly overwriting the "generic" wl1251 from linux-firmware).


Michal

Re: wl1251 & mac address & calibration data

[Michal Kazior]

On 21 November 2016 at 16:51, Pali Rohár  wrote:
> On Friday 11 November 2016 18:20:50 Pali Rohár wrote:
>> Hi! I will open discussion about mac address and calibration data for
>> wl1251 wireless chip again...
>>
>> Problem: Mac address & calibration data for wl1251 chip on Nokia N900
>> are stored on second nand partition (mtd1) in special proprietary format
>> which is used only for Nokia N900 (probably on N8x0 and N9 too).
>> Wireless driver wl1251.ko cannot work without mac address and
>> calibration data.

Same problem applies to some ath9k/ath10k supported routers. Some even
carry mac address as implicit offset from ethernet mac address. As far
as I understand OpenWRT cooks cal blobs on first boot prior to loading
modules.


>> Absence of mac address cause that driver generates random mac address at
>> every kernel boot which has couple of problems (unstable identifier of
>> wireless device due to udev permanent storage rules; unpredictable
>> behaviour for dhcp mac address assignment, mac address filtering, ...).
>>
>> Currently there is no way to set (permanent) mac address for network
>> interface from userspace. And it does not make sense to implement in
>> linux kernel large parser for proprietary format of second nand
>> partition where is mac address stored only for one device -- Nokia N900.
>>
>> Driver wl1251.ko loads calibration data via request_firmware() for file
>> wl1251-nvs.bin. There are some "example" calibration file in linux-
>> firmware repository, but it is not suitable for normal usage as real
>> calibration data are per-device specific.

You could hook up a script that cooks up the cal/mac file via
modprobe's install hook, no?


Michał

Re: ath10k stuck in mesh mode

[Michal Kazior]

On 22 November 2016 at 10:43, Matteo Grandi  wrote:
> Dear Bob, Michal, all
>
> I've finally managed to have a 80MHz channel bandwidth, thanks to your hint!
> The problem was related to the CRDA that even if it looks correctly
> installed, it actually doesn't work as supposed.
> I downloaded and recompiled the CRDA-3.18
> (http://drvbp1.linux-foundation.org/~mcgrof/rel-html/crda/) and set-up
> the regulatory domain.
> Notice that without setting up the reg. domain all the available
> channels have the "passive scanning" label because the CRDA prevent
> beaconing so mesh mode is not possible (maybe it's otherwise possible
> to operate in STA mode that doesn't require to send frames(?)).

STA can be allowed to transmit via beacon hints as far as I understand.


> Now I can have a mesh communication using 80MHz channel bandwidth, but
> MIMO still doesn't work.
> In fact the highest MCS reached was MCS9 that is the highest MCS with
> Single Spatial Stream, and I can't managed to have MIMO.

11n used mcs to imply nss. 11ac treats mcs and nss separately.
Therefore saying "MCS9 is 1SS" is incorrect. How are you checking
this? What device are you using for verifying?


> I played with the antennas position and distance, with the
> polarization and the presence or not of LOS, but by sniffing the
> transmissions I sow only MCS9 even if in the radiotap header field
> there are Antenna 0 and Antenna 1.
> However the radio information field report "Spatial streams: 1".
>
> Does anyone experienced something similar with the use of MIMO in mesh
> mode with the ath10k fw?
> I will thank any light you can shed to this!
> Thank you very much

You might want to debug peer_assoc commands sent to firmware. They may
be, for some reason, be requesting nss=1. Maybe sta_rc_update() isn't
properly updating peer in firmware. Driver debugs/dumps
(debug_mask=0xff) would be useful.


Michal

Re: [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently

[Michal Kazior]

On 21 November 2016 at 15:41, Zefir Kurtisi  wrote:
> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>> In the case that a spectral scan is enabled the PHY errors sent by the
>> hardware as part of the scanning might trigger the radar detection and
>> channels might be marked as 'unusable' incorrectly. This patch fixes
>> the issue by preventing the spectral scan to be enabled if DFS is used
>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>
>> [...]
>
> From the relevant source code portion in channel.c:ath_set_channel()
>
> 80 /* Enable radar pulse detection if on a DFS channel. Spectral
> 81  * scanning and radar detection can not be used concurrently.
> 82  */
> 83 if (hw->conf.radar_enabled) {
> 84 u32 rxfilter;
> 85
> 86 rxfilter = ath9k_hw_getrxfilter(ah);
> 87 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
> 88 ATH9K_RX_FILTER_PHYERR;
> 89 ath9k_hw_setrxfilter(ah, rxfilter);
> 90 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
> 91 chan->center_freq);
> 92 } else {
> 93 /* perform spectral scan if requested. */
> 94 if (test_bit(ATH_OP_SCANNING, >op_flags) &&
> 95 sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
> 96 ath9k_cmn_spectral_scan_trigger(common, 
> >spec_priv);
> 97 }
>
> it seems that spectral can't ever be activated while operating on a DFS 
> channel.
>
> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar 
> pulses
> into the pattern detector, you just need to ensure that they depend on
> hw->conf.radar_enabled. A patch like the attached one should be enough.

Good point. I guess set_channel could be oversimplified as well. I
mean, it makes sense to consider radar and spectral mutually exclusive
if they use the same phyerr code. However some chips actually seem (as
per the comment I mentioned) to distinguish the two so I don't know if
the "mutually exclusive" is true for all chips per se. Just thinking
out loud.

I also wonder if calling ieee80211_radar_detect() should have any
effect if there are no radar operated interfaces in the first place?


Michał

Re: ath10k stuck in mesh mode

[Michal Kazior]

On 21 November 2016 at 10:46, Matteo Grandi  wrote:
> Dear Bob, Michal, all,
>
> I've just tried your advices (actually I already tried it following
> the wireless.wiki.kernel web pages) and I had a look at the syslog
> while I was typing the commands
> At the beginning I have this
[...]
> Other (hopefully) useful info:
> root@MrProper:~# iw reg get
> global
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#2
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#0
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>
> phy#1
> country US: DFS-UNSET
> (2402 - 2472 @ 40), (3, 27), (N/A)
> (5170 - 5250 @ 40), (3, 17), (N/A)
> (5250 - 5330 @ 40), (3, 20), (0 ms), DFS
> (5490 - 5600 @ 40), (3, 20), (0 ms), DFS
> (5650 - 5710 @ 40), (3, 20), (0 ms), DFS
> (5735 - 5835 @ 40), (3, 30), (N/A)
> (57240 - 63720 @ 2160), (N/A, 40), (N/A)
>[...]
>
> Checking on Internet I didn't find a working solution, and the data
> rate stucks to 120Mbps MCS7.
> For me it still a mistery, I hope in your help.

Note the "@ 40" for all frequency ranges (except 60GHz band). Your
regulatory database seems to be limiting you to 40 MHz (it's probably
very old/ out of date). You'll need to update it to be able to use 80
MHz.


Michal

Re: [RFC 03/12] ath10k: htc: Changed order of wait target and ep connect

[Michal Kazior]

On 15 November 2016 at 18:07, Erik Stromdahl <erik.stromd...@gmail.com> wrote:
> On 11/15/2016 11:13 AM, Michal Kazior wrote:
>> On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromd...@gmail.com> 
>> wrote:
>>> This patch changes the order in which the driver waits for the
>>> target to become ready and the service connect of the HTC
>>> control service.
>>>
>>> The HTC control service is connected before the driver starts
>>> waiting for the HTC ready control message.
>>>
>>> The reason for this is that the HTC ready control message is
>>> transmitted on EP 0 and that sdio/mbox based systems will ignore
>>> messages received on unconnected endpoints.
>>>
>>> Signed-off-by: Erik Stromdahl <erik.stromd...@gmail.com>
>>> ---
>>>  drivers/net/wireless/ath/ath10k/htc.c |   32 
>>> 
>>>  1 file changed, 16 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htc.c 
>>> b/drivers/net/wireless/ath/ath10k/htc.c
>>> index e3f7bf4..7257366 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htc.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htc.c
>>> @@ -606,6 +606,22 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
>>> u16 credit_count;
>>> u16 credit_size;
>>>
>>> +   /* setup our pseudo HTC control endpoint connection */
>>> +   memset(_req, 0, sizeof(conn_req));
>>> +   memset(_resp, 0, sizeof(conn_resp));
>>> +   conn_req.ep_ops.ep_tx_complete = ath10k_htc_control_tx_complete;
>>> +   conn_req.ep_ops.ep_rx_complete = ath10k_htc_control_rx_complete;
>>> +   conn_req.max_send_queue_depth = ATH10K_NUM_CONTROL_TX_BUFFERS;
>>> +   conn_req.service_id = ATH10K_HTC_SVC_ID_RSVD_CTRL;
>>> +
>>> +   /* connect fake service */
>>> +   status = ath10k_htc_connect_service(htc, _req, _resp);
>>> +   if (status) {
>>> +   ath10k_err(ar, "could not connect to htc service (%d)\n",
>>> +  status);
>>> +   return status;
>>> +   }
>>> +
>>
>> How is this supposed to work? ath10k_htc_connect_service() requires
>> htc->target_credit_size to compute tx_credits_per_max_message. Or am I
>> missing something? Applying this patch alone results in:
>>
>> [6.680101] divide error:  [#1] SMP
>> [6.681342] Modules linked in: ath10k_pci(O) ath10k_core(O) ath
>> mac80211 cfg80211
>> [6.684876] CPU: 3 PID: 823 Comm: kworker/u8:2 Tainted: GW
>> O4.9.0-rc4-wt-ath+ #79
>> [6.688051] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
>> [6.691644] Workqueue: ath10k_wq ath10k_core_register_work [ath10k_core]
>> [6.694309] task: 88000a19 task.stack: c96d4000
>> [6.695458] RIP: 0010:[]  []
>> ath10k_htc_connect_service+0x21b/0x420 [ath10k_core]
>>
>>
>> Michał
>>
>
> You're right. I have totally missed this. What is strange is that my
> compiler (ARM linaro) seems to optimize the code in a way that removes
> the tx_credits_per_max_message value.
>
> If I add a printk in ath10k_htc_connect_service (printing the value) I
> get a similar oops.
>
> I think it has to do with the fact the this value isn't really used at
> all. grepping the code reveals that tx_credits_per_max_message is only
> used inside ath10k_htc_connect_service (only written, never read).
>
> Removing it doesn't seem to break anything, so perhaps it should be removed?

I think it's safe to remove now. This is legacy inherited from the
internal driver.

Otherwise this looks okay except commit log which is a bit unclear.
I'm still not sure *why* you need to reorder this. FWIW this can be a
general clean up thing that ends up moving the if (eid == EP0) {code}
to control endpoint handler in patch 4.


Michał

Re: [RFC 10/12] ath10k: Added QCA65XX hw definition

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/hw.h |1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h 
> b/drivers/net/wireless/ath/ath10k/hw.h
> index 46142e9..ef45ecf 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -224,6 +224,7 @@ enum ath10k_hw_rev {
> ATH10K_HW_QCA9377,
> ATH10K_HW_QCA4019,
> ATH10K_HW_QCA9887,
> +   ATH10K_HW_QCA65XX,

Are you 100% positive that you're supporting all QCA65XX chips? The
rule of thumb is to avoid Xs as much as possible unless totally
perfectly completely sure.


Michał

Re: [RFC 09/12] ath10k: Mailbox address definitions

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> Address definitions for SDIO/mbox based chipsets.
>
> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/hw.h |   53 
> ++
>  1 file changed, 53 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/hw.h 
> b/drivers/net/wireless/ath/ath10k/hw.h
> index 883547f..46142e9 100644
> --- a/drivers/net/wireless/ath/ath10k/hw.h
> +++ b/drivers/net/wireless/ath/ath10k/hw.h
> @@ -814,6 +814,59 @@ struct ath10k_hw_ops {
>  #define QCA9887_EEPROM_ADDR_LO_MASK0x00ff
>  #define QCA9887_EEPROM_ADDR_LO_LSB 16
>
> +#define MBOX_RESET_CONTROL_ADDRESS 0x
> +#define MBOX_HOST_INT_STATUS_ADDRESS   0x0800
> +#define MBOX_HOST_INT_STATUS_ERROR_LSB 7
> +#define MBOX_HOST_INT_STATUS_ERROR_MASK0x0080

I would again suggest using Jakub's bitfield GET_FIELD stuff to get
rid of MASK+LSB and just have single define per register field. Kalle,
thoughts?


Michał

Re: [RFC 06/12] ath10k: bmi: Added SOC reg read/write functions

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> Added functions implementing the following BMI commands:
>
> BMI_READ_SOC_REGISTER
> BMI_WRITE_SOC_REGISTER
>
> Reading and writing BMI registers is sometimes needed for
> SDIO chipsets.

I didn't see ath10k_bmi_write_soc_reg nor ath10k_bmi_read_soc_reg
being used in your Patch 12. Is this patch really necessary?


[...]
> diff --git a/drivers/net/wireless/ath/ath10k/bmi.c 
> b/drivers/net/wireless/ath/ath10k/bmi.c
> index 2872d34..1c378a2 100644
> --- a/drivers/net/wireless/ath/ath10k/bmi.c
> +++ b/drivers/net/wireless/ath/ath10k/bmi.c
> @@ -97,7 +97,8 @@ int ath10k_bmi_read_memory(struct ath10k *ar,
> u32 rxlen;
> int ret;
>
> -   ath10k_dbg(ar, ATH10K_DBG_BMI, "bmi read address 0x%x length %d\n",
> +   ath10k_dbg(ar, ATH10K_DBG_BMI,
> +  "bmi read memory address 0x%x length %d\n",
>address, length);
>
> if (ar->bmi.done_sent) {
> @@ -137,7 +138,8 @@ int ath10k_bmi_write_memory(struct ath10k *ar,
> u32 txlen;
> int ret;
>
> -   ath10k_dbg(ar, ATH10K_DBG_BMI, "bmi write address 0x%x length %d\n",
> +   ath10k_dbg(ar, ATH10K_DBG_BMI,
> +  "bmi write memory address 0x%x length %d\n",
>address, length);
>

These 2 hunks shouldn't be modified in this patch. If you want to do a
clean up this warrants a separate patch :)


Michał

Re: [RFC 05/12] ath10k: htc: Added ATH10K_HTC_FLAG_BUNDLE_LSB

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/htc.h |2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.h 
> b/drivers/net/wireless/ath/ath10k/htc.h
> index 589800a..df16a04 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.h
> +++ b/drivers/net/wireless/ath/ath10k/htc.h
> @@ -62,6 +62,8 @@ enum ath10k_htc_rx_flags {
> ATH10K_HTC_FLAG_BUNDLE_MASK = 0xF0
>  };
>
> +#define ATH10K_HTC_FLAG_BUNDLE_LSB 4

Just an idea - we could start using FIELD_GET() with
ATH10K_HTC_FLAG_BUNDLE_MASK alone. I would love to see Jakub's
bitfield stuff be used more. Kalle, thoughts?


Michał

Re: [RFC 04/12] ath10k: htc: refactorization

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> Code refactorization:
>
> Moved the code for ep 0 in ath10k_htc_rx_completion_handler
> to ath10k_htc_control_rx_complete.
>
> This eases the implementation of SDIO/mbox significantly since
> the ep_rx_complete cb is invoked directly from the SDIO/mbox
> hif layer.
>
> Since the ath10k_htc_control_rx_complete already is present
> (only containing a warning message) there is no reason for not
> using it (instead of having a special case for ep 0 in
> ath10k_htc_rx_completion_handler).

This should be squashed with Patch 3 since it's inseparable part of
the same refactorization.


Michał

Re: [RFC 03/12] ath10k: htc: Changed order of wait target and ep connect

[Michal Kazior]

On 14 November 2016 at 17:33, Erik Stromdahl  wrote:
> This patch changes the order in which the driver waits for the
> target to become ready and the service connect of the HTC
> control service.
>
> The HTC control service is connected before the driver starts
> waiting for the HTC ready control message.
>
> The reason for this is that the HTC ready control message is
> transmitted on EP 0 and that sdio/mbox based systems will ignore
> messages received on unconnected endpoints.
>
> Signed-off-by: Erik Stromdahl 
> ---
>  drivers/net/wireless/ath/ath10k/htc.c |   32 
>  1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htc.c 
> b/drivers/net/wireless/ath/ath10k/htc.c
> index e3f7bf4..7257366 100644
> --- a/drivers/net/wireless/ath/ath10k/htc.c
> +++ b/drivers/net/wireless/ath/ath10k/htc.c
> @@ -606,6 +606,22 @@ int ath10k_htc_wait_target(struct ath10k_htc *htc)
> u16 credit_count;
> u16 credit_size;
>
> +   /* setup our pseudo HTC control endpoint connection */
> +   memset(_req, 0, sizeof(conn_req));
> +   memset(_resp, 0, sizeof(conn_resp));
> +   conn_req.ep_ops.ep_tx_complete = ath10k_htc_control_tx_complete;
> +   conn_req.ep_ops.ep_rx_complete = ath10k_htc_control_rx_complete;
> +   conn_req.max_send_queue_depth = ATH10K_NUM_CONTROL_TX_BUFFERS;
> +   conn_req.service_id = ATH10K_HTC_SVC_ID_RSVD_CTRL;
> +
> +   /* connect fake service */
> +   status = ath10k_htc_connect_service(htc, _req, _resp);
> +   if (status) {
> +   ath10k_err(ar, "could not connect to htc service (%d)\n",
> +  status);
> +   return status;
> +   }
> +

How is this supposed to work? ath10k_htc_connect_service() requires
htc->target_credit_size to compute tx_credits_per_max_message. Or am I
missing something? Applying this patch alone results in:

[6.680101] divide error:  [#1] SMP
[6.681342] Modules linked in: ath10k_pci(O) ath10k_core(O) ath
mac80211 cfg80211
[6.684876] CPU: 3 PID: 823 Comm: kworker/u8:2 Tainted: GW
O4.9.0-rc4-wt-ath+ #79
[6.688051] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[6.691644] Workqueue: ath10k_wq ath10k_core_register_work [ath10k_core]
[6.694309] task: 88000a19 task.stack: c96d4000
[6.695458] RIP: 0010:[]  []
ath10k_htc_connect_service+0x21b/0x420 [ath10k_core]


Michał

[PATCH] ath10k: add spectral scan support to wmi-tlv

[Michal Kazior]

Command structure and event flow doesn't seem to
be any different compared to existing
implementation for other firmware branches.

This patch effectively adds in-driver support for
spectral scanning on QCA61x4 and QCA9377.

Tested QCA9377 w/ WLAN.TF.1.0-00267-1.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 72 +++
 1 file changed, 72 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e64f59300a7c..7f9b1185b549 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -3136,6 +3136,76 @@ ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value)
return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_tlv_op_gen_vdev_spectral_conf(struct ath10k *ar,
+const struct 
wmi_vdev_spectral_conf_arg *arg)
+{
+   struct wmi_vdev_spectral_conf_cmd *cmd;
+   struct sk_buff *skb;
+   struct wmi_tlv *tlv;
+   void *ptr;
+   size_t len;
+
+   len = sizeof(*tlv) + sizeof(*cmd);
+   skb = ath10k_wmi_alloc_skb(ar, len);
+   if (!skb)
+   return ERR_PTR(-ENOMEM);
+
+   ptr = (void *)skb->data;
+   tlv = ptr;
+   tlv->tag = 
__cpu_to_le16(WMI_TLV_TAG_STRUCT_VDEV_SPECTRAL_CONFIGURE_CMD);
+   tlv->len = __cpu_to_le16(sizeof(*cmd));
+   cmd = (void *)tlv->value;
+   cmd->vdev_id = __cpu_to_le32(arg->vdev_id);
+   cmd->scan_count = __cpu_to_le32(arg->scan_count);
+   cmd->scan_period = __cpu_to_le32(arg->scan_period);
+   cmd->scan_priority = __cpu_to_le32(arg->scan_priority);
+   cmd->scan_fft_size = __cpu_to_le32(arg->scan_fft_size);
+   cmd->scan_gc_ena = __cpu_to_le32(arg->scan_gc_ena);
+   cmd->scan_restart_ena = __cpu_to_le32(arg->scan_restart_ena);
+   cmd->scan_noise_floor_ref = __cpu_to_le32(arg->scan_noise_floor_ref);
+   cmd->scan_init_delay = __cpu_to_le32(arg->scan_init_delay);
+   cmd->scan_nb_tone_thr = __cpu_to_le32(arg->scan_nb_tone_thr);
+   cmd->scan_str_bin_thr = __cpu_to_le32(arg->scan_str_bin_thr);
+   cmd->scan_wb_rpt_mode = __cpu_to_le32(arg->scan_wb_rpt_mode);
+   cmd->scan_rssi_rpt_mode = __cpu_to_le32(arg->scan_rssi_rpt_mode);
+   cmd->scan_rssi_thr = __cpu_to_le32(arg->scan_rssi_thr);
+   cmd->scan_pwr_format = __cpu_to_le32(arg->scan_pwr_format);
+   cmd->scan_rpt_mode = __cpu_to_le32(arg->scan_rpt_mode);
+   cmd->scan_bin_scale = __cpu_to_le32(arg->scan_bin_scale);
+   cmd->scan_dbm_adj = __cpu_to_le32(arg->scan_dbm_adj);
+   cmd->scan_chn_mask = __cpu_to_le32(arg->scan_chn_mask);
+
+   return skb;
+}
+
+static struct sk_buff *
+ath10k_wmi_tlv_op_gen_vdev_spectral_enable(struct ath10k *ar, u32 vdev_id,
+  u32 trigger, u32 enable)
+{
+   struct wmi_vdev_spectral_enable_cmd *cmd;
+   struct sk_buff *skb;
+   struct wmi_tlv *tlv;
+   void *ptr;
+   size_t len;
+
+   len = sizeof(*tlv) + sizeof(*cmd);
+   skb = ath10k_wmi_alloc_skb(ar, len);
+   if (!skb)
+   return ERR_PTR(-ENOMEM);
+
+   ptr = (void *)skb->data;
+   tlv = ptr;
+   tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_VDEV_SPECTRAL_ENABLE_CMD);
+   tlv->len = __cpu_to_le16(sizeof(*cmd));
+   cmd = (void *)tlv->value;
+   cmd->vdev_id = __cpu_to_le32(vdev_id);
+   cmd->trigger_cmd = __cpu_to_le32(trigger);
+   cmd->enable_cmd = __cpu_to_le32(enable);
+
+   return skb;
+}
+
 //
 /* TLV mappings */
 //
@@ -3542,6 +3612,8 @@ static const struct wmi_ops wmi_tlv_ops = {
.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
.gen_echo = ath10k_wmi_tlv_op_gen_echo,
+   .gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf,
+   .gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable,
 };
 
 static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
-- 
2.1.4

[PATCH] ath10k: fix null deref on wmi-tlv when trying spectral scan

[Michal Kazior]

WMI ops wrappers did not properly check for null
function pointers for spectral scan. This caused
null dereference crash with WMI-TLV based firmware
which doesn't implement spectral scan.

The crash could be triggered with:

  ip link set dev wlan0 up
  echo background > /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan_ctl

The crash looked like this:

  [  168.031989] BUG: unable to handle kernel NULL pointer dereference at   
(null)
  [  168.037406] IP: [<  (null)>]   (null)
  [  168.040395] PGD cdd4067 PUD fa0f067 PMD 0
  [  168.043303] Oops: 0010 [#1] SMP
  [  168.045377] Modules linked in: ath10k_pci(O) ath10k_core(O) ath mac80211 
cfg80211 [last unloaded: cfg80211]
  [  168.051560] CPU: 1 PID: 1380 Comm: bash Tainted: GW  O4.8.0 #78
  [  168.054336] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.7.5-20140531_083030-gandalf 04/01/2014
  [  168.059183] task: 88000c460c00 task.stack: 88000d4bc000
  [  168.061736] RIP: 0010:[<>]  [<  (null)>]   
(null)
  ...
  [  168.100620] Call Trace:
  [  168.101910]  [] ? ath10k_spectral_scan_config+0x96/0x200 
[ath10k_core]
  [  168.104871]  [] ? filemap_fault+0xb2/0x4a0
  [  168.106696]  [] write_file_spec_scan_ctl+0x116/0x280 
[ath10k_core]
  [  168.109618]  [] full_proxy_write+0x51/0x80
  [  168.111443]  [] __vfs_write+0x28/0x120
  [  168.113090]  [] ? security_file_permission+0x3d/0xc0
  [  168.114932]  [] ? percpu_down_read+0x12/0x60
  [  168.116680]  [] vfs_write+0xb8/0x1a0
  [  168.118293]  [] SyS_write+0x46/0xa0
  [  168.119912]  [] entry_SYSCALL_64_fastpath+0x1a/0xa4
  [  168.121737] Code:  Bad RIP value.
  [  168.123318] RIP  [<  (null)>]   (null)

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h 
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index c9a8bb1186f2..c7956e181f80 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -660,6 +660,9 @@ ath10k_wmi_vdev_spectral_conf(struct ath10k *ar,
struct sk_buff *skb;
u32 cmd_id;
 
+   if (!ar->wmi.ops->gen_vdev_spectral_conf)
+   return -EOPNOTSUPP;
+
skb = ar->wmi.ops->gen_vdev_spectral_conf(ar, arg);
if (IS_ERR(skb))
return PTR_ERR(skb);
@@ -675,6 +678,9 @@ ath10k_wmi_vdev_spectral_enable(struct ath10k *ar, u32 
vdev_id, u32 trigger,
struct sk_buff *skb;
u32 cmd_id;
 
+   if (!ar->wmi.ops->gen_vdev_spectral_enable)
+   return -EOPNOTSUPP;
+
skb = ar->wmi.ops->gen_vdev_spectral_enable(ar, vdev_id, trigger,
enable);
if (IS_ERR(skb))
-- 
2.1.4

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

[Michal Kazior]

On 24 October 2016 at 15:42, Simon Wunderlich  wrote:
> On Monday, October 24, 2016 3:33:24 PM CEST Johannes Berg wrote:
>> > > I think it would be reasonable only if the target channel is the
>> > > one we are using and we have done CSA. But when scanning non-
>> > > operative channels I don't think this could work.
>> >
>> > this has been sleeping for a while.. :)
>> > Would it make sense to rebase it and resubmit it for inclusion?
>> >
>> > Given the previous discussion we could change the logic as:
>> > * always passively scan DFS channels that are not usable
>> > * always actively scan DFS channels that are usable (i.e. CAC was
>> > performed).
>>
>> Doesn't that contradict what you said above?
>>
>> If we scan, don't we possibly lose our CAC result anyway, since we went
>> off-channel? In FCC at least? In ETSI I think we're allowed to do that
>> for a bit?
>
> I believe going off-channel was allowed in general - in fact, some routers CAC
> all channels on boot up and then keep the "usable" state forever.

I recall a discussion around this behavior [scan all on boot] a long
time ago. I believe earlier ETSI spec revisions allowed it but recent
ones made it more explicit that you can't cache CAC results like that
but don't take my word for it. I don't have have the spec on me now so
can't check.


Michał

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

[Michal Kazior]

On 24 October 2016 at 12:18, Matthias Klein  wrote:
> I also try to get a pcie wifi card (Compex WLE600VX) running in the clearfog
> pro board with kernel 4.4.
>
> As I read in this thread the "irg.mode=1" shoud help:
> https://www.spinics.net/lists/linux-wireless/msg155685.html
>
> But it is not working for me:
>
[...]
> [ 97.899370] ath10k_pci :01:00.0: Refused to change power state,
> currently in D3
> [ 97.938045] ath10k_pci :01:00.0: failed to wake up device : -110
> [ 97.944973] ath10k_pci: probe of :01:00.0 failed with error -110
[...]
> 01:00.0 Network controller: Qualcomm Atheros QCA988x 802.11ac Wireless
> Network Adapter (rev ff) (prog-if ff)
> !!! Unknown header type 7f

The device looks completely unresponsive.

I don't think this is MSI problem. This error happens before
interrupts are even set up.

I suspect platform/PCI/PM specific problem. I would suggest bisecting
the kernel and seeing which patch is making the difference.

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

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

 PCI: Add runtime PM support for PCIe ports

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

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

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


Michał

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

[Michal Kazior]

On 21 October 2016 at 18:25, Oliver Zemann  wrote:
> The problem is gone with kernel 4.7.3 on armbian - both compex cards work
> (wle600, wle900), unfortunately that kernel does not support sfp (seems like
> its kind of proprietary drivers from clearfog). So its a driver issue but i
> have no clue who to contact or where to file a bug report. The only
> available kernel which supports sfp (from solidrun, vendor of the clearfog
> board) is 4.4.23
>
> I am wondering if i could simply checkout the 4.4.x sources, merge the
> atheros drivers in it and try to rebuild the kernel?

That's what backports[1] are for.

The problem could be in the pci subsystem though so backporting the
driver itself might not solve your problem.

[1]: https://wireless.wiki.kernel.org/en/users/drivers/ath10k/backports


Michał

Re: [PATCH] ath10k: cache calibration data when the core is stopped.

[Michal Kazior]

On 13 September 2016 at 23:11, Marty Faltesek  wrote:
[...]
> +int
> +ath10k_cal_data_alloc(struct ath10k *ar, void **buf)
> +{
> +   u32 hi_addr;
> +   __le32 addr;
> +   int ret;
> +
> +   vfree(*buf);
> +   *buf = vmalloc(QCA988X_CAL_DATA_LEN);

Shouldn't you use ar->hw_params to get hw-specific caldata length?


[...]
> @@ -1714,6 +1750,12 @@ int ath10k_core_start(struct ath10k *ar, enum 
> ath10k_firmware_mode mode)
>
> INIT_LIST_HEAD(>arvifs);
>
> +   /*
> +* We are up now, so no need to cache calibration data.
> +*/

The comment style is:

 /* comment */

If it's multi-line it should be:

 /* comment
  * comment
  */

Ditto for other instances.


[...]
> @@ -1757,6 +1799,11 @@ void ath10k_core_stop(struct ath10k *ar)
> lockdep_assert_held(>conf_mutex);
> ath10k_debug_stop(ar);
>
> +   /*
> +* Cache caclibration data while stopped.

typo. "calibration"


Michał

Re: ath10k mesh mode issue

[Michal Kazior]

On 21 September 2016 at 18:27, Matteo Grandi  wrote:
> Hi Michal,
> thanks for the reply:
> I've already loaded the ath10k_core with the rowmode parameters, this
> is the modinfo result:
> root@Tam:~# modinfo ath10k_core
> filename:
> /lib/modules/3.14.48-g408ccb9/kernel/drivers/net/wireless/ath/ath10k/ath10k_core.ko
> license:Dual BSD/GPL
> description:Core module for QCA988X PCIe devices.
> author: Qualcomm Atheros
> srcversion: 407804E6D5A63597F137E9B
> depends:mac80211,cfg80211,compat,ath
> vermagic:   3.14.48-g408ccb9 SMP mod_unload modversions ARMv7 p2v8
> parm:   debug_mask:Debugging mask (uint)
> parm:   uart_print:Uart target debugging (bool)
> parm:   skip_otp:Skip otp failure for calibration in testmode (bool)
> parm:   cryptmode:Crypto mode: 0-hardware, 1-software (uint)
> parm:   rawmode:Use raw 802.11 frame datapath (bool)
[...]
> I've loaded adding the line ath10k_core rawmode=1 to the file etc/modules

Hmm... are you sure the module is actually loaded with the parameter?

You can check it with:

 grep . /sys/module/ath10k*/parameters/*


Michał

Re: ath10k mesh mode issue

[Michal Kazior]

On 16 September 2016 at 12:56, Matteo Grandi  wrote:
> Hello all,
[...]
> [8.589474] ath10k_pci :07:00.0: qca988x hw2.0 (0x4100016c,
> 0x043222ff sub :) fw 10.2.4.70.54 fwapi 5 bdapi 1 htt-ver 2.1
> wmi-op 5 htt-op 2 cal otp max-sta 128 raw 0 hwcrypto 1 features
> no-p2p,raw-mode

The "raw-mode" here only hints the firmware is capable of raw mode
operation which needs to be explicitly enabled.


> [8.589491] ath10k_pci :07:00.0: debug 1 debugfs 1 tracing 0
> dfs 0 testmode 1
> [8.691670] ath: EEPROM regdomain: 0x0
> [8.691680] ath: EEPROM indicates default country code should be used
> [8.691686] ath: doing EEPROM country->regdmn map search
> [8.691695] ath: country maps to regdmn code: 0x3a
> [8.691702] ath: Country alpha2 being used: US
> [8.691706] ath: Regpair used: 0x3a
> [  176.983250] ath10k_pci :07:00.0: must load driver with
> rawmode=1 to add mesh interfaces
[...]
> - I've also tried to load the ath10k modules adding the parameter
> rowmode=1 but I had an error "rawmode unknown parameter"

Only ath10k_core needs to be loaded with rawmode=1.

You can check available module parameters with:
  modinfo ath10k_core
  modinfo ath10k_pci


Michał

Re: ath10k inoperatable after suspend

[Michal Kazior]

On 19 September 2016 at 00:52, Steffen Arntz  wrote:
> Hi at all,
>
> I have an QCA988X based card in my laptop and have some "issues" with
> it, that I was not experiencing with my previous ath9k based card.
>
> The main issue is, that after suspend to RAM the card seems to be
> "hung up" and only reloading ath10k_core and ath10k_pci helps to fix
> the problem.

Hmm.. just a guess - maybe PCI config space isn't well preserved over
suspend and reloading modules prompts the PCI subsys to reset it.


Michal

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 17 September 2016 at 00:37, Hsu, Ryan  wrote:
[...]
>> + /* WMI and HTT may use separate HIF pipes and are not guaranteed to be
>> +  * serialized properly implicitly.
>> +  *
>> +  * Moreover (most) WMI commands have no explicit acknowledges. It is
>> +  * possible to infer it implicitly by poking firmware with echo
>> +  * command - getting a reply means all preceding comments have been
>> +  * (mostly) processed.
>> +  *
>> +  * In case of vdev create/delete this is sufficient.
>> +  *
>> +  * Without this it's possible to end up with a race when HTT Rx ring is
>> +  * started before vdev create/delete hack is complete allowing a short
>> +  * window of opportunity to receive (and Tx ACK) a bunch of frames.
>> +  */
>> + ret = ath10k_wmi_barrier(ar);
> QCA6174 UTF firmware seems doesn't support the WMI_ECHO command.
>
> [16460.274822] ath10k_pci :04:00.0: wmi tlv echo value 0x0ba991e9
> ...
> [16463.461970] ath10k_pci :04:00.0: failed to ping firmware: -110
> [16463.461975] ath10k_pci :04:00.0: failed to reset rx filter: -110
>
> Has anyone verified any AP solution to see if UTF mode is still working
> with after this patch?
>
> Anyway, I would like to exclude the workaround from all solution's UTF mode.
>
> Michal any concerns? (or maybe just for QCA61x4 if any...)

I didn't expect UTF wouldn't support echo.. Sorry!

If you skip this workaround for UTF I guess the device will (again) be
able to generate some bogus traffic on boot for UTF case. Not sure how
much of a problem that is (assuming it is at all).


Michal

Re: Ath10k probe response error related to mac80211 commit.

[Michal Kazior]

On 1 September 2016 at 22:52, Ben Greear  wrote:
> On 09/01/2016 11:53 AM, Johannes Berg wrote:
>> On Thu, 2016-09-01 at 11:23 -0700, Ben Greear wrote:
>>>
>>> Could easily be that others are corrupted too, but since probe resp
>>> is bad, the association will not proceed.
>>
>> makes sense.
>>
>>> Heh, I spent 4 days tracking this down, so I wanted to be precise in
>>> my bug report :)
>>
>> Ahrg, ouch. Sorry about that. I really didn't think the flag would
>> cause any issues for anyone.
>>
>>> The result I see is that there is an extra 10 bytes at the end of the
>>> frame on air.  But, it looks like the exact same pkt is sent to the
>>> firmware both with and without this patch.  Maybe the firmware is
>>> using the wrong tid or something like that due to how the station is
>>> created differently with this patch.
>>
>> That makes no sense though, unless this only happens on say the second
>> station that connects? Until the first station sends an authentication
>> frame, that patch really should have no impact whatsoever.
>
> Ok, I found the problem.
>
> In the 10.1 firmware (at least), it will force the TID to be NON-QOS-TID
> if the peer object does not have the qos_enabled flag set.  This is probably
> a work-around for some other thing lost in antiquity.
>
> When using my firmware that puts mgt frames over HTT, the TID for mgt
> frames was being over-ridden and set to non-qos TID.  Due to other
> hackery and work-arounds, mgt frames cannot be sent on the non-qos
> TID because they will be put on-air 10 bytes too long.  They must be
> sent on the mgt-tid or non-pause tid.

Sounds like 802.11 header vs 802.3 header length problem (24 - 14 =
10). You can hit this if you start playing around tx encap mode as
well.

You could try fooling firmware into thinking the frame has a different
length either in htt TX_FRM or tx fragment list (or both) but since
this seems to be related to RA/DA peer state at xmit time it's
probably not going to be reliable unless you introduce extra tx
flushing barriers in the driver.


Michał

Re: [PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

On 24 August 2016 at 19:20, Ben Greear <gree...@candelatech.com> wrote:
> On 07/19/2016 03:34 AM, Michal Kazior wrote:
>>
>> HW Rx filters and masks are not configured
>> properly by firmware during boot sequences. The
>> MAC_PCU_ADDR1 is set to 0s instead of 1s which
>> allows the HW to ACK any frame that passes through
>> MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
>> is misconfigured on boot as well.
>>
>> The combination of these bugs ended up with the
>> following manifestations:
>>  - "no channel configured; ignoring frame(s)!"
>>warnings in the driver
>>  - spurious ACKs (transmission) on the air during
>>firmware bootup sequences
>>
>> The former was a long standing and known bug
>> originally though mostly harmless.
>>
>> However Marek recently discovered that this
>> problem also involves ACKing *all* frames the HW
>> receives (including beacons ;). Such frames
>> are delivered to host and generate the former
>> warning as well.
>>
>> This could be a problem with regulatory compliance
>> in some rare cases (e.g. Taiwan which forbids
>> transmissions on channel 36 which is the default
>> bootup channel on 5Ghz band cards). The good news
>> is that it'd require someone else to violate
>> regulatory first to coerce our device to generate
>> and transmit an ACK.
>>
>> The problem could be reproduced in a rather busy
>> environment that has a lot of APs. The likelihood
>> could be increased by injecting an msleep() of
>> 5000 or longer immediately after
>> ath10k_htt_setup() in ath10k_core_start().
>>
>> The reason why the former warnings were only
>> showing up seldom is because the device was either
>> quickly reset again (i.e. during firmware probing)
>> or wmi vdev was created (which fixes hw and fw
>> states).
>>
>> It is technically possible for host driver to
>> override adequate hw registers however this can't
>> work reliably because the bug root cause lies in
>> incorrect firmware state on boot (internal
>> structure used to program MAC_PCU_ADDR1 is not
>> properly initialized) and only vdev create/delete
>> events can fix it. This is why the patch takes
>> dummy vdev approach.
>>
>> This could be fixed in firmware as well but having
>> this fixed in driver is more robust, most notably
>> when thinking of users of older firmware such as
>> 999.999.0.636.
>>
>> Reported-by: Marek Puzyniak <marek.puzyn...@tieto.com>
>> Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
>
>
> I was looking at firmware to make sure that I fixed what I could there
>
> From what I can tell, 10.4 should not have this bug.  Did you see this only
> on 10.1/10.2 firmware?  It is of course possible that I am mis-understanding
> 10.4

I did see it on 10.1 and 10.2. Don't recall seeing it on 10.4 though.
If you didn't see warnings on 10.4 even after adding msleep() as per
commit log then I guess it doesn't suffer from the bug.


Michał

Re: [PATCH 1/3] ath10k: Ensure there are no stale ar->txqs entries.

[Michal Kazior]

On 19 August 2016 at 03:26,   wrote:
> From: Ben Greear 
>
> I was seeing kernel crashes due to accessing freed memory
> while debugging a 9984 firmware that was crashing often.
>
> This patch fixes the crashes.  I am not certain if there
> is a better way or not.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/mac.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
> b/drivers/net/wireless/ath/ath10k/mac.c
> index 5659ef1..916119c 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4172,8 +4172,10 @@ static void ath10k_mac_txq_init(struct ieee80211_txq 
> *txq)
>  static void ath10k_mac_txq_unref(struct ath10k *ar, struct ieee80211_txq 
> *txq)
>  {
> struct ath10k_txq *artxq = (void *)txq->drv_priv;
> +   struct ath10k_txq *tmp, *walker;
> struct ath10k_skb_cb *cb;
> struct sk_buff *msdu;
> +   struct ieee80211_txq *txq_tmp;
> int msdu_id;
>
> if (!txq)
> @@ -4182,6 +4184,14 @@ static void ath10k_mac_txq_unref(struct ath10k *ar, 
> struct ieee80211_txq *txq)
> spin_lock_bh(>txqs_lock);
> if (!list_empty(>list))
> list_del_init(>list);
> +
> +   /* Remove from ar->txqs in case it still exists there. */
> +   list_for_each_entry_safe(walker, tmp, >txqs, list) {
> +   txq_tmp = container_of((void *)walker, struct ieee80211_txq,
> +  drv_priv);
> +   if (txq_tmp == txq)
> +   list_del(>list);
> +   }

How could this even happen? All artxq->list accesses (add/del) are
protected by txqs_lock so this shouldn't happen, no?

Do you perhaps have the logic around txqs reworked in your tree?


Michał

Re: [PATCH] ath10k: Allow setting coverage class

[Michal Kazior]

On 29 July 2016 at 17:09, Ben Greear  wrote:
> On 07/29/2016 07:52 AM, Benjamin Berg wrote:
[...]
>> Yeah, I am aware of the fact that the firmware may do internal resets
>> from time to time. The interesting question (and one for which I do not
>> know the answer) is whether we get a wmi or other event under all
>> conditions where the register may be rewritten due to a reset.
>>
>> The current code will re-set the register value after any wmi event
>> including debug messages. If this is not enough, then the only solution
>> might be to periodically poll the register values instead of relying on
>> a received event.
>
> You will get a dbglog event at least most of the time, so maybe that
> will be good enough.

But you need to remember you need to enable dbglog first to get these
events. I guess when coverage class is set the driver could,
internally, override dbglog mask so that these events are guaranteed
to be reported for the purpose of properly refreshing registers on
channel programming.

At that point I guess the update hook could be just placed in dbglog
handler alone instead of all over wmi_rx variants.


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: [PATCH 2/4] ath10k: Add provision for Rx descriptor abstraction

[Michal Kazior]

On 27 July 2016 at 14:59, Thiagarajan, Vasanthakumar
<vthia...@qti.qualcomm.com> wrote:
> On Wednesday 27 July 2016 06:13 PM, Michal Kazior wrote:
>> On 27 July 2016 at 14:36, Vasanthakumar Thiagarajan
[...]
>>> --- a/drivers/net/wireless/ath/ath10k/core.c
>>> +++ b/drivers/net/wireless/ath/ath10k/core.c
>>> @@ -68,6 +68,7 @@ static const struct ath10k_hw_params 
>>> ath10k_hw_params_list[] = {
>>>  .board_size = QCA988X_BOARD_DATA_SZ,
>>>  .board_ext_size = QCA988X_BOARD_EXT_DATA_SZ,
>>>  },
>>> +   .hw_rx_desc_ops = _rx_desc_ops,
>> [...]
>>> +struct ath10k_hw_rx_desc_ops {
>>> +   int (*rx_desc_get_l3_pad_bytes)(struct htt_rx_desc *rxd);
>>> +};
>>
>> Benjamin is trying to implement coverage class by poking hw registers
>> over firmware's head [1]. I'm thinking the hw_rx_desc_ops could be
>> generalized to hw_ops so it can be used for doing hw-specific hacks as
>> well. What do you think?
>
> Sure. Instead of reworking this patch set, can this be done in a separate 
> patch?.

I don't really mind especially since there's not much comments on his work yet.


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: [PATCH 2/4] ath10k: Add provision for Rx descriptor abstraction

[Michal Kazior]

On 27 July 2016 at 14:36, Vasanthakumar Thiagarajan
 wrote:
> There are slight differences in Rx hw descriptor information
> among different chips. So far driver does not use those new
> information for any functionalities, but there is one important
> information which is available from QCA99X0 onwards to indicate
> the number of bytes that hw padded at the begining of the rx
> payload and this information is needed to undecap the rx
> packet. Add an abstraction for Rx desc to make use of the
> new desc information available. The callback that this patch
> defines to retrieve the padding bytes will be used in follow-up
> patch.
>
> Signed-off-by: Vasanthakumar Thiagarajan 
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 12 
>  drivers/net/wireless/ath/ath10k/hw.c   | 13 +
>  drivers/net/wireless/ath/ath10k/hw.h   | 12 
>  3 files changed, 37 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
> b/drivers/net/wireless/ath/ath10k/core.c
> index e889829..9c5e93b 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -68,6 +68,7 @@ static const struct ath10k_hw_params 
> ath10k_hw_params_list[] = {
> .board_size = QCA988X_BOARD_DATA_SZ,
> .board_ext_size = QCA988X_BOARD_EXT_DATA_SZ,
> },
> +   .hw_rx_desc_ops = _rx_desc_ops,
[...]
> +struct ath10k_hw_rx_desc_ops {
> +   int (*rx_desc_get_l3_pad_bytes)(struct htt_rx_desc *rxd);
> +};

Benjamin is trying to implement coverage class by poking hw registers
over firmware's head [1]. I'm thinking the hw_rx_desc_ops could be
generalized to hw_ops so it can be used for doing hw-specific hacks as
well. What do you think?

[1]: http://lists.infradead.org/pipermail/ath10k/2016-July/008107.html


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: [PATCH] ath10k: Allow setting coverage class

[Michal Kazior]

On 27 July 2016 at 10:33, Benjamin Berg  wrote:
> Unfortunately ath10k does not generally allow modifying the coverage class
> with the stock firmware and Qualcomm has so far refused to implement this
> feature so that it can be properly supported in ath10k. If we however know
> the registers that need to be modified for proper operation with a higher
> coverage class, then we can do these modifications from the driver.
>
> This patch implements this hack for first generation cards which are based
> on a core that is similar to ath9k. The registers are modified in place and
> need to be re-written every time the firmware sets them. To achieve this
> the register status is verified after any event from the firmware.

"After any event from firmware" would also need to include HTT events
which your patch doesn't consider.


> The coverage class may not be modified temporarily right after the card
> re-initializes the registers. This is for example the case during scanning.
>
> A warning will be generated if the hack is not supported on the card or
> unexpected values are hit. There is no error reporting for userspace
> applications though (this is a limitation in the mac80211 driver
> interface).

With a recent change in ath10k the ieee80211_ops can be updated in
ar->ops so you can NULL-ify .set_coverage_class before registering to
mac80211. See how wake_tx_queue() is masked. You could use this to
mask it for WAVE2 devices that haven't been verified.


[...]
> @@ -257,6 +258,12 @@ extern const struct ath10k_hw_regs qca6174_regs;
>  extern const struct ath10k_hw_regs qca99x0_regs;
>  extern const struct ath10k_hw_regs qca4019_regs;
>
> +enum ath10k_hw_mac_core_rev {
> +   ATH10K_HW_MAC_CORE_UNKNOWN = 0,
> +   ATH10K_HW_MAC_CORE_ATH9K,

WAVE1 would be more appropriate.


> +   ATH10K_HW_MAC_CORE_WAVE2,
> +};
> +
[...]
> +#define ATH9K_MAC_TIME_OUT 0x8014
> +#define ATH9K_MAC_TIME_OUT_MAX 0x3FFF
> +#define ATH9K_MAC_TIME_OUT_ACK 0x3FFF
> +#define ATH9K_MAC_TIME_OUT_ACK_S   0
> +#define ATH9K_MAC_TIME_OUT_CTS 0x3FFF
> +#define ATH9K_MAC_TIME_OUT_CTS_S   16
> +
> +#define ATH9K_MAC_IFS_SLOT 0x1070
> +#define ATH9K_MAC_IFS_SLOT_M   0x
> +#define ATH9K_MAC_IFS_SLOT_RESV0   0x

The prefix is wrong. It shouldn't be ATH9K.

Moreover FW refers to these register as PCU_ACK_CTS_TIMEOUT and
PCU_GBL_IFS_SLOT.


> +
>  #endif /* _HW_H_ */
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
> b/drivers/net/wireless/ath/ath10k/mac.c
> index 55c823f..a9b587e 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -5372,6 +5372,15 @@ static void ath10k_bss_info_changed(struct 
> ieee80211_hw *hw,
> mutex_unlock(>conf_mutex);
>  }
>
> +static void ath10k_set_coverage_class(struct ieee80211_hw *hw,
> + s16 value)

Wrong function prefix/name. Should be: ath10k_mac_op_set_coverage_class


> +{
> +   struct ath10k *ar = hw->priv;
> +
> +   if (ath10k_wmi_pdev_set_coverage_class(ar, value) == -EOPNOTSUPP)
> +   ath10k_warn(ar, "Modification of coverage class is not 
> supported!\n");

Warning doesn't match the style used in ath10k. "failed to set
coverage class: not supported\n".


[...]
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h 
> b/drivers/net/wireless/ath/ath10k/wmi-ops.h
> index 64ebd30..3ebc57e 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
> +++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
> @@ -52,6 +52,8 @@ struct wmi_ops {
> int (*pull_wow_event)(struct ath10k *ar, struct sk_buff *skb,
>   struct wmi_wow_ev_arg *arg);
> enum wmi_txbf_conf (*get_txbf_conf_scheme)(struct ath10k *ar);
> +   void (*set_pdev_coverage_class)(struct ath10k *ar,
> +   s16 value);

WMI ops are used to generate and process events and command buffers.
These ops are not supposed to have side-effects but
"set_pdev_coverage_class" implies it does.


[...]
>  /* MAIN WMI cmd track */
>  static struct wmi_cmd_map wmi_cmd_map = {
> @@ -3096,6 +3097,121 @@ ath10k_wmi_op_pull_peer_kick_ev(struct ath10k *ar, 
> struct sk_buff *skb,
> return 0;
>  }
>
> +static void
> +ath10k_ath9k_set_pdev_coverage_class(struct ath10k *ar, s16 value)

The prefix is wrong. "ath9k" should be used here.

ath10k_wmi_ is appropriate for stuff in wmi.c


[...]
> +   /* Do some sanity checks on the slottime register. */
> +   if (unlikely(slottime_reg % counters_freq_mhz)) {
> +   ath10k_warn(ar,
> +   "Not adjusting coverage class timeouts, expected 
> an integer microsecond slot time in HW register\n");

Wrong message style.


> +
> +   goto store_regs;
> +   }
> +
> +   slottime = (slottime_reg & ATH9K_MAC_IFS_SLOT_M) / counters_freq_mhz;
> +   if 

Re: TCP performance regression in mac80211 triggered by the fq code

[Michal Kazior]

On 20 July 2016 at 17:24, Toke Høiland-Jørgensen  wrote:
> Toke Høiland-Jørgensen  writes:
>
>> Felix Fietkau  writes:
>>
>>> - if I put a hack in the fq code to force the hash to a constant value
>>> (effectively disabling fq without disabling codel), the problem
>>> disappears and even multiple streams get proper performance.
>>
>> There's definitely something iffy about the hashing. Here's the output
>> relevant line from the aqm debug file after running a single TCP stream
>> for 60 seconds to that station:
>>
>> ifname addr tid ac backlog-bytes backlog-packets flows drops marks overlimit 
>> collisions
>> tx-bytes tx-packets
>> wlp2s0 04:f0:21:1e:74:20 0 2 0 0 146 16 0 0 0 717758966 467925
>>
>> (there are two extra fields here; I added per-txq CoDel stats, will send
>> a patch later).
>>
>> This shows that the txq has 146 flows associated from that one TCP flow.
>> Looking at this over time, it seems that each time the queue runs empty
>> (which happens way too often, which is what I was originally
>> investigating), another flow is assigned.
>>
>> Michal, any idea why? :)
>
> And to answer this: because the flow is being freed to be reassigned
> when it runs empty, but the counter is not decremented. Is this
> deliberate? I.e. is the 'flows' var supposed to be a total 'new_flows'
> counter and not a measure of the current number of assigned flows?

Yes, it is deliberate. fq_codel qdisc does the same thing and I just
mimicked it.


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: [PATCH 2/2] ath10k: Fix sending NULL/ Qos NULL data frames for QCA99X0 and later

[Michal Kazior]

On 20 July 2016 at 13:43, Shajakhan, Mohammed Shafi (Mohammed Shafi)
 wrote:
> Michal,
>
> Can you please let me know if this change is fine or not ?
> I am waiting infinitely for  your reply long time

Sorry. I was absent for a while and this email slipped by.

Quoting your other email:

> is this patch is good to go (or) should i re-work  ?
> I had replied to Michal's comment of introducing a new firmware
> feature flag will not address the issue in older firmware / code.
> Let me know if i had missed something very obvious.

I couldn't really find any conclusion to this thread in my inbox.

The hw_params approach is definitely wrong. The ACK problem is
firmware-specific, not hardware-specific.

I'm fine with removing the workaround completely if 636 is guaranteed
to not be broken, including AP and P2P GO operation. The client
operation will probably work since wmi-roam event is used for HW
connection monitoring.

Nullfunc tx-status ack/noack reports could be probably fixed up using
sta kickout events (with short timeouts) as a fallback. This would
make it easier for users because otherwise they'd need to enable
disassoc_low_ack in hostapd (which is probably impossible with
wpa_supplicant for p2p, no?).


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: TCP performance regression in mac80211 triggered by the fq code

[Michal Kazior]

On 12 July 2016 at 12:09, Felix Fietkau  wrote:
> Hi,
>
> With Toke's ath9k txq patch I've noticed a pretty nasty performance
> regression when running local iperf on an AP (running the txq stuff) to
> a wireless client.
>
> Here's some things that I found:
> - when I use only one TCP stream I get around 90-110 Mbit/s
> - when running multiple TCP streams, I get only 35-40 Mbit/s total

What is the baseline here (i.e. without fq/txq stuff)? Is it ~100mbps?

Did you try running multiple streams, each on separate tids (matching
the same AC perhaps) or different clients?


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

[RFC] ath10k: silence firmware file probing warnings

[Michal Kazior]

Firmware files are versioned to prevent older
driver instances to load unsupported firmware
blobs. This is reflected with a fallback logic
which attempts to load several firmware files.

This however produced a lot of unnecessary
warnings sometimes confusing users and leading
them to rename firmware files making things even
more confusing.

Hence use request_firmware_direct() which does not
produce extra warnings. This shouldn't really
break anything because most modern systems don't
rely on udev/hotplug helpers to load firmware
files anymore.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 11 +--
 drivers/net/wireless/ath/ath10k/testmode.c |  5 -
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index e88982921aa3..81bfb71fe876 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -431,7 +431,10 @@ static const struct firmware *ath10k_fetch_fw_file(struct 
ath10k *ar,
dir = ".";
 
snprintf(filename, sizeof(filename), "%s/%s", dir, file);
-   ret = request_firmware(, filename, ar->dev);
+   ret = request_firmware_direct(, filename, ar->dev);
+   ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n",
+  filename, ret);
+
if (ret)
return ERR_PTR(ret);
 
@@ -1089,12 +1092,8 @@ int ath10k_core_fetch_firmware_api_n(struct ath10k *ar, 
const char *name,
/* first fetch the firmware file (firmware-*.bin) */
fw_file->firmware = ath10k_fetch_fw_file(ar, ar->hw_params.fw.dir,
 name);
-   if (IS_ERR(fw_file->firmware)) {
-   ath10k_err(ar, "could not fetch firmware file '%s/%s': %ld\n",
-  ar->hw_params.fw.dir, name,
-  PTR_ERR(fw_file->firmware));
+   if (IS_ERR(fw_file->firmware))
return PTR_ERR(fw_file->firmware);
-   }
 
data = fw_file->firmware->data;
len = fw_file->firmware->size;
diff --git a/drivers/net/wireless/ath/ath10k/testmode.c 
b/drivers/net/wireless/ath/ath10k/testmode.c
index 120f4234d3b0..fe49e7a83d00 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -149,7 +149,10 @@ static int ath10k_tm_fetch_utf_firmware_api_1(struct 
ath10k *ar,
 ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE);
 
/* load utf firmware image */
-   ret = request_firmware(_file->firmware, filename, ar->dev);
+   ret = request_firmware_direct(_file->firmware, filename, ar->dev);
+   ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode fw request '%s': %d\n",
+  filename, ret);
+
if (ret) {
ath10k_warn(ar, "failed to retrieve utf firmware '%s': %d\n",
filename, ret);
-- 
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/4] ath10k: implement wmi echo command

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 17 +
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 29 +
 drivers/net/wireless/ath/ath10k/wmi.c | 23 +++
 3 files changed, 69 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h 
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 64ebd304f907..b1d88fa60d11 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -194,6 +194,7 @@ struct wmi_ops {
struct sk_buff *(*gen_pdev_bss_chan_info_req)
(struct ath10k *ar,
 enum wmi_bss_survey_req_type type);
+   struct sk_buff *(*gen_echo)(struct ath10k *ar, u32 value);
 };
 
 int ath10k_wmi_cmd_send(struct ath10k *ar, struct sk_buff *skb, u32 cmd_id);
@@ -1382,4 +1383,20 @@ ath10k_wmi_pdev_bss_chan_info_request(struct ath10k *ar,
   wmi->cmd->pdev_bss_chan_info_request_cmdid);
 }
 
+static inline int
+ath10k_wmi_echo(struct ath10k *ar, u32 value)
+{
+   struct ath10k_wmi *wmi = >wmi;
+   struct sk_buff *skb;
+
+   if (!wmi->ops->gen_echo)
+   return -EOPNOTSUPP;
+
+   skb = wmi->ops->gen_echo(ar, value);
+   if (IS_ERR(skb))
+   return PTR_ERR(skb);
+
+   return ath10k_wmi_cmd_send(ar, skb, wmi->cmd->echo_cmdid);
+}
+
 #endif
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e09337ee7c96..cd595855af36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -3081,6 +3081,34 @@ ath10k_wmi_tlv_op_gen_adaptive_qcs(struct ath10k *ar, 
bool enable)
return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_tlv_op_gen_echo(struct ath10k *ar, u32 value)
+{
+   struct wmi_echo_cmd *cmd;
+   struct wmi_tlv *tlv;
+   struct sk_buff *skb;
+   void *ptr;
+   size_t len;
+
+   len = sizeof(*tlv) + sizeof(*cmd);
+   skb = ath10k_wmi_alloc_skb(ar, len);
+   if (!skb)
+   return ERR_PTR(-ENOMEM);
+
+   ptr = (void *)skb->data;
+   tlv = ptr;
+   tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_ECHO_CMD);
+   tlv->len = __cpu_to_le16(sizeof(*cmd));
+   cmd = (void *)tlv->value;
+   cmd->value = cpu_to_le32(value);
+
+   ptr += sizeof(*tlv);
+   ptr += sizeof(*cmd);
+
+   ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv echo value 0x%08x\n", value);
+   return skb;
+}
+
 //
 /* TLV mappings */
 //
@@ -3485,6 +3513,7 @@ static const struct wmi_ops wmi_tlv_ops = {
.gen_adaptive_qcs = ath10k_wmi_tlv_op_gen_adaptive_qcs,
.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+   .gen_echo = ath10k_wmi_tlv_op_gen_echo,
 };
 
 static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
b/drivers/net/wireless/ath/ath10k/wmi.c
index 169cd2e783eb..9ae4aacadb38 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -7649,6 +7649,24 @@ ath10k_wmi_10_4_ext_resource_config(struct ath10k *ar,
return skb;
 }
 
+static struct sk_buff *
+ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
+{
+   struct wmi_echo_cmd *cmd;
+   struct sk_buff *skb;
+
+   skb = ath10k_wmi_alloc_skb(ar, sizeof(*cmd));
+   if (!skb)
+   return ERR_PTR(-ENOMEM);
+
+   cmd = (struct wmi_echo_cmd *)skb->data;
+   cmd->value = cpu_to_le32(value);
+
+   ath10k_dbg(ar, ATH10K_DBG_WMI,
+  "wmi echo value 0x%08x\n", value);
+   return skb;
+}
+
 static const struct wmi_ops wmi_ops = {
.rx = ath10k_wmi_op_rx,
.map_svc = wmi_main_svc_map,
@@ -7709,6 +7727,7 @@ static const struct wmi_ops wmi_ops = {
.gen_delba_send = ath10k_wmi_op_gen_delba_send,
.fw_stats_fill = ath10k_wmi_main_op_fw_stats_fill,
.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+   .gen_echo = ath10k_wmi_op_gen_echo,
/* .gen_bcn_tmpl not implemented */
/* .gen_prb_tmpl not implemented */
/* .gen_p2p_go_bcn_ie not implemented */
@@ -,6 +7796,7 @@ static const struct wmi_ops wmi_10_1_ops = {
.gen_delba_send = ath10k_wmi_op_gen_delba_send,
.fw_stats_fill = ath10k_wmi_10x_op_fw_stats_fill,
.get_vdev_subtype = ath10k_wmi_op_get_vdev_subtype,
+   .gen_echo = ath10k_wmi_op_gen_echo,
/* .gen_bcn_tmpl not implemented */
/* .gen_prb_tmpl not implemented */
/* .gen_p2p_go_bcn_ie not implemented */
@@ -7796,6 +7816,7 @@ static const struct wmi_ops wmi_10_2_ops = {
.

[PATCH 4/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

HW Rx filters and masks are not configured
properly by firmware during boot sequences. The
MAC_PCU_ADDR1 is set to 0s instead of 1s which
allows the HW to ACK any frame that passes through
MAC_PCU_RX_FILTER. The MAC_PCU_RX_FILTER itself
is misconfigured on boot as well.

The combination of these bugs ended up with the
following manifestations:
 - "no channel configured; ignoring frame(s)!"
   warnings in the driver
 - spurious ACKs (transmission) on the air during
   firmware bootup sequences

The former was a long standing and known bug
originally though mostly harmless.

However Marek recently discovered that this
problem also involves ACKing *all* frames the HW
receives (including beacons ;). Such frames
are delivered to host and generate the former
warning as well.

This could be a problem with regulatory compliance
in some rare cases (e.g. Taiwan which forbids
transmissions on channel 36 which is the default
bootup channel on 5Ghz band cards). The good news
is that it'd require someone else to violate
regulatory first to coerce our device to generate
and transmit an ACK.

The problem could be reproduced in a rather busy
environment that has a lot of APs. The likelihood
could be increased by injecting an msleep() of
5000 or longer immediately after
ath10k_htt_setup() in ath10k_core_start().

The reason why the former warnings were only
showing up seldom is because the device was either
quickly reset again (i.e. during firmware probing)
or wmi vdev was created (which fixes hw and fw
states).

It is technically possible for host driver to
override adequate hw registers however this can't
work reliably because the bug root cause lies in
incorrect firmware state on boot (internal
structure used to program MAC_PCU_ADDR1 is not
properly initialized) and only vdev create/delete
events can fix it. This is why the patch takes
dummy vdev approach.

This could be fixed in firmware as well but having
this fixed in driver is more robust, most notably
when thinking of users of older firmware such as
999.999.0.636.

Reported-by: Marek Puzyniak <marek.puzyn...@tieto.com>
Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c | 68 ++
 1 file changed, 68 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c 
b/drivers/net/wireless/ath/ath10k/core.c
index e88982921aa3..d2e255418d1b 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1705,6 +1705,55 @@ static int ath10k_core_init_firmware_features(struct 
ath10k *ar)
return 0;
 }
 
+static int ath10k_core_reset_rx_filter(struct ath10k *ar)
+{
+   int ret;
+   int vdev_id;
+   int vdev_type;
+   int vdev_subtype;
+   const u8 *vdev_addr;
+
+   vdev_id = 0;
+   vdev_type = WMI_VDEV_TYPE_STA;
+   vdev_subtype = ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
+   vdev_addr = ar->mac_addr;
+
+   ret = ath10k_wmi_vdev_create(ar, vdev_id, vdev_type, vdev_subtype,
+vdev_addr);
+   if (ret) {
+   ath10k_err(ar, "failed to create dummy vdev: %d\n", ret);
+   return ret;
+   }
+
+   ret = ath10k_wmi_vdev_delete(ar, vdev_id);
+   if (ret) {
+   ath10k_err(ar, "failed to delete dummy vdev: %d\n", ret);
+   return ret;
+   }
+
+   /* WMI and HTT may use separate HIF pipes and are not guaranteed to be
+* serialized properly implicitly.
+*
+* Moreover (most) WMI commands have no explicit acknowledges. It is
+* possible to infer it implicitly by poking firmware with echo
+* command - getting a reply means all preceding comments have been
+* (mostly) processed.
+*
+* In case of vdev create/delete this is sufficient.
+*
+* Without this it's possible to end up with a race when HTT Rx ring is
+* started before vdev create/delete hack is complete allowing a short
+* window of opportunity to receive (and Tx ACK) a bunch of frames.
+*/
+   ret = ath10k_wmi_barrier(ar);
+   if (ret) {
+   ath10k_err(ar, "failed to ping firmware: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
 int ath10k_core_start(struct ath10k *ar, enum ath10k_firmware_mode mode,
  const struct ath10k_fw_components *fw)
 {
@@ -1872,6 +1921,25 @@ int ath10k_core_start(struct ath10k *ar, enum 
ath10k_firmware_mode mode,
goto err_hif_stop;
}
 
+   /* Some firmware revisions do not properly set up hardware rx filter
+* registers.
+*
+* A known example from QCA9880 and 10.2.4 is that MAC_PCU_ADDR1_MASK
+* is filled with 0s instead of 1s allowing HW to respond with ACKs to
+* any frames that matches MAC_PCU_RX_FILTER wh

[PATCH 3/4] ath10k: add wmi command barrier utility

[Michal Kazior]

This allows placing command barriers for explicit
serializing and synchronizing state.

Useful for future driver development.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/wmi.c  | 31 +++
 drivers/net/wireless/ath/ath10k/wmi.h  |  1 +
 3 files changed, 33 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 9374bcde3d35..bf385052b168 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -142,6 +142,7 @@ struct ath10k_wmi {
enum ath10k_htc_ep_id eid;
struct completion service_ready;
struct completion unified_ready;
+   struct completion barrier;
wait_queue_head_t tx_credits_wq;
DECLARE_BITMAP(svc_map, WMI_SERVICE_MAX);
struct wmi_cmd_map *cmd;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
b/drivers/net/wireless/ath/ath10k/wmi.c
index 8ca76ecd7e9b..2bd9f2d0f186 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -29,6 +29,9 @@
 #include "p2p.h"
 #include "hw.h"
 
+#define ATH10K_WMI_BARRIER_ECHO_ID 0xBA991E9
+#define ATH10K_WMI_BARRIER_TIMEOUT_HZ (3 * HZ)
+
 /* MAIN WMI cmd track */
 static struct wmi_cmd_map wmi_cmd_map = {
.init_cmdid = WMI_INIT_CMDID,
@@ -2507,6 +2510,9 @@ void ath10k_wmi_event_echo(struct ath10k *ar, struct 
sk_buff *skb)
ath10k_dbg(ar, ATH10K_DBG_WMI,
   "wmi event echo value 0x%08x\n",
   le32_to_cpu(arg.value));
+
+   if (le32_to_cpu(arg.value) == ATH10K_WMI_BARRIER_ECHO_ID)
+   complete(>wmi.barrier);
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -7689,6 +7695,30 @@ ath10k_wmi_op_gen_echo(struct ath10k *ar, u32 value)
return skb;
 }
 
+int
+ath10k_wmi_barrier(struct ath10k *ar)
+{
+   int ret;
+   int time_left;
+
+   spin_lock_bh(>data_lock);
+   reinit_completion(>wmi.barrier);
+   spin_unlock_bh(>data_lock);
+
+   ret = ath10k_wmi_echo(ar, ATH10K_WMI_BARRIER_ECHO_ID);
+   if (ret) {
+   ath10k_warn(ar, "failed to submit wmi echo: %d\n", ret);
+   return ret;
+   }
+
+   time_left = wait_for_completion_timeout(>wmi.barrier,
+   ATH10K_WMI_BARRIER_TIMEOUT_HZ);
+   if (!time_left)
+   return -ETIMEDOUT;
+
+   return 0;
+}
+
 static const struct wmi_ops wmi_ops = {
.rx = ath10k_wmi_op_rx,
.map_svc = wmi_main_svc_map,
@@ -8086,6 +8116,7 @@ int ath10k_wmi_attach(struct ath10k *ar)
 
init_completion(>wmi.service_ready);
init_completion(>wmi.unified_ready);
+   init_completion(>wmi.barrier);
 
INIT_WORK(>svc_rdy_work, ath10k_wmi_event_service_ready_work);
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h 
b/drivers/net/wireless/ath/ath10k/wmi.h
index 086d78807d2f..89adfa90ee8d 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6628,5 +6628,6 @@ void ath10k_wmi_10_4_op_fw_stats_fill(struct ath10k *ar,
  char *buf);
 int ath10k_wmi_op_get_vdev_subtype(struct ath10k *ar,
   enum wmi_vdev_subtype subtype);
+int ath10k_wmi_barrier(struct ath10k *ar);
 
 #endif /* _WMI_H_ */
-- 
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 0/4] ath10k: fix spurious tx/rx during boot

[Michal Kazior]

Hi,

Recently Marek discovered the device transmits
"stuff" during device/driver boot.

The problem is related with the long known "no
channel" warning and it's a consequence of the
same bug - rx filters not being programmed
properly by firmware.

See last patch for more details.

I didn't do extensive testing but I can confirm
that I am no longer able to reroduce "no channel"
warnings and Marek tells me he no longer sees any
signal bumps on oscilloscope with his QCA9882.



Michal Kazior (4):
  ath10k: implement wmi echo command
  ath10k: implement wmi echo event
  ath10k: add wmi command barrier utility
  ath10k: fix spurious tx/rx during boot

 drivers/net/wireless/ath/ath10k/core.c| 68 +
 drivers/net/wireless/ath/ath10k/core.h|  1 +
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 29 +++
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 57 +
 drivers/net/wireless/ath/ath10k/wmi.c | 83 ++-
 drivers/net/wireless/ath/ath10k/wmi.h |  5 ++
 6 files changed, 242 insertions(+), 1 deletion(-)

-- 
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 2/4] ath10k: implement wmi echo event

[Michal Kazior]

Will be useful for implementing command barriers.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/wmi-ops.h | 12 
 drivers/net/wireless/ath/ath10k/wmi-tlv.c | 28 
 drivers/net/wireless/ath/ath10k/wmi.c | 29 -
 drivers/net/wireless/ath/ath10k/wmi.h |  4 
 4 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h 
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index b1d88fa60d11..c67eda78b69e 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -51,6 +51,8 @@ struct wmi_ops {
struct wmi_roam_ev_arg *arg);
int (*pull_wow_event)(struct ath10k *ar, struct sk_buff *skb,
  struct wmi_wow_ev_arg *arg);
+   int (*pull_echo_ev)(struct ath10k *ar, struct sk_buff *skb,
+   struct wmi_echo_ev_arg *arg);
enum wmi_txbf_conf (*get_txbf_conf_scheme)(struct ath10k *ar);
 
struct sk_buff *(*gen_pdev_suspend)(struct ath10k *ar, u32 suspend_opt);
@@ -350,6 +352,16 @@ ath10k_wmi_pull_wow_event(struct ath10k *ar, struct 
sk_buff *skb,
return ar->wmi.ops->pull_wow_event(ar, skb, arg);
 }
 
+static inline int
+ath10k_wmi_pull_echo_ev(struct ath10k *ar, struct sk_buff *skb,
+   struct wmi_echo_ev_arg *arg)
+{
+   if (!ar->wmi.ops->pull_echo_ev)
+   return -EOPNOTSUPP;
+
+   return ar->wmi.ops->pull_echo_ev(ar, skb, arg);
+}
+
 static inline enum wmi_txbf_conf
 ath10k_wmi_get_txbf_conf_scheme(struct ath10k *ar)
 {
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index cd595855af36..a42f52dd9a36 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1223,6 +1223,33 @@ ath10k_wmi_tlv_op_pull_wow_ev(struct ath10k *ar, struct 
sk_buff *skb,
return 0;
 }
 
+static int ath10k_wmi_tlv_op_pull_echo_ev(struct ath10k *ar,
+ struct sk_buff *skb,
+ struct wmi_echo_ev_arg *arg)
+{
+   const void **tb;
+   const struct wmi_echo_event *ev;
+   int ret;
+
+   tb = ath10k_wmi_tlv_parse_alloc(ar, skb->data, skb->len, GFP_ATOMIC);
+   if (IS_ERR(tb)) {
+   ret = PTR_ERR(tb);
+   ath10k_warn(ar, "failed to parse tlv: %d\n", ret);
+   return ret;
+   }
+
+   ev = tb[WMI_TLV_TAG_STRUCT_ECHO_EVENT];
+   if (!ev) {
+   kfree(tb);
+   return -EPROTO;
+   }
+
+   arg->value = ev->value;
+
+   kfree(tb);
+   return 0;
+}
+
 static struct sk_buff *
 ath10k_wmi_tlv_op_gen_pdev_suspend(struct ath10k *ar, u32 opt)
 {
@@ -3457,6 +3484,7 @@ static const struct wmi_ops wmi_tlv_ops = {
.pull_fw_stats = ath10k_wmi_tlv_op_pull_fw_stats,
.pull_roam_ev = ath10k_wmi_tlv_op_pull_roam_ev,
.pull_wow_event = ath10k_wmi_tlv_op_pull_wow_ev,
+   .pull_echo_ev = ath10k_wmi_tlv_op_pull_echo_ev,
.get_txbf_conf_scheme = ath10k_wmi_tlv_txbf_conf_scheme,
 
.gen_pdev_suspend = ath10k_wmi_tlv_op_gen_pdev_suspend,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c 
b/drivers/net/wireless/ath/ath10k/wmi.c
index 9ae4aacadb38..8ca76ecd7e9b 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2495,7 +2495,18 @@ exit:
 
 void ath10k_wmi_event_echo(struct ath10k *ar, struct sk_buff *skb)
 {
-   ath10k_dbg(ar, ATH10K_DBG_WMI, "WMI_ECHO_EVENTID\n");
+   struct wmi_echo_ev_arg arg = {};
+   int ret;
+
+   ret = ath10k_wmi_pull_echo_ev(ar, skb, );
+   if (ret) {
+   ath10k_warn(ar, "failed to parse echo: %d\n", ret);
+   return;
+   }
+
+   ath10k_dbg(ar, ATH10K_DBG_WMI,
+  "wmi event echo value 0x%08x\n",
+  le32_to_cpu(arg.value));
 }
 
 int ath10k_wmi_event_debug_mesg(struct ath10k *ar, struct sk_buff *skb)
@@ -4792,6 +4803,17 @@ static int ath10k_wmi_op_pull_roam_ev(struct ath10k *ar, 
struct sk_buff *skb,
return 0;
 }
 
+static int ath10k_wmi_op_pull_echo_ev(struct ath10k *ar,
+ struct sk_buff *skb,
+ struct wmi_echo_ev_arg *arg)
+{
+   struct wmi_echo_event *ev = (void *)skb->data;
+
+   arg->value = ev->value;
+
+   return 0;
+}
+
 int ath10k_wmi_event_ready(struct ath10k *ar, struct sk_buff *skb)
 {
struct wmi_rdy_ev_arg arg = {};
@@ -7683,6 +7705,7 @@ static const struct wmi_ops wmi_ops = {
.pull_rdy = ath10k_wmi_op_pull_rdy_ev,
.pull_fw_stats = ath10k_wmi_main_op_pull_fw_stats,
.pull_roam_ev = ath10k_wmi_op_pull_roa

[PATCH] mac80211: fix fq lockdep warnings

[Michal Kazior]

Some lockdep assertions were not fulfilled and
resulted in a kernel warning/call trace if driver
used intermediate software queues (e.g. ath10k).

Existing code sequences should've guranteed safety
but it's always good to be extra careful.

The call trace could look like this:

 [ 237.335805] [ cut here ]
 [ 237.335852] WARNING: CPU: 3 PID: 1921 at include/net/fq_impl.h:22 
fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.335855] Modules linked in: ath10k_pci(E-) ath10k_core(E) ath(E) 
mac80211(E) cfg80211(E)
 [ 237.335913] CPU: 3 PID: 1921 Comm: rmmod Tainted: GW   E   
4.7.0-rc4-wt-ath+ #1377
 [ 237.335916] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 68CDD 
Ver. F.04 01/27/2010
 [ 237.335918]  00200286 00200286 eff85dac c14151e2 f901574e  eff85de0 
c1081075
 [ 237.335928]  c1ab91f0 0003 0781 f901574e 0016 f8fbabad f8fbabad 
0016
 [ 237.335938]  eb24ff60  ef3886c0 eff85df4 c10810ba 0009  

 [ 237.335948] Call Trace:
 [ 237.335953]  [] dump_stack+0x76/0xb4
 [ 237.335957]  [] __warn+0xe5/0x100
 [ 237.336002]  [] ? fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336046]  [] ? fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336053]  [] warn_slowpath_null+0x2a/0x30
 [ 237.336095]  [] fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336137]  [] fq_flow_reset.constprop.56+0x2a/0x90 [mac80211]
 [ 237.336180]  [] fq_reset.constprop.59+0x2a/0x50 [mac80211]
 [ 237.336222]  [] ieee80211_txq_teardown_flows+0x38/0x40 [mac80211]
 [ 237.336258]  [] ieee80211_unregister_hw+0xe4/0x120 [mac80211]
 [ 237.336275]  [] ath10k_mac_unregister+0x16/0x50 [ath10k_core]
 [ 237.336292]  [] ath10k_core_unregister+0x3d/0x90 [ath10k_core]
 [ 237.336301]  [] ath10k_pci_remove+0x36/0xa0 [ath10k_pci]
 [ 237.336307]  [] pci_device_remove+0x38/0xb0
 ...

Fixes: 5caa328e3811 ("mac80211: implement codel on fair queuing flows")
Fixes: fa962b92120b ("mac80211: implement fair queueing per txq")
Tested-by: Kalle Valo <kv...@qca.qualcomm.com>
Reported-by: Kalle Valo <kv...@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v1:
 - added Tested-by [Kalle]
 - swapped first/last name in Reported-by to match Tested-by
 - improve commit log

 net/mac80211/tx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index fa8d38eb9236..91461c415525 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1449,7 +1449,9 @@ int ieee80211_txq_setup_flows(struct ieee80211_local 
*local)
local->cvars = kcalloc(fq->flows_cnt, sizeof(local->cvars[0]),
   GFP_KERNEL);
if (!local->cvars) {
+   spin_lock_bh(>lock);
fq_reset(fq, fq_skb_free_func);
+   spin_unlock_bh(>lock);
return -ENOMEM;
}
 
@@ -1469,7 +1471,9 @@ void ieee80211_txq_teardown_flows(struct ieee80211_local 
*local)
kfree(local->cvars);
local->cvars = NULL;
 
+   spin_lock_bh(>lock);
fq_reset(fq, fq_skb_free_func);
+   spin_unlock_bh(>lock);
 }
 
 struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
-- 
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] ath10k: disable wake_tx_queue for older devices

[Michal Kazior]

Ideally wake_tx_queue should be used regardless as
it is a requirement for reducing bufferbloat and
implementing airtime fairness in the future.

However some setups (typically low-end platforms
hosting QCA988X) suffer performance regressions
with the current wake_tx_queue implementation.
Therefore disable it unless it is really
beneficial with current codebase (which is when
firmware supports smart pull-push tx scheduling).

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v1:
 - improve commit log

 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/mac.c  | 24 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 3da18c9dbd7a..de580af2e5ef 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -667,6 +667,7 @@ struct ath10k_fw_components {
 struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
+   struct ieee80211_ops *ops;
struct device *dev;
u8 mac_addr[ETH_ALEN];
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index d4b7a168f7c0..958bc6c91aac 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7471,21 +7471,32 @@ static const struct ieee80211_channel 
ath10k_5ghz_channels[] = {
 struct ath10k *ath10k_mac_create(size_t priv_size)
 {
struct ieee80211_hw *hw;
+   struct ieee80211_ops *ops;
struct ath10k *ar;
 
-   hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, _ops);
-   if (!hw)
+   ops = kmemdup(_ops, sizeof(ath10k_ops), GFP_KERNEL);
+   if (!ops)
return NULL;
 
+   hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, ops);
+   if (!hw) {
+   kfree(ops);
+   return NULL;
+   }
+
ar = hw->priv;
ar->hw = hw;
+   ar->ops = ops;
 
return ar;
 }
 
 void ath10k_mac_destroy(struct ath10k *ar)
 {
+   struct ieee80211_ops *ops = ar->ops;
+
ieee80211_free_hw(ar->hw);
+   kfree(ops);
 }
 
 static const struct ieee80211_iface_limit ath10k_if_limits[] = {
@@ -7919,6 +7930,15 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_warn(ar, "failed to initialise DFS pattern 
detector\n");
}
 
+   /* Current wake_tx_queue implementation imposes a significant
+* performance penalty in some setups. The tx scheduling code needs
+* more work anyway so disable the wake_tx_queue unless firmware
+* supports the pull-push mechanism.
+*/
+   if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL,
+ ar->running_fw->fw_file.fw_features))
+   ar->ops->wake_tx_queue = NULL;
+
ret = ath_regd_init(>ath_common.regulatory, ar->hw->wiphy,
ath10k_reg_notifier);
if (ret) {
-- 
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 2/2] ath10k: Fix sending NULL/ Qos NULL data frames for QCA99X0 and later

[Michal Kazior]

On 27 June 2016 at 16:36, Mohammed Shafi Shajakhan
<moham...@codeaurora.org> wrote:
> Hi Michal,
>
> thanks for the review ..
>
> On Mon, Jun 27, 2016 at 11:27:27AM +0200, Michal Kazior wrote:
>> On 23 June 2016 at 18:40, Mohammed Shafi Shajakhan
>> <moham...@qti.qualcomm.com> wrote:
>> > From: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
>> >
>> > For chipsets like QCA99X0, IPQ4019 and later we are not getting proper
>> > NULL func status (always acked/successs !!) when hostapd does a
>> > PROBE_CLIENT via nullfunc frames when the station is powered off
>> > abruptly (inactive timer probes client via null func after the inactive
>> > time reaches beyond the threshold). Fix this by disabling the workaround
>> > (getting the ACK status of NULL func frames by sending via HTT mgmt-tx
>> >  path) introduced by the change ("ath10k: fix beacon loss handling ")
>> > for QCA99X0 and later chipsets. The normal tx path provides the proper
>> > ACK status for NULL data frames. As of now disable this workaround for
>> > chipsets QCA99X0 and later, once the 10.1 firmware is obselete we can
>> > completely get rid of this workaround for all the chipsets
>> >
>> > Signed-off-by: Tamizh chelvam <c_tr...@qti.qualcomm.com>
>> > Signed-off-by: Mohammed Shafi Shajakhan <moham...@qti.qualcomm.com>
>> > ---
>> >  drivers/net/wireless/ath/ath10k/core.c | 3 +++
>> >  drivers/net/wireless/ath/ath10k/core.h | 6 ++
>> >  drivers/net/wireless/ath/ath10k/mac.c  | 1 +
>> >  3 files changed, 10 insertions(+)
>> >
>> > diff --git a/drivers/net/wireless/ath/ath10k/core.c 
>> > b/drivers/net/wireless/ath/ath10k/core.c
>> > index 689d6ce..9978e4a 100644
>> > --- a/drivers/net/wireless/ath/ath10k/core.c
>> > +++ b/drivers/net/wireless/ath/ath10k/core.c
>> > @@ -181,6 +181,7 @@ static const struct ath10k_hw_params 
>> > ath10k_hw_params_list[] = {
>> > .board = QCA99X0_HW_2_0_BOARD_DATA_FILE,
>> > .board_size = QCA99X0_BOARD_DATA_SZ,
>> > .board_ext_size = QCA99X0_BOARD_EXT_DATA_SZ,
>> > +   .disable_null_func_workaround = true,
>>
>> Tx completion (bugs) are firmware specific, not hardware. This should
>> be expressed via features bits in ath10k FW API, no?
>>
>>
> [shafi] Are you suggesting me to introduce something like
> "ATH10K_FW_FEATURE_SUPPORTS_SKIP_CLOCK_INIT" ? Kalle any suggestions ?
>
> Also how about getting this workaround completely if Ben had fixed this in 
> his tree,
> will this affect older 10.2.4 ?

There's still 636.

We could probably get rid of this as long as:
 - ath10k can express the need to use Probe Requests for AP probing
(in client mode) and beacon loss handling purposes instead of NullFunc
to mac80211
 - everyone uses hostapd with disassoc_low_ack=1 with affected
firmware revisions
 - supplicant uses disassoc_low_ack=1 for p2p go
 - I have no idea about mesh/ibss but they might require some work as well

Otherwise you'll introduce regressions.


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

[RFT] mac80211: fix fq lockdep warnings

[Michal Kazior]

Some lockdep assertions were not fulfilled and
resulted in a kernel warning/call trace.

Existing code sequences should've guranteed safety
but it's always good to be extra careful.

The call trace could look like this:

 [ 237.335805] [ cut here ]
 [ 237.335852] WARNING: CPU: 3 PID: 1921 at include/net/fq_impl.h:22 
fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.335855] Modules linked in: ath10k_pci(E-) ath10k_core(E) ath(E) 
mac80211(E) cfg80211(E)
 [ 237.335913] CPU: 3 PID: 1921 Comm: rmmod Tainted: GW   E   
4.7.0-rc4-wt-ath+ #1377
 [ 237.335916] Hardware name: Hewlett-Packard HP ProBook 6540b/1722, BIOS 68CDD 
Ver. F.04 01/27/2010
 [ 237.335918]  00200286 00200286 eff85dac c14151e2 f901574e  eff85de0 
c1081075
 [ 237.335928]  c1ab91f0 0003 0781 f901574e 0016 f8fbabad f8fbabad 
0016
 [ 237.335938]  eb24ff60  ef3886c0 eff85df4 c10810ba 0009  

 [ 237.335948] Call Trace:
 [ 237.335953]  [] dump_stack+0x76/0xb4
 [ 237.335957]  [] __warn+0xe5/0x100
 [ 237.336002]  [] ? fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336046]  [] ? fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336053]  [] warn_slowpath_null+0x2a/0x30
 [ 237.336095]  [] fq_flow_dequeue+0xed/0x140 [mac80211]
 [ 237.336137]  [] fq_flow_reset.constprop.56+0x2a/0x90 [mac80211]
 [ 237.336180]  [] fq_reset.constprop.59+0x2a/0x50 [mac80211]
 [ 237.336222]  [] ieee80211_txq_teardown_flows+0x38/0x40 [mac80211]
 [ 237.336258]  [] ieee80211_unregister_hw+0xe4/0x120 [mac80211]
 [ 237.336275]  [] ath10k_mac_unregister+0x16/0x50 [ath10k_core]
 [ 237.336292]  [] ath10k_core_unregister+0x3d/0x90 [ath10k_core]
 [ 237.336301]  [] ath10k_pci_remove+0x36/0xa0 [ath10k_pci]
 [ 237.336307]  [] pci_device_remove+0x38/0xb0
 ...

Fixes: 5caa328e3811 ("mac80211: implement codel on fair queuing flows")
Fixes: fa962b92120b ("mac80211: implement fair queueing per txq")
Reported-by: Valo, Kalle <kv...@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Hi Kalle,

Can you verify this fixes call traces you're
seeing, please?


 net/mac80211/tx.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 44ec605a5682..1975c22132c6 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1445,7 +1445,9 @@ int ieee80211_txq_setup_flows(struct ieee80211_local 
*local)
local->cvars = kcalloc(fq->flows_cnt, sizeof(local->cvars[0]),
   GFP_KERNEL);
if (!local->cvars) {
+   spin_lock_bh(>lock);
fq_reset(fq, fq_skb_free_func);
+   spin_unlock_bh(>lock);
return -ENOMEM;
}
 
@@ -1465,7 +1467,9 @@ void ieee80211_txq_teardown_flows(struct ieee80211_local 
*local)
kfree(local->cvars);
local->cvars = NULL;
 
+   spin_lock_bh(>lock);
fq_reset(fq, fq_skb_free_func);
+   spin_unlock_bh(>lock);
 }
 
 struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
-- 
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 2/2] ath10k: Fix sending NULL/ Qos NULL data frames for QCA99X0 and later

[Michal Kazior]

On 23 June 2016 at 18:40, Mohammed Shafi Shajakhan
 wrote:
> From: Mohammed Shafi Shajakhan 
>
> For chipsets like QCA99X0, IPQ4019 and later we are not getting proper
> NULL func status (always acked/successs !!) when hostapd does a
> PROBE_CLIENT via nullfunc frames when the station is powered off
> abruptly (inactive timer probes client via null func after the inactive
> time reaches beyond the threshold). Fix this by disabling the workaround
> (getting the ACK status of NULL func frames by sending via HTT mgmt-tx
>  path) introduced by the change ("ath10k: fix beacon loss handling ")
> for QCA99X0 and later chipsets. The normal tx path provides the proper
> ACK status for NULL data frames. As of now disable this workaround for
> chipsets QCA99X0 and later, once the 10.1 firmware is obselete we can
> completely get rid of this workaround for all the chipsets
>
> Signed-off-by: Tamizh chelvam 
> Signed-off-by: Mohammed Shafi Shajakhan 
> ---
>  drivers/net/wireless/ath/ath10k/core.c | 3 +++
>  drivers/net/wireless/ath/ath10k/core.h | 6 ++
>  drivers/net/wireless/ath/ath10k/mac.c  | 1 +
>  3 files changed, 10 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c 
> b/drivers/net/wireless/ath/ath10k/core.c
> index 689d6ce..9978e4a 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -181,6 +181,7 @@ static const struct ath10k_hw_params 
> ath10k_hw_params_list[] = {
> .board = QCA99X0_HW_2_0_BOARD_DATA_FILE,
> .board_size = QCA99X0_BOARD_DATA_SZ,
> .board_ext_size = QCA99X0_BOARD_EXT_DATA_SZ,
> +   .disable_null_func_workaround = true,

Tx completion (bugs) are firmware specific, not hardware. This should
be expressed via features bits in ath10k FW API, no?


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: [PATCH] ath10k: fix potential null dereference bugs

[Michal Kazior]

On 10 June 2016 at 14:52, Bob Copeland  wrote:
> Smatch warns about a number of cases in ath10k where a pointer is
> null-checked after it has already been dereferenced, in code involving
> ath10k private virtual interface pointers.
>
> Fix these by making the dereference happen later.
>
> Addresses the following smatch warnings:
>
> drivers/net/wireless/ath/ath10k/mac.c:3651 ath10k_mac_txq_init() warn: 
> variable dereferenced before check 'txq' (see line 3649)
> drivers/net/wireless/ath/ath10k/mac.c:3664 ath10k_mac_txq_unref() warn: 
> variable dereferenced before check 'txq' (see line 3659)
> drivers/net/wireless/ath/ath10k/htt_tx.c:70 __ath10k_htt_tx_txq_recalc() 
> warn: variable dereferenced before check 'txq->sta' (see line 52)
> drivers/net/wireless/ath/ath10k/htt_tx.c:740 ath10k_htt_tx_get_vdev_id() 
> warn: variable dereferenced before check 'cb->vif' (see line 736)
> drivers/net/wireless/ath/ath10k/txrx.c:86 ath10k_txrx_tx_unref() warn: 
> variable dereferenced before check 'txq' (see line 84)
> drivers/net/wireless/ath/ath10k/wmi.c:1837 ath10k_wmi_op_gen_mgmt_tx() warn: 
> variable dereferenced before check 'cb->vif' (see line 1825)

FWIW all of these are false positives. I think this was already
pointed out some time ago. The drv_priv stuff is merely an offset (see
how ieee80211_vif and ieee80211_sta are defined) and the according
structure is always checked beforehand.


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] [RFC/RFT 5/5] ath9k: Count RX airtime in airtime deficit

[Michal Kazior]

On 10 June 2016 at 11:08, Toke Høiland-Jørgensen <t...@toke.dk> wrote:
> Michal Kazior <michal.kaz...@tieto.com> writes:
>
>> For A-MPDU all MPDU rx status (except last one) should share the same
>> timestamp. Last one has a different one so all you need is to
>> distinguish first and last MPDU. Non A-MPDU obviously are special case
>> (status bits are pricky).
>
> Right. So comparing the rs_stamp between first and last MPDU should give
> the duration of the entire thing?

Depends on how you define your "thing" :) I no, I don't know what
you'll actually measure. It should be reasonable either way.


> This would require keeping state
> between subsequent calls to the RX handler. Also, what happens if the
> last MPDU is lost?

No idea. If that's possible, then track last MPDU within PPDU, so you
can at least fallback to _something_ when you detect a new first
(A-)MPDU?

Or maybe it's impossible (i.e. not worth worrying) and HW always
reports last MPDU as far as status bits are concerned (regardless of
it being _actual_ last MPDU, i.e. it just says "ok, I'm done with this
PPDU").


>>> Is the entire A-MPDU received before the RX handler is called for the
>>> first frame?
>>
>> No idea. Maybe it is as there's distinction between "more" and
>> "moreaggr".
>
> Hmm. If it is, comparing the stamp of the first MPDU to the current time
> (when handling it) should give the needed duration? Will try doing that
> and see what the result is.

I'd say it's a little racy/inaccurate (and perhaps unreliable) to
compare any kind of global timer and compare it against your rx-status
descriptors.


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: ath10k/QCA9980 - Issues introduced in wireless testing 2016-05

[Michal Kazior]

On 10 June 2016 at 10:55, Felix Fietkau <n...@nbd.name> wrote:
> On 2016-06-10 10:50, Michal Kazior wrote:
>> On 9 June 2016 at 09:46, A. Benz <ash.b...@bk.ru> wrote:
>>> Dear All,
>>>
>>> I am using LEDE on my IPQ806x (QCA9980) system (Archer C2600).
>>> With compat-wireless-2016-05-12, I observed traces attached below.
>>> The router is unstable and eventually reboots by itself (randomly).
>>>
>>> Upon reverting to compat-wireless-2016-01, the issue disappears. Nothing
>>> else is changed (software-wise or hardware).
>>> This was confirmed with other users.
>>>
>>> A new compile with the fixes below:
>>> https://git.lede-project.org/?p=lede/nbd/staging.git;a=commit;h=858e26f3c0fc11231f25497cbb2ddca1e5f101e0
>>>
>>> Did not solve the problem.
>>>
>>> Please let me know if I need to provide any further information.
>>>
>>> [ cut here ]
>>> WARNING: CPU: 0 PID: 558 at
>>> compat-wireless-2016-05-12/net/mac80211/rx.c:4068
>>> ieee80211_rx_napi+0x8c/0x8a4 [mac80211]()
>>
>> Can you post what is at rx.c line 4068 (and +/- 3 lines), please?
> It's early in ieee80211_rx_napi:
>
> sband = local->hw.wiphy->bands[status->band];
> if (WARN_ON(!sband))
> goto drop;

Thanks.


> I could not easily find a scenario under which status->band would not be
> set properly by the driver, so my guess is there is some nasty memory
> corruption going on.

Hmm.. could it be related to ath10k not fulfilling (some) NAPI's
locking requirements and thus ending up with, e.g. linked-list mayhem?


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] [RFC/RFT 5/5] ath9k: Count RX airtime in airtime deficit

[Michal Kazior]

On 10 June 2016 at 10:53, Toke Høiland-Jørgensen  wrote:
>
>>> I initially thought that using the timestamp put into the frame by the
>>> hardware could be a way to get timing. But there's only a timestamp of
>>> the first symbol in rs_tstamp, and getting a time to compare it with is
>>> difficult; by the time the frame is handled in the rx tasklet, way too
>>> much time has been spent on interrupt handling etc for the current time
>>> to be worth comparing with.
>
> As an aside, I'm no longer sure this explanation for why I got wrong
> timings for this way of measuring RX time is correct. It may simply be
> that I was counting the same time interval more than once because I
> didn't realise that the handler would be called for each frame. See
> below.
>
>> I think rs_tstamp in rx-status is different for first MPDU and last
>> MPDU in an A-MPDU meaning you should be able to compute the entire
>> duration (if you track it, and this should be fairly straightforward
>> as you can't really rx interleaved MPDUs from different
>> A-MPDUs/stations). I'm not sure if the last MPDU defines the tstamp of
>> first symbol or last one.
>
> I actually went down this path again last night, but haven't had a
> chance to test it yet.
>
> So what I have now is this:
>
>/* Only count airtime for last frame in an aggregate. FIXME: Should
> * this be only the first frame? */
>if (!rs->rs_isaggr || !rs->rs_moreaggr)
>airtime = (tsf & 0x) - rs->rs_tstamp;
>
> Which was under the assumption that rs_tstamp will be the time of the
> first symbol *of the whole ampdu* for all the frames in that aggregate.
> However, if rs_tstamp differs between frames, tracking it at the first
> frame might be the right things to do?

For A-MPDU all MPDU rx status (except last one) should share the same
timestamp. Last one has a different one so all you need is to
distinguish first and last MPDU. Non A-MPDU obviously are special case
(status bits are pricky).


> Is the entire A-MPDU received before the RX handler is called for the
> first frame?

No idea. Maybe it is as there's distinction between "more" and "moreaggr".


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: ath10k/QCA9980 - Issues introduced in wireless testing 2016-05

[Michal Kazior]

On 9 June 2016 at 09:46, A. Benz  wrote:
> Dear All,
>
> I am using LEDE on my IPQ806x (QCA9980) system (Archer C2600).
> With compat-wireless-2016-05-12, I observed traces attached below.
> The router is unstable and eventually reboots by itself (randomly).
>
> Upon reverting to compat-wireless-2016-01, the issue disappears. Nothing
> else is changed (software-wise or hardware).
> This was confirmed with other users.
>
> A new compile with the fixes below:
> https://git.lede-project.org/?p=lede/nbd/staging.git;a=commit;h=858e26f3c0fc11231f25497cbb2ddca1e5f101e0
>
> Did not solve the problem.
>
> Please let me know if I need to provide any further information.
>
> [ cut here ]
> WARNING: CPU: 0 PID: 558 at
> compat-wireless-2016-05-12/net/mac80211/rx.c:4068
> ieee80211_rx_napi+0x8c/0x8a4 [mac80211]()

Can you post what is at rx.c line 4068 (and +/- 3 lines), please?


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] [RFC/RFT 5/5] ath9k: Count RX airtime in airtime deficit

[Michal Kazior]

On 7 June 2016 at 13:12, Toke Høiland-Jørgensen  wrote:
> Toke Høiland-Jørgensen  writes:
>
>>> [snip]
>>>
>>> I also found one of my notes in my version of this - how can we
>>> estimate the duration of an A-MPDU, when we only get hardware
>>> de-encapsulated frames?
>>
>> Well in my case I'm sidestepping this by getting the TX duration from
>> a register in the hardware. There seems to be registers containing the
>> duration spent on each step in the retry chain; I simply sum these.
>
> Ah, but you're still talking RX? Hmm, I'm using ath_pkt_duration() to
> compute the RX time, which does take into account MIMO (I think) but
> expects the size to include padding. Which is probably not included in
> the rs_datalen field of struct ath_rx_status that I'm using.
>
> So yeah, how to account for that?
>
> I initially thought that using the timestamp put into the frame by the
> hardware could be a way to get timing. But there's only a timestamp of
> the first symbol in rs_tstamp, and getting a time to compare it with is
> difficult; by the time the frame is handled in the rx tasklet, way too
> much time has been spent on interrupt handling etc for the current time
> to be worth comparing with.

I think rs_tstamp in rx-status is different for first MPDU and last
MPDU in an A-MPDU meaning you should be able to compute the entire
duration (if you track it, and this should be fairly straightforward
as you can't really rx interleaved MPDUs from different
A-MPDUs/stations). I'm not sure if the last MPDU defines the tstamp of
first symbol or last one.


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: Any reason to allow MGT TID to be paused by power-save?

[Michal Kazior]

On 10 June 2016 at 01:45, Ben Greear  wrote:
> I'm trying to track down a tricky problem with a Mac book and power save
> with my ath10k firmware.
>
> At least part of the issue is that when the Mac sleeps, it goes into
> power-save
> state as far as AP is concerned, and then it simply turns itself off.  When
> it wakes
> up a short time later, it then tries to scan and re-associate.  But, if
> hostapd
> hasn't timed out the station (due to longer idle timer), then STA still is
> treated
> as being in power-save in the firmware.  It appears that this causes
> mgt-frames (probe responses)
> not be sent back out of the firmware, so Mac gives up fairly quickly.
>
> I fixed at least some of this by forcing a power-save wake on
> receipt of probe requests and auth requests, but some issues remain
> for one reason or another.
>
> One thing that came to mind:  Is it *ever* a good idea to pause a mgt-tid
> due to
> power-save?

The spec defines some mgmt frames as bufferable (IEEE 802.11-2012
8.2.4.1.7). See ieee80211_is_bufferable_mmpdu() in
include/linux/ieee80211.h.

Action, deauth and disassoc are bufferable. It makes sense anyway.

E.g. On DFS channel you may opt for CSA action frame (instead of CSA
IE in beacons) in which case you do want to buffer frames to clients
that are asleep to give them a chance to notice it, wake up and get
it.


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] [PATCHv5 0/5] mac80211: implement fq_codel

[Michal Kazior]

On 31 May 2016 at 14:12, Toke Høiland-Jørgensen <t...@toke.dk> wrote:
> Michal Kazior <michal.kaz...@tieto.com> writes:
>
>> This patchset disables qdiscs for drivers
>> using software queuing and performs fq_codel-like
>> dequeuing on txqs.
>
> Hi Michal
>
> Is this version in a git repo somewhere I can pull from? :)

Just pushed, enjoy!

  https://github.com/kazikcz/linux/tree/fqmac-v5


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] [RFC] ath9k: Measure per-station airtime usage

[Michal Kazior]

On 26 May 2016 at 15:50, Toke Høiland-Jørgensen  wrote:
> This is my attempt to add per-station airtime usage accounting to ath9k.
> For now I just export it to a new debugfs entry, but my plan is to use
> it to make (station) scheduling decisions. However, before attempting
> that I would like some feedback from someone more familiar with the
> ath9k than me as to whether this way of measuring airtime usage is
> likely to give reasonable results.
>
> I realise that there's probably some things I'm missing, but an initial
> test run indicates that the values are at least in the right ballpark (I
> get a total of ~170k usecs of measured airtime per 200 ms sampling
> interval when running three simultaneous TCP streams to three different
> stations).
>
> So can anyone comment on whether I'm on the right track here? And
> possibly provide some more detail as to what I'm missing and how to
> remedy that?
[...]
>
> +void ath_debug_tx_airtime(struct ath_softc *sc,
> + struct ath_node *an,
> + struct ath_tx_status *ts)
> +{
> +   struct ath_airtime_stats *astats;
> +
> +   rcu_read_lock();
> +
> +   astats = >airtime_stats;
> +   astats->tx_airtime += ts->duration;

I'm not ath9k expert but this seems to be oblivious to tx retries. The
ts->duration is acquired from the last used tx rate for given frame.
Or am I missing something?

I think you should use ts->ts_rateindex and ts->ts_longretry to factor
in retries (see ath_tx_rc_status).


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

[PATCHv5 5/5] mac80211: add debug knobs for codel

[Michal Kazior]

This adds a few debugfs entries to make it easier
to test, debug and experiment.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v5:
 * use the single "aqm" debugfs knob [Dave]

v4:
 * stats adjustments (in-kernel codel has more of them)

 net/mac80211/debugfs.c | 33 +++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 2906c1004e1a..53a315401a4b 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -126,13 +126,31 @@ static int aqm_open(struct inode *inode, struct file 
*file)
 "R fq_overlimit %u\n"
 "R fq_collisions %u\n"
 "RW fq_limit %u\n"
-"RW fq_quantum %u\n",
+"RW fq_quantum %u\n"
+"R codel_maxpacket %u\n"
+"R codel_drop_count %u\n"
+"R codel_drop_len %u\n"
+"R codel_ecn_mark %u\n"
+"R codel_ce_mark %u\n"
+"RW codel_interval %u\n"
+"RW codel_target %u\n"
+"RW codel_mtu %u\n"
+"RW codel_ecn %u\n",
 fq->flows_cnt,
 fq->backlog,
 fq->overlimit,
 fq->collisions,
 fq->limit,
-fq->quantum);
+fq->quantum,
+local->cstats.maxpacket,
+local->cstats.drop_count,
+local->cstats.drop_len,
+local->cstats.ecn_mark,
+local->cstats.ce_mark,
+local->cparams.interval,
+local->cparams.target,
+local->cparams.mtu,
+local->cparams.ecn ? 1U : 0U);
 
len += scnprintf(info->buf + len,
 info->size - len,
@@ -214,6 +232,7 @@ static ssize_t aqm_write(struct file *file,
struct ieee80211_local *local = info->local;
char buf[100];
size_t len;
+   unsigned int ecn;
 
if (count > sizeof(buf))
return -EINVAL;
@@ -230,6 +249,16 @@ static ssize_t aqm_write(struct file *file,
return count;
else if (sscanf(buf, "fq_quantum %u", >fq.quantum) == 1)
return count;
+   else if (sscanf(buf, "codel_interval %u", >cparams.interval) == 
1)
+   return count;
+   else if (sscanf(buf, "codel_target %u", >cparams.target) == 1)
+   return count;
+   else if (sscanf(buf, "codel_mtu %u", >cparams.mtu) == 1)
+   return count;
+   else if (sscanf(buf, "codel_ecn %u", ) == 1) {
+   local->cparams.ecn = !!ecn;
+   return count;
+   }
 
return -EINVAL;
 }
-- 
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

[PATCHv5 3/5] mac80211: add debug knobs for fair queuing

[Michal Kazior]

This adds a debugfs entry to read and modify some
fq parameters and inroduces a module parameter to
control number of flows mac80211 shuold maintain.

This makes it easy to debug, test and experiment.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v5:
 * expose a single "aqm" debugfs knob to maintain
   coherent stat values [Dave]

 net/mac80211/debugfs.c | 173 +
 net/mac80211/tx.c  |   8 ++-
 2 files changed, 180 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index b251b2f7f8dd..2906c1004e1a 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include "ieee80211_i.h"
 #include "driver-ops.h"
 #include "rate.h"
@@ -70,6 +71,177 @@ DEBUGFS_READONLY_FILE(wep_iv, "%#08x",
 DEBUGFS_READONLY_FILE(rate_ctrl_alg, "%s",
local->rate_ctrl ? local->rate_ctrl->ops->name : "hw/driver");
 
+struct aqm_info {
+   struct ieee80211_local *local;
+   size_t size;
+   size_t len;
+   unsigned char buf[0];
+};
+
+#define AQM_HDR_LEN 200
+#define AQM_HW_ENTRY_LEN 40
+#define AQM_TXQ_ENTRY_LEN 110
+
+static int aqm_open(struct inode *inode, struct file *file)
+{
+   struct ieee80211_local *local = inode->i_private;
+   struct ieee80211_sub_if_data *sdata;
+   struct sta_info *sta;
+   struct txq_info *txqi;
+   struct fq *fq = >fq;
+   struct aqm_info *info = NULL;
+   int len = 0;
+   int i;
+
+   if (!local->ops->wake_tx_queue)
+   return -EOPNOTSUPP;
+
+   len += AQM_HDR_LEN;
+   len += 6 * AQM_HW_ENTRY_LEN;
+
+   rcu_read_lock();
+   list_for_each_entry_rcu(sdata, >interfaces, list)
+   len += AQM_TXQ_ENTRY_LEN;
+   list_for_each_entry_rcu(sta, >sta_list, list)
+   len += AQM_TXQ_ENTRY_LEN * ARRAY_SIZE(sta->sta.txq);
+   rcu_read_unlock();
+
+   info = vmalloc(len);
+   if (!info)
+   return -ENOMEM;
+
+   spin_lock_bh(>fq.lock);
+   rcu_read_lock();
+
+   file->private_data = info;
+   info->local = local;
+   info->size = len;
+   len = 0;
+
+   len += scnprintf(info->buf + len, info->size - len,
+"* hw\n"
+"access name value\n"
+"R fq_flows_cnt %u\n"
+"R fq_backlog %u\n"
+"R fq_overlimit %u\n"
+"R fq_collisions %u\n"
+"RW fq_limit %u\n"
+"RW fq_quantum %u\n",
+fq->flows_cnt,
+fq->backlog,
+fq->overlimit,
+fq->collisions,
+fq->limit,
+fq->quantum);
+
+   len += scnprintf(info->buf + len,
+info->size - len,
+"* vif\n"
+"ifname addr ac backlog-bytes backlog-packets flows 
overlimit collisions tx-bytes tx-packets\n");
+
+   list_for_each_entry_rcu(sdata, >interfaces, list) {
+   txqi = to_txq_info(sdata->vif.txq);
+   len += scnprintf(info->buf + len, info->size - len,
+"%s %pM %u %u %u %u %u %u %u %u\n",
+sdata->name,
+sdata->vif.addr,
+txqi->txq.ac,
+txqi->tin.backlog_bytes,
+txqi->tin.backlog_packets,
+txqi->tin.flows,
+txqi->tin.overlimit,
+txqi->tin.collisions,
+txqi->tin.tx_bytes,
+txqi->tin.tx_packets);
+   }
+
+   len += scnprintf(info->buf + len,
+info->size - len,
+"* sta\n"
+"ifname addr tid ac backlog-bytes backlog-packets 
flows overlimit collisions tx-bytes tx-packets\n");
+
+   list_for_each_entry_rcu(sta, >sta_list, list) {
+   sdata = sta->sdata;
+   for (i = 0; i < ARRAY_SIZE(sta->sta.txq); i++) {
+   txqi = to_txq_info(sta->sta.txq[i]);
+   len += scnprintf(info->buf + len, info->size - len,
+"%s %pM %d %d %u %u %u %u %u %u %u\n",
+sdata->name,
+

[PATCHv5 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

There is no other limit other than a global
packet count limit when using software queuing.
This means a single flow queue can grow insanely
long. This is particularly bad for TCP congestion
algorithms which requires a little more
sophisticated frame dropping scheme than a mere
headdrop on limit overflow.

Hence apply (a slighly modified, to fit the knobs)
CoDel5 on flow queues. This improves TCP
convergence and stability when combined with
wireless driver which keeps its own tx queue/fifo
at a minimum fill level for given link conditions.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * removed internal codel.h and re-used in-kernel one

 include/net/mac80211.h |  14 +-
 net/mac80211/ieee80211_i.h |   5 +++
 net/mac80211/tx.c  | 109 -
 3 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index a8683aec6dbe..a52009ffc19f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /**
@@ -895,7 +896,18 @@ struct ieee80211_tx_info {
unsigned long jiffies;
};
/* NB: vif can be NULL for injected frames */
-   struct ieee80211_vif *vif;
+   union {
+   /* NB: vif can be NULL for injected frames */
+   struct ieee80211_vif *vif;
+
+   /* When packets are enqueued on txq it's easy
+* to re-construct the vif pointer. There's no
+* more space in tx_info so it can be used to
+* store the necessary enqueue time for packet
+* sojourn time computation.
+*/
+   codel_time_t enqueue_time;
+   };
struct ieee80211_key_conf *hw_key;
u32 flags;
/* 4 bytes free */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 6f8375f1df88..54edfb6fc1d1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -812,10 +812,12 @@ enum txq_info_flags {
  * @tin: contains packets split into multiple flows
  * @def_flow: used as a fallback flow when a packet destined to @tin hashes to
  * a fq_flow which is already owned by a different tin
+ * @def_cvars: codel vars for @def_flow
  */
 struct txq_info {
struct fq_tin tin;
struct fq_flow def_flow;
+   struct codel_vars def_cvars;
unsigned long flags;
 
/* keep last! */
@@ -1108,6 +1110,9 @@ struct ieee80211_local {
struct ieee80211_hw hw;
 
struct fq fq;
+   struct codel_vars *cvars;
+   struct codel_params cparams;
+   struct codel_stats cstats;
 
const struct ieee80211_ops *ops;
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2b60b10e6990..a7c9b6704ffb 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 
@@ -1273,11 +1275,92 @@ static struct txq_info *ieee80211_get_txq(struct 
ieee80211_local *local,
return to_txq_info(txq);
 }
 
+static void ieee80211_set_skb_enqueue_time(struct sk_buff *skb)
+{
+   IEEE80211_SKB_CB(skb)->control.enqueue_time = codel_get_time();
+}
+
+static void ieee80211_set_skb_vif(struct sk_buff *skb, struct txq_info *txqi)
+{
+   IEEE80211_SKB_CB(skb)->control.vif = txqi->txq.vif;
+}
+
+static u32 codel_skb_len_func(const struct sk_buff *skb)
+{
+   return skb->len;
+}
+
+static codel_time_t codel_skb_time_func(const struct sk_buff *skb)
+{
+   const struct ieee80211_tx_info *info;
+
+   info = (const struct ieee80211_tx_info *)skb->cb;
+   return info->control.enqueue_time;
+}
+
+static struct sk_buff *codel_dequeue_func(struct codel_vars *cvars,
+ void *ctx)
+{
+   struct ieee80211_local *local;
+   struct txq_info *txqi;
+   struct fq *fq;
+   struct fq_flow *flow;
+
+   txqi = ctx;
+   local = vif_to_sdata(txqi->txq.vif)->local;
+   fq = >fq;
+
+   if (cvars == >def_cvars)
+   flow = >def_flow;
+   else
+   flow = >flows[cvars - local->cvars];
+
+   return fq_flow_dequeue(fq, flow);
+}
+
+static void codel_drop_func(struct sk_buff *skb,
+   void *ctx)
+{
+   struct ieee80211_local *local;
+   struct ieee80211_hw *hw;
+   struct txq_info *txqi;
+
+   txqi = ctx;
+   local = vif_to_sdata(txqi->txq.vif)->local;
+   hw = >hw;
+
+   ieee80211_free_txskb(hw, skb);
+}
+
 st

[PATCHv5 2/5] mac80211: implement fair queueing per txq

[Michal Kazior]

mac80211's software queues were designed to work
very closely with device tx queues. They are
required to make use of 802.11 packet aggregation
easily and efficiently.

Due to the way 802.11 aggregation is designed it
only makes sense to keep fair queuing as close to
hardware as possible to reduce induced latency and
inertia and provide the best flow responsivness.

This change doesn't translate directly to
immediate and significant gains. End result
depends on driver's induced latency. Best results
can be achieved if driver keeps its own tx
queue/fifo fill level to a minimum.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * removed internal fq.h and re-used in-kernel one

 net/mac80211/agg-tx.c  |   8 ++-
 net/mac80211/ieee80211_i.h |  24 ++--
 net/mac80211/iface.c   |  12 ++--
 net/mac80211/main.c|   7 +++
 net/mac80211/rx.c  |   2 +-
 net/mac80211/sta_info.c|  14 ++---
 net/mac80211/tx.c  | 136 ++---
 net/mac80211/util.c|  23 +---
 8 files changed, 162 insertions(+), 64 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 42fa81031dfa..5650c46bf91a 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -194,17 +194,21 @@ static void
 ieee80211_agg_stop_txq(struct sta_info *sta, int tid)
 {
struct ieee80211_txq *txq = sta->sta.txq[tid];
+   struct ieee80211_sub_if_data *sdata;
+   struct fq *fq;
struct txq_info *txqi;
 
if (!txq)
return;
 
txqi = to_txq_info(txq);
+   sdata = vif_to_sdata(txq->vif);
+   fq = >local->fq;
 
/* Lock here to protect against further seqno updates on dequeue */
-   spin_lock_bh(>queue.lock);
+   spin_lock_bh(>lock);
set_bit(IEEE80211_TXQ_STOP, >flags);
-   spin_unlock_bh(>queue.lock);
+   spin_unlock_bh(>lock);
 }
 
 static void
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 634603320374..6f8375f1df88 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "key.h"
 #include "sta_info.h"
 #include "debug.h"
@@ -805,10 +806,17 @@ enum txq_info_flags {
IEEE80211_TXQ_NO_AMSDU,
 };
 
+/**
+ * struct txq_info - per tid queue
+ *
+ * @tin: contains packets split into multiple flows
+ * @def_flow: used as a fallback flow when a packet destined to @tin hashes to
+ * a fq_flow which is already owned by a different tin
+ */
 struct txq_info {
-   struct sk_buff_head queue;
+   struct fq_tin tin;
+   struct fq_flow def_flow;
unsigned long flags;
-   unsigned long byte_cnt;
 
/* keep last! */
struct ieee80211_txq txq;
@@ -1099,6 +1107,8 @@ struct ieee80211_local {
 * it first anyway so they become a no-op */
struct ieee80211_hw hw;
 
+   struct fq fq;
+
const struct ieee80211_ops *ops;
 
/*
@@ -1931,9 +1941,13 @@ static inline bool ieee80211_can_run_worker(struct 
ieee80211_local *local)
return true;
 }
 
-void ieee80211_init_tx_queue(struct ieee80211_sub_if_data *sdata,
-struct sta_info *sta,
-struct txq_info *txq, int tid);
+int ieee80211_txq_setup_flows(struct ieee80211_local *local);
+void ieee80211_txq_teardown_flows(struct ieee80211_local *local);
+void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
+   struct sta_info *sta,
+   struct txq_info *txq, int tid);
+void ieee80211_txq_purge(struct ieee80211_local *local,
+struct txq_info *txqi);
 void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 u16 transaction, u16 auth_alg, u16 status,
 const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 609c5174d798..b123a9e325b3 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -779,6 +779,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
  bool going_down)
 {
struct ieee80211_local *local = sdata->local;
+   struct fq *fq = >fq;
unsigned long flags;
struct sk_buff *skb, *tmp;
u32 hw_reconf_flags = 0;
@@ -976,13 +977,10 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
 
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
-   int n = skb_queue_len(>queue);
 
-   spin_lock_bh(>queue.lock);
-   ieee80211_purge_tx_queue(>hw, >queue);
-   atomic_sub(n, >num_tx_queued);
-   txqi->byte_cnt = 0;
-   spin_unlock_bh(>queue.lock);
+   spin_lock_b

[PATCHv5 0/5] mac80211: implement fq_codel

[Michal Kazior]

Hi,

This patchset disables qdiscs for drivers
using software queuing and performs fq_codel-like
dequeuing on txqs.

This is based on net-next/master
(0b7962a6c4a37ef3cbb25d976af7b9ec4ce8ad01).

Background:

  https://www.spinics.net/lists/linux-wireless/msg149776.html
  https://www.spinics.net/lists/linux-wireless/msg148714.html
  https://www.spinics.net/lists/linux-wireless/msg149039.html
  http://blog.cerowrt.org/post/dql_on_wifi_2/
  http://blog.cerowrt.org/post/dql_on_wifi/
  http://blog.cerowrt.org/post/fq_codel_on_ath10k/

v5:
 * some fixes (crash, powersave) [me, Tim]
 * reworked debugfs knob (single file now) [Dave]

v4:
 * the taildrop stop-gap patch moved to
   per-interface limit (instead of per-radio) [Johannes]
 * pushed fq.h and codel.h changes to include/net/ [Johannes]

v3:
 * split taildrop, fq and codel functionalities
   into separate patches [Avery]

v2:
 * fix invalid ptr deref
 * fix compilation for backports


Michal Kazior (5):
  mac80211: skip netdev queue control with software queuing
  mac80211: implement fair queueing per txq
  mac80211: add debug knobs for fair queuing
  mac80211: implement codel on fair queuing flows
  mac80211: add debug knobs for codel

 include/net/mac80211.h |  18 ++-
 net/mac80211/agg-tx.c  |   8 +-
 net/mac80211/debugfs.c | 202 ++
 net/mac80211/ieee80211_i.h |  31 -
 net/mac80211/iface.c   |  26 ++--
 net/mac80211/main.c|  10 +-
 net/mac80211/rx.c  |   2 +-
 net/mac80211/sta_info.c|  14 +--
 net/mac80211/tx.c  | 298 +++--
 net/mac80211/util.c|  34 ++
 10 files changed, 545 insertions(+), 98 deletions(-)

-- 
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

[PATCHv5 1/5] mac80211: skip netdev queue control with software queuing

[Michal Kazior]

Qdiscs are designed with no regard to 802.11
aggregation requirements and hand out
packet-by-packet with no guarantee they are
destined to the same tid. This does more bad than
good no matter how fairly a given qdisc may behave
on an ethernet interface.

Software queuing used per-AC netdev subqueue
congestion control whenever a global AC limit was
hit. This meant in practice a single station or
tid queue could starve others rather easily. This
could resonate with qdiscs in a bad way or could
just end up with poor aggregation performance.
Increasing the AC limit would increase induced
latency which is also bad.

Disabling qdiscs by default and performing
taildrop instead of netdev subqueue congestion
control on the other hand makes it possible for
tid queues to fill up "in the meantime" while
preventing stations starving each other.

This increases aggregation opportunities and
should allow software queuing based drivers
achieve better performance by utilizing airtime
more efficiently with big aggregates.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v5:
 * fix null-deref for non-txq drivers
 * fix compilation after rebase
 * fix powersave on ath9k w/ wake_tx_queue
   [reported by Tim]

v4:
 * make queue depth limit per interface instead of
   per radio [Johannes]

 include/net/mac80211.h |  4 ---
 net/mac80211/ieee80211_i.h |  2 +-
 net/mac80211/iface.c   | 18 +--
 net/mac80211/main.c|  3 --
 net/mac80211/sta_info.c|  2 +-
 net/mac80211/tx.c  | 77 ++
 net/mac80211/util.c| 11 ---
 7 files changed, 67 insertions(+), 50 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index be30b0549b88..a8683aec6dbe 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2147,9 +2147,6 @@ enum ieee80211_hw_flags {
  * @n_cipher_schemes: a size of an array of cipher schemes definitions.
  * @cipher_schemes: a pointer to an array of cipher scheme definitions
  * supported by HW.
- *
- * @txq_ac_max_pending: maximum number of frames per AC pending in all txq
- * entries for a vif.
  */
 struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2180,7 +2177,6 @@ struct ieee80211_hw {
u8 uapsd_max_sp_len;
u8 n_cipher_schemes;
const struct ieee80211_cipher_scheme *cipher_schemes;
-   int txq_ac_max_pending;
 };
 
 static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9438c9406687..634603320374 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -856,7 +856,7 @@ struct ieee80211_sub_if_data {
bool control_port_no_encrypt;
int encrypt_headroom;
 
-   atomic_t txqs_len[IEEE80211_NUM_ACS];
+   atomic_t num_tx_queued;
struct ieee80211_tx_queue_params tx_conf[IEEE80211_NUM_ACS];
struct mac80211_qos_map __rcu *qos_map;
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index c59af3eb9fa4..609c5174d798 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -976,13 +976,13 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
 
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
+   int n = skb_queue_len(>queue);
 
spin_lock_bh(>queue.lock);
ieee80211_purge_tx_queue(>hw, >queue);
+   atomic_sub(n, >num_tx_queued);
txqi->byte_cnt = 0;
spin_unlock_bh(>queue.lock);
-
-   atomic_set(>txqs_len[txqi->txq.ac], 0);
}
 
if (local->open_count == 0)
@@ -1198,6 +1198,12 @@ static void ieee80211_if_setup(struct net_device *dev)
dev->destructor = ieee80211_if_free;
 }
 
+static void ieee80211_if_setup_no_queue(struct net_device *dev)
+{
+   ieee80211_if_setup(dev);
+   dev->priv_flags |= IFF_NO_QUEUE;
+}
+
 static void ieee80211_iface_work(struct work_struct *work)
 {
struct ieee80211_sub_if_data *sdata =
@@ -1707,6 +1713,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const 
char *name,
struct net_device *ndev = NULL;
struct ieee80211_sub_if_data *sdata = NULL;
struct txq_info *txqi;
+   void (*if_setup)(struct net_device *dev);
int ret, i;
int txqs = 1;
 
@@ -1734,12 +1741,17 @@ int ieee80211_if_add(struct ieee80211_local *local, 
const char *name,
txq_size += sizeof(struct txq_info) +
local->hw.txq_data_size;
 
+   if (local->ops->wake_tx_queue)
+   if_setup = ieee80211_if_setup_no_queue;
+   else
+   if_setup = ieee80211_if_setup;
+
if (local->hw.queues >= IEEE80211_NUM_ACS)
   

[RFC/RFT] ath10k: disable wake_tx_queue for older devices

[Michal Kazior]

Some setups suffer performance regressions with
current wake_tx_queue implementation.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
Hi Roman,

Can you give this patch a try and see if it helps
with your performance problems, please?


 drivers/net/wireless/ath/ath10k/core.h |  1 +
 drivers/net/wireless/ath/ath10k/mac.c  | 24 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 1379054000f9..44e7c2cd2e9b 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -657,6 +657,7 @@ struct ath10k_fw_components {
 struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
+   struct ieee80211_ops *ops;
struct device *dev;
u8 mac_addr[ETH_ALEN];
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 0e24f9ee8bff..5fa3888afa41 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7396,21 +7396,32 @@ static const struct ieee80211_channel 
ath10k_5ghz_channels[] = {
 struct ath10k *ath10k_mac_create(size_t priv_size)
 {
struct ieee80211_hw *hw;
+   struct ieee80211_ops *ops;
struct ath10k *ar;
 
-   hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, _ops);
-   if (!hw)
+   ops = kmemdup(_ops, sizeof(ath10k_ops), GFP_KERNEL);
+   if (!ops)
return NULL;
 
+   hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, ops);
+   if (!hw) {
+   kfree(ops);
+   return NULL;
+   }
+
ar = hw->priv;
ar->hw = hw;
+   ar->ops = ops;
 
return ar;
 }
 
 void ath10k_mac_destroy(struct ath10k *ar)
 {
+   struct ieee80211_ops *ops = ar->ops;
+
ieee80211_free_hw(ar->hw);
+   kfree(ops);
 }
 
 static const struct ieee80211_iface_limit ath10k_if_limits[] = {
@@ -7838,6 +7849,15 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_warn(ar, "failed to initialise DFS pattern 
detector\n");
}
 
+   /* Current wake_tx_queue implementation imposes a significant
+* performance penalty in some setups. The tx scheduling code needs
+* more work anyway so disable the wake_tx_queue unless firmware
+* supports the pull-push mechanism.
+*/
+   if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL,
+ ar->running_fw->fw_file.fw_features))
+   ar->ops->wake_tx_queue = NULL;
+
ret = ath_regd_init(>ath_common.regulatory, ar->hw->wiphy,
ath10k_reg_notifier);
if (ret) {
-- 
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 03/21] ath10k: Support setting debug mask from driver code.

[Michal Kazior]

On 10 May 2016 at 01:10,   wrote:
> From: Ben Greear 
>
> Might want to turn off verbose debug as soon as you
> see a firmware crash, for instance.  Helps keep dmesg
> output from over-running the stuff you care about.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/debug.c | 5 +
>  drivers/net/wireless/ath/ath10k/debug.h | 1 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
> b/drivers/net/wireless/ath/ath10k/debug.c
> index e251155..a689bf1 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -194,6 +194,11 @@ void ath10k_print_driver_info(struct ath10k *ar)
>  }
>  EXPORT_SYMBOL(ath10k_print_driver_info);
>
> +void ath10k_set_debug_mask(unsigned int v) {

The { should be on new line.


> +   ath10k_debug_mask = v;
> +}
> +EXPORT_SYMBOL(ath10k_set_debug_mask);

I didn't see any uses of this in your patchset (it's commented out in
21/21) and I'm not fully convinced it's a good idea to override
debug_mask like that. Once I set a debug_mask I expect it to stay
unchanged. What if I do want to trace what happens after fw crash?

Wouldn't it be better to have a knob to tell ath10k whether hw
recovery should be automatic or manual?


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: [PATCH 13/21] ath10k: Document cycle count related counters.

[Michal Kazior]

On 10 May 2016 at 01:11,   wrote:
> From: Ben Greear 
>
> They are not necessarily named in an intuitive manner,
> so at least add some comments to help the next person.
>
> Signed-off-by: Ben Greear 
> ---
>  drivers/net/wireless/ath/ath10k/core.h | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h 
> b/drivers/net/wireless/ath/ath10k/core.h
> index e7c228a..c4f649f 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -189,10 +189,10 @@ struct ath10k_fw_stats_pdev {
>
> /* PDEV stats */
> s32 ch_noise_floor;
> -   u32 tx_frame_count;
> -   u32 rx_frame_count;
> -   u32 rx_clear_count;
> -   u32 cycle_count;
> +   u32 tx_frame_count; /* cycles spent transmitting frames */
> +   u32 rx_frame_count; /* cycles spent receiving frames */
> +   u32 rx_clear_count; /* Total channel busy time, evidently */
> +   u32 cycle_count; /* Total on-channel time */

Hmm, there are also other instances of these vars in wmi.h. Although
redundant maybe it's worth to comment them as well (or first and
foremost they should be the ones that need a comment as they are the
"source"). Just an idea.

(oh, and my silly OCD is irritated at the big/small first letters in
the comments not being consistent)


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: [PATCHv4 1/5] mac80211: skip netdev queue control with software queuing

[Michal Kazior]

On 5 May 2016 at 13:00, Michal Kazior <michal.kaz...@tieto.com> wrote:
[...]
> -static void ieee80211_drv_tx(struct ieee80211_local *local,
> -struct ieee80211_vif *vif,
> -struct ieee80211_sta *pubsta,
> -struct sk_buff *skb)
> +static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
> + struct ieee80211_vif *vif,
> + struct ieee80211_sta *pubsta,
> + struct sk_buff *skb)
>  {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> -   struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> -   struct ieee80211_tx_control control = {
> -   .sta = pubsta,
> -   };
> -   struct ieee80211_txq *txq = NULL;
> -   struct txq_info *txqi;
> -   u8 ac;
>
> if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
> (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
> -   goto tx_normal;
> +   return NULL;
>
> if (!ieee80211_is_data(hdr->frame_control))
> -   goto tx_normal;
> +   return NULL;
>
> if (pubsta) {
> u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
>
> -   txq = pubsta->txq[tid];
> +   return to_txq_info(pubsta->txq[tid]);
> } else if (vif) {
> -   txq = vif->txq;
> +   return to_txq_info(vif->txq);
> }

I just noticed this crashes on non-wake_tx_queue drivers. I'll re-spin
a v5 with this fixed later.


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

[PATCH] ath10k: improve tx scheduling

[Michal Kazior]

Recent changes revolving around implementing
wake_tx_queue support introduced a significant
performance regressions on some (slower, uni-proc)
systems.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ---
 drivers/net/wireless/ath/ath10k/mac.c| 7 ---
 drivers/net/wireless/ath/ath10k/txrx.c   | 3 +++
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index cc979a4faeb0..7411b69fde42 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2291,7 +2291,6 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct 
sk_buff *skb)
ath10k_htt_tx_mgmt_dec_pending(htt);
spin_unlock_bh(>tx_lock);
}
-   ath10k_mac_tx_push_pending(ar);
break;
}
case HTT_T2H_MSG_TYPE_TX_COMPL_IND:
@@ -2442,8 +2441,6 @@ static void ath10k_htt_txrx_compl_task(unsigned long ptr)
dev_kfree_skb_any(skb);
}
 
-   ath10k_mac_tx_push_pending(ar);
-
num_mpdus = atomic_read(>num_mpdus_ready);
 
while (num_mpdus) {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
b/drivers/net/wireless/ath/ath10k/mac.c
index 6dd1d26b357f..1dd415d3c0ee 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3781,6 +3781,9 @@ void ath10k_mac_tx_push_pending(struct ath10k *ar)
int ret;
int max;
 
+   if (ar->htt.num_pending_tx >= (ar->htt.max_num_pending_tx / 2))
+   return;
+
spin_lock_bh(>txqs_lock);
rcu_read_lock();
 
@@ -4051,9 +4054,7 @@ static void ath10k_mac_op_wake_tx_queue(struct 
ieee80211_hw *hw,
list_add_tail(>list, >txqs);
spin_unlock_bh(>txqs_lock);
 
-   if (ath10k_mac_tx_can_push(hw, txq))
-   tasklet_schedule(>htt.txrx_compl_task);
-
+   ath10k_mac_tx_push_pending(ar);
ath10k_htt_tx_txq_update(hw, txq);
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c 
b/drivers/net/wireless/ath/ath10k/txrx.c
index 576e7c42ed65..1966c787998b 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -117,6 +117,9 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
 
ieee80211_tx_status(htt->ar->hw, msdu);
/* we do not own the msdu anymore */
+
+   ath10k_mac_tx_push_pending(ar);
+
return 0;
 }
 
-- 
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: [PATCHv4 5/5] mac80211: add debug knobs for codel

[Michal Kazior]

On 6 May 2016 at 07:51, Dave Taht <dave.t...@gmail.com> wrote:
> On Thu, May 5, 2016 at 10:27 PM, Michal Kazior <michal.kaz...@tieto.com> 
> wrote:
>> On 5 May 2016 at 17:21, Dave Taht <dave.t...@gmail.com> wrote:
>>> On Thu, May 5, 2016 at 4:00 AM, Michal Kazior <michal.kaz...@tieto.com> 
>>> wrote:
>>>> This adds a few debugfs entries to make it easier
>>>> to test, debug and experiment.
>>>
>>> I might argue in favor of moving all these (inc the fq ones) into
>>> their own dir, maybe "aqm" or "sqm".
>>>
>>> The mixture of read only stats and configuration vars is a bit confusing.
>>>
>>> Also in my testing of the previous patch, actually seeing the stats
>>> get updated seemed to be highly async or inaccurate. For example, it
>>> was obvious from the captures themselves that codel_ce_mark-ing was
>>> happening, but the actual numbers out of wack with the mark seen or
>>> fq_backlog seen.  (I can go back to revisit this)
>>
>> That's kind of expected since all of these bits are exposed as
>> separate debugfs entries/files. To avoid that it'd be necessary to
>> provide a single debugfs entry/file whose contents are generated on
>> open() while holding local->fq.lock. But then you could argue it
>> should contain all per-sta-tid info as well (backlog, flows, drops) as
>> well instead of having them in netdev*/stations/*/txqs.
>> Hmm..
>
> I have not had time to write up todays results to any full extent, but
> they were pretty spectacular.
>
> I have a comparison of the baseline ath10k driver vs your 3.5 patchset
> here on the second plot:
>
> http://blog.cerowrt.org/post/predictive_codeling/
>
> The raw data is here:
> https://github.com/dtaht/blog-cerowrt/tree/master/content/flent/qca-10.2-fqmac35-codel-5

It's probably good to explicitly mention that you test(ed) ath10k with
my RFC DQL patch applied. Without it the fqcodel benefits are a lot
less significant.

(oh, and the "3.5" is pre-PATCHv4 before fq/codel split work:
https://github.com/kazikcz/linux/tree/fqmac-v3.5 )


>
> ...
>
> a note: quantum of the mtu (typically 1514) is a saner default than 300,
>
> (the older patch I had, set it to 300, dunno what your default is now).

I still use 300.


> and quantum 1514, codel target 5ms rather than 20ms for this test
> series was *just fine* (but more testing of the lower target is
> needed)

I would keep 20ms for now until we get more test data. I'm mostly
concerned about MU performance on ath10k which requires significant
amount of buffering.


> However:
>
> quantum "300" only makes sense for very, very low bandwidths (say <
> 6mbits), in other scenarios it just eats extra cpu (5 passes through
> the loop to send a big packet) and disables
> the "new/old" queue feature which helps "push" new flows to flow
> balance. I'd default it to the larger value.

Perhaps this could be dynamically adjusted to match the slowest
station known to rate control in the future? Oh, and there's
multicast..


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: [PATCHv4 5/5] mac80211: add debug knobs for codel

[Michal Kazior]

On 5 May 2016 at 17:21, Dave Taht <dave.t...@gmail.com> wrote:
> On Thu, May 5, 2016 at 4:00 AM, Michal Kazior <michal.kaz...@tieto.com> wrote:
>> This adds a few debugfs entries to make it easier
>> to test, debug and experiment.
>
> I might argue in favor of moving all these (inc the fq ones) into
> their own dir, maybe "aqm" or "sqm".
>
> The mixture of read only stats and configuration vars is a bit confusing.
>
> Also in my testing of the previous patch, actually seeing the stats
> get updated seemed to be highly async or inaccurate. For example, it
> was obvious from the captures themselves that codel_ce_mark-ing was
> happening, but the actual numbers out of wack with the mark seen or
> fq_backlog seen.  (I can go back to revisit this)

That's kind of expected since all of these bits are exposed as
separate debugfs entries/files. To avoid that it'd be necessary to
provide a single debugfs entry/file whose contents are generated on
open() while holding local->fq.lock. But then you could argue it
should contain all per-sta-tid info as well (backlog, flows, drops) as
well instead of having them in netdev*/stations/*/txqs.

Hmm..


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

[PATCHv4 1/5] mac80211: skip netdev queue control with software queuing

[Michal Kazior]

Qdiscs are designed with no regard to 802.11
aggregation requirements and hand out
packet-by-packet with no guarantee they are
destined to the same tid. This does more bad than
good no matter how fairly a given qdisc may behave
on an ethernet interface.

Software queuing used per-AC netdev subqueue
congestion control whenever a global AC limit was
hit. This meant in practice a single station or
tid queue could starve others rather easily. This
could resonate with qdiscs in a bad way or could
just end up with poor aggregation performance.
Increasing the AC limit would increase induced
latency which is also bad.

Disabling qdiscs by default and performing
taildrop instead of netdev subqueue congestion
control on the other hand makes it possible for
tid queues to fill up "in the meantime" while
preventing stations starving each other.

This increases aggregation opportunities and
should allow software queuing based drivers
achieve better performance by utilizing airtime
more efficiently with big aggregates.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * make queue depth limit per interface instead of
   per radio [Johannes]

 include/net/mac80211.h |  4 ---
 net/mac80211/ieee80211_i.h |  2 +-
 net/mac80211/iface.c   | 18 --
 net/mac80211/main.c|  3 --
 net/mac80211/sta_info.c|  2 +-
 net/mac80211/tx.c  | 82 +-
 net/mac80211/util.c| 11 ---
 7 files changed, 67 insertions(+), 55 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 07ef9378df2b..ffb90dfe0d70 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2143,9 +2143,6 @@ enum ieee80211_hw_flags {
  * @n_cipher_schemes: a size of an array of cipher schemes definitions.
  * @cipher_schemes: a pointer to an array of cipher scheme definitions
  * supported by HW.
- *
- * @txq_ac_max_pending: maximum number of frames per AC pending in all txq
- * entries for a vif.
  */
 struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2176,7 +2173,6 @@ struct ieee80211_hw {
u8 uapsd_max_sp_len;
u8 n_cipher_schemes;
const struct ieee80211_cipher_scheme *cipher_schemes;
-   int txq_ac_max_pending;
 };
 
 static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 9438c9406687..634603320374 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -856,7 +856,7 @@ struct ieee80211_sub_if_data {
bool control_port_no_encrypt;
int encrypt_headroom;
 
-   atomic_t txqs_len[IEEE80211_NUM_ACS];
+   atomic_t num_tx_queued;
struct ieee80211_tx_queue_params tx_conf[IEEE80211_NUM_ACS];
struct mac80211_qos_map __rcu *qos_map;
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index c59af3eb9fa4..609c5174d798 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -976,13 +976,13 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
 
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
+   int n = skb_queue_len(>queue);
 
spin_lock_bh(>queue.lock);
ieee80211_purge_tx_queue(>hw, >queue);
+   atomic_sub(n, >num_tx_queued);
txqi->byte_cnt = 0;
spin_unlock_bh(>queue.lock);
-
-   atomic_set(>txqs_len[txqi->txq.ac], 0);
}
 
if (local->open_count == 0)
@@ -1198,6 +1198,12 @@ static void ieee80211_if_setup(struct net_device *dev)
dev->destructor = ieee80211_if_free;
 }
 
+static void ieee80211_if_setup_no_queue(struct net_device *dev)
+{
+   ieee80211_if_setup(dev);
+   dev->priv_flags |= IFF_NO_QUEUE;
+}
+
 static void ieee80211_iface_work(struct work_struct *work)
 {
struct ieee80211_sub_if_data *sdata =
@@ -1707,6 +1713,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const 
char *name,
struct net_device *ndev = NULL;
struct ieee80211_sub_if_data *sdata = NULL;
struct txq_info *txqi;
+   void (*if_setup)(struct net_device *dev);
int ret, i;
int txqs = 1;
 
@@ -1734,12 +1741,17 @@ int ieee80211_if_add(struct ieee80211_local *local, 
const char *name,
txq_size += sizeof(struct txq_info) +
local->hw.txq_data_size;
 
+   if (local->ops->wake_tx_queue)
+   if_setup = ieee80211_if_setup_no_queue;
+   else
+   if_setup = ieee80211_if_setup;
+
if (local->hw.queues >= IEEE80211_NUM_ACS)
txqs = IEEE80211_NUM_ACS;
 
ndev = alloc_netdev_mqs(size + txq_size,
name, name_assign_

[PATCHv4 3/5] mac80211: add debug knobs for fair queuing

[Michal Kazior]

This adds a few debugfs entries and a module
parameter to make it easier to test, debug and
experiment.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 net/mac80211/debugfs.c| 77 +++
 net/mac80211/debugfs_netdev.c | 28 +++-
 net/mac80211/debugfs_sta.c| 45 +
 net/mac80211/tx.c |  8 -
 4 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index b251b2f7f8dd..43592b6f79f0 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -31,6 +31,30 @@ int mac80211_format_buffer(char __user *userbuf, size_t 
count,
return simple_read_from_buffer(userbuf, count, ppos, buf, res);
 }
 
+static int mac80211_parse_buffer(const char __user *userbuf,
+size_t count,
+loff_t *ppos,
+char *fmt, ...)
+{
+   va_list args;
+   char buf[DEBUGFS_FORMAT_BUFFER_SIZE] = {};
+   int res;
+
+   if (count > sizeof(buf))
+   return -EINVAL;
+
+   if (copy_from_user(buf, userbuf, count))
+   return -EFAULT;
+
+   buf[sizeof(buf) - 1] = '\0';
+
+   va_start(args, fmt);
+   res = vsscanf(buf, fmt, args);
+   va_end(args);
+
+   return count;
+}
+
 #define DEBUGFS_READONLY_FILE_FN(name, fmt, value...)  \
 static ssize_t name## _read(struct file *file, char __user *userbuf,   \
size_t count, loff_t *ppos) \
@@ -70,6 +94,52 @@ DEBUGFS_READONLY_FILE(wep_iv, "%#08x",
 DEBUGFS_READONLY_FILE(rate_ctrl_alg, "%s",
local->rate_ctrl ? local->rate_ctrl->ops->name : "hw/driver");
 
+#define DEBUGFS_RW_FILE_FN(name, expr) \
+static ssize_t name## _write(struct file *file,\
+const char __user *userbuf,\
+size_t count,  \
+loff_t *ppos)  \
+{  \
+   struct ieee80211_local *local = file->private_data; \
+   return expr;\
+}
+
+#define DEBUGFS_RW_FILE(name, expr, fmt, value...) \
+   DEBUGFS_READONLY_FILE_FN(name, fmt, value)  \
+   DEBUGFS_RW_FILE_FN(name, expr)  \
+   DEBUGFS_RW_FILE_OPS(name)
+
+#define DEBUGFS_RW_FILE_OPS(name)  \
+static const struct file_operations name## _ops = {\
+   .read = name## _read,   \
+   .write = name## _write, \
+   .open = simple_open,\
+   .llseek = generic_file_llseek,  \
+}
+
+#define DEBUGFS_RW_EXPR_FQ(args...)\
+({ \
+   int res;\
+   res = mac80211_parse_buffer(userbuf, count, ppos, args);\
+   res;\
+})
+
+DEBUGFS_READONLY_FILE(fq_flows_cnt, "%u",
+ local->fq.flows_cnt);
+DEBUGFS_READONLY_FILE(fq_backlog, "%u",
+ local->fq.backlog);
+DEBUGFS_READONLY_FILE(fq_overlimit, "%u",
+ local->fq.overlimit);
+DEBUGFS_READONLY_FILE(fq_collisions, "%u",
+ local->fq.collisions);
+
+DEBUGFS_RW_FILE(fq_limit,
+   DEBUGFS_RW_EXPR_FQ("%u", >fq.limit),
+   "%u", local->fq.limit);
+DEBUGFS_RW_FILE(fq_quantum,
+   DEBUGFS_RW_EXPR_FQ("%u", >fq.quantum),
+   "%u", local->fq.quantum);
+
 #ifdef CONFIG_PM
 static ssize_t reset_write(struct file *file, const char __user *user_buf,
   size_t count, loff_t *ppos)
@@ -257,6 +327,13 @@ void debugfs_hw_add(struct ieee80211_local *local)
DEBUGFS_ADD(user_power);
DEBUGFS_ADD(power);
 
+   DEBUGFS_ADD(fq_flows_cnt);
+   DEBUGFS_ADD(fq_backlog);
+   DEBUGFS_ADD(fq_overlimit);
+   DEBUGFS_ADD(fq_collisions);
+   DEBUGFS_ADD(fq_limit);
+   DEBUGFS_ADD(fq_quantum);
+
statsd = debugfs_create_dir("statistics", phyd);
 
/* if the dir failed, don't put all the other things into the root! */
diff --git a/net/mac80211/debugfs_netdev.c b/net/mac80211/debugfs_netdev.c
index a5ba739cd2a7..369755b2b078 100644
--- a/net/mac80211/debugfs_netdev.c
+++ b/net/mac80211/debugfs_netdev.c
@@ -30,7 +30,7 @@ static ssize_t ieee80211_if_read(
size_t count, loff_t *ppos,
ssize_t (*format)(const struct ieee80211_sub_if_data *, char 

[PATCHv4 2/5] mac80211: implement fair queueing per txq

[Michal Kazior]

mac80211's software queues were designed to work
very closely with device tx queues. They are
required to make use of 802.11 packet aggregation
easily and efficiently.

Due to the way 802.11 aggregation is designed it
only makes sense to keep fair queuing as close to
hardware as possible to reduce induced latency and
inertia and provide the best flow responsivness.

This change doesn't translate directly to
immediate and significant gains. End result
depends on driver's induced latency. Best results
can be achieved if driver keeps its own tx
queue/fifo fill level to a minimum.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * removed internal fq.h and re-used in-kernel one

 net/mac80211/agg-tx.c  |   8 ++-
 net/mac80211/ieee80211_i.h |  24 ++--
 net/mac80211/iface.c   |  12 ++--
 net/mac80211/main.c|   7 +++
 net/mac80211/rx.c  |   2 +-
 net/mac80211/sta_info.c|  14 ++---
 net/mac80211/tx.c  | 137 ++---
 net/mac80211/util.c|  23 +---
 8 files changed, 163 insertions(+), 64 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 42fa81031dfa..5650c46bf91a 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -194,17 +194,21 @@ static void
 ieee80211_agg_stop_txq(struct sta_info *sta, int tid)
 {
struct ieee80211_txq *txq = sta->sta.txq[tid];
+   struct ieee80211_sub_if_data *sdata;
+   struct fq *fq;
struct txq_info *txqi;
 
if (!txq)
return;
 
txqi = to_txq_info(txq);
+   sdata = vif_to_sdata(txq->vif);
+   fq = >local->fq;
 
/* Lock here to protect against further seqno updates on dequeue */
-   spin_lock_bh(>queue.lock);
+   spin_lock_bh(>lock);
set_bit(IEEE80211_TXQ_STOP, >flags);
-   spin_unlock_bh(>queue.lock);
+   spin_unlock_bh(>lock);
 }
 
 static void
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 634603320374..6f8375f1df88 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "key.h"
 #include "sta_info.h"
 #include "debug.h"
@@ -805,10 +806,17 @@ enum txq_info_flags {
IEEE80211_TXQ_NO_AMSDU,
 };
 
+/**
+ * struct txq_info - per tid queue
+ *
+ * @tin: contains packets split into multiple flows
+ * @def_flow: used as a fallback flow when a packet destined to @tin hashes to
+ * a fq_flow which is already owned by a different tin
+ */
 struct txq_info {
-   struct sk_buff_head queue;
+   struct fq_tin tin;
+   struct fq_flow def_flow;
unsigned long flags;
-   unsigned long byte_cnt;
 
/* keep last! */
struct ieee80211_txq txq;
@@ -1099,6 +1107,8 @@ struct ieee80211_local {
 * it first anyway so they become a no-op */
struct ieee80211_hw hw;
 
+   struct fq fq;
+
const struct ieee80211_ops *ops;
 
/*
@@ -1931,9 +1941,13 @@ static inline bool ieee80211_can_run_worker(struct 
ieee80211_local *local)
return true;
 }
 
-void ieee80211_init_tx_queue(struct ieee80211_sub_if_data *sdata,
-struct sta_info *sta,
-struct txq_info *txq, int tid);
+int ieee80211_txq_setup_flows(struct ieee80211_local *local);
+void ieee80211_txq_teardown_flows(struct ieee80211_local *local);
+void ieee80211_txq_init(struct ieee80211_sub_if_data *sdata,
+   struct sta_info *sta,
+   struct txq_info *txq, int tid);
+void ieee80211_txq_purge(struct ieee80211_local *local,
+struct txq_info *txqi);
 void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 u16 transaction, u16 auth_alg, u16 status,
 const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 609c5174d798..b123a9e325b3 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -779,6 +779,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
  bool going_down)
 {
struct ieee80211_local *local = sdata->local;
+   struct fq *fq = >fq;
unsigned long flags;
struct sk_buff *skb, *tmp;
u32 hw_reconf_flags = 0;
@@ -976,13 +977,10 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
 
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
-   int n = skb_queue_len(>queue);
 
-   spin_lock_bh(>queue.lock);
-   ieee80211_purge_tx_queue(>hw, >queue);
-   atomic_sub(n, >num_tx_queued);
-   txqi->byte_cnt = 0;
-   spin_unlock_bh(>queue.lock);
+   spin_lock_b

[PATCHv4 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

There is no other limit other than a global
packet count limit when using software queuing.
This means a single flow queue can grow insanely
long. This is particularly bad for TCP congestion
algorithms which requires a little more
sophisticated frame dropping scheme than a mere
headdrop on limit overflow.

Hence apply (a slighly modified, to fit the knobs)
CoDel5 on flow queues. This improves TCP
convergence and stability when combined with
wireless driver which keeps its own tx queue/fifo
at a minimum fill level for given link conditions.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * removed internal codel.h and re-used in-kernel one

 include/net/mac80211.h |  14 +-
 net/mac80211/ieee80211_i.h |   5 +++
 net/mac80211/tx.c  | 109 -
 3 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ffb90dfe0d70..cc534f1b0f8e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 /**
@@ -895,7 +896,18 @@ struct ieee80211_tx_info {
unsigned long jiffies;
};
/* NB: vif can be NULL for injected frames */
-   struct ieee80211_vif *vif;
+   union {
+   /* NB: vif can be NULL for injected frames */
+   struct ieee80211_vif *vif;
+
+   /* When packets are enqueued on txq it's easy
+* to re-construct the vif pointer. There's no
+* more space in tx_info so it can be used to
+* store the necessary enqueue time for packet
+* sojourn time computation.
+*/
+   codel_time_t enqueue_time;
+   };
struct ieee80211_key_conf *hw_key;
u32 flags;
/* 4 bytes free */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 6f8375f1df88..54edfb6fc1d1 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -812,10 +812,12 @@ enum txq_info_flags {
  * @tin: contains packets split into multiple flows
  * @def_flow: used as a fallback flow when a packet destined to @tin hashes to
  * a fq_flow which is already owned by a different tin
+ * @def_cvars: codel vars for @def_flow
  */
 struct txq_info {
struct fq_tin tin;
struct fq_flow def_flow;
+   struct codel_vars def_cvars;
unsigned long flags;
 
/* keep last! */
@@ -1108,6 +1110,9 @@ struct ieee80211_local {
struct ieee80211_hw hw;
 
struct fq fq;
+   struct codel_vars *cvars;
+   struct codel_params cparams;
+   struct codel_stats cstats;
 
const struct ieee80211_ops *ops;
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 47936b939591..013b382f6888 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 
@@ -1269,11 +1271,92 @@ static struct txq_info *ieee80211_get_txq(struct 
ieee80211_local *local,
return NULL;
 }
 
+static void ieee80211_set_skb_enqueue_time(struct sk_buff *skb)
+{
+   IEEE80211_SKB_CB(skb)->control.enqueue_time = codel_get_time();
+}
+
+static void ieee80211_set_skb_vif(struct sk_buff *skb, struct txq_info *txqi)
+{
+   IEEE80211_SKB_CB(skb)->control.vif = txqi->txq.vif;
+}
+
+static u32 codel_skb_len_func(const struct sk_buff *skb)
+{
+   return skb->len;
+}
+
+static codel_time_t codel_skb_time_func(const struct sk_buff *skb)
+{
+   const struct ieee80211_tx_info *info;
+
+   info = (const struct ieee80211_tx_info *)skb->cb;
+   return info->control.enqueue_time;
+}
+
+static struct sk_buff *codel_dequeue_func(struct codel_vars *cvars,
+ void *ctx)
+{
+   struct ieee80211_local *local;
+   struct txq_info *txqi;
+   struct fq *fq;
+   struct fq_flow *flow;
+
+   txqi = ctx;
+   local = vif_to_sdata(txqi->txq.vif)->local;
+   fq = >fq;
+
+   if (cvars == >def_cvars)
+   flow = >def_flow;
+   else
+   flow = >flows[cvars - local->cvars];
+
+   return fq_flow_dequeue(fq, flow);
+}
+
+static void codel_drop_func(struct sk_buff *skb,
+   void *ctx)
+{
+   struct ieee80211_local *local;
+   struct ieee80211_hw *hw;
+   struct txq_info *txqi;
+
+   txqi = ctx;
+   local = vif_to_sdata(txqi->txq.vif)->local;
+   hw = >hw;
+
+   ieee80211_free_txskb(hw, skb);
+}
+
 st

[PATCHv4 5/5] mac80211: add debug knobs for codel

[Michal Kazior]

This adds a few debugfs entries to make it easier
to test, debug and experiment.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---

Notes:
v4:
 * stats adjustments (in-kernel codel has more of them)

 net/mac80211/debugfs.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 43592b6f79f0..c7cfedc61fc4 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -124,6 +124,15 @@ static const struct file_operations name## _ops = {
\
res;\
 })
 
+#define DEBUGFS_RW_BOOL(arg)   \
+({ \
+   int res;\
+   int val;\
+   res = mac80211_parse_buffer(userbuf, count, ppos, "%d", );  \
+   arg = !!(val);  \
+   res;\
+})
+
 DEBUGFS_READONLY_FILE(fq_flows_cnt, "%u",
  local->fq.flows_cnt);
 DEBUGFS_READONLY_FILE(fq_backlog, "%u",
@@ -132,6 +141,16 @@ DEBUGFS_READONLY_FILE(fq_overlimit, "%u",
  local->fq.overlimit);
 DEBUGFS_READONLY_FILE(fq_collisions, "%u",
  local->fq.collisions);
+DEBUGFS_READONLY_FILE(codel_maxpacket, "%u",
+ local->cstats.maxpacket);
+DEBUGFS_READONLY_FILE(codel_drop_count, "%u",
+ local->cstats.drop_count);
+DEBUGFS_READONLY_FILE(codel_drop_len, "%u",
+ local->cstats.drop_len);
+DEBUGFS_READONLY_FILE(codel_ecn_mark, "%u",
+ local->cstats.ecn_mark);
+DEBUGFS_READONLY_FILE(codel_ce_mark, "%u",
+ local->cstats.ce_mark);
 
 DEBUGFS_RW_FILE(fq_limit,
DEBUGFS_RW_EXPR_FQ("%u", >fq.limit),
@@ -139,6 +158,18 @@ DEBUGFS_RW_FILE(fq_limit,
 DEBUGFS_RW_FILE(fq_quantum,
DEBUGFS_RW_EXPR_FQ("%u", >fq.quantum),
"%u", local->fq.quantum);
+DEBUGFS_RW_FILE(codel_interval,
+   DEBUGFS_RW_EXPR_FQ("%u", >cparams.interval),
+   "%u", local->cparams.interval);
+DEBUGFS_RW_FILE(codel_target,
+   DEBUGFS_RW_EXPR_FQ("%u", >cparams.target),
+   "%u", local->cparams.target);
+DEBUGFS_RW_FILE(codel_mtu,
+   DEBUGFS_RW_EXPR_FQ("%u", >cparams.mtu),
+   "%u", local->cparams.mtu);
+DEBUGFS_RW_FILE(codel_ecn,
+   DEBUGFS_RW_BOOL(local->cparams.ecn),
+   "%d", local->cparams.ecn ? 1 : 0);
 
 #ifdef CONFIG_PM
 static ssize_t reset_write(struct file *file, const char __user *user_buf,
@@ -333,6 +364,15 @@ void debugfs_hw_add(struct ieee80211_local *local)
DEBUGFS_ADD(fq_collisions);
DEBUGFS_ADD(fq_limit);
DEBUGFS_ADD(fq_quantum);
+   DEBUGFS_ADD(codel_maxpacket);
+   DEBUGFS_ADD(codel_drop_count);
+   DEBUGFS_ADD(codel_drop_len);
+   DEBUGFS_ADD(codel_ecn_mark);
+   DEBUGFS_ADD(codel_ce_mark);
+   DEBUGFS_ADD(codel_interval);
+   DEBUGFS_ADD(codel_target);
+   DEBUGFS_ADD(codel_mtu);
+   DEBUGFS_ADD(codel_ecn);
 
statsd = debugfs_create_dir("statistics", phyd);
 
-- 
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

[PATCHv4 0/5] mac80211: implement fq_codel

[Michal Kazior]

Hi,

This patchset disables qdiscs for drivers
using software queuing and performs fq_codel-like
dequeuing on txqs.

This is based on net-next/master
(035cd6ba53eff060760c4f4d11339fcc916a967c).

For anyone interested I've pushed tree with my
(now oldish) ath10k DQL RFC and a small fix I've
been testing:

  https://github.com/kazikcz/linux/tree/fqmac-v4%2Bdqlrfc%2Bcpuregrfix

Background:

  https://www.spinics.net/lists/linux-wireless/msg149776.html
  https://www.spinics.net/lists/linux-wireless/msg148714.html
  https://www.spinics.net/lists/linux-wireless/msg149039.html
  http://blog.cerowrt.org/post/dql_on_wifi_2/
  http://blog.cerowrt.org/post/dql_on_wifi/
  http://blog.cerowrt.org/post/fq_codel_on_ath10k/


v4:
 * the taildrop stop-gap patch moved to
   per-interface limit (instead of per-radio) [Johannes]
 * pushed fq.h and codel.h changes to include/net/ [Johannes]

v3:
 * split taildrop, fq and codel functionalities
   into separate patches [Avery]

v2:
 * fix invalid ptr deref
 * fix compilation for backports


Michal Kazior (5):
  mac80211: skip netdev queue control with software queuing
  mac80211: implement fair queueing per txq
  mac80211: add debug knobs for fair queuing
  mac80211: implement codel on fair queuing flows
  mac80211: add debug knobs for codel

 include/net/mac80211.h|  18 ++-
 net/mac80211/agg-tx.c |   8 +-
 net/mac80211/debugfs.c| 117 
 net/mac80211/debugfs_netdev.c |  28 +++-
 net/mac80211/debugfs_sta.c|  45 +++
 net/mac80211/ieee80211_i.h|  31 -
 net/mac80211/iface.c  |  26 ++--
 net/mac80211/main.c   |  10 +-
 net/mac80211/rx.c |   2 +-
 net/mac80211/sta_info.c   |  14 +-
 net/mac80211/tx.c | 302 --
 net/mac80211/util.c   |  34 ++---
 12 files changed, 532 insertions(+), 103 deletions(-)

-- 
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: [RFC] mac80211: add extap functionality

[Michal Kazior]

On 19 April 2016 at 13:02, Johannes Berg  wrote:
>> > How much of that could be done with nftables btw?
>
>> I'm not sure if I follow. Do you mean what I've been able to do with
>> relayd until now? Without link-local ipv6 routing DHCPv6 is broken
>> (could probably addressed with DHCPv6 Relay to a certain degree) and
>> RS/RA may not work (if it propagates fe80:: routes). Also apps that
>> rely on fe80:: socket binding/addressing will fail.
>
> Ok, so that makes sense I guess - but you were speaking of some packet
> mangling etc. and I was wondering if the nftables virtual machine could
> actually do something like that.

By mangling I meant packet payload needs to be modified in various
ways (typically just ether_dest/src, but ARP/NS/NA/DHCP needs extra
care). I don't think you can force a link-local packet to switch
interfaces through nftables anyway (or can you?). Even if you could I
guess you could argue it's a bug?


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: [RFC] mac80211: add extap functionality

[Michal Kazior]

On 19 April 2016 at 11:11, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Mon, 2016-04-18 at 13:23 +0200, Michal Kazior wrote:
>
>> You can't really implement complete IPv6 support in relayd though.
>> Link-local routing is forbidden by the spec explicitly and a patch
>> I've cooked up was rejected[1].
>>
>> I guess this leaves either the kernel's wireless stack to take up the
>> task or a special link device (for which I'm soliciting feedback now
>> [2]).
>
> I think you'll probably have to cook up a simple patch to get that
> question in [2] taken seriously :)

Heh.


> How much of that could be done with nftables btw?

I'm not sure if I follow. Do you mean what I've been able to do with
relayd until now? Without link-local ipv6 routing DHCPv6 is broken
(could probably addressed with DHCPv6 Relay to a certain degree) and
RS/RA may not work (if it propagates fe80:: routes). Also apps that
rely on fe80:: socket binding/addressing will fail.


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: [PATCHv3 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

On 19 April 2016 at 11:06, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Mon, 2016-04-18 at 14:38 +0200, Michal Kazior wrote:
>> On 18 April 2016 at 07:31, Michal Kazior <michal.kaz...@tieto.com>
>> wrote:
>> >
>> > On 17 April 2016 at 00:29, Johannes Berg <johan...@sipsolutions.net
>> > > wrote:
>> > >
>> > > On Thu, 2016-04-14 at 14:18 +0200, Michal Kazior wrote:
>> > > >
>> > > >
>> > > > + struct ieee80211_vif *vif;
>> > > > +
>> > > > + /* When packets are enqueued on
>> > > > txq
>> > > > it's easy
>> > > > +  * to re-construct the vif
>> > > > pointer.
>> > > > There's no
>> > > > +  * more space in tx_info so it
>> > > > can
>> > > > be used to
>> > > > +  * store the necessary enqueue
>> > > > time
>> > > > for packet
>> > > > +  * sojourn time computation.
>> > > > +  */
>> > > > + u64 enqueue_time;
>> > > > + };
>> > > I wonder if we could move something like the hw_key into
>> > > tx_control
>> > > instead?
>> > Hmm.. It's probably doable. From a quick look it'll require quite
>> > some
>> > change here and there (e.g. tdls_channel_switch op will need to be
>> > extended to pass tx_control). I'll play with the idea..
>> This is actually far more than I thought initially.
>
> Fair enough. Perhaps it could be done for the vif? But ISTR there were
> issues with that when I looked.

Still tricky in a similar fashion as hw_key.


> We should just get rid of all the rate stuff and convert everything to
> use rate tables, but ... :)

I'm guessing it's not trivial either and you risk breaking a lot of stuff? :)


>> A lot of drivers
>> (b43, b43legacy, rtlwifi, wl, cw1200) access hw_key outside of tx
>> op context (tx workers, tx completions). I'm not even sure this is
>> safe (keys can be freed in the meantime by mac80211 hence invaliding
>> the pointer inside skb, no?).
>>
>
> Hm, yeah, that does seem problematic unless they synchronize against
> key removal somehow?

I didn't see any explicit synchronization but maybe I missed something.


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: [Codel] [PATCHv3 2/5] mac80211: implement fair queueing per txq

[Michal Kazior]

On 18 April 2016 at 14:31, Eric Dumazet <eric.duma...@gmail.com> wrote:
> On Mon, 2016-04-18 at 07:16 +0200, Michal Kazior wrote:
>
>>
>> I guess .h file can give the compiler an opportunity for more
>> optimizations. With .c you would need LTO which I'm not sure if it's
>> available everywhere.
>>
>
> This makes little sense really. Otherwise everything would be in .h
> files.
>
> include/net/codel.h is an include file because both codel and fq_codel
> use a common template for codel_dequeue() in fast path.
>
> But net/mac80211/fq.h is included once, so should be a .c

FWIW cfg80211 drivers might become another user of the fq/codel stuff
in the future.

Arguably I should make include/net/codel.h not be qdisc specific as it
is now (and hence re-usable by mac80211) and submit fq.h to
include/net/. Would that be better (it'll probably take a lot longer
to propagate over trees, no?)


>
> Certainly all the code in control plan is not fast path and does not
> deserve being duplicated.

Good point. The fq init/reset stuff is probably a good example.

However if I were to put fq.h into include/net/ where should I put the
init/reset stuff then? net/core/?


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: [PATCHv3 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

On 18 April 2016 at 07:31, Michal Kazior <michal.kaz...@tieto.com> wrote:
> On 17 April 2016 at 00:29, Johannes Berg <johan...@sipsolutions.net> wrote:
>> On Thu, 2016-04-14 at 14:18 +0200, Michal Kazior wrote:
>>>
>>> + struct ieee80211_vif *vif;
>>> +
>>> + /* When packets are enqueued on txq
>>> it's easy
>>> +  * to re-construct the vif pointer.
>>> There's no
>>> +  * more space in tx_info so it can
>>> be used to
>>> +  * store the necessary enqueue time
>>> for packet
>>> +  * sojourn time computation.
>>> +  */
>>> + u64 enqueue_time;
>>> + };
>>
>> I wonder if we could move something like the hw_key into tx_control
>> instead?
>
> Hmm.. It's probably doable. From a quick look it'll require quite some
> change here and there (e.g. tdls_channel_switch op will need to be
> extended to pass tx_control). I'll play with the idea..

This is actually far more than I thought initially. A lot of drivers
(b43, b43legacy, rtlwifi, wl, cw1200) access hw_key outside of tx
op context (tx workers, tx completions). I'm not even sure this is
safe (keys can be freed in the meantime by mac80211 hence invaliding
the pointer inside skb, no?).


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: [RFC] mac80211: add extap functionality

[Michal Kazior]

On 17 February 2016 at 17:55, Felix Fietkau  wrote:
> On 2016-02-17 12:55, Grzegorz Bajorski wrote:
>> Client interface briding was only possible when 4addr frames were used with
>> a 4addr/WDS aware AP. It was not possible to do it otherwise due to 3addr
>> frame limitation.
>>
>> The extap logic introduces a smart MAC address masking/translation
>> (including modyfing packets beyond SA/DA, e.g. DHCP broadcast flag is set).
>>
>> There are still some unsolved problems and bugs:
>>  - due to bridge port routing and sk_buff payload sharing skb_copy() is
>>performed; this ideally should be reworked
>>  - ipv6 support is still not finished
>>  - extap is enabled by default currently; it should be configurable via
>>nl80211 the same way 4addr is
>>
>> There's also an idea to move this as a generic link driver (just like
>> macvlan, et al) which would allow unmodified cfg80211 drivers to enjoy the
>> extap functionality. Thoughts?
>>
>> Note: This changes cfg80211 file in this single patch only for reviewing
>> convienence.
>>
>> This is an early draft to solicit comments on the design.
>>
>> Signed-off-by: Grzegorz Bajorski 
> You can get a lot of the same effect (sharing the same subnet between
> hosts behind multiple interfaces and having forwarding between them)
> without any changes to mac80211.
>
> OpenWrt uses a daemon called 'relayd' which I wrote some years ago. It
> does ARP translation, DHCP packet mangling and sets up policy routing to
> forward packets between multiple interfaces.
>
> You can find it here:
> http://git.openwrt.org/?p=project/relayd.git;a=summary
> git://git.openwrt.org/project/relayd.git
>
> Since you can cover the same use cases with user space code, I don't
> think it's a good idea to put bridge emulation hacks in the kernel's
> wireless stack.

You can't really implement complete IPv6 support in relayd though.
Link-local routing is forbidden by the spec explicitly and a patch
I've cooked up was rejected[1].

I guess this leaves either the kernel's wireless stack to take up the
task or a special link device (for which I'm soliciting feedback now
[2]).

Thoughts?


[1]: http://marc.info/?l=linux-netdev=146084698311664=2
[2]: http://marc.info/?l=linux-netdev=146097234002385=2


Michal
--
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 1/5] mac80211: skip netdev queue control with software queuing

[Michal Kazior]

On 17 April 2016 at 00:21, Johannes Berg  wrote:
>> +static void ieee80211_txq_enqueue(struct ieee80211_local *local,
>> +   struct txq_info *txqi,
>> +   struct sk_buff *skb)
>> +{
>> + lockdep_assert_held(>queue.lock);
> [...]
>> + atomic_inc(>num_tx_queued);
>
> This global kinda bothers me - anything we can do about removing it?

I don't think so. Re-counting via sta/vif/txq iteration every time is
rather a bad idea.

FWIW This is removed by the "fq" patch. The main purpose of taildrop
patch is to make some comparisons easier.


> We obviously didn't have it now - just one (even bigger limit!) per
> queue, so that's 4000 frames default per interface ... now you're down
> to 512 for the entire hardware. Perhaps keeping it per interface at
> least gets away the worst of the contention here?

The default qdisc limits were arguably already too big anyway.
Nevertheless it makes sense to have the 512 limit per interface
instead of per radio. I'll move num_tx_queued to sdata.


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: [PATCHv3 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

On 17 April 2016 at 00:29, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Thu, 2016-04-14 at 14:18 +0200, Michal Kazior wrote:
>>
>> + struct ieee80211_vif *vif;
>> +
>> + /* When packets are enqueued on txq
>> it's easy
>> +  * to re-construct the vif pointer.
>> There's no
>> +  * more space in tx_info so it can
>> be used to
>> +  * store the necessary enqueue time
>> for packet
>> +  * sojourn time computation.
>> +  */
>> + u64 enqueue_time;
>> + };
>
> I wonder if we could move something like the hw_key into tx_control
> instead?

Hmm.. It's probably doable. From a quick look it'll require quite some
change here and there (e.g. tdls_channel_switch op will need to be
extended to pass tx_control). I'll play with the idea..


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: [PATCHv3 2/5] mac80211: implement fair queueing per txq

[Michal Kazior]

On 17 April 2016 at 00:25, Johannes Berg <johan...@sipsolutions.net> wrote:
> On Sun, 2016-04-17 at 00:23 +0200, Johannes Berg wrote:
>> On Thu, 2016-04-14 at 14:18 +0200, Michal Kazior wrote:
>> >
>> >
>> > +++ b/net/mac80211/fq.h
>> >
>> Now that you've mostly rewritten it, why keep it in a .h file?
>>
>
> I think I just confused this with codel.h, but still - why a .h file
> with all this "real" code, meaning the file can really only be included
> once?

I guess .h file can give the compiler an opportunity for more
optimizations. With .c you would need LTO which I'm not sure if it's
available everywhere.


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

[PATCHv3 0/5] mac80211: implement fq_codel

[Michal Kazior]

Hi,

This patchset disables qdiscs for drivers
using software queuing and performs fq_codel-like
dequeuing on txqs.

I've reworked it as per Avery's suggestion (and
more).

I've (re)tested it against ath10k with and without
DQL (my ath10k RFC which is limited but sufficient
for some proofing scenarios) and got quite nice
looking results:

  http://imgur.com/a/8ruhK
  http://kazikcz.github.io/dl/2016-04-12-flent-fqmac-ath10k-dql.tar.gz

All DQL cases show incremental improvement and are
within expectations. The "dql-fq" case loses TCP
fairness/convergence (compared to "dql-taildrop")
because it removes per-txq 64 packet limit and
"dql-fqcodel" gets it back.


v3:
 * split taildrop, fq and codel functionalities
   into separate patches [Avery]

v2:
 * fix invalid ptr deref
 * fix compilation for backports


Michal Kazior (5):
  mac80211: skip netdev queue control with software queuing
  mac80211: implement fair queueing per txq
  mac80211: add debug knobs for fair queuing
  mac80211: implement codel on fair queuing flows
  mac80211: add debug knobs for codel

 include/net/mac80211.h|  17 ++-
 net/mac80211/agg-tx.c |   8 +-
 net/mac80211/codel.h  | 265 
 net/mac80211/codel_i.h| 100 +++
 net/mac80211/debugfs.c|  91 ++
 net/mac80211/debugfs_netdev.c |  28 -
 net/mac80211/debugfs_sta.c|  45 +++
 net/mac80211/fq.h | 276 ++
 net/mac80211/fq_i.h   |  82 +
 net/mac80211/ieee80211_i.h|  32 -
 net/mac80211/iface.c  |  26 ++--
 net/mac80211/main.c   |  10 +-
 net/mac80211/rx.c |   2 +-
 net/mac80211/sta_info.c   |  14 +--
 net/mac80211/tx.c | 274 ++---
 net/mac80211/util.c   |  34 ++
 16 files changed, 1204 insertions(+), 100 deletions(-)
 create mode 100644 net/mac80211/codel.h
 create mode 100644 net/mac80211/codel_i.h
 create mode 100644 net/mac80211/fq.h
 create mode 100644 net/mac80211/fq_i.h

-- 
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

[PATCHv3 1/5] mac80211: skip netdev queue control with software queuing

[Michal Kazior]

Qdiscs are designed with no regard to 802.11
aggregation requirements and hand out
packet-by-packet with no guarantee they are
destined to the same tid. This does more bad than
good no matter how fairly a given qdisc may behave
on an ethernet interface.

Software queuing used per-AC netdev subqueue
congestion control whenever a global AC limit was
hit. This meant in practice a single station or
tid queue could starve others rather easily. This
could resonate with qdiscs in a bad way or could
just end up with poor aggregation performance.
Increasing the AC limit would increase induced
latency which is also bad.

Disabling qdiscs by default and performing
taildrop instead of netdev subqueue congestion
control on the other hand makes it possible for
tid queues to fill up "in the meantime" while
preventing stations starving each other.

This increases aggregation opportunities and
should allow software queuing based drivers
achieve better performance by utilizing airtime
more efficiently with big aggregates.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 include/net/mac80211.h |  4 ---
 net/mac80211/ieee80211_i.h |  2 +-
 net/mac80211/iface.c   | 18 +--
 net/mac80211/main.c|  3 --
 net/mac80211/sta_info.c|  2 +-
 net/mac80211/tx.c  | 80 --
 net/mac80211/util.c| 11 ---
 7 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index a5cb1528..c24d0b8e4deb 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2113,9 +2113,6 @@ enum ieee80211_hw_flags {
  * @n_cipher_schemes: a size of an array of cipher schemes definitions.
  * @cipher_schemes: a pointer to an array of cipher scheme definitions
  * supported by HW.
- *
- * @txq_ac_max_pending: maximum number of frames per AC pending in all txq
- * entries for a vif.
  */
 struct ieee80211_hw {
struct ieee80211_conf conf;
@@ -2145,7 +2142,6 @@ struct ieee80211_hw {
u8 uapsd_max_sp_len;
u8 n_cipher_schemes;
const struct ieee80211_cipher_scheme *cipher_schemes;
-   int txq_ac_max_pending;
 };
 
 static inline bool _ieee80211_hw_check(struct ieee80211_hw *hw,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c6830fbe7d68..b2570aa66d33 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -855,7 +855,6 @@ struct ieee80211_sub_if_data {
bool control_port_no_encrypt;
int encrypt_headroom;
 
-   atomic_t txqs_len[IEEE80211_NUM_ACS];
struct ieee80211_tx_queue_params tx_conf[IEEE80211_NUM_ACS];
struct mac80211_qos_map __rcu *qos_map;
 
@@ -1118,6 +1117,7 @@ struct ieee80211_local {
fif_probe_req;
int probe_req_reg;
unsigned int filter_flags; /* FIF_* */
+   atomic_t num_tx_queued;
 
bool wiphy_ciphers_allocated;
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 453b4e741780..67c9b1e565ad 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -976,13 +976,13 @@ static void ieee80211_do_stop(struct 
ieee80211_sub_if_data *sdata,
 
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
+   int n = skb_queue_len(>queue);
 
spin_lock_bh(>queue.lock);
ieee80211_purge_tx_queue(>hw, >queue);
+   atomic_sub(n, >num_tx_queued);
txqi->byte_cnt = 0;
spin_unlock_bh(>queue.lock);
-
-   atomic_set(>txqs_len[txqi->txq.ac], 0);
}
 
if (local->open_count == 0)
@@ -1198,6 +1198,12 @@ static void ieee80211_if_setup(struct net_device *dev)
dev->destructor = ieee80211_if_free;
 }
 
+static void ieee80211_if_setup_no_queue(struct net_device *dev)
+{
+   ieee80211_if_setup(dev);
+   dev->priv_flags |= IFF_NO_QUEUE;
+}
+
 static void ieee80211_iface_work(struct work_struct *work)
 {
struct ieee80211_sub_if_data *sdata =
@@ -1707,6 +1713,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const 
char *name,
struct net_device *ndev = NULL;
struct ieee80211_sub_if_data *sdata = NULL;
struct txq_info *txqi;
+   void (*if_setup)(struct net_device *dev);
int ret, i;
int txqs = 1;
 
@@ -1734,12 +1741,17 @@ int ieee80211_if_add(struct ieee80211_local *local, 
const char *name,
txq_size += sizeof(struct txq_info) +
local->hw.txq_data_size;
 
+   if (local->ops->wake_tx_queue)
+   if_setup = ieee80211_if_setup_no_queue;
+   else
+   if_setup = ieee80211_if_setup;
+
if (local->hw.queues >= IEEE80211_NUM_ACS)
txqs = IEEE80211_NUM_ACS;
 
ndev = alloc_netdev_mqs(size +

[PATCHv3 4/5] mac80211: implement codel on fair queuing flows

[Michal Kazior]

There is no other limit other than a global
packet count limit when using software queuing.
This means a single flow queue can grow insanely
long. This is particularly bad for TCP congestion
algorithms which requires a little more
sophisticated frame dropping scheme than a mere
headdrop on limit overflow.

Hence apply (a slighly modified, to fit the knobs)
CoDel5 on flow queues. This improves TCP
convergence and stability when combined with
wireless driver which keeps its own tx queue/fifo
at a minimum fill level for given link conditions.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 include/net/mac80211.h |  13 ++-
 net/mac80211/codel.h   | 265 +
 net/mac80211/codel_i.h | 100 +
 net/mac80211/ieee80211_i.h |   5 +
 net/mac80211/tx.c  |  99 -
 5 files changed, 480 insertions(+), 2 deletions(-)
 create mode 100644 net/mac80211/codel.h
 create mode 100644 net/mac80211/codel_i.h

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index c24d0b8e4deb..d53b14bc4e79 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -889,7 +889,18 @@ struct ieee80211_tx_info {
unsigned long jiffies;
};
/* NB: vif can be NULL for injected frames */
-   struct ieee80211_vif *vif;
+   union {
+   /* NB: vif can be NULL for injected frames */
+   struct ieee80211_vif *vif;
+
+   /* When packets are enqueued on txq it's easy
+* to re-construct the vif pointer. There's no
+* more space in tx_info so it can be used to
+* store the necessary enqueue time for packet
+* sojourn time computation.
+*/
+   u64 enqueue_time;
+   };
struct ieee80211_key_conf *hw_key;
u32 flags;
/* 4 bytes free */
diff --git a/net/mac80211/codel.h b/net/mac80211/codel.h
new file mode 100644
index ..63ccedcbce04
--- /dev/null
+++ b/net/mac80211/codel.h
@@ -0,0 +1,265 @@
+#ifndef __NET_MAC80211_CODEL_H
+#define __NET_MAC80211_CODEL_H
+
+/*
+ * Codel - The Controlled-Delay Active Queue Management algorithm
+ *
+ *  Copyright (C) 2011-2012 Kathleen Nichols <nich...@pollere.com>
+ *  Copyright (C) 2011-2012 Van Jacobson <v...@pollere.net>
+ *  Copyright (C) 2016 Michael D. Taht <dave.t...@bufferbloat.net>
+ *  Copyright (C) 2012 Eric Dumazet <eduma...@google.com>
+ *  Copyright (C) 2015 Jonathan Morton <chromati...@gmail.com>
+ *  Copyright (C) 2016 Michal Kazior <michal.kaz...@tieto.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions, and the following disclaimer,
+ *without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. The names of the authors may not be used to endorse or promote products
+ *derived from this software without specific prior written permission.
+ *
+ * Alternatively, provided that this notice is retained in full, this
+ * software may be distributed under the terms of the GNU General
+ * Public License ("GPL") version 2, in which case the provisions of the
+ * GPL apply INSTEAD OF those given above.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "codel_i.h"
+
+/* Controlling Queue Delay (CoDel) algorithm
+ * =
+ * Source : Kathleen Nichols and Van Jacobson
+ *

[PATCHv3 5/5] mac80211: add debug knobs for codel

[Michal Kazior]

This adds a few debugfs entries to make it easier
to test, debug and experiment.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 net/mac80211/debugfs.c | 14 ++
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/tx.c  | 21 ++---
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 5cbaa5872e6b..9088e505fa85 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -132,6 +132,10 @@ DEBUGFS_READONLY_FILE(fq_overlimit, "%u",
  local->fq.overlimit);
 DEBUGFS_READONLY_FILE(fq_collisions, "%u",
  local->fq.collisions);
+DEBUGFS_READONLY_FILE(codel_drop_count, "%u",
+ local->cdrop_count);
+DEBUGFS_READONLY_FILE(codel_ecn_mark, "%u",
+ local->cecn_mark);
 
 DEBUGFS_RW_FILE(fq_limit,
DEBUGFS_RW_EXPR_FQ("%u", >fq.limit),
@@ -139,6 +143,12 @@ DEBUGFS_RW_FILE(fq_limit,
 DEBUGFS_RW_FILE(fq_quantum,
DEBUGFS_RW_EXPR_FQ("%u", >fq.quantum),
"%u", local->fq.quantum);
+DEBUGFS_RW_FILE(codel_interval,
+   DEBUGFS_RW_EXPR_FQ("%llu", >cparams.interval),
+   "%llu", local->cparams.interval);
+DEBUGFS_RW_FILE(codel_target,
+   DEBUGFS_RW_EXPR_FQ("%llu", >cparams.target),
+   "%llu", local->cparams.target);
 
 #ifdef CONFIG_PM
 static ssize_t reset_write(struct file *file, const char __user *user_buf,
@@ -330,6 +340,10 @@ void debugfs_hw_add(struct ieee80211_local *local)
DEBUGFS_ADD(fq_collisions);
DEBUGFS_ADD(fq_limit);
DEBUGFS_ADD(fq_quantum);
+   DEBUGFS_ADD(codel_interval);
+   DEBUGFS_ADD(codel_target);
+   DEBUGFS_ADD(codel_drop_count);
+   DEBUGFS_ADD(codel_ecn_mark);
 
statsd = debugfs_create_dir("statistics", phyd);
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 78953b495a25..7aecb7b6528c 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -,6 +,8 @@ struct ieee80211_local {
struct fq fq;
struct codel_vars *cvars;
struct codel_params cparams;
+   unsigned int cdrop_count;
+   unsigned int cecn_mark;
 
const struct ieee80211_ops *ops;
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 238cb8e979fd..b5506411b8e6 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1314,6 +1314,7 @@ static void codel_drop_fn(void *ctx,
local = vif_to_sdata(txqi->txq.vif)->local;
hw = >hw;
 
+   local->cdrop_count++;
ieee80211_free_txskb(hw, skb);
 }
 
@@ -1325,6 +1326,8 @@ static struct sk_buff *fq_tin_dequeue_fn(struct fq *fq,
struct txq_info *txqi;
struct codel_vars *cvars;
struct codel_params *cparams;
+   struct sk_buff *skb;
+   u16 ecn_mark;
bool overloaded;
 
local = container_of(fq, struct ieee80211_local, fq);
@@ -1339,13 +1342,17 @@ static struct sk_buff *fq_tin_dequeue_fn(struct fq *fq,
/* TODO */
overloaded = false;
 
-   return codel_dequeue(txqi,
->backlog,
-0,
-cvars,
-cparams,
-codel_get_time(),
-overloaded);
+   ecn_mark = cvars->ecn_mark;
+   skb = codel_dequeue(txqi,
+   >backlog,
+   0,
+   cvars,
+   cparams,
+   codel_get_time(),
+   overloaded);
+   local->cecn_mark += cvars->ecn_mark - ecn_mark;
+
+   return skb;
 }
 
 static void fq_skb_free_fn(struct fq *fq,
-- 
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

[PATCHv3 2/5] mac80211: implement fair queueing per txq

[Michal Kazior]

mac80211's software queues were designed to work
very closely with device tx queues. They are
required to make use of 802.11 packet aggregation
easily and efficiently.

Due to the way 802.11 aggregation is designed it
only makes sense to keep fair queuing as close to
hardware as possible to reduce induced latency and
inertia and provide the best flow responsivness.

This change doesn't translate directly to
immediate and significant gains. End result
depends on driver's induced latency. Best results
can be achieved if driver keeps its own tx
queue/fifo fill level to a minimum.

Signed-off-by: Michal Kazior <michal.kaz...@tieto.com>
---
 net/mac80211/agg-tx.c  |   8 +-
 net/mac80211/fq.h  | 265 +
 net/mac80211/fq_i.h|  75 +
 net/mac80211/ieee80211_i.h |  25 -
 net/mac80211/iface.c   |  12 +-
 net/mac80211/main.c|   7 ++
 net/mac80211/rx.c  |   2 +-
 net/mac80211/sta_info.c|  14 +--
 net/mac80211/tx.c  | 110 ---
 net/mac80211/util.c|  23 +---
 10 files changed, 481 insertions(+), 60 deletions(-)
 create mode 100644 net/mac80211/fq.h
 create mode 100644 net/mac80211/fq_i.h

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 4932e9f243a2..908ac84a1962 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -194,17 +194,21 @@ static void
 ieee80211_agg_stop_txq(struct sta_info *sta, int tid)
 {
struct ieee80211_txq *txq = sta->sta.txq[tid];
+   struct ieee80211_sub_if_data *sdata;
+   struct fq *fq;
struct txq_info *txqi;
 
if (!txq)
return;
 
txqi = to_txq_info(txq);
+   sdata = vif_to_sdata(txq->vif);
+   fq = >local->fq;
 
/* Lock here to protect against further seqno updates on dequeue */
-   spin_lock_bh(>queue.lock);
+   spin_lock_bh(>lock);
set_bit(IEEE80211_TXQ_STOP, >flags);
-   spin_unlock_bh(>queue.lock);
+   spin_unlock_bh(>lock);
 }
 
 static void
diff --git a/net/mac80211/fq.h b/net/mac80211/fq.h
new file mode 100644
index ..fa98576e1825
--- /dev/null
+++ b/net/mac80211/fq.h
@@ -0,0 +1,265 @@
+/*
+ * Copyright (c) 2016 Qualcomm Atheros, Inc
+ *
+ * GPL v2
+ *
+ * Based on net/sched/sch_fq_codel.c
+ */
+#ifndef FQ_H
+#define FQ_H
+
+#include "fq_i.h"
+
+/* forward declarations the includer must implement */
+
+static struct sk_buff *fq_tin_dequeue_fn(struct fq *,
+struct fq_tin *,
+struct fq_flow *flow);
+
+static void fq_skb_free_fn(struct fq *,
+  struct fq_tin *,
+  struct fq_flow *,
+  struct sk_buff *);
+
+static struct fq_flow *fq_flow_get_default_fn(struct fq *,
+ struct fq_tin *,
+ int idx,
+ struct sk_buff *);
+
+/* functions that are embedded into includer */
+
+static struct sk_buff *fq_flow_dequeue(struct fq *fq,
+  struct fq_flow *flow)
+{
+   struct fq_tin *tin = flow->tin;
+   struct fq_flow *i;
+   struct sk_buff *skb;
+
+   lockdep_assert_held(>lock);
+
+   skb = __skb_dequeue(>queue);
+   if (!skb)
+   return NULL;
+
+   tin->backlog_bytes -= skb->len;
+   tin->backlog_packets--;
+   flow->backlog -= skb->len;
+   fq->backlog--;
+
+   if (flow->backlog == 0) {
+   list_del_init(>backlogchain);
+   } else {
+   i = flow;
+
+   list_for_each_entry_continue(i, >backlogs, backlogchain)
+   if (i->backlog < flow->backlog)
+   break;
+
+   list_move_tail(>backlogchain,
+  >backlogchain);
+   }
+
+   return skb;
+}
+
+static struct sk_buff *fq_tin_dequeue(struct fq *fq,
+ struct fq_tin *tin)
+{
+   struct fq_flow *flow;
+   struct list_head *head;
+   struct sk_buff *skb;
+
+   lockdep_assert_held(>lock);
+
+begin:
+   head = >new_flows;
+   if (list_empty(head)) {
+   head = >old_flows;
+   if (list_empty(head))
+   return NULL;
+   }
+
+   flow = list_first_entry(head, struct fq_flow, flowchain);
+
+   if (flow->deficit <= 0) {
+   flow->deficit += fq->quantum;
+   list_move_tail(>flowchain,
+  >old_flows);
+   goto begin;
+   }
+
+   skb = fq_tin_dequeue_fn(fq, tin, flow);
+   if (!skb) {
+   /* force a pass through old_flows to prevent starvation */
+   if ((head == &g