On Fri, Apr 30, 2021 at 02:36:49PM +0300, Pavel Balaev wrote:

On Thu, Apr 29, 2021 at 06:29:20PM +0300, Ido Schimmel wrote:

quoted

On Wed, Apr 28, 2021 at 03:31:33PM +0300, Pavel Balaev wrote:
This looks overly complex to me and I believe a lot of users will ask
themselves why they need to specify a seed using two hex numbers
separated by a comma. Looking at other implementations that already
allow specifying the seed, it is specified as a single integer.

32-bit in Cumulus:
https://docs.nvidia.com/networking-ethernet-software/cumulus-linux-43/Layer-3/Routing/Equal-Cost-Multipath-Load-Sharing-Hardware-ECMP/#configure-a-hash-seed-to-avoid-hash-polarization

Up to 16-bit in Arista:
https://eos.arista.com/hashing-for-l2-port-channels-and-l3-ecmp/

I believe you chose this interface because of the structure of the
SipHash key that is used for the multipath hash calculation. This is an
internal implementation detail and should not determine the user
interface.

Looking at the history of the code, the flow dissector was migrated to
SipHash in commit 55667441c84f ("net/flow_dissector: switch to
siphash"). The motivating use case was flow label generation since these
are sent on the wire together with the fields from which they were
computed, not multipath hash calculation that also happens to rely on
the flow dissector.

Given the above, do you see a problem with having the user specify a
32-bit number for the multipath hash seed? Note that SipHash is still
used and that the number can be used to fill the entire 128-bit space.

Do you mean take 32-bit number from user and multiply it like this:
u32 key = val;
u64 key64;
memset(&key64, val, sizeof(u32));
memset(&key64 + sizeof(u32), val, sizeof(u32));
memset(seed.key[0], &key64, sizeof(u64));
memset(seed.key[1], &key64, sizeof(u64));
?

Something like that, yes. It's still only 32-bit of user input, but it
can't hurt. Do you see a need to specify more than 32 bits for multipath
hash seed when the purpose is to force the same seed on multiple
machines?