netdev - Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS hash for any flow type

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, 17 Oct 2023 11:42:02 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: Ahmed Zaki <ahmed.zaki@...el.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, 
	intel-wired-lan@...ts.osuosl.org, corbet@....net, jesse.brandeburg@...el.com, 
	anthony.l.nguyen@...el.com, davem@...emloft.net, edumazet@...gle.com, 
	pabeni@...hat.com, vladimir.oltean@....com, andrew@...n.ch, horms@...nel.org, 
	mkubecek@...e.cz, willemdebruijn.kernel@...il.com, linux-doc@...r.kernel.org, 
	Wojciech Drewek <wojciech.drewek@...el.com>
Subject: Re: [PATCH net-next v4 1/6] net: ethtool: allow symmetric-xor RSS
 hash for any flow type

On Mon, Oct 16, 2023 at 5:08 PM Ahmed Zaki <ahmed.zaki@...el.com> wrote:
>
>
>
> On 2023-10-16 17:30, Jakub Kicinski wrote:
> > On Mon, 16 Oct 2023 15:55:21 -0700 Alexander Duyck wrote:
> >> It would make more sense to just add it as a variant hash function of
> >> toeplitz. If you did it right you could probably make the formatting
> >> pretty, something like:
> >> RSS hash function:
> >>      toeplitz: on
> >>          symmetric xor: on
> >>      xor: off
> >>      crc32: off
> >>
> >> It doesn't make sense to place it in the input flags and will just
> >> cause quick congestion as things get added there. This is an algorithm
> >> change so it makes more sense to place it there.
> >
> > Algo is also a bit confusing, it's more like key pre-processing?
> > There's nothing toeplitz about xoring input fields. Works as well
> > for CRC32.. or XOR.
> >
> > We can use one of the reserved fields of struct ethtool_rxfh to carry
> > this extension. I think I asked for this at some point, but there's
> > only so much repeated feedback one can send in a day :(
>
> Sorry you felt that. I took you comment [1]:
>
> "Using hashing algo for configuring fields feels like a dirty hack".
>
> To mean that the we should not use the hfunc API ("ethtool_rxfh"). This
> is why in the new series I chose to configure the RSS fields. This also
> provides the user with more control and better granularity on which
> flow-types to be symmetric, and which protocols (L3 and/or L4) to use. I
> have no idea how to do any of these via hfunc/ethtool_rxfh API so it
> seemed a better approach.
>
> I see you marked the series as "Changes Requested". I will send a new
> version tomorrow and move the sanity checks inside ice_ethtool.
>
>
> [1]: https://lore.kernel.org/netdev/20230824174336.6fb801d5@kernel.org/

So one question I would have is what happens if you were to ignore the
extra configuration that prevents people from disabling either source
or destination from the input? Does it actually have to be hard
restricted or do you end up with the hardware generating non-symmetric
hashes because it isn't doing the XOR with both source and destination
fields?

My thought would be to possibly just look at reducing your messaging
to a warning from the driver if the inputs are not symmetric, but you
have your symmetric xor hash function enabled.