Re: [RFC PATCH net-next 1/3] net: ethtool: add symmetric Toeplitz RSS hash function

[Ahmed Zaki <ahmed.zaki@intel.com>]

On 2023-08-25 18:49, Jakub Kicinski wrote:
> On Fri, 25 Aug 2023 14:46:42 -0600 Ahmed Zaki wrote:
>>> I'm just trying to help, if you want a single knob you'd need to add
>>> new fields to the API and the RXFH API is not netlink-ified.
>>>
>>> Using hashing algo for configuring fields feels like a dirty hack.
>> Ok. Another way to add a single knob is to a flag in "struct
>> ethtool_rxfh" (there are still some reserved bytes) and then:
> Sorry we do have ETHTOOL_MSG_RSS_GET. It just doesn't cover the flow
> config now. But you can add the new field there without a problem.
>
>> ethtool -X eth0 --symmetric hfunc toeplitz
>>
>> This will also allow drivers/NICs to implement this as they wish (XOR,
>> sorted, ..etc). Better ?
> We should specify the fields, I reckon, something like:
>
> ethtool -X eth0 --symmetric sdfn hfunc toeplitz
>
> So that the driver can make sure the user expects symmetry on fields
> the device supports.

Seems fair. I will prepare this and the per-flow based config code 
("-U|-N") and re-send.


>
>>>> I agree that we will need to take care of some cases like if the user
>>>> removes only "source IP" or "destination port" from the hash fields,
>>>> without that field's counterpart (we can prevent this, or show a
>>>> warning, ..etc). I was planning to address that in a follow-up
>>>> series; ie. handling the "ethtool -U rx-flow-hash". Do you want that
>>>> to be included in the same series as well?
>>> Yes, the validation needs to be part of the same series. But the
>>> semantics of selecting only src or dst need to be established, too.
>>> You said you feed dst ^ src into the hashing twice - why?
>> To maintain the same input length (same as the regular Toeplitz input)
>> to the hash H/W block
> But that's a choice, right? We're configuring the input we could as
> well choose to make it shorter? v4 and v6 use the same key with
> different input lengths, right?

Correct. All RSS fields' offsets and lengths are configurable. The 
example I gave before was from the datasheet, but it seems we can feed 
the Xored values once.

Thanks,