Re: [PATCH 7/8] tcp: Protect init socket pools with mutex for thread safety

[David Gibson <david@gibson.dropbear.id.au>]

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

On Tue, Jun 16, 2026 at 07:10:51PM +0200, Laurent Vivier wrote:
> The pre-opened socket pools init_sock_pool4/6 are consumed by
> tcp_conn_pool_sock() when creating new connections from any worker
> thread, and refilled by tcp_sock_refill_pool() from tcp_timer() in
> post_handler(). These can run concurrently on different threads.
> 
> Add a mutex protecting both operations in tcp_conn_sock() and
> tcp_sock_refill_init(), where init namespace pools are accessed.

I wonder if we might be better off with a per-qpair/thread socket pool
instead of a mutexed common pool.  The obvious tradeoff would be that
the pool is smaller so a thread could run out of sockets while we
still have some available on another thread.  But, having a common
pool only gains us there if the mutex lock is significantly cheaper
than the system calls to refill the pool.  That might be the case, but
I'm not certain.

> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tcp.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 955012355d69..019340c1c348 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -293,6 +293,7 @@
>  #include <sys/uio.h>
>  #include <time.h>
>  #include <arpa/inet.h>
> +#include <pthread.h>
>  
>  #include <linux/sockios.h>
>  #include <linux/sock_diag.h>
> @@ -439,6 +440,7 @@ static socklen_t tcp_info_size;
>  /* Pools for pre-opened sockets (in init) */
>  int init_sock_pool4		[TCP_SOCK_POOL_SIZE];
>  int init_sock_pool6		[TCP_SOCK_POOL_SIZE];
> +static pthread_mutex_t sock_pool_lock = PTHREAD_MUTEX_INITIALIZER;
>  
>  /**
>   * conn_at_sidx() - Get TCP connection specific flow at given sidx
> @@ -1568,7 +1570,11 @@ int tcp_conn_sock(sa_family_t af)
>  	int *pool = af == AF_INET6 ? init_sock_pool6 : init_sock_pool4;
>  	int s;
>  
> -	if ((s = tcp_conn_pool_sock(pool)) >= 0)
> +	pthread_mutex_lock(&sock_pool_lock);
> +	s = tcp_conn_pool_sock(pool);
> +	pthread_mutex_unlock(&sock_pool_lock);
> +
> +	if (s >= 0)
>  		return s;
>  
>  	/* If the pool is empty we just open a new one without refilling the
> @@ -2857,6 +2863,7 @@ int tcp_sock_refill_pool(int pool[], sa_family_t af)
>   */
>  static void tcp_sock_refill_init(const struct ctx *c)
>  {
> +	pthread_mutex_lock(&sock_pool_lock);
>  	if (c->ifi4) {
>  		int rc = tcp_sock_refill_pool(init_sock_pool4, AF_INET);
>  		if (rc < 0)
> @@ -2869,6 +2876,7 @@ static void tcp_sock_refill_init(const struct ctx *c)
>  			warn("TCP: Error refilling IPv6 host socket pool: %s",
>  			     strerror_(-rc));
>  	}
> +	pthread_mutex_unlock(&sock_pool_lock);
>  }
>  
>  /**
> -- 
> 2.54.0
> 

-- 
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 --]