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

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

[Felix Fietkau]

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 <nbd@openwrt.org>
---
 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

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

[Felix Fietkau]

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 <nbd@openwrt.org>
---
 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

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

[Felix Fietkau]

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

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 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

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

[Felix Fietkau]

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 <nbd@openwrt.org>
---
 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

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

[Ben Greear]

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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com