netdev - Re: question about size of sk_buff and skb_shared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1492562843.10587.153.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Tue, 18 Apr 2017 17:47:23 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Code Soldier1 <codesoldier1@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: question about size of sk_buff and skb_shared_info


For the last time, do not top post on netdev.

On Tue, 2017-04-18 at 17:15 -0700, Code Soldier1 wrote:
> I am not suggesting that we do not do cache alignment. That is
> required for performance. I have gone through this exercise because I
> need to add a field to sk_buff and I want to do that without causing
> any adverse effects.
> 
> Now that we have discovered that there are 40 bytes that can be used
> without any adverse effect, may I increase skb->cb by 8 bytes ?
> 

skb->cb is already 48 bytes, not 40.

> If not then may I increase skb_shared_info -- However that would have
> to be by 64bytes.


You will have a very hard time to convince us that this 8 byte field is
needed on all skbs, regardless of current sk_buff size.



> 
> 
> On Tue, Apr 18, 2017 at 4:55 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> > Please do not top post on netdev
> >
> > On Tue, 2017-04-18 at 16:26 -0700, Code Soldier1 wrote:
> >> Eric,
> >>
> >> This alignment flag is passed to the cache constructor and the
> >> allocation is indeed cache aligned. However, since the allocated size
> >> is not a multiple of the alignment, wont memory be wasted ?. We can
> >> get 40 extra bytes without any side effects since they are on the same
> >> cache line ?
> >
> > We _want_ to align skb to cache lines.
> >
> > Fact that few bytes might be wasted is unfortunate, but negligible.
> >
> > BTW, if you do kmalloc(1025), kmalloc() wastes 1023 bytes.
> >
> >
> >>
> >> kmem_cache_create() code does an ALIGN() to round up the size.
> >>
> >>         kasan_cache_create(cachep, &size, &flags);
> >>
> >>         size = ALIGN(size, cachep->align);
> >>
> >> This is the size used in calculate_slab_order() to calculate num. I am
> >> assuming that in the non debug case gfp_order will be 0.
> >>
> >> Perhaps I am missing something.
> >>
> >>
> >> On Tue, Apr 18, 2017 at 12:00 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> >> > On Tue, 2017-04-18 at 10:34 -0700, Code Soldier1 wrote:
> >> >> Hi Folks,
> >> >>
> >> >> I am sure there is a reason for the current sizes of these structures,
> >> >> However the reason is not obvious to me. So please help me understand.
> >> >>
> >> >> Currently the size of sk_buff on an x86_64 system is 232 bytes -- Why
> >> >> is that. I expected it to be a multiple of 32/64 as they are the most
> >> >> common cache lines. Since the alignment calculation will align the
> >> >> structure with the hw cache line, it seems like we might be wasting
> >> >> space ?
> >> >>
> >> >> skb_shared_info on the other hand is perfectly aligned with a size of 320 bytes.
> >> >>
> >> >> Thanks,
> >> >>
> >> >
> >> > The alignment is there.
> >> > Look at skb_init() code, using SLAB_HWCACHE_ALIGN
> >> >
> >> >
> >> >
> >> >
> >> >
> >>
> >>
> >>
> >
> >
> 
> 
>