netdev - Re: [PATCH net-next 1/2] net: Expose header length compution function

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 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:	Wed, 21 May 2014 08:03:58 -0700
From:	Alexander Duyck <alexander.h.duyck@...el.com>
To:	Eric Dumazet <eric.dumazet@...il.com>,
	Alexander Duyck <alexander.duyck@...il.com>
CC:	David Miller <davem@...emloft.net>, amirv@...lanox.com,
	netdev@...r.kernel.org, idos@...lanox.com,
	jeffrey.t.kirsher@...el.com, jesse.brandeburg@...el.com,
	bruce.w.allan@...el.com, carolyn.wyborny@...el.com,
	donald.c.skidmore@...el.com, gregory.v.rose@...el.com,
	john.ronciak@...el.com, mitch.a.williams@...el.com,
	yevgenyp@...lanox.com, ogerlitz@...lanox.com
Subject: Re: [PATCH net-next 1/2] net: Expose header length compution function

On 05/19/2014 02:01 PM, Eric Dumazet wrote:
> On Sat, 2014-05-10 at 14:53 -0700, Alexander Duyck wrote:
> 
>> I'm more of a fan of purpose built functions in hot-path.  In the case
>> of skb_flow_dissect, it is meant to collect the inputs for a Jenkins
>> hash.
> 
> Not really.
> 
> And having multiple flow dissectors is really a lot of trouble for us,
> and contributes to code bloat.
> 
>>   If we also expand it to get the length my concern is that it may
>> do both, but it won't be very efficient at doing either, and that
>> doesn't even take into account that somebody at some point might want
>> the flow dissector to not do things like coalesce IPv6 addresses to
>> support things like a Toeplitz hash which would slow things down further.
>>
>> I can wait for the patch. I don't really see what you're talking about
>> since we are trying to linearize the header portion of the buffers and
>> for jumbos frames all 2K of the buffer has been used so you can't do any
>> tricks like use a paged frag for the head.
> 

So it looks like you did kind of what I expected you would, only you
allocated a temporary sk_buff on the stack and then pointed the head to
the start of the page.  I'm not really a fan of this approach though it
does give me a couple ideas.

One thought I just had though, what if we were to do something like
create an eth_build_skb function?  It would essentially be a cross
between eth_type_trans, your new eth_frame_headlen function, and
build_skb.  It would allow us to avoid the unnecessary allocation of an
skb on the stack and avoid any unnecessary data duplication since we
already would be doing a number of the eth_type_trans steps in your
eth_frame_headlen function.  The one limitation is that we would need to
allocate a block of memory for the head, but that would be done after we
figure out what the size of the header is.

If I get a chance I might try coding it up on Friday to see what
something like that might look like.

Thanks,

Alex

--
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