Re: [RFC PATCH v2 net-next 0/4] txhash: Make hash rethink configurable

From: Eric Dumazet <eric.dumazet@gmail.com>
To: Akhmat Karakotov <hmukos@yandex-team.ru>
Cc: Alexander Azimov <mitradir@yandex-team.ru>,
	netdev@vger.kernel.org, tom@herbertland.com, zeil@yandex-team.ru
Subject: Re: [RFC PATCH v2 net-next 0/4] txhash: Make hash rethink configurable
Date: Wed, 1 Dec 2021 08:49:31 -0800	[thread overview]
Message-ID: <5c7100d2-8327-1e5d-d04b-3db1bb86227a@gmail.com> (raw)
In-Reply-To: <82E7E49A-5EB9-4719-87FC-836718A031A9@yandex-team.ru>

On 11/30/21 1:58 AM, Akhmat Karakotov wrote:
> Hi Eric,
>
> I wonder if you have time to provide review regarding my last update to the patch.

Sorry, your patches did not reach me. Can you resend them, adding

"Eric Dumazet <edumazet@google.com>"  address to make sure I can catch 
them ?

Thanks !

>
>> On Nov 23, 2021, at 16:20, Akhmat Karakotov <hmukos@yandex-team.ru> wrote:
>>
>> Hi Eric,
>>
>> I've sent v2 of the patch. I've removed confusing part with sysctl default values
>> and made other changes according to your comments. I look forward for your
>> review.
>>
>>> On Nov 12, 2021, at 21:19, Akhmat Karakotov <hmukos@yandex-team.ru> wrote:
>>>
>>> As it was shown in the report by Alexander Azimov, hash rethink at the
>>> client-side may lead to connection timeout toward stateful anycast
>>> services. Tom Herbert created a patchset to address this issue by applying
>>> hash rethink only after a negative routing event (3RTOs) [1]. This change
>>> also affects server-side behavior, which we found undesirable. This
>>> patchset changes defaults in a way to make them safe: hash rethink at the
>>> client-side is disabled and enabled at the server-side upon each RTO
>>> event or in case of duplicate acknowledgments.
>>>
>>> This patchset provides two options to change default behaviour. The hash
>>> rethink may be disabled at the server-side by the new sysctl option.
>>> Changes in the sysctl option don't affect default behavior at the
>>> client-side.
>>>
>>> Hash rethink can also be enabled/disabled with socket option or bpf
>>> syscalls which ovewrite both default and sysctl settings. This socket
>>> option is available on both client and server-side. This should provide
>>> mechanics to enable hash rethink inside administrative domain, such as DC,
>>> where hash rethink at the client-side can be desirable.
>>>
>>> [1] https://lore.kernel.org/netdev/20210809185314.38187-1-tom@herbertland.com/
>>>
>>> v2:
>>> 	- Changed sysctl default to ENABLED in all patches. Reduced sysctl
>>> 	  and socket option size to u8. Fixed netns bug reported by kernel
>>> 	  test robot.
>>>
>>> Akhmat Karakotov (4):
>>> txhash: Make rethinking txhash behavior configurable via sysctl
>>> txhash: Add socket option to control TX hash rethink behavior
>>> bpf: Add SO_TXREHASH setsockopt
>>> tcp: change SYN ACK retransmit behaviour to account for rehash
>>>
>>> arch/alpha/include/uapi/asm/socket.h  |  2 ++
>>> arch/mips/include/uapi/asm/socket.h   |  2 ++
>>> arch/parisc/include/uapi/asm/socket.h |  2 ++
>>> arch/sparc/include/uapi/asm/socket.h  |  2 ++
>>> include/net/netns/core.h              |  1 +
>>> include/net/sock.h                    | 28 ++++++++++++++-------------
>>> include/uapi/asm-generic/socket.h     |  2 ++
>>> include/uapi/linux/socket.h           |  4 ++++
>>> net/core/filter.c                     | 10 ++++++++++
>>> net/core/net_namespace.c              |  2 ++
>>> net/core/sock.c                       | 13 +++++++++++++
>>> net/core/sysctl_net_core.c            | 15 ++++++++++++--
>>> net/ipv4/inet_connection_sock.c       |  3 +++
>>> net/ipv4/tcp_output.c                 |  3 ++-
>>> 14 files changed, 73 insertions(+), 16 deletions(-)
>>>
>>> -- 
>>> 2.17.1
>>>