Re: [PATCH 12/12] vhost-user,udp: Use 2 iovec entries per element

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

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

On Fri, Feb 27, 2026 at 03:03:30PM +0100, Laurent Vivier wrote:
> iPXE places the vnet header in one virtqueue descriptor and the payload
> in another. When passt maps these descriptors, it needs two iovecs per
> virtqueue element to handle this layout.
> 
> Without this, passt crashes with:
> 
>   ASSERTION FAILED in virtqueue_map_desc (virtio.c:403): num_sg < max_num_sg
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

It looks like a reasonable short term fix for the specific iPXE
problem.

Longer term, having a fixed 2 iovecs per element still seems fragile,
if we encounter some other occasionally-used guest side virtio driver
which splits its Tx buffers up in an even weirder way.  It would be
nicer if we can actually take our cue from what the guest actually
gives us.  I'm not sure how feasible that is, though.

> ---
>  udp_vu.c    |  8 ++++----
>  vu_common.c | 34 +++++++++++++++++++++++-----------
>  2 files changed, 27 insertions(+), 15 deletions(-)
> 
> diff --git a/udp_vu.c b/udp_vu.c
> index 7e486b74883e..13fea87e1b9f 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -34,7 +34,7 @@
>  #include "vu_common.h"
>  
>  static struct iovec     iov_vu		[VIRTQUEUE_MAX_SIZE];
> -static struct vu_virtq_element	elem		[VIRTQUEUE_MAX_SIZE];
> +static struct vu_virtq_element	elem	[VIRTQUEUE_MAX_SIZE / IOV_PER_ELEM];
>  
>  /**
>   * udp_vu_hdrlen() - Sum size of all headers, from UDP to virtio-net
> @@ -214,21 +214,21 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
>  		int elem_cnt, elem_used;
>  		ssize_t dlen;
>  
> -		vu_init_elem(elem, iov_vu, ARRAY_SIZE(elem), 1);
> +		vu_init_elem(elem, iov_vu, ARRAY_SIZE(elem), IOV_PER_ELEM);
>  
>  		elem_cnt = vu_collect(vdev, vq, elem, ARRAY_SIZE(elem),
>  				      IP_MAX_MTU + ETH_HLEN + VNET_HLEN, NULL);
>  		if (elem_cnt == 0)
>  			break;
>  
> -		data = IOV_TAIL(iov_vu, elem_cnt, 0);
> +		data = IOV_TAIL(iov_vu, (size_t)(elem_cnt * IOV_PER_ELEM), 0);
>  
>  		dlen = udp_vu_sock_recv(&data, s, v6);
>  		if (dlen < 0) {
>  			vu_queue_rewind(vq, elem_cnt);
>  			continue;
>  		}
> -		elem_used = data.cnt;
> +		elem_used = DIV_ROUND_UP(data.cnt, IOV_PER_ELEM);
>  
>  		/* release unused buffers */
>  		vu_queue_rewind(vq, elem_cnt - elem_used);
> diff --git a/vu_common.c b/vu_common.c
> index 67d8f3e47338..3f50d31da633 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -63,8 +63,15 @@ void vu_init_elem(struct vu_virtq_element *elem, struct iovec *iov,
>  {
>  	int i, j;
>  
> -	for (i = 0, j = 0; i < elem_cnt; i++, j += iov_per_elem)
> +	for (i = 0, j = 0; i < elem_cnt; i++, j += iov_per_elem) {
> +		int k;
> +
> +		for (k = 0; k < iov_per_elem; k++) {
> +			iov[j + k].iov_base = NULL;
> +			iov[j + k].iov_len = 0;
> +		}
>  		vu_set_element(&elem[i], 0, NULL, iov_per_elem, &iov[j]);
> +	}
>  }
>  
>  /**
> @@ -88,7 +95,8 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq,
>  	int elem_cnt = 0;
>  
>  	while (current_size < size && elem_cnt < max_elem) {
> -		struct iovec *iov;
> +		struct iov_tail tail;
> +		size_t elem_size;
>  		int ret;
>  
>  		ret = vu_queue_pop(vdev, vq, &elem[elem_cnt]);
> @@ -101,12 +109,14 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq,
>  			break;
>  		}
>  
> -		iov = &elem[elem_cnt].in_sg[0];
> +		tail = IOV_TAIL(elem[elem_cnt].in_sg, elem[elem_cnt].in_num, 0);
> +		iov_tail_truncate(&tail, size - current_size);
> +		elem[elem_cnt].in_num = tail.cnt;
>  
> -		if (iov->iov_len > size - current_size)
> -			iov->iov_len = size - current_size;
> +		elem_size = iov_size(elem[elem_cnt].in_sg,
> +				     elem[elem_cnt].in_num);
>  
> -		current_size += iov->iov_len;
> +		current_size += elem_size;
>  		elem_cnt++;
>  
>  		if (!vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> @@ -153,8 +163,10 @@ 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);
> +		vu_queue_fill(vdev, vq, &elem[i], elem_size, i);
> +	}
>  
>  	vu_queue_flush(vdev, vq, elem_cnt);
>  	vu_queue_notify(vdev, vq);
> @@ -253,7 +265,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
>  {
>  	struct vu_dev *vdev = c->vdev;
>  	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
> -	struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE];
> +	struct vu_virtq_element elem[VIRTQUEUE_MAX_SIZE / IOV_PER_ELEM];
>  	struct iovec in_sg[VIRTQUEUE_MAX_SIZE];
>  	struct iov_tail data;
>  	size_t total;
> @@ -267,7 +279,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
>  		return -1;
>  	}
>  
> -	vu_init_elem(elem, in_sg, ARRAY_SIZE(elem), 1);
> +	vu_init_elem(elem, in_sg, ARRAY_SIZE(elem), IOV_PER_ELEM);
>  
>  	size += VNET_HLEN;
>  	elem_cnt = vu_collect(vdev, vq, elem, ARRAY_SIZE(elem), size, &total);
> @@ -277,7 +289,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
>  		goto err;
>  	}
>  
> -	data = IOV_TAIL(&in_sg[0], elem_cnt, 0);
> +	data = IOV_TAIL(&in_sg[0], (size_t)(elem_cnt * IOV_PER_ELEM), 0);
>  	vu_set_vnethdr(vdev, &data, elem_cnt);
>  	total -= VNET_HLEN;
>  
> -- 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]