Re: [PATCH v2 3/3] vu_common: Move iovec management into vu_collect()

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