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=kE4eOk/z; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 151A55A0269 for ; Wed, 25 Mar 2026 00:46:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774396012; bh=m/NwIq63b6pzZoVTJCBiPqwiOi3U/zyxpYUFeXscGtY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kE4eOk/z2+Vd1MMqTRwo3LpT7wA+KG2B/K8ufxIW+p/IufdNdXAhjm1rkjbjqyyRr Xoijt5uCYtIbquFcHHzFfwTRCF7hr7OdeVJ7e4Yf8gLoposps8X/h6gofb/xCDX8RZ 3yGrG3buo6xicjN6WqgYHuAttc+bfDTYWfbroXaEyHtzmu8Sd5tI9KikDif2qEO2vz s7DYWTXYAehsgCQSVx806h1+KimGL55vkbPDAlEwS1FqJMFjSJ8v5gwKhRDz8Cgwff mo9hxWzOaovEDp0knIOruTPZHU2KiBpSdGTBVtnHtEnZBvh1cFfU0QQ/Mlzb/P8+m/ b47d6IUHC3VYA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fgRXD4n32z4wHX; Wed, 25 Mar 2026 10:46:52 +1100 (AEDT) Date: Wed, 25 Mar 2026 10:46:47 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v4 4/5] iov: Add IOV_PUT_HEADER() and with_header() to write header data back to iov_tail Message-ID: References: <20260323143151.538673-1-lvivier@redhat.com> <20260323143151.538673-5-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="5xcqUFQXRZw/elyp" Content-Disposition: inline In-Reply-To: Message-ID-Hash: F7W23SJJKPN5NXY36F4I7BMGPLQ3YCOT X-Message-ID-Hash: F7W23SJJKPN5NXY36F4I7BMGPLQ3YCOT 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: --5xcqUFQXRZw/elyp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 24, 2026 at 08:44:40AM +0100, Laurent Vivier wrote: > On 3/24/26 03:48, David Gibson wrote: > > On Tue, Mar 24, 2026 at 01:41:18PM +1100, David Gibson wrote: > > > On Mon, Mar 23, 2026 at 03:31:50PM +0100, Laurent Vivier wrote: > > > > Add iov_put_header_() and its wrapper macro IOV_PUT_HEADER() as a > > > > counterpart to IOV_PEEK_HEADER(). This writes header data back to an > > > > iov_tail after modification. If the header pointer matches the > > > > original iov buffer location, the data was already modified in place > > > > and no copy is needed. Otherwise, it copies the data back using > > > > iov_from_buf(). > > > >=20 > > > > Add with_header(), a for-loop macro that combines IOV_PEEK_HEADER() > > > > and IOV_PUT_HEADER() to allow modifying a header in place within a > > > > block scope. > > > >=20 > > > > Signed-off-by: Laurent Vivier > > > > --- > > > > iov.c | 22 ++++++++++++++++++++++ > > > > iov.h | 25 ++++++++++++++++++++++++- > > > > 2 files changed, 46 insertions(+), 1 deletion(-) > > > >=20 > > > > diff --git a/iov.c b/iov.c > > > > index 8134b8c9f988..7fc9c3c78a32 100644 > > > > --- a/iov.c > > > > +++ b/iov.c > > > > @@ -308,6 +308,28 @@ void *iov_peek_header_(struct iov_tail *tail, = void *v, size_t len, size_t align) > > > > return v; > > > > } > > > > +/** > > > > + * iov_put_header_() - Write header back to an IOV tail > > > > + * @tail: IOV tail to write header to > > > > + * @v: Pointer to header data to write > > > > + * @len: Length of header to write, in bytes > > > > + * > > > > + * Return: number of bytes written > > > > + */ > > > > +/* cppcheck-suppress unusedFunction */ > > > > +size_t iov_put_header_(const struct iov_tail *tail, const void *v,= size_t len) > > > > +{ > > > > + size_t l =3D len; > > > > + > > > > + /* iov_peek_header_() already called iov_check_header() */ > > > > + if ((char *)tail->iov[0].iov_base + tail->off !=3D v) > > > > + l =3D iov_from_buf(tail->iov, tail->cnt, tail->off, v, len); > > > > + > > > > + assert(l =3D=3D len); > > > > + > > > > + return l; > > > > +} > > > > + > > > > /** > > > > * iov_remove_header_() - Remove a header from an IOV tail > > > > * @tail: IOV tail to remove header from (modified) > > > > diff --git a/iov.h b/iov.h > > > > index d295d05b3bab..4ce425ccdbe5 100644 > > > > --- a/iov.h > > > > +++ b/iov.h > > > > @@ -90,6 +90,7 @@ bool iov_tail_prune(struct iov_tail *tail); > > > > size_t iov_tail_size(struct iov_tail *tail); > > > > bool iov_drop_header(struct iov_tail *tail, size_t len); > > > > void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len= , size_t align); > > > > +size_t iov_put_header_(const struct iov_tail *tail, const void *v,= size_t len); > > > > void *iov_remove_header_(struct iov_tail *tail, void *v, size_t l= en, size_t align); > > > > ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, > > > > struct iov_tail *tail); > > > > @@ -112,6 +113,16 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, = size_t dst_iov_cnt, > > > > sizeof(var_), \ > > > > __alignof__(var_)))) > > > > +/** > > > > + * IOV_PUT_HEADER() - Write header back to an IOV tail > > > > + * @tail_: IOV tail to write header to > > > > + * @var_: Pointer to a variable containing the header data to write > > > > + * > > > > + * Return: number of bytes written > > > > + */ > > > > +#define IOV_PUT_HEADER(tail_, var_) \ > > > > + (iov_put_header_((tail_), (var_), sizeof(*var_))) > > > > + > > > > /** > > > > * IOV_REMOVE_HEADER() - Remove and return typed header from an I= OV tail > > > > * @tail_: IOV tail to remove header from (modified) > > > > @@ -130,7 +141,8 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, s= ize_t dst_iov_cnt, > > > > ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \ > > > > sizeof(var_), __alignof__(var_)))) > > > > -/** IOV_DROP_HEADER() - Remove a typed header from an IOV tail > > > > +/** > > > > + * IOV_DROP_HEADER() - Remove a typed header from an IOV tail > > > > * @tail_: IOV tail to remove header from (modified) > > > > * @type_: Data type of the header to remove > > > > * > > > > @@ -138,4 +150,15 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, = size_t dst_iov_cnt, > > > > */ > > > > #define IOV_DROP_HEADER(tail_, type_) iov_drop_header((tail_), s= izeof(type_)) > > > > +/** > > > > + * with_header() - Execute a block on a given header > > > > + * @type: Data type of the header to modify > > >=20 > > > We already use __typeof__ in IOV_PEEK_HEADER(), so we should be able > > > to use that to avoid explicitly passing the type. > > >=20 > > > > + * @hdr_: Variable name to receive the header pointer > > > > + * @tail_: IOV tail to peek/put the header from/to > > > > + */ > > > > +#define with_header(type_, hdr_, tail_) \ > > > > + for (type_ store_, *hdr_ =3D IOV_PEEK_HEADER(tail_, store_); \ > > > > + hdr_; \ > > > > + IOV_PUT_HEADER(tail_, hdr_), hdr_ =3D NULL) > > > > + > >=20 > > I know I suggested this, but looking at it now, I'm wondering if the > > fact that you _must not_ alter the tail in the block is too > > non-obvious a constraint :/. In particular this means you can never > > work with multiple headers at once, say: > > with_header(iphdr) { > > with_header(udphdr) { > > ... > > } > > } > >=20 >=20 > You're right, but I don't think there is an easy way to stack them (we ne= ed > to add a "rewind" function to do that or to save and store the tail state= ). > I think it introduces unnecessary complexity. Right, I tend to agree. > Whereas using "with_header()" makes the code easier to read. See, the fact that there's several vital, invisible constraints about how you use it means I'm not sure that's really true any more. > If we want to keep it simple, the easy way is to copy all the headers to a > buffer (but we cannot have the size to copy without decoding the headers), > update them and write them back. But if we do that we can have alignment > warning coming from the compiler when we want to get a point to one of the > headers (when we do things like udp_xxx(&headers->uh,...) Few (do any?) paths both read and write the headers to/from iov; they generally do one or the other. The existing PEEK and REMOVE work well on read-only paths. For write-only paths we could instead always build the header in a local outside the iov (separate local for each header). Then PUSH_HEADER onto the iov_tail at the right point, which would do an unconditional iov_from_buf(). Yes, that means we sacrifice in-place acess even in cases we could use it. Honestly though, given the sizes of the headers, I'm not sure that's actually going to be a win compared to the extra complexity and conditionals we need for with_header(). --=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 --5xcqUFQXRZw/elyp Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnDImYACgkQzQJF27ox 2Gf3Eg//atFkQTHXkaHo7HDxRCNVG1BplypPAI5Nyg5HM0uU/Mzcz4kJ9dX4W7SL 15VW6fI8miPk15O5MKWsmldBLBuK8+8UPrQYKx3+Bgj0UWMyTM054G03FLCRIWKD 6/hVSzq13J1YL3idfspd1nVy8sgZ4OnksKKGxAadLwE1e5iBv7XCTfZOIlLgkqqp oZVGtpqU2dCc+5vCTOx1u2kViZYrrFwg1ZlTS4LK6sq7vy6+6kekYZLEovkJFFM3 2Vz13+D2ctDmWHRe5+o3z7ECiLpNf94B/gc1Ww5blFqKAI2CGkkAfnGVRCr0xkRz Zj7UwROAcdxseQDJW1R/H+IWTSfK6F5Q2nonMqBXMr/zF6rEkhmNFK1u4lEFJxts LxQuD8LBKs1U5ETQzwx8vzu/y8bSAvMJsRahYnpv5AJWNtV0x9yegtQuTQc38bTh PEVIkMIDPACemEHokMhD/dVcMkHKaosOKe8QXOUDDHG0GemfbFhu1EOMgRdOReup 9hWqjkdmgRR9CyBDIsFe6iQNqr3tneQq/pHL5zOX3P6nRiCnilvG9oe+FiC7Kz1j JPkuRPyACDpf7ozOILmPm/0Jw1BbxBloHmD+3cEvDjRFqk4vEDcJvYYrtB+KKhAR hJuKPPwwn30nPsmYnKB9ABKkDHEoAibr/hpyGqJElBG/voFphks= =ipfe -----END PGP SIGNATURE----- --5xcqUFQXRZw/elyp--