On 2023-08-24 18:43, Jakub Kicinski wrote:

On Thu, 24 Aug 2023 16:55:40 -0600 Ahmed Zaki wrote:

quoted

When "Symmetric Toeplitz" is set in the NIC, the H/W will yield the same
hash as the regular Toeplitz for protocol types that do not have such
symmetric fields in both directions (i.e. there will be no RSS hash
symmetry and the TX/RX traffic will land on different Rx queues).

The goal of this series is to enable the "default" behavior of the whole
device ("-X hfunc") to be the symmetric hash (again, only for protocols
that have symmetric src/dst counterparts). If I understand the first
option correctly, the user would need to manually configure all RXH
fields for all flow types (tcp4, udp4, sctp4, tcp6, ..etc), to get
symmetric RSS on them, instead of the proposed single "-X" command?
The second option is closer to what I had in mind. We can re-name and
provide any details.

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:

ethtool -X eth0 --symmetric hfunc toeplitz

This will also allow drivers/NICs to implement this as they wish (XOR, 
sorted, ..etc). Better ?

quoted

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

length(src_ip , dst_ip, src_port, dst_port)  = length(src_ip ^ dst_ip , 
src_ip ^ dst_ip, src_port ^ dst_port, src_port ^ dst_port)