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 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:	Sat, 10 May 2014 14:53:43 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Eric Dumazet <eric.dumazet@...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,
	alexander.h.duyck@...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/10/2014 10:49 AM, Eric Dumazet wrote:
> On Sat, 2014-05-10 at 10:12 -0700, Alexander Duyck wrote:
>
>> Actually the flow keys with the exception of maybe thoff and ip_proto
>> are pretty much useless to us for our purpose.  As is we are left to
>> then go off and compute the header length of the transport before we get
>> to the information we need.
>>
>> The two functions are very similar though.  It might be worth while to
>> actually park the two next to each other so that it is obvious that when
>> one gets updated to support a new protocol the other should as well.  So
>> for example the headlen function could pick up some of the tunnel stuff.
>>
>> The main difference between __net_get_headlen() and skb_flow_dissect()
>> is that skb_flow_dissect is meant to work on a packet headed in either
>> direction, as such there are multiple calls to skb_copy_bits and all the
>> copies out of the paged area that go with it.  __net_get_headlen() is
>> meant to be called by the base netdev on a linear buffer to parse
>> through everything from the start of the Ethernet header to the start of
>> the data section and determine the length of that region.  So one
>> obvious difference is that we care about the length of non-l4 headers,
>> whereas skb_flow_dissect does not.
> Its actually possible to prepare an skb with the needed parts so that
> skb_flow_dissect() has no copy to do.
>
> Its a 10 lines helper maybe.
>
> I already mentioned this in the past, I fail to understand your
> resistance.
>
> I will provide it unless someone beats me, because its the week end and
> I have other plans at the moment.

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

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