Re: [PATCH v8 02/30] iov: Introduce iov_tail_clone() and iov_tail_drop().

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

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

On Tue, Aug 05, 2025 at 05:46:00PM +0200, Laurent Vivier wrote:
> These utilities enhance iov_tail manipulation, useful for
> efficient packet processing by enabling iovec array cloning and
> header stripping without data copies.
> 
> - iov_tail_drop(): Discards a specified number of bytes from the
>   beginning of an iov_tail by advancing its internal offset and pruning
>   consumed elements.
> 
> - iov_tail_clone(): Clone an iov_tail into an iovec array, adjusting the
> first iovec entry to remove the iov_tail offset.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

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

To the extent that the functions look useful and the implementations
look correct; a couple of suggested improvements below.

> ---
>  iov.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  iov.h |  3 +++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/iov.c b/iov.c
> index edf0444d1955..9d282d4af461 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -192,6 +192,21 @@ size_t iov_tail_size(struct iov_tail *tail)
>  	return iov_size(tail->iov, tail->cnt) - tail->off;
>  }
>  
> +/**
> + * iov_tail_drop() - Discard a header from an IOV tail

Maybe _drop_hdr() to clarify that it's dropping specifically from the
start.  Or maybe not, I'm torn between brevity and specificity.

> + * @tail:	IO vector tail
> + * @len:	length to move the head of the tail
> + *
> + * Return: true if the item still contains any bytes, otherwise false
> + */
> +/* cppcheck-suppress unusedFunction */
> +bool iov_tail_drop(struct iov_tail *tail, size_t len)
> +{
> +	tail->off = tail->off + len;
> +
> +	return iov_tail_prune(tail);
> +}
> +
>  /**
>   * iov_peek_header_() - Get pointer to a header from an IOV tail
>   * @tail:	IOV tail to get header from
> @@ -248,3 +263,40 @@ void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align)
>  	tail->off = tail->off + len;
>  	return p;
>  }
> +
> +/**
> + * iov_tail_clone() - Assign iov references referencing a subset of the data
> + *		      in an iov_tail
> + *
> + * @dst_iov:     Pointer to the destination array of struct iovec describing
> + *		 the scatter/gather I/O vector to shallow copy to.
> + * @dst_iov_cnt: Maximum number of elements in the destination iov array.
> + * @tail:	 Pointer to the source iov_tail
> + *
> + * Return: the number of elements successfully referenced from the destination
> + *	   iov array, a negative value if there is not enough room in the
> + *	   destination iov array
> + */
> +/* cppcheck-suppress unusedFunction */
> +ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
> +		       struct iov_tail *tail)
> +{
> +	const struct iovec *iov = &tail->iov[0];
> +	size_t iov_cnt = tail->cnt;
> +	size_t offset = tail->off;
> +	unsigned int i, j;
> +
> +	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);

If you prune first, then you can assume that offset is within the
first entry, and you'll also know that you need exactly tail->cnt
entries in the destination.

You could then:
	if (tail->off != 0)
		/* copy partial first entry */
	/* memcpy the rest */

> +	/* assign iov references referencing a subset of the source one */
> +	for (j = 0; i < iov_cnt && j < dst_iov_cnt; i++, j++) {
> +		dst_iov[j].iov_base = (char *)iov[i].iov_base + offset;
> +		dst_iov[j].iov_len = iov[i].iov_len - offset;
> +		offset = 0;
> +	}
> +
> +	if (j == dst_iov_cnt && i != iov_cnt)
> +		return -1;
> +
> +	return j;
> +}
> diff --git a/iov.h b/iov.h
> index 3fc96ab9755a..bf9820ac52ab 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -72,8 +72,11 @@ struct iov_tail {
>  
>  bool iov_tail_prune(struct iov_tail *tail);
>  size_t iov_tail_size(struct iov_tail *tail);
> +bool iov_tail_drop(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);
> +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

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