Re: [PATCH v4 4/5] iov: Add IOV_PUT_HEADER() and with_header() to write header data back to iov_tail

[Laurent Vivier <lvivier@redhat.com>]

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().
>>>
>>> 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.
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>   iov.c | 22 ++++++++++++++++++++++
>>>   iov.h | 25 ++++++++++++++++++++++++-
>>>   2 files changed, 46 insertions(+), 1 deletion(-)
>>>
>>> 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 = len;
>>> +
>>> +	/* iov_peek_header_() already called iov_check_header() */
>>> +	if ((char *)tail->iov[0].iov_base + tail->off != v)
>>> +		l = iov_from_buf(tail->iov, tail->cnt, tail->off, v, len);
>>> +
>>> +	assert(l == 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 len, 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 IOV tail
>>>    * @tail_:	IOV tail to remove header from (modified)
>>> @@ -130,7 +141,8 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_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_), sizeof(type_))
>>>   
>>> +/**
>>> + * with_header() - Execute a block on a given header
>>> + * @type:	Data type of the header to modify
>>
>> We already use __typeof__ in IOV_PEEK_HEADER(), so we should be able
>> to use that to avoid explicitly passing the type.
>>
>>> + * @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_ = IOV_PEEK_HEADER(tail_, store_); 	\
>>> +	     hdr_;							\
>>> +	     IOV_PUT_HEADER(tail_, hdr_), hdr_ = NULL)
>>> +
> 
> 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) {
> 			...
> 		}
> 	}
> 

You're right, but I don't think there is an easy way to stack them (we need to add a 
"rewind" function to do that or to save and store the tail state). I think it introduces 
unnecessary complexity.

Whereas using "with_header()" makes the code easier to read.

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,...)

Thanks,
Laurent