Re: [PATCH RFC 7/8] vsock: preserve host-to-guest datagram boundaries

[Bobby Eshleman <bobbyeshleman@meta.com>]

On Wed, Oct 28, 2026 at 05:04:36PM +0100, Sergio Lopez wrote:
> Vhost splits large datagrams across guest RX buffers, delivering each
> fragment as a separate message. Allocate full-size RX buffers with
> page fragments when datagrams are negotiated, and drop whole datagrams
> if a guest buffer is too small. Refill empty RX queues even when only
> one full-size buffer fits in the ring.

I wonder if it might be better to do what seqpacket does, and just split
the datagram across the buffers (for seqpacket, until the EOM flag is
seen)? I'd expect that we will get better utilization of our vq buffers
this way?

Best,
Bobby

> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>
> ---
>  drivers/vhost/vsock.c            |  8 ++++++++
>  net/vmw_vsock/virtio_transport.c | 28 +++++++++++++++++++---------
>  2 files changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index 79ffdb28f4d9..2294ed8d30a8 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -185,6 +185,14 @@ vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>  		 * buffer, we split it using multiple buffers.
>  		 */
>  		if (payload_len > iov_len - sizeof(*hdr)) {
> +			/* Datagrams must fit in a single descriptor chain. */
> +			if (le16_to_cpu(hdr->type) == VIRTIO_VSOCK_TYPE_DGRAM) {
> +				kfree_skb(skb);
> +				vhost_add_used(vq, head, 0);
> +				added = true;
> +				continue;
> +			}
> +
>  			payload_len = iov_len - sizeof(*hdr);
>  
>  			/* As we are copying pieces of large packet's buffer to
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index 14ce0836bc39..d256274088b3 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -54,6 +54,7 @@ struct virtio_vsock {
>  	bool rx_run;
>  	int rx_buf_nr;
>  	int rx_buf_max_nr;
> +	struct scatterlist in_bufs[MAX_SKB_FRAGS + 1];
>  
>  	u32 guest_cid;
>  	bool seqpacket_allow;
> @@ -312,22 +313,31 @@ virtio_transport_cancel_pkt(struct vsock_sock *vsk)
>  static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
>  {
>  	int total_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
> -	struct scatterlist pkt, *p;
>  	struct virtqueue *vq;
>  	struct sk_buff *skb;
>  	int ret;
>  
>  	vq = vsock->vqs[VSOCK_VQ_RX];
> +	/* A datagram must fit in one RX buffer, including its header. */
> +	if (vsock->dgram_allow)
> +		total_len = VIRTIO_VSOCK_SKB_HEADROOM +
> +			    VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
>  
>  	do {
> -		skb = virtio_vsock_alloc_linear_skb(total_len, GFP_KERNEL);
> +		skb = virtio_vsock_alloc_skb(total_len, GFP_KERNEL);
>  		if (!skb)
>  			break;
>  
>  		memset(skb->head, 0, VIRTIO_VSOCK_SKB_HEADROOM);
> -		sg_init_one(&pkt, virtio_vsock_hdr(skb), total_len);
> -		p = &pkt;
> -		ret = virtqueue_add_sgs(vq, &p, 0, 1, skb, GFP_KERNEL);
> +		/* Post the header and the entire payload as one RX buffer. */
> +		virtio_vsock_skb_put(skb, total_len - VIRTIO_VSOCK_SKB_HEADROOM);
> +		skb_push(skb, VIRTIO_VSOCK_SKB_HEADROOM);
> +		sg_init_table(vsock->in_bufs, ARRAY_SIZE(vsock->in_bufs));
> +		ret = skb_to_sgvec(skb, vsock->in_bufs, 0, total_len);
> +		skb_pull(skb, VIRTIO_VSOCK_SKB_HEADROOM);
> +		if (ret >= 0)
> +			ret = virtqueue_add_inbuf(vq, vsock->in_bufs, ret, skb,
> +						  GFP_KERNEL);
>  		if (ret < 0) {
>  			kfree_skb(skb);
>  			break;
> @@ -698,7 +708,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
>  
>  			/* Drop short/long packets */
>  			if (unlikely(len < sizeof(*hdr) ||
> -				     len > virtio_vsock_skb_len(skb))) {
> +				     len > sizeof(*hdr) + skb->len)) {
>  				kfree_skb(skb);
>  				continue;
>  			}
> @@ -710,8 +720,7 @@ static void virtio_transport_rx_work(struct work_struct *work)
>  				continue;
>  			}
>  
> -			if (payload_len)
> -				virtio_vsock_skb_put(skb, payload_len);
> +			pskb_trim_unique(skb, payload_len);
>  
>  			virtio_transport_deliver_tap_pkt(skb);
>  
> @@ -723,7 +732,8 @@ static void virtio_transport_rx_work(struct work_struct *work)
>  	} while (!virtqueue_enable_cb(vq));
>  
>  out:
> -	if (vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
> +	if (!vsock->rx_buf_nr ||
> +	    vsock->rx_buf_nr < vsock->rx_buf_max_nr / 2)
>  		virtio_vsock_rx_fill(vsock);
>  out_nofill:
>  	mutex_unlock(&vsock->rx_lock);
> 
> -- 
> 2.56.0
>