Re: [PATCH RFC 3/8] vsock/virtio: implement the datagram data path

[Bobby Eshleman <bobbyeshleman@meta.com>]

On Wed, Oct 28, 2026 at 05:04:32PM +0100, Sergio Lopez wrote:
> Add the datagram feature bit and packet type, and implement the common
> virtio transport operations needed to bind, send, and receive
> datagrams.
> 
> Send each message as an OP_RW packet with a maximum 64 KiB payload,
> including support for empty messages. Bound pending sends by the
> socket send buffer instead of connection-oriented peer credits. Charge
> pending payload bytes before handing the skb to the backend and
> release that charge exactly once on completion, rejection, or queued
> drops.
> 
> Deliver incoming datagrams through the bound table without entering
> the connection state machine. Validate the transport and connected
> peer under the socket lock, including after shutdown, and silently
> discard undeliverable messages. Use the socket datagram queue for
> recvmsg(), including source addresses, peeking, and truncation
> reporting based on the payload length after socket filtering.  Reject
> MSG_OOB and MSG_ERRQUEUE before accessing the receive queue.  Reject
> datagram sockmap admission until its receive callbacks support it.
> 
> Signed-off-by: Sergio Lopez <slp@redhat.com>
> ---
>  include/linux/virtio_vsock.h            |   2 +
>  include/uapi/linux/virtio_vsock.h       |   2 +
>  net/vmw_vsock/virtio_transport_common.c | 150 +++++++++++++++++++++++++++++---
>  net/vmw_vsock/vsock_bpf.c               |   3 +-
>  4 files changed, 142 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index f91704731057..0bccae661221 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -12,6 +12,8 @@
>  struct virtio_vsock_skb_cb {
>  	bool reply;
>  	bool tap_delivered;
> +	/* Datagram payload is still charged to bytes_unsent. */
> +	bool tx_accounted;
>  	u32 offset;
>  };
>  
> diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
> index 64738838bee5..27b4b2b8bf13 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -40,6 +40,7 @@
>  
>  /* The feature bitmap for virtio vsock */
>  #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
> +#define VIRTIO_VSOCK_F_DGRAM		3	/* SOCK_DGRAM supported */
>  
>  struct virtio_vsock_config {
>  	__le64 guest_cid;
> @@ -69,6 +70,7 @@ struct virtio_vsock_hdr {
>  enum virtio_vsock_type {
>  	VIRTIO_VSOCK_TYPE_STREAM = 1,
>  	VIRTIO_VSOCK_TYPE_SEQPACKET = 2,
> +	VIRTIO_VSOCK_TYPE_DGRAM = 3,
>  };
>  
>  enum virtio_vsock_op {
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 1e762a480df4..ae33630d99e7 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -204,10 +204,18 @@ static u16 virtio_transport_get_type(struct sock *sk)
>  {
>  	if (sk->sk_type == SOCK_STREAM)
>  		return VIRTIO_VSOCK_TYPE_STREAM;
> +	else if (sk->sk_type == SOCK_DGRAM)
> +		return VIRTIO_VSOCK_TYPE_DGRAM;
>  	else
>  		return VIRTIO_VSOCK_TYPE_SEQPACKET;
>  }
>  
> +static void virtio_transport_dgram_skb_destructor(struct sk_buff *skb)
> +{
> +	virtio_transport_consume_skb_sent(skb, false);
> +	sock_wfree(skb);
> +}
> +
>  /* Returns new sk_buff on success, otherwise returns NULL. */
>  static struct sk_buff *virtio_transport_alloc_skb(struct virtio_vsock_pkt_info *info,
>  						  size_t payload_len,
> @@ -303,6 +311,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	u32 src_cid, src_port, dst_cid, dst_port;
>  	const struct virtio_transport *t_ops;
>  	struct iov_iter_state msg_iter_state;
> +	struct sock *sk = sk_vsock(vsk);
>  	struct virtio_vsock_sock *vvs;
>  	struct ubuf_info *uarg = NULL;
>  	u32 pkt_len = info->pkt_len;
> @@ -329,12 +338,24 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  
>  	vvs = vsk->trans;
>  
> -	/* virtio_transport_get_credit might return less than pkt_len credit */
> -	pkt_len = virtio_transport_get_credit(vvs, pkt_len);
> +	if (info->type == VIRTIO_VSOCK_TYPE_DGRAM) {
> +		if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
> +			return -EMSGSIZE;
>  
> -	/* Do not send zero length OP_RW pkt */
> -	if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> -		return pkt_len;
> +		/* Datagram sends are serialized by the socket lock. */
> +		if (sk_wmem_alloc_get(sk) >= READ_ONCE(sk->sk_sndbuf)) {
> +			sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
> +			set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
> +			return -EAGAIN;
> +		}
> +	} else {
> +		/* virtio_transport_get_credit might return less than pkt_len credit */
> +		pkt_len = virtio_transport_get_credit(vvs, pkt_len);
> +
> +		/* Do not send zero length OP_RW pkt */
> +		if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> +			return pkt_len;
> +	}
>  
>  	if (info->msg && (info->msg->msg_flags & MSG_ZEROCOPY)) {
>  		/* If 'info->msg' is not NULL, this is only VIRTIO_VSOCK_OP_RW.
> @@ -348,7 +369,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  			uarg = msg_zerocopy_realloc(sk_vsock(vsk), pkt_len,
>  						    NULL, false);
>  			if (!uarg) {
> -				virtio_transport_put_credit(vvs, pkt_len);
> +				if (info->type != VIRTIO_VSOCK_TYPE_DGRAM)
> +					virtio_transport_put_credit(vvs, pkt_len);
>  				return -ENOMEM;
>  			}
>  
> @@ -393,7 +415,15 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  			break;
>  		}
>  
> -		virtio_transport_inc_tx_pkt(vvs, skb);
> +		if (info->type == VIRTIO_VSOCK_TYPE_DGRAM) {
> +			spin_lock_bh(&vvs->tx_lock);
> +			vvs->bytes_unsent += skb_len;
> +			spin_unlock_bh(&vvs->tx_lock);
> +			VIRTIO_VSOCK_SKB_CB(skb)->tx_accounted = true;
> +			skb->destructor = virtio_transport_dgram_skb_destructor;
> +		} else {
> +			virtio_transport_inc_tx_pkt(vvs, skb);
> +		}
>  
>  		ret = t_ops->send_pkt(skb, info->net);
>  		if (ret < 0)
> @@ -416,7 +446,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	if (info->msg && ret < 0)
>  		iov_iter_restore(&info->msg->msg_iter, &msg_iter_state);
>  
> -	virtio_transport_put_credit(vvs, rest_len);
> +	if (info->type != VIRTIO_VSOCK_TYPE_DGRAM)
> +		virtio_transport_put_credit(vvs, rest_len);
>  
>  	/* msg_zerocopy_realloc() initializes the ubuf_info refcnt to 1.
>  	 * skb_zcopy_set() increases it for each skb, so we can drop that
> @@ -620,7 +651,9 @@ void virtio_transport_consume_skb_sent(struct sk_buff *skb, bool consume)
>  {
>  	struct sock *s = skb->sk;
>  
> -	if (s && skb->len) {
> +	if (s && skb->len &&
> +	    (s->sk_type != SOCK_DGRAM ||
> +	     VIRTIO_VSOCK_SKB_CB(skb)->tx_accounted)) {
>  		struct vsock_sock *vs = vsock_sk(s);
>  		struct virtio_vsock_sock *vvs;
>  
> @@ -631,6 +664,9 @@ void virtio_transport_consume_skb_sent(struct sk_buff *skb, bool consume)
>  		spin_unlock_bh(&vvs->tx_lock);
>  	}
>  
> +	if (s && s->sk_type == SOCK_DGRAM)
> +		VIRTIO_VSOCK_SKB_CB(skb)->tx_accounted = false;
> +
>  	if (consume)
>  		consume_skb(skb);
>  }
> @@ -999,7 +1035,47 @@ virtio_transport_dgram_dequeue(struct vsock_sock *vsk,
>  			       struct msghdr *msg,
>  			       size_t len, int flags)
>  {
> -	return -EOPNOTSUPP;
> +	struct sock *sk = sk_vsock(vsk);
> +	struct virtio_vsock_hdr *hdr;
> +	struct sk_buff *skb;
> +	size_t payload_len;
> +	int err;
> +
> +	if (flags & (MSG_OOB | MSG_ERRQUEUE))
> +		return -EOPNOTSUPP;
> +
> +	skb = skb_recv_datagram(sk, flags, &err);
> +	if (!skb)
> +		return err;
> +
> +	hdr = virtio_vsock_hdr(skb);
> +	payload_len = skb->len;
> +
> +	if (payload_len > len) {
> +		payload_len = len;
> +		msg->msg_flags |= MSG_TRUNC;
> +	}
> +
> +	err = skb_copy_datagram_iter(skb, 0, &msg->msg_iter, payload_len);
> +	if (err)
> +		goto out;
> +
> +	if (msg->msg_name) {
> +		DECLARE_SOCKADDR(struct sockaddr_vm *, vm_addr, msg->msg_name);
> +
> +		vsock_addr_init(vm_addr, le64_to_cpu(hdr->src_cid),
> +				le32_to_cpu(hdr->src_port));
> +		msg->msg_namelen = sizeof(*vm_addr);
> +	}
> +
> +	if (flags & MSG_TRUNC)
> +		err = skb->len;
> +	else
> +		err = payload_len;
> +
> +out:
> +	skb_free_datagram(sk, skb);
> +	return err;
>  }
>  EXPORT_SYMBOL_GPL(virtio_transport_dgram_dequeue);
>  
> @@ -1202,13 +1278,13 @@ EXPORT_SYMBOL_GPL(virtio_transport_stream_is_active);
>  int virtio_transport_dgram_bind(struct vsock_sock *vsk,
>  				struct sockaddr_vm *addr)
>  {
> -	return -EOPNOTSUPP;
> +	return vsock_bind_common(vsk, addr);
>  }
>  EXPORT_SYMBOL_GPL(virtio_transport_dgram_bind);
>  
>  bool virtio_transport_dgram_allow(struct vsock_sock *vsk, u32 cid, u32 port)
>  {
> -	return false;
> +	return true;
>  }
>  EXPORT_SYMBOL_GPL(virtio_transport_dgram_allow);
>  
> @@ -1246,7 +1322,17 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
>  			       struct msghdr *msg,
>  			       size_t dgram_len)
>  {
> -	return -EOPNOTSUPP;
> +	struct virtio_vsock_pkt_info info = {
> +		.op = VIRTIO_VSOCK_OP_RW,
> +		.msg = msg,
> +		.pkt_len = dgram_len,
> +		.vsk = vsk,
> +		.remote_cid = remote_addr->svm_cid,
> +		.remote_port = remote_addr->svm_port,
> +		.net = sock_net(sk_vsock(vsk)),
> +	};
> +
> +	return virtio_transport_send_pkt_info(vsk, &info);
>  }
>  EXPORT_SYMBOL_GPL(virtio_transport_dgram_enqueue);
>  
> @@ -1771,7 +1857,8 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
>  static bool virtio_transport_valid_type(u16 type)
>  {
>  	return (type == VIRTIO_VSOCK_TYPE_STREAM) ||
> -	       (type == VIRTIO_VSOCK_TYPE_SEQPACKET);
> +	       (type == VIRTIO_VSOCK_TYPE_SEQPACKET) ||
> +	       (type == VIRTIO_VSOCK_TYPE_DGRAM);
>  }
>  
>  /* We are under the virtio-vsock's vsock->rx_lock or vhost-vsock's vq->mutex
> @@ -1805,6 +1892,41 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>  		goto free_pkt;
>  	}
>  
> +	/* Dgram packets are delivered directly to bound sockets without
> +	 * going through the connection state machine. Undeliverable
> +	 * datagrams are silently dropped (no RST).
> +	 */
> +	if (le16_to_cpu(hdr->type) == VIRTIO_VSOCK_TYPE_DGRAM) {
> +		if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_RW)
> +			goto free_pkt;
> +
> +		sk = vsock_find_bound_socket_net(&dst, net);
> +
> +		if (!sk || sk->sk_type != SOCK_DGRAM) {
> +			if (sk)
> +				sock_put(sk);
> +			goto free_pkt;
> +		}
> +
> +		vsk = vsock_sk(sk);
> +		lock_sock(sk);
> +		if (sock_flag(sk, SOCK_DEAD) ||
> +		    vsk->transport != &t->transport ||
> +		    (sk->sk_socket->state != SS_UNCONNECTED &&
> +		     !vsock_check_source(vsk, &t->transport, &src))) {
> +			release_sock(sk);
> +			sock_put(sk);
> +			goto free_pkt;
> +		}
> +
> +		if (sock_queue_rcv_skb(sk, skb))
> +			kfree_skb(skb);

I'd love to see the receive path be free of lock_sock the way that UDP
does.

My understanding from udp is that we can avoid the race with
close/SOCK_DEAD by using SOCK_RCU_FREE and putting this receive in an
rcu read-side section. I think this makes sk->sk_socket->state safe
inside the RCU read side section too.

I think vsock_check_source() in a later patch goes away with
virtio_transport_dgram_source_allow(), so I think maybe that patch
should come before this?

Best,
Bobby

> +
> +		release_sock(sk);
> +		sock_put(sk);
> +		return;
> +	}
> +
>  	/* The socket must be in connected or bound table
>  	 * otherwise send reset back
>  	 */
> diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
> index 9049d2648646..296e1fa7970b 100644
> --- a/net/vmw_vsock/vsock_bpf.c
> +++ b/net/vmw_vsock/vsock_bpf.c
> @@ -161,7 +161,8 @@ int vsock_bpf_update_proto(struct sock *sk, struct sk_psock *psock, bool restore
>  	if (!vsk->transport)
>  		return -ENODEV;
>  
> -	if (!vsk->transport->read_skb)
> +	/* Datagram receive is not implemented by the sockmap callbacks. */
> +	if (sk->sk_type == SOCK_DGRAM || !vsk->transport->read_skb)
>  		return -EOPNOTSUPP;
>  
>  	vsock_bpf_check_needs_rebuild(psock->sk_proto);
> 
> -- 
> 2.56.0
>