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=LBbJcaDO; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id B8E2B5A0262 for ; Thu, 12 Mar 2026 11:19:31 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773310768; bh=BR4o4u2jWTfmZyT1shkP5j4Aq1rwXKC5Ev4Kj+tZ5XM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LBbJcaDO9O1TpEJ5wnvBHyiCwCInqpEiQjYHocHr1RdRoCS03UKTzm6Q3T5xwzkJX KNBG6/to4mv9zxAeOSv4lk4sCNClo1k3GRBFnwK30rw4PXC9TGYqcjn6hZhayhGKYw mjabM89F+yd9+5am/e72dDyZbv3FyugTX4CJKUq/2IiP/XPteZoSbi8bCU+geJ4llT FuXHj4Batd91Hl+9dDkHVXebs9QPiHrQ4YUMtq0QmxnxI3+4suSPNegHL2Q7xHRzYD QpZIxKPCyapYSNmIaqTujHjeX3fiNlzmeSBviK3CD5RgOu3g9k3aVMt2bJ1tJPNl3Y Qw6OKFBRM/HTg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fWkB83RTYz4wM1; Thu, 12 Mar 2026 21:19:28 +1100 (AEDT) Date: Thu, 12 Mar 2026 20:51:00 +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> <3f8ad2e8-55dc-4dc7-8fab-746bc9932c6b@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="0YTdHriWo/PRv9Xd" Content-Disposition: inline In-Reply-To: <3f8ad2e8-55dc-4dc7-8fab-746bc9932c6b@redhat.com> Message-ID-Hash: B6TONYVIYMBE4CUM3ZHW7XO5MWWQY6LU X-Message-ID-Hash: B6TONYVIYMBE4CUM3ZHW7XO5MWWQY6LU 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: --0YTdHriWo/PRv9Xd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 12, 2026 at 09:19:52AM +0100, Laurent Vivier wrote: > On 3/12/26 05:30, David Gibson wrote: > > 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-sp= ecific > > > helpers (vu_eth(), vu_ip(), vu_payloadv4(), vu_payloadv6()) that assu= me a > > > contiguous buffer. The payload length is now implicit in the iov_tai= l, 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, voi= d *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 *i= ov, 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; > > > /* ethernet header */ > > > - eh =3D vu_eth(iov[0].iov_base); > > > + eh =3D IOV_REMOVE_HEADER(¤t, eh_storage); > > > memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest)); > > > memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source)); > > > /* 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; > > > eh->h_proto =3D htons(ETH_P_IP); > > > + iph =3D IOV_REMOVE_HEADER(¤t, iph_storage); > > > *iph =3D (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_UDP); > > > - 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; > >=20 > > 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. >=20 > I can use one PEEK and PUT with something like: > struct { > struct ethhdr eh; > struct struct iphdr > struct udphdr *uh > } __attribute__((__packed__)) *hdr4, hdr4_storage; > hdr4 =3D IOV_PEEK_HEADER(¤t, hdr4_storage); > ... > IOV_PUT_HEADER(¤t, hdr4); True. It means the headers need to be jointly contiguous, not just individually, which seems a pity - at least if we're attempting to keep in-place updates. > or as you proposed: >=20 > with_header(hdr4, ¤t) { > .... > } >=20 > Thanks, > Laurent >=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 --0YTdHriWo/PRv9Xd Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmyjGkACgkQzQJF27ox 2Gc8VQ//Rn4xuXct9euIBV6bRzh8Ex8cx3pFBfbnT3AWcS9oPfBeuDmjpzAfo5lk 7GtVfZNMgQriuLIacMoQuI5FHoC6U2c9gIcm+P0UeCAHqej1gkcJObtwFzBLppRP iLBh4OhiXxQrZPIpkqE+npBlqPIA+pLKMqKyZ1o/3DzBD3+pkiVWpXdFRws0h/aP lfz0b6eyTbrL7acpyp5KPfKY3PFUQzk5/d3DJb89eyfMHAZxk/s2vaw049nvHsoK jcIXDIzL0Kaa+bW5cgeos99/wJF+3mwmAfz+3/0Svq2pWglMw/TBR9KxVEiq54gm yyz4RxepE0CkvaXmFIJ6HyKuLH7qDMcUZPuSSuTJsme/bhUX6pJyc/Mpo1Yv0pwK NkjN98cG5U6hDqfHgN67cwPIWiArqW4yC+mPtxXZeWbET4PlKPuTY6xJIyDn4g4T bEVF7MP913RZ07U6fTELc8t+R99WXNQKUURMQIWF27Wayw3Jg1ktRx9gLShGT6+t vqBfQjwN9GYnVsCuaa9vBra50aO++FCQICpOfEuC4dthAM/Lj+zBHXRExwqMwCRe s4hwrbrrb/Z3Eb+M+ALecFRblmCXZNqUo6smfSD3hWnw/1g9G9vEHcmIzMhGsKhC geoWpOzz8Z60rST+nxWuokSYf4MNBD+nF3V+iTjEj38lCGHApX8= =Z6Ml -----END PGP SIGNATURE----- --0YTdHriWo/PRv9Xd--