This patch fixes the following problems with the rate control feedback
generated by ath9k for A-MPDU frames:
1. Rate control feedback is carried on the first frame of an aggregate
that is either ACKed, or has execeeded the software retry count and is
considered failed. However, ath9k would incorrectly assume the aggregate
had the length 1 if one of these conditions did not apply to the first
frame of the aggregate, but instead a later frame. This fix therefor
copies the bf_length field of the buffer in the same manner as the rates
field of the tx status.
2. Sometimes the ampdu_len and ampdu_ack_len fields of the tx status was
left uninitialized even though the IEEE80211_TX_STAT_AMPDU flag was set.
This is now avoid by setting flag and fields in the same place.
3. Eventhough a frame has been selected for aggregation by mac80211 and
marked with the IEEE80211_TX_CTL_AMPDU flag it can sometimes happen that
ath9k transmits the frame without aggregation. In these cases the
ampdu_ack_len field could be incorrectly computed because the nbad
parameter to ath_tx_rc_status was incorrect.
Cc: <[email protected]>
Signed-off-by: Bj?rn Smedman <[email protected]>
---
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index aa44777..9a11099 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -317,6 +317,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
int isaggr, txfail, txpending, sendbar = 0, needreset = 0, nbad = 0;
bool rc_update = true;
struct ieee80211_tx_rate rates[4];
+ int nframes;
skb = bf->bf_mpdu;
hdr = (struct ieee80211_hdr *)skb->data;
@@ -325,6 +326,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
hw = bf->aphy->hw;
memcpy(rates, tx_info->control.rates, sizeof(rates));
+ nframes = bf->bf_nframes;
rcu_read_lock();
@@ -341,7 +343,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
!bf->bf_stale || bf_next != NULL)
list_move_tail(&bf->list, &bf_head);
- ath_tx_rc_status(bf, ts, 0, 0, false);
+ ath_tx_rc_status(bf, ts, 1, 0, false);
ath_tx_complete_buf(sc, bf, txq, &bf_head, ts,
0, 0);
@@ -446,6 +448,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
if (rc_update && (acked_cnt == 1 || txfail_cnt == 1)) {
memcpy(tx_info->control.rates, rates, sizeof(rates));
+ bf->bf_nframes = nframes;
ath_tx_rc_status(bf, ts, nbad, txok, true);
rc_update = false;
} else {
@@ -1979,9 +1982,15 @@ static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts,
if (ts->ts_status & ATH9K_TXERR_FILT)
tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
- if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && update_rc)
+ if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && update_rc) {
tx_info->flags |= IEEE80211_TX_STAT_AMPDU;
+ BUG_ON(nbad > bf->bf_nframes);
+
+ tx_info->status.ampdu_len = bf->bf_nframes;
+ tx_info->status.ampdu_ack_len = bf->bf_nframes - nbad;
+ }
+
if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 &&
(bf->bf_flags & ATH9K_TXDESC_NOACK) == 0 && update_rc) {
if (ieee80211_is_data(hdr->frame_control)) {
@@ -1991,8 +2000,6 @@ static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts,
if ((ts->ts_status & ATH9K_TXERR_XRETRY) ||
(ts->ts_status & ATH9K_TXERR_FIFO))
tx_info->pad[0] |= ATH_TX_INFO_XRETRY;
- tx_info->status.ampdu_len = bf->bf_nframes;
- tx_info->status.ampdu_ack_len = bf->bf_nframes - nbad;
}
}
@@ -2102,7 +2109,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
*/
if (ts.ts_status & ATH9K_TXERR_XRETRY)
bf->bf_state.bf_type |= BUF_XRETRY;
- ath_tx_rc_status(bf, &ts, 0, txok, true);
+ ath_tx_rc_status(bf, &ts, txok ? 0 : 1, txok, true);
}
if (bf_isampdu(bf))
@@ -2220,7 +2227,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
if (!bf_isampdu(bf)) {
if (txs.ts_status & ATH9K_TXERR_XRETRY)
bf->bf_state.bf_type |= BUF_XRETRY;
- ath_tx_rc_status(bf, &txs, 0, txok, true);
+ ath_tx_rc_status(bf, &txs, txok ? 0 : 1, txok, true);
}
if (bf_isampdu(bf))
Oops, sorry about that subject. Let me try that again. ;)
/Bj?rn
2010/10/10 Bj?rn Smedman <[email protected]>:
> This patch fixes the following problems with the rate control feedback
> generated by ath9k for A-MPDU frames:
>
> 1. Rate control feedback is carried on the first frame of an aggregate
> that is either ACKed, or has execeeded the software retry count and is
> considered failed. However, ath9k would incorrectly assume the aggregate
> had the length 1 if one of these conditions did not apply to the first
> frame of the aggregate, but instead a later frame. This fix therefor
> copies the bf_length field of the buffer in the same manner as the rates
> field of the tx status.
>
> 2. Sometimes the ampdu_len and ampdu_ack_len fields of the tx status was
> left uninitialized even though the IEEE80211_TX_STAT_AMPDU flag was set.
> This is now avoid by setting flag and fields in the same place.
>
> 3. Eventhough a frame has been selected for aggregation by mac80211 and
> marked with the IEEE80211_TX_CTL_AMPDU flag it can sometimes happen that
> ath9k transmits the frame without aggregation. In these cases the
> ampdu_ack_len field could be incorrectly computed because the nbad
> parameter to ath_tx_rc_status was incorrect.
>
> Cc: <[email protected]>
> Signed-off-by: Bj?rn Smedman <[email protected]>
> ---
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index aa44777..9a11099 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -317,6 +317,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> ? ? ? ?int isaggr, txfail, txpending, sendbar = 0, needreset = 0, nbad = 0;
> ? ? ? ?bool rc_update = true;
> ? ? ? ?struct ieee80211_tx_rate rates[4];
> + ? ? ? int nframes;
>
> ? ? ? ?skb = bf->bf_mpdu;
> ? ? ? ?hdr = (struct ieee80211_hdr *)skb->data;
> @@ -325,6 +326,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> ? ? ? ?hw = bf->aphy->hw;
>
> ? ? ? ?memcpy(rates, tx_info->control.rates, sizeof(rates));
> + ? ? ? nframes = bf->bf_nframes;
>
> ? ? ? ?rcu_read_lock();
>
> @@ -341,7 +343,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?!bf->bf_stale || bf_next != NULL)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?list_move_tail(&bf->list, &bf_head);
>
> - ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, ts, 0, 0, false);
> + ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, ts, 1, 0, false);
> ? ? ? ? ? ? ? ? ? ? ? ?ath_tx_complete_buf(sc, bf, txq, &bf_head, ts,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0, 0);
>
> @@ -446,6 +448,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
>
> ? ? ? ? ? ? ? ? ? ? ? ?if (rc_update && (acked_cnt == 1 || txfail_cnt == 1)) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?memcpy(tx_info->control.rates, rates, sizeof(rates));
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? bf->bf_nframes = nframes;
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ath_tx_rc_status(bf, ts, nbad, txok, true);
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rc_update = false;
> ? ? ? ? ? ? ? ? ? ? ? ?} else {
> @@ -1979,9 +1982,15 @@ static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts,
>
> ? ? ? ?if (ts->ts_status & ATH9K_TXERR_FILT)
> ? ? ? ? ? ? ? ?tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
> - ? ? ? if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && update_rc)
> + ? ? ? if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) && update_rc) {
> ? ? ? ? ? ? ? ?tx_info->flags |= IEEE80211_TX_STAT_AMPDU;
>
> + ? ? ? ? ? ? ? BUG_ON(nbad > bf->bf_nframes);
> +
> + ? ? ? ? ? ? ? tx_info->status.ampdu_len = bf->bf_nframes;
> + ? ? ? ? ? ? ? tx_info->status.ampdu_ack_len = bf->bf_nframes - nbad;
> + ? ? ? }
> +
> ? ? ? ?if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 &&
> ? ? ? ? ? ?(bf->bf_flags & ATH9K_TXDESC_NOACK) == 0 && update_rc) {
> ? ? ? ? ? ? ? ?if (ieee80211_is_data(hdr->frame_control)) {
> @@ -1991,8 +2000,6 @@ static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts,
> ? ? ? ? ? ? ? ? ? ? ? ?if ((ts->ts_status & ATH9K_TXERR_XRETRY) ||
> ? ? ? ? ? ? ? ? ? ? ? ? ? ?(ts->ts_status & ATH9K_TXERR_FIFO))
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?tx_info->pad[0] |= ATH_TX_INFO_XRETRY;
> - ? ? ? ? ? ? ? ? ? ? ? tx_info->status.ampdu_len = bf->bf_nframes;
> - ? ? ? ? ? ? ? ? ? ? ? tx_info->status.ampdu_ack_len = bf->bf_nframes - nbad;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> @@ -2102,7 +2109,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
> ? ? ? ? ? ? ? ? ? ? ? ? */
> ? ? ? ? ? ? ? ? ? ? ? ?if (ts.ts_status & ATH9K_TXERR_XRETRY)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bf->bf_state.bf_type |= BUF_XRETRY;
> - ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, &ts, 0, txok, true);
> + ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, &ts, txok ? 0 : 1, txok, true);
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?if (bf_isampdu(bf))
> @@ -2220,7 +2227,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
> ? ? ? ? ? ? ? ?if (!bf_isampdu(bf)) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (txs.ts_status & ATH9K_TXERR_XRETRY)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?bf->bf_state.bf_type |= BUF_XRETRY;
> - ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, &txs, 0, txok, true);
> + ? ? ? ? ? ? ? ? ? ? ? ath_tx_rc_status(bf, &txs, txok ? 0 : 1, txok, true);
> ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ?if (bf_isampdu(bf))
>
--
Venatech AB
Ideon Innovation
Ole R?mers v?g 12
SE-22370 LUND
Sweden
+46 (0) 46 286 86 20
[email protected]
http://www.venatech.se