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

On Oct 29, 2021, at 13:01, Akhmat Karakotov [off-list ref] wrote:

quoted

On Oct 26, 2021, at 00:05, Eric Dumazet [off-list ref] wrote:



On 10/25/21 1:35 PM, Akhmat Karakotov wrote:

quoted

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.

quoted

quoted

#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

On 11/8/21 4:48 AM, Akhmat Karakotov wrote:

quoted

On Oct 29, 2021, at 13:01, Akhmat Karakotov [off-list ref] wrote:

quoted

On Oct 26, 2021, at 00:05, Eric Dumazet [off-list ref] wrote:



On 10/25/21 1:35 PM, Akhmat Karakotov wrote:

quoted

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.

quoted

quoted

#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.