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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YI5oA8XtW+DVE8xD@shredder>
Date:   Sun, 2 May 2021 11:51:15 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     Pavel Balaev <mail@...d.so>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
        David Ahern <dsahern@...nel.org>,
        Shuah Khan <shuah@...nel.org>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [PATCH v6 net-next 1/3] net/ipv4: multipath routing:
 configurable seed

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?