From: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> When a packet is supposed to sent be as an a-MPDU, mac80211 sets IEEE80211_TX_CTL_AMPDU to let the driver know. On the other hand, mac80211 configures the driver for aggregration with the ampdu_action callback. There is race between these two mechanisms since the following scenario can occur when the BA agreement is torn down: Tx softIRQ drv configuration ========== ================= check OPERATIONAL bit Set the TX_CTL_AMPDU bit in the packet clear OPERATIONAL bit stop Tx AGG Pass Tx packet to the driver. In that case the driver would get a packet with TX_CTL_AMPDU set although it has already been notified that the BA session has been torn down. To fix this, we need to synchronize all the Qdisc activity after we cleared the OPERATIONAL bit. After that step, all the following packets will be buffered until the driver reports it is ready to get new packets for this RA / TID. This buffering allows not to run into another race that would send packets with TX_CTL_AMPDU unset while the driver hasn't been requested to tear down the BA session yet. This race occurs in practice and iwlwifi complains with a WARN_ON when it happens. Cc: stable@xxxxxxxxxxxxxxx Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> --- net/mac80211/agg-tx.c | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 39d72cc..d9b5340 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -188,6 +188,18 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, */ clear_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state); + /* + * There might be a few packets that have been scheduled + * before the HT_AGG_STATE_OPERATIONAL has been cleared + * and that may get to the driver only after the following + * drv_ampdu_action gets called. In that case, the driver + * would get a packet with the IEEE80211_TX_CTL_AMPDU flag + * set while it has been already notified that the BA session + * has been torn down. Sync the Qdisc to make sure this does + * not happen. + */ + synchronize_net(); + tid_tx->stop_initiator = initiator; tid_tx->tx_stop = tx; -- 1.7.1 --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. -- 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
