From 6a8579d0e62c0eac428184ce45e86bc46677724a Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 27 May 2010 14:41:07 +0200 Subject: mac80211: clean up ieee80211_stop_tx_ba_session There's no sense in letting anything but internal mac80211 functions set the initiator to anything but WLAN_BACK_INITIATOR, since WLAN_BACK_RECIPIENT is only valid when we have received a frame from the peer, which we react to directly in mac80211. The debugfs code I recently added got this wrong as well. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index c163d0a149f..feb15c4a1fa 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -538,14 +538,13 @@ int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, return ret; } -int ieee80211_stop_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid, - enum ieee80211_back_parties initiator) +int ieee80211_stop_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) { struct sta_info *sta = container_of(pubsta, struct sta_info, sta); struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_local *local = sdata->local; - trace_api_stop_tx_ba_session(pubsta, tid, initiator); + trace_api_stop_tx_ba_session(pubsta, tid); if (!local->ops->ampdu_action) return -EINVAL; @@ -553,7 +552,7 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid, if (tid >= STA_TID_NUM) return -EINVAL; - return __ieee80211_stop_tx_ba_session(sta, tid, initiator); + return __ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR); } EXPORT_SYMBOL(ieee80211_stop_tx_ba_session); -- cgit v1.2.3 From 2a419056c15478d2df3f3e9d4fa64e34eb1faa7d Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:29 +0200 Subject: mac80211: simplify station/aggregation code A number of places use RCU locking for accessing the station list, even though they do not need to. Use mutex locking instead to prepare for the locking changes I want to make. The mlme code is also using a WLAN_STA_DISASSOC flag that has the same meaning as WLAN_STA_BLOCK_BA, so use that. While doing so, combine places where we loop over stations twice, and optimise away some of the loops by checking if the hardware supports aggregation at all first. Also fix a more theoretical race condition: right now we could resume, set up an aggregation session, and right after tear it down again due to the code that is needed for hardware reconfiguration here. Also mark add a comment to that code marking it as a workaround. Finally, remove a pointless aggregation disabling loop when an interface is stopped, directly after that we remove all stations from it which will also disable all aggregation sessions that may still be active, and does so in a race-free way unlike the current loop that doesn't block new sessions. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 10 +--------- net/mac80211/iface.c | 13 ------------- net/mac80211/mlme.c | 6 +++--- net/mac80211/pm.c | 16 ++++------------ net/mac80211/sta_info.h | 4 ---- net/mac80211/util.c | 31 ++++++++++++++++--------------- 6 files changed, 24 insertions(+), 56 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index d1b6664a253..9b9f21be0ff 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -239,17 +239,9 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) sdata->vif.type != NL80211_IFTYPE_AP) return -EINVAL; - if (test_sta_flags(sta, WLAN_STA_DISASSOC)) { -#ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "Disassociation is in progress. " - "Denying BA session request\n"); -#endif - return -EINVAL; - } - if (test_sta_flags(sta, WLAN_STA_BLOCK_BA)) { #ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "Suspend in progress. " + printk(KERN_DEBUG "BA sessions blocked. " "Denying BA session request\n"); #endif return -EINVAL; diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 1afa9ec81fe..906fc2be0cf 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -339,7 +339,6 @@ static int ieee80211_stop(struct net_device *dev) { struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev); struct ieee80211_local *local = sdata->local; - struct sta_info *sta; unsigned long flags; struct sk_buff *skb, *tmp; u32 hw_reconf_flags = 0; @@ -355,18 +354,6 @@ static int ieee80211_stop(struct net_device *dev) */ ieee80211_work_purge(sdata); - /* - * Now delete all active aggregation sessions. - */ - rcu_read_lock(); - - list_for_each_entry_rcu(sta, &local->sta_list, list) { - if (sta->sdata == sdata) - ieee80211_sta_tear_down_BA_sessions(sta); - } - - rcu_read_unlock(); - /* * Remove all stations associated with this interface. * diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 1373b3dde8b..0154d74905c 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -898,13 +898,13 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata, netif_tx_stop_all_queues(sdata->dev); netif_carrier_off(sdata->dev); - rcu_read_lock(); + mutex_lock(&local->sta_mtx); sta = sta_info_get(sdata, bssid); if (sta) { - set_sta_flags(sta, WLAN_STA_DISASSOC); + set_sta_flags(sta, WLAN_STA_BLOCK_BA); ieee80211_sta_tear_down_BA_sessions(sta); } - rcu_read_unlock(); + mutex_unlock(&local->sta_mtx); changed |= ieee80211_reset_erp_info(sdata); diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c index 75202b295a4..e145a949b82 100644 --- a/net/mac80211/pm.c +++ b/net/mac80211/pm.c @@ -40,22 +40,14 @@ int __ieee80211_suspend(struct ieee80211_hw *hw) list_for_each_entry(sdata, &local->interfaces, list) ieee80211_disable_keys(sdata); - /* Tear down aggregation sessions */ - - rcu_read_lock(); - - if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) { - list_for_each_entry_rcu(sta, &local->sta_list, list) { + /* tear down aggregation sessions and remove STAs */ + mutex_lock(&local->sta_mtx); + list_for_each_entry(sta, &local->sta_list, list) { + if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) { set_sta_flags(sta, WLAN_STA_BLOCK_BA); ieee80211_sta_tear_down_BA_sessions(sta); } - } - - rcu_read_unlock(); - /* remove STAs */ - mutex_lock(&local->sta_mtx); - list_for_each_entry(sta, &local->sta_list, list) { if (sta->uploaded) { sdata = sta->sdata; if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN) diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 813da34db73..786bbd3103b 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -42,9 +42,6 @@ * be in the queues * @WLAN_STA_PSPOLL: Station sent PS-poll while driver was keeping * station in power-save mode, reply when the driver unblocks. - * @WLAN_STA_DISASSOC: Disassociation in progress. - * This is used to reject TX BA session requests when disassociation - * is in progress. */ enum ieee80211_sta_info_flags { WLAN_STA_AUTH = 1<<0, @@ -60,7 +57,6 @@ enum ieee80211_sta_info_flags { WLAN_STA_BLOCK_BA = 1<<11, WLAN_STA_PS_DRIVER = 1<<12, WLAN_STA_PSPOLL = 1<<13, - WLAN_STA_DISASSOC = 1<<14, }; #define STA_TID_NUM 16 diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 5b79d552780..a54cf146ed5 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -1138,18 +1138,6 @@ int ieee80211_reconfig(struct ieee80211_local *local) } mutex_unlock(&local->sta_mtx); - /* Clear Suspend state so that ADDBA requests can be processed */ - - rcu_read_lock(); - - if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) { - list_for_each_entry_rcu(sta, &local->sta_list, list) { - clear_sta_flags(sta, WLAN_STA_BLOCK_BA); - } - } - - rcu_read_unlock(); - /* setup RTS threshold */ drv_set_rts_threshold(local, hw->wiphy->rts_threshold); @@ -1202,13 +1190,26 @@ int ieee80211_reconfig(struct ieee80211_local *local) } } - rcu_read_lock(); + /* + * Clear the WLAN_STA_BLOCK_BA flag so new aggregation + * sessions can be established after a resume. + * + * Also tear down aggregation sessions since reconfiguring + * them in a hardware restart scenario is not easily done + * right now, and the hardware will have lost information + * about the sessions, but we and the AP still think they + * are active. This is really a workaround though. + */ if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) { - list_for_each_entry_rcu(sta, &local->sta_list, list) { + mutex_lock(&local->sta_mtx); + + list_for_each_entry(sta, &local->sta_list, list) { ieee80211_sta_tear_down_BA_sessions(sta); + clear_sta_flags(sta, WLAN_STA_BLOCK_BA); } + + mutex_unlock(&local->sta_mtx); } - rcu_read_unlock(); /* add back keys */ list_for_each_entry(sdata, &local->interfaces, list) -- cgit v1.2.3 From c1475ca99edcc7216ddc45838ab2c3281c14ba22 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:37 +0200 Subject: mac80211: move aggregation callback processing This moves the aggregation callback processing to the per-sdata skb queue and a work function rather than the tasklet. Unfortunately, this means that it extends the pkt_type hack to that skb queue. However, it will enable making ampdu_action API changes gradually, my current plan is to get rid of this again by forcing drivers to only return from ampdu_action() when everything is done, thus removing the callbacks completely. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 14 ++++++-------- net/mac80211/ieee80211_i.h | 11 +++++++---- net/mac80211/iface.c | 15 ++++++++++++--- net/mac80211/main.c | 13 ------------- net/mac80211/rx.c | 5 +++++ 5 files changed, 30 insertions(+), 28 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 9b9f21be0ff..c7b7ac40316 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -502,11 +502,10 @@ void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, ra_tid = (struct ieee80211_ra_tid *) &skb->cb; memcpy(&ra_tid->ra, ra, ETH_ALEN); ra_tid->tid = tid; - ra_tid->vif = vif; - skb->pkt_type = IEEE80211_ADDBA_MSG; - skb_queue_tail(&local->skb_queue, skb); - tasklet_schedule(&local->tasklet); + skb->pkt_type = IEEE80211_SDATA_QUEUE_AGG_START; + skb_queue_tail(&sdata->skb_queue, skb); + ieee80211_queue_work(&local->hw, &sdata->work); } EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_irqsafe); @@ -637,11 +636,10 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, ra_tid = (struct ieee80211_ra_tid *) &skb->cb; memcpy(&ra_tid->ra, ra, ETH_ALEN); ra_tid->tid = tid; - ra_tid->vif = vif; - skb->pkt_type = IEEE80211_DELBA_MSG; - skb_queue_tail(&local->skb_queue, skb); - tasklet_schedule(&local->tasklet); + skb->pkt_type = IEEE80211_SDATA_QUEUE_AGG_STOP; + skb_queue_tail(&sdata->skb_queue, skb); + ieee80211_queue_work(&local->hw, &sdata->work); } EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb_irqsafe); diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index a3bcaa3239d..bafe610dcf7 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -564,11 +564,15 @@ ieee80211_sdata_set_mesh_id(struct ieee80211_sub_if_data *sdata, #endif } +enum sdata_queue_type { + IEEE80211_SDATA_QUEUE_TYPE_FRAME = 0, + IEEE80211_SDATA_QUEUE_AGG_START = 1, + IEEE80211_SDATA_QUEUE_AGG_STOP = 2, +}; + enum { IEEE80211_RX_MSG = 1, IEEE80211_TX_STATUS_MSG = 2, - IEEE80211_DELBA_MSG = 3, - IEEE80211_ADDBA_MSG = 4, }; enum queue_stop_reason { @@ -870,9 +874,8 @@ IEEE80211_DEV_TO_SUB_IF(struct net_device *dev) return netdev_priv(dev); } -/* this struct represents 802.11n's RA/TID combination along with our vif */ +/* this struct represents 802.11n's RA/TID combination */ struct ieee80211_ra_tid { - struct ieee80211_vif *vif; u8 ra[ETH_ALEN]; u16 tid; }; diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 87fc012e4ab..9f00d3f174e 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -482,7 +482,7 @@ static int ieee80211_stop(struct net_device *dev) } /* fall through */ default: - cancel_work_sync(&sdata->work); + flush_work(&sdata->work); /* * When we get here, the interface is marked down. * Call synchronize_rcu() to wait for the RX path @@ -708,6 +708,7 @@ static void ieee80211_iface_work(struct work_struct *work) struct ieee80211_local *local = sdata->local; struct sk_buff *skb; struct sta_info *sta; + struct ieee80211_ra_tid *ra_tid; if (!ieee80211_sdata_running(sdata)) return; @@ -727,8 +728,16 @@ static void ieee80211_iface_work(struct work_struct *work) while ((skb = skb_dequeue(&sdata->skb_queue))) { struct ieee80211_mgmt *mgmt = (void *)skb->data; - if (ieee80211_is_action(mgmt->frame_control) && - mgmt->u.action.category == WLAN_CATEGORY_BACK) { + if (skb->pkt_type == IEEE80211_SDATA_QUEUE_AGG_START) { + ra_tid = (void *)&skb->cb; + ieee80211_start_tx_ba_cb(&sdata->vif, ra_tid->ra, + ra_tid->tid); + } else if (skb->pkt_type == IEEE80211_SDATA_QUEUE_AGG_STOP) { + ra_tid = (void *)&skb->cb; + ieee80211_stop_tx_ba_cb(&sdata->vif, ra_tid->ra, + ra_tid->tid); + } else if (ieee80211_is_action(mgmt->frame_control) && + mgmt->u.action.category == WLAN_CATEGORY_BACK) { int len = skb->len; rcu_read_lock(); diff --git a/net/mac80211/main.c b/net/mac80211/main.c index 88b671a16a4..a2d10d44641 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -259,7 +259,6 @@ static void ieee80211_tasklet_handler(unsigned long data) { struct ieee80211_local *local = (struct ieee80211_local *) data; struct sk_buff *skb; - struct ieee80211_ra_tid *ra_tid; while ((skb = skb_dequeue(&local->skb_queue)) || (skb = skb_dequeue(&local->skb_queue_unreliable))) { @@ -274,18 +273,6 @@ static void ieee80211_tasklet_handler(unsigned long data) skb->pkt_type = 0; ieee80211_tx_status(local_to_hw(local), skb); break; - case IEEE80211_DELBA_MSG: - ra_tid = (struct ieee80211_ra_tid *) &skb->cb; - ieee80211_stop_tx_ba_cb(ra_tid->vif, ra_tid->ra, - ra_tid->tid); - dev_kfree_skb(skb); - break; - case IEEE80211_ADDBA_MSG: - ra_tid = (struct ieee80211_ra_tid *) &skb->cb; - ieee80211_start_tx_ba_cb(ra_tid->vif, ra_tid->ra, - ra_tid->tid); - dev_kfree_skb(skb); - break ; default: WARN(1, "mac80211: Packet is of unknown type %d\n", skb->pkt_type); diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index b716fa2370b..84f11733b9f 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -741,6 +741,7 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx, sc = le16_to_cpu(hdr->seq_ctrl); if (sc & IEEE80211_SCTL_FRAG) { spin_unlock(&sta->lock); + skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb_queue_tail(&rx->sdata->skb_queue, skb); ieee80211_queue_work(&local->hw, &rx->sdata->work); return; @@ -1969,6 +1970,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) goto invalid; } + rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb_queue_tail(&sdata->skb_queue, rx->skb); ieee80211_queue_work(&local->hw, &sdata->work); return RX_QUEUED; @@ -2001,6 +2003,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) if (memcmp(mgmt->bssid, sdata->u.mgd.bssid, ETH_ALEN)) break; + rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb_queue_tail(&sdata->skb_queue, rx->skb); ieee80211_queue_work(&local->hw, &sdata->work); return RX_QUEUED; @@ -2023,6 +2026,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx) case WLAN_CATEGORY_MESH_PATH_SEL: if (!ieee80211_vif_is_mesh(&sdata->vif)) break; + rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb_queue_tail(&sdata->skb_queue, rx->skb); ieee80211_queue_work(&local->hw, &sdata->work); return RX_QUEUED; @@ -2128,6 +2132,7 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_data *rx) } /* queue up frame and kick off work to process it */ + rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb_queue_tail(&sdata->skb_queue, rx->skb); ieee80211_queue_work(&rx->local->hw, &sdata->work); -- cgit v1.2.3 From a622ab72b4dcfdf53e24b16e9530cb876979a00c Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:39 +0200 Subject: mac80211: use RCU for TX aggregation Currently we allocate some memory for each TX aggregation session and additionally keep a state bitmap indicating the state it is in. By using RCU to protect the pointer, moving the state into the structure and some locking trickery we can avoid locking when the TX agg session is fully operational. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 229 +++++++++++++++++++------------------ net/mac80211/debugfs_sta.c | 8 +- net/mac80211/ht.c | 9 +- net/mac80211/ieee80211_i.h | 2 - net/mac80211/rc80211_minstrel_ht.c | 2 +- net/mac80211/sta_info.c | 12 +- net/mac80211/sta_info.h | 33 +++--- net/mac80211/tx.c | 88 +++++++++----- 8 files changed, 206 insertions(+), 177 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index c7b7ac40316..7d8656d51c6 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -125,25 +125,42 @@ void ieee80211_send_bar(struct ieee80211_sub_if_data *sdata, u8 *ra, u16 tid, u1 ieee80211_tx_skb(sdata, skb); } -int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, - enum ieee80211_back_parties initiator) +static void kfree_tid_tx(struct rcu_head *rcu_head) +{ + struct tid_ampdu_tx *tid_tx = + container_of(rcu_head, struct tid_ampdu_tx, rcu_head); + + kfree(tid_tx); +} + +static int ___ieee80211_stop_tx_ba_session( + struct sta_info *sta, u16 tid, + enum ieee80211_back_parties initiator) { struct ieee80211_local *local = sta->local; + struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid]; int ret; - u8 *state; + + lockdep_assert_held(&sta->lock); + + if (WARN_ON(!tid_tx)) + return -ENOENT; #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n", sta->sta.addr, tid); #endif /* CONFIG_MAC80211_HT_DEBUG */ - state = &sta->ampdu_mlme.tid_state_tx[tid]; + set_bit(HT_AGG_STATE_STOPPING, &tid_tx->state); - if (*state == HT_AGG_STATE_OPERATIONAL) - sta->ampdu_mlme.addba_req_num[tid] = 0; + /* + * After this packets are no longer handed right through + * to the driver but are put onto tid_tx->pending instead, + * with locking to ensure proper access. + */ + clear_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state); - *state = HT_AGG_STATE_REQ_STOP_BA_MSK | - (initiator << HT_AGG_STATE_INITIATOR_SHIFT); + tid_tx->stop_initiator = initiator; ret = drv_ampdu_action(local, sta->sdata, IEEE80211_AMPDU_TX_STOP, @@ -174,15 +191,13 @@ static void sta_addba_resp_timer_expired(unsigned long data) u16 tid = *(u8 *)data; struct sta_info *sta = container_of((void *)data, struct sta_info, timer_to_tid[tid]); - u8 *state; - - state = &sta->ampdu_mlme.tid_state_tx[tid]; + struct tid_ampdu_tx *tid_tx; /* check if the TID waits for addBA response */ spin_lock_bh(&sta->lock); - if ((*state & (HT_ADDBA_REQUESTED_MSK | HT_ADDBA_RECEIVED_MSK | - HT_AGG_STATE_REQ_STOP_BA_MSK)) != - HT_ADDBA_REQUESTED_MSK) { + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + if (!tid_tx || + test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) { spin_unlock_bh(&sta->lock); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "timer expired on tid %d but we are not " @@ -210,7 +225,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) struct sta_info *sta = container_of(pubsta, struct sta_info, sta); struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_local *local = sdata->local; - u8 *state; + struct tid_ampdu_tx *tid_tx; int ret = 0; u16 start_seq_num; @@ -256,9 +271,9 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) goto err_unlock_sta; } - state = &sta->ampdu_mlme.tid_state_tx[tid]; + tid_tx = sta->ampdu_mlme.tid_tx[tid]; /* check if the TID is not in aggregation flow already */ - if (*state != HT_AGG_STATE_IDLE) { + if (tid_tx) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "BA request denied - session is not " "idle on tid %u\n", tid); @@ -279,9 +294,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) IEEE80211_QUEUE_STOP_REASON_AGGREGATION); /* prepare A-MPDU MLME for Tx aggregation */ - sta->ampdu_mlme.tid_tx[tid] = - kmalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC); - if (!sta->ampdu_mlme.tid_tx[tid]) { + tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC); + if (!tid_tx) { #ifdef CONFIG_MAC80211_HT_DEBUG if (net_ratelimit()) printk(KERN_ERR "allocate tx mlme to tid %d failed\n", @@ -291,33 +305,27 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) goto err_wake_queue; } - skb_queue_head_init(&sta->ampdu_mlme.tid_tx[tid]->pending); + skb_queue_head_init(&tid_tx->pending); /* Tx timer */ - sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.function = - sta_addba_resp_timer_expired; - sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.data = - (unsigned long)&sta->timer_to_tid[tid]; - init_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer); - - /* Ok, the Addba frame hasn't been sent yet, but if the driver calls the - * call back right away, it must see that the flow has begun */ - *state |= HT_ADDBA_REQUESTED_MSK; + tid_tx->addba_resp_timer.function = sta_addba_resp_timer_expired; + tid_tx->addba_resp_timer.data = (unsigned long)&sta->timer_to_tid[tid]; + init_timer(&tid_tx->addba_resp_timer); start_seq_num = sta->tid_seq[tid] >> 4; ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START, pubsta, tid, &start_seq_num); - if (ret) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "BA request denied - HW unavailable for" " tid %d\n", tid); #endif /* CONFIG_MAC80211_HT_DEBUG */ - *state = HT_AGG_STATE_IDLE; goto err_free; } + rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx); + /* Driver vetoed or OKed, but we can take packets again now */ ieee80211_wake_queue_by_reason( &local->hw, ieee80211_ac_from_tid(tid), @@ -325,32 +333,30 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) spin_unlock(&local->ampdu_lock); + /* activate the timer for the recipient's addBA response */ + tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL; + add_timer(&tid_tx->addba_resp_timer); +#ifdef CONFIG_MAC80211_HT_DEBUG + printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid); +#endif + /* prepare tid data */ sta->ampdu_mlme.dialog_token_allocator++; - sta->ampdu_mlme.tid_tx[tid]->dialog_token = - sta->ampdu_mlme.dialog_token_allocator; - sta->ampdu_mlme.tid_tx[tid]->ssn = start_seq_num; + tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator; + tid_tx->ssn = start_seq_num; + + sta->ampdu_mlme.addba_req_num[tid]++; spin_unlock_bh(&sta->lock); /* send AddBA request */ ieee80211_send_addba_request(sdata, pubsta->addr, tid, - sta->ampdu_mlme.tid_tx[tid]->dialog_token, - sta->ampdu_mlme.tid_tx[tid]->ssn, + tid_tx->dialog_token, tid_tx->ssn, 0x40, 5000); - sta->ampdu_mlme.addba_req_num[tid]++; - /* activate the timer for the recipient's addBA response */ - sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.expires = - jiffies + ADDBA_RESP_INTERVAL; - add_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer); -#ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid); -#endif return 0; err_free: - kfree(sta->ampdu_mlme.tid_tx[tid]); - sta->ampdu_mlme.tid_tx[tid] = NULL; + kfree(tid_tx); err_wake_queue: ieee80211_wake_queue_by_reason( &local->hw, ieee80211_ac_from_tid(tid), @@ -368,7 +374,8 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_session); * local->ampdu_lock across both calls. */ static void ieee80211_agg_splice_packets(struct ieee80211_local *local, - struct sta_info *sta, u16 tid) + struct tid_ampdu_tx *tid_tx, + u16 tid) { unsigned long flags; u16 queue = ieee80211_ac_from_tid(tid); @@ -377,31 +384,23 @@ static void ieee80211_agg_splice_packets(struct ieee80211_local *local, &local->hw, queue, IEEE80211_QUEUE_STOP_REASON_AGGREGATION); - if (!(sta->ampdu_mlme.tid_state_tx[tid] & HT_ADDBA_REQUESTED_MSK)) - return; - - if (WARN(!sta->ampdu_mlme.tid_tx[tid], - "TID %d gone but expected when splicing aggregates from" - "the pending queue\n", tid)) + if (WARN(!tid_tx, "TID %d gone but expected when splicing aggregates" + " from the pending queue\n", tid)) return; - if (!skb_queue_empty(&sta->ampdu_mlme.tid_tx[tid]->pending)) { + if (!skb_queue_empty(&tid_tx->pending)) { spin_lock_irqsave(&local->queue_stop_reason_lock, flags); /* copy over remaining packets */ - skb_queue_splice_tail_init( - &sta->ampdu_mlme.tid_tx[tid]->pending, - &local->pending[queue]); + skb_queue_splice_tail_init(&tid_tx->pending, + &local->pending[queue]); spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags); } } -static void ieee80211_agg_splice_finish(struct ieee80211_local *local, - struct sta_info *sta, u16 tid) +static void ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid) { - u16 queue = ieee80211_ac_from_tid(tid); - ieee80211_wake_queue_by_reason( - &local->hw, queue, + &local->hw, ieee80211_ac_from_tid(tid), IEEE80211_QUEUE_STOP_REASON_AGGREGATION); } @@ -409,19 +408,21 @@ static void ieee80211_agg_splice_finish(struct ieee80211_local *local, static void ieee80211_agg_tx_operational(struct ieee80211_local *local, struct sta_info *sta, u16 tid) { + lockdep_assert_held(&sta->lock); + #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid); #endif spin_lock(&local->ampdu_lock); - ieee80211_agg_splice_packets(local, sta, tid); + ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid); /* - * NB: we rely on sta->lock being taken in the TX - * processing here when adding to the pending queue, - * otherwise we could only change the state of the - * session to OPERATIONAL _here_. + * Now mark as operational. This will be visible + * in the TX path, and lets it go lock-free in + * the common case. */ - ieee80211_agg_splice_finish(local, sta, tid); + set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state); + ieee80211_agg_splice_finish(local, tid); spin_unlock(&local->ampdu_lock); drv_ampdu_action(local, sta->sdata, @@ -434,7 +435,7 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid) struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif); struct ieee80211_local *local = sdata->local; struct sta_info *sta; - u8 *state; + struct tid_ampdu_tx *tid_tx; trace_api_start_tx_ba_cb(sdata, ra, tid); @@ -456,25 +457,22 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid) return; } - state = &sta->ampdu_mlme.tid_state_tx[tid]; spin_lock_bh(&sta->lock); + tid_tx = sta->ampdu_mlme.tid_tx[tid]; - if (WARN_ON(!(*state & HT_ADDBA_REQUESTED_MSK))) { + if (WARN_ON(!tid_tx)) { #ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "addBA was not requested yet, state is %d\n", - *state); + printk(KERN_DEBUG "addBA was not requested!\n"); #endif spin_unlock_bh(&sta->lock); rcu_read_unlock(); return; } - if (WARN_ON(*state & HT_ADDBA_DRV_READY_MSK)) + if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state))) goto out; - *state |= HT_ADDBA_DRV_READY_MSK; - - if (*state == HT_AGG_STATE_OPERATIONAL) + if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) ieee80211_agg_tx_operational(local, sta, tid); out: @@ -512,14 +510,14 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_irqsafe); int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator) { - u8 *state; + struct tid_ampdu_tx *tid_tx; int ret; - /* check if the TID is in aggregation */ - state = &sta->ampdu_mlme.tid_state_tx[tid]; spin_lock_bh(&sta->lock); + tid_tx = sta->ampdu_mlme.tid_tx[tid]; - if (*state != HT_AGG_STATE_OPERATIONAL) { + /* check if the TID is in aggregation */ + if (!tid_tx || !test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) { ret = -ENOENT; goto unlock; } @@ -554,7 +552,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif); struct ieee80211_local *local = sdata->local; struct sta_info *sta; - u8 *state; + struct tid_ampdu_tx *tid_tx; trace_api_stop_tx_ba_cb(sdata, ra, tid); @@ -580,39 +578,45 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) rcu_read_unlock(); return; } - state = &sta->ampdu_mlme.tid_state_tx[tid]; - /* NOTE: no need to use sta->lock in this state check, as - * ieee80211_stop_tx_ba_session will let only one stop call to - * pass through per sta/tid - */ - if ((*state & HT_AGG_STATE_REQ_STOP_BA_MSK) == 0) { + spin_lock_bh(&sta->lock); + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + + if (!tid_tx || !test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n"); #endif + spin_unlock_bh(&sta->lock); rcu_read_unlock(); return; } - if (*state & HT_AGG_STATE_INITIATOR_MSK) + if (tid_tx->stop_initiator == WLAN_BACK_INITIATOR) ieee80211_send_delba(sta->sdata, ra, tid, WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE); - spin_lock_bh(&sta->lock); - spin_lock(&local->ampdu_lock); + /* + * When we get here, the TX path will not be lockless any more wrt. + * aggregation, since the OPERATIONAL bit has long been cleared. + * Thus it will block on getting the lock, if it occurs. So if we + * stop the queue now, we will not get any more packets, and any + * that might be being processed will wait for us here, thereby + * guaranteeing that no packets go to the tid_tx pending queue any + * more. + */ - ieee80211_agg_splice_packets(local, sta, tid); + spin_lock(&local->ampdu_lock); + ieee80211_agg_splice_packets(local, tid_tx, tid); - *state = HT_AGG_STATE_IDLE; - /* from now on packets are no longer put onto sta->pending */ - kfree(sta->ampdu_mlme.tid_tx[tid]); - sta->ampdu_mlme.tid_tx[tid] = NULL; + /* future packets must not find the tid_tx struct any more */ + rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); - ieee80211_agg_splice_finish(local, sta, tid); + ieee80211_agg_splice_finish(local, tid); + call_rcu(&tid_tx->rcu_head, kfree_tid_tx); spin_unlock(&local->ampdu_lock); - spin_unlock_bh(&sta->lock); + spin_unlock_bh(&sta->lock); rcu_read_unlock(); } EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb); @@ -649,40 +653,41 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, struct ieee80211_mgmt *mgmt, size_t len) { + struct tid_ampdu_tx *tid_tx; u16 capab, tid; - u8 *state; capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab); tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2; - state = &sta->ampdu_mlme.tid_state_tx[tid]; - spin_lock_bh(&sta->lock); - if (!(*state & HT_ADDBA_REQUESTED_MSK)) + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + + if (!tid_tx) goto out; - if (mgmt->u.action.u.addba_resp.dialog_token != - sta->ampdu_mlme.tid_tx[tid]->dialog_token) { + if (mgmt->u.action.u.addba_resp.dialog_token != tid_tx->dialog_token) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "wrong addBA response token, tid %d\n", tid); -#endif /* CONFIG_MAC80211_HT_DEBUG */ +#endif goto out; } - del_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer); + del_timer(&tid_tx->addba_resp_timer); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "switched off addBA timer for tid %d\n", tid); -#endif /* CONFIG_MAC80211_HT_DEBUG */ +#endif if (le16_to_cpu(mgmt->u.action.u.addba_resp.status) == WLAN_STATUS_SUCCESS) { - u8 curstate = *state; - - *state |= HT_ADDBA_RECEIVED_MSK; + if (test_and_set_bit(HT_AGG_STATE_RESPONSE_RECEIVED, + &tid_tx->state)) { + /* ignore duplicate response */ + goto out; + } - if (*state != curstate && *state == HT_AGG_STATE_OPERATIONAL) + if (test_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state)) ieee80211_agg_tx_operational(local, sta, tid); sta->ampdu_mlme.addba_req_num[tid] = 0; diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index 95c1ea47ad3..8a74ffb36ad 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -134,15 +134,15 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, sta->ampdu_mlme.tid_rx[i]->ssn : 0); p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x", - sta->ampdu_mlme.tid_state_tx[i]); + !!sta->ampdu_mlme.tid_tx[i]); p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x", - sta->ampdu_mlme.tid_state_tx[i] ? + sta->ampdu_mlme.tid_tx[i] ? sta->ampdu_mlme.tid_tx[i]->dialog_token : 0); p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x", - sta->ampdu_mlme.tid_state_tx[i] ? + sta->ampdu_mlme.tid_tx[i] ? sta->ampdu_mlme.tid_tx[i]->ssn : 0); p += scnprintf(p, sizeof(buf) + buf - p, "\t%03d", - sta->ampdu_mlme.tid_state_tx[i] ? + sta->ampdu_mlme.tid_tx[i] ? skb_queue_len(&sta->ampdu_mlme.tid_tx[i]->pending) : 0); p += scnprintf(p, sizeof(buf) + buf - p, "\n"); } diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index 2ab106a0a49..1af173ed2d5 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -176,13 +176,8 @@ void ieee80211_process_delba(struct ieee80211_sub_if_data *sdata, if (initiator == WLAN_BACK_INITIATOR) __ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_INITIATOR, 0); - else { /* WLAN_BACK_RECIPIENT */ - spin_lock_bh(&sta->lock); - if (sta->ampdu_mlme.tid_state_tx[tid] & HT_ADDBA_REQUESTED_MSK) - ___ieee80211_stop_tx_ba_session(sta, tid, - WLAN_BACK_RECIPIENT); - spin_unlock_bh(&sta->lock); - } + else + __ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_RECIPIENT); } int ieee80211_send_smps_action(struct ieee80211_sub_if_data *sdata, diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index bafe610dcf7..71bdd8b3c3f 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1119,8 +1119,6 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator); -int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, - enum ieee80211_back_parties initiator); /* Spectrum management */ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata, diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c index c23f08251da..7a04951fcb1 100644 --- a/net/mac80211/rc80211_minstrel_ht.c +++ b/net/mac80211/rc80211_minstrel_ht.c @@ -365,7 +365,7 @@ minstrel_aggr_check(struct minstrel_priv *mp, struct ieee80211_sta *pubsta, stru return; tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK; - if (likely(sta->ampdu_mlme.tid_state_tx[tid] != HT_AGG_STATE_IDLE)) + if (likely(sta->ampdu_mlme.tid_tx[tid])) return; ieee80211_start_tx_ba_session(pubsta, tid); diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index c426c572d98..06d8e00a253 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -246,14 +246,12 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, } for (i = 0; i < STA_TID_NUM; i++) { - /* timer_to_tid must be initialized with identity mapping to - * enable session_timer's data differentiation. refer to - * sta_rx_agg_session_timer_expired for useage */ + /* + * timer_to_tid must be initialized with identity mapping + * to enable session_timer's data differentiation. See + * sta_rx_agg_session_timer_expired for usage. + */ sta->timer_to_tid[i] = i; - /* tx */ - sta->ampdu_mlme.tid_state_tx[i] = HT_AGG_STATE_IDLE; - sta->ampdu_mlme.tid_tx[i] = NULL; - sta->ampdu_mlme.addba_req_num[i] = 0; } skb_queue_head_init(&sta->ps_tx_buf); skb_queue_head_init(&sta->tx_filtered); diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 16864a6045b..05278e6288c 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -61,33 +61,40 @@ enum ieee80211_sta_info_flags { #define STA_TID_NUM 16 #define ADDBA_RESP_INTERVAL HZ -#define HT_AGG_MAX_RETRIES (0x3) +#define HT_AGG_MAX_RETRIES 0x3 -#define HT_AGG_STATE_INITIATOR_SHIFT (4) - -#define HT_ADDBA_REQUESTED_MSK BIT(0) -#define HT_ADDBA_DRV_READY_MSK BIT(1) -#define HT_ADDBA_RECEIVED_MSK BIT(2) -#define HT_AGG_STATE_REQ_STOP_BA_MSK BIT(3) -#define HT_AGG_STATE_INITIATOR_MSK BIT(HT_AGG_STATE_INITIATOR_SHIFT) -#define HT_AGG_STATE_IDLE (0x0) -#define HT_AGG_STATE_OPERATIONAL (HT_ADDBA_REQUESTED_MSK | \ - HT_ADDBA_DRV_READY_MSK | \ - HT_ADDBA_RECEIVED_MSK) +#define HT_AGG_STATE_DRV_READY 0 +#define HT_AGG_STATE_RESPONSE_RECEIVED 1 +#define HT_AGG_STATE_OPERATIONAL 2 +#define HT_AGG_STATE_STOPPING 3 /** * struct tid_ampdu_tx - TID aggregation information (Tx). * + * @rcu_head: rcu head for freeing structure * @addba_resp_timer: timer for peer's response to addba request * @pending: pending frames queue -- use sta's spinlock to protect * @ssn: Starting Sequence Number expected to be aggregated. * @dialog_token: dialog token for aggregation session + * @state: session state (see above) + * @stop_initiator: initiator of a session stop + * + * This structure is protected by RCU and the per-station + * spinlock. Assignments to the array holding it must hold + * the spinlock, only the TX path can access it under RCU + * lock-free if, and only if, the state has the flag + * %HT_AGG_STATE_OPERATIONAL set. Otherwise, the TX path + * must also acquire the spinlock and re-check the state, + * see comments in the tx code touching it. */ struct tid_ampdu_tx { + struct rcu_head rcu_head; struct timer_list addba_resp_timer; struct sk_buff_head pending; + unsigned long state; u16 ssn; u8 dialog_token; + u8 stop_initiator; }; /** @@ -129,7 +136,6 @@ struct tid_ampdu_rx { * struct sta_ampdu_mlme - STA aggregation information. * * @tid_rx: aggregation info for Rx per TID -- RCU protected - * @tid_state_tx: TID's state in Tx session state machine. * @tid_tx: aggregation info for Tx per TID * @addba_req_num: number of times addBA request has been sent. * @dialog_token_allocator: dialog token enumerator for each new session; @@ -138,7 +144,6 @@ struct sta_ampdu_mlme { /* rx */ struct tid_ampdu_rx *tid_rx[STA_TID_NUM]; /* tx */ - u8 tid_state_tx[STA_TID_NUM]; struct tid_ampdu_tx *tid_tx[STA_TID_NUM]; u8 addba_req_num[STA_TID_NUM]; u8 dialog_token_allocator; diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 680bcb7093d..7bf1f9c9ea3 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1092,6 +1092,54 @@ static bool __ieee80211_parse_tx_radiotap(struct ieee80211_tx_data *tx, return true; } +static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx, + struct sk_buff *skb, + struct ieee80211_tx_info *info, + struct tid_ampdu_tx *tid_tx, + int tid) +{ + bool queued = false; + + if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) { + info->flags |= IEEE80211_TX_CTL_AMPDU; + } else { + spin_lock(&tx->sta->lock); + /* + * Need to re-check now, because we may get here + * + * 1) in the window during which the setup is actually + * already done, but not marked yet because not all + * packets are spliced over to the driver pending + * queue yet -- if this happened we acquire the lock + * either before or after the splice happens, but + * need to recheck which of these cases happened. + * + * 2) during session teardown, if the OPERATIONAL bit + * was cleared due to the teardown but the pointer + * hasn't been assigned NULL yet (or we loaded it + * before it was assigned) -- in this case it may + * now be NULL which means we should just let the + * packet pass through because splicing the frames + * back is already done. + */ + tid_tx = tx->sta->ampdu_mlme.tid_tx[tid]; + + if (!tid_tx) { + /* do nothing, let packet pass through */ + } else if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) { + info->flags |= IEEE80211_TX_CTL_AMPDU; + } else { + queued = true; + info->control.vif = &tx->sdata->vif; + info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING; + __skb_queue_tail(&tid_tx->pending, skb); + } + spin_unlock(&tx->sta->lock); + } + + return queued; +} + /* * initialises @tx */ @@ -1104,8 +1152,7 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata, struct ieee80211_hdr *hdr; struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb); int hdrlen, tid; - u8 *qc, *state; - bool queued = false; + u8 *qc; memset(tx, 0, sizeof(*tx)); tx->skb = skb; @@ -1157,35 +1204,16 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata, qc = ieee80211_get_qos_ctl(hdr); tid = *qc & IEEE80211_QOS_CTL_TID_MASK; - spin_lock(&tx->sta->lock); - /* - * XXX: This spinlock could be fairly expensive, but see the - * comment in agg-tx.c:ieee80211_agg_tx_operational(). - * One way to solve this would be to do something RCU-like - * for managing the tid_tx struct and using atomic bitops - * for the actual state -- by introducing an actual - * 'operational' bit that would be possible. It would - * require changing ieee80211_agg_tx_operational() to - * set that bit, and changing the way tid_tx is managed - * everywhere, including races between that bit and - * tid_tx going away (tid_tx being added can be easily - * committed to memory before the 'operational' bit). - */ - tid_tx = tx->sta->ampdu_mlme.tid_tx[tid]; - state = &tx->sta->ampdu_mlme.tid_state_tx[tid]; - if (*state == HT_AGG_STATE_OPERATIONAL) { - info->flags |= IEEE80211_TX_CTL_AMPDU; - } else if (*state != HT_AGG_STATE_IDLE) { - /* in progress */ - queued = true; - info->control.vif = &sdata->vif; - info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING; - __skb_queue_tail(&tid_tx->pending, skb); - } - spin_unlock(&tx->sta->lock); + tid_tx = rcu_dereference(tx->sta->ampdu_mlme.tid_tx[tid]); + if (tid_tx) { + bool queued; - if (unlikely(queued)) - return TX_QUEUED; + queued = ieee80211_tx_prep_agg(tx, skb, info, + tid_tx, tid); + + if (unlikely(queued)) + return TX_QUEUED; + } } if (is_multicast_ether_addr(hdr->addr1)) { -- cgit v1.2.3 From 5d22c89b9bea17a0e48e7534a9b237885e2c0809 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:40 +0200 Subject: mac80211: remove non-irqsafe aggregation callbacks The non-irqsafe aggregation start/stop done callbacks are currently only used by ath9k_htc, and can cause callbacks into the driver again. This might lead to locking issues, which will only get worse as we modify locking. To avoid trouble, remove the non-irqsafe versions and change ath9k_htc to use those instead. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- drivers/net/wireless/ath/ath9k/htc_drv_main.c | 6 ++--- include/net/mac80211.h | 32 +++++---------------------- net/mac80211/agg-tx.c | 2 -- net/mac80211/ieee80211_i.h | 2 ++ 4 files changed, 10 insertions(+), 32 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c index 7aefbc63877..8c463f5965f 100644 --- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c +++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c @@ -510,13 +510,13 @@ void ath9k_htc_aggr_work(struct work_struct *work) ret = ath9k_htc_aggr_oper(priv, wk->vif, wk->sta_addr, wk->tid, true); if (!ret) - ieee80211_start_tx_ba_cb(wk->vif, wk->sta_addr, - wk->tid); + ieee80211_start_tx_ba_cb_irqsafe(wk->vif, wk->sta_addr, + wk->tid); break; case IEEE80211_AMPDU_TX_STOP: ath9k_htc_aggr_oper(priv, wk->vif, wk->sta_addr, wk->tid, false); - ieee80211_stop_tx_ba_cb(wk->vif, wk->sta_addr, wk->tid); + ieee80211_stop_tx_ba_cb_irqsafe(wk->vif, wk->sta_addr, wk->tid); break; default: ath_print(ath9k_hw_common(priv->ah), ATH_DBG_FATAL, diff --git a/include/net/mac80211.h b/include/net/mac80211.h index abb3b1a9ddc..7f9401b3d3c 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -1445,7 +1445,7 @@ enum ieee80211_filter_flags { * * Note that drivers MUST be able to deal with a TX aggregation * session being stopped even before they OK'ed starting it by - * calling ieee80211_start_tx_ba_cb(_irqsafe), because the peer + * calling ieee80211_start_tx_ba_cb_irqsafe, because the peer * might receive the addBA frame and send a delBA right away! * * @IEEE80211_AMPDU_RX_START: start Rx aggregation @@ -2313,17 +2313,6 @@ void ieee80211_queue_delayed_work(struct ieee80211_hw *hw, */ int ieee80211_start_tx_ba_session(struct ieee80211_sta *sta, u16 tid); -/** - * ieee80211_start_tx_ba_cb - low level driver ready to aggregate. - * @vif: &struct ieee80211_vif pointer from the add_interface callback - * @ra: receiver address of the BA session recipient. - * @tid: the TID to BA on. - * - * This function must be called by low level driver once it has - * finished with preparations for the BA session. - */ -void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid); - /** * ieee80211_start_tx_ba_cb_irqsafe - low level driver ready to aggregate. * @vif: &struct ieee80211_vif pointer from the add_interface callback @@ -2331,8 +2320,8 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid); * @tid: the TID to BA on. * * This function must be called by low level driver once it has - * finished with preparations for the BA session. - * This version of the function is IRQ-safe. + * finished with preparations for the BA session. It can be called + * from any context. */ void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra, u16 tid); @@ -2350,17 +2339,6 @@ void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra, */ int ieee80211_stop_tx_ba_session(struct ieee80211_sta *sta, u16 tid); -/** - * ieee80211_stop_tx_ba_cb - low level driver ready to stop aggregate. - * @vif: &struct ieee80211_vif pointer from the add_interface callback - * @ra: receiver address of the BA session recipient. - * @tid: the desired TID to BA on. - * - * This function must be called by low level driver once it has - * finished with preparations for the BA session tear down. - */ -void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid); - /** * ieee80211_stop_tx_ba_cb_irqsafe - low level driver ready to stop aggregate. * @vif: &struct ieee80211_vif pointer from the add_interface callback @@ -2368,8 +2346,8 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid); * @tid: the desired TID to BA on. * * This function must be called by low level driver once it has - * finished with preparations for the BA session tear down. - * This version of the function is IRQ-safe. + * finished with preparations for the BA session tear down. It + * can be called from any context. */ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra, u16 tid); diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 7d8656d51c6..5a7ef51e302 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -479,7 +479,6 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid) spin_unlock_bh(&sta->lock); rcu_read_unlock(); } -EXPORT_SYMBOL(ieee80211_start_tx_ba_cb); void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra, u16 tid) @@ -619,7 +618,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) spin_unlock_bh(&sta->lock); rcu_read_unlock(); } -EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb); void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra, u16 tid) diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 71bdd8b3c3f..a3ae5130803 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1119,6 +1119,8 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator); +void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid); +void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid); /* Spectrum management */ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata, -- cgit v1.2.3 From a6a67db2bc89d2b1ff07e0817f11235c20d2c329 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:41 +0200 Subject: mac80211: refcount aggregation queue stop mac80211 currently maintains the ampdu_lock to avoid starting a queue due to one aggregation session while another aggregation session needs the queue stopped. We can do better, however, and instead refcount the queue stops for this particular purpose, thus removing the need for the lock. This will help making ampdu_action able to sleep. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 77 ++++++++++++++++++++++++++++------------------ net/mac80211/ieee80211_i.h | 8 +---- net/mac80211/main.c | 6 ++-- 3 files changed, 51 insertions(+), 40 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 5a7ef51e302..7f2042d3790 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -220,6 +220,41 @@ static inline int ieee80211_ac_from_tid(int tid) return ieee802_1d_to_ac[tid & 7]; } +/* + * When multiple aggregation sessions on multiple stations + * are being created/destroyed simultaneously, we need to + * refcount the global queue stop caused by that in order + * to not get into a situation where one of the aggregation + * setup or teardown re-enables queues before the other is + * ready to handle that. + * + * These two functions take care of this issue by keeping + * a global "agg_queue_stop" refcount. + */ +static void __acquires(agg_queue) +ieee80211_stop_queue_agg(struct ieee80211_local *local, int tid) +{ + int queue = ieee80211_ac_from_tid(tid); + + if (atomic_inc_return(&local->agg_queue_stop[queue]) == 1) + ieee80211_stop_queue_by_reason( + &local->hw, queue, + IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + __acquire(agg_queue); +} + +static void __releases(agg_queue) +ieee80211_wake_queue_agg(struct ieee80211_local *local, int tid) +{ + int queue = ieee80211_ac_from_tid(tid); + + if (atomic_dec_return(&local->agg_queue_stop[queue]) == 0) + ieee80211_wake_queue_by_reason( + &local->hw, queue, + IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + __release(agg_queue); +} + int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) { struct sta_info *sta = container_of(pubsta, struct sta_info, sta); @@ -263,7 +298,6 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) } spin_lock_bh(&sta->lock); - spin_lock(&local->ampdu_lock); /* we have tried too many times, receiver does not want A-MPDU */ if (sta->ampdu_mlme.addba_req_num[tid] > HT_AGG_MAX_RETRIES) { @@ -289,9 +323,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) * which would require us to put them to the AC pending * afterwards which just makes the code more complex. */ - ieee80211_stop_queue_by_reason( - &local->hw, ieee80211_ac_from_tid(tid), - IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + ieee80211_stop_queue_agg(local, tid); /* prepare A-MPDU MLME for Tx aggregation */ tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC); @@ -327,11 +359,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx); /* Driver vetoed or OKed, but we can take packets again now */ - ieee80211_wake_queue_by_reason( - &local->hw, ieee80211_ac_from_tid(tid), - IEEE80211_QUEUE_STOP_REASON_AGGREGATION); - - spin_unlock(&local->ampdu_lock); + ieee80211_wake_queue_agg(local, tid); /* activate the timer for the recipient's addBA response */ tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL; @@ -358,11 +386,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) err_free: kfree(tid_tx); err_wake_queue: - ieee80211_wake_queue_by_reason( - &local->hw, ieee80211_ac_from_tid(tid), - IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + ieee80211_wake_queue_agg(local, tid); err_unlock_sta: - spin_unlock(&local->ampdu_lock); spin_unlock_bh(&sta->lock); return ret; } @@ -370,19 +395,16 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_session); /* * splice packets from the STA's pending to the local pending, - * requires a call to ieee80211_agg_splice_finish and holding - * local->ampdu_lock across both calls. + * requires a call to ieee80211_agg_splice_finish later */ -static void ieee80211_agg_splice_packets(struct ieee80211_local *local, - struct tid_ampdu_tx *tid_tx, - u16 tid) +static void __acquires(agg_queue) +ieee80211_agg_splice_packets(struct ieee80211_local *local, + struct tid_ampdu_tx *tid_tx, u16 tid) { + int queue = ieee80211_ac_from_tid(tid); unsigned long flags; - u16 queue = ieee80211_ac_from_tid(tid); - ieee80211_stop_queue_by_reason( - &local->hw, queue, - IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + ieee80211_stop_queue_agg(local, tid); if (WARN(!tid_tx, "TID %d gone but expected when splicing aggregates" " from the pending queue\n", tid)) @@ -397,11 +419,10 @@ static void ieee80211_agg_splice_packets(struct ieee80211_local *local, } } -static void ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid) +static void __releases(agg_queue) +ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid) { - ieee80211_wake_queue_by_reason( - &local->hw, ieee80211_ac_from_tid(tid), - IEEE80211_QUEUE_STOP_REASON_AGGREGATION); + ieee80211_wake_queue_agg(local, tid); } /* caller must hold sta->lock */ @@ -414,7 +435,6 @@ static void ieee80211_agg_tx_operational(struct ieee80211_local *local, printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid); #endif - spin_lock(&local->ampdu_lock); ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid); /* * Now mark as operational. This will be visible @@ -423,7 +443,6 @@ static void ieee80211_agg_tx_operational(struct ieee80211_local *local, */ set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state); ieee80211_agg_splice_finish(local, tid); - spin_unlock(&local->ampdu_lock); drv_ampdu_action(local, sta->sdata, IEEE80211_AMPDU_TX_OPERATIONAL, @@ -604,7 +623,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) * more. */ - spin_lock(&local->ampdu_lock); ieee80211_agg_splice_packets(local, tid_tx, tid); /* future packets must not find the tid_tx struct any more */ @@ -613,7 +631,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) ieee80211_agg_splice_finish(local, tid); call_rcu(&tid_tx->rcu_head, kfree_tid_tx); - spin_unlock(&local->ampdu_lock); spin_unlock_bh(&sta->lock); rcu_read_unlock(); diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index a3ae5130803..8a91b5d8387 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -723,13 +723,7 @@ struct ieee80211_local { struct sk_buff_head pending[IEEE80211_MAX_QUEUES]; struct tasklet_struct tx_pending_tasklet; - /* - * This lock is used to prevent concurrent A-MPDU - * session start/stop processing, this thus also - * synchronises the ->ampdu_action() callback to - * drivers and limits it to one at a time. - */ - spinlock_t ampdu_lock; + atomic_t agg_queue_stop[IEEE80211_MAX_QUEUES]; /* number of interfaces with corresponding IFF_ flags */ atomic_t iff_allmultis, iff_promiscs; diff --git a/net/mac80211/main.c b/net/mac80211/main.c index a2d10d44641..c2e46e88f3c 100644 --- a/net/mac80211/main.c +++ b/net/mac80211/main.c @@ -463,8 +463,10 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len, sta_info_init(local); - for (i = 0; i < IEEE80211_MAX_QUEUES; i++) + for (i = 0; i < IEEE80211_MAX_QUEUES; i++) { skb_queue_head_init(&local->pending[i]); + atomic_set(&local->agg_queue_stop[i], 0); + } tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending, (unsigned long)local); @@ -475,8 +477,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len, skb_queue_head_init(&local->skb_queue); skb_queue_head_init(&local->skb_queue_unreliable); - spin_lock_init(&local->ampdu_lock); - return local_to_hw(local); } EXPORT_SYMBOL(ieee80211_alloc_hw); -- cgit v1.2.3 From 0ab337032a0dfcd5f2527d3306d3deeba5f95b59 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:42 +0200 Subject: mac80211: make TX aggregation start/stop request async When the driver or rate control requests starting or stopping an aggregation session, that currently causes a direct callback into the driver, which could potentially cause locking problems. Also, the functions need to be callable from contexts that cannot sleep, and thus will interfere with making the ampdu_action callback sleeping. To address these issues, add a new work item for each station that will process any start or stop requests out of line. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 177 ++++++++++++++++++++++++++++++++------------- net/mac80211/debugfs_sta.c | 5 +- net/mac80211/ht.c | 2 + net/mac80211/ieee80211_i.h | 1 + net/mac80211/sta_info.c | 1 + net/mac80211/sta_info.h | 6 +- net/mac80211/tx.c | 5 ++ 7 files changed, 139 insertions(+), 58 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 7f2042d3790..ee8bfe18b48 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -146,6 +146,13 @@ static int ___ieee80211_stop_tx_ba_session( if (WARN_ON(!tid_tx)) return -ENOENT; + if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) { + /* not even started yet! */ + rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); + call_rcu(&tid_tx->rcu_head, kfree_tid_tx); + return 0; + } + #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n", sta->sta.addr, tid); @@ -255,6 +262,94 @@ ieee80211_wake_queue_agg(struct ieee80211_local *local, int tid) __release(agg_queue); } +static void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) +{ + struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid]; + struct ieee80211_local *local = sta->local; + struct ieee80211_sub_if_data *sdata = sta->sdata; + u16 start_seq_num; + int ret; + + /* + * While we're asking the driver about the aggregation, + * stop the AC queue so that we don't have to worry + * about frames that came in while we were doing that, + * which would require us to put them to the AC pending + * afterwards which just makes the code more complex. + */ + ieee80211_stop_queue_agg(local, tid); + + clear_bit(HT_AGG_STATE_WANT_START, &tid_tx->state); + + /* + * This might be off by one due to a race that we can't + * really prevent here without synchronize_net() which + * can't be called now. + */ + start_seq_num = sta->tid_seq[tid] >> 4; + + ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START, + &sta->sta, tid, &start_seq_num); + if (ret) { +#ifdef CONFIG_MAC80211_HT_DEBUG + printk(KERN_DEBUG "BA request denied - HW unavailable for" + " tid %d\n", tid); +#endif + rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); + ieee80211_wake_queue_agg(local, tid); + call_rcu(&tid_tx->rcu_head, kfree_tid_tx); + return; + } + + /* we can take packets again now */ + ieee80211_wake_queue_agg(local, tid); + + /* activate the timer for the recipient's addBA response */ + mod_timer(&tid_tx->addba_resp_timer, jiffies + ADDBA_RESP_INTERVAL); +#ifdef CONFIG_MAC80211_HT_DEBUG + printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid); +#endif + + sta->ampdu_mlme.addba_req_num[tid]++; + + /* send AddBA request */ + ieee80211_send_addba_request(sdata, sta->sta.addr, tid, + tid_tx->dialog_token, start_seq_num, + 0x40, 5000); +} + +void ieee80211_tx_ba_session_work(struct work_struct *work) +{ + struct sta_info *sta = + container_of(work, struct sta_info, ampdu_mlme.work); + struct tid_ampdu_tx *tid_tx; + int tid; + + /* + * When this flag is set, new sessions should be + * blocked, and existing sessions will be torn + * down by the code that set the flag, so this + * need not run. + */ + if (test_sta_flags(sta, WLAN_STA_BLOCK_BA)) + return; + + spin_lock_bh(&sta->lock); + for (tid = 0; tid < STA_TID_NUM; tid++) { + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + if (!tid_tx) + continue; + + if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) + ieee80211_tx_ba_session_handle_start(sta, tid); + else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, + &tid_tx->state)) + ___ieee80211_stop_tx_ba_session(sta, tid, + WLAN_BACK_INITIATOR); + } + spin_unlock_bh(&sta->lock); +} + int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) { struct sta_info *sta = container_of(pubsta, struct sta_info, sta); @@ -262,7 +357,6 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) struct ieee80211_local *local = sdata->local; struct tid_ampdu_tx *tid_tx; int ret = 0; - u16 start_seq_num; trace_api_start_tx_ba_session(pubsta, tid); @@ -316,15 +410,6 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) goto err_unlock_sta; } - /* - * While we're asking the driver about the aggregation, - * stop the AC queue so that we don't have to worry - * about frames that came in while we were doing that, - * which would require us to put them to the AC pending - * afterwards which just makes the code more complex. - */ - ieee80211_stop_queue_agg(local, tid); - /* prepare A-MPDU MLME for Tx aggregation */ tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC); if (!tid_tx) { @@ -334,59 +419,27 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) tid); #endif ret = -ENOMEM; - goto err_wake_queue; + goto err_unlock_sta; } skb_queue_head_init(&tid_tx->pending); + __set_bit(HT_AGG_STATE_WANT_START, &tid_tx->state); /* Tx timer */ tid_tx->addba_resp_timer.function = sta_addba_resp_timer_expired; tid_tx->addba_resp_timer.data = (unsigned long)&sta->timer_to_tid[tid]; init_timer(&tid_tx->addba_resp_timer); - start_seq_num = sta->tid_seq[tid] >> 4; - - ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START, - pubsta, tid, &start_seq_num); - if (ret) { -#ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "BA request denied - HW unavailable for" - " tid %d\n", tid); -#endif /* CONFIG_MAC80211_HT_DEBUG */ - goto err_free; - } - - rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx); - - /* Driver vetoed or OKed, but we can take packets again now */ - ieee80211_wake_queue_agg(local, tid); - - /* activate the timer for the recipient's addBA response */ - tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL; - add_timer(&tid_tx->addba_resp_timer); -#ifdef CONFIG_MAC80211_HT_DEBUG - printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid); -#endif - - /* prepare tid data */ + /* assign a dialog token */ sta->ampdu_mlme.dialog_token_allocator++; tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator; - tid_tx->ssn = start_seq_num; - - sta->ampdu_mlme.addba_req_num[tid]++; - spin_unlock_bh(&sta->lock); + /* finally, assign it to the array */ + rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx); - /* send AddBA request */ - ieee80211_send_addba_request(sdata, pubsta->addr, tid, - tid_tx->dialog_token, tid_tx->ssn, - 0x40, 5000); - return 0; + ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work); - err_free: - kfree(tid_tx); - err_wake_queue: - ieee80211_wake_queue_agg(local, tid); + /* this flow continues off the work */ err_unlock_sta: spin_unlock_bh(&sta->lock); return ret; @@ -534,8 +587,7 @@ int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, spin_lock_bh(&sta->lock); tid_tx = sta->ampdu_mlme.tid_tx[tid]; - /* check if the TID is in aggregation */ - if (!tid_tx || !test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) { + if (!tid_tx) { ret = -ENOENT; goto unlock; } @@ -552,6 +604,8 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) struct sta_info *sta = container_of(pubsta, struct sta_info, sta); struct ieee80211_sub_if_data *sdata = sta->sdata; struct ieee80211_local *local = sdata->local; + struct tid_ampdu_tx *tid_tx; + int ret = 0; trace_api_stop_tx_ba_session(pubsta, tid); @@ -561,7 +615,26 @@ int ieee80211_stop_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) if (tid >= STA_TID_NUM) return -EINVAL; - return __ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR); + spin_lock_bh(&sta->lock); + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + + if (!tid_tx) { + ret = -ENOENT; + goto unlock; + } + + if (test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) { + /* already in progress stopping it */ + ret = 0; + goto unlock; + } + + set_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state); + ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work); + + unlock: + spin_unlock_bh(&sta->lock); + return ret; } EXPORT_SYMBOL(ieee80211_stop_tx_ba_session); diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index 8a74ffb36ad..76839d4dfaa 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -121,7 +121,7 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, p += scnprintf(p, sizeof(buf) + buf - p, "next dialog_token: %#02x\n", sta->ampdu_mlme.dialog_token_allocator + 1); p += scnprintf(p, sizeof(buf) + buf - p, - "TID\t\tRX active\tDTKN\tSSN\t\tTX\tDTKN\tSSN\tpending\n"); + "TID\t\tRX active\tDTKN\tSSN\t\tTX\tDTKN\tpending\n"); for (i = 0; i < STA_TID_NUM; i++) { p += scnprintf(p, sizeof(buf) + buf - p, "%02d", i); p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x", @@ -138,9 +138,6 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x", sta->ampdu_mlme.tid_tx[i] ? sta->ampdu_mlme.tid_tx[i]->dialog_token : 0); - p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x", - sta->ampdu_mlme.tid_tx[i] ? - sta->ampdu_mlme.tid_tx[i]->ssn : 0); p += scnprintf(p, sizeof(buf) + buf - p, "\t%03d", sta->ampdu_mlme.tid_tx[i] ? skb_queue_len(&sta->ampdu_mlme.tid_tx[i]->pending) : 0); diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index 1af173ed2d5..4dfba7808a2 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -105,6 +105,8 @@ void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta) { int i; + cancel_work_sync(&sta->ampdu_mlme.work); + for (i = 0; i < STA_TID_NUM; i++) { __ieee80211_stop_tx_ba_session(sta, i, WLAN_BACK_INITIATOR); __ieee80211_stop_rx_ba_session(sta, i, WLAN_BACK_RECIPIENT, diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 8a91b5d8387..aec84c36c4c 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1115,6 +1115,7 @@ int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator); void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid); void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid); +void ieee80211_tx_ba_session_work(struct work_struct *work); /* Spectrum management */ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata, diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 06d8e00a253..8aa8558ba21 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -235,6 +235,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, spin_lock_init(&sta->lock); spin_lock_init(&sta->flaglock); INIT_WORK(&sta->drv_unblock_wk, sta_unblock); + INIT_WORK(&sta->ampdu_mlme.work, ieee80211_tx_ba_session_work); memcpy(sta->sta.addr, addr, ETH_ALEN); sta->local = local; diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 05278e6288c..040cbb0ac3a 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -67,6 +67,8 @@ enum ieee80211_sta_info_flags { #define HT_AGG_STATE_RESPONSE_RECEIVED 1 #define HT_AGG_STATE_OPERATIONAL 2 #define HT_AGG_STATE_STOPPING 3 +#define HT_AGG_STATE_WANT_START 4 +#define HT_AGG_STATE_WANT_STOP 5 /** * struct tid_ampdu_tx - TID aggregation information (Tx). @@ -74,7 +76,6 @@ enum ieee80211_sta_info_flags { * @rcu_head: rcu head for freeing structure * @addba_resp_timer: timer for peer's response to addba request * @pending: pending frames queue -- use sta's spinlock to protect - * @ssn: Starting Sequence Number expected to be aggregated. * @dialog_token: dialog token for aggregation session * @state: session state (see above) * @stop_initiator: initiator of a session stop @@ -92,7 +93,6 @@ struct tid_ampdu_tx { struct timer_list addba_resp_timer; struct sk_buff_head pending; unsigned long state; - u16 ssn; u8 dialog_token; u8 stop_initiator; }; @@ -139,11 +139,13 @@ struct tid_ampdu_rx { * @tid_tx: aggregation info for Tx per TID * @addba_req_num: number of times addBA request has been sent. * @dialog_token_allocator: dialog token enumerator for each new session; + * @work: work struct for starting/stopping aggregation */ struct sta_ampdu_mlme { /* rx */ struct tid_ampdu_rx *tid_rx[STA_TID_NUM]; /* tx */ + struct work_struct work; struct tid_ampdu_tx *tid_tx[STA_TID_NUM]; u8 addba_req_num[STA_TID_NUM]; u8 dialog_token_allocator; diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 7bf1f9c9ea3..698d4718b1a 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -1102,6 +1102,11 @@ static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx, if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) { info->flags |= IEEE80211_TX_CTL_AMPDU; + } else if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) { + /* + * nothing -- this aggregation session is being started + * but that might still fail with the driver + */ } else { spin_lock(&tx->sta->lock); /* -- cgit v1.2.3 From 67c282c00c9c06733aae229662d209957f6d23a7 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:43 +0200 Subject: mac80211: move BA session work Move the block-ack session works into common code, since it will be needed for RX agg too in the next patches. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 39 +++------------------------------------ net/mac80211/ht.c | 32 ++++++++++++++++++++++++++++++++ net/mac80211/ieee80211_i.h | 5 ++++- net/mac80211/sta_info.c | 2 +- 4 files changed, 40 insertions(+), 38 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index ee8bfe18b48..9a00a79a868 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -133,9 +133,8 @@ static void kfree_tid_tx(struct rcu_head *rcu_head) kfree(tid_tx); } -static int ___ieee80211_stop_tx_ba_session( - struct sta_info *sta, u16 tid, - enum ieee80211_back_parties initiator) +int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, + enum ieee80211_back_parties initiator) { struct ieee80211_local *local = sta->local; struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid]; @@ -262,7 +261,7 @@ ieee80211_wake_queue_agg(struct ieee80211_local *local, int tid) __release(agg_queue); } -static void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) +void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) { struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid]; struct ieee80211_local *local = sta->local; @@ -318,38 +317,6 @@ static void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) 0x40, 5000); } -void ieee80211_tx_ba_session_work(struct work_struct *work) -{ - struct sta_info *sta = - container_of(work, struct sta_info, ampdu_mlme.work); - struct tid_ampdu_tx *tid_tx; - int tid; - - /* - * When this flag is set, new sessions should be - * blocked, and existing sessions will be torn - * down by the code that set the flag, so this - * need not run. - */ - if (test_sta_flags(sta, WLAN_STA_BLOCK_BA)) - return; - - spin_lock_bh(&sta->lock); - for (tid = 0; tid < STA_TID_NUM; tid++) { - tid_tx = sta->ampdu_mlme.tid_tx[tid]; - if (!tid_tx) - continue; - - if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) - ieee80211_tx_ba_session_handle_start(sta, tid); - else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, - &tid_tx->state)) - ___ieee80211_stop_tx_ba_session(sta, tid, - WLAN_BACK_INITIATOR); - } - spin_unlock_bh(&sta->lock); -} - int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid) { struct sta_info *sta = container_of(pubsta, struct sta_info, sta); diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index 4dfba7808a2..531a19d358d 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -114,6 +114,38 @@ void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta) } } +void ieee80211_ba_session_work(struct work_struct *work) +{ + struct sta_info *sta = + container_of(work, struct sta_info, ampdu_mlme.work); + struct tid_ampdu_tx *tid_tx; + int tid; + + /* + * When this flag is set, new sessions should be + * blocked, and existing sessions will be torn + * down by the code that set the flag, so this + * need not run. + */ + if (test_sta_flags(sta, WLAN_STA_BLOCK_BA)) + return; + + spin_lock_bh(&sta->lock); + for (tid = 0; tid < STA_TID_NUM; tid++) { + tid_tx = sta->ampdu_mlme.tid_tx[tid]; + if (!tid_tx) + continue; + + if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) + ieee80211_tx_ba_session_handle_start(sta, tid); + else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, + &tid_tx->state)) + ___ieee80211_stop_tx_ba_session(sta, tid, + WLAN_BACK_INITIATOR); + } + spin_unlock_bh(&sta->lock); +} + void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata, const u8 *da, u16 tid, u16 initiator, u16 reason_code) diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index aec84c36c4c..628a53db9f7 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1113,9 +1113,12 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator); +int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, + enum ieee80211_back_parties initiator); void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid); void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid); -void ieee80211_tx_ba_session_work(struct work_struct *work); +void ieee80211_ba_session_work(struct work_struct *work); +void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid); /* Spectrum management */ void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata, diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index 8aa8558ba21..bd726c6204d 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -235,7 +235,7 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, spin_lock_init(&sta->lock); spin_lock_init(&sta->flaglock); INIT_WORK(&sta->drv_unblock_wk, sta_unblock); - INIT_WORK(&sta->ampdu_mlme.work, ieee80211_tx_ba_session_work); + INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work); memcpy(sta->sta.addr, addr, ETH_ALEN); sta->local = local; -- cgit v1.2.3 From 83a5cbf73a13d0c8be019b22afec4407e4285aed Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:47 +0200 Subject: mac80211: defer TX agg session teardown to work Since we want the code to be able to sleep in the future, it must not be called from the timer directly. To achieve that, simply call the function drivers would call, and also use RCU in the timer to get the struct so we don't need to rely on the spinlock in the future. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 9a00a79a868..0026604cfe3 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -200,11 +200,11 @@ static void sta_addba_resp_timer_expired(unsigned long data) struct tid_ampdu_tx *tid_tx; /* check if the TID waits for addBA response */ - spin_lock_bh(&sta->lock); - tid_tx = sta->ampdu_mlme.tid_tx[tid]; + rcu_read_lock(); + tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]); if (!tid_tx || test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) { - spin_unlock_bh(&sta->lock); + rcu_read_unlock(); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "timer expired on tid %d but we are not " "(or no longer) expecting addBA response there\n", @@ -217,8 +217,8 @@ static void sta_addba_resp_timer_expired(unsigned long data) printk(KERN_DEBUG "addBA response timer expired on tid %d\n", tid); #endif - ___ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR); - spin_unlock_bh(&sta->lock); + ieee80211_stop_tx_ba_session(&sta->sta, tid); + rcu_read_unlock(); } static inline int ieee80211_ac_from_tid(int tid) -- cgit v1.2.3 From cfcdbde35e2b621cf56bedc38a3a81e8c28addb9 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:48 +0200 Subject: mac80211: change TX aggregation locking To prepare for allowing drivers to sleep in ampdu_action, change the locking in the TX aggregation code to use the mutex the RX part already uses. The spinlock is still necessary around some code to avoid races with TX, but now we can also synchronize_net() to avoid getting an inconsistent sequence number. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 94 ++++++++++++++++++++++++++--------------------- net/mac80211/driver-ops.h | 3 ++ net/mac80211/ht.c | 8 +--- 3 files changed, 58 insertions(+), 47 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 0026604cfe3..5dff73eebef 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -6,7 +6,7 @@ * Copyright 2005-2006, Devicescape Software, Inc. * Copyright 2006-2007 Jiri Benc * Copyright 2007, Michael Wu - * Copyright 2007-2009, Intel Corporation + * Copyright 2007-2010, Intel Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -140,18 +140,23 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid]; int ret; - lockdep_assert_held(&sta->lock); + lockdep_assert_held(&sta->ampdu_mlme.mtx); - if (WARN_ON(!tid_tx)) + if (!tid_tx) return -ENOENT; + spin_lock_bh(&sta->lock); + if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) { /* not even started yet! */ rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); + spin_unlock_bh(&sta->lock); call_rcu(&tid_tx->rcu_head, kfree_tid_tx); return 0; } + spin_unlock_bh(&sta->lock); + #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n", sta->sta.addr, tid); @@ -269,6 +274,8 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) u16 start_seq_num; int ret; + lockdep_assert_held(&sta->ampdu_mlme.mtx); + /* * While we're asking the driver about the aggregation, * stop the AC queue so that we don't have to worry @@ -281,10 +288,11 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) clear_bit(HT_AGG_STATE_WANT_START, &tid_tx->state); /* - * This might be off by one due to a race that we can't - * really prevent here without synchronize_net() which - * can't be called now. + * make sure no packets are being processed to get + * valid starting sequence number */ + synchronize_net(); + start_seq_num = sta->tid_seq[tid] >> 4; ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START, @@ -294,7 +302,10 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) printk(KERN_DEBUG "BA request denied - HW unavailable for" " tid %d\n", tid); #endif + spin_lock_bh(&sta->lock); rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); + spin_unlock_bh(&sta->lock); + ieee80211_wake_queue_agg(local, tid); call_rcu(&tid_tx->rcu_head, kfree_tid_tx); return; @@ -309,7 +320,9 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid) printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid); #endif + spin_lock_bh(&sta->lock); sta->ampdu_mlme.addba_req_num[tid]++; + spin_unlock_bh(&sta->lock); /* send AddBA request */ ieee80211_send_addba_request(sdata, sta->sta.addr, tid, @@ -445,16 +458,25 @@ ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid) ieee80211_wake_queue_agg(local, tid); } -/* caller must hold sta->lock */ static void ieee80211_agg_tx_operational(struct ieee80211_local *local, struct sta_info *sta, u16 tid) { - lockdep_assert_held(&sta->lock); + lockdep_assert_held(&sta->ampdu_mlme.mtx); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid); #endif + drv_ampdu_action(local, sta->sdata, + IEEE80211_AMPDU_TX_OPERATIONAL, + &sta->sta, tid, NULL); + + /* + * synchronize with TX path, while splicing the TX path + * should block so it won't put more packets onto pending. + */ + spin_lock_bh(&sta->lock); + ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid); /* * Now mark as operational. This will be visible @@ -464,9 +486,7 @@ static void ieee80211_agg_tx_operational(struct ieee80211_local *local, set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state); ieee80211_agg_splice_finish(local, tid); - drv_ampdu_action(local, sta->sdata, - IEEE80211_AMPDU_TX_OPERATIONAL, - &sta->sta, tid, NULL); + spin_unlock_bh(&sta->lock); } void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid) @@ -486,37 +506,35 @@ void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid) return; } - rcu_read_lock(); + mutex_lock(&local->sta_mtx); sta = sta_info_get(sdata, ra); if (!sta) { - rcu_read_unlock(); + mutex_unlock(&local->sta_mtx); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Could not find station: %pM\n", ra); #endif return; } - spin_lock_bh(&sta->lock); + mutex_lock(&sta->ampdu_mlme.mtx); tid_tx = sta->ampdu_mlme.tid_tx[tid]; if (WARN_ON(!tid_tx)) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "addBA was not requested!\n"); #endif - spin_unlock_bh(&sta->lock); - rcu_read_unlock(); - return; + goto unlock; } if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state))) - goto out; + goto unlock; if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) ieee80211_agg_tx_operational(local, sta, tid); - out: - spin_unlock_bh(&sta->lock); - rcu_read_unlock(); + unlock: + mutex_unlock(&sta->ampdu_mlme.mtx); + mutex_unlock(&local->sta_mtx); } void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, @@ -548,21 +566,14 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_irqsafe); int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, enum ieee80211_back_parties initiator) { - struct tid_ampdu_tx *tid_tx; int ret; - spin_lock_bh(&sta->lock); - tid_tx = sta->ampdu_mlme.tid_tx[tid]; - - if (!tid_tx) { - ret = -ENOENT; - goto unlock; - } + mutex_lock(&sta->ampdu_mlme.mtx); ret = ___ieee80211_stop_tx_ba_session(sta, tid, initiator); - unlock: - spin_unlock_bh(&sta->lock); + mutex_unlock(&sta->ampdu_mlme.mtx); + return ret; } @@ -627,16 +638,17 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) ra, tid); #endif /* CONFIG_MAC80211_HT_DEBUG */ - rcu_read_lock(); + mutex_lock(&local->sta_mtx); + sta = sta_info_get(sdata, ra); if (!sta) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "Could not find station: %pM\n", ra); #endif - rcu_read_unlock(); - return; + goto unlock; } + mutex_lock(&sta->ampdu_mlme.mtx); spin_lock_bh(&sta->lock); tid_tx = sta->ampdu_mlme.tid_tx[tid]; @@ -644,9 +656,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n"); #endif - spin_unlock_bh(&sta->lock); - rcu_read_unlock(); - return; + goto unlock_sta; } if (tid_tx->stop_initiator == WLAN_BACK_INITIATOR) @@ -672,8 +682,11 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid) call_rcu(&tid_tx->rcu_head, kfree_tid_tx); + unlock_sta: spin_unlock_bh(&sta->lock); - rcu_read_unlock(); + mutex_unlock(&sta->ampdu_mlme.mtx); + unlock: + mutex_unlock(&local->sta_mtx); } void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, @@ -714,10 +727,9 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab); tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2; - spin_lock_bh(&sta->lock); + mutex_lock(&sta->ampdu_mlme.mtx); tid_tx = sta->ampdu_mlme.tid_tx[tid]; - if (!tid_tx) goto out; @@ -751,5 +763,5 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, } out: - spin_unlock_bh(&sta->lock); + mutex_unlock(&sta->ampdu_mlme.mtx); } diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h index e5e7ef175ca..7e86c6f89be 100644 --- a/net/mac80211/driver-ops.h +++ b/net/mac80211/driver-ops.h @@ -349,6 +349,9 @@ static inline int drv_ampdu_action(struct ieee80211_local *local, u16 *ssn) { int ret = -EOPNOTSUPP; + + might_sleep(); + local_bh_disable(); if (local->ops->ampdu_action) ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action, diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index e29be64083c..be928ef7ef5 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -6,7 +6,7 @@ * Copyright 2005-2006, Devicescape Software, Inc. * Copyright 2006-2007 Jiri Benc * Copyright 2007, Michael Wu - * Copyright 2007-2008, Intel Corporation + * Copyright 2007-2010, Intel Corporation * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -136,11 +136,7 @@ void ieee80211_ba_session_work(struct work_struct *work) ___ieee80211_stop_rx_ba_session( sta, tid, WLAN_BACK_RECIPIENT, WLAN_REASON_QSTA_TIMEOUT); - } - mutex_unlock(&sta->ampdu_mlme.mtx); - spin_lock_bh(&sta->lock); - for (tid = 0; tid < STA_TID_NUM; tid++) { tid_tx = sta->ampdu_mlme.tid_tx[tid]; if (!tid_tx) continue; @@ -152,7 +148,7 @@ void ieee80211_ba_session_work(struct work_struct *work) ___ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR); } - spin_unlock_bh(&sta->lock); + mutex_unlock(&sta->ampdu_mlme.mtx); } void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata, -- cgit v1.2.3 From 73a72a81d27b71f9ace31668d2dd7f3ac1c8228e Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 10 Jun 2010 10:21:50 +0200 Subject: mac80211: update aggregation documentation Even before the recent changes, the documentation for TX aggregation was somewhat out of date. Update it and also add documentation for the RX side. Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-rx.c | 23 +++++++++++++++++++++++ net/mac80211/agg-tx.c | 43 +++++++++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 16 deletions(-) (limited to 'net/mac80211/agg-tx.c') diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c index a843df26f38..965b272499f 100644 --- a/net/mac80211/agg-rx.c +++ b/net/mac80211/agg-rx.c @@ -13,6 +13,29 @@ * published by the Free Software Foundation. */ +/** + * DOC: RX A-MPDU aggregation + * + * Aggregation on the RX side requires only implementing the + * @ampdu_action callback that is invoked to start/stop any + * block-ack sessions for RX aggregation. + * + * When RX aggregation is started by the peer, the driver is + * notified via @ampdu_action function, with the + * %IEEE80211_AMPDU_RX_START action, and may reject the request + * in which case a negative response is sent to the peer, if it + * accepts it a positive response is sent. + * + * While the session is active, the device/driver are required + * to de-aggregate frames and pass them up one by one to mac80211, + * which will handle the reorder buffer. + * + * When the aggregation session is stopped again by the peer or + * ourselves, the driver's @ampdu_action function will be called + * with the action %IEEE80211_AMPDU_RX_STOP. In this case, the + * call must not fail. + */ + #include #include #include diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 5dff73eebef..c893f236ace 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -21,28 +21,39 @@ #include "wme.h" /** - * DOC: TX aggregation + * DOC: TX A-MPDU aggregation * * Aggregation on the TX side requires setting the hardware flag - * %IEEE80211_HW_AMPDU_AGGREGATION as well as, if present, the @ampdu_queues - * hardware parameter to the number of hardware AMPDU queues. If there are no - * hardware queues then the driver will (currently) have to do all frame - * buffering. + * %IEEE80211_HW_AMPDU_AGGREGATION. The driver will then be handed + * packets with a flag indicating A-MPDU aggregation. The driver + * or device is responsible for actually aggregating the frames, + * as well as deciding how many and which to aggregate. * - * When TX aggregation is started by some subsystem (usually the rate control - * algorithm would be appropriate) by calling the - * ieee80211_start_tx_ba_session() function, the driver will be notified via - * its @ampdu_action function, with the %IEEE80211_AMPDU_TX_START action. + * When TX aggregation is started by some subsystem (usually the rate + * control algorithm would be appropriate) by calling the + * ieee80211_start_tx_ba_session() function, the driver will be + * notified via its @ampdu_action function, with the + * %IEEE80211_AMPDU_TX_START action. * * In response to that, the driver is later required to call the - * ieee80211_start_tx_ba_cb() (or ieee80211_start_tx_ba_cb_irqsafe()) - * function, which will start the aggregation session. + * ieee80211_start_tx_ba_cb_irqsafe() function, which will really + * start the aggregation session after the peer has also responded. + * If the peer responds negatively, the session will be stopped + * again right away. Note that it is possible for the aggregation + * session to be stopped before the driver has indicated that it + * is done setting it up, in which case it must not indicate the + * setup completion. * - * Similarly, when the aggregation session is stopped by - * ieee80211_stop_tx_ba_session(), the driver's @ampdu_action function will - * be called with the action %IEEE80211_AMPDU_TX_STOP. In this case, the - * call must not fail, and the driver must later call ieee80211_stop_tx_ba_cb() - * (or ieee80211_stop_tx_ba_cb_irqsafe()). + * Also note that, since we also need to wait for a response from + * the peer, the driver is notified of the completion of the + * handshake by the %IEEE80211_AMPDU_TX_OPERATIONAL action to the + * @ampdu_action callback. + * + * Similarly, when the aggregation session is stopped by the peer + * or something calling ieee80211_stop_tx_ba_session(), the driver's + * @ampdu_action function will be called with the action + * %IEEE80211_AMPDU_TX_STOP. In this case, the call must not fail, + * and the driver must later call ieee80211_stop_tx_ba_cb_irqsafe(). */ static void ieee80211_send_addba_request(struct ieee80211_sub_if_data *sdata, -- cgit v1.2.3