Re: [PATCH v6 net-next 1/3] net/ipv4: multipath routing: configurable seed

[Ido Schimmel]

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:
> > 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?