2011-01-11 00:21:11

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 1/4] ath9k: fix bogus sequence number increases on aggregation tid flush

When a tid pointer is passed to ath_tx_send_normal(), it increases the
starting sequence number for the next AMPDU action frame, which should
only be done if the sequence number assignment is fresh. In this case
it is clearly not.

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

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 332d1fe..1adfebc 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -169,7 +169,7 @@ static void ath_tx_flush_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
ath_tx_update_baw(sc, tid, fi->seqno);
ath_tx_complete_buf(sc, bf, txq, &bf_head, &ts, 0, 0);
} else {
- ath_tx_send_normal(sc, txq, tid, &bf_head);
+ ath_tx_send_normal(sc, txq, NULL, &bf_head);
}
spin_lock_bh(&txq->axq_lock);
}
--
1.7.3.2



2011-01-11 00:21:13

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 4/4] ath9k: reduce the likelihood of baseband hang check false positives

Since baseband hangs are rare, but the hang check function has a high
false positive rate in some situations, we need to add more reliable
indicators.

In AP mode we can use blocked beacon transmissions as an indicator,
they should be rare enough.

In station mode, we can skip the hang check entirely, since a true
hang will trigger beacon loss detection, and mac80211 will rescan,
which leads to a hw reset that will bring the hardware back to life.

To make this more reliable, we need to skip fast channel changes
if the hardware appears to be stuck.

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

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index f90a6ca..c753ba4 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -251,6 +251,9 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
if (!ath_stoprecv(sc))
stopped = false;

+ if (!ath9k_hw_check_alive(ah))
+ stopped = false;
+
/* XXX: do not flush receive queue here. We don't want
* to flush data frames already in queue because of
* changing channel. */
@@ -602,7 +605,15 @@ void ath9k_tasklet(unsigned long data)

spin_lock(&sc->sc_pcu_lock);

- if (!ath9k_hw_check_alive(ah))
+ /*
+ * Only run the baseband hang check if beacons stop working in AP or
+ * IBSS mode, because it has a high false positive rate. For station
+ * mode it should not be necessary, since the upper layers will detect
+ * this through a beacon miss automatically and the following channel
+ * change will trigger a hardware reset anyway
+ */
+ if (ath9k_hw_numtxpending(ah, sc->beacon.beaconq) != 0 &&
+ !ath9k_hw_check_alive(ah))
ieee80211_queue_work(sc->hw, &sc->hw_check_work);

if (ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)
--
1.7.3.2


2011-01-11 00:20:56

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 3/4] ath9k: reinitialize block ack window data when starting aggregation

There might be some old stale data left, which could confuse tracking
of pending tx frames.

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

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 6ddba4b..ab4f7b4 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -858,6 +858,9 @@ int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,
txtid->paused = true;
*ssn = txtid->seq_start = txtid->seq_next;

+ memset(txtid->tx_buf, 0, sizeof(txtid->tx_buf));
+ txtid->baw_head = txtid->baw_tail = 0;
+
return 0;
}

--
1.7.3.2


2011-01-11 00:21:11

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 2/4] ath9k: fix initial sequence number after starting an ampdu session

txtid->seq_start may not always be up to date, when there is HT non-AMPDU
traffic just before starting an AMPDU session. Relying on txtid->seq_next
is better, since it is also used to generate the sequence numbers for
all QoS data frames.

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

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 1adfebc..6ddba4b 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -856,7 +856,7 @@ int ath_tx_aggr_start(struct ath_softc *sc, struct ieee80211_sta *sta,

txtid->state |= AGGR_ADDBA_PROGRESS;
txtid->paused = true;
- *ssn = txtid->seq_start;
+ *ssn = txtid->seq_start = txtid->seq_next;

return 0;
}
--
1.7.3.2


2011-01-11 04:39:54

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/4] ath9k: fix bogus sequence number increases on aggregation tid flush

On 01/10/2011 04:05 PM, Felix Fietkau wrote:
> When a tid pointer is passed to ath_tx_send_normal(), it increases the
> starting sequence number for the next AMPDU action frame, which should
> only be done if the sequence number assignment is fresh. In this case
> it is clearly not.

I added these to my system. It doesn't seem any worse, but I'm
still seeing my tx-hang logic kick off, so at least some of the
pending queue stuff still has issues I think:

ath: txq: ec731e78 axq_qnum: 2, mac80211_qnum: 2 axq_link: (null) pending frames: 73 axq_acq empty: 0 stopped: 0 axq_depth: 0 Attempting to restart tx logic.

In general though, my testing is finally starting to surprise with good results
instead of bugs, so I'm quite optimistic these days!

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com