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=202602 header.b=YtflP01U; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 5A0CD5A0265 for ; Thu, 12 Mar 2026 04:52:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773287572; bh=eBTVn7ciwJmvWeHuSNRlzWnWHrbZoAgDlOgXore3Q18=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=YtflP01Ul66URkVqKeEh7awtyHlNutO6SlG9HYIZbztEf5gSwxh92TIaOuigXGhvM WZGD04PY8kMJXPfVDZ1l57aZKAOBBcTiDk15UMLhy3AkYLw86lDui4zwZvbdiSKS5k crpHR6f2ZkJlhgNZSDTTRIwbN3HsBQ0o66P0Awv+iigI6oRQr46kxSR3zqEFyLv+1e pTmnh4Ywd10gBItdeZTdYc9wJZ577Bje+TXDN6zUhF+5uxL41qnnQY8NNL98HTP3Y1 RKv+2Fq+NuyojAx3oifzJy31BSVY0acy1LhNvPSwg+HIyXqyQk/URsn7QA/vp86YHq dZg7qKJer7hdg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fWYc40fnzz4wHj; Thu, 12 Mar 2026 14:52:52 +1100 (AEDT) Date: Thu, 12 Mar 2026 14:44:21 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 05/13] udp_vu: Move virtqueue management from udp_vu_sock_recv() to its caller Message-ID: References: <20260309094744.1907754-1-lvivier@redhat.com> <20260309094744.1907754-6-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="b4ZXJCF3QLxRe7EX" Content-Disposition: inline In-Reply-To: <20260309094744.1907754-6-lvivier@redhat.com> Message-ID-Hash: DDGL5FP2MOQX4ZCYVTNLTLSLOLUQUMWB X-Message-ID-Hash: DDGL5FP2MOQX4ZCYVTNLTLSLOLUQUMWB 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: --b4ZXJCF3QLxRe7EX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 09, 2026 at 10:47:36AM +0100, Laurent Vivier wrote: > udp_vu_sock_recv() currently mixes two concerns: receiving data from the > socket and managing virtqueue buffers (collecting, rewinding, releasing). > This makes the function harder to reason about and couples socket I/O > with virtqueue state. >=20 > Move all virtqueue operations, vu_collect(), vu_init_elem(), > vu_queue_rewind(), vu_set_vnethdr(), and the queue-readiness check, into > udp_vu_sock_to_tap(), which is the only caller. This turns > udp_vu_sock_recv() into a pure socket receive function that simply reads > into the provided iov array and adjusts its length. >=20 > Signed-off-by: Laurent Vivier LGTM, although one note below. > --- > udp_vu.c | 87 ++++++++++++++++++++++++++------------------------------ > 1 file changed, 41 insertions(+), 46 deletions(-) >=20 > diff --git a/udp_vu.c b/udp_vu.c > index a39254776099..27ae93de4420 100644 > --- a/udp_vu.c > +++ b/udp_vu.c > @@ -58,73 +58,40 @@ static size_t udp_vu_hdrlen(bool v6) > =20 > /** > * udp_vu_sock_recv() - Receive datagrams from socket into vhost-user bu= ffers > - * @c: Execution context > * @iov: IO vector for the frame (modified on output) > * @cnt: Number of IO vector entries (in/out) > - * @vq: virtqueue to use to receive data > * @s: Socket to receive from > * @v6: Set for IPv6 connections > * > - * Return: size of received data, 0 if the datagram > - * was discarded because the virtqueue is not ready, -1 on error > + * Return: size of received data, -1 on error > */ > -static ssize_t udp_vu_sock_recv(const struct ctx *c, struct iovec *iov, > - size_t *cnt, struct vu_virtq *vq, int s, > - bool v6) > +static ssize_t udp_vu_sock_recv(struct iovec *iov, size_t *cnt, int s, b= ool v6) > { > - const struct vu_dev *vdev =3D c->vdev; > - struct msghdr msg =3D { 0 }; > + struct iovec msg_iov[*cnt]; > + struct msghdr msg =3D { 0 }; > struct iov_tail payload; > size_t hdrlen; > ssize_t dlen; > - int iov_cnt; > - > - ASSERT(!c->no_udp); > - > - if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) { > - debug("Got UDP packet, but RX virtqueue not usable yet"); > - > - if (recvmsg(s, &msg, MSG_DONTWAIT) < 0) > - debug_perror("Failed to discard datagram"); > - > - *cnt =3D 0; > - return 0; > - } > =20 > /* compute L2 header length */ > hdrlen =3D udp_vu_hdrlen(v6); > =20 > - vu_init_elem(elem, iov, *cnt); > - > - iov_cnt =3D vu_collect(vdev, vq, elem, ARRAY_SIZE(elem), > - IP_MAX_MTU + ETH_HLEN + VNET_HLEN, NULL); > - if (iov_cnt =3D=3D 0) > - return -1; > - > - payload =3D IOV_TAIL(iov, iov_cnt, hdrlen); > + payload =3D IOV_TAIL(iov, *cnt, hdrlen); > =20 > - struct iovec msg_iov[payload.cnt]; > msg.msg_iov =3D msg_iov; > msg.msg_iovlen =3D iov_tail_clone(msg.msg_iov, payload.cnt, &payload); > =20 > /* read data from the socket */ > dlen =3D recvmsg(s, &msg, 0); > - if (dlen < 0) { > - vu_queue_rewind(vq, iov_cnt); > + if (dlen < 0) > return -1; > - } > =20 > /* Pad short frames to ETH_ZLEN */ > if (ETH_ZLEN + VNET_HLEN > dlen + hdrlen) { > - iov_memset(iov, iov_cnt, dlen + hdrlen, 0, > + iov_memset(iov, *cnt, dlen + hdrlen, 0, > ETH_ZLEN + VNET_HLEN - (dlen + hdrlen)); > } > - *cnt =3D iov_truncate(iov, iov_cnt, dlen + hdrlen); > - > - vu_set_vnethdr(iov[0].iov_base, *cnt); > - > - /* release unused buffers */ > - vu_queue_rewind(vq, iov_cnt - *cnt); > + *cnt =3D iov_truncate(iov, *cnt, dlen + hdrlen); Padding the frame, then truncating that padding seems kind of odd. > =20 > return dlen; > } > @@ -212,20 +179,48 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s,= int n, flow_sidx_t tosidx) > bool v6 =3D !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)); > struct vu_dev *vdev =3D c->vdev; > struct vu_virtq *vq =3D &vdev->vq[VHOST_USER_RX_QUEUE]; > - struct iov_tail data; > int i; > =20 > + ASSERT(!c->no_udp); > + > + if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) { > + struct msghdr msg =3D { 0 }; > + > + debug("Got UDP packet, but RX virtqueue not usable yet"); > + > + for (i =3D 0; i < n; i++) { > + if (recvmsg(s, &msg, MSG_DONTWAIT) < 0) > + debug_perror("Failed to discard datagram"); > + } > + > + return; > + } > + > for (i =3D 0; i < n; i++) { > size_t iov_cnt; > ssize_t dlen; > + int elem_cnt; > =20 > - iov_cnt =3D VIRTQUEUE_MAX_SIZE; > - dlen =3D udp_vu_sock_recv(c, iov_vu, &iov_cnt, vq, s, v6); > - if (dlen < 0) > + vu_init_elem(elem, iov_vu, ARRAY_SIZE(elem)); > + > + elem_cnt =3D vu_collect(vdev, vq, elem, ARRAY_SIZE(elem), > + IP_MAX_MTU + ETH_HLEN + VNET_HLEN, NULL); > + if (elem_cnt =3D=3D 0) > break; > =20 > + iov_cnt =3D elem_cnt; > + dlen =3D udp_vu_sock_recv(iov_vu, &iov_cnt, s, v6); > + if (dlen < 0) { > + vu_queue_rewind(vq, elem_cnt); > + break; > + } > + > + /* release unused buffers */ > + vu_queue_rewind(vq, elem_cnt - iov_cnt); > + > if (iov_cnt > 0) { > - data =3D IOV_TAIL(iov_vu, iov_cnt, 0); > + struct iov_tail data =3D IOV_TAIL(iov_vu, iov_cnt, 0); > + vu_set_vnethdr(iov_vu[0].iov_base, iov_cnt); > udp_vu_prepare(c, &data, toside, dlen); > if (*c->pcap) { > udp_vu_csum(toside, &data); > --=20 > 2.53.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 --b4ZXJCF3QLxRe7EX Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmyNpQACgkQzQJF27ox 2GcDdw//Qa8ytMzj55PP0xlULSS6zNfpU3hWNl5txqN1WzjolqjSyEBsj2bk/Fmm qf0sYPt3rIeU7uiDDUn8ymdGP+EYk8kO4/RTG/I64cwpRsb8oVE06sYPJBHO39TA LPYff69CWyxLS66eSg2/OoVB8LfXggDRxwR7UAMgCjgtw3CJvD9myqKAyl5M5Ph9 ROqOoMiaFh4jPnrtHmbey7Dz5f9kIM0zdNgsz7P7U+mrcbjPn78CUCxLpghUI+mW hvGONwOykxhGnVDV8FtnKDKliM2I6V4+MhS9xW8omeJHBm9wVa7ou3ZKIoItd0ZY 7+wdSYvyWB0nNH4kC9LYxSkQKOj36WAfrHKSESDMa5YhGEPqYj2hEglC9Tiu4ekH 1fzkyPAdVvmvHkupuYCNDB7aByezIH7uFyxkr82RcwY3hVT0JpsZbJvGqsDZtfRd TPY5HwzO8Z7/7c4k38+5szaCOihPdXouEZ9qB/7wKOpoGZIjJU7EF6+UvtFXhDcR 7VjZTvi1sj2czT9WqFzFRKVG/eEap8FXxooL/BOwRXil+GOhdKjmGZNrhxqFI2pt 15SPI0TmlGBrhCt8W99yJsU5cOSH6QqeOqZSOfrHmconfx8CW3TePJCWxrxaoHfg ghBg25hSYCRsu4ZA5N7Lkwzy2661IT1/VUfMAqD1bLeX/HIpAPk= =4mxs -----END PGP SIGNATURE----- --b4ZXJCF3QLxRe7EX--