netdev - Re: [PATCH] net: 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: <20210409114847.02435bb4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 9 Apr 2021 11:50:01 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Balaev Pavel <balaevpa@...otecs.ru>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] net: multipath routing: configurable seed

On Fri, 9 Apr 2021 16:52:05 +0300 Balaev Pavel wrote:
> Hello, this patch adds ability for user to set seed value for

nit: please drop the 'Hello' and use imperative form to describe 
the commit.

> multipath routing hashes. Now kernel uses random seed value:
> this is done to prevent hash-flooding DoS attacks,
> but it breaks some scenario, f.e:
> 
> +-------+        +------+        +--------+
> |       |-eth0---| FW0  |---eth0-|        |
> |       |        +------+        |        |
> |  GW0  |ECMP                ECMP|  GW1   |
> |       |        +------+        |        |
> |       |-eth1---| FW1  |---eth1-|        |
> +-------+        +------+        +--------+
> 
> In this scenario two ECMP routers used as traffic balancers between
> two firewalls. So if return path of one flow will not be the same,
> such flow will be dropped, because keep-state rules was created on
> other firewall.
> 
> This patch add sysctl variable: net.ipv4.fib_multipath_hash_seed.
> User can set the same seed value on GW0 and GW1 and traffic will
> be mirror-balanced. By default random value is used.
> 
> Signed-off-by: Balaev Pavel <balaevpa@...otecs.ru>

Please try to find relevant reviewers and put them on CC.
Try to find people who have worked on this code in the past.

This patch seems to add new sparse warnings:

net/ipv4/sysctl_net_ipv4.c:544:38: warning: incorrect type in assignment (different base types)
net/ipv4/sysctl_net_ipv4.c:544:38:    expected unsigned long long
net/ipv4/sysctl_net_ipv4.c:544:38:    got restricted __le64
net/ipv4/sysctl_net_ipv4.c:545:38: warning: incorrect type in assignment (different base types)
net/ipv4/sysctl_net_ipv4.c:545:38:    expected unsigned long long
net/ipv4/sysctl_net_ipv4.c:545:38:    got restricted __le64

> {
> 	u32 multipath_hash = fl4 ? fl4->flowi4_multipath_hash : 0;
> 	struct flow_keys hash_keys;
> +	struct multipath_seed_ctx *seed_ctx;

Please order variable declaration lines longest to shortest.