netdev - Re: [PATCH 1/2][RFC] vlan: use pskb_copy() when inserting a vlan tag by hand

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]

Date:	Tue, 08 Jul 2008 20:50:09 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Lennert Buytenhek <buytenh@...tstofly.org>
CC:	Nicolas Pitre <nico@....org>,
	Dale Farnsworth <dale@...nsworth.org>,
	Ashish Karkare <akarkare@...vell.com>,
	Jesper Dangaard Brouer <jdb@...x.dk>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 1/2][RFC] vlan: use pskb_copy() when inserting a vlan
 tag by hand

Lennert Buytenhek wrote:
> On Tue, Jul 08, 2008 at 06:24:44PM +0200, Patrick McHardy wrote:
>
>   
>> Actually, are you sure this patch is helping for the case
>> you describe? The function you changed is only called on
>> the RX path.
>>     
>
> You're right, I got confused with setting ->vlan_features, which is
> the actual thing that controls whether ->hard_start_xmit() gets send
> fragmented skbs or not.
>
> skb_copy() shows high in the profiles, but it's not the skb_copy() in
> vlan_check_reorder_header() (my bad), it's the skb_copy() call in
> skb_unshare() called via include/linux/if_vlan.h:__vlan_put_tag().
>   

Yes, thats expected for TCP packets.
> I've gathered some numbers for zero-copy (sendfile) sending a 1 GiB
> file filled with zeroes from the box that has the mv643xx_eth to a
> random x86 box:
>
> - 2.6.26-rc9, no VLAN tagging: ~71 sec, 14.4 MiB/s
> - 2.6.29-rc9, VLAN tagging: ~107 sec, 9.57 MiB/s
> - 2.6.29-rc9, VLAN tagging with [1] + [2]: ~94 sec: 10.9 MiB/s
> - 2.6.29-rc9, VLAN tagging with [1] + [2] + [3]: ~81 sec: 12.6 MiB/s
>
> I'm wondering whether lying to the stack about HW VLAN accel
> capability and adding the VLAN tag to the ethernet header in a
> private buffer in the driver will give me the performance back.
>
> (If I'd have to guess, I'd say that the existence of a frag list
> shouldn't matter for the shareability of an skb, but there's probably
> a good reason why skb_unshare() calls skb_copy() and not pskb_copy().)
>   

Without checking if its actually needed, I would tend to agree because
a caller can't rely on getting a linearized skb back except when its
guaranteed to be cloned, in the case it could simply copy it always.

Anyway, the copy in __vlan_put_tag() is overkill since the header
is usually writable. See the patch I sent in my second mail, it
should reduce the overhead significantly.

Actually there was a small bug in the one I sent, so attached again
to this mail.



View attachment "x" of type "text/plain" (4123 bytes)