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

Message-ID: <e9ca04cd-ec21-43ad-fd99-11af754e3279@gmail.com>
Date:   Mon, 8 Nov 2021 11:39:31 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Akhmat Karakotov <hmukos@...dex-team.ru>, netdev@...r.kernel.org
Subject: Re: [RFC PATCH net-next 2/4] txhash: Add socket option to control TX
 hash rethink behavior



On 11/8/21 4:48 AM, Akhmat Karakotov wrote:
>> On Oct 29, 2021, at 13:01, Akhmat Karakotov <hmukos@...dex-team.ru> wrote:
>>
>>>
>>> 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?
> 
> Hi Eric,
> 
> I wonder if you have time to take a closer look at my last comments.
> 
> P. S.: resending this message without CC, because delivery system rejected it first time
> 

I guess the confusing part was that one patch sets SOCK_TXREHASH_DISABLED,
then a following one (in the series) sets it to SOCK_TXREHASH_ENABLED

My brain hurts.