netdev - Re: [RFC PATCH net-next 02/10] ipv4: Add a sysctl to control multipath hash fields

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 10:48:34 +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 02/10] ipv4: Add a sysctl to control
 multipath hash fields

On Mon, May 03, 2021 at 08:33:40PM -0600, David Ahern wrote:
> On 5/2/21 10:22 AM, Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@...dia.com>
> > 
> > A subsequent patch will add a new multipath hash policy where the packet
> > fields used for multipath hash calculation are determined by user space.
> > This patch adds a sysctl that allows user space to set these fields.
> > 
> > The packet fields are represented using a bitmap and are common between
> > IPv4 and IPv6 to allow user space to use the same numbering across both
> > protocols. For example, to hash based on standard 5-tuple:
> > 
> >  # sysctl -w net.ipv4.fib_multipath_hash_fields=0-2,4-5
> >  net.ipv4.fib_multipath_hash_fields = 0-2,4-5
> > 
> > More fields can be added in the future, if needed.
> > 
> > The 'need_outer' and 'need_inner' variables are set in the control path
> > to indicate whether dissection of the outer or inner flow is needed.
> > They will be used by a subsequent patch to allow the data path to avoid
> > dissection of the outer or inner flow when not needed.
> > 
> > Signed-off-by: Ido Schimmel <idosch@...dia.com>
> > ---
> >  Documentation/networking/ip-sysctl.rst | 29 ++++++++++++++++
> >  include/net/ip_fib.h                   | 46 ++++++++++++++++++++++++++
> >  include/net/netns/ipv4.h               |  4 +++
> >  net/ipv4/fib_frontend.c                | 24 ++++++++++++++
> >  net/ipv4/sysctl_net_ipv4.c             | 32 ++++++++++++++++++
> >  5 files changed, 135 insertions(+)
> > 
> > diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> > index c2ecc9894fd0..8ab61f4edf02 100644
> > --- a/Documentation/networking/ip-sysctl.rst
> > +++ b/Documentation/networking/ip-sysctl.rst
> > @@ -100,6 +100,35 @@ fib_multipath_hash_policy - INTEGER
> >  	- 1 - Layer 4
> >  	- 2 - Layer 3 or inner Layer 3 if present
> >  
> > +fib_multipath_hash_fields - list of comma separated ranges
> > +	When fib_multipath_hash_policy is set to 3 (custom multipath hash), the
> > +	fields used for multipath hash calculation are determined by this
> > +	sysctl.
> > +
> > +	The format used for both input and output is a comma separated list of
> > +	ranges (e.g., "0-2" for source IP, destination IP and IP protocol).
> > +	Writing to the file will clear all previous ranges and update the
> > +	current list with the input.
> > +
> > +	Possible fields are:
> > +
> > +	== ============================
> > +	 0 Source IP address
> > +	 1 Destination IP address
> > +	 2 IP protocol
> > +	 3 Unused
> > +	 4 Source port
> > +	 5 Destination port
> > +	 6 Inner source IP address
> > +	 7 Inner destination IP address
> > +	 8 Inner IP protocol
> > +	 9 Inner Flow Label
> > +	10 Inner source port
> > +	11 Inner destination port
> > +	== ============================
> > +
> > +	Default: 0-2 (source IP, destination IP and IP protocol)
> 
> since you are already requiring a name to id conversion, why not just
> use a bitmask here as the input? if the value is a 32-bit bitmask do you
> need bitmap_zalloc and its overhead?

A bitmask was what I originally planned to use, but a bitmap seemed to
provide a better user interface. In practice, it is probably not a big
deal given that most people will just put the relevant value in
/etc/sysctl.d/ and forget about it. Will try the bitmask option.

> Also, you could implement the current default using this scheme since
> you have the default fields as the current L3 policy.

Will reply in patch #3.