Return-path: Received: from mail-iw0-f174.google.com ([209.85.214.174]:60047 "EHLO mail-iw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751440Ab0JJUid convert rfc822-to-8bit (ORCPT ); Sun, 10 Oct 2010 16:38:33 -0400 Received: by iwn6 with SMTP id 6so2544833iwn.19 for ; Sun, 10 Oct 2010 13:38:33 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: Date: Sun, 10 Oct 2010 22:38:32 +0200 Message-ID: Subject: Re: ath9k: A-MPDU rate control info fix From: =?ISO-8859-1?Q?Bj=F6rn_Smedman?= To: linville@tuxdriver.com, johannes@sipsolutions.net, lrodriguez@atheros.com Cc: linux-wireless@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: Oops, sorry about that subject. Let me try that again. ;) /Bj?rn 2010/10/10 Bj?rn Smedman : > 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: > Signed-off-by: Bj?rn Smedman > --- > 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 info@venatech.se http://www.venatech.se