* [PATCH V2] mac80211: Fix bug in getting rx status for frames pending in reorder buffer
@ 2009-03-25 12:04 Vasanthakumar Thiagarajan
2009-03-25 12:16 ` Johannes Berg
0 siblings, 1 reply; 3+ messages in thread
From: Vasanthakumar Thiagarajan @ 2009-03-25 12:04 UTC (permalink / raw)
To: linville, johannes; +Cc: linux-wireless
Currently rx status for frames which are completed from reorder buffer
is taken from it's cb area which is not always right, cb is not holding
the rx status when driver uses mac80211's non-irq rx handler to pass it's
received frames. This results in dropping almost all frames from reorder
buffer when security is enabled by doing double decryption (first in hw,
second in sw because of wrong rx status). This patch copies rx status into
cb area before the frame is put into reorder buffer. After this patch,
there is a significant improvement in throughput with ath9k + WPA2(AES).
Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
---
v2
---
As issue lies only in non-irq rx handler path, change
the commit log accordingly.
net/mac80211/rx.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 64ebe66..5fa7aed 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -29,6 +29,7 @@
static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx,
struct sk_buff *skb,
+ struct ieee80211_rx_status *status,
u16 mpdu_seq_num,
int bar_req);
/*
@@ -1688,7 +1689,7 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx)
/* manage reordering buffer according to requested */
/* sequence number */
rcu_read_lock();
- ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, NULL,
+ ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, NULL, NULL,
start_seq_num, 1);
rcu_read_unlock();
return RX_DROP_UNUSABLE;
@@ -2293,6 +2294,7 @@ static inline u16 seq_sub(u16 sq1, u16 sq2)
static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
struct tid_ampdu_rx *tid_agg_rx,
struct sk_buff *skb,
+ struct ieee80211_rx_status *rxstatus,
u16 mpdu_seq_num,
int bar_req)
{
@@ -2374,6 +2376,8 @@ static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
/* put the frame in the reordering buffer */
tid_agg_rx->reorder_buf[index] = skb;
+ memcpy(tid_agg_rx->reorder_buf[index]->cb, rxstatus,
+ sizeof(*rxstatus));
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)
@@ -2399,7 +2403,8 @@ static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
}
static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
- struct sk_buff *skb)
+ struct sk_buff *skb,
+ struct ieee80211_rx_status *status)
{
struct ieee80211_hw *hw = &local->hw;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
@@ -2448,7 +2453,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
/* according to mpdu sequence number deal with reordering buffer */
mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;
- ret = ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb,
+ ret = ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, status,
mpdu_seq_num, 0);
end_reorder:
return ret;
@@ -2512,7 +2517,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
return;
}
- if (!ieee80211_rx_reorder_ampdu(local, skb))
+ if (!ieee80211_rx_reorder_ampdu(local, skb, status))
__ieee80211_rx_handle_packet(hw, skb, status, rate);
rcu_read_unlock();
--
1.5.5.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH V2] mac80211: Fix bug in getting rx status for frames pending in reorder buffer
2009-03-25 12:04 [PATCH V2] mac80211: Fix bug in getting rx status for frames pending in reorder buffer Vasanthakumar Thiagarajan
@ 2009-03-25 12:16 ` Johannes Berg
2009-03-25 12:31 ` Vasanthakumar Thiagarajan
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Berg @ 2009-03-25 12:16 UTC (permalink / raw)
To: Vasanthakumar Thiagarajan; +Cc: linville, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 3890 bytes --]
On Wed, 2009-03-25 at 17:34 +0530, Vasanthakumar Thiagarajan wrote:
> Currently rx status for frames which are completed from reorder buffer
> is taken from it's cb area which is not always right, cb is not holding
> the rx status when driver uses mac80211's non-irq rx handler to pass it's
> received frames. This results in dropping almost all frames from reorder
> buffer when security is enabled by doing double decryption (first in hw,
> second in sw because of wrong rx status). This patch copies rx status into
> cb area before the frame is put into reorder buffer. After this patch,
> there is a significant improvement in throughput with ath9k + WPA2(AES).
>
> Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
Acked-by: Johannes Berg <johannes@sipsolutions.net>
I'll look at putting it there in the drivers right away for .31, this
should probably get a Cc: stable@kernel.org too.
johannes
> ---
> v2
> ---
> As issue lies only in non-irq rx handler path, change
> the commit log accordingly.
>
> net/mac80211/rx.c | 13 +++++++++----
> 1 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 64ebe66..5fa7aed 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -29,6 +29,7 @@
> static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
> struct tid_ampdu_rx *tid_agg_rx,
> struct sk_buff *skb,
> + struct ieee80211_rx_status *status,
> u16 mpdu_seq_num,
> int bar_req);
> /*
> @@ -1688,7 +1689,7 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_data *rx)
> /* manage reordering buffer according to requested */
> /* sequence number */
> rcu_read_lock();
> - ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, NULL,
> + ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, NULL, NULL,
> start_seq_num, 1);
> rcu_read_unlock();
> return RX_DROP_UNUSABLE;
> @@ -2293,6 +2294,7 @@ static inline u16 seq_sub(u16 sq1, u16 sq2)
> static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
> struct tid_ampdu_rx *tid_agg_rx,
> struct sk_buff *skb,
> + struct ieee80211_rx_status *rxstatus,
> u16 mpdu_seq_num,
> int bar_req)
> {
> @@ -2374,6 +2376,8 @@ static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
>
> /* put the frame in the reordering buffer */
> tid_agg_rx->reorder_buf[index] = skb;
> + memcpy(tid_agg_rx->reorder_buf[index]->cb, rxstatus,
> + sizeof(*rxstatus));
> 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)
> @@ -2399,7 +2403,8 @@ static u8 ieee80211_sta_manage_reorder_buf(struct ieee80211_hw *hw,
> }
>
> static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
> - struct sk_buff *skb)
> + struct sk_buff *skb,
> + struct ieee80211_rx_status *status)
> {
> struct ieee80211_hw *hw = &local->hw;
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> @@ -2448,7 +2453,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
>
> /* according to mpdu sequence number deal with reordering buffer */
> mpdu_seq_num = (sc & IEEE80211_SCTL_SEQ) >> 4;
> - ret = ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb,
> + ret = ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, status,
> mpdu_seq_num, 0);
> end_reorder:
> return ret;
> @@ -2512,7 +2517,7 @@ void __ieee80211_rx(struct ieee80211_hw *hw, struct sk_buff *skb,
> return;
> }
>
> - if (!ieee80211_rx_reorder_ampdu(local, skb))
> + if (!ieee80211_rx_reorder_ampdu(local, skb, status))
> __ieee80211_rx_handle_packet(hw, skb, status, rate);
>
> rcu_read_unlock();
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH V2] mac80211: Fix bug in getting rx status for frames pending in reorder buffer
2009-03-25 12:16 ` Johannes Berg
@ 2009-03-25 12:31 ` Vasanthakumar Thiagarajan
0 siblings, 0 replies; 3+ messages in thread
From: Vasanthakumar Thiagarajan @ 2009-03-25 12:31 UTC (permalink / raw)
To: Johannes Berg
Cc: Vasanth Thiagarajan, linville@tuxdriver.com,
linux-wireless@vger.kernel.org
On Wed, Mar 25, 2009 at 05:46:48PM +0530, Johannes Berg wrote:
> On Wed, 2009-03-25 at 17:34 +0530, Vasanthakumar Thiagarajan wrote:
> > Currently rx status for frames which are completed from reorder buffer
> > is taken from it's cb area which is not always right, cb is not holding
> > the rx status when driver uses mac80211's non-irq rx handler to pass it's
> > received frames. This results in dropping almost all frames from reorder
> > buffer when security is enabled by doing double decryption (first in hw,
> > second in sw because of wrong rx status). This patch copies rx status into
> > cb area before the frame is put into reorder buffer. After this patch,
> > there is a significant improvement in throughput with ath9k + WPA2(AES).
> >
> > Signed-off-by: Vasanthakumar Thiagarajan <vasanth@atheros.com>
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
>
> I'll look at putting it there in the drivers right away for .31, this
> should probably get a Cc: stable@kernel.org too.
>
> johannes
>
I will resend the patch adding stable in Cc, thanks.
Vasanth
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-03-25 12:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-25 12:04 [PATCH V2] mac80211: Fix bug in getting rx status for frames pending in reorder buffer Vasanthakumar Thiagarajan
2009-03-25 12:16 ` Johannes Berg
2009-03-25 12:31 ` Vasanthakumar Thiagarajan
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).