netdev - Re: [net-next 09/14] igb: Report L4 Rx hash via skb->l4

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:	Thu, 17 Jan 2013 10:46:52 -0800
From:	Jesse Gross <jesse@...ira.com>
To:	Alexander Duyck <alexander.h.duyck@...el.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	davem@...emloft.net, netdev@...r.kernel.org, gospo@...hat.com,
	sassmann@...hat.com
Subject: Re: [net-next 09/14] igb: Report L4 Rx hash via skb->l4_rxhash

On Thu, Jan 17, 2013 at 9:23 AM, Alexander Duyck
<alexander.h.duyck@...el.com> wrote:
> On 01/17/2013 09:14 AM, Eric Dumazet wrote:
>> On Thu, 2013-01-17 at 09:07 -0800, Alexander Duyck wrote:
>>
>>> Isn't the same true of TCP?  I believe STT is intended to run over the
>>> TCP protocol, or am I getting ahead of myself since STT is not supported
>>> by the kernel?
>>>
>>
>> probably ;)
>>
>>>> Also, is IGB really using the ports in the rss for UDP packets ?
>>>
>>> Not by default.  The default is to only hash on the IP header for UDP
>>> packets.
>>>
>>> As such the default would only be setting the l4_rxhash on TCP frames
>>> only.  The user would have to specifically request L4 port hashing for
>>> UDP via the "ethtool -N" command for configuring rx-flow-hash.
>>
>> So you should rewrite this patch ?
>>
>> Or have I missed something ?
>
> I'm probably going to scrap it.  No point in rewriting it.
>
> I has assumed that there were other uses for the l4_rxhash value.  If
> all it is meant for is to indicate that the inner header of a tunnel was
> used to compute the hash then there isn't much point to adding support
> for this in igb/ixgbe since they don't support any inner header hashing.
>
> It might add value at some point to rename the l4_rxhash flag to
> something else though since it seems like there are now tunnels that are
> encapsulated inside of l4 headers and it is going to get confusing.

I think there is value in setting it for TCP since it is used by
things like RPS and VXLAN (on the encapsulation side, to generate the
source port).

Ultimately, the semantics that I think we are looking for here is
whether the software flow dissector would look deeper into the packet
than the NIC.  In the case of STT, the answer to this is going to be
no, so there's no reason to not set the flag.  Even for protocols like
VXLAN, in software we don't currently look deeply into the packet on
receive, so we could set it there as well.  Both of these protocols
also have the advantage that the outer header contains an inner hash.

In the case of UDP tunnels, it would be nice to use the ports for RSS
since they often do contain these hashes.  We just have to make sure
that they aren't IP fragments.
--
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