Re: [PATCH 3/8] tcp: Make static buffers stack-local for thread safety

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

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

On Tue, Jun 16, 2026 at 07:10:47PM +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>

Regardless of the thread implications, I think this is nicer than the
global just used by one function.  I'd be happy to see this applied
independent of the rest of the series.

Some tiny nits noted below, though:

> ---
>  tcp.c    |  3 +--
>  tcp_vu.c | 33 ++++++++++++++++++++-------------
>  2 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 1549e14adaf4..f4fe866ba7c3 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];
> @@ -1900,6 +1898,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 4f76f599156f..9270ece43d17 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
> @@ -224,6 +221,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)
>   *
> @@ -233,9 +233,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);
> @@ -252,16 +255,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)
> @@ -327,7 +330,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,

Nit: Unrelated whitespace change

>  					     MAX(hdrlen + ret, VNET_HLEN + ETH_ZLEN),
>  					     NULL);
>  			if (cnt < (unsigned)f->num_iovec)
> @@ -433,6 +437,9 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>  int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
>  			  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];

These aren't ordered according to our reverse-christmas tree convention.

>  	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
>  	int rx_queue = QPAIR_TOGUEST_QUEUE(qpair);
>  	struct vu_dev *vdev = c->vdev;
> @@ -477,7 +484,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, 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 --]