[PATCH v2 0/3] Decouple iovec management from virtqueue elements
This series prepares the vhost-user path for multi-buffer support, where a single virtqueue element can use more than one iovec entry. Currently, iovec arrays are tightly coupled to virtqueue elements: callers must pre-initialize each element's in_sg/out_sg pointers before calling vu_queue_pop(), and each element is assumed to own exactly one iovec slot. This makes it impossible for a single element to span multiple iovec entries, which is needed for UDP multi-buffer reception. The series decouples iovec storage from elements in three patches: - Patch 1 passes iovec arrays as separate parameters to vu_queue_pop() and vu_queue_map_desc(), so the caller controls where descriptors are mapped rather than reading them from pre-initialized element fields. - Patch 2 passes the actual remaining out_sg capacity to vu_queue_pop() in vu_handle_tx() instead of a fixed per-element constant, enabling dynamic iovec allocation. - Patch 3 moves iovec pool management into vu_collect(), which now accepts the iovec array and tracks consumed entries across elements with a running counter. This removes vu_set_element() and vu_init_elem() entirely. Callers that still assume one iovec per element assert this invariant explicitly until they are updated for multi-buffer. The follow-up udp-iov_vu series builds on this to implement actual multi-buffer support in the UDP vhost-user path. v2: - in patch 3, use iov_used in iov_truncate() rather than elem_cnt as vu_collect() is now providing the number of iovec collected. Laurent Vivier (3): virtio: Pass iovec arrays as separate parameters to vu_queue_pop() vu_handle_tx: Pass actual remaining out_sg capacity to vu_queue_pop() vu_common: Move iovec management into vu_collect() tcp_vu.c | 25 +++++++++------- udp_vu.c | 21 ++++++++------ virtio.c | 29 ++++++++++++++----- virtio.h | 4 ++- vu_common.c | 83 ++++++++++++++++++++++++----------------------------- vu_common.h | 22 ++------------ 6 files changed, 92 insertions(+), 92 deletions(-) -- 2.53.0
Currently vu_queue_pop() and vu_queue_map_desc() read the iovec arrays
(in_sg/out_sg) and their sizes (in_num/out_num) from the vu_virtq_element
struct. This couples the iovec storage to the element, requiring callers
like vu_handle_tx() to pre-initialize the element fields before calling
vu_queue_pop().
Pass the iovec arrays and their maximum sizes as separate parameters
instead. vu_queue_map_desc() now writes the actual descriptor count
and iovec pointers back into the element after mapping, rather than
using the element as both input and output.
This decouples the iovec storage from the element, which is a
prerequisite for multi-buffer support where a single frame can span
multiple virtqueue elements sharing a common iovec pool.
No functional change.
Signed-off-by: Laurent Vivier
In vu_handle_tx(), pass the actual remaining iovec capacity
(ARRAY_SIZE(out_sg) - out_sg_count) to vu_queue_pop() rather than a
fixed VU_MAX_TX_BUFFER_NB.
This enables dynamic allocation of iovec entries to each element rather
than reserving a fixed number of slots per descriptor.
Signed-off-by: Laurent Vivier
Previously, callers had to pre-initialize virtqueue elements with iovec
entries using vu_set_element() or vu_init_elem() before calling
vu_collect(). This meant each element owned a fixed, pre-assigned iovec
slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg,
max_in_sg, and in_num), letting it pass the remaining iovec capacity
directly to vu_queue_pop(). A running current_iov counter tracks
consumed entries across elements, so multiple elements share a single
iovec pool. The optional in_num output parameter reports how many iovec
entries were consumed, allowing callers to track usage across multiple
vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer
needed, and is a prerequisite for multi-buffer support where a single
virtqueue element can use more than one iovec entry. For now, callers
assert the current single-iovec-per-element invariant until they are
updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
On Fri, Mar 13, 2026 at 07:26:16PM +0100, Laurent Vivier wrote:
Currently vu_queue_pop() and vu_queue_map_desc() read the iovec arrays (in_sg/out_sg) and their sizes (in_num/out_num) from the vu_virtq_element struct. This couples the iovec storage to the element, requiring callers like vu_handle_tx() to pre-initialize the element fields before calling vu_queue_pop().
Pass the iovec arrays and their maximum sizes as separate parameters instead. vu_queue_map_desc() now writes the actual descriptor count and iovec pointers back into the element after mapping, rather than using the element as both input and output.
This decouples the iovec storage from the element, which is a prerequisite for multi-buffer support where a single frame can span multiple virtqueue elements sharing a common iovec pool.
No functional change.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
--- virtio.c | 29 ++++++++++++++++++++++------- virtio.h | 4 +++- vu_common.c | 14 +++++++------- 3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/virtio.c b/virtio.c index 447137ee83dd..a671163c27a0 100644 --- a/virtio.c +++ b/virtio.c @@ -428,12 +428,18 @@ static bool virtqueue_map_desc(const struct vu_dev *dev, * @vq: Virtqueue * @idx: First descriptor ring entry to map * @elem: Virtqueue element to store descriptor ring iov + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @out_sg: Outgoing iovec array for device-readable descriptors + * @max_out_sg: Maximum number of entries in @out_sg * * Return: -1 if there is an error, 0 otherwise */ static int vu_queue_map_desc(const struct vu_dev *dev, struct vu_virtq *vq, unsigned int idx, - struct vu_virtq_element *elem) + struct vu_virtq_element *elem, + struct iovec *in_sg, size_t max_in_sg, + struct iovec *out_sg, size_t max_out_sg) { const struct vring_desc *desc = vq->vring.desc; struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE]; @@ -470,16 +476,16 @@ static int vu_queue_map_desc(const struct vu_dev *dev, /* Collect all the descriptors */ do { if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) { - if (!virtqueue_map_desc(dev, &in_num, elem->in_sg, - elem->in_num, + if (!virtqueue_map_desc(dev, &in_num, in_sg, + max_in_sg, le64toh(desc[i].addr), le32toh(desc[i].len))) return -1; } else { if (in_num) die("Incorrect order for descriptors"); - if (!virtqueue_map_desc(dev, &out_num, elem->out_sg, - elem->out_num, + if (!virtqueue_map_desc(dev, &out_num, out_sg, + max_out_sg, le64toh(desc[i].addr), le32toh(desc[i].len))) { return -1; @@ -496,7 +502,9 @@ static int vu_queue_map_desc(const struct vu_dev *dev, die("vhost-user: Failed to read descriptor list");
elem->index = idx; + elem->in_sg = in_sg; elem->in_num = in_num; + elem->out_sg = out_sg; elem->out_num = out_num;
return 0; @@ -507,11 +515,17 @@ static int vu_queue_map_desc(const struct vu_dev *dev, * @dev: Vhost-user device * @vq: Virtqueue * @elem: Virtqueue element to fill with the entry information + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @out_sg: Outgoing iovec array for device-readable descriptors + * @max_out_sg: Maximum number of entries in @out_sg * * Return: -1 if there is an error, 0 otherwise */ int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq, - struct vu_virtq_element *elem) + struct vu_virtq_element *elem, + struct iovec *in_sg, size_t max_in_sg, + struct iovec *out_sg, size_t max_out_sg) { unsigned int head; int ret; @@ -535,7 +549,8 @@ int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq, if (vu_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) vring_set_avail_event(vq, vq->last_avail_idx);
- ret = vu_queue_map_desc(dev, vq, head, elem); + ret = vu_queue_map_desc(dev, vq, head, elem, in_sg, max_in_sg, + out_sg, max_out_sg);
if (ret < 0) return ret; diff --git a/virtio.h b/virtio.h index d04bbe84e5c4..c7e447d59860 100644 --- a/virtio.h +++ b/virtio.h @@ -188,7 +188,9 @@ static inline bool vu_has_protocol_feature(const struct vu_dev *vdev,
void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq); int vu_queue_pop(const struct vu_dev *dev, struct vu_virtq *vq, - struct vu_virtq_element *elem); + struct vu_virtq_element *elem, + struct iovec *in_sg, size_t max_in_sg, + struct iovec *out_sg, size_t max_out_sg); void vu_queue_detach_element(struct vu_virtq *vq); void vu_queue_unpop(struct vu_virtq *vq); bool vu_queue_rewind(struct vu_virtq *vq, unsigned int num); diff --git a/vu_common.c b/vu_common.c index 5f2ce18e5b71..4d809ac38a4b 100644 --- a/vu_common.c +++ b/vu_common.c @@ -91,7 +91,11 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct iovec *iov; int ret;
- ret = vu_queue_pop(vdev, vq, &elem[elem_cnt]); + ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], + elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + elem[elem_cnt].out_sg, + elem[elem_cnt].out_num); if (ret < 0) break;
@@ -178,12 +182,8 @@ static void vu_handle_tx(struct vu_dev *vdev, int index, int ret; struct iov_tail data;
- elem[count].out_num = VU_MAX_TX_BUFFER_NB; - elem[count].out_sg = &out_sg[out_sg_count]; - elem[count].in_num = 0; - elem[count].in_sg = NULL; - - ret = vu_queue_pop(vdev, vq, &elem[count]); + ret = vu_queue_pop(vdev, vq, &elem[count], NULL, 0, + &out_sg[out_sg_count], VU_MAX_TX_BUFFER_NB); if (ret < 0) break; out_sg_count += elem[count].out_num; -- 2.53.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
On Fri, Mar 13, 2026 at 07:26:17PM +0100, Laurent Vivier wrote:
In vu_handle_tx(), pass the actual remaining iovec capacity (ARRAY_SIZE(out_sg) - out_sg_count) to vu_queue_pop() rather than a fixed VU_MAX_TX_BUFFER_NB.
This enables dynamic allocation of iovec entries to each element rather than reserving a fixed number of slots per descriptor.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
--- vu_common.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/vu_common.c b/vu_common.c index 4d809ac38a4b..ed0033d6bb11 100644 --- a/vu_common.c +++ b/vu_common.c @@ -20,8 +20,6 @@ #include "migrate.h" #include "epoll_ctl.h"
-#define VU_MAX_TX_BUFFER_NB 2 - /** * vu_packet_check_range() - Check if a given memory zone is contained in * a mapped guest memory region @@ -177,13 +175,14 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
count = 0; out_sg_count = 0; - while (count < VIRTQUEUE_MAX_SIZE && - out_sg_count + VU_MAX_TX_BUFFER_NB <= VIRTQUEUE_MAX_SIZE) { - int ret; + while (count < ARRAY_SIZE(elem) && + out_sg_count < ARRAY_SIZE(out_sg)) { struct iov_tail data; + int ret;
ret = vu_queue_pop(vdev, vq, &elem[count], NULL, 0, - &out_sg[out_sg_count], VU_MAX_TX_BUFFER_NB); + &out_sg[out_sg_count], + ARRAY_SIZE(out_sg) - out_sg_count); if (ret < 0) break; out_sg_count += elem[count].out_num; -- 2.53.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
On Fri, 13 Mar 2026 19:26:17 +0100
Laurent Vivier
In vu_handle_tx(), pass the actual remaining iovec capacity (ARRAY_SIZE(out_sg) - out_sg_count) to vu_queue_pop() rather than a fixed VU_MAX_TX_BUFFER_NB.
This enables dynamic allocation of iovec entries to each element rather than reserving a fixed number of slots per descriptor.
Signed-off-by: Laurent Vivier
--- vu_common.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/vu_common.c b/vu_common.c index 4d809ac38a4b..ed0033d6bb11 100644 --- a/vu_common.c +++ b/vu_common.c @@ -20,8 +20,6 @@ #include "migrate.h" #include "epoll_ctl.h"
-#define VU_MAX_TX_BUFFER_NB 2 - /** * vu_packet_check_range() - Check if a given memory zone is contained in * a mapped guest memory region @@ -177,13 +175,14 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
count = 0; out_sg_count = 0; - while (count < VIRTQUEUE_MAX_SIZE && - out_sg_count + VU_MAX_TX_BUFFER_NB <= VIRTQUEUE_MAX_SIZE) { - int ret; + while (count < ARRAY_SIZE(elem) && + out_sg_count < ARRAY_SIZE(out_sg)) {
Nit: you could do this now: while (count < ARRAY_SIZE(elem) && out_sg_count < ARRAY_SIZE(out_sg)) { I still need a bit of time to review 3/3, if you don't need to re-spin for other reasons I can also fix this up on merge.
struct iov_tail data; + int ret;
ret = vu_queue_pop(vdev, vq, &elem[count], NULL, 0, - &out_sg[out_sg_count], VU_MAX_TX_BUFFER_NB); + &out_sg[out_sg_count], + ARRAY_SIZE(out_sg) - out_sg_count); if (ret < 0) break; out_sg_count += elem[count].out_num;
-- Stefano
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0;
- while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret;
ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break;
@@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; }
- iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
- current_size += iov->iov_len; + current_size += iov_size(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num); + current_iov += elem[elem_cnt].in_num; elem_cnt++;
if (!vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) break; }
+ if (in_num) + *in_num = current_iov; + if (collected) *collected = current_size;
@@ -147,8 +139,11 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, { int i;
- for (i = 0; i < elem_cnt; i++) - vu_queue_fill(vdev, vq, &elem[i], elem[i].in_sg[0].iov_len, i); + for (i = 0; i < elem_cnt; i++) { + size_t elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
IIUC, the elem structure itself isn't shared with vhost, so we can alter it. Would it make sense to cache the number of bytes allocated to the element there, to avoid repeated calls to iov_size()?
+ + vu_queue_fill(vdev, vq, &elem[i], elem_size, i); + }
vu_queue_flush(vdev, vq, elem_cnt); vu_queue_notify(vdev, vq); @@ -246,7 +241,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 total, in_num; int elem_cnt; int i;
@@ -257,11 +252,10 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size) return -1; }
- vu_init_elem(elem, in_sg, VIRTQUEUE_MAX_SIZE); - size += VNET_HLEN; - elem_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, size, &total); - if (total < size) { + elem_cnt = vu_collect(vdev, vq, elem, ARRAY_SIZE(elem), in_sg, + ARRAY_SIZE(in_sg), &in_num, size, &total); + if (elem_cnt == 0 || total < size) { debug("vu_send_single: no space to send the data " "elem_cnt %d size %zd", elem_cnt, total); goto err; @@ -272,10 +266,10 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size) total -= VNET_HLEN;
/* copy data from the buffer to the iovec */ - iov_from_buf(in_sg, elem_cnt, VNET_HLEN, buf, total); + iov_from_buf(in_sg, in_num, VNET_HLEN, buf, total);
if (*c->pcap) - pcap_iov(in_sg, elem_cnt, VNET_HLEN); + pcap_iov(in_sg, in_num, VNET_HLEN);
vu_flush(vdev, vq, elem, elem_cnt);
diff --git a/vu_common.h b/vu_common.h index 865d9771fa89..6c31630e8712 100644 --- a/vu_common.h +++ b/vu_common.h @@ -35,26 +35,10 @@ static inline void *vu_payloadv6(void *base) return (struct ipv6hdr *)vu_ip(base) + 1; }
-/** - * vu_set_element() - Initialize a vu_virtq_element - * @elem: Element to initialize - * @out_sg: One out iovec entry to set in elem - * @in_sg: One in iovec entry to set in elem - */ -static inline void vu_set_element(struct vu_virtq_element *elem, - struct iovec *out_sg, struct iovec *in_sg) -{ - elem->out_num = !!out_sg; - elem->out_sg = out_sg; - elem->in_num = !!in_sg; - elem->in_sg = in_sg; -} - -void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, - int elem_cnt); int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, - struct vu_virtq_element *elem, int max_elem, size_t size, - size_t *collected); + struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, + size_t size, size_t *collected); void vu_set_vnethdr(struct virtio_net_hdr_mrg_rxbuf *vnethdr, int num_buffers); void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int elem_cnt); -- 2.53.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
On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0;
- while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret;
ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break;
@@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; }
- iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()? @in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
- current_size += iov->iov_len; + current_size += iov_size(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num); + current_iov += elem[elem_cnt].in_num; elem_cnt++;
if (!vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) break; }
+ if (in_num) + *in_num = current_iov; + if (collected) *collected = current_size;
@@ -147,8 +139,11 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, { int i;
- for (i = 0; i < elem_cnt; i++) - vu_queue_fill(vdev, vq, &elem[i], elem[i].in_sg[0].iov_len, i); + for (i = 0; i < elem_cnt; i++) { + size_t elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
IIUC, the elem structure itself isn't shared with vhost, so we can alter it. Would it make sense to cache the number of bytes allocated to the element there, to avoid repeated calls to iov_size()?
It's possible. But I think it could be complicated to keep in sync the actual size of the iovec array and the value we store in elem, as we alter the array at several points. Thanks, Laurent
+ + vu_queue_fill(vdev, vq, &elem[i], elem_size, i); + }
vu_queue_flush(vdev, vq, elem_cnt); vu_queue_notify(vdev, vq); @@ -246,7 +241,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 total, in_num; int elem_cnt; int i;
@@ -257,11 +252,10 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size) return -1; }
- vu_init_elem(elem, in_sg, VIRTQUEUE_MAX_SIZE); - size += VNET_HLEN; - elem_cnt = vu_collect(vdev, vq, elem, VIRTQUEUE_MAX_SIZE, size, &total); - if (total < size) { + elem_cnt = vu_collect(vdev, vq, elem, ARRAY_SIZE(elem), in_sg, + ARRAY_SIZE(in_sg), &in_num, size, &total); + if (elem_cnt == 0 || total < size) { debug("vu_send_single: no space to send the data " "elem_cnt %d size %zd", elem_cnt, total); goto err; @@ -272,10 +266,10 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size) total -= VNET_HLEN;
/* copy data from the buffer to the iovec */ - iov_from_buf(in_sg, elem_cnt, VNET_HLEN, buf, total); + iov_from_buf(in_sg, in_num, VNET_HLEN, buf, total);
if (*c->pcap) - pcap_iov(in_sg, elem_cnt, VNET_HLEN); + pcap_iov(in_sg, in_num, VNET_HLEN);
vu_flush(vdev, vq, elem, elem_cnt);
diff --git a/vu_common.h b/vu_common.h index 865d9771fa89..6c31630e8712 100644 --- a/vu_common.h +++ b/vu_common.h @@ -35,26 +35,10 @@ static inline void *vu_payloadv6(void *base) return (struct ipv6hdr *)vu_ip(base) + 1; }
-/** - * vu_set_element() - Initialize a vu_virtq_element - * @elem: Element to initialize - * @out_sg: One out iovec entry to set in elem - * @in_sg: One in iovec entry to set in elem - */ -static inline void vu_set_element(struct vu_virtq_element *elem, - struct iovec *out_sg, struct iovec *in_sg) -{ - elem->out_num = !!out_sg; - elem->out_sg = out_sg; - elem->in_num = !!in_sg; - elem->in_sg = in_sg; -} - -void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, - int elem_cnt); int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, - struct vu_virtq_element *elem, int max_elem, size_t size, - size_t *collected); + struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, + size_t size, size_t *collected); void vu_set_vnethdr(struct virtio_net_hdr_mrg_rxbuf *vnethdr, int num_buffers); void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int elem_cnt); -- 2.53.0
On Tue, 17 Mar 2026 08:25:49 +0100
Laurent Vivier
On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0;
- while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret;
ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break;
@@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; }
- iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out. Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need? What if we rename it to @in_used or @in_collected? -- Stefano
On Fri, 13 Mar 2026 19:26:18 +0100
Laurent Vivier
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
This looks fine to me other than the comment about @in_num comment (not sure if it makes sense) and pending clarification with David regarding that argument to vu_collect(), so once that's sorted, if you don't want further changes, I would apply it like it is. But I have a question, just to be sure:
--- tcp_vu.c | 25 +++++++++++--------- udp_vu.c | 21 ++++++++++------- vu_common.c | 68 ++++++++++++++++++++++++----------------------------- vu_common.h | 22 +++-------------- 4 files changed, 60 insertions(+), 76 deletions(-)
diff --git a/tcp_vu.c b/tcp_vu.c index fd734e857b3b..d470ab54bcea 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -87,13 +87,13 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
hdrlen = tcp_vu_hdrlen(CONN_V6(conn));
- vu_set_element(&flags_elem[0], NULL, &flags_iov[0]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[0], 1, + &flags_iov[0], 1, NULL, MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN), NULL); if (elem_cnt != 1) return -1;
+ ASSERT(flags_elem[0].in_num == 1); ASSERT(flags_elem[0].in_sg[0].iov_len >= MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN));
@@ -148,9 +148,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) nb_ack = 1;
if (flags & DUP_ACK) { - vu_set_element(&flags_elem[1], NULL, &flags_iov[1]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[1], 1, + &flags_iov[1], 1, NULL, flags_elem[0].in_sg[0].iov_len, NULL); if (elem_cnt == 1 && flags_elem[1].in_sg[0].iov_len >= @@ -191,8 +190,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, const struct vu_dev *vdev = c->vdev; struct msghdr mh_sock = { 0 }; uint16_t mss = MSS_GET(conn); + size_t hdrlen, iov_used; int s = conn->sock; - size_t hdrlen; int elem_cnt; ssize_t ret; int i; @@ -201,22 +200,26 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
hdrlen = tcp_vu_hdrlen(v6);
- vu_init_elem(elem, &iov_vu[DISCARD_IOV_NUM], VIRTQUEUE_MAX_SIZE); - + iov_used = 0; elem_cnt = 0; *head_cnt = 0; - while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE) { + while (fillsize > 0 && elem_cnt < ARRAY_SIZE(elem) && + iov_used < VIRTQUEUE_MAX_SIZE) { + size_t frame_size, dlen, in_num; struct iovec *iov; - size_t frame_size, dlen; int cnt;
cnt = vu_collect(vdev, vq, &elem[elem_cnt], - VIRTQUEUE_MAX_SIZE - elem_cnt, + ARRAY_SIZE(elem) - elem_cnt, + &iov_vu[DISCARD_IOV_NUM + iov_used], + VIRTQUEUE_MAX_SIZE - iov_used, &in_num, MAX(MIN(mss, fillsize) + hdrlen, ETH_ZLEN + VNET_HLEN), &frame_size); if (cnt == 0) break; + ASSERT((size_t)cnt == in_num); /* one iovec per element */
...this (and the UDP equivalent) will trigger if there are multiple iovecs per element, until the point the next pending series deals with it. My assumption is that, if there multiple iovecs per element, we crash in the way you reported in https://bugs.passt.top/show_bug.cgi?id=196 anyway, so triggering this assertion here doesn't make it any worse for the moment. Is my assumption correct, or do we risk adding additional cases where we crash meanwhile? If we do, maybe it would be better to only merge this patch with the next series. -- Stefano
On 3/17/26 16:23, Stefano Brivio wrote:
On Fri, 13 Mar 2026 19:26:18 +0100 Laurent Vivier
wrote: Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
This looks fine to me other than the comment about @in_num comment (not sure if it makes sense) and pending clarification with David regarding that argument to vu_collect(), so once that's sorted, if you don't want further changes, I would apply it like it is. But I have a question, just to be sure:
--- tcp_vu.c | 25 +++++++++++--------- udp_vu.c | 21 ++++++++++------- vu_common.c | 68 ++++++++++++++++++++++++----------------------------- vu_common.h | 22 +++-------------- 4 files changed, 60 insertions(+), 76 deletions(-)
diff --git a/tcp_vu.c b/tcp_vu.c index fd734e857b3b..d470ab54bcea 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -87,13 +87,13 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
hdrlen = tcp_vu_hdrlen(CONN_V6(conn));
- vu_set_element(&flags_elem[0], NULL, &flags_iov[0]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[0], 1, + &flags_iov[0], 1, NULL, MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN), NULL); if (elem_cnt != 1) return -1;
+ ASSERT(flags_elem[0].in_num == 1); ASSERT(flags_elem[0].in_sg[0].iov_len >= MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN));
@@ -148,9 +148,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) nb_ack = 1;
if (flags & DUP_ACK) { - vu_set_element(&flags_elem[1], NULL, &flags_iov[1]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[1], 1, + &flags_iov[1], 1, NULL, flags_elem[0].in_sg[0].iov_len, NULL); if (elem_cnt == 1 && flags_elem[1].in_sg[0].iov_len >= @@ -191,8 +190,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, const struct vu_dev *vdev = c->vdev; struct msghdr mh_sock = { 0 }; uint16_t mss = MSS_GET(conn); + size_t hdrlen, iov_used; int s = conn->sock; - size_t hdrlen; int elem_cnt; ssize_t ret; int i; @@ -201,22 +200,26 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
hdrlen = tcp_vu_hdrlen(v6);
- vu_init_elem(elem, &iov_vu[DISCARD_IOV_NUM], VIRTQUEUE_MAX_SIZE); - + iov_used = 0; elem_cnt = 0; *head_cnt = 0; - while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE) { + while (fillsize > 0 && elem_cnt < ARRAY_SIZE(elem) && + iov_used < VIRTQUEUE_MAX_SIZE) { + size_t frame_size, dlen, in_num; struct iovec *iov; - size_t frame_size, dlen; int cnt;
cnt = vu_collect(vdev, vq, &elem[elem_cnt], - VIRTQUEUE_MAX_SIZE - elem_cnt, + ARRAY_SIZE(elem) - elem_cnt, + &iov_vu[DISCARD_IOV_NUM + iov_used], + VIRTQUEUE_MAX_SIZE - iov_used, &in_num, MAX(MIN(mss, fillsize) + hdrlen, ETH_ZLEN + VNET_HLEN), &frame_size); if (cnt == 0) break; + ASSERT((size_t)cnt == in_num); /* one iovec per element */
...this (and the UDP equivalent) will trigger if there are multiple iovecs per element, until the point the next pending series deals with it.
My assumption is that, if there multiple iovecs per element, we crash in the way you reported in https://bugs.passt.top/show_bug.cgi?id=196 anyway, so triggering this assertion here doesn't make it any worse for the moment.
Is my assumption correct, or do we risk adding additional cases where we crash meanwhile? If we do, maybe it would be better to only merge this patch with the next series.
You're right. The idea is to mark clearly that udp_vu.c and tcp_vu.c only manage 1 iovec per element. But they can be removed from this series if you prefer. Thanks, Laurent
On Tue, 17 Mar 2026 17:18:58 +0100
Laurent Vivier
On 3/17/26 16:23, Stefano Brivio wrote:
On Fri, 13 Mar 2026 19:26:18 +0100 Laurent Vivier
wrote: Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
This looks fine to me other than the comment about @in_num comment (not sure if it makes sense) and pending clarification with David regarding that argument to vu_collect(), so once that's sorted, if you don't want further changes, I would apply it like it is. But I have a question, just to be sure:
--- tcp_vu.c | 25 +++++++++++--------- udp_vu.c | 21 ++++++++++------- vu_common.c | 68 ++++++++++++++++++++++++----------------------------- vu_common.h | 22 +++-------------- 4 files changed, 60 insertions(+), 76 deletions(-)
diff --git a/tcp_vu.c b/tcp_vu.c index fd734e857b3b..d470ab54bcea 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -87,13 +87,13 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
hdrlen = tcp_vu_hdrlen(CONN_V6(conn));
- vu_set_element(&flags_elem[0], NULL, &flags_iov[0]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[0], 1, + &flags_iov[0], 1, NULL, MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN), NULL); if (elem_cnt != 1) return -1;
+ ASSERT(flags_elem[0].in_num == 1); ASSERT(flags_elem[0].in_sg[0].iov_len >= MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN));
@@ -148,9 +148,8 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) nb_ack = 1;
if (flags & DUP_ACK) { - vu_set_element(&flags_elem[1], NULL, &flags_iov[1]); - elem_cnt = vu_collect(vdev, vq, &flags_elem[1], 1, + &flags_iov[1], 1, NULL, flags_elem[0].in_sg[0].iov_len, NULL); if (elem_cnt == 1 && flags_elem[1].in_sg[0].iov_len >= @@ -191,8 +190,8 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, const struct vu_dev *vdev = c->vdev; struct msghdr mh_sock = { 0 }; uint16_t mss = MSS_GET(conn); + size_t hdrlen, iov_used; int s = conn->sock; - size_t hdrlen; int elem_cnt; ssize_t ret; int i; @@ -201,22 +200,26 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
hdrlen = tcp_vu_hdrlen(v6);
- vu_init_elem(elem, &iov_vu[DISCARD_IOV_NUM], VIRTQUEUE_MAX_SIZE); - + iov_used = 0; elem_cnt = 0; *head_cnt = 0; - while (fillsize > 0 && elem_cnt < VIRTQUEUE_MAX_SIZE) { + while (fillsize > 0 && elem_cnt < ARRAY_SIZE(elem) && + iov_used < VIRTQUEUE_MAX_SIZE) { + size_t frame_size, dlen, in_num; struct iovec *iov; - size_t frame_size, dlen; int cnt;
cnt = vu_collect(vdev, vq, &elem[elem_cnt], - VIRTQUEUE_MAX_SIZE - elem_cnt, + ARRAY_SIZE(elem) - elem_cnt, + &iov_vu[DISCARD_IOV_NUM + iov_used], + VIRTQUEUE_MAX_SIZE - iov_used, &in_num, MAX(MIN(mss, fillsize) + hdrlen, ETH_ZLEN + VNET_HLEN), &frame_size); if (cnt == 0) break; + ASSERT((size_t)cnt == in_num); /* one iovec per element */
...this (and the UDP equivalent) will trigger if there are multiple iovecs per element, until the point the next pending series deals with it.
My assumption is that, if there multiple iovecs per element, we crash in the way you reported in https://bugs.passt.top/show_bug.cgi?id=196 anyway, so triggering this assertion here doesn't make it any worse for the moment.
Is my assumption correct, or do we risk adding additional cases where we crash meanwhile? If we do, maybe it would be better to only merge this patch with the next series.
You're right. The idea is to mark clearly that udp_vu.c and tcp_vu.c only manage 1 iovec per element. But they can be removed from this series if you prefer.
No, no, then it's convenient! It's also more convenient for any debugging meanwhile to have things crash right there, I suppose. -- Stefano
On 3/17/26 16:23, Stefano Brivio wrote:
On Tue, 17 Mar 2026 08:25:49 +0100 Laurent Vivier
wrote: On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0;
- while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret;
ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break;
@@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; }
- iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out.
Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need?
For an element, *ìn_*num is the number of *in_*sg we have read from the ring for an element. It's virtio semantic, so *in_* means sg going *in* the guest. For *out_*sg we have *out_*num.
What if we rename it to @in_used or @in_collected?
The idea was to keep the same name as in the element. But we can change this to @in_used. Thanks, Laurent
On Tue, Mar 17, 2026 at 08:25:49AM +0100, Laurent Vivier wrote:
On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0; - while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret; ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break; @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; } - iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
Oh, right, sorry. I'm getting confused again by the two-level heirarchy - this gathers multiple elems as well as multiple iovs.
- current_size += iov->iov_len; + current_size += iov_size(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num); + current_iov += elem[elem_cnt].in_num; elem_cnt++; if (!vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) break; } + if (in_num) + *in_num = current_iov; + if (collected) *collected = current_size; @@ -147,8 +139,11 @@ void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, { int i; - for (i = 0; i < elem_cnt; i++) - vu_queue_fill(vdev, vq, &elem[i], elem[i].in_sg[0].iov_len, i); + for (i = 0; i < elem_cnt; i++) { + size_t elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
IIUC, the elem structure itself isn't shared with vhost, so we can alter it. Would it make sense to cache the number of bytes allocated to the element there, to avoid repeated calls to iov_size()?
It's possible. But I think it could be complicated to keep in sync the actual size of the iovec array and the value we store in elem, as we alter the array at several points.
Ok. We do expect the iovs to be pretty short in practice, so iov_size() shouldn't be too expensive. -- 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
On Tue, Mar 17, 2026 at 05:30:32PM +0100, Laurent Vivier wrote:
On 3/17/26 16:23, Stefano Brivio wrote:
On Tue, 17 Mar 2026 08:25:49 +0100 Laurent Vivier
wrote: On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0; - while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret; ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break; @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; } - iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out.
Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need?
For an element, *ìn_*num is the number of *in_*sg we have read from the ring for an element.
It's virtio semantic, so *in_* means sg going *in* the guest.
For *out_*sg we have *out_*num.
What if we rename it to @in_used or @in_collected?
The idea was to keep the same name as in the element. But we can change this to @in_used.
Would "in_total" work better to suggest that it's the sum of all the elements' in_num? -- 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
On 3/18/26 02:16, David Gibson wrote:
On Tue, Mar 17, 2026 at 05:30:32PM +0100, Laurent Vivier wrote:
On 3/17/26 16:23, Stefano Brivio wrote:
On Tue, 17 Mar 2026 08:25:49 +0100 Laurent Vivier
wrote: On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote:
Previously, callers had to pre-initialize virtqueue elements with iovec entries using vu_set_element() or vu_init_elem() before calling vu_collect(). This meant each element owned a fixed, pre-assigned iovec slot.
Move the iovec array into vu_collect() as explicit parameters (in_sg, max_in_sg, and in_num), letting it pass the remaining iovec capacity directly to vu_queue_pop(). A running current_iov counter tracks consumed entries across elements, so multiple elements share a single iovec pool. The optional in_num output parameter reports how many iovec entries were consumed, allowing callers to track usage across multiple vu_collect() calls.
This removes vu_set_element() and vu_init_elem() which are no longer needed, and is a prerequisite for multi-buffer support where a single virtqueue element can use more than one iovec entry. For now, callers assert the current single-iovec-per-element invariant until they are updated to handle multiple iovecs.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip]
/** * vu_collect() - collect virtio buffers from a given virtqueue * @vdev: vhost-user device * @vq: virtqueue to collect from - * @elem: Array of virtqueue element - * each element must be initialized with one iovec entry - * in the in_sg array. + * @elem: Array of @max_elem virtqueue elements * @max_elem: Number of virtqueue elements in the array + * @in_sg: Incoming iovec array for device-writable descriptors + * @max_in_sg: Maximum number of entries in @in_sg + * @in_num: Number of collected entries from @in_sg (output) * @size: Maximum size of the data in the frame * @collected: Collected buffer length, up to @size, set on return * @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt */ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, struct vu_virtq_element *elem, int max_elem, + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, size_t size, size_t *collected) { size_t current_size = 0; + size_t current_iov = 0; int elem_cnt = 0; - while (current_size < size && elem_cnt < max_elem) { - struct iovec *iov; + while (current_size < size && elem_cnt < max_elem && + current_iov < max_in_sg) { int ret; ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], - elem[elem_cnt].in_sg, - elem[elem_cnt].in_num, - elem[elem_cnt].out_sg, - elem[elem_cnt].out_num); + &in_sg[current_iov], + max_in_sg - current_iov, + NULL, 0); if (ret < 0) break; @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, break; } - iov = &elem[elem_cnt].in_sg[0]; - - if (iov->iov_len > size - current_size) - iov->iov_len = size - current_size; + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, + elem[elem_cnt].in_num, + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out.
Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need?
For an element, *ìn_*num is the number of *in_*sg we have read from the ring for an element.
It's virtio semantic, so *in_* means sg going *in* the guest.
For *out_*sg we have *out_*num.
What if we rename it to @in_used or @in_collected?
The idea was to keep the same name as in the element. But we can change this to @in_used.
Would "in_total" work better to suggest that it's the sum of all the elements' in_num?
Yes, I think it gives the information it's the sum of the in_num Thanks, Laurent
On Wed, 18 Mar 2026 08:21:10 +0100
Laurent Vivier
On 3/18/26 02:16, David Gibson wrote:
On Tue, Mar 17, 2026 at 05:30:32PM +0100, Laurent Vivier wrote:
On 3/17/26 16:23, Stefano Brivio wrote:
On Tue, 17 Mar 2026 08:25:49 +0100 Laurent Vivier
wrote: On 3/17/26 03:40, David Gibson wrote:
On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote: > Previously, callers had to pre-initialize virtqueue elements with iovec > entries using vu_set_element() or vu_init_elem() before calling > vu_collect(). This meant each element owned a fixed, pre-assigned iovec > slot. > > Move the iovec array into vu_collect() as explicit parameters (in_sg, > max_in_sg, and in_num), letting it pass the remaining iovec capacity > directly to vu_queue_pop(). A running current_iov counter tracks > consumed entries across elements, so multiple elements share a single > iovec pool. The optional in_num output parameter reports how many iovec > entries were consumed, allowing callers to track usage across multiple > vu_collect() calls. > > This removes vu_set_element() and vu_init_elem() which are no longer > needed, and is a prerequisite for multi-buffer support where a single > virtqueue element can use more than one iovec entry. For now, callers > assert the current single-iovec-per-element invariant until they are > updated to handle multiple iovecs. > > Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Couple of thoughts on possible polish below.
[snip] > /** > * vu_collect() - collect virtio buffers from a given virtqueue > * @vdev: vhost-user device > * @vq: virtqueue to collect from > - * @elem: Array of virtqueue element > - * each element must be initialized with one iovec entry > - * in the in_sg array. > + * @elem: Array of @max_elem virtqueue elements > * @max_elem: Number of virtqueue elements in the array > + * @in_sg: Incoming iovec array for device-writable descriptors > + * @max_in_sg: Maximum number of entries in @in_sg > + * @in_num: Number of collected entries from @in_sg (output) > * @size: Maximum size of the data in the frame > * @collected: Collected buffer length, up to @size, set on return > * > @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt > */ > int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, > struct vu_virtq_element *elem, int max_elem, > + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, > size_t size, size_t *collected) > { > size_t current_size = 0; > + size_t current_iov = 0; > int elem_cnt = 0; > - while (current_size < size && elem_cnt < max_elem) { > - struct iovec *iov; > + while (current_size < size && elem_cnt < max_elem && > + current_iov < max_in_sg) { > int ret; > ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], > - elem[elem_cnt].in_sg, > - elem[elem_cnt].in_num, > - elem[elem_cnt].out_sg, > - elem[elem_cnt].out_num); > + &in_sg[current_iov], > + max_in_sg - current_iov, > + NULL, 0); > if (ret < 0) > break; > @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, > break; > } > - iov = &elem[elem_cnt].in_sg[0]; > - > - if (iov->iov_len > size - current_size) > - iov->iov_len = size - current_size; > + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, > + elem[elem_cnt].in_num, > + size - current_size);
Will elem[].in_num always end up with the same value as the @in_num parameter? If so, do we need the explicit parameter?
@in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out.
Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need?
For an element, *ìn_*num is the number of *in_*sg we have read from the ring for an element.
It's virtio semantic, so *in_* means sg going *in* the guest.
For *out_*sg we have *out_*num.
What if we rename it to @in_used or @in_collected?
The idea was to keep the same name as in the element. But we can change this to @in_used.
Would "in_total" work better to suggest that it's the sum of all the elements' in_num?
Yes, I think it gives the information it's the sum of the in_num
Okay, should I change this on merge, or do you plan to repost (which would be slightly more convenient for me but not really needed)? -- Stefano
On 3/18/26 10:04, Stefano Brivio wrote:
On Wed, 18 Mar 2026 08:21:10 +0100 Laurent Vivier
wrote: On 3/18/26 02:16, David Gibson wrote:
On Tue, Mar 17, 2026 at 05:30:32PM +0100, Laurent Vivier wrote:
On 3/17/26 16:23, Stefano Brivio wrote:
On Tue, 17 Mar 2026 08:25:49 +0100 Laurent Vivier
wrote: On 3/17/26 03:40, David Gibson wrote: > On Fri, Mar 13, 2026 at 07:26:18PM +0100, Laurent Vivier wrote: >> Previously, callers had to pre-initialize virtqueue elements with iovec >> entries using vu_set_element() or vu_init_elem() before calling >> vu_collect(). This meant each element owned a fixed, pre-assigned iovec >> slot. >> >> Move the iovec array into vu_collect() as explicit parameters (in_sg, >> max_in_sg, and in_num), letting it pass the remaining iovec capacity >> directly to vu_queue_pop(). A running current_iov counter tracks >> consumed entries across elements, so multiple elements share a single >> iovec pool. The optional in_num output parameter reports how many iovec >> entries were consumed, allowing callers to track usage across multiple >> vu_collect() calls. >> >> This removes vu_set_element() and vu_init_elem() which are no longer >> needed, and is a prerequisite for multi-buffer support where a single >> virtqueue element can use more than one iovec entry. For now, callers >> assert the current single-iovec-per-element invariant until they are >> updated to handle multiple iovecs. >> >> Signed-off-by: Laurent Vivier
> > Reviewed-by: David Gibson > > Couple of thoughts on possible polish below. > > [snip] >> /** >> * vu_collect() - collect virtio buffers from a given virtqueue >> * @vdev: vhost-user device >> * @vq: virtqueue to collect from >> - * @elem: Array of virtqueue element >> - * each element must be initialized with one iovec entry >> - * in the in_sg array. >> + * @elem: Array of @max_elem virtqueue elements >> * @max_elem: Number of virtqueue elements in the array >> + * @in_sg: Incoming iovec array for device-writable descriptors >> + * @max_in_sg: Maximum number of entries in @in_sg >> + * @in_num: Number of collected entries from @in_sg (output) >> * @size: Maximum size of the data in the frame >> * @collected: Collected buffer length, up to @size, set on return >> * >> @@ -80,20 +67,21 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov, int elem_cnt >> */ >> int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, >> struct vu_virtq_element *elem, int max_elem, >> + struct iovec *in_sg, size_t max_in_sg, size_t *in_num, >> size_t size, size_t *collected) >> { >> size_t current_size = 0; >> + size_t current_iov = 0; >> int elem_cnt = 0; >> - while (current_size < size && elem_cnt < max_elem) { >> - struct iovec *iov; >> + while (current_size < size && elem_cnt < max_elem && >> + current_iov < max_in_sg) { >> int ret; >> ret = vu_queue_pop(vdev, vq, &elem[elem_cnt], >> - elem[elem_cnt].in_sg, >> - elem[elem_cnt].in_num, >> - elem[elem_cnt].out_sg, >> - elem[elem_cnt].out_num); >> + &in_sg[current_iov], >> + max_in_sg - current_iov, >> + NULL, 0); >> if (ret < 0) >> break; >> @@ -103,18 +91,22 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, >> break; >> } >> - iov = &elem[elem_cnt].in_sg[0]; >> - >> - if (iov->iov_len > size - current_size) >> - iov->iov_len = size - current_size; >> + elem[elem_cnt].in_num = iov_truncate(elem[elem_cnt].in_sg, >> + elem[elem_cnt].in_num, >> + size - current_size); > > Will elem[].in_num always end up with the same value as the @in_num > parameter? If so, do we need the explicit parameter? @in_num parameter of vu_collect()?
@in_num is the sum of all elem[].in_num, it can be computed by the caller function from elem, but it is simpler to return it as we need to compute it in the loop.
I'm not sure I understood the point of David's comment here, and this explanation makes sense to me now, but it took me a bit to figure that out.
Could it be that @in_num is a bit confusing as it has "in" and "num" in it, but it's actually an output representing how many "in" entries we used/need?
For an element, *ìn_*num is the number of *in_*sg we have read from the ring for an element.
It's virtio semantic, so *in_* means sg going *in* the guest.
For *out_*sg we have *out_*num.
What if we rename it to @in_used or @in_collected?
The idea was to keep the same name as in the element. But we can change this to @in_used.
Would "in_total" work better to suggest that it's the sum of all the elements' in_num?
Yes, I think it gives the information it's the sum of the in_num
Okay, should I change this on merge, or do you plan to repost (which would be slightly more convenient for me but not really needed)?
I'm preparing the v3. Thanks, Laurent
participants (3)
-
David Gibson
-
Laurent Vivier
-
Stefano Brivio