Re: [PATCH 08/10] vu_common: Pass explicit frame length to vu_flush()

[Stefano Brivio <sbrivio@redhat.com>]

On Wed,  1 Apr 2026 21:18:24 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Currently vu_flush() derives the frame size from the iov, but in
> preparation for iov arrays that may be larger than the actual frame,
> pass the total length (including vnet header) explicitly so that only
> the relevant portion is reported to the virtqueue.
> 
> Ensure a minimum frame size of ETH_ZLEN + VNET_HLEN to handle short
> frames. All elements are still flushed to avoid descriptor leaks,
> but trailing elements beyond frame_len will report a zero length.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  tcp_vu.c    |  6 +++---
>  udp_vu.c    |  2 +-
>  vu_common.c | 15 ++++++++++++---
>  vu_common.h |  2 +-
>  4 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 329fa969fca1..105bca41c6de 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -140,7 +140,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  	l2len = optlen + hdrlen - VNET_HLEN;
>  	vu_pad(&flags_elem[0].in_sg[0], l2len);
>  
> -	vu_flush(vdev, vq, flags_elem, 1);
> +	vu_flush(vdev, vq, flags_elem, 1, hdrlen + optlen);
>  
>  	if (*c->pcap)
>  		pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN,
> @@ -157,7 +157,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>  			       flags_elem[0].in_sg[0].iov_base,
>  			       flags_elem[0].in_sg[0].iov_len);
>  
> -			vu_flush(vdev, vq, &flags_elem[1], 1);
> +			vu_flush(vdev, vq, &flags_elem[1], 1, hdrlen + optlen);
>  
>  			if (*c->pcap)
>  				pcap_iov(&flags_elem[1].in_sg[0], 1, VNET_HLEN,
> @@ -463,7 +463,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
>  		l2len = dlen + hdrlen - VNET_HLEN;
>  		vu_pad(iov, l2len);
>  
> -		vu_flush(vdev, vq, &elem[head[i]], buf_cnt);
> +		vu_flush(vdev, vq, &elem[head[i]], buf_cnt, dlen + hdrlen);
>  
>  		if (*c->pcap)
>  			pcap_iov(iov, buf_cnt, VNET_HLEN,
> diff --git a/udp_vu.c b/udp_vu.c
> index 81491afa7e6a..4641f42eb5c4 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -234,7 +234,7 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
>  				pcap_iov(iov_vu, iov_cnt, VNET_HLEN,
>  					 hdrlen + dlen - VNET_HLEN);
>  			}
> -			vu_flush(vdev, vq, elem, elem_used);
> +			vu_flush(vdev, vq, elem, elem_used, hdrlen + dlen);
>  			vu_queue_notify(vdev, vq);
>  		}
>  	}
> diff --git a/vu_common.c b/vu_common.c
> index f254cb67ec78..d371a59a1813 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -134,18 +134,27 @@ static void vu_set_vnethdr(struct virtio_net_hdr_mrg_rxbuf *vnethdr,
>   * @vq:		vhost-user virtqueue
>   * @elem:	virtqueue elements array to send back to the virtqueue
>   * @elem_cnt:	Length of the array
> + * @frame_len:	Total frame length including vnet header
>   */
>  void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq,
> -	      struct vu_virtq_element *elem, int elem_cnt)
> +	      struct vu_virtq_element *elem, int elem_cnt, size_t frame_len)
>  {
> +	size_t len;
>  	int i;
>  
>  	vu_set_vnethdr(elem[0].in_sg[0].iov_base, elem_cnt);
>  
> +	len = MAX(ETH_ZLEN + VNET_HLEN, frame_len);
>  	for (i = 0; i < elem_cnt; i++) {
> -		size_t elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
> +		size_t elem_size;
> +
> +		elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
> +		if (elem_size > len)
> +			elem_size = len;

Convenient, but this doesn't really represent 'elem_size' anymore and
it's a bit confusing because we later subtract it from 'len' (at a
first glance, it looks like one might underflow it). What about:

		size_t elem_size, fill_size;

		elem_size = iov_size(elem[i].in_sg, elem[i].in_num);
		fill_size = MIN(len, elem_size);

		vu_queue_fill(vdev, vq, &elem[i], fill_size, i);
		len -= fill_size;

?

>  
>  		vu_queue_fill(vdev, vq, &elem[i], elem_size, i);
> +
> +		len -= elem_size;
>  	}

Should we now add a debug message or warning here if we happen to have
any residual 'len'? Or it can never happen by design? I'm not quite
sure.

>  
>  	vu_queue_flush(vdev, vq, elem_cnt);
> @@ -270,7 +279,7 @@ int vu_send_single(const struct ctx *c, const void *buf, size_t size)
>  	if (*c->pcap)
>  		pcap_iov(in_sg, in_total, VNET_HLEN, size);
>  
> -	vu_flush(vdev, vq, elem, elem_cnt);
> +	vu_flush(vdev, vq, elem, elem_cnt, VNET_HLEN + size);
>  	vu_queue_notify(vdev, vq);
>  
>  	trace("vhost-user sent %zu", total);
> diff --git a/vu_common.h b/vu_common.h
> index 4037ab765b7d..77d1849e6115 100644
> --- a/vu_common.h
> +++ b/vu_common.h
> @@ -40,7 +40,7 @@ int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq,
>  	       struct iovec *in_sg, size_t max_in_sg, size_t *in_total,
>  	       size_t size, size_t *collected);
>  void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq,
> -	      struct vu_virtq_element *elem, int elem_cnt);
> +	      struct vu_virtq_element *elem, int elem_cnt, size_t frame_len);
>  void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref,
>  		const struct timespec *now);
>  int vu_send_single(const struct ctx *c, const void *buf, size_t size);

-- 
Stefano