Commit 83eb935e authored by Michal Kazior's avatar Michal Kazior Committed by Johannes Berg
Browse files

mac80211: fix Rx reordering with RX_FLAG_AMSDU_MORE 



Some drivers (e.g. ath10k) report A-MSDU subframes
individually with identical seqno. The A-MPDU Rx
reorder code did not account for that which made
it practically unusable with drivers using
RX_FLAG_AMSDU_MORE because it would end up
dropping a lot of frames resulting in confusion in
upper network transport layers.

Signed-off-by: default avatarMichal Kazior <michal.kazior@tieto.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent 60e83deb

net/mac80211/agg-rx.c

+6 −3
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ static void ieee80211_free_tid_rx(struct rcu_head *h) 
	del_timer_sync(&tid_rx->reorder_timer);



	for (i = 0; i < tid_rx->buf_size; i++)

		dev_kfree_skb(tid_rx->reorder_buf[i]);

		__skb_queue_purge(&tid_rx->reorder_buf[i]);

	kfree(tid_rx->reorder_buf);

	kfree(tid_rx->reorder_time);

	kfree(tid_rx);
@@ -232,7 +232,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, 
	struct tid_ampdu_rx *tid_agg_rx;

	u16 capab, tid, timeout, ba_policy, buf_size, start_seq_num, status;

	u8 dialog_token;

	int ret = -EOPNOTSUPP;

	int i, ret = -EOPNOTSUPP;



	/* extract session parameters from addba request frame */

	dialog_token = mgmt->u.action.u.addba_req.dialog_token;
@@ -308,7 +308,7 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, 


	/* prepare reordering buffer */

	tid_agg_rx->reorder_buf =

		kcalloc(buf_size, sizeof(struct sk_buff *), GFP_KERNEL);

		kcalloc(buf_size, sizeof(struct sk_buff_head), GFP_KERNEL);

	tid_agg_rx->reorder_time =

		kcalloc(buf_size, sizeof(unsigned long), GFP_KERNEL);

	if (!tid_agg_rx->reorder_buf || !tid_agg_rx->reorder_time) {
@@ -318,6 +318,9 @@ void ieee80211_process_addba_request(struct ieee80211_local *local, 
		goto end;

	}



	for (i = 0; i < buf_size; i++)

		__skb_queue_head_init(&tid_agg_rx->reorder_buf[i]);



	ret = drv_ampdu_action(local, sta->sdata, IEEE80211_AMPDU_RX_START,

			       &sta->sta, tid, &start_seq_num, 0);

	ht_dbg(sta->sdata, "Rx A-MPDU request on %pM tid %d result %d\n",

net/mac80211/ieee80211_i.h

+15 −0
Original line number Diff line number Diff line
@@ -1729,6 +1729,21 @@ static inline void ieee802_11_parse_elems(const u8 *start, size_t len, 
	ieee802_11_parse_elems_crc(start, len, action, elems, 0, 0);

}



static inline bool ieee80211_rx_reorder_ready(struct sk_buff_head *frames)

{

	struct sk_buff *tail = skb_peek_tail(frames);

	struct ieee80211_rx_status *status;



	if (!tail)

		return false;



	status = IEEE80211_SKB_RXCB(tail);

	if (status->flag & RX_FLAG_AMSDU_MORE)

		return false;



	return true;

}



void ieee80211_dynamic_ps_enable_work(struct work_struct *work);

void ieee80211_dynamic_ps_disable_work(struct work_struct *work);

void ieee80211_dynamic_ps_timer(unsigned long data);

net/mac80211/rx.c

+38 −19
Original line number Diff line number Diff line
@@ -688,20 +688,27 @@ static void ieee80211_release_reorder_frame(struct ieee80211_sub_if_data *sdata, 
					    int index,

					    struct sk_buff_head *frames)

{

	struct sk_buff *skb = tid_agg_rx->reorder_buf[index];

	struct sk_buff_head *skb_list = &tid_agg_rx->reorder_buf[index];

	struct sk_buff *skb;

	struct ieee80211_rx_status *status;



	lockdep_assert_held(&tid_agg_rx->reorder_lock);



	if (!skb)

	if (skb_queue_empty(skb_list))

		goto no_frame;



	/* release the frame from the reorder ring buffer */

	if (!ieee80211_rx_reorder_ready(skb_list)) {

		__skb_queue_purge(skb_list);

		goto no_frame;

	}



	/* release frames from the reorder ring buffer */

	tid_agg_rx->stored_mpdu_num--;

	tid_agg_rx->reorder_buf[index] = NULL;

	while ((skb = __skb_dequeue(skb_list))) {

		status = IEEE80211_SKB_RXCB(skb);

		status->rx_flags |= IEEE80211_RX_DEFERRED_RELEASE;

		__skb_queue_tail(frames, skb);

	}



no_frame:

	tid_agg_rx->head_seq_num = ieee80211_sn_inc(tid_agg_rx->head_seq_num);
@@ -738,13 +745,13 @@ static void ieee80211_sta_reorder_release(struct ieee80211_sub_if_data *sdata, 
					  struct tid_ampdu_rx *tid_agg_rx,

					  struct sk_buff_head *frames)

{

	int index, j;

	int index, i, j;



	lockdep_assert_held(&tid_agg_rx->reorder_lock);



	/* release the buffer until next missing frame */

	index = tid_agg_rx->head_seq_num % tid_agg_rx->buf_size;

	if (!tid_agg_rx->reorder_buf[index] &&

	if (!ieee80211_rx_reorder_ready(&tid_agg_rx->reorder_buf[index]) &&

	    tid_agg_rx->stored_mpdu_num) {

		/*

		 * No buffers ready to be released, but check whether any
@@ -753,7 +760,8 @@ static void ieee80211_sta_reorder_release(struct ieee80211_sub_if_data *sdata, 
		int skipped = 1;

		for (j = (index + 1) % tid_agg_rx->buf_size; j != index;

		     j = (j + 1) % tid_agg_rx->buf_size) {

			if (!tid_agg_rx->reorder_buf[j]) {

			if (!ieee80211_rx_reorder_ready(

					&tid_agg_rx->reorder_buf[j])) {

				skipped++;

				continue;

			}
@@ -762,6 +770,11 @@ static void ieee80211_sta_reorder_release(struct ieee80211_sub_if_data *sdata, 
					HT_RX_REORDER_BUF_TIMEOUT))

				goto set_release_timer;



			/* don't leave incomplete A-MSDUs around */

			for (i = (index + 1) % tid_agg_rx->buf_size; i != j;

			     i = (i + 1) % tid_agg_rx->buf_size)

				__skb_queue_purge(&tid_agg_rx->reorder_buf[i]);



			ht_dbg_ratelimited(sdata,

					   "release an RX reorder frame due to timeout on earlier frames\n");

			ieee80211_release_reorder_frame(sdata, tid_agg_rx, j,
@@ -775,7 +788,8 @@ static void ieee80211_sta_reorder_release(struct ieee80211_sub_if_data *sdata, 
				 skipped) & IEEE80211_SN_MASK;

			skipped = 0;

		}

	} else while (tid_agg_rx->reorder_buf[index]) {

	} else while (ieee80211_rx_reorder_ready(

				&tid_agg_rx->reorder_buf[index])) {

		ieee80211_release_reorder_frame(sdata, tid_agg_rx, index,

						frames);

		index =	tid_agg_rx->head_seq_num % tid_agg_rx->buf_size;
@@ -786,7 +800,8 @@ static void ieee80211_sta_reorder_release(struct ieee80211_sub_if_data *sdata, 


		for (; j != (index - 1) % tid_agg_rx->buf_size;

		     j = (j + 1) % tid_agg_rx->buf_size) {

			if (tid_agg_rx->reorder_buf[j])

			if (ieee80211_rx_reorder_ready(

					&tid_agg_rx->reorder_buf[j]))

				break;

		}
@@ -811,6 +826,7 @@ static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_sub_if_data *sdata 
					     struct sk_buff_head *frames)

{

	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;

	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);

	u16 sc = le16_to_cpu(hdr->seq_ctrl);

	u16 mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;

	u16 head_seq_num, buf_size;
@@ -845,7 +861,7 @@ static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_sub_if_data *sdata 
	index = mpdu_seq_num % tid_agg_rx->buf_size;



	/* check if we already stored this frame */

	if (tid_agg_rx->reorder_buf[index]) {

	if (ieee80211_rx_reorder_ready(&tid_agg_rx->reorder_buf[index])) {

		dev_kfree_skb(skb);

		goto out;

	}
@@ -858,6 +874,7 @@ static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_sub_if_data *sdata 
	 */

	if (mpdu_seq_num == tid_agg_rx->head_seq_num &&

	    tid_agg_rx->stored_mpdu_num == 0) {

		if (!(status->flag & RX_FLAG_AMSDU_MORE))

			tid_agg_rx->head_seq_num =

				ieee80211_sn_inc(tid_agg_rx->head_seq_num);

		ret = false;
@@ -865,10 +882,12 @@ static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_sub_if_data *sdata 
	}



	/* put the frame in the reordering buffer */

	tid_agg_rx->reorder_buf[index] = skb;

	__skb_queue_tail(&tid_agg_rx->reorder_buf[index], skb);

	if (!(status->flag & RX_FLAG_AMSDU_MORE)) {

		tid_agg_rx->reorder_time[index] = jiffies;

		tid_agg_rx->stored_mpdu_num++;

		ieee80211_sta_reorder_release(sdata, tid_agg_rx, frames);

	}



 out:

	spin_unlock(&tid_agg_rx->reorder_lock);

net/mac80211/sta_info.h

+3 −2
Original line number Diff line number Diff line
@@ -155,7 +155,8 @@ struct tid_ampdu_tx { 
/**

 * struct tid_ampdu_rx - TID aggregation information (Rx).

 *

 * @reorder_buf: buffer to reorder incoming aggregated MPDUs

 * @reorder_buf: buffer to reorder incoming aggregated MPDUs. An MPDU may be an

 *	A-MSDU with individually reported subframes.

 * @reorder_time: jiffies when skb was added

 * @session_timer: check if peer keeps Tx-ing on the TID (by timeout value)

 * @reorder_timer: releases expired frames from the reorder buffer.
@@ -180,7 +181,7 @@ struct tid_ampdu_tx { 
struct tid_ampdu_rx {

	struct rcu_head rcu_head;

	spinlock_t reorder_lock;

	struct sk_buff **reorder_buf;

	struct sk_buff_head *reorder_buf;

	unsigned long *reorder_time;

	struct timer_list session_timer;

	struct timer_list reorder_timer;

CRA Git | Maintained and supported by SUSTech CRA and CCSE