The recent changes of ath_tx_rc_status makes it to be called
only for the first frame of aggregation or if there is a
single subframe and for all normal frames. And also for all
aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
patch removes the unnecessary checks and make the ampdu_[ack]_len
to be filled on both aggregation and normal frames.
Signed-off-by: Rajkumar Manoharan <[email protected]>
---
drivers/net/wireless/ath/ath9k/rc.c | 11 -----------
drivers/net/wireless/ath/ath9k/xmit.c | 5 ++---
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/rc.c b/drivers/net/wireless/ath/ath9k/rc.c
index 4f13018..81c48fb 100644
--- a/drivers/net/wireless/ath/ath9k/rc.c
+++ b/drivers/net/wireless/ath/ath9k/rc.c
@@ -1354,20 +1354,9 @@ static void ath_tx_status(void *priv, struct ieee80211_supported_band *sband,
if (!priv_sta || !ieee80211_is_data(fc))
return;
- /* This packet was aggregated but doesn't carry status info */
- if ((tx_info->flags & IEEE80211_TX_CTL_AMPDU) &&
- !(tx_info->flags & IEEE80211_TX_STAT_AMPDU))
- return;
-
if (tx_info->flags & IEEE80211_TX_STAT_TX_FILTERED)
return;
- if (!(tx_info->flags & IEEE80211_TX_STAT_AMPDU)) {
- tx_info->status.ampdu_ack_len =
- (tx_info->flags & IEEE80211_TX_STAT_ACK ? 1 : 0);
- tx_info->status.ampdu_len = 1;
- }
-
if (!(tx_info->flags & IEEE80211_TX_STAT_ACK))
tx_status = 1;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index fa3dcfd..f5d4764 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2043,10 +2043,9 @@ static void ath_tx_rc_status(struct ath_softc *sc, struct ath_buf *bf,
tx_info->flags |= IEEE80211_TX_STAT_AMPDU;
BUG_ON(nbad > nframes);
-
- tx_info->status.ampdu_len = nframes;
- tx_info->status.ampdu_ack_len = nframes - nbad;
}
+ tx_info->status.ampdu_len = nframes;
+ tx_info->status.ampdu_ack_len = nframes - nbad;
if ((ts->ts_status & ATH9K_TXERR_FILT) == 0 &&
(tx_info->flags & IEEE80211_TX_CTL_NO_ACK) == 0) {
--
1.7.6.4
On Mon, Sep 26, 2011 at 07:27:43AM -0600, Felix Fietkau wrote:
> On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
> >The recent changes of ath_tx_rc_status makes it to be called
> >only for the first frame of aggregation or if there is a
> >single subframe and for all normal frames. And also for all
> >aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
> >patch removes the unnecessary checks and make the ampdu_[ack]_len
> >to be filled on both aggregation and normal frames.
> This patch looks wrong to me. Only one subframe per A-MPDU is
> supposed to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you
> removed from rc.c is actually necessary, otherwise it'll process the
> tx status of an A-MPDU multiple times (once per completed subframe).
>
I dont understand. Right now ath_tx_rc_status is called only for the first
frame of aggregation or there is only one subframe in the aggregation.
It never be called for all subframes. Am i right?
--
Rajkumar
On 26.09.2011, at 17:12, Rajkumar Manoharan <[email protected]> wrote:
> On Mon, Sep 26, 2011 at 07:27:43AM -0600, Felix Fietkau wrote:
>> On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
>>> The recent changes of ath_tx_rc_status makes it to be called
>>> only for the first frame of aggregation or if there is a
>>> single subframe and for all normal frames. And also for all
>>> aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
>>> patch removes the unnecessary checks and make the ampdu_[ack]_len
>>> to be filled on both aggregation and normal frames.
>> This patch looks wrong to me. Only one subframe per A-MPDU is
>> supposed to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you
>> removed from rc.c is actually necessary, otherwise it'll process the
>> tx status of an A-MPDU multiple times (once per completed subframe).
>>
> I dont understand. Right now ath_tx_rc_status is called only for the first
> frame of aggregation or there is only one subframe in the aggregation.
> It never be called for all subframes. Am i right?
Right, but the rate control status update is called per subframe, so the rc.c change is broken
On 2011-09-26 3:42 AM, Rajkumar Manoharan wrote:
> The recent changes of ath_tx_rc_status makes it to be called
> only for the first frame of aggregation or if there is a
> single subframe and for all normal frames. And also for all
> aggregated frames IEEE80211_TX_STAT_AMPDU will be set. This
> patch removes the unnecessary checks and make the ampdu_[ack]_len
> to be filled on both aggregation and normal frames.
This patch looks wrong to me. Only one subframe per A-MPDU is supposed
to carry the IEEE80211_TX_STAT_AMPDU flag, so the check you removed from
rc.c is actually necessary, otherwise it'll process the tx status of an
A-MPDU multiple times (once per completed subframe).