netdev - Re: [RFC PATCH net-next 2/4] txhash: Add socket option to control TX hash rethink behavior

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:   Fri, 29 Oct 2021 13:01:29 +0300
From:   Akhmat Karakotov <hmukos@...dex-team.ru>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     netdev@...r.kernel.org, tom@...bertland.com,
        mitradir@...dex-team.ru, zeil@...dex-team.ru
Subject: Re: [RFC PATCH net-next 2/4] txhash: Add socket option to control TX
 hash rethink behavior


> On Oct 26, 2021, at 00:05, Eric Dumazet <eric.dumazet@...il.com> wrote:
> 
> 
> 
> On 10/25/21 1:35 PM, Akhmat Karakotov wrote:
>> Add the SO_TXREHASH socket option to control hash rethink behavior per socket.
>> When default mode is set, sockets disable rehash at initialization and use
>> sysctl option when entering listen state. setsockopt() overrides default
>> behavior.
> 
> What values are accepted, and what are their meaning ?
> 
> It seems weird to do anything in inet_csk_listen_start()
> 
> 
> For sockets that have not used SO_TXREHASH
> (this includes passive sockets where their parent did not use SO_TXREHASH),
> the sysctl _current_ value should be used every time we consider a rehash.
SO_TXREHASH_DEFAULT value means default behavior: for listening sockets
the sysctl value is taken, while for others rehash is disabled. The motivation
was to disallow hash rethink at the client-side to avoid connection timeout to
anycast services (as was stated in cover letter).
ENABLED and DISABLED values are for enforcing rehash option values.

To be honest I didn't quite understand how you propose to change patch behaviour.

> 
>> #define SOCK_TXREHASH_DISABLED	0
>> #define SOCK_TXREHASH_ENABLED	1
>> 
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index 0d833b861f00..537a8532ff8a 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -360,7 +360,7 @@ static int __net_init net_defaults_init_net(struct net *net)
>> {
>> 	net->core.sysctl_somaxconn = SOMAXCONN;
>> 
>> -	net->core.sysctl_txrehash = SOCK_TXREHASH_DISABLED;
>> +	net->core.sysctl_txrehash = SOCK_TXREHASH_ENABLED;
> 
> This is very confusing.
> 

Could you, please, elaborate what exactly is confusing?