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

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

[Helmut Schaa]

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 <helmut.schaa@googlemail.com>
---

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

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

[Helmut Schaa]

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

Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
Cc: Ivo Van Doorn <ivdoorn@gmail.com>
Cc: Gertjan van Wingerde <gwingerde@gmail.com>
---

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

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

[Felix Fietkau]

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<helmut.schaa@googlemail.com>
> ---
>
> 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

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

[Helmut Schaa]

On Thu, Sep 8, 2011 at 11:08 AM, Felix Fietkau <nbd@openwrt.org> 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<helmut.schaa@googlemail.com>
>> ---
>>
>> 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

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

[Ivo Van Doorn]

On Thu, Sep 8, 2011 at 10:58 AM, Helmut Schaa
<helmut.schaa@googlemail.com> wrote:
> This is handled in minstrel_ht now. No need to special case it here
> again.
>
> Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
> Cc: Ivo Van Doorn <ivdoorn@gmail.com>
> Cc: Gertjan van Wingerde <gwingerde@gmail.com>

Acked-by: Ivo van Doorn <IvDoorn@gmail.com>

> ---
>
> 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
>
>