2011-09-08 08:57:57

by Helmut Schaa

Subject: [PATCH 1/2] mac80211: Don't aggregate rate probe frames in minstrel_ht

This is already done by rt2x00 and ath9k by checking the
IEEE80211_TX_CTL_RATE_CTRL_PROBE flag. Instead we can simply do it in
minstrel_ht.

Signed-off-by: Helmut Schaa <[email protected]>
---

Felix, I didn't have a closer look at the ath9k aggregation code but I guess it
can also be simplified with this change.

Any objections?

Thanks,
Helmut

net/mac80211/rc80211_minstrel_ht.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 2158838..6a7bbe0 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -627,6 +627,8 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
minstrel_ht_set_rate(mp, mi, &ar[0], sample_idx,
txrc, true, false);
info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
+ /* don't aggregate probe frames */
+ info->flags &= ~IEEE80211_TX_CTL_AMPDU;
} else {
minstrel_ht_set_rate(mp, mi, &ar[0], mi->max_tp_rate,
txrc, false, false);
--
1.7.3.4

2011-09-08 09:29:10

by Helmut Schaa

Subject: Re: [PATCH 1/2] mac80211: Don't aggregate rate probe frames in minstrel_ht

On Thu, Sep 8, 2011 at 11:08 AM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-08 10:58 AM, Helmut Schaa wrote:
>>
>> This is already done by rt2x00 and ath9k by checking the
>> IEEE80211_TX_CTL_RATE_CTRL_PROBE flag. Instead we can simply do it in
>> minstrel_ht.
>>
>> Signed-off-by: Helmut Schaa<[email protected]>
>> ---
>>
>> Felix, I didn't have a closer look at the ath9k aggregation code but I
>> guess it
>> can also be simplified with this change.
>>
>> Any objections?
>
> Yes, this probably breaks ath9k. IEEE80211_TX_CTL_AMPDU is used to indicate
> that the packet is part of an aggregation session, so ath9k only does block
> ack window tracking for packets that have this flag set.
> I think the drivers should continue to check for
> IEEE80211_TX_CTL_RATE_CTRL_PROBE

Ok, the flag description is a bit misleading then:

@IEEE80211_TX_CTL_AMPDU: this frame should be sent as part of an A-MPDU

John, please drop this series.

Helmut

2011-09-08 08:58:12

by Helmut Schaa

Subject: [PATCH 2/2] rt2x00: Remove special case for rate probe frames

This is handled in minstrel_ht now. No need to special case it here
again.

Signed-off-by: Helmut Schaa <[email protected]>
Cc: Ivo Van Doorn <[email protected]>
Cc: Gertjan van Wingerde <[email protected]>
---

Ivo, I'm sending this directly to John due to the dependency on the
minstrel_ht patch.

drivers/net/wireless/rt2x00/rt2x00queue.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index ba0d7e6..033d2cd 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -330,11 +330,9 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
}

/*
- * This frame is eligible for an AMPDU, however, don't aggregate
- * frames that are intended to probe a specific tx rate.
+ * This frame is eligible for an AMPDU.
*/
- if (tx_info->flags & IEEE80211_TX_CTL_AMPDU &&
- !(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE))
+ if (tx_info->flags & IEEE80211_TX_CTL_AMPDU)
__set_bit(ENTRY_TXD_HT_AMPDU, &txdesc->flags);

/*
--
1.7.3.4

2011-09-08 09:08:23

by Felix Fietkau

Subject: Re: [PATCH 1/2] mac80211: Don't aggregate rate probe frames in minstrel_ht

On 2011-09-08 10:58 AM, Helmut Schaa wrote:
> This is already done by rt2x00 and ath9k by checking the
> IEEE80211_TX_CTL_RATE_CTRL_PROBE flag. Instead we can simply do it in
> minstrel_ht.
>
> Signed-off-by: Helmut Schaa<[email protected]>
> ---
>
> Felix, I didn't have a closer look at the ath9k aggregation code but I guess it
> can also be simplified with this change.
>
> Any objections?
Yes, this probably breaks ath9k. IEEE80211_TX_CTL_AMPDU is used to
indicate that the packet is part of an aggregation session, so ath9k
only does block ack window tracking for packets that have this flag set.
I think the drivers should continue to check for
IEEE80211_TX_CTL_RATE_CTRL_PROBE

- Felix

2011-09-08 12:41:33

by Ivo Van Doorn

Subject: Re: [PATCH 2/2] rt2x00: Remove special case for rate probe frames

On Thu, Sep 8, 2011 at 10:58 AM, Helmut Schaa
<[email protected]> wrote:
> This is handled in minstrel_ht now. No need to special case it here
> again.
>
> Signed-off-by: Helmut Schaa <[email protected]>
> Cc: Ivo Van Doorn <[email protected]>
> Cc: Gertjan van Wingerde <[email protected]>

Acked-by: Ivo van Doorn <[email protected]>

> ---
>
> Ivo, I'm sending this directly to John due to the dependency on the
> minstrel_ht patch.
>
> ?drivers/net/wireless/rt2x00/rt2x00queue.c | ? ?6 ++----
> ?1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
> index ba0d7e6..033d2cd 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00queue.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
> @@ -330,11 +330,9 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
> ? ? ? ?}
>
> ? ? ? ?/*
> - ? ? ? ?* This frame is eligible for an AMPDU, however, don't aggregate
> - ? ? ? ?* frames that are intended to probe a specific tx rate.
> + ? ? ? ?* This frame is eligible for an AMPDU.
> ? ? ? ? */
> - ? ? ? if (tx_info->flags & IEEE80211_TX_CTL_AMPDU &&
> - ? ? ? ? ? !(tx_info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE))
> + ? ? ? if (tx_info->flags & IEEE80211_TX_CTL_AMPDU)
> ? ? ? ? ? ? ? ?__set_bit(ENTRY_TXD_HT_AMPDU, &txdesc->flags);
>
> ? ? ? ?/*
> --
> 1.7.3.4
>
>