On 16.08.2022 05:32, Bobby Eshleman wrote: > CC'ing virtio-dev@lists.oasis-open.org > > On Mon, Aug 15, 2022 at 10:56:08AM -0700, Bobby Eshleman wrote: >> This patch supports dgram in virtio and on the vhost side. Hello,
sorry, i don't understand, how this maintains message boundaries? Or it is unnecessary for SOCK_DGRAM? Thanks >> >> Signed-off-by: Jiang Wang <jiang.w...@bytedance.com> >> Signed-off-by: Bobby Eshleman <bobby.eshle...@bytedance.com> >> --- >> drivers/vhost/vsock.c | 2 +- >> include/net/af_vsock.h | 2 + >> include/uapi/linux/virtio_vsock.h | 1 + >> net/vmw_vsock/af_vsock.c | 26 +++- >> net/vmw_vsock/virtio_transport.c | 2 +- >> net/vmw_vsock/virtio_transport_common.c | 173 ++++++++++++++++++++++-- >> 6 files changed, 186 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >> index a5d1bdb786fe..3dc72a5647ca 100644 >> --- a/drivers/vhost/vsock.c >> +++ b/drivers/vhost/vsock.c >> @@ -925,7 +925,7 @@ static int __init vhost_vsock_init(void) >> int ret; >> >> ret = vsock_core_register(&vhost_transport.transport, >> - VSOCK_TRANSPORT_F_H2G); >> + VSOCK_TRANSPORT_F_H2G | >> VSOCK_TRANSPORT_F_DGRAM); >> if (ret < 0) >> return ret; >> >> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h >> index 1c53c4c4d88f..37e55c81e4df 100644 >> --- a/include/net/af_vsock.h >> +++ b/include/net/af_vsock.h >> @@ -78,6 +78,8 @@ struct vsock_sock { >> s64 vsock_stream_has_data(struct vsock_sock *vsk); >> s64 vsock_stream_has_space(struct vsock_sock *vsk); >> struct sock *vsock_create_connected(struct sock *parent); >> +int vsock_bind_stream(struct vsock_sock *vsk, >> + struct sockaddr_vm *addr); >> >> /**** TRANSPORT ****/ >> >> diff --git a/include/uapi/linux/virtio_vsock.h >> b/include/uapi/linux/virtio_vsock.h >> index 857df3a3a70d..0975b9c88292 100644 >> --- a/include/uapi/linux/virtio_vsock.h >> +++ b/include/uapi/linux/virtio_vsock.h >> @@ -70,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/af_vsock.c b/net/vmw_vsock/af_vsock.c >> index 1893f8aafa48..87e4ae1866d3 100644 >> --- a/net/vmw_vsock/af_vsock.c >> +++ b/net/vmw_vsock/af_vsock.c >> @@ -675,6 +675,19 @@ static int __vsock_bind_connectible(struct vsock_sock >> *vsk, >> return 0; >> } >> >> +int vsock_bind_stream(struct vsock_sock *vsk, >> + struct sockaddr_vm *addr) >> +{ >> + int retval; >> + >> + spin_lock_bh(&vsock_table_lock); >> + retval = __vsock_bind_connectible(vsk, addr); >> + spin_unlock_bh(&vsock_table_lock); >> + >> + return retval; >> +} >> +EXPORT_SYMBOL(vsock_bind_stream); >> + >> static int __vsock_bind_dgram(struct vsock_sock *vsk, >> struct sockaddr_vm *addr) >> { >> @@ -2363,11 +2376,16 @@ int vsock_core_register(const struct vsock_transport >> *t, int features) >> } >> >> if (features & VSOCK_TRANSPORT_F_DGRAM) { >> - if (t_dgram) { >> - err = -EBUSY; >> - goto err_busy; >> + /* TODO: always chose the G2H variant over others, support >> nesting later */ >> + if (features & VSOCK_TRANSPORT_F_G2H) { >> + if (t_dgram) >> + pr_warn("virtio_vsock: t_dgram already set\n"); >> + t_dgram = t; >> + } >> + >> + if (!t_dgram) { >> + t_dgram = t; >> } >> - t_dgram = t; >> } >> >> if (features & VSOCK_TRANSPORT_F_LOCAL) { >> diff --git a/net/vmw_vsock/virtio_transport.c >> b/net/vmw_vsock/virtio_transport.c >> index 073314312683..d4526ca462d2 100644 >> --- a/net/vmw_vsock/virtio_transport.c >> +++ b/net/vmw_vsock/virtio_transport.c >> @@ -850,7 +850,7 @@ static int __init virtio_vsock_init(void) >> return -ENOMEM; >> >> ret = vsock_core_register(&virtio_transport.transport, >> - VSOCK_TRANSPORT_F_G2H); >> + VSOCK_TRANSPORT_F_G2H | >> VSOCK_TRANSPORT_F_DGRAM); >> if (ret) >> goto out_wq; >> >> diff --git a/net/vmw_vsock/virtio_transport_common.c >> b/net/vmw_vsock/virtio_transport_common.c >> index bdf16fff054f..aedb48728677 100644 >> --- a/net/vmw_vsock/virtio_transport_common.c >> +++ b/net/vmw_vsock/virtio_transport_common.c >> @@ -229,7 +229,9 @@ EXPORT_SYMBOL_GPL(virtio_transport_deliver_tap_pkt); >> >> static u16 virtio_transport_get_type(struct sock *sk) >> { >> - if (sk->sk_type == SOCK_STREAM) >> + if (sk->sk_type == SOCK_DGRAM) >> + return VIRTIO_VSOCK_TYPE_DGRAM; >> + else if (sk->sk_type == SOCK_STREAM) >> return VIRTIO_VSOCK_TYPE_STREAM; >> else >> return VIRTIO_VSOCK_TYPE_SEQPACKET; >> @@ -287,22 +289,29 @@ static int virtio_transport_send_pkt_info(struct >> vsock_sock *vsk, >> vvs = vsk->trans; >> >> /* we can send less than pkt_len bytes */ >> - if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) >> - pkt_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE; >> + if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE) { >> + if (info->type != VIRTIO_VSOCK_TYPE_DGRAM) >> + pkt_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE; >> + else >> + return 0; >> + } >> >> - /* 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) { >> + /* 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; >> + /* Do not send zero length OP_RW pkt */ >> + if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW) >> + return pkt_len; >> + } >> >> skb = virtio_transport_alloc_skb(info, pkt_len, >> src_cid, src_port, >> dst_cid, dst_port, >> &err); >> if (!skb) { >> - virtio_transport_put_credit(vvs, pkt_len); >> + if (info->type != VIRTIO_VSOCK_TYPE_DGRAM) >> + virtio_transport_put_credit(vvs, pkt_len); >> return err; >> } >> >> @@ -586,6 +595,61 @@ virtio_transport_seqpacket_dequeue(struct vsock_sock >> *vsk, >> } >> EXPORT_SYMBOL_GPL(virtio_transport_seqpacket_dequeue); >> >> +static ssize_t >> +virtio_transport_dgram_do_dequeue(struct vsock_sock *vsk, >> + struct msghdr *msg, size_t len) >> +{ >> + struct virtio_vsock_sock *vvs = vsk->trans; >> + struct sk_buff *skb; >> + size_t total = 0; >> + u32 free_space; >> + int err = -EFAULT; >> + >> + spin_lock_bh(&vvs->rx_lock); >> + if (total < len && !skb_queue_empty_lockless(&vvs->rx_queue)) { >> + skb = __skb_dequeue(&vvs->rx_queue); >> + >> + total = len; >> + if (total > skb->len - vsock_metadata(skb)->off) >> + total = skb->len - vsock_metadata(skb)->off; >> + else if (total < skb->len - vsock_metadata(skb)->off) >> + msg->msg_flags |= MSG_TRUNC; >> + >> + /* sk_lock is held by caller so no one else can dequeue. >> + * Unlock rx_lock since memcpy_to_msg() may sleep. >> + */ >> + spin_unlock_bh(&vvs->rx_lock); >> + >> + err = memcpy_to_msg(msg, skb->data + vsock_metadata(skb)->off, >> total); >> + if (err) >> + return err; >> + >> + spin_lock_bh(&vvs->rx_lock); >> + >> + virtio_transport_dec_rx_pkt(vvs, skb); >> + consume_skb(skb); >> + } >> + >> + free_space = vvs->buf_alloc - (vvs->fwd_cnt - vvs->last_fwd_cnt); >> + >> + spin_unlock_bh(&vvs->rx_lock); >> + >> + if (total > 0 && msg->msg_name) { >> + /* Provide the address of the sender. */ >> + DECLARE_SOCKADDR(struct sockaddr_vm *, vm_addr, msg->msg_name); >> + >> + vsock_addr_init(vm_addr, le64_to_cpu(vsock_hdr(skb)->src_cid), >> + le32_to_cpu(vsock_hdr(skb)->src_port)); >> + msg->msg_namelen = sizeof(*vm_addr); >> + } >> + return total; >> +} >> + >> +static s64 virtio_transport_dgram_has_data(struct vsock_sock *vsk) >> +{ >> + return virtio_transport_stream_has_data(vsk); >> +} >> + >> int >> virtio_transport_seqpacket_enqueue(struct vsock_sock *vsk, >> struct msghdr *msg, >> @@ -611,7 +675,66 @@ virtio_transport_dgram_dequeue(struct vsock_sock *vsk, >> struct msghdr *msg, >> size_t len, int flags) >> { >> - return -EOPNOTSUPP; >> + struct sock *sk; >> + size_t err = 0; >> + long timeout; >> + >> + DEFINE_WAIT(wait); >> + >> + sk = &vsk->sk; >> + err = 0; >> + >> + if (flags & MSG_OOB || flags & MSG_ERRQUEUE || flags & MSG_PEEK) >> + return -EOPNOTSUPP; >> + >> + lock_sock(sk); >> + >> + if (!len) >> + goto out; >> + >> + timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT); >> + >> + while (1) { >> + s64 ready; >> + >> + prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE); >> + ready = virtio_transport_dgram_has_data(vsk); >> + >> + if (ready == 0) { >> + if (timeout == 0) { >> + err = -EAGAIN; >> + finish_wait(sk_sleep(sk), &wait); >> + break; >> + } >> + >> + release_sock(sk); >> + timeout = schedule_timeout(timeout); >> + lock_sock(sk); >> + >> + if (signal_pending(current)) { >> + err = sock_intr_errno(timeout); >> + finish_wait(sk_sleep(sk), &wait); >> + break; >> + } else if (timeout == 0) { >> + err = -EAGAIN; >> + finish_wait(sk_sleep(sk), &wait); >> + break; >> + } >> + } else { >> + finish_wait(sk_sleep(sk), &wait); >> + >> + if (ready < 0) { >> + err = -ENOMEM; >> + goto out; >> + } >> + >> + err = virtio_transport_dgram_do_dequeue(vsk, msg, len); >> + break; >> + } >> + } >> +out: >> + release_sock(sk); >> + return err; >> } >> EXPORT_SYMBOL_GPL(virtio_transport_dgram_dequeue); >> >> @@ -819,13 +942,13 @@ EXPORT_SYMBOL_GPL(virtio_transport_stream_allow); >> int virtio_transport_dgram_bind(struct vsock_sock *vsk, >> struct sockaddr_vm *addr) >> { >> - return -EOPNOTSUPP; >> + return vsock_bind_stream(vsk, addr); >> } >> EXPORT_SYMBOL_GPL(virtio_transport_dgram_bind); >> >> bool virtio_transport_dgram_allow(u32 cid, u32 port) >> { >> - return false; >> + return true; >> } >> EXPORT_SYMBOL_GPL(virtio_transport_dgram_allow); >> >> @@ -861,7 +984,16 @@ 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, >> + }; >> + >> + return virtio_transport_send_pkt_info(vsk, &info); >> } >> EXPORT_SYMBOL_GPL(virtio_transport_dgram_enqueue); >> >> @@ -1165,6 +1297,12 @@ virtio_transport_recv_connected(struct sock *sk, >> struct virtio_vsock_hdr *hdr = vsock_hdr(skb); >> int err = 0; >> >> + if (le16_to_cpu(vsock_hdr(skb)->type) == VIRTIO_VSOCK_TYPE_DGRAM) { >> + virtio_transport_recv_enqueue(vsk, skb); >> + sk->sk_data_ready(sk); >> + return err; >> + } >> + >> switch (le16_to_cpu(hdr->op)) { >> case VIRTIO_VSOCK_OP_RW: >> virtio_transport_recv_enqueue(vsk, skb); >> @@ -1320,7 +1458,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 >> @@ -1384,6 +1523,11 @@ void virtio_transport_recv_pkt(struct >> virtio_transport *t, >> goto free_pkt; >> } >> >> + if (sk->sk_type == SOCK_DGRAM) { >> + virtio_transport_recv_connected(sk, skb); >> + goto out; >> + } >> + >> space_available = virtio_transport_space_update(sk, skb); >> >> /* Update CID in case it has changed after a transport reset event */ >> @@ -1415,6 +1559,7 @@ void virtio_transport_recv_pkt(struct virtio_transport >> *t, >> break; >> } >> >> +out: >> release_sock(sk); >> >> /* Release refcnt obtained when we fetched this socket out of the >> -- >> 2.35.1 >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org >