Re: [RFC PATCH 3/3] iov, vu_common: Make iov_from_buf() fill destination iov entirely

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

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

On Mon, Nov 03, 2025 at 11:16:29AM +0100, Stefano Brivio wrote:
> ...and, for consistency, rename 'bytes' to 'copy' in iov_to_buf().
> 
> Two commits ago, I changed vhost-user functions to use iov_from_buf()
> to copy only up to the size of source buffers, instead of using the
> size of the destination vhost-user buffers.
> 
> This change pads the rest with zeroes, which is not strictly needed,
> but looks definitely cleaner.

This seems like a useful thing, but maybe it would be clearer as a
separate iov_memset() / iov_from_zero() rather than built into
iov_from_buf().

> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  iov.c       | 54 ++++++++++++++++++++++++++++++++++-------------------
>  iov.h       |  4 ++--
>  vu_common.c |  2 +-
>  3 files changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/iov.c b/iov.c
> index dc1b6b1..557be55 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -59,35 +59,51 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n,
>   * @iov_cnt:	Number of elements in the iovec array
>   * @offset:	Destination offset in iovec array
>   * @buf:	Source buffer
> - * @bytes:	Bytes to copy
> + * @copy:	Bytes to copy
> + * @fill:	Bytes to zero-fill after copied bytes
>   *
> - * Return: number of bytes copied
> + * Return: number of bytes filled, with data or zeroes
>   */
>  size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> -		    size_t offset, const void *buf, size_t bytes)
> +		    size_t offset, const void *buf, size_t copy, size_t fill)
>  {
> +	size_t copied, filled;
>  	unsigned int i;
> -	size_t copied;
>  
> -	if (__builtin_constant_p(bytes) && iov_cnt &&
> -		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
> -		memcpy((char *)iov[0].iov_base + offset, buf, bytes);
> +	if (__builtin_constant_p(copy) && iov_cnt &&
> +		offset <= iov[0].iov_len &&
> +		(copy + fill) <= iov[0].iov_len - offset) {
> +		memcpy((char *)iov[0].iov_base + offset, buf, copy);
> +		memset((char *)iov[0].iov_base + offset + copy, 0, fill);
>  
> -		return bytes;
> +		return copy + fill;
>  	}
>  
>  	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
>  
> -	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
> -		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
> +	for (copied = 0; copied < copy && i < iov_cnt; i++) {
> +		size_t len = MIN(iov[i].iov_len - offset, copy - copied);
>  
>  		memcpy((char *)iov[i].iov_base + offset, (char *)buf + copied,
>  		       len);
>  		copied += len;
> +
> +		if (copied < copy)
> +			offset = 0;	/* More to copy in the next iteration */
> +		else
> +			offset = len;	/* Start of zero-filling, see below */
> +	}
> +
> +	for (filled = 0; filled < fill && i < iov_cnt; i++) {
> +		size_t len = MIN(iov[i].iov_len - offset, fill - filled);
> +
> +		memcpy((char *)iov[i].iov_base + offset,
> +		       (char *)buf + copied + filled, len);
> +		filled += len;
>  		offset = 0;
>  	}
>  
> -	return copied;
> +	return copied + filled;
>  }
>  
>  /**
> @@ -96,27 +112,27 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>   * @iov_cnt:	Number of elements in iovec array
>   * @offset:	Source offset altogether, counted in flattened iovec
>   * @buf:	Destination buffer
> - * @bytes:	Bytes to copy
> + * @copy:	Bytes to copy
>   *
>   * Return: number of bytes copied
>   */
>  size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> -		  size_t offset, void *buf, size_t bytes)
> +		  size_t offset, void *buf, size_t copy)
>  {
>  	unsigned int i;
>  	size_t copied;
>  
> -	if (__builtin_constant_p(bytes) && iov_cnt &&
> -		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
> -		memcpy(buf, (char *)iov[0].iov_base + offset, bytes);
> +	if (__builtin_constant_p(copy) && iov_cnt &&
> +		offset <= iov[0].iov_len && copy <= iov[0].iov_len - offset) {
> +		memcpy(buf, (char *)iov[0].iov_base + offset, copy);
>  
> -		return bytes;
> +		return copy;
>  	}
>  
>  	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
>  
> -	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
> -		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
> +	for (copied = 0; copied < copy && i < iov_cnt; i++) {
> +		size_t len = MIN(iov[i].iov_len - offset, copy - copied);
>  
>  		ASSERT(iov[i].iov_base);
>  
> diff --git a/iov.h b/iov.h
> index ba1fda5..9b5d910 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -24,9 +24,9 @@
>  size_t iov_skip_bytes(const struct iovec *iov, size_t n,
>  		      size_t skip, size_t *offset);
>  size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> -                    size_t offset, const void *buf, size_t bytes);
> +                    size_t offset, const void *buf, size_t copy, size_t fill);
>  size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> -                  size_t offset, void *buf, size_t bytes);
> +                  size_t offset, void *buf, size_t copy);
>  size_t iov_size(const struct iovec *iov, size_t iov_cnt);
>  
>  /*
> diff --git a/vu_common.c b/vu_common.c
> index 21cfb2a..d941b72 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -274,7 +274,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
>  
>  	/* copy data from the buffer to the iovec */
>  	iov_from_buf(in_sg, elem_cnt, sizeof(struct virtio_net_hdr_mrg_rxbuf),
> -		     buf, size);
> +		     buf, size, vu_buf_size - size);
>  
>  	if (*c->pcap) {
>  		pcap_iov(in_sg, elem_cnt,
> -- 
> 2.43.0
> 

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