netdev - Re: [RFC PATCH net-next 03/10] ipv4: Add custom multipath hash policy

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 May 2021 11:45:00 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        petrm@...dia.com, roopa@...dia.com, nikolay@...dia.com,
        ssuryaextr@...il.com, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH net-next 03/10] ipv4: Add custom multipath hash policy

On Mon, May 03, 2021 at 08:40:18PM -0600, David Ahern wrote:
> On 5/3/21 8:38 PM, David Ahern wrote:
> > On 5/2/21 10:22 AM, Ido Schimmel wrote:
> >> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> >> index 8ab61f4edf02..549601494694 100644
> >> --- a/Documentation/networking/ip-sysctl.rst
> >> +++ b/Documentation/networking/ip-sysctl.rst
> >> @@ -99,6 +99,8 @@ fib_multipath_hash_policy - INTEGER
> >>  	- 0 - Layer 3
> >>  	- 1 - Layer 4
> >>  	- 2 - Layer 3 or inner Layer 3 if present
> >> +	- 3 - Custom multipath hash. Fields used for multipath hash calculation
> >> +	  are determined by fib_multipath_hash_fields sysctl
> >>  
> > 
> > If you default the fields to L3, then seems like the custom option is a
> > more generic case of '0'.
> > 
> 
> Actually this is the more generic case of all of them, so all of them
> could be implemented using this custom field selection.

Yes and no. The default policy (0) special cases ICMP packets whereas
the rest of the policies do not. Therefore, entirely ignoring the chosen
policy and only looking at the hash fields is not possible here.

Policy 1 can be implemented based on the hash fields alone, as it only
considers outer fields.

Policy 2 is a bit quirky. It will consider inner layer 3 fields and
fallback to outer fields if no encapsulation was encountered. It is not
possible to implement it as-is based on hash fields alone. A good
approximation would be to hash based on both outer and inner layer 3
fields, as the outer fields should remain constant throughout the
lifetime of a given encapsulated flow.

However, while some code can be reused between the different policies, I
do not think it is the primary consideration here. With the new policy,
I tried to minimize the amount of checks as much as I could, but at the
worst case it still adds a per-field check. Avoiding this penalty for
existing use cases was my primary motivation. Especially when the
multipath code can be called from XDP via bpf_fib_lookup() helper.