Skip to contentSkip to conversation

minstrel_ht: fix Open BA session request floods

PATCH RESEND In review Christian Lamparter 3 participants 5 messages Sep 3, 2011 – Sep 6, 2011 mac80211

lore
Share & tools
lore
#0 lore raw

PATCH RESEND 1/1 minstrel_ht: fix Open BA session request floods

Minstrel HT tries very hard to establish a BA session with each peer once there's some data on the way. However the stack does not inform minstrel if an aggregation session is already in place, so it keeps trying and wastes good cycles in the tx status path.

[ 8149.946393] Open BA session requested for $AP tid 0
[ 8150.048765] Open BA session requested for $AP tid 0
[ 8150.174509] Open BA session requested for $AP tid 0
[ 8150.274376] Open BA session requested for $AP tid 0

...

---
Hm, this time without the HTML-part.
---

net/mac80211/rc80211_minstrel_ht.c +2 -1
index 2158838..e19249b 100644
@@ -452,7 +452,8 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
 
 	if (time_after(jiffies, mi->stats_update + (mp->update_interval / 2 * HZ) / 1000)) {
 		minstrel_ht_update_stats(mp, mi);
-		minstrel_aggr_check(mp, sta, skb);
+		if (!(info->flags & IEEE80211_TX_CTL_AMPDU))
+			minstrel_aggr_check(mp, sta, skb);
 	}
 }
 
Acked-by on v1 1/1
#1 lore raw
Quoted 6 lines · Christian Lamparter

Minstrel HT tries very hard to establish a BA session with each peer once there's some data on the way. However the stack does not inform minstrel if an aggregation session is already in place, so it keeps trying and wastes good cycles in the tx status path.

Quoted 7 lines
[ 8149.946393] Open BA session requested for $AP tid 0
[ 8150.048765] Open BA session requested for $AP tid 0
[ 8150.174509] Open BA session requested for $AP tid 0
[ 8150.274376] Open BA session requested for $AP tid 0

...

#2 lore raw
In reply to Christian Lamparter
Quoted 11 lines · Christian Lamparter

Minstrel HT tries very hard to establish a BA session with each peer once there's some data on the way. However the stack does not inform minstrel if an aggregation session is already in place, so it keeps trying and wastes good cycles in the tx status path.

[ 8149.946393] Open BA session requested for $AP tid 0
[ 8150.048765] Open BA session requested for $AP tid 0
[ 8150.174509] Open BA session requested for $AP tid 0
[ 8150.274376] Open BA session requested for $AP tid 0

...

Hi Christian,

I got this messages in ath9k after few cycles of suspend and resume. doing the same fix in ath9k rate control tx_status callback the messages did not re-appear. I will soon send a proper patch for ath9k too.

thanks,
shafi

#3 lore raw
Quoted 15 lines · Mohammed Shafi
Quoted 11 lines · Christian Lamparter

Minstrel HT tries very hard to establish a BA session with each peer once there's some data on the way. However the stack does not inform minstrel if an aggregation session is already in place, so it keeps trying and wastes good cycles in the tx status path.

[ 8149.946393] Open BA session requested for $AP tid 0
[ 8150.048765] Open BA session requested for $AP tid 0
[ 8150.174509] Open BA session requested for $AP tid 0
[ 8150.274376] Open BA session requested for $AP tid 0

...

Quoted 3 lines

Hi Christian,

I got this messages in ath9k after few cycles of suspend and resume.

There's nothing wrong with the message... as long as its reasonable and does not spam the logs as in my case [had about 100k entries].

Quoted 3 lines

doing the same fix in ath9k rate control tx_status callback the messages did not re-appear. I will soon send a proper patch for ath9k too.

Well, ath9k's rc works different than minstrel_ht. In fact that check is not needed as ath9k's rc can check the tid state directly in ath_tx_aggr_check [which I think it already does... unless something is wrong with the txtid->state check?!].

Regards,
Chr

#4 lore raw
Quoted 16 lines · Christian Lamparter
Quoted 15 lines · Mohammed Shafi
Quoted 11 lines · Christian Lamparter

Minstrel HT tries very hard to establish a BA session with each peer once there's some data on the way. However the stack does not inform minstrel if an aggregation session is already in place, so it keeps trying and wastes good cycles in the tx status path.

[ 8149.946393] Open BA session requested for $AP tid 0
[ 8150.048765] Open BA session requested for $AP tid 0
[ 8150.174509] Open BA session requested for $AP tid 0
[ 8150.274376] Open BA session requested for $AP tid 0

...

Quoted 5 lines
Quoted 3 lines

Hi Christian,

I got this messages in ath9k after few cycles of suspend and resume.

There's nothing wrong with the message... as long as its reasonable and does not spam the logs as in my case [had about 100k entries].

indeed, it spam's the log.

Quoted 8 lines
Quoted 4 lines

doing the same fix in ath9k rate control tx_status callback the messages did not re-appear. I will soon send a proper patch for ath9k too.

Well, ath9k's rc works different than minstrel_ht. In fact that check is not needed as ath9k's rc can check the tid state directly in ath_tx_aggr_check [which I think it already does... unless something is wrong with the txtid->state check?!].

thanks, i will check into that.

thanks,
shafi