Jiang Wang .
2021-Feb-13 23:26 UTC
[External] Re: vsock virtio: questions about supporting DGRAM type
On Fri, Feb 12, 2021 at 1:02 AM Stefano Garzarella <sgarzare at redhat.com> wrote:> > 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. >I see. thanks.> > > >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? >As of now, I don't have any threshold to drop packets on RX side. In my view, DGRAM is like UDP and is a best effort service. If the virtual queue is full on TX (or the available buffer size is less than the packet size), I drop the packets on the TX 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. >Good to know.> >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. >I did not consider the nested VMs when I started working on this. I just checked your nested VM support patch, and I agree it is harder to support it for DGRAM. One idea is that we can also prepare two transport layers for DGRAM ( similar to STREAM) and assign transport for every DGRAM packet. The downside is that it will introduce some overhead. I will think about it more.> 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.Sure. I will check Arseny's patch.> 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. >Got it. Thanks. We have some use cases for DGRAM. Basically, we send some kind of non-critical logging data from guest to the host. It is one way communication and does not require reliable delivery. I will continue working on the patch and send it out for review when it is ready. Thanks again for all the inputs.> Thanks, > Stefano > > [1] > https://lore.kernel.org/lkml/20210207151451.804498-1-arseny.krasnov at kaspersky.com/T/#m81d3ee4ccd54cd301b92c00b3b1bca6e7bc91fc9 >
Stefano Garzarella
2021-Feb-15 08:31 UTC
[External] Re: vsock virtio: questions about supporting DGRAM type
On Sat, Feb 13, 2021 at 03:26:18PM -0800, Jiang Wang . wrote:>On Fri, Feb 12, 2021 at 1:02 AM Stefano Garzarella <sgarzare at redhat.com> wrote: >> >> 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. >> > >I see. thanks. > >> > >> >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? >> > >As of now, I don't have any threshold to drop packets on RX side. In my >view, DGRAM is like UDP and is a best effort service. If the virtual >queue >is full on TX (or the available buffer size is less than the packet size), >I drop the packets on the TX side.I have to check the code, my concern is we should have a limit for the allocation, for example if the user space doesn't consume RX packets.> >> >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. >> >Good to know. > >> >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. >> > >I did not consider the nested VMs when I started working on this. I >just checked your >nested VM support patch, and I agree it is harder to support it for DGRAM. One >idea is that we can also prepare two transport layers for DGRAM ( >similar to STREAM)Yeah, or just remove the VSOCK_TRANSPORT_F_DGRAM and use the same flags used for STREAM also for DGRAM. If the transport does not support DGRAM, we return an error.>and assign transport for every DGRAM packet. The downside is that it >will >introduce some overhead. I will think about it more.I agree that maybe we should check the right transport for every DGRAM packet.> >> 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. > >Sure. I will check Arseny's patch. > >> 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. >> >Got it. Thanks. We have some use cases for DGRAM. Basically, we send some >kind of non-critical logging data from guest to the host. It is one way >communication and does not require reliable delivery. I will continue >working on the patch and send it out for review when it is ready. >Thanks again for all the inputs.I see, in fact DGRAM might make sense when we have a single receiver (host) and many transmitters (guests). Thanks, Stefano