Re: [PATCH v2 03/10] tcp: Make static buffers stack-local for thread safety

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

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

On Fri, Jul 31, 2026 at 06:23:22PM +0200, Laurent Vivier wrote:
> Static buffers shared across all call sites are not safe when multiple
> worker threads handle TCP connections concurrently.
> 
> In tcp.c, move tcp_iov[] from file scope into tcp_data_from_tap() where
> it is exclusively used.  At UIO_MAXIOV (1024) entries of struct iovec
> (16 bytes each), this adds 16 KiB to the stack frame.
> 
> In tcp_vu.c, move iov_vu[], elem[], and frame[] from file scope into
> tcp_vu_data_from_sock() and pass them to tcp_vu_sock_recv() as
> parameters.  Also make iov_msg[] in tcp_vu_sock_recv() a local variable
> instead of static, as it is only used within a single call.  Combined,
> these add roughly 80 KiB across the nested stack frames, which is
> acceptable for per-thread stacks.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

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

> ---
>  tcp.c    |  3 +--
>  tcp_vu.c | 33 ++++++++++++++++++++-------------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index e80a64e38f24..ee4ec599e055 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -435,8 +435,6 @@ static socklen_t tcp_info_size;
>  /* Kernel reports delivery rate in TCP_INFO (kernel commit eb8329e0a04d) */
>  #define delivery_rate_cap	tcp_info_cap(delivery_rate)
>  
> -/* sendmsg() to socket */
> -static struct iovec	tcp_iov			[UIO_MAXIOV];
>  
>  /* Pools for pre-opened sockets (in init) */
>  int init_sock_pool4		[TCP_SOCK_POOL_SIZE];
> @@ -1945,6 +1943,7 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>  	uint16_t max_ack_seq_wnd = conn->wnd_from_tap;
>  	uint32_t max_ack_seq = conn->seq_ack_from_tap;
>  	uint32_t seq_from_tap = conn->seq_from_tap;
> +	struct iovec tcp_iov[UIO_MAXIOV];
>  	struct msghdr mh = { .msg_iov = tcp_iov };
>  	size_t len;
>  	ssize_t n;
> diff --git a/tcp_vu.c b/tcp_vu.c
> index c9a1e08747b5..b4baecc1f717 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -35,9 +35,6 @@
>  #include "vu_common.h"
>  #include <time.h>
>  
> -static struct iovec iov_vu[VIRTQUEUE_MAX_SIZE];
> -static struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
> -
>  /**
>   * struct vu_frame - Descriptor for a TCP frame mapped to virtqueue elements
>   * @idx_element:	Index of first element in elem[] for this frame
> @@ -46,13 +43,13 @@ static struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
>   * @num_iovec:		Number of iovecs covering this frame's buffers
>   * @size:		Total frame size including all headers
>   */
> -static struct vu_frame {
> +struct vu_frame {
>  	int idx_element;
>  	int num_element;
>  	int idx_iovec;
>  	int num_iovec;
>  	size_t size;
> -} frame[VIRTQUEUE_MAX_SIZE];
> +};
>  
>  /**
>   * tcp_vu_hdrlen() - Sum size of all headers, from TCP to virtio-net
> @@ -225,6 +222,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags,
>   * @v6:			Set for IPv6 connections
>   * @already_sent:	Number of bytes already sent
>   * @fillsize:		Maximum bytes to fill in guest-side receiving window
> + * @iov_vu:		IO vector array for virtqueue buffers
> + * @elem:		Virtqueue element array
> + * @frame:		Frame descriptor array
>   * @elem_used:		number of element (output)
>   * @frame_cnt:		Pointer to store the number of frames (output)
>   *
> @@ -234,9 +234,12 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags,
>  static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>  				const struct tcp_tap_conn *conn, bool v6,
>  				uint32_t already_sent, size_t fillsize,
> +				struct iovec *iov_vu,
> +				struct vu_virtq_element *elem,
> +				struct vu_frame *frame,
>  				int *elem_used, int *frame_cnt)
>  {
> -	static struct iovec iov_msg[VIRTQUEUE_MAX_SIZE + DISCARD_IOV_NUM];
> +	struct iovec iov_msg[VIRTQUEUE_MAX_SIZE + DISCARD_IOV_NUM];
>  	const struct vu_dev *vdev = c->vdev;
>  	struct msghdr mh_sock = { 0 };
>  	uint16_t mss = MSS_GET(conn);
> @@ -253,16 +256,16 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>  	iov_used = 0;
>  	elem_cnt = 0;
>  	*frame_cnt = 0;
> -	while (fillsize > 0 && elem_cnt < ARRAY_SIZE(elem) &&
> -	       iov_used < ARRAY_SIZE(iov_vu) &&
> -	       *frame_cnt < ARRAY_SIZE(frame)) {
> +	while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE &&
> +	       iov_used < VIRTQUEUE_MAX_SIZE &&
> +	       *frame_cnt < VIRTQUEUE_MAX_SIZE) {
>  		size_t frame_size, in_total;
>  		int cnt;
>  
>  		cnt = vu_collect(vdev, vq, &elem[elem_cnt],
> -				 ARRAY_SIZE(elem) - elem_cnt,
> +				 VIRTQUEUE_MAX_SIZE - elem_cnt,
>  				 &iov_vu[iov_used],
> -				 ARRAY_SIZE(iov_vu) - iov_used, &in_total,
> +				 VIRTQUEUE_MAX_SIZE - iov_used, &in_total,
>  				 MIN(mss, fillsize) + hdrlen,
>  				 &frame_size);
>  		if (cnt == 0)
> @@ -328,7 +331,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>  		if ((size_t)ret <= f->size - hdrlen) {
>  			unsigned cnt;
>  
> -			cnt = iov_skip_bytes(&iov_vu[f->idx_iovec], f->num_iovec,
> +			cnt = iov_skip_bytes(&iov_vu[f->idx_iovec],
> +					     f->num_iovec,
>  					     MAX(hdrlen + ret, VNET_HLEN + ETH_ZLEN),
>  					     NULL);
>  			if (cnt < (unsigned)f->num_iovec)
> @@ -437,6 +441,9 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
>  			  uint32_t already_sent, const struct timespec *now,
>  			  unsigned int qpair)
>  {
> +	struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
> +	struct iovec iov_vu[VIRTQUEUE_MAX_SIZE];
> +	struct vu_frame frame[VIRTQUEUE_MAX_SIZE];
>  	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
>  	int toguest = QPAIR_TOGUEST_QUEUE(qpair);
>  	struct vu_dev *vdev = c->vdev;
> @@ -460,7 +467,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
>  	 * data from the socket
>  	 */
>  	len = tcp_vu_sock_recv(c, vq, conn, v6, already_sent, fillsize,
> -			       &elem_cnt, &frame_cnt);
> +			       iov_vu, elem, frame, &elem_cnt, &frame_cnt);
>  	if (len < 0) {
>  		if (len != -EAGAIN && len != -EWOULDBLOCK) {
>  			tcp_rst(c, conn, now, qpair);
> -- 
> 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 --]