* [RFC 0/3] fix the aMPDU reorder placement
@ 2009-11-16 14:21 Johannes Berg
2009-11-16 14:21 ` [RFC 1/3] mac80211: move cmntr flag out of rx flags Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Johannes Berg @ 2009-11-16 14:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Jouni Malinen
Our code has a comment about where the reordering
should happen, and why it doesn't hurt being where
it currently is.
However, this code needs to use sta_info_get()
without knowing which virtual interface is the one
we're processing the frames for, which is getting
in my way.
Rather than trying to find the virtual interface
by the DA, I decided to put the reorder code at the
right place instead because then the station pointer
is already known (so we even save a lookup).
Please test! Especially against APs that are prone
to hitting the timeout.
johannes
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC 1/3] mac80211: move cmntr flag out of rx flags
2009-11-16 14:21 [RFC 0/3] fix the aMPDU reorder placement Johannes Berg
@ 2009-11-16 14:21 ` Johannes Berg
2009-11-16 14:21 ` [RFC 2/3] mac80211: move aMPDU RX reorder code Johannes Berg
2009-11-16 14:21 ` [RFC 3/3] mac80211: correctly place " Johannes Berg
2 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2009-11-16 14:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Jouni Malinen
The RX flags should soon be used only for flags
that cannot change within an a-MPDU, so move the
cooked monitor flag into the RX status flags.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
include/net/mac80211.h | 4 ++++
net/mac80211/ieee80211_i.h | 3 +--
net/mac80211/rx.c | 4 ++--
3 files changed, 7 insertions(+), 4 deletions(-)
--- wireless-testing.orig/include/net/mac80211.h 2009-11-16 15:11:58.000000000 +0100
+++ wireless-testing/include/net/mac80211.h 2009-11-16 15:12:28.000000000 +0100
@@ -510,6 +510,9 @@ ieee80211_tx_info_clear_status(struct ie
* @RX_FLAG_HT: HT MCS was used and rate_idx is MCS index
* @RX_FLAG_40MHZ: HT40 (40 MHz) was used
* @RX_FLAG_SHORT_GI: Short guard interval was used
+ * @RX_FLAG_INTERNAL_CMTR: set internally after frame was reported
+ * on cooked monitor to avoid double-reporting it for multiple
+ * virtual interfaces
*/
enum mac80211_rx_flags {
RX_FLAG_MMIC_ERROR = 1<<0,
@@ -523,6 +526,7 @@ enum mac80211_rx_flags {
RX_FLAG_HT = 1<<9,
RX_FLAG_40MHZ = 1<<10,
RX_FLAG_SHORT_GI = 1<<11,
+ RX_FLAG_INTERNAL_CMTR = 1<<12,
};
/**
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-11-16 15:11:46.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2009-11-16 15:11:50.000000000 +0100
@@ -162,8 +162,7 @@ typedef unsigned __bitwise__ ieee80211_r
/* frame is destined to interface currently processed (incl. multicast frames) */
#define IEEE80211_RX_RA_MATCH BIT(1)
#define IEEE80211_RX_AMSDU BIT(2)
-#define IEEE80211_RX_CMNTR_REPORTED BIT(3)
-#define IEEE80211_RX_FRAGMENTED BIT(4)
+#define IEEE80211_RX_FRAGMENTED BIT(3)
struct ieee80211_rx_data {
struct sk_buff *skb;
--- wireless-testing.orig/net/mac80211/rx.c 2009-11-16 15:12:38.000000000 +0100
+++ wireless-testing/net/mac80211/rx.c 2009-11-16 15:13:15.000000000 +0100
@@ -1864,7 +1864,7 @@ static void ieee80211_rx_cooked_monitor(
struct net_device *prev_dev = NULL;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
- if (rx->flags & IEEE80211_RX_CMNTR_REPORTED)
+ if (status->flag & RX_FLAG_INTERNAL_CMTR)
goto out_free_skb;
if (skb_headroom(skb) < sizeof(*rthdr) &&
@@ -1925,7 +1925,7 @@ static void ieee80211_rx_cooked_monitor(
} else
goto out_free_skb;
- rx->flags |= IEEE80211_RX_CMNTR_REPORTED;
+ status->flag |= RX_FLAG_INTERNAL_CMTR;
return;
out_free_skb:
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC 2/3] mac80211: move aMPDU RX reorder code
2009-11-16 14:21 [RFC 0/3] fix the aMPDU reorder placement Johannes Berg
2009-11-16 14:21 ` [RFC 1/3] mac80211: move cmntr flag out of rx flags Johannes Berg
@ 2009-11-16 14:21 ` Johannes Berg
2009-11-16 14:21 ` [RFC 3/3] mac80211: correctly place " Johannes Berg
2 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2009-11-16 14:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Jouni Malinen
This code should be part of RX handlers, so move it
to the place where it belongs without changing it.
A follow-up patch will do the changes to hook it up.
The sole purpose of this code move is to make the
other patch readable, it doesn't change the code at
all except that it now requires a different static
function declaration (which will go away too).
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
net/mac80211/rx.c | 459 ++++++++++++++++++++++++++----------------------------
1 file changed, 229 insertions(+), 230 deletions(-)
--- wireless-testing.orig/net/mac80211/rx.c 2009-11-16 15:13:15.000000000 +0100
+++ wireless-testing/net/mac80211/rx.c 2009-11-16 15:13:40.000000000 +0100
@@ -27,9 +27,9 @@
#include "tkip.h"
#include "wme.h"
-static void ieee80211_release_reorder_frames(struct ieee80211_hw *hw,
- struct tid_ampdu_rx *tid_agg_rx,
- u16 head_seq_num);
+static void __ieee80211_rx_handle_packet(struct ieee80211_hw *hw,
+ struct sk_buff *skb,
+ struct ieee80211_rate *rate);
/*
* monitor mode reception
@@ -534,6 +534,232 @@ ieee80211_rx_mesh_check(struct ieee80211
return RX_CONTINUE;
}
+#define SEQ_MODULO 0x1000
+#define SEQ_MASK 0xfff
+
+static inline int seq_less(u16 sq1, u16 sq2)
+{
+ return ((sq1 - sq2) & SEQ_MASK) > (SEQ_MODULO >> 1);
+}
+
+static inline u16 seq_inc(u16 sq)
+{
+ return (sq + 1) & SEQ_MASK;
+}
+
+static inline u16 seq_sub(u16 sq1, u16 sq2)
+{
+ return (sq1 - sq2) & SEQ_MASK;
+}
+
+
+static void ieee80211_release_reorder_frame(struct ieee80211_hw *hw,
+ struct tid_ampdu_rx *tid_agg_rx,
+ int index)
+{
+ struct ieee80211_supported_band *sband;
+ struct ieee80211_rate *rate = NULL;
+ struct sk_buff *skb = tid_agg_rx->reorder_buf[index];
+ struct ieee80211_rx_status *status;
+
+ if (!skb)
+ goto no_frame;
+
+ status = IEEE80211_SKB_RXCB(skb);
+
+ /* release the reordered frames to stack */
+ sband = hw->wiphy->bands[status->band];
+ if (!(status->flag & RX_FLAG_HT))
+ rate = &sband->bitrates[status->rate_idx];
+ __ieee80211_rx_handle_packet(hw, skb, rate);
+ tid_agg_rx->stored_mpdu_num--;
+ tid_agg_rx->reorder_buf[index] = NULL;
+
+no_frame:
+ tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
+}
+
+static void ieee80211_release_reorder_frames(struct ieee80211_hw *hw,
+ struct tid_ampdu_rx *tid_agg_rx,
+ u16 head_seq_num)
+{
+ int index;
+
+ while (seq_less(tid_agg_rx->head_seq_num, head_seq_num)) {
+ index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
+ tid_agg_rx->buf_size;
+ ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
+ }
+}
+
+/*
+ * Timeout (in jiffies) for skb's that are waiting in the RX reorder buffer. If
+ * the skb was added to the buffer longer than this time ago, the earlier
+ * frames that have not yet been received are assumed to be lost and the skb
+ * can be released for processing. This may also release other skb's from the
+ * reorder buffer if there are no additional gaps between the frames.
+ */
+#define HT_RX_REORDER_BUF_TIMEOUT (HZ / 10)
+
+/*
+ * As this function belongs to the RX path it must be under
+ * rcu_read_lock protection. It returns false if the frame
+ * can be processed immediately, true if it was consumed.
+ */
+static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
+ struct tid_ampdu_rx *tid_agg_rx,
+ struct sk_buff *skb)
+{
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ u16 sc = le16_to_cpu(hdr->seq_ctrl);
+ u16 mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;
+ u16 head_seq_num, buf_size;
+ int index;
+
+ buf_size = tid_agg_rx->buf_size;
+ head_seq_num = tid_agg_rx->head_seq_num;
+
+ /* frame with out of date sequence number */
+ if (seq_less(mpdu_seq_num, head_seq_num)) {
+ dev_kfree_skb(skb);
+ return true;
+ }
+
+ /*
+ * If frame the sequence number exceeds our buffering window
+ * size release some previous frames to make room for this one.
+ */
+ if (!seq_less(mpdu_seq_num, head_seq_num + buf_size)) {
+ head_seq_num = seq_inc(seq_sub(mpdu_seq_num, buf_size));
+ /* release stored frames up to new head to stack */
+ ieee80211_release_reorder_frames(hw, tid_agg_rx, head_seq_num);
+ }
+
+ /* Now the new frame is always in the range of the reordering buffer */
+
+ index = seq_sub(mpdu_seq_num, tid_agg_rx->ssn) % tid_agg_rx->buf_size;
+
+ /* check if we already stored this frame */
+ if (tid_agg_rx->reorder_buf[index]) {
+ dev_kfree_skb(skb);
+ return true;
+ }
+
+ /*
+ * If the current MPDU is in the right order and nothing else
+ * is stored we can process it directly, no need to buffer it.
+ */
+ if (mpdu_seq_num == tid_agg_rx->head_seq_num &&
+ tid_agg_rx->stored_mpdu_num == 0) {
+ tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
+ return false;
+ }
+
+ /* put the frame in the reordering buffer */
+ tid_agg_rx->reorder_buf[index] = skb;
+ tid_agg_rx->reorder_time[index] = jiffies;
+ tid_agg_rx->stored_mpdu_num++;
+ /* release the buffer until next missing frame */
+ index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
+ tid_agg_rx->buf_size;
+ if (!tid_agg_rx->reorder_buf[index] &&
+ tid_agg_rx->stored_mpdu_num > 1) {
+ /*
+ * No buffers ready to be released, but check whether any
+ * frames in the reorder buffer have timed out.
+ */
+ int j;
+ 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]) {
+ skipped++;
+ continue;
+ }
+ if (!time_after(jiffies, tid_agg_rx->reorder_time[j] +
+ HT_RX_REORDER_BUF_TIMEOUT))
+ break;
+
+#ifdef CONFIG_MAC80211_HT_DEBUG
+ if (net_ratelimit())
+ printk(KERN_DEBUG "%s: release an RX reorder "
+ "frame due to timeout on earlier "
+ "frames\n",
+ wiphy_name(hw->wiphy));
+#endif
+ ieee80211_release_reorder_frame(hw, tid_agg_rx, j);
+
+ /*
+ * Increment the head seq# also for the skipped slots.
+ */
+ tid_agg_rx->head_seq_num =
+ (tid_agg_rx->head_seq_num + skipped) & SEQ_MASK;
+ skipped = 0;
+ }
+ } else while (tid_agg_rx->reorder_buf[index]) {
+ ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
+ index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
+ tid_agg_rx->buf_size;
+ }
+
+ return true;
+}
+
+/*
+ * Reorder MPDUs from A-MPDUs, keeping them on a buffer. Returns
+ * true if the MPDU was buffered, false if it should be processed.
+ */
+static bool ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
+ struct sk_buff *skb)
+{
+ struct ieee80211_hw *hw = &local->hw;
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct sta_info *sta;
+ struct tid_ampdu_rx *tid_agg_rx;
+ u16 sc;
+ int tid;
+
+ if (!ieee80211_is_data_qos(hdr->frame_control))
+ return false;
+
+ /*
+ * filter the QoS data rx stream according to
+ * STA/TID and check if this STA/TID is on aggregation
+ */
+
+ sta = sta_info_get(local, hdr->addr2);
+ if (!sta)
+ return false;
+
+ tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
+
+ if (sta->ampdu_mlme.tid_state_rx[tid] != HT_AGG_STATE_OPERATIONAL)
+ return false;
+
+ tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
+
+ /* qos null data frames are excluded */
+ if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
+ return false;
+
+ /* new, potentially un-ordered, ampdu frame - process it */
+
+ /* reset session timer */
+ if (tid_agg_rx->timeout)
+ mod_timer(&tid_agg_rx->session_timer,
+ TU_TO_EXP_TIME(tid_agg_rx->timeout));
+
+ /* if this mpdu is fragmented - terminate rx aggregation session */
+ sc = le16_to_cpu(hdr->seq_ctrl);
+ if (sc & IEEE80211_SCTL_FRAG) {
+ ieee80211_sta_stop_rx_ba_session(sta->sdata, sta->sta.addr,
+ tid, 0, WLAN_REASON_QSTA_REQUIRE_SETUP);
+ dev_kfree_skb(skb);
+ return true;
+ }
+
+ return ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb);
+}
static ieee80211_rx_result debug_noinline
ieee80211_rx_h_check(struct ieee80211_rx_data *rx)
@@ -2183,233 +2409,6 @@ static void __ieee80211_rx_handle_packet
dev_kfree_skb(skb);
}
-#define SEQ_MODULO 0x1000
-#define SEQ_MASK 0xfff
-
-static inline int seq_less(u16 sq1, u16 sq2)
-{
- return ((sq1 - sq2) & SEQ_MASK) > (SEQ_MODULO >> 1);
-}
-
-static inline u16 seq_inc(u16 sq)
-{
- return (sq + 1) & SEQ_MASK;
-}
-
-static inline u16 seq_sub(u16 sq1, u16 sq2)
-{
- return (sq1 - sq2) & SEQ_MASK;
-}
-
-
-static void ieee80211_release_reorder_frame(struct ieee80211_hw *hw,
- struct tid_ampdu_rx *tid_agg_rx,
- int index)
-{
- struct ieee80211_supported_band *sband;
- struct ieee80211_rate *rate = NULL;
- struct sk_buff *skb = tid_agg_rx->reorder_buf[index];
- struct ieee80211_rx_status *status;
-
- if (!skb)
- goto no_frame;
-
- status = IEEE80211_SKB_RXCB(skb);
-
- /* release the reordered frames to stack */
- sband = hw->wiphy->bands[status->band];
- if (!(status->flag & RX_FLAG_HT))
- rate = &sband->bitrates[status->rate_idx];
- __ieee80211_rx_handle_packet(hw, skb, rate);
- tid_agg_rx->stored_mpdu_num--;
- tid_agg_rx->reorder_buf[index] = NULL;
-
-no_frame:
- tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
-}
-
-static void ieee80211_release_reorder_frames(struct ieee80211_hw *hw,
- struct tid_ampdu_rx *tid_agg_rx,
- u16 head_seq_num)
-{
- int index;
-
- while (seq_less(tid_agg_rx->head_seq_num, head_seq_num)) {
- index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
- tid_agg_rx->buf_size;
- ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
- }
-}
-
-/*
- * Timeout (in jiffies) for skb's that are waiting in the RX reorder buffer. If
- * the skb was added to the buffer longer than this time ago, the earlier
- * frames that have not yet been received are assumed to be lost and the skb
- * can be released for processing. This may also release other skb's from the
- * reorder buffer if there are no additional gaps between the frames.
- */
-#define HT_RX_REORDER_BUF_TIMEOUT (HZ / 10)
-
-/*
- * As this function belongs to the RX path it must be under
- * rcu_read_lock protection. It returns false if the frame
- * can be processed immediately, true if it was consumed.
- */
-static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
- struct tid_ampdu_rx *tid_agg_rx,
- struct sk_buff *skb)
-{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- u16 sc = le16_to_cpu(hdr->seq_ctrl);
- u16 mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;
- u16 head_seq_num, buf_size;
- int index;
-
- buf_size = tid_agg_rx->buf_size;
- head_seq_num = tid_agg_rx->head_seq_num;
-
- /* frame with out of date sequence number */
- if (seq_less(mpdu_seq_num, head_seq_num)) {
- dev_kfree_skb(skb);
- return true;
- }
-
- /*
- * If frame the sequence number exceeds our buffering window
- * size release some previous frames to make room for this one.
- */
- if (!seq_less(mpdu_seq_num, head_seq_num + buf_size)) {
- head_seq_num = seq_inc(seq_sub(mpdu_seq_num, buf_size));
- /* release stored frames up to new head to stack */
- ieee80211_release_reorder_frames(hw, tid_agg_rx, head_seq_num);
- }
-
- /* Now the new frame is always in the range of the reordering buffer */
-
- index = seq_sub(mpdu_seq_num, tid_agg_rx->ssn) % tid_agg_rx->buf_size;
-
- /* check if we already stored this frame */
- if (tid_agg_rx->reorder_buf[index]) {
- dev_kfree_skb(skb);
- return true;
- }
-
- /*
- * If the current MPDU is in the right order and nothing else
- * is stored we can process it directly, no need to buffer it.
- */
- if (mpdu_seq_num == tid_agg_rx->head_seq_num &&
- tid_agg_rx->stored_mpdu_num == 0) {
- tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
- return false;
- }
-
- /* put the frame in the reordering buffer */
- tid_agg_rx->reorder_buf[index] = skb;
- tid_agg_rx->reorder_time[index] = jiffies;
- tid_agg_rx->stored_mpdu_num++;
- /* release the buffer until next missing frame */
- index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
- tid_agg_rx->buf_size;
- if (!tid_agg_rx->reorder_buf[index] &&
- tid_agg_rx->stored_mpdu_num > 1) {
- /*
- * No buffers ready to be released, but check whether any
- * frames in the reorder buffer have timed out.
- */
- int j;
- 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]) {
- skipped++;
- continue;
- }
- if (!time_after(jiffies, tid_agg_rx->reorder_time[j] +
- HT_RX_REORDER_BUF_TIMEOUT))
- break;
-
-#ifdef CONFIG_MAC80211_HT_DEBUG
- if (net_ratelimit())
- printk(KERN_DEBUG "%s: release an RX reorder "
- "frame due to timeout on earlier "
- "frames\n",
- wiphy_name(hw->wiphy));
-#endif
- ieee80211_release_reorder_frame(hw, tid_agg_rx, j);
-
- /*
- * Increment the head seq# also for the skipped slots.
- */
- tid_agg_rx->head_seq_num =
- (tid_agg_rx->head_seq_num + skipped) & SEQ_MASK;
- skipped = 0;
- }
- } else while (tid_agg_rx->reorder_buf[index]) {
- ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
- index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
- tid_agg_rx->buf_size;
- }
-
- return true;
-}
-
-/*
- * Reorder MPDUs from A-MPDUs, keeping them on a buffer. Returns
- * true if the MPDU was buffered, false if it should be processed.
- */
-static bool ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
- struct sk_buff *skb)
-{
- struct ieee80211_hw *hw = &local->hw;
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- struct sta_info *sta;
- struct tid_ampdu_rx *tid_agg_rx;
- u16 sc;
- int tid;
-
- if (!ieee80211_is_data_qos(hdr->frame_control))
- return false;
-
- /*
- * filter the QoS data rx stream according to
- * STA/TID and check if this STA/TID is on aggregation
- */
-
- sta = sta_info_get(local, hdr->addr2);
- if (!sta)
- return false;
-
- tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
-
- if (sta->ampdu_mlme.tid_state_rx[tid] != HT_AGG_STATE_OPERATIONAL)
- return false;
-
- tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
-
- /* qos null data frames are excluded */
- if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
- return false;
-
- /* new, potentially un-ordered, ampdu frame - process it */
-
- /* reset session timer */
- if (tid_agg_rx->timeout)
- mod_timer(&tid_agg_rx->session_timer,
- TU_TO_EXP_TIME(tid_agg_rx->timeout));
-
- /* if this mpdu is fragmented - terminate rx aggregation session */
- sc = le16_to_cpu(hdr->seq_ctrl);
- if (sc & IEEE80211_SCTL_FRAG) {
- ieee80211_sta_stop_rx_ba_session(sta->sdata, sta->sta.addr,
- tid, 0, WLAN_REASON_QSTA_REQUIRE_SETUP);
- dev_kfree_skb(skb);
- return true;
- }
-
- return ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb);
-}
-
/*
* This is the receive path handler. It is called by a low level driver when an
* 802.11 MPDU is received from the hardware.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC 3/3] mac80211: correctly place aMPDU RX reorder code
2009-11-16 14:21 [RFC 0/3] fix the aMPDU reorder placement Johannes Berg
2009-11-16 14:21 ` [RFC 1/3] mac80211: move cmntr flag out of rx flags Johannes Berg
2009-11-16 14:21 ` [RFC 2/3] mac80211: move aMPDU RX reorder code Johannes Berg
@ 2009-11-16 14:21 ` Johannes Berg
2 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2009-11-16 14:21 UTC (permalink / raw)
To: linux-wireless; +Cc: Jouni Malinen
As indicated by the comment, the aMPDU RX reorder code
should logically be after ieee80211_rx_h_check(). The
previous patch moved the code there, and this patch now
hooks it up in that place by introducing a list of skbs
that are then processed by the remaining handlers. The
list may be empty if the function is buffering the skb
to release it later.
The only change needed to the RX data is that the crypto
handler needs to clear the key that may be set from a
previous loop iteration, and that not everything can be
in the rx flags now.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
net/mac80211/ieee80211_i.h | 1
net/mac80211/rx.c | 164 ++++++++++++++++++++++++++-------------------
2 files changed, 96 insertions(+), 69 deletions(-)
--- wireless-testing.orig/net/mac80211/rx.c 2009-11-16 15:14:23.000000000 +0100
+++ wireless-testing/net/mac80211/rx.c 2009-11-16 15:14:24.000000000 +0100
@@ -555,7 +555,8 @@ static inline u16 seq_sub(u16 sq1, u16 s
static void ieee80211_release_reorder_frame(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx,
- int index)
+ int index,
+ struct sk_buff_head *frames)
{
struct ieee80211_supported_band *sband;
struct ieee80211_rate *rate = NULL;
@@ -571,9 +572,9 @@ static void ieee80211_release_reorder_fr
sband = hw->wiphy->bands[status->band];
if (!(status->flag & RX_FLAG_HT))
rate = &sband->bitrates[status->rate_idx];
- __ieee80211_rx_handle_packet(hw, skb, rate);
tid_agg_rx->stored_mpdu_num--;
tid_agg_rx->reorder_buf[index] = NULL;
+ skb_queue_tail(frames, skb);
no_frame:
tid_agg_rx->head_seq_num = seq_inc(tid_agg_rx->head_seq_num);
@@ -581,14 +582,15 @@ no_frame:
static void ieee80211_release_reorder_frames(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx,
- u16 head_seq_num)
+ u16 head_seq_num,
+ struct sk_buff_head *frames)
{
int index;
while (seq_less(tid_agg_rx->head_seq_num, head_seq_num)) {
index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
tid_agg_rx->buf_size;
- ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
+ ieee80211_release_reorder_frame(hw, tid_agg_rx, index, frames);
}
}
@@ -608,7 +610,8 @@ static void ieee80211_release_reorder_fr
*/
static bool ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ struct sk_buff_head *frames)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
u16 sc = le16_to_cpu(hdr->seq_ctrl);
@@ -632,7 +635,8 @@ static bool ieee80211_sta_manage_reorder
if (!seq_less(mpdu_seq_num, head_seq_num + buf_size)) {
head_seq_num = seq_inc(seq_sub(mpdu_seq_num, buf_size));
/* release stored frames up to new head to stack */
- ieee80211_release_reorder_frames(hw, tid_agg_rx, head_seq_num);
+ ieee80211_release_reorder_frames(hw, tid_agg_rx, head_seq_num,
+ frames);
}
/* Now the new frame is always in the range of the reordering buffer */
@@ -687,7 +691,8 @@ static bool ieee80211_sta_manage_reorder
"frames\n",
wiphy_name(hw->wiphy));
#endif
- ieee80211_release_reorder_frame(hw, tid_agg_rx, j);
+ ieee80211_release_reorder_frame(hw, tid_agg_rx,
+ j, frames);
/*
* Increment the head seq# also for the skipped slots.
@@ -697,7 +702,7 @@ static bool ieee80211_sta_manage_reorder
skipped = 0;
}
} else while (tid_agg_rx->reorder_buf[index]) {
- ieee80211_release_reorder_frame(hw, tid_agg_rx, index);
+ ieee80211_release_reorder_frame(hw, tid_agg_rx, index, frames);
index = seq_sub(tid_agg_rx->head_seq_num, tid_agg_rx->ssn) %
tid_agg_rx->buf_size;
}
@@ -709,38 +714,39 @@ static bool ieee80211_sta_manage_reorder
* Reorder MPDUs from A-MPDUs, keeping them on a buffer. Returns
* true if the MPDU was buffered, false if it should be processed.
*/
-static bool ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
- struct sk_buff *skb)
+static void ieee80211_rx_reorder_ampdu(struct ieee80211_rx_data *rx,
+ struct sk_buff_head *frames)
{
+ struct sk_buff *skb = rx->skb;
+ struct ieee80211_local *local = rx->local;
struct ieee80211_hw *hw = &local->hw;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- struct sta_info *sta;
+ struct sta_info *sta = rx->sta;
struct tid_ampdu_rx *tid_agg_rx;
u16 sc;
int tid;
if (!ieee80211_is_data_qos(hdr->frame_control))
- return false;
+ goto dont_reorder;
/*
* filter the QoS data rx stream according to
* STA/TID and check if this STA/TID is on aggregation
*/
- sta = sta_info_get(local, hdr->addr2);
if (!sta)
- return false;
+ goto dont_reorder;
tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
if (sta->ampdu_mlme.tid_state_rx[tid] != HT_AGG_STATE_OPERATIONAL)
- return false;
+ goto dont_reorder;
tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
/* qos null data frames are excluded */
if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
- return false;
+ goto dont_reorder;
/* new, potentially un-ordered, ampdu frame - process it */
@@ -755,10 +761,14 @@ static bool ieee80211_rx_reorder_ampdu(s
ieee80211_sta_stop_rx_ba_session(sta->sdata, sta->sta.addr,
tid, 0, WLAN_REASON_QSTA_REQUIRE_SETUP);
dev_kfree_skb(skb);
- return true;
+ return;
}
- return ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb);
+ ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, frames);
+ return;
+
+ dont_reorder:
+ __skb_queue_tail(frames, skb);
}
static ieee80211_rx_result debug_noinline
@@ -863,6 +873,9 @@ ieee80211_rx_h_decrypt(struct ieee80211_
if (!(rx->flags & IEEE80211_RX_RA_MATCH))
return RX_CONTINUE;
+ /* start without a key */
+ rx->key = NULL;
+
if (rx->sta)
stakey = rcu_dereference(rx->sta->key);
@@ -1811,7 +1824,7 @@ ieee80211_rx_h_data(struct ieee80211_rx_
}
static ieee80211_rx_result debug_noinline
-ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx)
+ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx, struct sk_buff_head *frames)
{
struct ieee80211_local *local = rx->local;
struct ieee80211_hw *hw = &local->hw;
@@ -1841,7 +1854,8 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_
TU_TO_EXP_TIME(tid_agg_rx->timeout));
/* release stored frames up to start of BAR */
- ieee80211_release_reorder_frames(hw, tid_agg_rx, start_seq_num);
+ ieee80211_release_reorder_frames(hw, tid_agg_rx, start_seq_num,
+ frames);
kfree_skb(skb);
return RX_QUEUED;
}
@@ -2164,8 +2178,11 @@ static void ieee80211_invoke_rx_handlers
struct sk_buff *skb,
struct ieee80211_rate *rate)
{
+ struct sk_buff_head reorder_release;
ieee80211_rx_result res = RX_DROP_MONITOR;
+ __skb_queue_head_init(&reorder_release);
+
rx->skb = skb;
rx->sdata = sdata;
@@ -2173,50 +2190,72 @@ static void ieee80211_invoke_rx_handlers
do { \
res = rxh(rx); \
if (res != RX_CONTINUE) \
- goto rxh_done; \
+ goto rxh_next; \
} while (0);
+ /*
+ * NB: the rxh_next label works even if we jump
+ * to it from here because then the list will
+ * be empty, which is a trivial check
+ */
CALL_RXH(ieee80211_rx_h_passive_scan)
CALL_RXH(ieee80211_rx_h_check)
- CALL_RXH(ieee80211_rx_h_decrypt)
- CALL_RXH(ieee80211_rx_h_check_more_data)
- CALL_RXH(ieee80211_rx_h_sta_process)
- CALL_RXH(ieee80211_rx_h_defragment)
- CALL_RXH(ieee80211_rx_h_ps_poll)
- CALL_RXH(ieee80211_rx_h_michael_mic_verify)
- /* must be after MMIC verify so header is counted in MPDU mic */
- CALL_RXH(ieee80211_rx_h_remove_qos_control)
- CALL_RXH(ieee80211_rx_h_amsdu)
+
+ ieee80211_rx_reorder_ampdu(rx, &reorder_release);
+
+ while ((skb = __skb_dequeue(&reorder_release))) {
+ /*
+ * all the other fields are valid across frames
+ * that belong to an aMPDU since they are on the
+ * same TID from the same station
+ */
+ rx->skb = skb;
+
+ CALL_RXH(ieee80211_rx_h_decrypt)
+ CALL_RXH(ieee80211_rx_h_check_more_data)
+ CALL_RXH(ieee80211_rx_h_sta_process)
+ CALL_RXH(ieee80211_rx_h_defragment)
+ CALL_RXH(ieee80211_rx_h_ps_poll)
+ CALL_RXH(ieee80211_rx_h_michael_mic_verify)
+ /* must be after MMIC verify so header is counted in MPDU mic */
+ CALL_RXH(ieee80211_rx_h_remove_qos_control)
+ CALL_RXH(ieee80211_rx_h_amsdu)
#ifdef CONFIG_MAC80211_MESH
- if (ieee80211_vif_is_mesh(&sdata->vif))
- CALL_RXH(ieee80211_rx_h_mesh_fwding);
+ if (ieee80211_vif_is_mesh(&sdata->vif))
+ CALL_RXH(ieee80211_rx_h_mesh_fwding);
#endif
- CALL_RXH(ieee80211_rx_h_data)
- CALL_RXH(ieee80211_rx_h_ctrl)
- CALL_RXH(ieee80211_rx_h_action)
- CALL_RXH(ieee80211_rx_h_mgmt)
+ CALL_RXH(ieee80211_rx_h_data)
+
+ /* special treatment -- needs the queue */
+ res = ieee80211_rx_h_ctrl(rx, &reorder_release);
+ if (res != RX_CONTINUE)
+ goto rxh_next;
+
+ CALL_RXH(ieee80211_rx_h_action)
+ CALL_RXH(ieee80211_rx_h_mgmt)
#undef CALL_RXH
- rxh_done:
- switch (res) {
- case RX_DROP_MONITOR:
- I802_DEBUG_INC(sdata->local->rx_handlers_drop);
- if (rx->sta)
- rx->sta->rx_dropped++;
- /* fall through */
- case RX_CONTINUE:
- ieee80211_rx_cooked_monitor(rx, rate);
- break;
- case RX_DROP_UNUSABLE:
- I802_DEBUG_INC(sdata->local->rx_handlers_drop);
- if (rx->sta)
- rx->sta->rx_dropped++;
- dev_kfree_skb(rx->skb);
- break;
- case RX_QUEUED:
- I802_DEBUG_INC(sdata->local->rx_handlers_queued);
- break;
+ rxh_next:
+ switch (res) {
+ case RX_DROP_MONITOR:
+ I802_DEBUG_INC(sdata->local->rx_handlers_drop);
+ if (rx->sta)
+ rx->sta->rx_dropped++;
+ /* fall through */
+ case RX_CONTINUE:
+ ieee80211_rx_cooked_monitor(rx, rate);
+ break;
+ case RX_DROP_UNUSABLE:
+ I802_DEBUG_INC(sdata->local->rx_handlers_drop);
+ if (rx->sta)
+ rx->sta->rx_dropped++;
+ dev_kfree_skb(rx->skb);
+ break;
+ case RX_QUEUED:
+ I802_DEBUG_INC(sdata->local->rx_handlers_queued);
+ break;
+ }
}
}
@@ -2490,20 +2529,7 @@ void ieee80211_rx(struct ieee80211_hw *h
return;
}
- /*
- * In theory, the block ack reordering should happen after duplicate
- * removal (ieee80211_rx_h_check(), which is an RX handler). As such,
- * the call to ieee80211_rx_reorder_ampdu() should really be moved to
- * happen as a new RX handler between ieee80211_rx_h_check and
- * ieee80211_rx_h_decrypt. This cleanup may eventually happen, but for
- * the time being, the call can be here since RX reorder buf processing
- * will implicitly skip duplicates. We could, in theory at least,
- * process frames that ieee80211_rx_h_passive_scan would drop (e.g.,
- * frames from other than operational channel), but that should not
- * happen in normal networks.
- */
- if (!ieee80211_rx_reorder_ampdu(local, skb))
- __ieee80211_rx_handle_packet(hw, skb, rate);
+ __ieee80211_rx_handle_packet(hw, skb, rate);
rcu_read_unlock();
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-11-16 15:14:27.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2009-11-16 15:14:51.000000000 +0100
@@ -163,6 +163,7 @@ typedef unsigned __bitwise__ ieee80211_r
#define IEEE80211_RX_RA_MATCH BIT(1)
#define IEEE80211_RX_AMSDU BIT(2)
#define IEEE80211_RX_FRAGMENTED BIT(3)
+/* only add flags here that do not change with subframes of an aMPDU */
struct ieee80211_rx_data {
struct sk_buff *skb;
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-11-16 14:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-16 14:21 [RFC 0/3] fix the aMPDU reorder placement Johannes Berg
2009-11-16 14:21 ` [RFC 1/3] mac80211: move cmntr flag out of rx flags Johannes Berg
2009-11-16 14:21 ` [RFC 2/3] mac80211: move aMPDU RX reorder code Johannes Berg
2009-11-16 14:21 ` [RFC 3/3] mac80211: correctly place " Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).