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=IPg5uo8V; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id A53D95A026E for ; Thu, 12 Mar 2026 05:30:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773289822; bh=j+9bWEi9YZB3YxbenN6Q0kfYED3elTLoOcjc5mpbGTA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IPg5uo8VXAKjTxi0D+PU0CYV9wfqZMX7z0exbQ8g4VA44SCceUmEOVCQQCKfYjfNJ 2e8DxshhU6i/r85emTXtd4kdJ18XE4EzkSOvD/BPvdakaXl1zmkKin7RXqmyia3DYh jzO6rwEHZiAxnWmqEkXib8ReG/rUU/JC11uy6iLio4EogAKyB3+mvLGEYeFvwnB0bi nfA8KmmzCwC/vHTRUqIiCEgmBMXSl9mZOfNwpsEu9GCZldqo7IPf0m318OglZRc6+x FLiXsG4egkVwPToytkiQ5LwRd5y33jla0XoSrcmbfdjILjHbpi0PPFHuXFTh1iX5Dp gGHoWkx4OUeeA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fWZRL0zDyz4wDm; Thu, 12 Mar 2026 15:30:22 +1100 (AEDT) Date: Thu, 12 Mar 2026 15:30:14 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v2 08/13] udp_vu: Use iov_tail in udp_vu_prepare() Message-ID: References: <20260309094744.1907754-1-lvivier@redhat.com> <20260309094744.1907754-9-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="5GwKoBJLYUPjNbHN" Content-Disposition: inline In-Reply-To: <20260309094744.1907754-9-lvivier@redhat.com> Message-ID-Hash: DZNFAEMYARW26YIZD6HLD7GSZ7VUJY5L X-Message-ID-Hash: DZNFAEMYARW26YIZD6HLD7GSZ7VUJY5L 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: --5GwKoBJLYUPjNbHN Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 09, 2026 at 10:47:39AM +0100, Laurent Vivier wrote: > Rework udp_vu_prepare() to use IOV_REMOVE_HEADER() and IOV_PUT_HEADER() > to walk through Ethernet, IP and UDP headers instead of the layout-specif= ic > helpers (vu_eth(), vu_ip(), vu_payloadv4(), vu_payloadv6()) that assume a > contiguous buffer. The payload length is now implicit in the iov_tail, so > drop the dlen parameter. >=20 > Signed-off-by: Laurent Vivier > --- > iov.c | 1 - > udp_vu.c | 64 ++++++++++++++++++++++++++++++-------------------------- > 2 files changed, 34 insertions(+), 31 deletions(-) >=20 > diff --git a/iov.c b/iov.c > index 296f24b61067..1f554f5ac297 100644 > --- a/iov.c > +++ b/iov.c > @@ -313,7 +313,6 @@ void *iov_peek_header_(struct iov_tail *tail, void *v= , size_t len, size_t align) > * > * Return: number of bytes written > */ > -/* cppcheck-suppress unusedFunction */ > size_t iov_put_header_(struct iov_tail *tail, const void *v, size_t len) > { > size_t l =3D len; > diff --git a/udp_vu.c b/udp_vu.c > index 2a5d3f822bf6..a21a03dbf23e 100644 > --- a/udp_vu.c > +++ b/udp_vu.c > @@ -101,52 +101,54 @@ static ssize_t udp_vu_sock_recv(struct iovec *iov, = size_t *cnt, int s, bool v6) > * @c: Execution context > * @data: IO vector tail for the frame > * @toside: Address information for one side of the flow > - * @dlen: Packet data length > * > * Return: Layer-4 length > */ > static size_t udp_vu_prepare(const struct ctx *c, const struct iov_tail = *data, > - const struct flowside *toside, ssize_t dlen) > + const struct flowside *toside) > { > - const struct iovec *iov =3D data->iov; > - struct ethhdr *eh; > + struct iov_tail current =3D *data; > + struct ethhdr *eh, eh_storage; > + struct udphdr *uh, uh_storage; > size_t l4len; > =20 > /* ethernet header */ > - eh =3D vu_eth(iov[0].iov_base); > + eh =3D IOV_REMOVE_HEADER(¤t, eh_storage); > =20 > memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest)); > memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source)); > =20 > /* initialize header */ > if (inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)) { > - struct iphdr *iph =3D vu_ip(iov[0].iov_base); > - struct udp_payload_t *bp =3D vu_payloadv4(iov[0].iov_base); > - const struct iovec payload_iov =3D { > - .iov_base =3D bp->data, > - .iov_len =3D dlen, > - }; > - struct iov_tail payload =3D IOV_TAIL(&payload_iov, 1, 0); > + struct iphdr *iph, iph_storage; > =20 > eh->h_proto =3D htons(ETH_P_IP); > =20 > + iph =3D IOV_REMOVE_HEADER(¤t, iph_storage); > *iph =3D (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP); > =20 > - l4len =3D udp_update_hdr4(iph, &bp->uh, &payload, toside, true); > + uh =3D IOV_REMOVE_HEADER(¤t, uh_storage); > + l4len =3D udp_update_hdr4(iph, uh, ¤t, toside, true); > + > + current =3D *data; Oof, having to reset the tail to the original position in order to replay the PUTs in the right order seems kind of fragile. I'm beginning to wonder if for writing (not reading) headers, trying to use an in-place pointer is more trouble than it's worth. It would certainly be easier to reason about this, if you always construct the header in an external buffer, then there's just a sequence of IOV_PUT_HEADER()s to stack them into the iov tail. > + IOV_PUT_HEADER(¤t, eh); > + IOV_PUT_HEADER(¤t, iph); > + IOV_PUT_HEADER(¤t, uh); > } else { > - struct ipv6hdr *ip6h =3D vu_ip(iov[0].iov_base); > - struct udp_payload_t *bp =3D vu_payloadv6(iov[0].iov_base); > - const struct iovec payload_iov =3D { > - .iov_base =3D bp->data, > - .iov_len =3D dlen, > - }; > - struct iov_tail payload =3D IOV_TAIL(&payload_iov, 1, 0); > + struct ipv6hdr *ip6h, ip6h_storage; > =20 > eh->h_proto =3D htons(ETH_P_IPV6); > =20 > + ip6h =3D IOV_REMOVE_HEADER(¤t, ip6h_storage); > *ip6h =3D (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_UDP); > =20 > - l4len =3D udp_update_hdr6(ip6h, &bp->uh, &payload, toside, true); > + uh =3D IOV_REMOVE_HEADER(¤t, uh_storage); > + l4len =3D udp_update_hdr6(ip6h, uh, ¤t, toside, true); > + > + current =3D *data; > + IOV_PUT_HEADER(¤t, eh); > + IOV_PUT_HEADER(¤t, ip6h); > + IOV_PUT_HEADER(¤t, uh); > } > =20 > return l4len; > @@ -165,9 +167,10 @@ static void udp_vu_csum(const struct flowside *tosid= e, > struct iov_tail payload =3D *data; > struct udphdr *uh, uh_storage; > bool ipv4 =3D src4 && dst4; > + int hdrlen =3D sizeof(struct ethhdr) + > + (ipv4 ? sizeof(struct iphdr) : sizeof(struct ipv6hdr)); > =20 > - iov_drop_header(&payload, > - udp_vu_hdrlen(!ipv4) - sizeof(struct udphdr)); > + iov_drop_header(&payload, hdrlen); > uh =3D IOV_REMOVE_HEADER(&payload, uh_storage); > =20 > if (ipv4) > @@ -207,9 +210,9 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, i= nt n, flow_sidx_t tosidx) > } > =20 > for (i =3D 0; i < n; i++) { > + int elem_cnt, elem_used; > size_t iov_cnt; > ssize_t dlen; > - int elem_cnt; > =20 > vu_init_elem(elem, iov_vu, ARRAY_SIZE(elem)); > =20 > @@ -224,19 +227,20 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s,= int n, flow_sidx_t tosidx) > vu_queue_rewind(vq, elem_cnt); > break; > } > + elem_used =3D iov_cnt; I don't see under circumstances elem_used would become different from iov_cnt (and therefore I'm not sure why elem_used is needed). > =20 > /* release unused buffers */ > - vu_queue_rewind(vq, elem_cnt - iov_cnt); > - > + vu_queue_rewind(vq, elem_cnt - elem_used); > if (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); > + vu_set_vnethdr(iov_vu[0].iov_base, elem_used); > + iov_drop_header(&data, VNET_HLEN); > + udp_vu_prepare(c, &data, toside); > if (*c->pcap) { > udp_vu_csum(toside, &data); > - pcap_iov(data.iov, data.cnt, VNET_HLEN); > + pcap_iov(data.iov, data.cnt, data.off); A pcap_iov_tail() might be a useful at some point. > } > - vu_flush(vdev, vq, elem, data.cnt); > + vu_flush(vdev, vq, elem, elem_used); > } > } > } > --=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 --5GwKoBJLYUPjNbHN Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmyQVUACgkQzQJF27ox 2GeSKg//SLNSO7z3dou9613ipZnaCwVDrByJRW5s2DxAfS0oxkbHIo4J0hFBI8ul 87A/EoppiCMsyM3j+jHz6G8npQfqQ3d3qELAQ4qPZ16hYx5dsD99l9VpeKH9mo2C pX2njNll/xcJQf2q8SO4pKBhU7jHohVKzi0XjEq/36/Skw3Tbn4DUqJU9TBmDwyU 90R0czANLo02ZYAQXypC9RZl3YNXaF2Beu7uA8nvnCvEH2dZpwyegPMAGzHMYp+L 9grQAlSVpLFK4IIHeJ84rSMMkWcmiIHMJeW8w57DZNQUc7ZVHS5X9cdQfCQPu4DL XJhbPKVEuCHmqBOVDrceNdLEcDmsDYT4N+Vp51AnhIRBmqz3cbJhEl0DY+ju+hsv AxxQcs4aNaBZElWSfzBV0xRiMQgwToFQrU6RAcvT7q/KkeeRiJGkuR4/E6+RNVdZ 4ZRaF5va5rmjSTT6A77K+78m2WmOUCKCz48cqpRyw1RGo2tOecUaAcdnjPG/76oL 2xngRyGKQB1hxA9MMs5IQssT9Bai997xdVmXhM0BqBUfVkzC7FIBz+k/d0GSoRqA 5teu0yVMdduetqsNbKsBtQbsDhDGsLzmV6LjWYe4QVkPYTCo45KphUkcomvuYHMv dYmMj5BXNJybkX59U+4v1b/wUGEknAt46kUC3qRHESmRuidbJwg= =/OIV -----END PGP SIGNATURE----- --5GwKoBJLYUPjNbHN--