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

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

[Reinette Chatre]

From: Zhu Yi <yi.zhu@intel.com>

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 <abhijeet.kolekar@intel.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 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

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]

On Wed, 2009-10-28 at 13:13 -0700, Reinette Chatre wrote:
> From: Zhu Yi <yi.zhu@intel.com>
> 
> 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 <abhijeet.kolekar@intel.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

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


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Luis R. Rodriguez]

On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
<reinette.chatre@intel.com> wrote:
> From: Zhu Yi <yi.zhu@intel.com>
>
> 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

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On Thu, 2009-10-29 at 09:18 -0700, Luis R. Rodriguez wrote:
> On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
> <reinette.chatre@intel.com> wrote:
> > From: Zhu Yi <yi.zhu@intel.com>
> >
> > 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Luis R. Rodriguez]

On Thu, Oct 29, 2009 at 9:18 AM, Luis R. Rodriguez <mcgrof@gmail.com> wrote:
> On Wed, Oct 28, 2009 at 1:13 PM, Reinette Chatre
> <reinette.chatre@intel.com> wrote:
>> From: Zhu Yi <yi.zhu@intel.com>
>>
>> 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

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Zhu Yi]

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