On Wed, Jan 22, 2014 at 07:51:53PM +0100, Johannes Berg wrote: > On Wed, 2014-01-22 at 17:41 +0100, Karl Beldan wrote: > > > > >> If a DELBA is sent as AC_VO it might get received before the last AMPDU > > > >> of the BlockAck session. So, the pending AMPDUs will get dropped at the > > > >> receiver. > > > >> > > > >> In theory this could also be avoided by properly flushing all pending AMPDUs > > > >> of the TID in question from the hw queues or by waiting for the tx status > > > >> of all pending AMPDUs. > > Let's get the issue straight first. > > The (current) documentation for the TX_STOP ampdu actions says: > > * @IEEE80211_AMPDU_TX_STOP_CONT: stop TX aggregation but continue > transmitting > * queued packets, now unaggregated. After all packets are > transmitted the > * driver has to call ieee80211_stop_tx_ba_cb_irqsafe(). > * @IEEE80211_AMPDU_TX_STOP_FLUSH: stop TX aggregation and flush all > packets, > * called when the station is removed. There's no need or reason to > call > * ieee80211_stop_tx_ba_cb_irqsafe() in this case as mac80211 > assumes the > * session is gone and removes the station. > * @IEEE80211_AMPDU_TX_STOP_FLUSH_CONT: called when TX aggregation is > stopped > * but the driver hasn't called ieee80211_stop_tx_ba_cb_irqsafe() > yet and > * now the connection is dropped and the station will be removed. > Drivers > * should clean up and drop remaining packets when this is called. > > I say "current" because we only completed this fairly recently (couple > months ago?) to make it complete with the three different possibilities. > > Therefore, I don't think there's a need to ever *flush* the queues, > although the term "flush" is getting confusing and we should probably > call this IEEE80211_AMPDU_TX_STOP_DROP instead of _FLUSH, as that's what > it really means, drop all the remaining frames (if any.) > > Given the fact that we only send the frame from > ieee80211_stop_tx_ba_cb() I don't see any problem. Even if we were to > send the frame directly after calling the ampdu_action, it seems it > would be fine, since the callback (now) requires sending the remaining > frames unaggregated. (Given that, I'm not even sure why we required the > packets to be sent unaggregated, Emmanuel, do you remember?) > I'd expect most device to not block ack such frames, and they'd be right to do so, sending them unaggregated seems the right thing to do. > > Callers of ieee80211_stop_tx_ba_cb_irqsafe are: > > ath9k > > ath9k_htc > > carl9170 > > wcn36xx > > brcm80211 > > iwlegacy > > iwlwifi > > mwl8k > > rt2x00 > > rtlwifi > > > > Johannes, what is your impression ? > > I'm pretty sure iwlwifi should be safe. ath9k/htc/carl9170/wcn36xx are > probably fine, I can't really say anything about the other ones. I'd > guess brcm80211 should be OK since it builds aggregates in software and > should be able to easily stop transmitting aggregates. But see above - I > don't see how even if the driver didn't stop sending aggregates it could > be wrong. Unless the driver called ieee80211_stop_tx_ba_cb() immediately > but didn't actually stop sending. That's pretty much a bug anyway > though. > So, I guess you are taking what I sent ? Karl -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
