Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mac80211: fix TX aggregation stop race

The mac80211 tear down code is not waiting for the driver call back.
This can bring down the the TX path (TID) till the user manually
reconnects. (Observed with iwldvm and enabled TX aggregation.)

The race can be prevented when the ampdu_mlme worker handles the tear
down.

The race:
* ieee80211_sta_tear_down_BA_sessions calls
___ieee80211_stop_tx_ba_session for all TIDs,

* then cancels the ampdu_mlme worker

* and cleanups the TIDs the driver already has called back for.

* ieee80211_stop_tx_ba_cb will never be called for a TID if the callback
came after the the check in ieee80211_sta_tear_down_BA_sessions.

Signed-off-by: Alexander Wetzel <Alexander.Wetzel@web.de>
[johannes: "enabled" -> "blocked" and invert logic, simplify init]
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

authored by

Alexander Wetzel and committed by
Johannes Berg
39c1134c 4a22b00b

+21 -23
+21 -23
net/mac80211/ht.c
··· 301 301 ___ieee80211_stop_tx_ba_session(sta, i, reason); 302 302 mutex_unlock(&sta->ampdu_mlme.mtx); 303 303 304 - /* stopping might queue the work again - so cancel only afterwards */ 305 - cancel_work_sync(&sta->ampdu_mlme.work); 306 - 307 304 /* 308 305 * In case the tear down is part of a reconfigure due to HW restart 309 306 * request, it is possible that the low level driver requested to stop 310 307 * the BA session, so handle it to properly clean tid_tx data. 311 308 */ 312 - mutex_lock(&sta->ampdu_mlme.mtx); 313 - for (i = 0; i < IEEE80211_NUM_TIDS; i++) { 314 - struct tid_ampdu_tx *tid_tx = 315 - rcu_dereference_protected_tid_tx(sta, i); 309 + if(reason == AGG_STOP_DESTROY_STA) { 310 + cancel_work_sync(&sta->ampdu_mlme.work); 316 311 317 - if (!tid_tx) 318 - continue; 312 + mutex_lock(&sta->ampdu_mlme.mtx); 313 + for (i = 0; i < IEEE80211_NUM_TIDS; i++) { 314 + struct tid_ampdu_tx *tid_tx = 315 + rcu_dereference_protected_tid_tx(sta, i); 319 316 320 - if (test_and_clear_bit(HT_AGG_STATE_STOP_CB, &tid_tx->state)) 321 - ieee80211_stop_tx_ba_cb(sta, i, tid_tx); 317 + if (!tid_tx) 318 + continue; 319 + 320 + if (test_and_clear_bit(HT_AGG_STATE_STOP_CB, &tid_tx->state)) 321 + ieee80211_stop_tx_ba_cb(sta, i, tid_tx); 322 + } 323 + mutex_unlock(&sta->ampdu_mlme.mtx); 322 324 } 323 - mutex_unlock(&sta->ampdu_mlme.mtx); 324 325 } 325 326 326 327 void ieee80211_ba_session_work(struct work_struct *work) ··· 329 328 struct sta_info *sta = 330 329 container_of(work, struct sta_info, ampdu_mlme.work); 331 330 struct tid_ampdu_tx *tid_tx; 331 + bool blocked; 332 332 int tid; 333 333 334 - /* 335 - * When this flag is set, new sessions should be 336 - * blocked, and existing sessions will be torn 337 - * down by the code that set the flag, so this 338 - * need not run. 339 - */ 340 - if (test_sta_flag(sta, WLAN_STA_BLOCK_BA)) 341 - return; 334 + /* When this flag is set, new sessions should be blocked. */ 335 + blocked = test_sta_flag(sta, WLAN_STA_BLOCK_BA); 342 336 343 337 mutex_lock(&sta->ampdu_mlme.mtx); 344 338 for (tid = 0; tid < IEEE80211_NUM_TIDS; tid++) { ··· 348 352 sta, tid, WLAN_BACK_RECIPIENT, 349 353 WLAN_REASON_UNSPECIFIED, true); 350 354 351 - if (test_and_clear_bit(tid, 355 + if (!blocked && 356 + test_and_clear_bit(tid, 352 357 sta->ampdu_mlme.tid_rx_manage_offl)) 353 358 ___ieee80211_start_rx_ba_session(sta, 0, 0, 0, 1, tid, 354 359 IEEE80211_MAX_AMPDU_BUF, ··· 364 367 spin_lock_bh(&sta->lock); 365 368 366 369 tid_tx = sta->ampdu_mlme.tid_start_tx[tid]; 367 - if (tid_tx) { 370 + if (!blocked && tid_tx) { 368 371 /* 369 372 * Assign it over to the normal tid_tx array 370 373 * where it "goes live". ··· 387 390 if (!tid_tx) 388 391 continue; 389 392 390 - if (test_and_clear_bit(HT_AGG_STATE_START_CB, &tid_tx->state)) 393 + if (!blocked && 394 + test_and_clear_bit(HT_AGG_STATE_START_CB, &tid_tx->state)) 391 395 ieee80211_start_tx_ba_cb(sta, tid, tid_tx); 392 396 if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state)) 393 397 ___ieee80211_stop_tx_ba_session(sta, tid,