Re: [PATCH] tcp: assign the sock correctly to an outgoing SYNACK packet

[Paul Moore <pmoore@redhat.com>]

On Tuesday, April 09, 2013 09:19:30 AM Paul Moore wrote:
> On Monday, April 08, 2013 06:24:59 PM Casey Schaufler wrote:
> > On 4/8/2013 6:09 PM, Eric Dumazet wrote:
> > > On Mon, 2013-04-08 at 17:59 -0700, Casey Schaufler wrote:
> > >> I don't see that with adding 4 bytes. Again, I'm willing to be
> > >> educated if I'm wrong.
> > > 
> > > Feel free to add 4 bytes without having the 'align to 8 bytes' problem
> > > on 64 bit arches. Show us your patch.
> > 
> > Recall that it's replacing an existing 4 byte value with an 8 byte value.
> > My compiler days were quite short and long ago, but it would seem that
> > an 8 byte value ought not have an 'align to 8 bytes' problem.
> > 
> > Again, I'm willing to be educated.
> 
> Armed with a cup of coffee I took a look at the sk_buff structure this
> morning with the pahole tool and using the current sk_buff if we turn on
> all the #ifdefs here is what I see on x86_64:
> 
> struct sk_buff {

...

>         /* size: 280, cachelines: 5, members: 62 */
>         /* sum members: 270, holes: 3, sum holes: 10 */
>         /* bit holes: 1, sum bit holes: 6 bits */
>         /* last cacheline: 24 bytes */
> };
> 
> It looks like there some holes we might be able to capitalize on.  If we
> remove "secmark" (we can handle it inside a security blob) and move
> "protocol" to after the flags2 bit field we can make an aligned 8 byte hold
> for a security blob before "destructor".  According to pahole the structure
> size stays the same and the only field which moves to a different cacheline
> is "dma_cookie" which moves from cacheline 2 to 3.  Here is the pahole
> output:
> 
> struct sk_buff_test {

...

>         /* size: 280, cachelines: 5, members: 62 */
>         /* sum members: 274, holes: 3, sum holes: 6 */
>         /* bit holes: 1, sum bit holes: 6 bits */
>         /* last cacheline: 24 bytes */
> };
> 
> As Casey already mentioned, if this isn't acceptable please help me
> understand why.

For the sake of completeness I also checked out the changes when compiled for 
32 bit and it was very much the same; same structure size and in the 32 bit 
case no field movement from one cacheline to another.

-- 
paul moore
security and virtualization @ redhat