[RFC/RFT] rt2x00: do not set IEEE80211_TX_STAT_AMPDU_NO_BACK on tx status

[RFC/RFT] rt2x00: do not set IEEE80211_TX_STAT_AMPDU_NO_BACK on tx status

[Stanislaw Gruszka]

According to documentation IEEE80211_TX_STAT_AMPDU_NO_BACK is suppose
to be used when we do not recive BA (BlockAck). However on rt2x00 we
use it when remote station fail to decode one or more subframes within
AMPDU (some bits are not set in BlockAck bitmap). Setting the flag result
in sent of BAR (BlockAck Request) frame and this might result of abuse
of BA session, since remote station can sent BA with incorrect 
sequence numbers after receiving BAR. This problem is visible especially
when connecting two rt2800 devices.

Previously I observed some performance benefits when using the flag
when connecting with iwlwifi devices. But currently possibly due
to reacent changes in rt2x00 removing the flag has no effect on
those test cases.

So remove the IEEE80211_TX_STAT_AMPDU_NO_BACK.

Perhaps we should send BAR exlicitly on BA session start/stop
and when remote STA went to PowerSave mode (for AP) like mt76 does.
But I do not understand for what this is needed.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index a6c374c483c2..c547bec044a8 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -371,9 +371,6 @@ static void rt2x00lib_fill_tx_status(struct rt2x00_dev *rt2x00dev,
 				  IEEE80211_TX_CTL_AMPDU;
 		tx_info->status.ampdu_len = 1;
 		tx_info->status.ampdu_ack_len = success ? 1 : 0;
-
-		if (!success)
-			tx_info->flags |= IEEE80211_TX_STAT_AMPDU_NO_BACK;
 	}
 
 	if (rate_flags & IEEE80211_TX_RC_USE_RTS_CTS) {
-- 
2.20.1

Re: [RFC/RFT] rt2x00: do not set IEEE80211_TX_STAT_AMPDU_NO_BACK on tx status

[Stanislaw Gruszka]

On Thu, Jul 04, 2019 at 01:06:52PM +0200, Stanislaw Gruszka wrote:
> According to documentation IEEE80211_TX_STAT_AMPDU_NO_BACK is suppose
> to be used when we do not recive BA (BlockAck). However on rt2x00 we
> use it when remote station fail to decode one or more subframes within
> AMPDU (some bits are not set in BlockAck bitmap). Setting the flag result
> in sent of BAR (BlockAck Request) frame and this might result of abuse
> of BA session, since remote station can sent BA with incorrect 
> sequence numbers after receiving BAR. This problem is visible especially
> when connecting two rt2800 devices.
> 
> Previously I observed some performance benefits when using the flag
> when connecting with iwlwifi devices. But currently possibly due
> to reacent changes in rt2x00 removing the flag has no effect on
> those test cases.
> 
> So remove the IEEE80211_TX_STAT_AMPDU_NO_BACK.
> 
> Perhaps we should send BAR exlicitly on BA session start/stop
> and when remote STA went to PowerSave mode (for AP) like mt76 does.
> But I do not understand for what this is needed.

This commit 

https://github.com/openwrt/mt76/commit/3e447e7797d64dbf4dc1dd5553d08be0d8150d7e

explained that sending BAR on stop aggregation is workaround for
buggy clients. I can implement that on rt2x00. But I will skip
sending BAR on remote station PS wakeup, since we do not implement
all PS related code in rt2x00.  

Stanislaw