Re: [PATCH] Fix up truesize after pskb_expand_head() in wireless stack

[Johannes Berg <johannes@sipsolutions.net>]

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

On Mon, 2009-01-05 at 14:21 +0100, Andi Kleen wrote:

> > This statement is incompatible with your patch when you think about the
> > exact definition of truesize and the (unconditional!) adjustments your
> > patch makes.
> 
> __alloc_skb does 
> 
>         size = SKB_DATA_ALIGN(size);
>         skb->truesize = size + sizeof(struct sk_buff);
> 
> [ BTW it would be probably better if alloc_skb() just asked
> slab what the truesize is for kmalloc instead of guessing wrong like this.
> But that's a different topic]
> 
> and SKB_DATA_ALIGN is 
> 
> #define SKB_DATA_ALIGN(X)       (((X) + (SMP_CACHE_BYTES - 1)) & \
>                                  ~(SMP_CACHE_BYTES - 1))
> 
> and on my configuration SMP_CACHE_BYTES is 64
> 
> skb_truesize_check does 
> 
>  int len = sizeof(struct sk_buff) + skb->len;
> 
>         if (unlikely((int)skb->truesize < len))
>                 skb_truesize_bug(skb);
> 
> This means if the change is less than the 64byte cache alignment 
> (or more commonly 128 bytes on GENERIC_CPU distro kernels)
> it won't be reported. To my knowledge header adjustments are usually
> smaller and that is what pskb_expand_head() is usually used for.

Ah, indeed, I thought I remembered that truesize had to match exactly,
but obviously that wasn't done either. We should experiment with that
though, and let pskb_expand_head() adjust things.

> I didn't use any monitoring with this. No tcpdump, no wireless
> sniffer tools or anything. It happened all the time during
> normal operation.

You did have a monitor interface up though, didn't you? If nothing
actually used those skbs then it's likely that the warning didn't result
in any corruption at all.

johannes

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