Commit a87f736d authored by Johannes Berg's avatar Johannes Berg Committed by John W. Linville

mac80211: use RCU for RX aggregation

Currently we allocate some memory for each RX
aggregation session and additionally keep a
flag indicating whether or not it is valid.
By using RCU to protect the pointer and making
sure that the memory is fully set up before it
becomes visible to the RX path, we can remove
the need for the bool that indicates validity,
as well as for locking on the RX path since it
is always synchronised against itself, and we
can guarantee that all other modifications are
done when the structure is not visible to the
RX path.

The net result is that since we remove locking
requirements from the RX path, we can in the
future use any kind of lock for the setup and
teardown code paths.
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent c1475ca9
...@@ -19,25 +19,36 @@ ...@@ -19,25 +19,36 @@
#include "ieee80211_i.h" #include "ieee80211_i.h"
#include "driver-ops.h" #include "driver-ops.h"
static void ieee80211_free_tid_rx(struct rcu_head *h)
{
struct tid_ampdu_rx *tid_rx =
container_of(h, struct tid_ampdu_rx, rcu_head);
int i;
for (i = 0; i < tid_rx->buf_size; i++)
dev_kfree_skb(tid_rx->reorder_buf[i]);
kfree(tid_rx->reorder_buf);
kfree(tid_rx->reorder_time);
kfree(tid_rx);
}
static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid, static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
u16 initiator, u16 reason, u16 initiator, u16 reason,
bool from_timer) bool from_timer)
{ {
struct ieee80211_local *local = sta->local; struct ieee80211_local *local = sta->local;
struct tid_ampdu_rx *tid_rx; struct tid_ampdu_rx *tid_rx;
int i;
spin_lock_bh(&sta->lock); spin_lock_bh(&sta->lock);
/* check if TID is in operational state */ tid_rx = sta->ampdu_mlme.tid_rx[tid];
if (!sta->ampdu_mlme.tid_active_rx[tid]) {
if (!tid_rx) {
spin_unlock_bh(&sta->lock); spin_unlock_bh(&sta->lock);
return; return;
} }
sta->ampdu_mlme.tid_active_rx[tid] = false; rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], NULL);
tid_rx = sta->ampdu_mlme.tid_rx[tid];
#ifdef CONFIG_MAC80211_HT_DEBUG #ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "Rx BA session stop requested for %pM tid %u\n", printk(KERN_DEBUG "Rx BA session stop requested for %pM tid %u\n",
...@@ -54,26 +65,12 @@ static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid, ...@@ -54,26 +65,12 @@ static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
ieee80211_send_delba(sta->sdata, sta->sta.addr, ieee80211_send_delba(sta->sdata, sta->sta.addr,
tid, 0, reason); tid, 0, reason);
/* free the reordering buffer */
for (i = 0; i < tid_rx->buf_size; i++) {
if (tid_rx->reorder_buf[i]) {
/* release the reordered frames */
dev_kfree_skb(tid_rx->reorder_buf[i]);
tid_rx->stored_mpdu_num--;
tid_rx->reorder_buf[i] = NULL;
}
}
/* free resources */
kfree(tid_rx->reorder_buf);
kfree(tid_rx->reorder_time);
sta->ampdu_mlme.tid_rx[tid] = NULL;
spin_unlock_bh(&sta->lock); spin_unlock_bh(&sta->lock);
if (!from_timer) if (!from_timer)
del_timer_sync(&tid_rx->session_timer); del_timer_sync(&tid_rx->session_timer);
kfree(tid_rx);
call_rcu(&tid_rx->rcu_head, ieee80211_free_tid_rx);
} }
void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid, void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
...@@ -214,7 +211,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -214,7 +211,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
/* examine state machine */ /* examine state machine */
spin_lock_bh(&sta->lock); spin_lock_bh(&sta->lock);
if (sta->ampdu_mlme.tid_active_rx[tid]) { if (sta->ampdu_mlme.tid_rx[tid]) {
#ifdef CONFIG_MAC80211_HT_DEBUG #ifdef CONFIG_MAC80211_HT_DEBUG
if (net_ratelimit()) if (net_ratelimit())
printk(KERN_DEBUG "unexpected AddBA Req from " printk(KERN_DEBUG "unexpected AddBA Req from "
...@@ -225,9 +222,8 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -225,9 +222,8 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
} }
/* prepare A-MPDU MLME for Rx aggregation */ /* prepare A-MPDU MLME for Rx aggregation */
sta->ampdu_mlme.tid_rx[tid] = tid_agg_rx = kmalloc(sizeof(struct tid_ampdu_rx), GFP_ATOMIC);
kmalloc(sizeof(struct tid_ampdu_rx), GFP_ATOMIC); if (!tid_agg_rx) {
if (!sta->ampdu_mlme.tid_rx[tid]) {
#ifdef CONFIG_MAC80211_HT_DEBUG #ifdef CONFIG_MAC80211_HT_DEBUG
if (net_ratelimit()) if (net_ratelimit())
printk(KERN_ERR "allocate rx mlme to tid %d failed\n", printk(KERN_ERR "allocate rx mlme to tid %d failed\n",
...@@ -235,14 +231,11 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -235,14 +231,11 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
#endif #endif
goto end; goto end;
} }
/* rx timer */
sta->ampdu_mlme.tid_rx[tid]->session_timer.function =
sta_rx_agg_session_timer_expired;
sta->ampdu_mlme.tid_rx[tid]->session_timer.data =
(unsigned long)&sta->timer_to_tid[tid];
init_timer(&sta->ampdu_mlme.tid_rx[tid]->session_timer);
tid_agg_rx = sta->ampdu_mlme.tid_rx[tid]; /* rx timer */
tid_agg_rx->session_timer.function = sta_rx_agg_session_timer_expired;
tid_agg_rx->session_timer.data = (unsigned long)&sta->timer_to_tid[tid];
init_timer(&tid_agg_rx->session_timer);
/* prepare reordering buffer */ /* prepare reordering buffer */
tid_agg_rx->reorder_buf = tid_agg_rx->reorder_buf =
...@@ -257,8 +250,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -257,8 +250,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
#endif #endif
kfree(tid_agg_rx->reorder_buf); kfree(tid_agg_rx->reorder_buf);
kfree(tid_agg_rx->reorder_time); kfree(tid_agg_rx->reorder_time);
kfree(sta->ampdu_mlme.tid_rx[tid]); kfree(tid_agg_rx);
sta->ampdu_mlme.tid_rx[tid] = NULL;
goto end; goto end;
} }
...@@ -270,13 +262,12 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -270,13 +262,12 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
if (ret) { if (ret) {
kfree(tid_agg_rx->reorder_buf); kfree(tid_agg_rx->reorder_buf);
kfree(tid_agg_rx->reorder_time);
kfree(tid_agg_rx); kfree(tid_agg_rx);
sta->ampdu_mlme.tid_rx[tid] = NULL;
goto end; goto end;
} }
/* change state and send addba resp */ /* update data */
sta->ampdu_mlme.tid_active_rx[tid] = true;
tid_agg_rx->dialog_token = dialog_token; tid_agg_rx->dialog_token = dialog_token;
tid_agg_rx->ssn = start_seq_num; tid_agg_rx->ssn = start_seq_num;
tid_agg_rx->head_seq_num = start_seq_num; tid_agg_rx->head_seq_num = start_seq_num;
...@@ -284,6 +275,9 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, ...@@ -284,6 +275,9 @@ void ieee80211_process_addba_request(struct ieee80211_local *local,
tid_agg_rx->timeout = timeout; tid_agg_rx->timeout = timeout;
tid_agg_rx->stored_mpdu_num = 0; tid_agg_rx->stored_mpdu_num = 0;
status = WLAN_STATUS_SUCCESS; status = WLAN_STATUS_SUCCESS;
/* activate it for RX */
rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], tid_agg_rx);
end: end:
spin_unlock_bh(&sta->lock); spin_unlock_bh(&sta->lock);
......
...@@ -125,12 +125,12 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf, ...@@ -125,12 +125,12 @@ static ssize_t sta_agg_status_read(struct file *file, char __user *userbuf,
for (i = 0; i < STA_TID_NUM; i++) { for (i = 0; i < STA_TID_NUM; i++) {
p += scnprintf(p, sizeof(buf) + buf - p, "%02d", i); p += scnprintf(p, sizeof(buf) + buf - p, "%02d", i);
p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x", p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",
sta->ampdu_mlme.tid_active_rx[i]); !!sta->ampdu_mlme.tid_rx[i]);
p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x", p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x",
sta->ampdu_mlme.tid_active_rx[i] ? sta->ampdu_mlme.tid_rx[i] ?
sta->ampdu_mlme.tid_rx[i]->dialog_token : 0); sta->ampdu_mlme.tid_rx[i]->dialog_token : 0);
p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x", p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x",
sta->ampdu_mlme.tid_active_rx[i] ? sta->ampdu_mlme.tid_rx[i] ?
sta->ampdu_mlme.tid_rx[i]->ssn : 0); sta->ampdu_mlme.tid_rx[i]->ssn : 0);
p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x", p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",
......
...@@ -719,16 +719,13 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx, ...@@ -719,16 +719,13 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx,
tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK; tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
spin_lock(&sta->lock); tid_agg_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[tid]);
if (!tid_agg_rx)
if (!sta->ampdu_mlme.tid_active_rx[tid]) goto dont_reorder;
goto dont_reorder_unlock;
tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
/* qos null data frames are excluded */ /* qos null data frames are excluded */
if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC))) if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
goto dont_reorder_unlock; goto dont_reorder;
/* new, potentially un-ordered, ampdu frame - process it */ /* new, potentially un-ordered, ampdu frame - process it */
...@@ -740,20 +737,22 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx, ...@@ -740,20 +737,22 @@ static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx,
/* if this mpdu is fragmented - terminate rx aggregation session */ /* if this mpdu is fragmented - terminate rx aggregation session */
sc = le16_to_cpu(hdr->seq_ctrl); sc = le16_to_cpu(hdr->seq_ctrl);
if (sc & IEEE80211_SCTL_FRAG) { if (sc & IEEE80211_SCTL_FRAG) {
spin_unlock(&sta->lock);
skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME; skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
skb_queue_tail(&rx->sdata->skb_queue, skb); skb_queue_tail(&rx->sdata->skb_queue, skb);
ieee80211_queue_work(&local->hw, &rx->sdata->work); ieee80211_queue_work(&local->hw, &rx->sdata->work);
return; return;
} }
if (ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, frames)) { /*
spin_unlock(&sta->lock); * No locking needed -- we will only ever process one
* RX packet at a time, and thus own tid_agg_rx. All
* other code manipulating it needs to (and does) make
* sure that we cannot get to it any more before doing
* anything with it.
*/
if (ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, frames))
return; return;
}
dont_reorder_unlock:
spin_unlock(&sta->lock);
dont_reorder: dont_reorder:
__skb_queue_tail(frames, skb); __skb_queue_tail(frames, skb);
} }
...@@ -1830,13 +1829,11 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx, struct sk_buff_head *frames) ...@@ -1830,13 +1829,11 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx, struct sk_buff_head *frames)
&bar_data, sizeof(bar_data))) &bar_data, sizeof(bar_data)))
return RX_DROP_MONITOR; return RX_DROP_MONITOR;
spin_lock(&rx->sta->lock);
tid = le16_to_cpu(bar_data.control) >> 12; tid = le16_to_cpu(bar_data.control) >> 12;
if (!rx->sta->ampdu_mlme.tid_active_rx[tid]) {
spin_unlock(&rx->sta->lock); tid_agg_rx = rcu_dereference(rx->sta->ampdu_mlme.tid_rx[tid]);
if (!tid_agg_rx)
return RX_DROP_MONITOR; return RX_DROP_MONITOR;
}
tid_agg_rx = rx->sta->ampdu_mlme.tid_rx[tid];
start_seq_num = le16_to_cpu(bar_data.start_seq_num) >> 4; start_seq_num = le16_to_cpu(bar_data.start_seq_num) >> 4;
...@@ -1849,7 +1846,6 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx, struct sk_buff_head *frames) ...@@ -1849,7 +1846,6 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx, struct sk_buff_head *frames)
ieee80211_release_reorder_frames(hw, tid_agg_rx, start_seq_num, ieee80211_release_reorder_frames(hw, tid_agg_rx, start_seq_num,
frames); frames);
kfree_skb(skb); kfree_skb(skb);
spin_unlock(&rx->sta->lock);
return RX_QUEUED; return RX_QUEUED;
} }
......
...@@ -102,8 +102,18 @@ struct tid_ampdu_tx { ...@@ -102,8 +102,18 @@ struct tid_ampdu_tx {
* @buf_size: buffer size for incoming A-MPDUs * @buf_size: buffer size for incoming A-MPDUs
* @timeout: reset timer value (in TUs). * @timeout: reset timer value (in TUs).
* @dialog_token: dialog token for aggregation session * @dialog_token: dialog token for aggregation session
* @rcu_head: RCU head used for freeing this struct
*
* This structure is protected by RCU and the per-station
* spinlock. Assignments to the array holding it must hold
* the spinlock, only the RX path can access it under RCU
* lock-free. The RX path, since it is single-threaded,
* can even modify the structure without locking since the
* only other modifications to it are done when the struct
* can not yet or no longer be found by the RX path.
*/ */
struct tid_ampdu_rx { struct tid_ampdu_rx {
struct rcu_head rcu_head;
struct sk_buff **reorder_buf; struct sk_buff **reorder_buf;
unsigned long *reorder_time; unsigned long *reorder_time;
struct timer_list session_timer; struct timer_list session_timer;
...@@ -118,8 +128,7 @@ struct tid_ampdu_rx { ...@@ -118,8 +128,7 @@ struct tid_ampdu_rx {
/** /**
* struct sta_ampdu_mlme - STA aggregation information. * struct sta_ampdu_mlme - STA aggregation information.
* *
* @tid_active_rx: TID's state in Rx session state machine. * @tid_rx: aggregation info for Rx per TID -- RCU protected
* @tid_rx: aggregation info for Rx per TID
* @tid_state_tx: TID's state in Tx session state machine. * @tid_state_tx: TID's state in Tx session state machine.
* @tid_tx: aggregation info for Tx per TID * @tid_tx: aggregation info for Tx per TID
* @addba_req_num: number of times addBA request has been sent. * @addba_req_num: number of times addBA request has been sent.
...@@ -127,7 +136,6 @@ struct tid_ampdu_rx { ...@@ -127,7 +136,6 @@ struct tid_ampdu_rx {
*/ */
struct sta_ampdu_mlme { struct sta_ampdu_mlme {
/* rx */ /* rx */
bool tid_active_rx[STA_TID_NUM];
struct tid_ampdu_rx *tid_rx[STA_TID_NUM]; struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
/* tx */ /* tx */
u8 tid_state_tx[STA_TID_NUM]; u8 tid_state_tx[STA_TID_NUM];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment