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

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