netdev - Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than individually

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: <1325857030.2999.28.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Fri, 06 Jan 2012 14:37:10 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Ian Campbell <Ian.Campbell@...rix.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>
Subject: Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than
 individually

Le vendredi 06 janvier 2012 à 13:20 +0000, Ian Campbell a écrit :
> On Fri, 2012-01-06 at 12:33 +0000, Eric Dumazet wrote:
> > Le vendredi 06 janvier 2012 à 11:20 +0000, Ian Campbell a écrit :
> > 
> > > It doesn't fit in a single cache line today.
> > 
> > It really does, thanks to your (net: pack skb_shared_info more
> > efficiently) previous patch.
> > 
> > I dont understand your numbers, very hard to read.
> > 
> > Current net-next :
> > 
> > offsetof(struct skb_shared_info, nr_frags)=0x0
> > offsetof(struct skb_shared_info, frags[1])=0x40   (0x30 on 32bit arches)
> 
> I see 0x48 here at cset 6386994e03ebbe60338ded3d586308a41e81c0dc:

If you read my mail, I said "current net-next" 

Please "git pull" again ?


Here I really have your commit 9f42f126154786e6e76df513004800c8c633f020
in.



> (gdb) print &((struct skb_shared_info *)0)->nr_frags
> $1 = (short unsigned int *) 0x0
> (gdb) print &((struct skb_shared_info *)0)->frags[1]
> $2 = (skb_frag_t *) 0x48
> (gdb) print &((struct skb_shared_info *)0)->frags[0]
> $3 = (skb_frag_t *) 0x38
> 
> (it's 0x34 and 0x2c on 32 bit) and these numbers match what I posted for
> v3.1 (and I imagine earlier since this stuff doesn't seem to change very
> often).
> 
> I provided the offsets of each field in struct skb_shared_info, which
> one do you think is wrong?
> 
> Remember that the end shared info is explicitly aligned (to the end of
> the allocation == a cache line) while the front just ends up at wherever
> the size dictates so you can't measure the alignment of the fields from
> the beginning  of the struct, you need to measure from the end.

You're mistaken. Really.

Current code makes SURE skb->end starts at a cache line boundary
(assuminf kmalloc() returned an aligned bloc, this might be not true
with SLAB debugging)

size = SKB_WITH_OVERHEAD(ksize(data));
...
skb->end = skb->tail + size;

So there is possibility of padding (less than 64 bytes) _after_
skb_shared_info and before the end of allocated area.

After your 9f42f1261547 commit on 64bit, we have no padding anymore
since sizeof(struct skb_shared_info)=0x140



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html