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

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

On Tue, Mar 30, 2021 at 8:32 AM Stefano Garzarella <sgarzare at
redhat.com> wrote:
>
> 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.
Got it. We briefly discussed multiple dgram transport
support in my old email thread. At that time, I was thinking
maybe check for transport for each packet. I did not spend more
time on that part after that. I will think about it more and get
back to you. Thanks
> Thanks,
> Stefano
>
> [1] https://www.spinics.net/lists/netdev/msg570274.html
> [2] https://www.spinics.net/lists/netdev/msg575792.html
>