Re: [External] Re: [RFC PATCH] virtio-vsock: add description for datagram type

From: Stefano Garzarella <sgarzare@redhat.com>
To: "Jiang Wang ." <jiang.wang@bytedance.com>
Cc: cong.wang@bytedance.com,
	Xiongchun Duan <duanxiongchun@bytedance.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	cohuck@redhat.com, virtualization@lists.linux-foundation.org,
	xieyongji@bytedance.com, Stefan Hajnoczi <stefanha@redhat.com>,
	asias@redhat.com, Arseny Krasnov <arseny.krasnov@kaspersky.com>
Subject: Re: [External] Re: [RFC PATCH] virtio-vsock: add description for datagram type
Date: Tue, 30 Mar 2021 17:32:31 +0200	[thread overview]
Message-ID: <20210330153231.duj6wqnekztdherp@steredhat> (raw)
In-Reply-To: <CAP_N_Z9g3VUeLGtC0D0EGmHk7D1RxFQvUBMzLDMkct5b=t6x3A@mail.gmail.com>

Hi Jiang,

On Fri, Mar 26, 2021 at 04:40:09PM -0700, Jiang Wang . wrote:
>Hi Michael and Stefan,
>
>I thought about this and discussed it with my colleague Cong Wang.
>One idea is to make current asynchronous send_pkt flow to be synchronous,
>then if the virtqueue is full, the function can return  ENOMEM all the way back
>to the caller and the caller can check the return value of sendmsg
>and slow down when necessary.
>
>In the spec, we can put something like, if the virtqueue is full, the caller
>should be notified with an error etc.
>
>In terms of implementation, that means we will remove the current
>send_pkt_work for both stream and dgram sockets. Currently, the
>code path uses RCU and a work queue, then grab a mutex in the
>work queue function. Since we cannot grab mutex when in rcu
>critical section, we have to change RCU to a normal reference
>counting mechanism. I think this is doable. The drawback is
>that the reference counting in general spends a little more
>cycles than the RCU, so there is a small price to pay. Another
>option is to use Sleepable RCU and remove the work queue.
>
>What do you guys think?

another thing that came to mind not related to the spec but to the Linux 
implementation, is the multi-transport support.
When we discussed the initial proposals [1][2], we decided to take a 
shortcut for DGRAM, since the only transport with DGRAM support was 
vmci. So for now only a single transport with VSOCK_TRANSPORT_F_DGRAM 
set can be registered.

Since also virtio-transport and vhost-transport will support DGRAM, we 
need to find a way to allow multiple transports that support DGRAM to be 
registered together to support nested VMs.

Do you have already thought about how to solve this problem?

We should definitely allow the registration of more transports with 
VSOCK_TRANSPORT_F_DGRAM, and dynamically choose which one to use when 
sending a packet.

Thanks,
Stefano

[1] https://www.spinics.net/lists/netdev/msg570274.html
[2] https://www.spinics.net/lists/netdev/msg575792.html

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization