2009-10-28 20:13:47

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH] mac80211: make align adjustment code support paged SKB

From: Zhu Yi <[email protected]>

This fixed a BUG_ON in __skb_trim() when paged rx is used in
iwlwifi driver. Yes, the whole mac80211 stack doesn't support
paged SKB yet. But let's start the work slowly from small
code snippets.

Reported-and-tested-by: Abhijeet Kolekar <[email protected]>
Signed-off-by: Zhu Yi <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
net/mac80211/rx.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5c385e3..a50d5f3 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1337,10 +1337,10 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
skb = NULL;
} else {
u8 *data = skb->data;
- size_t len = skb->len;
- u8 *new = __skb_push(skb, align);
- memmove(new, data, len);
- __skb_trim(skb, len);
+ size_t len = skb_headlen(skb);
+ skb->data -= align;
+ memmove(skb->data, data, len);
+ skb_set_tail_pointer(skb, len);
}
}
#endif
--
1.5.6.3



2009-10-29 16:18:36

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
<[email protected]> wrote:
> From: Zhu Yi <[email protected]>
>
> This fixed a BUG_ON in __skb_trim() when paged rx is used in
> iwlwifi driver. Yes, the whole mac80211 stack doesn't support
> paged SKB yet. But let's start the work slowly from small
> code snippets.

So I just noticed ar9271 segments some frames upon RX. The current
implementation approach is to stitch them back together through a
newly allocated skb after all segments are received. How does iwlwifi
ues paged skbs? How do you keep track of which page is for what frame?

Can you elaborate on the planned paged skb support on mac80211 you
have. Do you have any queued up patches I can look at this point?

Luis

2009-10-29 18:58:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Thu, 2009-10-29 at 11:52 -0700, Luis R. Rodriguez wrote:

> > Can you elaborate on the planned paged skb support on mac80211 you
> > have. Do you have any queued up patches I can look at this point?
>
> OK I see git show 924e10
>
> Anyone taking the mac80211 paged skb support on? Any idea how long it
> may take to complete?

It's not very hard.

Step 1)
fix a few corner cases other drivers might have, and stick the bit of
code iwlwifi has right before passing the frame to mac80211 into
mac80211 as the first thing it does with the frame

Step 2)
Look through rx.c and check how much stuff it really needs linearised,
and where. I suspect that most of the time it _only_ needs the first
two bytes, until it comes to actually passing the frame up.

Step 3)
After the audit, push down the linearizing to where needed, i.e.
management frames completely, other frames up to and including the
ethernet header, software decryption all of the frame

Step 4)
make software decryption aware of paged SKBs, and remove the
linearising in that case

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-29 18:32:11

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Thu, 2009-10-29 at 09:18 -0700, Luis R. Rodriguez wrote:
> On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
> <[email protected]> wrote:
> > From: Zhu Yi <[email protected]>
> >
> > This fixed a BUG_ON in __skb_trim() when paged rx is used in
> > iwlwifi driver. Yes, the whole mac80211 stack doesn't support
> > paged SKB yet. But let's start the work slowly from small
> > code snippets.
>
> So I just noticed ar9271 segments some frames upon RX. The current
> implementation approach is to stitch them back together through a
> newly allocated skb after all segments are received. How does iwlwifi
> ues paged skbs? How do you keep track of which page is for what frame?
>
> Can you elaborate on the planned paged skb support on mac80211 you
> have. Do you have any queued up patches I can look at this point?

Look at commit 924e103a665300f4f25538889bdd37b256d8f787 -- the magic is
in this bit of code:

+ /* mac80211 currently doesn't support paged SKB. Convert it to
+ * linear SKB for management frame and data frame requires
+ * software decryption or software defragementation. */
+ if (ieee80211_is_mgmt(hdr->frame_control) ||
+ ieee80211_has_protected(hdr->frame_control) ||
+ ieee80211_has_morefrags(hdr->frame_control) ||
+ le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG)
+ ret = skb_linearize(skb);
+ else
+ ret = __pskb_pull_tail(skb, min_t(u16, IWL_LINK_HDR_MAX, len)) ?
+ 0 : -ENOMEM;
+
+ if (ret) {
+ kfree_skb(skb);
+ goto out;
+ }

I plan on moving that into mac80211 so that you can just pass an
arbitrarily fragmented frame into mac80211.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-28 20:15:54

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Wed, 2009-10-28 at 13:13 -0700, Reinette Chatre wrote:
> From: Zhu Yi <[email protected]>
>
> This fixed a BUG_ON in __skb_trim() when paged rx is used in
> iwlwifi driver. Yes, the whole mac80211 stack doesn't support
> paged SKB yet. But let's start the work slowly from small
> code snippets.
>
> Reported-and-tested-by: Abhijeet Kolekar <[email protected]>
> Signed-off-by: Zhu Yi <[email protected]>
> Signed-off-by: Reinette Chatre <[email protected]>

Acked-by: Johannes Berg <[email protected]>

we'll also be working on moving more paged RX awareness into the stack
so other drivers can benefit and things get more efficient.

johannes

> ---
> net/mac80211/rx.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 5c385e3..a50d5f3 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1337,10 +1337,10 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
> skb = NULL;
> } else {
> u8 *data = skb->data;
> - size_t len = skb->len;
> - u8 *new = __skb_push(skb, align);
> - memmove(new, data, len);
> - __skb_trim(skb, len);
> + size_t len = skb_headlen(skb);
> + skb->data -= align;
> + memmove(skb->data, data, len);
> + skb_set_tail_pointer(skb, len);
> }
> }
> #endif


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-29 18:52:48

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Thu, Oct 29, 2009 at 9:18 AM, Luis R. Rodriguez <[email protected]> wrote:
> On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
> <[email protected]> wrote:
>> From: Zhu Yi <[email protected]>
>>
>> This fixed a BUG_ON in __skb_trim() when paged rx is used in
>> iwlwifi driver. Yes, the whole mac80211 stack doesn't support
>> paged SKB yet. But let's start the work slowly from small
>> code snippets.
>
> So I just noticed ar9271 segments some frames upon RX. The current
> implementation approach is to stitch them back together through a
> newly allocated skb after all segments are received. How does iwlwifi
> ues paged skbs? How do you keep track of which page is for what frame?
>
> Can you elaborate on the planned paged skb support on mac80211 you
> have. Do you have any queued up patches I can look at this point?

OK I see git show 924e10

Anyone taking the mac80211 paged skb support on? Any idea how long it
may take to complete?

Luis

2009-10-30 02:46:05

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] mac80211: make align adjustment code support paged SKB

On Fri, 2009-10-30 at 02:58 +0800, Johannes Berg wrote:
> Step 1)
> fix a few corner cases other drivers might have, and stick the bit
> of
> code iwlwifi has right before passing the frame to mac80211 into
> mac80211 as the first thing it does with the frame
>
> Step 2)
> Look through rx.c and check how much stuff it really needs
> linearised,
> and where. I suspect that most of the time it _only_ needs the first
> two bytes, until it comes to actually passing the frame up.
>
> Step 3)
> After the audit, push down the linearizing to where needed, i.e.
> management frames completely, other frames up to and including the
> ethernet header, software decryption all of the frame
>
> Step 4)
> make software decryption aware of paged SKBs, and remove the
> linearising in that case

Yes, this sounds like a very good plan.

Thanks,
-yi