netdev - Re: [PATCH net-next] flow_dissector: remove __flow_hash

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: <CALx6S35ZmVyhX5vv15Ao459pVzWk3tRJuieBQ_dp5ymgQXM9wg@mail.gmail.com>
Date:	Wed, 29 Jul 2015 14:47:48 -0700
From:	Tom Herbert <tom@...bertland.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Kernel Team <kernel-team@...com>
Subject: Re: [PATCH net-next] flow_dissector: remove __flow_hash_consistentify

On Wed, Jul 29, 2015 at 2:19 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Wed, 2015-07-29 at 13:49 -0700, Tom Herbert wrote:
>> The intent of this function was to produce a consistent hash for both
>> directions of a flow. However, since we added more inputs to the flow
>> hashing (IPv6 flow labels for instance) in a lot of cases we won't get
>> the same hash computed for each direction anyway. Also, there is no
>> defined correlation between the hashes computed in each direction of a
>> flow.
>>
>> This patch removes the function since it is not providing significant
>> value and is expensive to be called for every packet. If there are
>> ever users of the flow_hash_from_keys that did require consistency
>> they can swap addresses and ports as needed in the flow_keys before
>> calling flow_hash_from_keys.
>
> Have you tested this change with conntracking and RPS enabled ?
>
> This was whole point from commit b249dcb82d327e41
>
> I guess difference is even bigger today after removal of central
> conntracking lock.
>
Hi Eric,

So the scenario you're thinking is conntrack in the forwarding path,
RPS enabled (RSS not relevant), no hash from device, no IPv6 flow
labels or any other asymmetric inputs into the flow hash? I can look
at that, but it does make me wonder if maybe conntrack should set RFS
for both sides to avoid any issue with asymmetric hashes. With more
IPv6 and flow labels (which we will enable by default), asymmetric
hashes will likely become the norm.

Thanks,
Tom


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