[PATCH] tcp: Buffer sizes are *not* inherited on accept()/accept4()

[PATCH] tcp: Buffer sizes are *not* inherited on accept()/accept4()

[Stefano Brivio]

...so it's pointless to set SO_RCVBUF and SO_SNDBUF on listening
sockets.

Call tcp_sock_set_bufsize() after accept4(), for inbound sockets.

As we didn't have large buffer sizes set for inbound sockets for
a long time (they are set explicitly only if the maximum size is
big enough, more than than the ~200 KiB default), I ran some more
throughput tests for this one, and I see slightly better numbers
(say, 17 gbps instead of 15 gbps guest to host without vhost-user).

Fixes: 904b86ade7db ("tcp: Rework window handling, timers, add SO_RCVLOWAT and pools for sockets/pipes")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tcp.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tcp.c b/tcp.c
index 3b3193a..a012b81 100644
--- a/tcp.c
+++ b/tcp.c
@@ -2057,6 +2057,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
 	if (s < 0)
 		goto cancel;
 
+	tcp_sock_set_bufsize(c, s);
+
 	/* FIXME: When listening port has a specific bound address, record that
 	 * as our address
 	 */
@@ -2260,7 +2262,6 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr,
 	if (s < 0)
 		return s;
 
-	tcp_sock_set_bufsize(c, s);
 	return s;
 }
 
@@ -2317,9 +2318,7 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port)
 
 	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback4,
 			NULL, port, tref.u32);
-	if (s >= 0)
-		tcp_sock_set_bufsize(c, s);
-	else
+	if (s < 0)
 		s = -1;
 
 	if (c->tcp.fwd_out.mode == FWD_AUTO)
@@ -2343,9 +2342,7 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
 
 	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback6,
 			NULL, port, tref.u32);
-	if (s >= 0)
-		tcp_sock_set_bufsize(c, s);
-	else
+	if (s < 0)
 		s = -1;
 
 	if (c->tcp.fwd_out.mode == FWD_AUTO)
-- 
@@ -2057,6 +2057,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
 	if (s < 0)
 		goto cancel;
 
+	tcp_sock_set_bufsize(c, s);
+
 	/* FIXME: When listening port has a specific bound address, record that
 	 * as our address
 	 */
@@ -2260,7 +2262,6 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr,
 	if (s < 0)
 		return s;
 
-	tcp_sock_set_bufsize(c, s);
 	return s;
 }
 
@@ -2317,9 +2318,7 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port)
 
 	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback4,
 			NULL, port, tref.u32);
-	if (s >= 0)
-		tcp_sock_set_bufsize(c, s);
-	else
+	if (s < 0)
 		s = -1;
 
 	if (c->tcp.fwd_out.mode == FWD_AUTO)
@@ -2343,9 +2342,7 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
 
 	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback6,
 			NULL, port, tref.u32);
-	if (s >= 0)
-		tcp_sock_set_bufsize(c, s);
-	else
+	if (s < 0)
 		s = -1;
 
 	if (c->tcp.fwd_out.mode == FWD_AUTO)
-- 
2.43.0

Re: [PATCH] tcp: Buffer sizes are *not* inherited on accept()/accept4()

[David Gibson]

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

On Mon, Jan 20, 2025 at 06:26:36PM +0100, Stefano Brivio wrote:
> ...so it's pointless to set SO_RCVBUF and SO_SNDBUF on listening
> sockets.
> 
> Call tcp_sock_set_bufsize() after accept4(), for inbound sockets.
> 
> As we didn't have large buffer sizes set for inbound sockets for
> a long time (they are set explicitly only if the maximum size is
> big enough, more than than the ~200 KiB default), I ran some more
> throughput tests for this one, and I see slightly better numbers
> (say, 17 gbps instead of 15 gbps guest to host without vhost-user).
> 
> Fixes: 904b86ade7db ("tcp: Rework window handling, timers, add SO_RCVLOWAT and pools for sockets/pipes")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 3b3193a..a012b81 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2057,6 +2057,8 @@ void tcp_listen_handler(const struct ctx *c, union epoll_ref ref,
>  	if (s < 0)
>  		goto cancel;
>  
> +	tcp_sock_set_bufsize(c, s);
> +
>  	/* FIXME: When listening port has a specific bound address, record that
>  	 * as our address
>  	 */
> @@ -2260,7 +2262,6 @@ static int tcp_sock_init_one(const struct ctx *c, const union inany_addr *addr,
>  	if (s < 0)
>  		return s;
>  
> -	tcp_sock_set_bufsize(c, s);
>  	return s;
>  }
>  
> @@ -2317,9 +2318,7 @@ static void tcp_ns_sock_init4(const struct ctx *c, in_port_t port)
>  
>  	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback4,
>  			NULL, port, tref.u32);
> -	if (s >= 0)
> -		tcp_sock_set_bufsize(c, s);
> -	else
> +	if (s < 0)
>  		s = -1;
>  
>  	if (c->tcp.fwd_out.mode == FWD_AUTO)
> @@ -2343,9 +2342,7 @@ static void tcp_ns_sock_init6(const struct ctx *c, in_port_t port)
>  
>  	s = pif_sock_l4(c, EPOLL_TYPE_TCP_LISTEN, PIF_SPLICE, &inany_loopback6,
>  			NULL, port, tref.u32);
> -	if (s >= 0)
> -		tcp_sock_set_bufsize(c, s);
> -	else
> +	if (s < 0)
>  		s = -1;
>  
>  	if (c->tcp.fwd_out.mode == FWD_AUTO)

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]