Virtualization - Mar 2021 - [External] Re: vsock virtio: questions about supporting DGRAM type

Hi guys,

I am working on supporting DGRAM type for virtio/vhost vsock. I
already did some work and a draft code is here (which passed my tests,
but still need some cleanup and only works from host to guest as of
now, will add host to guest soon):
https://github.com/Jiang1155/linux/commit/4e89736e0bce15496460ff411cb4694b143d1c3d
qemu changes are here:
https://github.com/Jiang1155/qemu/commit/7ab778801e3e8969ab98e44539943810a2fb03eb

Today, I just noticed that the Asias had an old version of virtio
which had both dgram and stream support, see this link:
https://kvm.vger.kernel.narkive.com/BMvH9eEr/rfc-v2-0-7-introduce-vm-sockets-virtio-transport#post1

But somehow, the dgram part seems never merged to upstream linux (the
stream part is merged). If so, does anyone know what is the reason for
this? Did we drop dgram support for some specific reason or the code
needs some improvement?

My current code differs from Asias' code in some ways. It does not use
credit and does not support fragmentation.  It basically adds two virt
queues and re-uses the existing functions for tx and rx ( there is
somewhat duplicate code for now, but I will try to make common
functions to reduce it). If we still want to support dgram in upstream
linux, which way do you guys recommend? If necessary, I can try to
base on Asias' old code and continue working on it. If there is
anything unclear, just let me know. Thanks.

Regards,

Jiang

Hi Jiang,

CCing Arseny who is working on SOCK_SEQPACKET support for virtio-vsock 
[1].

On Thu, Feb 11, 2021 at 10:04:34PM -0800, Jiang Wang .
wrote:
>Hi guys,
>
>I am working on supporting DGRAM type for virtio/vhost vsock. I
>already did some work and a draft code is here (which passed my tests,
>but still need some cleanup and only works from host to guest as of
>now, will add host to guest soon):
>https://github.com/Jiang1155/linux/commit/4e89736e0bce15496460ff411cb4694b143d1c3d
>qemu changes are here:
>https://github.com/Jiang1155/qemu/commit/7ab778801e3e8969ab98e44539943810a2fb03eb
>
>Today, I just noticed that the Asias had an old version of virtio
>which had both dgram and stream support, see this link:
>https://kvm.vger.kernel.narkive.com/BMvH9eEr/rfc-v2-0-7-introduce-vm-sockets-virtio-transport#post1
>
>But somehow, the dgram part seems never merged to upstream linux (the
>stream part is merged). If so, does anyone know what is the reason for
>this? Did we drop dgram support for some specific reason or the code
>needs some improvement?
I wasn't involved yet in virtio-vsock development when Asias posted that 
patches, so I don't know the exact reason.

Maybe could be related on how to handle the credit mechanism for a 
connection-less sockets and how to manage the packet queue, if for 
example no one is listening.
>
>My current code differs from Asias' code in some ways. It does not use
>credit and does not support fragmentation.  It basically adds two virt
If you don't use credit, do you have some threshold when you start to 
drop packets on the RX side?
>queues and re-uses the existing functions for tx and rx ( there is
This make sense, some time ago I was thinking about this and also came 
to the conclusion that 2 new virtqueues were needed to handle DGRAM 
traffic.
>somewhat duplicate code for now, but I will try to make common
>functions to reduce it). If we still want to support dgram in upstream
>linux, which way do you guys recommend? If necessary, I can try to
>base on Asias' old code and continue working on it. If there is
>anything unclear, just let me know. Thanks.
A problem I see is how to handle multiple transports to support nested 
VMs. Since the sockets are not connected, we can't assign them to a 
single transport.

Arseny is working on SOCK_SEQPACKET [1], it's similar to DGRAM, but it 
is connection oriented, so we can reuse most of the STREAM stuff and 
also the credit mechanism.

Maybe you can reuse some of the Arseny's stuff to handle the 
fragmentation.

For the channel type (lossless) I think SEQPACKET makes more sense, but 
if you have any use-cases for DGRAM and want to support it, you are 
welcome to send patches and I will be happy to review them.

Thanks,
Stefano

[1] 
https://lore.kernel.org/lkml/20210207151451.804498-1-arseny.krasnov at
kaspersky.com/T/#m81d3ee4ccd54cd301b92c00b3b1bca6e7bc91fc9

On Thu, Feb 11, 2021 at 10:04:34PM -0800, Jiang Wang .
wrote:
> Hi guys,
> 
> I am working on supporting DGRAM type for virtio/vhost vsock. I
> already did some work and a draft code is here (which passed my tests,
> but still need some cleanup and only works from host to guest as of
> now, will add host to guest soon):
>
https://github.com/Jiang1155/linux/commit/4e89736e0bce15496460ff411cb4694b143d1c3d
> qemu changes are here:
>
https://github.com/Jiang1155/qemu/commit/7ab778801e3e8969ab98e44539943810a2fb03eb
> 
> Today, I just noticed that the Asias had an old version of virtio
> which had both dgram and stream support, see this link:
>
https://kvm.vger.kernel.narkive.com/BMvH9eEr/rfc-v2-0-7-introduce-vm-sockets-virtio-transport#post1
> 
> But somehow, the dgram part seems never merged to upstream linux (the
> stream part is merged). If so, does anyone know what is the reason for
> this? Did we drop dgram support for some specific reason or the code
> needs some improvement?
It's not just code it's the spec that needs work.

See some issues pointed out here:

https://lists.oasis-open.org/archives/virtio-dev/201506/msg00003.html


> My current code differs from Asias' code in some ways. It does not use
> credit and does not support fragmentation.  It basically adds two virt
> queues and re-uses the existing functions for tx and rx ( there is
> somewhat duplicate code for now, but I will try to make common
> functions to reduce it). If we still want to support dgram in upstream
> linux, which way do you guys recommend? If necessary, I can try to
> base on Asias' old code and continue working on it. If there is
> anything unclear, just let me know. Thanks.
> 
> Regards,
> 
> Jiang

Hi Michael,

Sorry for the late reply. I just saw your email yesterday somehow.

I read the email thread you mentioned, and I think the issue with
dgram is that it may drop packets because the sender cannot track the
tx_cnt with subtracting it from peer_fwd_cnt.

I agree with Stefan that the dgram is a best-effort service and may
drop packets. For the sender, I just add a maximum buffer size to
limit the memory usage. On the receiving side, I reuse the existing
virtio_transport_inc_rx_pkt() that Stefano added a year ago to limit
the memory usage. This will avoid denial of service attack to the
other end (host or guest VM).

For the application of dgram, we will use it for some remote logging
application. The application running in the VM will write some logs to
a server running on the host. This is  one way communication and the
log is not critical.

Regards,

Jiang

On Tue, Feb 23, 2021 at 1:53 AM Michael S. Tsirkin <mst at redhat.com>
wrote:
>
> On Thu, Feb 11, 2021 at 10:04:34PM -0800, Jiang Wang . wrote:
> > Hi guys,
> >
> > I am working on supporting DGRAM type for virtio/vhost vsock. I
> > already did some work and a draft code is here (which passed my tests,
> > but still need some cleanup and only works from host to guest as of
> > now, will add host to guest soon):
> >
https://github.com/Jiang1155/linux/commit/4e89736e0bce15496460ff411cb4694b143d1c3d
> > qemu changes are here:
> >
https://github.com/Jiang1155/qemu/commit/7ab778801e3e8969ab98e44539943810a2fb03eb
> >
> > Today, I just noticed that the Asias had an old version of virtio
> > which had both dgram and stream support, see this link:
> >
https://kvm.vger.kernel.narkive.com/BMvH9eEr/rfc-v2-0-7-introduce-vm-sockets-virtio-transport#post1
> >
> > But somehow, the dgram part seems never merged to upstream linux (the
> > stream part is merged). If so, does anyone know what is the reason for
> > this? Did we drop dgram support for some specific reason or the code
> > needs some improvement?
>
> It's not just code it's the spec that needs work.
>
> See some issues pointed out here:
>
> https://lists.oasis-open.org/archives/virtio-dev/201506/msg00003.html
>
>
>
> > My current code differs from Asias' code in some ways. It does not
use
> > credit and does not support fragmentation.  It basically adds two virt
> > queues and re-uses the existing functions for tx and rx ( there is
> > somewhat duplicate code for now, but I will try to make common
> > functions to reduce it). If we still want to support dgram in upstream
> > linux, which way do you guys recommend? If necessary, I can try to
> > base on Asias' old code and continue working on it. If there is
> > anything unclear, just let me know. Thanks.
> >
> > Regards,
> >
> > Jiang
>