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

On Tue, Oct 31, 2023 at 5:01 AM Gal Pressman <gal@...dia.com> wrote:
>
> On 29/10/2023 18:59, Ahmed Zaki wrote:
> >
> >
> > On 2023-10-29 06:48, Gal Pressman wrote:
> >> On 29/10/2023 14:42, Ahmed Zaki wrote:
> >>>
> >>>
> >>> On 2023-10-29 06:25, Gal Pressman wrote:
> >>>> On 21/10/2023 3:00, Ahmed Zaki wrote:
> >>>>>
> >>>>>
> >>>>> On 2023-10-20 17:49, Jakub Kicinski wrote:
> >>>>>> On Fri, 20 Oct 2023 17:14:11 -0600 Ahmed Zaki wrote:
> >>>>>>> I replied to that here:
> >>>>>>>
> >>>>>>> https://lore.kernel.org/all/afb4a06f-cfba-47ba-adb3-09bea7cb5f00@intel.com/
> >>>>>>>
> >>>>>>> I am kind of confused now so please bear with me. ethtool either
> >>>>>>> sends
> >>>>>>> "ethtool_rxfh" or "ethtool_rxnfc". AFAIK "ethtool_rxfh" is the
> >>>>>>> interface
> >>>>>>> for "ethtool -X" which is used to set the RSS algorithm. But we
> >>>>>>> kind of
> >>>>>>> agreed to go with "ethtool -U|-N" for symmetric-xor, and that uses
> >>>>>>> "ethtool_rxnfc" (as implemented in this series).
> >>>>>>
> >>>>>> I have no strong preference. Sounds like Alex prefers to keep it
> >>>>>> closer
> >>>>>> to algo, which is "ethtool_rxfh".
> >>>>>>
> >>>>>>> Do you mean use "ethtool_rxfh" instead of "ethtool_rxnfc"? how would
> >>>>>>> that work on the ethtool user interface?
> >>>>>>
> >>>>>> I don't know what you're asking of us. If you find the code to
> >>>>>> confusing
> >>>>>> maybe someone at Intel can help you :|
> >>>>>
> >>>>> The code is straightforward. I am confused by the requirements: don't
> >>>>> add a new algorithm but use "ethtool_rxfh".
> >>>>>
> >>>>> I'll see if I can get more help, may be I am missing something.
> >>>>>
> >>>>
> >>>> What was the decision here?
> >>>> Is this going to be exposed through ethtool -N or -X?
> >>>
> >>> I am working on a new version that uses "ethtool_rxfh" to set the
> >>> symmetric-xor. The user will set per-device via:
> >>>
> >>> ethtool -X eth0 hfunc toeplitz symmetric-xor
> >>>
> >>> then specify the per-flow type RSS fields as usual:
> >>>
> >>> ethtool -N|-U eth0 rx-flow-hash <flow_type> s|d|f|n
> >>>
> >>> The downside is that all flow-types will have to be either symmetric or
> >>> asymmetric.
> >>
> >> Why are we making the interface less flexible than it can be with -N?
> >
> > Alexander Duyck prefers to implement the "symmetric-xor" interface as an
> > algorithm or extension (please refer to previous messages), but ethtool
> > does not provide flowtype/RSS fields setting via "-X". The above was the
> > best solution that we (at Intel) could think of.
>
> OK, it's a weird we're deliberately limiting our interface, given
> there's already hardware that supports controlling symmetric hashing per
> flow type.
>
> I saw you mentioned the way ice hardware implements symmetric-xor
> somewhere, it definitely needs to be added somewhere in our
> documentation to prevent confusion.
> mlx5 hardware also does symmetric hashing with xor, but not exactly as
> you described, we need the algorithm to be clear.

It is precisely because of the way the symmetric-xor implements it
that I suggested that they change the algo type instead of the input
fields.

Instead of doing something such as rearranging the inputs, what they
do is start XORing them together and then using those values for both
the source and destination ports. It would be one thing if they
swapped them, but instead they destroy the entropy provided by XORing
the values together and then doubling them up in the source and
destination fields. The result is the hash value becomes predictable
in that once you know the target you just have to offset the source
and destination port/IP by the same amount so that they hash out to
the same values, and as a result it would make DDoS attacks based on
the RSS hash much easier.

Where I draw the line in this is if we start losing entropy without
explicitly removing the value then it is part of the algo, whereas if
it is something such as placement or us explicitly saying we don't
want certain fields in there then it would be part of the input.
Adding fields to the input should increase or at least maintain the
entropy is my point of view.