2013-08-05 19:56:32

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 01/12] ath9k: add utility functions for accessing tid queues

Useful for further fixes / cleanups

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 51 ++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 52cd521..ccc146c 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -168,6 +168,16 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
}
}

+static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
+{
+ return !skb_queue_empty(&tid->buf_q);
+}
+
+static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
+{
+ return __skb_dequeue(&tid->buf_q);
+}
+
static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
{
struct ath_txq *txq = tid->ac->txq;
@@ -182,7 +192,7 @@ static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)

memset(&ts, 0, sizeof(ts));

- while ((skb = __skb_dequeue(&tid->buf_q))) {
+ while ((skb = ath_tid_dequeue(tid))) {
fi = get_frame_info(skb);
bf = fi->bf;

@@ -266,7 +276,7 @@ static void ath_tid_drain(struct ath_softc *sc, struct ath_txq *txq,
memset(&ts, 0, sizeof(ts));
INIT_LIST_HEAD(&bf_head);

- while ((skb = __skb_dequeue(&tid->buf_q))) {
+ while ((skb = ath_tid_dequeue(tid))) {
fi = get_frame_info(skb);
bf = fi->bf;

@@ -815,7 +825,7 @@ static int ath_compute_num_delims(struct ath_softc *sc, struct ath_atx_tid *tid,

static struct ath_buf *
ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+ struct ath_atx_tid *tid, struct sk_buff_head **q)
{
struct ath_frame_info *fi;
struct sk_buff *skb;
@@ -823,7 +833,8 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
u16 seqno;

while (1) {
- skb = skb_peek(&tid->buf_q);
+ *q = &tid->buf_q;
+ skb = skb_peek(*q);
if (!skb)
break;

@@ -833,7 +844,7 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
bf = ath_tx_setup_buffer(sc, txq, tid, skb);

if (!bf) {
- __skb_unlink(skb, &tid->buf_q);
+ __skb_unlink(skb, *q);
ath_txq_skb_done(sc, txq, skb);
ieee80211_free_txskb(sc->hw, skb);
continue;
@@ -852,7 +863,7 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,

INIT_LIST_HEAD(&bf_head);
list_add(&bf->list, &bf_head);
- __skb_unlink(skb, &tid->buf_q);
+ __skb_unlink(skb, *q);
ath_tx_update_baw(sc, tid, seqno);
ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0);
continue;
@@ -881,9 +892,10 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
struct sk_buff *skb;
+ struct sk_buff_head *tid_q;

do {
- bf = ath_tx_get_tid_subframe(sc, txq, tid);
+ bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
if (!bf) {
status = ATH_AGGR_BAW_CLOSED;
break;
@@ -940,14 +952,14 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
ath_tx_addto_baw(sc, tid, bf->bf_state.seqno);
bf->bf_state.ndelim = ndelim;

- __skb_unlink(skb, &tid->buf_q);
+ __skb_unlink(skb, tid_q);
list_add_tail(&bf->list, bf_q);
if (bf_prev)
bf_prev->bf_next = bf;

bf_prev = bf;

- } while (!skb_queue_empty(&tid->buf_q));
+ } while (ath_tid_has_buffered(tid));

*aggr_len = al;

@@ -1250,7 +1262,7 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
int aggr_len;

do {
- if (skb_queue_empty(&tid->buf_q))
+ if (!ath_tid_has_buffered(tid))
return;

INIT_LIST_HEAD(&bf_q);
@@ -1354,7 +1366,7 @@ void ath_tx_aggr_sleep(struct ieee80211_sta *sta, struct ath_softc *sc,

ath_txq_lock(sc, txq);

- buffered = !skb_queue_empty(&tid->buf_q);
+ buffered = ath_tid_has_buffered(tid);

tid->sched = false;
list_del(&tid->list);
@@ -1386,7 +1398,7 @@ void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
ath_txq_lock(sc, txq);
ac->clear_ps_filter = true;

- if (!skb_queue_empty(&tid->buf_q) && !tid->paused) {
+ if (!tid->paused && ath_tid_has_buffered(tid)) {
ath_tx_queue_tid(txq, tid);
ath_txq_schedule(sc, txq);
}
@@ -1411,7 +1423,7 @@ void ath_tx_aggr_resume(struct ath_softc *sc, struct ieee80211_sta *sta,
tid->baw_size = IEEE80211_MIN_AMPDU_BUF << sta->ht_cap.ampdu_factor;
tid->paused = false;

- if (!skb_queue_empty(&tid->buf_q)) {
+ if (ath_tid_has_buffered(tid)) {
ath_tx_queue_tid(txq, tid);
ath_txq_schedule(sc, txq);
}
@@ -1431,6 +1443,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
struct ieee80211_tx_info *info;
struct list_head bf_q;
struct ath_buf *bf_tail = NULL, *bf;
+ struct sk_buff_head *tid_q;
int sent = 0;
int i;

@@ -1446,12 +1459,12 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
continue;

ath_txq_lock(sc, tid->ac->txq);
- while (!skb_queue_empty(&tid->buf_q) && nframes > 0) {
- bf = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq, tid);
+ while (nframes > 0) {
+ bf = ath_tx_get_tid_subframe(sc, sc->tx.uapsdq, tid, &tid_q);
if (!bf)
break;

- __skb_unlink(bf->bf_mpdu, &tid->buf_q);
+ __skb_unlink(bf->bf_mpdu, tid_q);
list_add_tail(&bf->list, &bf_q);
ath_set_rates(tid->an->vif, tid->an->sta, bf);
ath_tx_addto_baw(sc, tid, bf->bf_state.seqno);
@@ -1464,7 +1477,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
sent++;
TX_STAT_INC(txq->axq_qnum, a_queued_hw);

- if (skb_queue_empty(&tid->buf_q))
+ if (ath_tid_has_buffered(tid))
ieee80211_sta_set_buffered(an->sta, i, false);
}
ath_txq_unlock_complete(sc, tid->ac->txq);
@@ -1750,7 +1763,7 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
* add tid to round-robin queue if more frames
* are pending for the tid
*/
- if (!skb_queue_empty(&tid->buf_q))
+ if (ath_tid_has_buffered(tid))
ath_tx_queue_tid(txq, tid);

if (tid == last_tid ||
@@ -1859,7 +1872,7 @@ static void ath_tx_send_ampdu(struct ath_softc *sc, struct ath_txq *txq,
* - seqno is not within block-ack window
* - h/w queue depth exceeds low water mark
*/
- if ((!skb_queue_empty(&tid->buf_q) || tid->paused ||
+ if ((ath_tid_has_buffered(tid) || tid->paused ||
!BAW_WITHIN(tid->seq_start, tid->baw_size, tid->seq_next) ||
txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) &&
txq != sc->tx.uapsdq) {
--
1.8.0.2



2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 07/12] ath9k: prepare queueing code for handling unaggregated traffic

- Allow ath_tx_get_tid_subframe to return non-AMPDU subframes.
- Reset the tid paused state on aggregation stop
- Initialize software queues even when HT is not supported

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/main.c | 3 ---
drivers/net/wireless/ath/ath9k/xmit.c | 20 ++++++++++++++------
2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 1adb803..4da9864 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1374,9 +1374,6 @@ static void ath9k_sta_notify(struct ieee80211_hw *hw,
struct ath_softc *sc = hw->priv;
struct ath_node *an = (struct ath_node *) sta->drv_priv;

- if (!sta->ht_cap.ht_supported)
- return;
-
switch (cmd) {
case STA_NOTIFY_SLEEP:
an->sleeping = true;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 6c50249..e4d3723 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -672,7 +672,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
} else
ath_tx_complete_aggr(sc, txq, bf, bf_head, ts, txok);

- if ((sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) && !flush)
+ if (!flush)
ath_txq_schedule(sc, txq);
}

@@ -848,6 +848,7 @@ static struct ath_buf *
ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
struct ath_atx_tid *tid, struct sk_buff_head **q)
{
+ struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
struct sk_buff *skb;
struct ath_buf *bf;
@@ -874,6 +875,16 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
continue;
}

+ bf->bf_next = NULL;
+ bf->bf_lastbf = bf;
+
+ tx_info = IEEE80211_SKB_CB(skb);
+ tx_info->flags &= ~IEEE80211_TX_CTL_CLEAR_PS_FILT;
+ if (!(tx_info->flags & IEEE80211_TX_CTL_AMPDU)) {
+ bf->bf_state.bf_type = 0;
+ return bf;
+ }
+
bf->bf_state.bf_type = BUF_AMPDU | BUF_AGGR;
seqno = bf->bf_state.seqno;

@@ -893,8 +904,6 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
continue;
}

- bf->bf_next = NULL;
- bf->bf_lastbf = bf;
return bf;
}

@@ -1356,7 +1365,7 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)

ath_txq_lock(sc, txq);
txtid->active = false;
- txtid->paused = true;
+ txtid->paused = false;
ath_tx_flush_tid(sc, txtid);
ath_txq_unlock_complete(sc, txq);
}
@@ -2425,8 +2434,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)

if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
- if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT)
- ath_txq_schedule(sc, txq);
+ ath_txq_schedule(sc, txq);
break;
}
bf = list_first_entry(&txq->axq_q, struct ath_buf, list);
--
1.8.0.2


2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 03/12] ath9k: add function for getting the tx tid for a packet

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index ac6bd50..e54c3e5 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -168,6 +168,20 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
}
}

+static struct ath_atx_tid *
+ath_get_skb_tid(struct ath_softc *sc, struct ath_node *an, struct sk_buff *skb)
+{
+ struct ieee80211_hdr *hdr;
+ u8 tidno = 0;
+
+ hdr = (struct ieee80211_hdr *) skb->data;
+ if (ieee80211_is_data_qos(hdr->frame_control))
+ tidno = ieee80211_get_qos_ctl(hdr)[0];
+
+ tidno &= IEEE80211_QOS_CTL_TID_MASK;
+ return ATH_AN_2_TID(an, tidno);
+}
+
static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
{
return !skb_queue_empty(&tid->buf_q) || !skb_queue_empty(&tid->retry_q);
@@ -419,7 +433,6 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
struct ieee80211_tx_rate rates[4];
struct ath_frame_info *fi;
int nframes;
- u8 tidno;
bool flush = !!(ts->ts_status & ATH9K_TX_FLUSH);
int i, retries;
int bar_index = -1;
@@ -456,8 +469,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
}

an = (struct ath_node *)sta->drv_priv;
- tidno = ieee80211_get_qos_ctl(hdr)[0] & IEEE80211_QOS_CTL_TID_MASK;
- tid = ATH_AN_2_TID(an, tidno);
+ tid = ath_get_skb_tid(sc, an, skb);
seq_first = tid->seq_start;
isba = ts->ts_flags & ATH9K_TX_BA;

@@ -469,7 +481,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
* Only BlockAcks have a TID and therefore normal Acks cannot be
* checked
*/
- if (isba && tidno != ts->tid)
+ if (isba && tid->tidno != ts->tid)
txok = false;

isaggr = bf_isaggr(bf);
@@ -2116,7 +2128,6 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
struct ath_txq *txq = txctl->txq;
struct ath_atx_tid *tid = NULL;
struct ath_buf *bf;
- u8 tidno;
int q;
int ret;

@@ -2147,9 +2158,7 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
}

if (txctl->an && ieee80211_is_data_qos(hdr->frame_control)) {
- tidno = ieee80211_get_qos_ctl(hdr)[0] &
- IEEE80211_QOS_CTL_TID_MASK;
- tid = ATH_AN_2_TID(txctl->an, tidno);
+ tid = ath_get_skb_tid(sc, txctl->an, skb);

WARN_ON(tid->ac->txq != txctl->txq);
}
--
1.8.0.2


2013-08-06 08:57:13

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 07/12] ath9k: prepare queueing code for handling unaggregated traffic

Felix Fietkau wrote:
> + tx_info = IEEE80211_SKB_CB(skb);
> + tx_info->flags &= ~IEEE80211_TX_CTL_CLEAR_PS_FILT;

Why is the PS filter flag cleared here ?

> - txtid->paused = true;
> + txtid->paused = false;

Why change this ?

Sujith

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 06/12] ath9k: fix block ack window tracking check

When a packet has been tracked as part of the BlockAck window and added
to the hardware queue, it can end up back in the TID queue again with
fi->retries still set to 0 (e.g. if the frame was filtered). Keep an
extra bit for the BAW tracking status to fix this corner case.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 3 ++-
drivers/net/wireless/ath/ath9k/xmit.c | 15 +++++++++------
2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 88ef785..f9be2b2 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -211,8 +211,9 @@ struct ath_frame_info {
int framelen;
enum ath9k_key_type keytype;
u8 keyix;
- u8 retries;
u8 rtscts_rate;
+ u8 retries : 7;
+ u8 baw_tracked : 1;
};

struct ath_buf_state {
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 8ce2d36..6c50249 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -225,7 +225,7 @@ static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
}
}

- if (fi->retries) {
+ if (fi->baw_tracked) {
list_add_tail(&bf->list, &bf_head);
ath_tx_update_baw(sc, tid, bf->bf_state.seqno);
ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0);
@@ -262,13 +262,16 @@ static void ath_tx_update_baw(struct ath_softc *sc, struct ath_atx_tid *tid,
}

static void ath_tx_addto_baw(struct ath_softc *sc, struct ath_atx_tid *tid,
- u16 seqno)
+ struct ath_buf *bf)
{
+ struct ath_frame_info *fi = get_frame_info(bf->bf_mpdu);
+ u16 seqno = bf->bf_state.seqno;
int index, cindex;

index = ATH_BA_INDEX(tid->seq_start, seqno);
cindex = (tid->baw_head + index) & (ATH_TID_MAX_BUFS - 1);
__set_bit(cindex, tid->tx_buf);
+ fi->baw_tracked = 1;

if (index >= ((tid->baw_tail - tid->baw_head) &
(ATH_TID_MAX_BUFS - 1))) {
@@ -960,8 +963,8 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
bf->bf_next = NULL;

/* link buffers of this frame to the aggregate */
- if (!fi->retries)
- ath_tx_addto_baw(sc, tid, bf->bf_state.seqno);
+ if (!fi->baw_tracked)
+ ath_tx_addto_baw(sc, tid, bf);
bf->bf_state.ndelim = ndelim;

__skb_unlink(skb, tid_q);
@@ -1479,7 +1482,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
__skb_unlink(bf->bf_mpdu, tid_q);
list_add_tail(&bf->list, &bf_q);
ath_set_rates(tid->an->vif, tid->an->sta, bf);
- ath_tx_addto_baw(sc, tid, bf->bf_state.seqno);
+ ath_tx_addto_baw(sc, tid, bf);
bf->bf_state.bf_type &= ~BUF_AGGR;
if (bf_tail)
bf_tail->bf_next = bf;
@@ -1912,7 +1915,7 @@ static void ath_tx_send_ampdu(struct ath_softc *sc, struct ath_txq *txq,
list_add(&bf->list, &bf_head);

/* Add sub-frame to BAW */
- ath_tx_addto_baw(sc, tid, bf->bf_state.seqno);
+ ath_tx_addto_baw(sc, tid, bf);

/* Queue to h/w without aggregation */
TX_STAT_INC(txq->axq_qnum, a_queued_hw);
--
1.8.0.2


2013-08-06 08:57:25

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 10/12] ath9k: use software queues for un-aggregated data packets

Felix Fietkau wrote:
> + aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
> + if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
> + (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH))
> + break;
> +
> + ath_set_rates(tid->an->vif, tid->an->sta, bf);
> + if (aggr)
> + last = ath_tx_form_aggr(sc, txq, tid, &bf_q, bf,
> + tid_q, &aggr_len);
> + else
> + ath_tx_form_burst(sc, txq, tid, &bf_q, bf, tid_q);
> +
> + if (list_empty(&bf_q))
> + return;

Handling non-AMPDU and AMPDU packets in the same path makes the code hard
to follow. Since a TID can either have aggregated or unaggregated packets,
why not separate the logic early, maybe in the schedule() function ?

Sujith

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 12/12] ath9k: use software queueing for multicast traffic

Create a per-vif dummy node entry for keeping the multicast software
queues. This helps in setups with a lot of mulitcast traffic that could
otherwise potentially drown out unicast traffic to stations.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 ++
drivers/net/wireless/ath/ath9k/main.c | 11 +++++++++++
drivers/net/wireless/ath/ath9k/xmit.c | 12 ++++++++++--
3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 162fa59..98a8b80 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -265,6 +265,7 @@ struct ath_node {
u8 mpdudensity;

bool sleeping;
+ bool no_ps_filter;

#if defined(CONFIG_MAC80211_DEBUGFS) && defined(CONFIG_ATH9K_DEBUGFS)
struct dentry *node_stat;
@@ -364,6 +365,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
/********/

struct ath_vif {
+ struct ath_node mcast_node;
int av_bslot;
bool primary_sta_vif;
__le64 tsf_adjust; /* TSF adjustment for staggered beacons */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4da9864..5b93cfe 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -966,6 +966,8 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
struct ath_softc *sc = hw->priv;
struct ath_hw *ah = sc->sc_ah;
struct ath_common *common = ath9k_hw_common(ah);
+ struct ath_vif *avp = (void *)vif->drv_priv;
+ struct ath_node *an = &avp->mcast_node;

mutex_lock(&sc->mutex);

@@ -979,6 +981,12 @@ static int ath9k_add_interface(struct ieee80211_hw *hw,
if (ath9k_uses_beacons(vif->type))
ath9k_beacon_assign_slot(sc, vif);

+ an->sc = sc;
+ an->sta = NULL;
+ an->vif = vif;
+ an->no_ps_filter = true;
+ ath_tx_node_init(sc, an);
+
mutex_unlock(&sc->mutex);
return 0;
}
@@ -1016,6 +1024,7 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
{
struct ath_softc *sc = hw->priv;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
+ struct ath_vif *avp = (void *)vif->drv_priv;

ath_dbg(common, CONFIG, "Detach Interface\n");

@@ -1030,6 +1039,8 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
ath9k_calculate_summary_state(hw, NULL);
ath9k_ps_restore(sc);

+ ath_tx_node_cleanup(sc, &avp->mcast_node);
+
mutex_unlock(&sc->mutex);
}

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 274fe3f..c383243 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -135,6 +135,9 @@ static struct ath_frame_info *get_frame_info(struct sk_buff *skb)

static void ath_send_bar(struct ath_atx_tid *tid, u16 seqno)
{
+ if (!tid->an->sta)
+ return;
+
ieee80211_send_bar(tid->an->vif, tid->an->sta->addr, tid->tidno,
seqno << IEEE80211_SEQ_SEQ_SHIFT);
}
@@ -1380,7 +1383,7 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
if (list_empty(&bf_q))
return false;

- if (tid->ac->clear_ps_filter) {
+ if (tid->ac->clear_ps_filter || tid->an->no_ps_filter) {
tid->ac->clear_ps_filter = false;
tx_info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
}
@@ -1570,7 +1573,7 @@ void ath9k_release_buffered_frames(struct ieee80211_hw *hw,
sent++;
TX_STAT_INC(txq->axq_qnum, a_queued_hw);

- if (ath_tid_has_buffered(tid))
+ if (an->sta && ath_tid_has_buffered(tid))
ieee80211_sta_set_buffered(an->sta, i, false);
}
ath_txq_unlock_complete(sc, tid->ac->txq);
@@ -2103,6 +2106,7 @@ static int ath_tx_prepare(struct ieee80211_hw *hw, struct sk_buff *skb,
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_sta *sta = txctl->sta;
struct ieee80211_vif *vif = info->control.vif;
+ struct ath_vif *avp;
struct ath_softc *sc = hw->priv;
int frmlen = skb->len + FCS_LEN;
int padpos, padsize;
@@ -2110,6 +2114,10 @@ static int ath_tx_prepare(struct ieee80211_hw *hw, struct sk_buff *skb,
/* NOTE: sta can be NULL according to net/mac80211.h */
if (sta)
txctl->an = (struct ath_node *)sta->drv_priv;
+ else if (vif && ieee80211_is_data(hdr->frame_control)) {
+ avp = (void *)vif->drv_priv;
+ txctl->an = &avp->mcast_node;
+ }

if (info->control.hw_key)
frmlen += info->control.hw_key->icv_len;
--
1.8.0.2


2013-08-06 08:57:30

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 11/12] ath9k: improve tx scheduling fairness

Felix Fietkau wrote:
> Instead of trying to schedule the same TID multiple times in a loop,
> iterate over other TIDs/stations first.

The txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH check in the beginning
of txq_schedule() is not really correct after this, no ?

Sujith

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 02/12] ath9k: split tid retry packets into a separate queue

Improves packet retry order and helps with further tx queueing
improvements.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/xmit.c | 18 ++++++++++++++----
2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index ed8f8ef..919c8ee 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -241,6 +241,7 @@ struct ath_buf {
struct ath_atx_tid {
struct list_head list;
struct sk_buff_head buf_q;
+ struct sk_buff_head retry_q;
struct ath_node *an;
struct ath_atx_ac *ac;
unsigned long tx_buf[BITS_TO_LONGS(ATH_TID_MAX_BUFS)];
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index ccc146c..ac6bd50 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -170,12 +170,18 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,

static bool ath_tid_has_buffered(struct ath_atx_tid *tid)
{
- return !skb_queue_empty(&tid->buf_q);
+ return !skb_queue_empty(&tid->buf_q) || !skb_queue_empty(&tid->retry_q);
}

static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
{
- return __skb_dequeue(&tid->buf_q);
+ struct sk_buff *skb;
+
+ skb = __skb_dequeue(&tid->retry_q);
+ if (!skb)
+ skb = __skb_dequeue(&tid->buf_q);
+
+ return skb;
}

static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
@@ -593,7 +599,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
if (an->sleeping)
ieee80211_sta_set_buffered(sta, tid->tidno, true);

- skb_queue_splice(&bf_pending, &tid->buf_q);
+ skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
ath_tx_queue_tid(txq, tid);

@@ -833,7 +839,10 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
u16 seqno;

while (1) {
- *q = &tid->buf_q;
+ *q = &tid->retry_q;
+ if (skb_queue_empty(*q))
+ *q = &tid->buf_q;
+
skb = skb_peek(*q);
if (!skb)
break;
@@ -2636,6 +2645,7 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
tid->paused = false;
tid->active = false;
__skb_queue_head_init(&tid->buf_q);
+ __skb_queue_head_init(&tid->retry_q);
acno = TID_TO_WME_AC(tidno);
tid->ac = &an->ac[acno];
}
--
1.8.0.2


2013-08-05 19:56:32

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 10/12] ath9k: use software queues for un-aggregated data packets

This is a first step for improving fairness between legacy and 802.11n
traffic, and it should also improve reliability of resets and channel
changes by keeping the hardware queue depth very short.

When an aggregation session is torn down, all packets in the retry queue
will be removed from the BAW and freed.

For all subframes that have not been transmitted yet, the A-MPDU flag
will be cleared, and a sequence number allocated. This ensures that the
next A-MPDU session will get the correct initial sequence number.
This happens both on aggregation session start and stop.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 8 +-
drivers/net/wireless/ath/ath9k/xmit.c | 264 ++++++++++++++++++---------------
2 files changed, 145 insertions(+), 127 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index f9be2b2..162fa59 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -137,6 +137,8 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_AGGR_ENCRYPTDELIM 10
/* minimum h/w qdepth to be sustained to maximize aggregation */
#define ATH_AGGR_MIN_QDEPTH 2
+/* minimum h/w qdepth for non-aggregated traffic */
+#define ATH_NON_AGGR_MIN_QDEPTH 8

#define IEEE80211_SEQ_SEQ_SHIFT 4
#define IEEE80211_SEQ_MAX 4096
@@ -173,12 +175,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,

#define ATH_TX_COMPLETE_POLL_INT 1000

-enum ATH_AGGR_STATUS {
- ATH_AGGR_DONE,
- ATH_AGGR_BAW_CLOSED,
- ATH_AGGR_LIMITED,
-};
-
#define ATH_TXFIFO_DEPTH 8
struct ath_txq {
int mac80211_qnum; /* mac80211 queue number, -1 means not mac80211 Q */
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 11202ea..037cc91 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -198,6 +198,41 @@ static struct sk_buff *ath_tid_dequeue(struct ath_atx_tid *tid)
return skb;
}

+/*
+ * ath_tx_tid_change_state:
+ * - clears a-mpdu flag of previous session
+ * - force sequence number allocation to fix next BlockAck Window
+ */
+static void
+ath_tx_tid_change_state(struct ath_softc *sc, struct ath_atx_tid *tid)
+{
+ struct ath_txq *txq = tid->ac->txq;
+ struct ieee80211_tx_info *tx_info;
+ struct sk_buff *skb, *tskb;
+ struct ath_buf *bf;
+ struct ath_frame_info *fi;
+
+ skb_queue_walk_safe(&tid->buf_q, skb, tskb) {
+ fi = get_frame_info(skb);
+ bf = fi->bf;
+
+ tx_info = IEEE80211_SKB_CB(skb);
+ tx_info->flags &= ~IEEE80211_TX_CTL_AMPDU;
+
+ if (bf)
+ continue;
+
+ bf = ath_tx_setup_buffer(sc, txq, tid, skb);
+ if (!bf) {
+ __skb_unlink(skb, &tid->buf_q);
+ ath_txq_skb_done(sc, txq, skb);
+ ieee80211_free_txskb(sc->hw, skb);
+ continue;
+ }
+ }
+
+}
+
static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
{
struct ath_txq *txq = tid->ac->txq;
@@ -212,28 +247,22 @@ static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)

memset(&ts, 0, sizeof(ts));

- while ((skb = ath_tid_dequeue(tid))) {
+ while ((skb = __skb_dequeue(&tid->retry_q))) {
fi = get_frame_info(skb);
bf = fi->bf;
-
if (!bf) {
- bf = ath_tx_setup_buffer(sc, txq, tid, skb);
- if (!bf) {
- ath_txq_skb_done(sc, txq, skb);
- ieee80211_free_txskb(sc->hw, skb);
- continue;
- }
+ ath_txq_skb_done(sc, txq, skb);
+ ieee80211_free_txskb(sc->hw, skb);
+ continue;
}

if (fi->baw_tracked) {
- list_add_tail(&bf->list, &bf_head);
ath_tx_update_baw(sc, tid, bf->bf_state.seqno);
- ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0);
sendbar = true;
- } else {
- ath_set_rates(tid->an->vif, tid->an->sta, bf);
- ath_tx_send_normal(sc, txq, NULL, skb);
}
+
+ list_add_tail(&bf->list, &bf_head);
+ ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0);
}

if (sendbar) {
@@ -911,50 +940,39 @@ ath_tx_get_tid_subframe(struct ath_softc *sc, struct ath_txq *txq,
return NULL;
}

-static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
- struct ath_txq *txq,
- struct ath_atx_tid *tid,
- struct list_head *bf_q,
- int *aggr_len)
+static bool
+ath_tx_form_aggr(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid, struct list_head *bf_q,
+ struct ath_buf *bf_first, struct sk_buff_head *tid_q,
+ int *aggr_len)
{
#define PADBYTES(_len) ((4 - ((_len) % 4)) % 4)
- struct ath_buf *bf, *bf_first = NULL, *bf_prev = NULL;
+ struct ath_buf *bf = bf_first, *bf_prev = NULL;
int nframes = 0, ndelim;
u16 aggr_limit = 0, al = 0, bpad = 0,
al_delta, h_baw = tid->baw_size / 2;
- enum ATH_AGGR_STATUS status = ATH_AGGR_DONE;
struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
struct sk_buff *skb;
- struct sk_buff_head *tid_q;
+ bool closed = false;

- do {
- bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
- if (!bf) {
- status = ATH_AGGR_BAW_CLOSED;
- break;
- }
+ bf = bf_first;
+ aggr_limit = ath_lookup_rate(sc, bf, tid);

+ do {
skb = bf->bf_mpdu;
fi = get_frame_info(skb);

- if (!bf_first) {
- bf_first = bf;
- ath_set_rates(tid->an->vif, tid->an->sta, bf);
- aggr_limit = ath_lookup_rate(sc, bf, tid);
- }
-
/* do not exceed aggregation limit */
al_delta = ATH_AGGR_DELIM_SZ + fi->framelen;
if (nframes) {
if (aggr_limit < al + bpad + al_delta ||
- ath_lookup_legacy(bf) || nframes >= h_baw) {
- status = ATH_AGGR_LIMITED;
+ ath_lookup_legacy(bf) || nframes >= h_baw)
break;
- }

tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
- if (tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)
+ if ((tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE) ||
+ !(tx_info->flags & IEEE80211_TX_CTL_AMPDU))
break;
}

@@ -984,11 +1002,26 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,

bf_prev = bf;

+ bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
+ if (!bf) {
+ closed = true;
+ break;
+ }
} while (ath_tid_has_buffered(tid));

+ bf = bf_first;
+ bf->bf_lastbf = bf_prev;
+
+ if (bf == bf_prev) {
+ al = get_frame_info(bf->bf_mpdu)->framelen;
+ bf->bf_state.bf_type = BUF_AMPDU;
+ } else {
+ TX_STAT_INC(txq->axq_qnum, a_aggr);
+ }
+
*aggr_len = al;

- return status;
+ return closed;
#undef PADBYTES
}

@@ -1277,14 +1310,50 @@ static void ath_tx_fill_desc(struct ath_softc *sc, struct ath_buf *bf,
}
}

+static void
+ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid, struct list_head *bf_q,
+ struct ath_buf *bf_first, struct sk_buff_head *tid_q)
+{
+ struct ath_buf *bf = bf_first, *bf_prev = NULL;
+ struct sk_buff *skb;
+ int nframes = 0;
+
+ do {
+ struct ieee80211_tx_info *tx_info;
+ skb = bf->bf_mpdu;
+
+ nframes++;
+ __skb_unlink(skb, tid_q);
+ list_add_tail(&bf->list, bf_q);
+ if (bf_prev)
+ bf_prev->bf_next = bf;
+ bf_prev = bf;
+
+ if (nframes >= 2)
+ break;
+
+ bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
+ if (!bf)
+ break;
+
+ tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
+ if (tx_info->flags & IEEE80211_TX_CTL_AMPDU)
+ break;
+
+ ath_set_rates(tid->an->vif, tid->an->sta, bf);
+ } while (1);
+}
+
static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
struct ath_atx_tid *tid)
{
struct ath_buf *bf;
- enum ATH_AGGR_STATUS status;
struct ieee80211_tx_info *tx_info;
+ struct sk_buff_head *tid_q;
struct list_head bf_q;
- int aggr_len;
+ int aggr_len = 0;
+ bool aggr, last = true;

do {
if (!ath_tid_has_buffered(tid))
@@ -1292,38 +1361,34 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,

INIT_LIST_HEAD(&bf_q);

- status = ath_tx_form_aggr(sc, txq, tid, &bf_q, &aggr_len);
-
- /*
- * no frames picked up to be aggregated;
- * block-ack window is not open.
- */
- if (list_empty(&bf_q))
+ bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
+ if (!bf)
break;

- bf = list_first_entry(&bf_q, struct ath_buf, list);
- bf->bf_lastbf = list_entry(bf_q.prev, struct ath_buf, list);
tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
+ aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
+ if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
+ (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH))
+ break;
+
+ ath_set_rates(tid->an->vif, tid->an->sta, bf);
+ if (aggr)
+ last = ath_tx_form_aggr(sc, txq, tid, &bf_q, bf,
+ tid_q, &aggr_len);
+ else
+ ath_tx_form_burst(sc, txq, tid, &bf_q, bf, tid_q);
+
+ if (list_empty(&bf_q))
+ return;

if (tid->ac->clear_ps_filter) {
tid->ac->clear_ps_filter = false;
tx_info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
- } else {
- tx_info->flags &= ~IEEE80211_TX_CTL_CLEAR_PS_FILT;
- }
-
- /* if only one frame, send as non-aggregate */
- if (bf == bf->bf_lastbf) {
- aggr_len = get_frame_info(bf->bf_mpdu)->framelen;
- bf->bf_state.bf_type = BUF_AMPDU;
- } else {
- TX_STAT_INC(txq->axq_qnum, a_aggr);
}

ath_tx_fill_desc(sc, bf, txq, aggr_len);
ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- } while (txq->axq_ampdu_depth < ATH_AGGR_MIN_QDEPTH &&
- status != ATH_AGGR_BAW_CLOSED);
+ } while (!last);
}

int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1347,6 +1412,9 @@ int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
an->mpdudensity = density;
}

+ /* force sequence number allocation for pending frames */
+ ath_tx_tid_change_state(sc, txtid);
+
txtid->active = true;
txtid->paused = true;
*ssn = txtid->seq_start = txtid->seq_next;
@@ -1368,6 +1436,7 @@ void ath_tx_aggr_stop(struct ath_softc *sc, struct ieee80211_sta *sta, u16 tid)
txtid->active = false;
txtid->paused = false;
ath_tx_flush_tid(sc, txtid);
+ ath_tx_tid_change_state(sc, txtid);
ath_txq_unlock_complete(sc, txq);
}

@@ -1882,58 +1951,6 @@ static void ath_tx_txqaddbuf(struct ath_softc *sc, struct ath_txq *txq,
}
}

-static void ath_tx_send_ampdu(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid, struct sk_buff *skb,
- struct ath_tx_control *txctl)
-{
- struct ath_frame_info *fi = get_frame_info(skb);
- struct list_head bf_head;
- struct ath_buf *bf;
-
- /*
- * Do not queue to h/w when any of the following conditions is true:
- * - there are pending frames in software queue
- * - the TID is currently paused for ADDBA/BAR request
- * - seqno is not within block-ack window
- * - h/w queue depth exceeds low water mark
- */
- if ((ath_tid_has_buffered(tid) || tid->paused ||
- !BAW_WITHIN(tid->seq_start, tid->baw_size, tid->seq_next) ||
- txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) &&
- txq != sc->tx.uapsdq) {
- /*
- * Add this frame to software queue for scheduling later
- * for aggregation.
- */
- TX_STAT_INC(txq->axq_qnum, a_queued_sw);
- __skb_queue_tail(&tid->buf_q, skb);
- if (!txctl->an || !txctl->an->sleeping)
- ath_tx_queue_tid(txq, tid);
- return;
- }
-
- bf = ath_tx_setup_buffer(sc, txq, tid, skb);
- if (!bf) {
- ath_txq_skb_done(sc, txq, skb);
- ieee80211_free_txskb(sc->hw, skb);
- return;
- }
-
- ath_set_rates(tid->an->vif, tid->an->sta, bf);
- bf->bf_state.bf_type = BUF_AMPDU;
- INIT_LIST_HEAD(&bf_head);
- list_add(&bf->list, &bf_head);
-
- /* Add sub-frame to BAW */
- ath_tx_addto_baw(sc, tid, bf);
-
- /* Queue to h/w without aggregation */
- TX_STAT_INC(txq->axq_qnum, a_queued_hw);
- bf->bf_lastbf = bf;
- ath_tx_fill_desc(sc, bf, txq, fi->framelen);
- ath_tx_txqaddbuf(sc, txq, &bf_head, false);
-}
-
static void ath_tx_send_normal(struct ath_softc *sc, struct ath_txq *txq,
struct ath_atx_tid *tid, struct sk_buff *skb)
{
@@ -2159,20 +2176,25 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
ath_txq_unlock(sc, txq);
txq = sc->tx.uapsdq;
ath_txq_lock(sc, txq);
- }
-
- if (txctl->an && ieee80211_is_data_qos(hdr->frame_control)) {
+ } else if (txctl->an &&
+ ieee80211_is_data_present(hdr->frame_control)) {
tid = ath_get_skb_tid(sc, txctl->an, skb);

WARN_ON(tid->ac->txq != txctl->txq);
- }

- if ((info->flags & IEEE80211_TX_CTL_AMPDU) && tid) {
+ if (info->flags & IEEE80211_TX_CTL_CLEAR_PS_FILT)
+ tid->ac->clear_ps_filter = true;
+
/*
- * Try aggregation if it's a unicast data frame
- * and the destination is HT capable.
+ * Add this frame to software queue for scheduling later
+ * for aggregation.
*/
- ath_tx_send_ampdu(sc, txq, tid, skb, txctl);
+ TX_STAT_INC(txq->axq_qnum, a_queued_sw);
+ __skb_queue_tail(&tid->buf_q, skb);
+ if (!txctl->an->sleeping)
+ ath_tx_queue_tid(txq, tid);
+
+ ath_txq_schedule(sc, txq);
goto out;
}

--
1.8.0.2


2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 04/12] ath9k: add CAB queue info to debugfs

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/debug.c | 43 +++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index e744d97..d94facc 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -726,6 +726,28 @@ static ssize_t read_file_xmit(struct file *file, char __user *user_buf,
return retval;
}

+static ssize_t print_queue(struct ath_softc *sc, struct ath_txq *txq,
+ char *buf, ssize_t size)
+{
+ ssize_t len = 0;
+
+ ath_txq_lock(sc, txq);
+
+ len += snprintf(buf + len, size - len, "%s: %d ",
+ "qnum", txq->axq_qnum);
+ len += snprintf(buf + len, size - len, "%s: %2d ",
+ "qdepth", txq->axq_depth);
+ len += snprintf(buf + len, size - len, "%s: %2d ",
+ "ampdu-depth", txq->axq_ampdu_depth);
+ len += snprintf(buf + len, size - len, "%s: %3d ",
+ "pending", txq->pending_frames);
+ len += snprintf(buf + len, size - len, "%s: %d\n",
+ "stopped", txq->stopped);
+
+ ath_txq_unlock(sc, txq);
+ return len;
+}
+
static ssize_t read_file_queues(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -743,24 +765,13 @@ static ssize_t read_file_queues(struct file *file, char __user *user_buf,

for (i = 0; i < IEEE80211_NUM_ACS; i++) {
txq = sc->tx.txq_map[i];
- len += snprintf(buf + len, size - len, "(%s): ", qname[i]);
-
- ath_txq_lock(sc, txq);
-
- len += snprintf(buf + len, size - len, "%s: %d ",
- "qnum", txq->axq_qnum);
- len += snprintf(buf + len, size - len, "%s: %2d ",
- "qdepth", txq->axq_depth);
- len += snprintf(buf + len, size - len, "%s: %2d ",
- "ampdu-depth", txq->axq_ampdu_depth);
- len += snprintf(buf + len, size - len, "%s: %3d ",
- "pending", txq->pending_frames);
- len += snprintf(buf + len, size - len, "%s: %d\n",
- "stopped", txq->stopped);
-
- ath_txq_unlock(sc, txq);
+ len += snprintf(buf + len, size - len, "(%s): ", qname[i]);
+ len += print_queue(sc, txq, buf + len, size - len);
}

+ len += snprintf(buf + len, size - len, "(CAB): ");
+ len += print_queue(sc, sc->beacon.cabq, buf + len, size - len);
+
if (len > size)
len = size;

--
1.8.0.2


2013-08-06 09:49:23

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 09/12] ath9k: always clear ps filter bit on new assoc

On 2013-08-06 9:55 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> Otherwise in some cases, EAPOL frames might be filtered during the
>> initial handshake, causing delays and assoc failures.
>
> I think this is a stable candidate.
Okay, will add Cc:stable in v2.

- Felix

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 08/12] ath9k: fix clearing expired A-MPDU subframes in tx completion

When the tid aggregation state has been marked as inactive, free
completed tx packets immediately. When a new aggregation session has not
been initialized yet, the BAW checks do not recognize it as expired.

Might fix potential stalls in setting up a new aggregation session.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index e4d3723..2cd8268 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -520,7 +520,8 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
tx_info = IEEE80211_SKB_CB(skb);
fi = get_frame_info(skb);

- if (!BAW_WITHIN(tid->seq_start, tid->baw_size, seqno)) {
+ if (!BAW_WITHIN(tid->seq_start, tid->baw_size, seqno) ||
+ !tid->active) {
/*
* Outside of the current BlockAck window,
* maybe part of a previous session
--
1.8.0.2


2013-08-06 09:53:53

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 11/12] ath9k: improve tx scheduling fairness

On 2013-08-06 10:48 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> Instead of trying to schedule the same TID multiple times in a loop,
>> iterate over other TIDs/stations first.
>
> The txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH check in the beginning
> of txq_schedule() is not really correct after this, no ?
Right, I'll take care of that.

Thanks for the detailed review.

- Felix

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 11/12] ath9k: improve tx scheduling fairness

Instead of trying to schedule the same TID multiple times in a loop,
iterate over other TIDs/stations first.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 84 ++++++++++++++++++++---------------
1 file changed, 47 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 037cc91..274fe3f 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1345,8 +1345,8 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
} while (1);
}

-static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_atx_tid *tid, bool *stop)
{
struct ath_buf *bf;
struct ieee80211_tx_info *tx_info;
@@ -1355,40 +1355,39 @@ static void ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
int aggr_len = 0;
bool aggr, last = true;

- do {
- if (!ath_tid_has_buffered(tid))
- return;
+ if (!ath_tid_has_buffered(tid))
+ return false;

- INIT_LIST_HEAD(&bf_q);
+ INIT_LIST_HEAD(&bf_q);

- bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
- if (!bf)
- break;
+ bf = ath_tx_get_tid_subframe(sc, txq, tid, &tid_q);
+ if (!bf)
+ return false;

- tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
- aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
- if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
- (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH))
- break;
+ tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
+ aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
+ if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
+ (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH))
+ return false;

- ath_set_rates(tid->an->vif, tid->an->sta, bf);
- if (aggr)
- last = ath_tx_form_aggr(sc, txq, tid, &bf_q, bf,
- tid_q, &aggr_len);
- else
- ath_tx_form_burst(sc, txq, tid, &bf_q, bf, tid_q);
+ ath_set_rates(tid->an->vif, tid->an->sta, bf);
+ if (aggr)
+ last = ath_tx_form_aggr(sc, txq, tid, &bf_q, bf,
+ tid_q, &aggr_len);
+ else
+ ath_tx_form_burst(sc, txq, tid, &bf_q, bf, tid_q);

- if (list_empty(&bf_q))
- return;
+ if (list_empty(&bf_q))
+ return false;

- if (tid->ac->clear_ps_filter) {
- tid->ac->clear_ps_filter = false;
- tx_info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
- }
+ if (tid->ac->clear_ps_filter) {
+ tid->ac->clear_ps_filter = false;
+ tx_info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
+ }

- ath_tx_fill_desc(sc, bf, txq, aggr_len);
- ath_tx_txqaddbuf(sc, txq, &bf_q, false);
- } while (!last);
+ ath_tx_fill_desc(sc, bf, txq, aggr_len);
+ ath_tx_txqaddbuf(sc, txq, &bf_q, false);
+ return true;
}

int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
@@ -1824,8 +1823,9 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
*/
void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
{
- struct ath_atx_ac *ac, *ac_tmp, *last_ac;
+ struct ath_atx_ac *ac, *last_ac;
struct ath_atx_tid *tid, *last_tid;
+ bool sent = false;

if (test_bit(SC_OP_HW_RESET, &sc->sc_flags) ||
list_empty(&txq->axq_acq) ||
@@ -1834,15 +1834,17 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)

rcu_read_lock();

- ac = list_first_entry(&txq->axq_acq, struct ath_atx_ac, list);
last_ac = list_entry(txq->axq_acq.prev, struct ath_atx_ac, list);
+ while (!list_empty(&txq->axq_acq)) {
+ bool stop = false;

- list_for_each_entry_safe(ac, ac_tmp, &txq->axq_acq, list) {
+ ac = list_first_entry(&txq->axq_acq, struct ath_atx_ac, list);
last_tid = list_entry(ac->tid_q.prev, struct ath_atx_tid, list);
list_del(&ac->list);
ac->sched = false;

while (!list_empty(&ac->tid_q)) {
+
tid = list_first_entry(&ac->tid_q, struct ath_atx_tid,
list);
list_del(&tid->list);
@@ -1851,7 +1853,8 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
if (tid->paused)
continue;

- ath_tx_sched_aggr(sc, txq, tid);
+ if (ath_tx_sched_aggr(sc, txq, tid, &stop))
+ sent = true;

/*
* add tid to round-robin queue if more frames
@@ -1860,8 +1863,7 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
if (ath_tid_has_buffered(tid))
ath_tx_queue_tid(txq, tid);

- if (tid == last_tid ||
- txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH)
+ if (stop || tid == last_tid)
break;
}

@@ -1870,9 +1872,17 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
list_add_tail(&ac->list, &txq->axq_acq);
}

- if (ac == last_ac ||
- txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH)
+ if (stop)
break;
+
+ if (ac == last_ac) {
+ if (!sent)
+ break;
+
+ sent = false;
+ last_ac = list_entry(txq->axq_acq.prev,
+ struct ath_atx_ac, list);
+ }
}

rcu_read_unlock();
--
1.8.0.2


2013-08-06 09:48:01

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 07/12] ath9k: prepare queueing code for handling unaggregated traffic

On 2013-08-06 9:45 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> + tx_info = IEEE80211_SKB_CB(skb);
>> + tx_info->flags &= ~IEEE80211_TX_CTL_CLEAR_PS_FILT;
> Why is the PS filter flag cleared here ?
Because the previous value doesn't matter here. With software
scheduling, clearing the ps filter is triggered via
tid->ac->clear_ps_filter, and that is used to set the flag when a frame
is put into the hardware queue.
The IEEE80211_TX_CTL_CLEAR_PS_FILT is checked whenever a frame enters
the tid queue, and if it's set, tid->ac->clear_ps_filter gets set.
That way it doesn't have to loop the entire tid traffic up to the new
frame through the hw queue as filtered until the destination index gets
cleared.

>> - txtid->paused = true;
>> + txtid->paused = false;
>
> Why change this ?
When going from aggregated to un-aggregated, the TID still needs to be
scheduled, and txtid->paused == true would prevent that.

- Felix


2013-08-06 09:38:26

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 10/12] ath9k: use software queues for un-aggregated data packets

On 2013-08-06 10:44 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> + aggr = !!(tx_info->flags & IEEE80211_TX_CTL_AMPDU);
>> + if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
>> + (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH))
>> + break;
>> +
>> + ath_set_rates(tid->an->vif, tid->an->sta, bf);
>> + if (aggr)
>> + last = ath_tx_form_aggr(sc, txq, tid, &bf_q, bf,
>> + tid_q, &aggr_len);
>> + else
>> + ath_tx_form_burst(sc, txq, tid, &bf_q, bf, tid_q);
>> +
>> + if (list_empty(&bf_q))
>> + return;
>
> Handling non-AMPDU and AMPDU packets in the same path makes the code hard
> to follow. Since a TID can either have aggregated or unaggregated packets,
> why not separate the logic early, maybe in the schedule() function ?
When aggregation is enabled on a TID, it can still hold some
non-aggregated packets that need to be sent out first until it starts
forming A-MPDUs. I think separating this early doesn't work.

- Felix


2013-08-06 16:35:52

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 07/12] ath9k: prepare queueing code for handling unaggregated traffic

Felix Fietkau wrote:
> Because the previous value doesn't matter here. With software
> scheduling, clearing the ps filter is triggered via
> tid->ac->clear_ps_filter, and that is used to set the flag when a frame
> is put into the hardware queue.
> The IEEE80211_TX_CTL_CLEAR_PS_FILT is checked whenever a frame enters
> the tid queue, and if it's set, tid->ac->clear_ps_filter gets set.
> That way it doesn't have to loop the entire tid traffic up to the new
> frame through the hw queue as filtered until the destination index gets
> cleared.

Ok.

> When going from aggregated to un-aggregated, the TID still needs to be
> scheduled, and txtid->paused == true would prevent that.

Yeah, makes sense.

Sujith

2013-08-06 08:57:11

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 01/12] ath9k: add utility functions for accessing tid queues

Felix Fietkau wrote:
> - if (skb_queue_empty(&tid->buf_q))
> + if (ath_tid_has_buffered(tid))

Shouldn't this be !ath_tid_has_buffered(tid) ?

Sujith

2013-08-06 09:40:57

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH 01/12] ath9k: add utility functions for accessing tid queues

On 2013-08-06 9:21 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> - if (skb_queue_empty(&tid->buf_q))
>> + if (ath_tid_has_buffered(tid))
>
> Shouldn't this be !ath_tid_has_buffered(tid) ?
Right, thanks.

- Felix


2013-08-06 08:57:19

by Sujith Manoharan

[permalink] [raw]
Subject: Re: [PATCH 09/12] ath9k: always clear ps filter bit on new assoc

Felix Fietkau wrote:
> Otherwise in some cases, EAPOL frames might be filtered during the
> initial handshake, causing delays and assoc failures.

I think this is a stable candidate.

Sujith

2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 05/12] ath9k: simplify ath_tx_form_aggr

The check for ATH_AMPDU_SUBFRAME_DEFAULT is unnecessary, since it's set
to half the maximum BlockAck Window size, which is already the maximum
value that h_baw could possibly have. Also remove unnecessary variables.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 -
drivers/net/wireless/ath/ath9k/xmit.c | 33 ++++++++++++---------------------
2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 919c8ee..88ef785 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -137,7 +137,6 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_AGGR_ENCRYPTDELIM 10
/* minimum h/w qdepth to be sustained to maximize aggregation */
#define ATH_AGGR_MIN_QDEPTH 2
-#define ATH_AMPDU_SUBFRAME_DEFAULT 32

#define IEEE80211_SEQ_SEQ_SHIFT 4
#define IEEE80211_SEQ_MAX 4096
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index e54c3e5..8ce2d36 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -906,9 +906,9 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
{
#define PADBYTES(_len) ((4 - ((_len) % 4)) % 4)
struct ath_buf *bf, *bf_first = NULL, *bf_prev = NULL;
- int rl = 0, nframes = 0, ndelim, prev_al = 0;
+ int nframes = 0, ndelim;
u16 aggr_limit = 0, al = 0, bpad = 0,
- al_delta, h_baw = tid->baw_size / 2;
+ al_delta, h_baw = tid->baw_size / 2;
enum ATH_AGGR_STATUS status = ATH_AGGR_DONE;
struct ieee80211_tx_info *tx_info;
struct ath_frame_info *fi;
@@ -925,33 +925,24 @@ static enum ATH_AGGR_STATUS ath_tx_form_aggr(struct ath_softc *sc,
skb = bf->bf_mpdu;
fi = get_frame_info(skb);

- if (!bf_first)
+ if (!bf_first) {
bf_first = bf;
-
- if (!rl) {
ath_set_rates(tid->an->vif, tid->an->sta, bf);
aggr_limit = ath_lookup_rate(sc, bf, tid);
- rl = 1;
}

/* do not exceed aggregation limit */
al_delta = ATH_AGGR_DELIM_SZ + fi->framelen;
+ if (nframes) {
+ if (aggr_limit < al + bpad + al_delta ||
+ ath_lookup_legacy(bf) || nframes >= h_baw) {
+ status = ATH_AGGR_LIMITED;
+ break;
+ }

- if (nframes &&
- ((aggr_limit < (al + bpad + al_delta + prev_al)) ||
- ath_lookup_legacy(bf))) {
- status = ATH_AGGR_LIMITED;
- break;
- }
-
- tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
- if (nframes && (tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE))
- break;
-
- /* do not exceed subframe limit */
- if (nframes >= min((int)h_baw, ATH_AMPDU_SUBFRAME_DEFAULT)) {
- status = ATH_AGGR_LIMITED;
- break;
+ tx_info = IEEE80211_SKB_CB(bf->bf_mpdu);
+ if (tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE)
+ break;
}

/* add padding for previous frame to aggregation length */
--
1.8.0.2


2013-08-05 19:56:31

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 09/12] ath9k: always clear ps filter bit on new assoc

Otherwise in some cases, EAPOL frames might be filtered during the
initial handshake, causing delays and assoc failures.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/xmit.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 2cd8268..11202ea 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2665,6 +2665,7 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
for (acno = 0, ac = &an->ac[acno];
acno < IEEE80211_NUM_ACS; acno++, ac++) {
ac->sched = false;
+ ac->clear_ps_filter = true;
ac->txq = sc->tx.txq_map[acno];
INIT_LIST_HEAD(&ac->tid_q);
}
--
1.8.0.2