All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/5] Make hash rethink configurable
@ 2022-01-28 19:44 Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 1/5] txhash: Make rethinking txhash behavior configurable via sysctl Akhmat Karakotov
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

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.

v3:
	- Fixed bug with bad u8 comparison. Moved sk->txrehash to use less
	  bytes in struct. Added WRITE_ONCE() in setsockopt in and
	  READ_ONCE() in tcp_rtx_synack.

v4:
	- Rebase and add documentation for sysctl option.


Akhmat Karakotov (5):
  txhash: Make rethinking txhash behavior configurable via sysctl
  txhash: Add socket option to control TX hash rethink behavior
  txhash: Add txrehash sysctl description
  bpf: Add SO_TXREHASH setsockopt
  tcp: Change SYN ACK retransmit behaviour to account for rehash

 Documentation/admin-guide/sysctl/net.rst |  9 ++++++++
 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                          | 14 ++++++++++++
 net/core/sysctl_net_core.c               | 14 ++++++++++--
 net/ipv4/inet_connection_sock.c          |  3 +++
 net/ipv4/tcp_output.c                    |  4 +++-
 15 files changed, 83 insertions(+), 16 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 1/5] txhash: Make rethinking txhash behavior configurable via sysctl
  2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
@ 2022-01-28 19:44 ` Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior Akhmat Karakotov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

Add a per ns sysctl that controls the txhash rethink behavior:
net.core.txrehash. When enabled, the same behavior is retained,
when disabled, rethink is not performed. Sysctl is enabled by default.

Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 include/net/netns/core.h    |  1 +
 include/net/sock.h          | 34 +++++++++++++++++++++-------------
 include/uapi/linux/socket.h |  3 +++
 net/core/net_namespace.c    |  2 ++
 net/core/sysctl_net_core.c  | 14 ++++++++++++--
 5 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/include/net/netns/core.h b/include/net/netns/core.h
index 552bc25b1933..388244e315e7 100644
--- a/include/net/netns/core.h
+++ b/include/net/netns/core.h
@@ -10,6 +10,7 @@ struct netns_core {
 	struct ctl_table_header	*sysctl_hdr;
 
 	int	sysctl_somaxconn;
+	u8	sysctl_txrehash;
 
 #ifdef CONFIG_PROC_FS
 	struct prot_inuse __percpu *prot_inuse;
diff --git a/include/net/sock.h b/include/net/sock.h
index ff9b508d9c5f..0540e1b2aeb0 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -587,6 +587,18 @@ static inline bool sk_user_data_is_nocopy(const struct sock *sk)
 			   __tmp | SK_USER_DATA_NOCOPY);		\
 })
 
+static inline
+struct net *sock_net(const struct sock *sk)
+{
+	return read_pnet(&sk->sk_net);
+}
+
+static inline
+void sock_net_set(struct sock *sk, struct net *net)
+{
+	write_pnet(&sk->sk_net, net);
+}
+
 /*
  * SK_CAN_REUSE and SK_NO_REUSE on a socket mean that the socket is OK
  * or not whether his port will be reused by someone else. SK_FORCE_REUSE
@@ -2054,10 +2066,18 @@ static inline void sk_set_txhash(struct sock *sk)
 
 static inline bool sk_rethink_txhash(struct sock *sk)
 {
-	if (sk->sk_txhash) {
+	u8 rehash;
+
+	if (!sk->sk_txhash)
+		return false;
+
+	rehash = READ_ONCE(sock_net(sk)->core.sysctl_txrehash);
+
+	if (rehash) {
 		sk_set_txhash(sk);
 		return true;
 	}
+
 	return false;
 }
 
@@ -2704,18 +2724,6 @@ static inline void sk_eat_skb(struct sock *sk, struct sk_buff *skb)
 	__kfree_skb(skb);
 }
 
-static inline
-struct net *sock_net(const struct sock *sk)
-{
-	return read_pnet(&sk->sk_net);
-}
-
-static inline
-void sock_net_set(struct sock *sk, struct net *net)
-{
-	write_pnet(&sk->sk_net, net);
-}
-
 static inline bool
 skb_sk_is_prefetched(struct sk_buff *skb)
 {
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
index eb0a9a5b6e71..0accd6102ece 100644
--- a/include/uapi/linux/socket.h
+++ b/include/uapi/linux/socket.h
@@ -31,4 +31,7 @@ struct __kernel_sockaddr_storage {
 
 #define SOCK_BUF_LOCK_MASK (SOCK_SNDBUF_LOCK | SOCK_RCVBUF_LOCK)
 
+#define SOCK_TXREHASH_DISABLED	0
+#define SOCK_TXREHASH_ENABLED	1
+
 #endif /* _UAPI_LINUX_SOCKET_H */
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index c53d9aab38ab..8711350085d6 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -364,6 +364,8 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
 static int __net_init net_defaults_init_net(struct net *net)
 {
 	net->core.sysctl_somaxconn = SOMAXCONN;
+	net->core.sysctl_txrehash = SOCK_TXREHASH_ENABLED;
+
 	return 0;
 }
 
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 7b4d485aac7a..dbeb8ecbcd98 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -593,6 +593,15 @@ static struct ctl_table netns_core_table[] = {
 		.extra1		= SYSCTL_ZERO,
 		.proc_handler	= proc_dointvec_minmax
 	},
+	{
+		.procname	= "txrehash",
+		.data		= &init_net.core.sysctl_txrehash,
+		.maxlen		= sizeof(u8),
+		.mode		= 0644,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_ONE,
+		.proc_handler	= proc_dou8vec_minmax,
+	},
 	{ }
 };
 
@@ -611,7 +620,7 @@ __setup("fb_tunnels=", fb_tunnels_only_for_init_net_sysctl_setup);
 
 static __net_init int sysctl_core_net_init(struct net *net)
 {
-	struct ctl_table *tbl;
+	struct ctl_table *tbl, *tmp;
 
 	tbl = netns_core_table;
 	if (!net_eq(net, &init_net)) {
@@ -619,7 +628,8 @@ static __net_init int sysctl_core_net_init(struct net *net)
 		if (tbl == NULL)
 			goto err_dup;
 
-		tbl[0].data = &net->core.sysctl_somaxconn;
+		for (tmp = tbl; tmp->procname; tmp++)
+			tmp->data += (char *)net - (char *)&init_net;
 
 		/* Don't export any sysctls to unprivileged users */
 		if (net->user_ns != &init_user_ns) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior
  2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 1/5] txhash: Make rethinking txhash behavior configurable via sysctl Akhmat Karakotov
@ 2022-01-28 19:44 ` Akhmat Karakotov
  2022-01-28 22:05     ` kernel test robot
  2022-01-28 19:44 ` [PATCH net-next v4 3/5] txhash: Add txrehash sysctl description Akhmat Karakotov
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

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.

Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 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/sock.h                    | 12 +++---------
 include/uapi/asm-generic/socket.h     |  2 ++
 include/uapi/linux/socket.h           |  1 +
 net/core/sock.c                       | 13 +++++++++++++
 net/ipv4/inet_connection_sock.c       |  3 +++
 9 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index 284d28755b8d..7d81535893af 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -133,6 +133,8 @@
 
 #define SO_RESERVE_MEM		73
 
+#define SO_TXREHASH		74
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 24e0efb360f6..1d55e57b8466 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -144,6 +144,8 @@
 
 #define SO_RESERVE_MEM		73
 
+#define SO_TXREHASH		74
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 845ddc63c882..654061e0964e 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -125,6 +125,8 @@
 
 #define SO_RESERVE_MEM		0x4047
 
+#define SO_TXREHASH		0x4048
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 2672dd03faf3..666f81e617ea 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -126,6 +126,8 @@
 
 #define SO_RESERVE_MEM           0x0052
 
+#define SO_TXREHASH              0x0053
+
 
 #if !defined(__KERNEL__)
 
diff --git a/include/net/sock.h b/include/net/sock.h
index 0540e1b2aeb0..15cf639a5f23 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -316,6 +316,7 @@ struct sk_filter;
   *	@sk_rcvtimeo: %SO_RCVTIMEO setting
   *	@sk_sndtimeo: %SO_SNDTIMEO setting
   *	@sk_txhash: computed flow hash for use on transmit
+  *	@sk_txrehash: enable TX hash rethink
   *	@sk_filter: socket filtering instructions
   *	@sk_timer: sock cleanup timer
   *	@sk_stamp: time stamp of last packet received
@@ -493,6 +494,7 @@ struct sock {
 	kuid_t			sk_uid;
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	u8			sk_prefer_busy_poll;
+	u8			sk_txrehash;
 	u16			sk_busy_poll_budget;
 #endif
 	spinlock_t		sk_peer_lock;
@@ -2066,18 +2068,10 @@ static inline void sk_set_txhash(struct sock *sk)
 
 static inline bool sk_rethink_txhash(struct sock *sk)
 {
-	u8 rehash;
-
-	if (!sk->sk_txhash)
-		return false;
-
-	rehash = READ_ONCE(sock_net(sk)->core.sysctl_txrehash);
-
-	if (rehash) {
+	if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
 		sk_set_txhash(sk);
 		return true;
 	}
-
 	return false;
 }
 
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index c77a1313b3b0..467ca2f28760 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -128,6 +128,8 @@
 
 #define SO_RESERVE_MEM		73
 
+#define SO_TXREHASH		74
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
index 0accd6102ece..51d6bb2f6765 100644
--- a/include/uapi/linux/socket.h
+++ b/include/uapi/linux/socket.h
@@ -31,6 +31,7 @@ struct __kernel_sockaddr_storage {
 
 #define SOCK_BUF_LOCK_MASK (SOCK_SNDBUF_LOCK | SOCK_RCVBUF_LOCK)
 
+#define SOCK_TXREHASH_DEFAULT	((u8)-1)
 #define SOCK_TXREHASH_DISABLED	0
 #define SOCK_TXREHASH_ENABLED	1
 
diff --git a/net/core/sock.c b/net/core/sock.c
index cccf21f3618d..5e711b42898f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1447,6 +1447,14 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 		break;
 	}
 
+	case SO_TXREHASH:
+		if (val < -1 || val > 1) {
+			ret = -EINVAL;
+			break;
+		}
+		sk->sk_txrehash = (u8)val;
+		break;
+
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -1834,6 +1842,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = sk->sk_reserved_mem;
 		break;
 
+	case SO_TXREHASH:
+		v.val = sk->sk_txrehash;
+		break;
+
 	default:
 		/* We implement the SO_SNDLOWAT etc to not be settable
 		 * (1003.1g 7).
@@ -3279,6 +3291,7 @@ void sock_init_data(struct socket *sock, struct sock *sk)
 	sk->sk_pacing_rate = ~0UL;
 	WRITE_ONCE(sk->sk_pacing_shift, 10);
 	sk->sk_incoming_cpu = -1;
+	sk->sk_txrehash = SOCK_TXREHASH_DEFAULT;
 
 	sk_rx_queue_clear(sk);
 	/*
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fc2a985f6064..b81fb13fc5f4 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1046,6 +1046,9 @@ int inet_csk_listen_start(struct sock *sk)
 	sk->sk_ack_backlog = 0;
 	inet_csk_delack_init(sk);
 
+	if (sk->sk_txrehash == SOCK_TXREHASH_DEFAULT)
+		sk->sk_txrehash = READ_ONCE(sock_net(sk)->core.sysctl_txrehash);
+
 	/* There is race window here: we announce ourselves listening,
 	 * but this transition is still not validated by get_port().
 	 * It is OK, because this socket enters to hash table only
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 3/5] txhash: Add txrehash sysctl description
  2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 1/5] txhash: Make rethinking txhash behavior configurable via sysctl Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior Akhmat Karakotov
@ 2022-01-28 19:44 ` Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 4/5] bpf: Add SO_TXREHASH setsockopt Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 5/5] tcp: Change SYN ACK retransmit behaviour to account for rehash Akhmat Karakotov
  4 siblings, 0 replies; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

Update Documentation/admin-guide/sysctl/net.rst with txrehash usage
description.

Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
---
 Documentation/admin-guide/sysctl/net.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/admin-guide/sysctl/net.rst b/Documentation/admin-guide/sysctl/net.rst
index 4150f74c521a..f86b5e1623c6 100644
--- a/Documentation/admin-guide/sysctl/net.rst
+++ b/Documentation/admin-guide/sysctl/net.rst
@@ -365,6 +365,15 @@ new netns has been created.
 
 Default : 0  (for compatibility reasons)
 
+txrehash
+--------
+
+Controls default hash rethink behaviour on listening socket when SO_TXREHASH
+option is set to SOCK_TXREHASH_DEFAULT (i. e. not overridden by setsockopt).
+
+If set to 1 (default), hash rethink is performed on listening socket.
+If set to 0, hash rethink is not performed.
+
 2. /proc/sys/net/unix - Parameters for Unix domain sockets
 ----------------------------------------------------------
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 4/5] bpf: Add SO_TXREHASH setsockopt
  2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
                   ` (2 preceding siblings ...)
  2022-01-28 19:44 ` [PATCH net-next v4 3/5] txhash: Add txrehash sysctl description Akhmat Karakotov
@ 2022-01-28 19:44 ` Akhmat Karakotov
  2022-01-28 19:44 ` [PATCH net-next v4 5/5] tcp: Change SYN ACK retransmit behaviour to account for rehash Akhmat Karakotov
  4 siblings, 0 replies; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

Add bpf socket option to override rehash behaviour from userspace or from bpf.

Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 net/core/filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index a06931c27eeb..9615ae1ab530 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5091,6 +5091,13 @@ static int _bpf_setsockopt(struct sock *sk, int level, int optname,
 		case SO_REUSEPORT:
 			sk->sk_reuseport = valbool;
 			break;
+		case SO_TXREHASH:
+			if (val < -1 || val > 1) {
+				ret = -EINVAL;
+				break;
+			}
+			sk->sk_txrehash = (u8)val;
+			break;
 		default:
 			ret = -EINVAL;
 		}
@@ -5269,6 +5276,9 @@ static int _bpf_getsockopt(struct sock *sk, int level, int optname,
 		case SO_REUSEPORT:
 			*((int *)optval) = sk->sk_reuseport;
 			break;
+		case SO_TXREHASH:
+			*((int *)optval) = sk->sk_txrehash;
+			break;
 		default:
 			goto err_clear;
 		}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next v4 5/5] tcp: Change SYN ACK retransmit behaviour to account for rehash
  2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
                   ` (3 preceding siblings ...)
  2022-01-28 19:44 ` [PATCH net-next v4 4/5] bpf: Add SO_TXREHASH setsockopt Akhmat Karakotov
@ 2022-01-28 19:44 ` Akhmat Karakotov
  4 siblings, 0 replies; 8+ messages in thread
From: Akhmat Karakotov @ 2022-01-28 19:44 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, edumazet, eric.dumazet, bpf, ast, daniel, andrii,
	tom, hmukos, zeil, mitradir

Disabling rehash behavior did not affect SYN ACK retransmits because hash
was forcefully changed bypassing the sk_rethink_hash function. This patch
adds a condition which checks for rehash mode before resetting hash.

Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 net/core/sock.c       | 3 ++-
 net/ipv4/tcp_output.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 5e711b42898f..d6804685f17f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1452,7 +1452,8 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 			ret = -EINVAL;
 			break;
 		}
-		sk->sk_txrehash = (u8)val;
+		/* Paired with READ_ONCE() in tcp_rtx_synack() */
+		WRITE_ONCE(sk->sk_txrehash, (u8)val);
 		break;
 
 	default:
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 11c06b9db801..e76bf1e9251e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4092,7 +4092,9 @@ int tcp_rtx_synack(const struct sock *sk, struct request_sock *req)
 	struct flowi fl;
 	int res;
 
-	tcp_rsk(req)->txhash = net_tx_rndhash();
+	/* Paired with WRITE_ONCE() in sock_setsockopt() */
+	if (READ_ONCE(sk->sk_txrehash) == SOCK_TXREHASH_ENABLED)
+		tcp_rsk(req)->txhash = net_tx_rndhash();
 	res = af_ops->send_synack(sk, NULL, &fl, req, NULL, TCP_SYNACK_NORMAL,
 				  NULL);
 	if (!res) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior
  2022-01-28 19:44 ` [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior Akhmat Karakotov
@ 2022-01-28 22:05     ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-01-28 22:05 UTC (permalink / raw)
  To: Akhmat Karakotov, netdev
  Cc: kbuild-all, davem, kuba, edumazet, eric.dumazet, bpf, ast,
	daniel, andrii, tom

Hi Akhmat,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Akhmat-Karakotov/Make-hash-rethink-configurable/20220129-034621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git b76bbb34dc80258f5079b4067f0dae07b394b8fe
config: nds32-allnoconfig (https://download.01.org/0day-ci/archive/20220129/202201290514.oladOq5e-lkp@intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/92d1267cbe457eff1ea65aa32f38356861bfa5f5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Akhmat-Karakotov/Make-hash-rethink-configurable/20220129-034621
        git checkout 92d1267cbe457eff1ea65aa32f38356861bfa5f5
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/tcp.h:19,
                    from include/linux/ipv6.h:92,
                    from include/net/ipv6.h:12,
                    from include/linux/sunrpc/clnt.h:29,
                    from include/linux/nfs_fs.h:32,
                    from init/do_mounts.c:23:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
--
   In file included from fs/io_uring.c:63:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
   fs/io_uring.c: In function '__io_submit_flush_completions':
   fs/io_uring.c:2523:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
    2523 |         struct io_wq_work_node *node, *prev;
         |                                        ^~~~
--
   In file included from include/linux/tcp.h:19,
                    from include/linux/ipv6.h:92,
                    from include/net/addrconf.h:52,
                    from lib/vsprintf.c:40:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
   lib/vsprintf.c: In function 'va_format':
   lib/vsprintf.c:1684:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1684 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~


vim +2071 include/net/sock.h

  2068	
  2069	static inline bool sk_rethink_txhash(struct sock *sk)
  2070	{
> 2071		if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
  2072			sk_set_txhash(sk);
  2073			return true;
  2074		}
  2075		return false;
  2076	}
  2077	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior
@ 2022-01-28 22:05     ` kernel test robot
  0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2022-01-28 22:05 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4010 bytes --]

Hi Akhmat,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Akhmat-Karakotov/Make-hash-rethink-configurable/20220129-034621
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git b76bbb34dc80258f5079b4067f0dae07b394b8fe
config: nds32-allnoconfig (https://download.01.org/0day-ci/archive/20220129/202201290514.oladOq5e-lkp(a)intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/92d1267cbe457eff1ea65aa32f38356861bfa5f5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Akhmat-Karakotov/Make-hash-rethink-configurable/20220129-034621
        git checkout 92d1267cbe457eff1ea65aa32f38356861bfa5f5
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/tcp.h:19,
                    from include/linux/ipv6.h:92,
                    from include/net/ipv6.h:12,
                    from include/linux/sunrpc/clnt.h:29,
                    from include/linux/nfs_fs.h:32,
                    from init/do_mounts.c:23:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
--
   In file included from fs/io_uring.c:63:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
   fs/io_uring.c: In function '__io_submit_flush_completions':
   fs/io_uring.c:2523:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
    2523 |         struct io_wq_work_node *node, *prev;
         |                                        ^~~~
--
   In file included from include/linux/tcp.h:19,
                    from include/linux/ipv6.h:92,
                    from include/net/addrconf.h:52,
                    from lib/vsprintf.c:40:
   include/net/sock.h: In function 'sk_rethink_txhash':
>> include/net/sock.h:2071:34: error: 'struct sock' has no member named 'sk_txrehash'; did you mean 'sk_txhash'?
    2071 |         if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
         |                                  ^~~~~~~~~~~
         |                                  sk_txhash
   lib/vsprintf.c: In function 'va_format':
   lib/vsprintf.c:1684:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1684 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~


vim +2071 include/net/sock.h

  2068	
  2069	static inline bool sk_rethink_txhash(struct sock *sk)
  2070	{
> 2071		if (sk->sk_txhash && sk->sk_txrehash == SOCK_TXREHASH_ENABLED) {
  2072			sk_set_txhash(sk);
  2073			return true;
  2074		}
  2075		return false;
  2076	}
  2077	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-01-28 22:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 19:44 [PATCH net-next v4 0/5] Make hash rethink configurable Akhmat Karakotov
2022-01-28 19:44 ` [PATCH net-next v4 1/5] txhash: Make rethinking txhash behavior configurable via sysctl Akhmat Karakotov
2022-01-28 19:44 ` [PATCH net-next v4 2/5] txhash: Add socket option to control TX hash rethink behavior Akhmat Karakotov
2022-01-28 22:05   ` kernel test robot
2022-01-28 22:05     ` kernel test robot
2022-01-28 19:44 ` [PATCH net-next v4 3/5] txhash: Add txrehash sysctl description Akhmat Karakotov
2022-01-28 19:44 ` [PATCH net-next v4 4/5] bpf: Add SO_TXREHASH setsockopt Akhmat Karakotov
2022-01-28 19:44 ` [PATCH net-next v4 5/5] tcp: Change SYN ACK retransmit behaviour to account for rehash Akhmat Karakotov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.