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=202502 header.b=F5o4uYDT; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 124E65A061E for ; Thu, 20 Feb 2025 11:07:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1740046016; bh=3MEwnDt71MyjLccZCme6blcWlTzkLaxU9jiqMjzD2CA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=F5o4uYDTDYsdePhV8WIaC6+WsvjtRmYLEYPX/4GZX60gvu1T/AwJ0VKLhN9hjJGYy +BY/elUybe8Ap4VH1zS2Qfc8Lp4VjLinG4xqfg1Ao/YQu9Sti1PeJWnb+vYWeGEbK4 FKjyUUWXKIeN7UcBNSTAO74eSrWzJN2HkyES/xLiRcZIUSrZhEPepjNHvsDKjAhOKX G3k0kASnOE/dxUr8mmS9R4I3BIprozqQ3K6Aq/M/n0kmoXK7PlRTA7o94ugV93Mx3p DZMoh7zjiG+AcENXOxefWTuTtCYV0lsovBHaOBxVYbJ0p0/xzD+UtgeBd7MUJe0yIO M1aDiqYCDXmVg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Yz87N00vNz4x2c; Thu, 20 Feb 2025 21:06:55 +1100 (AEDT) Date: Thu, 20 Feb 2025 21:06:38 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH] tcp_vu: head_cnt need not be global Message-ID: References: <20250218025013.1035148-1-david@gibson.dropbear.id.au> <53e1b351-da19-4491-b438-1e77e0dc836c@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Y/+xq7R7CdbPWC8h" Content-Disposition: inline In-Reply-To: <53e1b351-da19-4491-b438-1e77e0dc836c@redhat.com> Message-ID-Hash: RYQFPD7WM5ZD7MFGOLVNDGAQR4E37ABF X-Message-ID-Hash: RYQFPD7WM5ZD7MFGOLVNDGAQR4E37ABF 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: Stefano Brivio , 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: --Y/+xq7R7CdbPWC8h Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Feb 18, 2025 at 09:01:30AM +0100, Laurent Vivier wrote: > On 18/02/2025 03:50, David Gibson wrote: > > head_cnt is a global variable which tracks how many entries in head[] a= re > > currently used. The fact that it's global obscures the fact that the > > lifetime over which it has a meaningful value is quite short: a single > > call to of tcp_vu_data_from_sock(). > >=20 > > Make it a local to tcp_vu_data_from_sock() to make that lifetime cleare= r. > > We keep the head[] array global for now - although technically it has t= he > > same valid lifetime - because it's large enough we might not want to put > > it on the stack. >=20 > We can make the array local but on the heap using a static > declaration in the function. We could. I kind of dislike static locals, as a rule, because it's pretty easy to miss them and make things non-reentrant by accident. > Reviewed-by: Laurent Vivier >=20 > >=20 > > Cc: Laurent Vivier > >=20 > > Signed-off-by: David Gibson > > --- > > tcp_vu.c | 21 +++++++++++---------- > > 1 file changed, 11 insertions(+), 10 deletions(-) > >=20 > > diff --git a/tcp_vu.c b/tcp_vu.c > > index 0622f173..6891ed13 100644 > > --- a/tcp_vu.c > > +++ b/tcp_vu.c > > @@ -38,7 +38,6 @@ > > static struct iovec iov_vu[VIRTQUEUE_MAX_SIZE + 1]; > > static struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE]; > > static int head[VIRTQUEUE_MAX_SIZE + 1]; > > -static int head_cnt; > > /** > > * tcp_vu_hdrlen() - return the size of the header in level 2 frame (= TCP) > > @@ -183,7 +182,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tc= p_tap_conn *conn, int flags) > > static ssize_t tcp_vu_sock_recv(const struct ctx *c, > > const struct tcp_tap_conn *conn, bool v6, > > uint32_t already_sent, size_t fillsize, > > - int *iov_cnt) > > + int *iov_cnt, int *head_cnt) > > { > > struct vu_dev *vdev =3D c->vdev; > > struct vu_virtq *vq =3D &vdev->vq[VHOST_USER_RX_QUEUE]; > > @@ -202,7 +201,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, > > vu_init_elem(elem, &iov_vu[1], VIRTQUEUE_MAX_SIZE); > > elem_cnt =3D 0; > > - head_cnt =3D 0; > > + *head_cnt =3D 0; > > while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE) { > > struct iovec *iov; > > size_t frame_size, dlen; > > @@ -221,7 +220,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, > > ASSERT(iov->iov_len >=3D hdrlen); > > iov->iov_base =3D (char *)iov->iov_base + hdrlen; > > iov->iov_len -=3D hdrlen; > > - head[head_cnt++] =3D elem_cnt; > > + head[(*head_cnt)++] =3D elem_cnt; > > fillsize -=3D dlen; > > elem_cnt +=3D cnt; > > @@ -261,17 +260,18 @@ static ssize_t tcp_vu_sock_recv(const struct ctx = *c, > > len -=3D iov->iov_len; > > } > > /* adjust head count */ > > - while (head_cnt > 0 && head[head_cnt - 1] >=3D i) > > - head_cnt--; > > + while (*head_cnt > 0 && head[*head_cnt - 1] >=3D i) > > + (*head_cnt)--; > > + > > /* mark end of array */ > > - head[head_cnt] =3D i; > > + head[*head_cnt] =3D i; > > *iov_cnt =3D i; > > /* release unused buffers */ > > vu_queue_rewind(vq, elem_cnt - i); > > /* restore space for headers in iov */ > > - for (i =3D 0; i < head_cnt; i++) { > > + for (i =3D 0; i < *head_cnt; i++) { > > struct iovec *iov =3D &elem[head[i]].in_sg[0]; > > iov->iov_base =3D (char *)iov->iov_base - hdrlen; > > @@ -357,11 +357,11 @@ int tcp_vu_data_from_sock(const struct ctx *c, st= ruct tcp_tap_conn *conn) > > struct vu_dev *vdev =3D c->vdev; > > struct vu_virtq *vq =3D &vdev->vq[VHOST_USER_RX_QUEUE]; > > ssize_t len, previous_dlen; > > + int i, iov_cnt, head_cnt; > > size_t hdrlen, fillsize; > > int v6 =3D CONN_V6(conn); > > uint32_t already_sent; > > const uint16_t *check; > > - int i, iov_cnt; > > if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) { > > debug("Got packet, but RX virtqueue not usable yet"); > > @@ -396,7 +396,8 @@ int tcp_vu_data_from_sock(const struct ctx *c, stru= ct tcp_tap_conn *conn) > > /* collect the buffers from vhost-user and fill them with the > > * data from the socket > > */ > > - len =3D tcp_vu_sock_recv(c, conn, v6, already_sent, fillsize, &iov_cn= t); > > + len =3D tcp_vu_sock_recv(c, conn, v6, already_sent, fillsize, > > + &iov_cnt, &head_cnt); > > if (len < 0) { > > if (len !=3D -EAGAIN && len !=3D -EWOULDBLOCK) { > > tcp_rst(c, conn); >=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 --Y/+xq7R7CdbPWC8h Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAme2/qgACgkQzQJF27ox 2GdSNxAAkVRdM4kkyB9XhU3ZgGxbezoPEgR3xg4bjoJ2WeUS0tWUeAByB21R14zT pxyf83pAW4AwbCR2mG8lomRcWIua6TRyovULRN4or0kdwUNZ7UYEd0HlS7lnXS+/ JXTucvx+6/q9cx0pPNxwWm8p5hdgbndkrL8B6bUkirD8kSkn5ENCNp47pp+jhiUA hlfqYm2A7/nqAI9/jyLCdCEmlc/SjFwXWco1LlPPDjB1uAv6SYA+gaRR604Nqa4W A3yBTUIAo0Xfi1qEfnVmQxQ1YTHcx5Z9mGQqbCfxtpwQuWQyoZK8iBAZUiogtZSF 5NBtkZzXOwufjYblFbv/QL1uX+1dQsHY9L1lTMURGK9CsFGePu/bnPMukx/tidCn LXtecCjU7SNVaTwDKcqgFbxnMvVwkQBa/e5Y2gwQPJUFLdVgfQL1jDtIyrgz/Mh3 Vrfc1jiNV2bkmucioEk8xvdPCuySHGxTbGWTXqX7nagd2XvXExd4mXTjwbScUDWl i3D0FmSa5Cfk680hTZ9/7YxOsomnBMx00SPYt8Z9q2F6nvi/076km9oQh9fkKouv TrxzGzqEpBv8dC3VHn0J7wfGfKaxGoOgoCJA6ygf0i1Bk0IS+9Rla9XhEC5UEBiD eda8GUjhpGEU+z+uXwuKWYTt+PPGRCD70+/waGVaNAK0l1Eh2tI= =2JyQ -----END PGP SIGNATURE----- --Y/+xq7R7CdbPWC8h--