Message-ID: <ef588877-0ec9-43fd-a532-e3605139593b@intel.com>
Date: Tue, 17 Oct 2023 16:12:11 -0600
From: Ahmed Zaki <ahmed.zaki@...el.com>
To: Alexander Duyck <alexander.duyck@...il.com>, Jakub Kicinski
	<kuba@...nel.org>
CC: <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>,
	<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 2023-10-17 14:41, Alexander Duyck wrote:
> On Tue, Oct 17, 2023 at 1:17 PM Jakub Kicinski <kuba@...nel.org> wrote:
>>
>> On Tue, 17 Oct 2023 11:37:52 -0700 Alexander Duyck wrote:
>>>> 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.
>>>
>>> I agree that the change to the algorithm doesn't necessarily have
>>> anything to do with toeplitz, however it is still a change to the
>>> algorithm by performing the extra XOR on the inputs prior to
>>> processing. That is why I figured it might make sense to just add a
>>> new hfunc value that would mean toeplitz w/ symmetric XOR.
>>
>> XOR is just one form of achieving symmetric hashing, sorting is another.
> 
> Right, but there are huge algorithmic differences between the two.
> With sorting you don't lose any entropy, whereas with XOR you do. For
> example one side effect of XOR is that for every two hosts on the same
> IP subnet the IP subnets will cancel out. As such with the same key
> 192.168.0.1->192.168.0.2 will hash out essentially the same as
> fc::1->fc::2.

I agree of course that we lose entropy by XORing, but don't we also lose 
entropy, for example, if we hash only the L4 dst_port vs (ip_src, 
ip_dst, l4_src, l4_dst,..etc)? we still say we are using the same alg.


>>>> 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 :(
>>>
>>> Why add an extra reserved field when this is just a variant on a hash
>>> function? I view it as not being dissimilar to how we handle TSO or
>>> tx-checksumming. It would make sense to me to just set something like
>>> toeplitz-symmetric-xor to on in order to turn this on.
>>
>> It's entirely orthogonal. {sym-XOR, sym-sort} x {toep, crc, xor} -
>> all combinations can work.
>>
>> Forget the "is it algo or not algo" question, just purely from data
>> normalization perspective, in terms of the API, if combinations make
>> sense they should be controllable independently.
>>
>> https://en.wikipedia.org/wiki/First_normal_form
> 
> I am thinking of this from a software engineering perspective. This
> symmetric-xor aka simplified-toeplitz is actually much cheaper to
> implement in software than the original. As such I would want it to be
> considered a separate algorithm as I could make use of something like
> that when having to implement RSS in QEMU for instance. Based on
> earlier comments it doesn't change the inputs, it just changes how I
> have to handle the data and the key. It starts reducing things down to
> something like the Intel implementation of Flow Director in terms of
> how the key gets generated and hashed.

The key is independent of all of this discussion. It is set by the user 
and whatever that key is, the hardware (after properly configuring what 
fields are XOR'd) will generate the symmetric hash from the input data. 
The "alg" does not handle or manipulate the key.

Powered by blists - more mailing lists