Re: [PATCH v2 07/10] tcp: Protect init socket pools with mutex for thread safety

On Fri, Jul 31, 2026 at 06:23:26PM +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'm guessing you're going with a mutex rather than a per-thread socket pool for simplicity? That might be the right choice, but I do wonder a bit about it. The pool exists to avoid the latency of creating a new socket for a new connection. If the latency of taking the lock exceeds that of creating a socket, there's no longer any point to the pool. In the unconstest case, that's almost certainly not the case - a happy path futex() lock should be be much faster than a syscall. If the lock _is_ contested, I suspect opening a socket directly might win - at least for host ns sockets. For guest ns sockets, the latency is higher because we need vfork()/setns()/etc. Then again... taking a socket from the pool also involves writing the pool, which potentially means a cacheline pingpong. That might incur a pretty substantial latency. In theory we could trylock() and open a socket directly if we don't get the lock immediately, but at that point it would probably be simpler to have per-thread socket pools anyway. I guess, since there is no meaningful shared state in the socket pool, a per-thread pool seems like the more natural approach to me.

Signed-off-by: Laurent Vivier --- tcp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tcp.c b/tcp.c index ab7cbfa5de83..aef689faf031 100644 --- a/tcp.c +++ b/tcp.c @@ -293,6 +293,7 @@ #include #include #include +#include

#include #include @@ -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 @@ -1581,7 +1583,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 @@ -2858,6 +2864,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) @@ -2870,6 +2877,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