From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202606 header.b=p+z4/upf; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 56E4D5A0269 for ; Thu, 02 Jul 2026 04:37:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1782959865; bh=BqQ83eKIbUe1G/HsoppeQpUnPNKhW+8zHorFRKL1ciM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=p+z4/upfmSjzetaxHN1GBYJfcErEkfsk8yXhnahYVtao+nrAxfIvPmoM7cXKRusOY iJgbGHYnwuq5EyNJ7NjMAsxxaiONn5r4DOnUWIR1JVoHhXqHrdFN7ne85eQxbdlKLD 4qoYU9j+6GJKyZbJKP1lmS0UJOwXsnKLUHfFQGacWjqcYXPuLnFdMNWSYLa5xEeDlz DMa4D7vjsjfRgsVGhALc0ZWkOCrLE/rmuiB3W2F7UMeQssom7GdJN9SLNljsDRp0NC Xs2uCAvyEpG+UkSWzx7l87/GDs5GZTMInBh4jjs7wLtPT4eXUyHgCT6eanQVg7IUnk D+2z5MeRsXDcg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4grLdj6nqqz58mg; Thu, 02 Jul 2026 12:37:45 +1000 (AEST) Date: Thu, 2 Jul 2026 12:32:50 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 3/8] tcp: Make static buffers stack-local for thread safety Message-ID: References: <20260616171052.3785909-1-lvivier@redhat.com> <20260616171052.3785909-4-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="IR8yDnUOrtXDsZWG" Content-Disposition: inline In-Reply-To: <20260616171052.3785909-4-lvivier@redhat.com> Message-ID-Hash: AZSXNY3AAJQFTXDU4TEB7N7MKG4KB53E X-Message-ID-Hash: AZSXNY3AAJQFTXDU4TEB7N7MKG4KB53E X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --IR8yDnUOrtXDsZWG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > 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. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson 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(-) >=20 > 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) > =20 > -/* sendmsg() to socket */ > -static struct iovec tcp_iov [UIO_MAXIOV]; > =20 > /* 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, s= truct tcp_tap_conn *conn, > uint16_t max_ack_seq_wnd =3D conn->wnd_from_tap; > uint32_t max_ack_seq =3D conn->seq_ack_from_tap; > uint32_t seq_from_tap =3D conn->seq_from_tap; > + struct iovec tcp_iov[UIO_MAXIOV]; > struct msghdr mh =3D { .msg_iov =3D 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 > =20 > -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 elem= ents > * @idx_element: Index of first element in elem[] for this frame > @@ -46,13 +43,13 @@ static struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZ= E]; > * @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]; > +}; > =20 > /** > * 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 =3D c->vdev; > struct msghdr mh_sock =3D { 0 }; > uint16_t mss =3D MSS_GET(conn); > @@ -252,16 +255,16 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c= , struct vu_virtq *vq, > iov_used =3D 0; > elem_cnt =3D 0; > *frame_cnt =3D 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; > =20 > cnt =3D 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 =3D=3D 0) > @@ -327,7 +330,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, = struct vu_virtq *vq, > if ((size_t)ret <=3D f->size - hdrlen) { > unsigned cnt; > =20 > - cnt =3D iov_skip_bytes(&iov_vu[f->idx_iovec], f->num_iovec, > + cnt =3D 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, struc= t 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 =3D conn->wnd_from_tap << conn->ws_from_tap; > int rx_queue =3D QPAIR_TOGUEST_QUEUE(qpair); > struct vu_dev *vdev =3D 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 =3D 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 !=3D -EAGAIN && len !=3D -EWOULDBLOCK) { > tcp_rst(c, conn, qpair); > --=20 > 2.54.0 >=20 --=20 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 --IR8yDnUOrtXDsZWG Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmpFzccACgkQzQJF27ox 2GeZOQ/7Bs/Qg4Tp47oWTuDnO0VcZfTJKiKxEpU9YTuVkfUU1fk31L3ksNdWNThc Yd+cKmX8menbAGJ3q1h42das/JBd9CnuxrxSut6XYCwlV6d9W99jKfu10vualg0w p7r5GhptlH/gNH599Gg/HpaH55XnOdUV/AJ8/hdUmVn/jHzKnB08ZuwpbxBmaR+r kpTf8FmEzxnAYs8cQASnS8YXyoiiTdf4xNSBQ58ltsTMPIhu1OCkplTJmH1t+Hqr 3zmrfDr06X600grLm94jFFoIc6JIyamuTmBEjo6SgJp3lAAkkPqTyYsbQpZpK7gs l0SR6fhGXEa4GOwLKzophe3IwnjgR9i/EPneB1j6NFcNqEDZxZU6F2yncZxc4Id3 O6dIjC1FMnLH4W21TDHaJjDRQhgLnL4vYK7su6Otaj26ZR3usi24e101Q1r35ROJ iTxQXYeqsEwZoBFQJr3ozgsaFz2NSx2BbZdp/ZpTk9R9NEHJLCwzJY+Tjvg2XCYR 7UFBqckwSYHbe+4u5HKaIfKbP9XpDVnhsAqOeweY0WjAtJdhG48jJgADoInPbuB+ UkXBHZHu6gPVVHKzlbMubra2l2T7C8tRqwrP+i0Q1SIIw5JGjSFv647vm0NvgRGt xLTlXbT/VSawNFuyW9j4pLCrV+GNAk9GokH68mgluXHVZ6AcbW0= =qDSp -----END PGP SIGNATURE----- --IR8yDnUOrtXDsZWG--