Re: [PATCH v10 03/30] iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 8346 bytes --]

On Thu, Aug 14, 2025 at 11:48:22AM +0200, Laurent Vivier wrote:
> Provide a temporary variable of the wanted type to store
> the header if the memory in the iovec array is not contiguous.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  iov.c     | 53 ++++++++++++++++++++++++++++++++++++++++++++---------
>  iov.h     | 35 +++++++++++++++++++++--------------
>  tcp_buf.c |  2 +-
>  3 files changed, 66 insertions(+), 24 deletions(-)
> 
> diff --git a/iov.c b/iov.c
> index 9a91eb63026a..d20fbb854933 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -109,7 +109,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>   *
>   * Return: the number of bytes successfully copied.
>   */
> -/* cppcheck-suppress unusedFunction */
> +/* cppcheck-suppress [staticFunction] */
>  size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>  		  size_t offset, void *buf, size_t bytes)
>  {
> @@ -127,6 +127,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>  	/* copying data */
>  	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
>  		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
> +		ASSERT(iov[i].iov_base);
>  		memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset,
>  		       len);
>  		copied += len;
> @@ -208,7 +209,7 @@ bool iov_drop_header(struct iov_tail *tail, size_t len)
>  }
>  
>  /**
> - * iov_peek_header_() - Get pointer to a header from an IOV tail
> + * iov_check_header() - Check if a header can be accessed
>   * @tail:	IOV tail to get header from
>   * @len:	Length of header to get, in bytes
>   * @align:	Required alignment of header, in bytes
> @@ -219,8 +220,7 @@ bool iov_drop_header(struct iov_tail *tail, size_t len)
>   *	   overruns the IO vector, is not contiguous or doesn't have the
>   *	   requested alignment.
>   */
> -/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
> -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
> +static void *iov_check_header(struct iov_tail *tail, size_t len, size_t align)
>  {
>  	char *p;
>  
> @@ -240,27 +240,62 @@ void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align)
>  	return p;
>  }
>  
> +/**
> + * iov_peek_header_() - Get pointer to a header from an IOV tail
> + * @tail:	IOV tail to get header from
> + * @v:		Temporary memory to use if the memory in @tail
> + *		is discontinuous
> + * @len:	Length of header to get, in bytes
> + * @align:	Required alignment of header, in bytes
> + *
> + * @tail may be pruned, but will represent the same bytes as before.
> + *
> + * Return: pointer to the first @len logical bytes of the tail, or to
> + *         a copy if that overruns the IO vector, is not contiguous or
> + *         doesn't have the requested alignment. NULL if that overruns the
> + *         IO vector.
> + */
> +/* cppcheck-suppress [staticFunction,unmatchedSuppression] */
> +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
> +{
> +	char *p = iov_check_header(tail, len, align);
> +	size_t l;
> +
> +	if (p)
> +		return p;
> +
> +	l = iov_to_buf(tail->iov, tail->cnt, tail->off, v, len);
> +	if (l != len)
> +		return NULL;
> +
> +	return v;
> +}
> +
>  /**
>   * iov_remove_header_() - Remove a header from an IOV tail
>   * @tail:	IOV tail to remove header from (modified)
> + * @v:		Temporary memory to use if the memory in @tail
> + *		is discontinuous
>   * @len:	Length of header to remove, in bytes
>   * @align:	Required alignment of header, in bytes
>   *
>   * On success, @tail is updated so that it longer includes the bytes of the
>   * returned header.
>   *
> - * Return: pointer to the first @len logical bytes of the tail, NULL if that
> - *	   overruns the IO vector, is not contiguous or doesn't have the
> - *	   requested alignment.
> + * Return: pointer to the first @len logical bytes of the tail, or to
> + *         a copy if that overruns the IO vector, is not contiguous or
> + *         doesn't have the requested alignment. NULL if that overruns the
> + *         IO vector.
>   */
> -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align)
> +void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
>  {
> -	char *p = iov_peek_header_(tail, len, align);
> +	char *p = iov_peek_header_(tail, v, len, align);
>  
>  	if (!p)
>  		return NULL;
>  
>  	tail->off = tail->off + len;
> +
>  	return p;
>  }
>  
> diff --git a/iov.h b/iov.h
> index c94fabe1eb43..1821b40b1cb5 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -73,38 +73,45 @@ struct iov_tail {
>  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, size_t len, size_t align);
> -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align);
> +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
> +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);
>  
>  /**
>   * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail
>   * @tail_:	IOV tail to get header from
> - * @type_:	Data type of the header
> + * @var_:	Temporary buffer of the type of the header to use if
> + *		the memory in the iovec array is not contiguous.
>   *
>   * @tail_ may be pruned, but will represent the same bytes as before.
>   *
> - * Return: pointer of type (@type_ *) located at the start of @tail_, NULL if
> - *         we can't get a contiguous and aligned pointer.
> + * Return: pointer of type (@type_ *) located at the start of @tail_
> + *         or to @var_ if iovec memory is not contiguous, NULL if
> + *         that overruns the iovec.
>   */
> -#define IOV_PEEK_HEADER(tail_, type_)					\
> -	((type_ *)(iov_peek_header_((tail_),				\
> -				    sizeof(type_), __alignof__(type_))))
> +
> +#define IOV_PEEK_HEADER(tail_, var_)					\
> +	((__typeof__(var_) *)(iov_peek_header_((tail_), &(var_),	\
> +					       sizeof(var_),		\
> +					       __alignof__(var_))))
>  
>  /**
>   * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail
>   * @tail_:	IOV tail to remove header from (modified)
> - * @type_:	Data type of the header to remove
> + * @var_:	Temporary buffer of the type of the header to use if
> + *		the memory in the iovec array is not contiguous.
>   *
>   * On success, @tail_ is updated so that it longer includes the bytes of the
>   * returned header.
>   *
> - * Return: pointer of type (@type_ *) located at the old start of @tail_, NULL
> - *         if we can't get a contiguous and aligned pointer.
> + * Return: pointer of type (@type_ *) located at the start of @tail_
> + *         or to @var_ if iovec memory is not contiguous, NULL if
> + *         that overruns the iovec.
>   */
> -#define IOV_REMOVE_HEADER(tail_, type_)					\
> -	((type_ *)(iov_remove_header_((tail_),				\
> -				      sizeof(type_), __alignof__(type_))))
> +
> +#define IOV_REMOVE_HEADER(tail_, var_)					\
> +	((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_),	\
> +				    sizeof(var_), __alignof__(var_))))
>  
>  #endif /* IOVEC_H */
> diff --git a/tcp_buf.c b/tcp_buf.c
> index d1fca676c9a7..bc898de86919 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn,
>  				    uint32_t seq, bool no_tcp_csum)
>  {
>  	struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0);
> -	struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr);
> +	struct tcphdr th_storage, *th = IOV_REMOVE_HEADER(&tail, th_storage);
>  	struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base;
>  	const struct flowside *tapside = TAPFLOW(conn);
>  	const struct in_addr *a4 = inany_v4(&tapside->oaddr);

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]