As I'm going to touch those in the next commit. Signed-off-by: Stefano Brivio --- iov.c | 68 +++++++++++++++++++++++++---------------------------------- 1 file changed, 29 insertions(+), 39 deletions(-) diff --git a/iov.c b/iov.c index 8c74a59..dc1b6b1 100644 --- a/iov.c +++ b/iov.c @@ -20,24 +20,21 @@ * Contributions after 2012-01-13 are licensed under the terms of the * GNU GPL, version 2 or (at your option) any later version. */ + #include #include "util.h" #include "iov.h" - /** - * iov_skip_bytes() - Skip leading bytes of an IO vector - * @iov: IO vector + * iov_skip_bytes() - Find index and offset in iovec array given byte offset + * @iov: iovec array * @n: Number of entries in @iov - * @skip: Number of leading bytes of @iov to skip - * @offset: Offset of first unskipped byte in its @iov entry + * @skip: Byte offset: leading bytes of @iov to skip + * @offset: Offset within matching @iov entry, set on return, can be NULL * - * Return: index I of individual struct iovec which contains the byte at @skip - * bytes into the vector (as though all its buffers were contiguous). - * If @offset is non-NULL, update it to the offset of that byte within - * @iov[I] (guaranteed to be less than @iov[I].iov_len) If the whole - * vector has <= @skip bytes, return @n. + * Return: index of iovec array containing the @skip byte counted as if buffers + * were contiguous. If iovec has less than @skip bytes, return @n. */ size_t iov_skip_bytes(const struct iovec *iov, size_t n, size_t skip, size_t *offset) @@ -57,17 +54,14 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n, } /** - * iov_from_buf() - Copy data from a buffer to an I/O vector (struct iovec) - * efficiently. - * - * @iov: Pointer to the array of struct iovec describing the - * scatter/gather I/O vector. - * @iov_cnt: Number of elements in the iov array. - * @offset: Byte offset in the iov array where copying should start. - * @buf: Pointer to the source buffer containing the data to copy. - * @bytes: Total number of bytes to copy from buf to iov. + * iov_from_buf() - Copy from flat buffer to iovec array + * @iov: Destination iovec array + * @iov_cnt: Number of elements in the iovec array + * @offset: Destination offset in iovec array + * @buf: Source buffer + * @bytes: Bytes to copy * - * Return: the number of bytes successfully copied. + * Return: number of bytes copied */ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, size_t offset, const void *buf, size_t bytes) @@ -78,12 +72,12 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, 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); + return bytes; } i = iov_skip_bytes(iov, iov_cnt, offset, &offset); - /* copying data */ for (copied = 0; copied < bytes && i < iov_cnt; i++) { size_t len = MIN(iov[i].iov_len - offset, bytes - copied); @@ -97,17 +91,14 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, } /** - * iov_to_buf() - Copy data from a scatter/gather I/O vector (struct iovec) to - * a buffer efficiently. - * - * @iov: Pointer to the array of struct iovec describing the scatter/gather - * I/O vector. - * @iov_cnt: Number of elements in the iov array. - * @offset: Offset within the first element of iov from where copying should start. - * @buf: Pointer to the destination buffer where data will be copied. - * @bytes: Total number of bytes to copy from iov to buf. + * iov_to_buf() - Copy from iovec to flat buffer + * @iov: Source iovec array + * @iov_cnt: Number of elements in iovec array + * @offset: Source offset altogether, counted in flattened iovec + * @buf: Destination buffer + * @bytes: Bytes to copy * - * Return: the number of bytes successfully copied. + * 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) @@ -118,15 +109,17 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, 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); + return bytes; } i = iov_skip_bytes(iov, iov_cnt, offset, &offset); - /* 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; @@ -137,14 +130,11 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, } /** - * iov_size() - Calculate the total size of a scatter/gather I/O vector - * (struct iovec). + * iov_size() - Calculate total data size of iovec + * @iov: Source iovec array + * @iov_cnt: Number of elements in iovec array * - * @iov: Pointer to the array of struct iovec describing the - * scatter/gather I/O vector. - * @iov_cnt: Number of elements in the iov array. - * - * Return: the total size in bytes. + * Return: total size in bytes */ size_t iov_size(const struct iovec *iov, size_t iov_cnt) { -- 2.43.0

On 11/3/25 11:16, Stefano Brivio wrote:

...instead of copying, from the input buffer, the amount of available bytes in the buffer provided via vhost-user.

The existing behaviour should be harmless due to the fact that we don't request overly large buffers from vhost-user, but it doesn't look correct to me.

I don't understand where is the problem. I think the existing behaviour is correct because the iov array is padded to size in vu_collect() by: if (iov->iov_len > size - current_size) iov->iov_len = size - current_size; total can be lesser than size but in this case we exit. So total should be equal to input size + sizeof(struct virtio_net_hdr_mrg_rxbuf). And in iov_from_buf() we copy the content of the buffer according the available room in the iovec and the size of the input buffer. Thanks, Laurent>

Signed-off-by: Stefano Brivio --- vu_common.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/vu_common.c b/vu_common.c index b13b7c3..21cfb2a 100644 --- a/vu_common.c +++ b/vu_common.c @@ -238,7 +238,7 @@ void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref, * vu_send_single() - Send a buffer to the front-end using the RX virtqueue * @c: execution context * @buf: address of the buffer - * @size: size of the buffer + * @size: size of the input buffer * * Return: number of bytes sent, -1 if there is an error */ @@ -248,7 +248,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size) struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE]; struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE]; struct iovec in_sg[VIRTQUEUE_MAX_SIZE]; - size_t total; + size_t vu_buf_size; int elem_cnt; int i;

@@ -261,21 +261,20 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)

vu_init_elem(elem, in_sg, VIRTQUEUE_MAX_SIZE);

- size += sizeof(struct virtio_net_hdr_mrg_rxbuf); - elem_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, size, &total); - if (total < size) { + elem_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, + size + sizeof(struct virtio_net_hdr_mrg_rxbuf), + &vu_buf_size); + if (vu_buf_size < size + sizeof(struct virtio_net_hdr_mrg_rxbuf)) { debug("vu_send_single: no space to send the data " - "elem_cnt %d size %zd", elem_cnt, total); + "elem_cnt %d size %zd", elem_cnt, size); goto err; }

vu_set_vnethdr(vdev, in_sg[0].iov_base, elem_cnt);

- total -= sizeof(struct virtio_net_hdr_mrg_rxbuf); - /* copy data from the buffer to the iovec */ iov_from_buf(in_sg, elem_cnt, sizeof(struct virtio_net_hdr_mrg_rxbuf), - buf, total); + buf, size);

if (*c->pcap) { pcap_iov(in_sg, elem_cnt, @@ -284,9 +283,10 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)

vu_flush(vdev, vq, elem, elem_cnt);

- trace("vhost-user sent %zu", total); + trace("vhost-user sent %zu", + vu_buf_size - sizeof(struct virtio_net_hdr_mrg_rxbuf));

- return total; + return size; err: for (i = 0; i < elem_cnt; i++) vu_queue_detach_element(vq);

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