On Mon, Feb 15, 2021 at 12:53 AM Arseny Krasnov <arseny.krasnov@xxxxxxxxxxxxx> wrote: > > > On 14.02.2021 02:46, Jiang Wang . wrote: > > On Fri, Feb 12, 2021 at 7:19 AM Arseny Krasnov > > <arseny.krasnov@xxxxxxxxxxxxx> wrote: > >> > >> On 12.02.2021 12:02, Stefano Garzarella 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. > >>> > >>>> 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. > >> Sorry, but what is purpose of two new virt queues? > >> > > The two new virt queues will be dedicated for the DGRAM packets. Current > > queues are used by STREAM (and your SEQPACKET) and the credit > > mechanism is bound up with current queues. For DGRAM, the packets can be lost > > and no need to use the credit based mechanism. Allocating two new queues > > will make this simpler and DGRAM will not interfere with the STREAM > > or SEQPACKET. > > I think that credit mechanism is per socket(e.g. every socket has > > its own "free space"). If socket has free space, but queue is full, > > packet still inserted in tx list. Otherwise if queue is empty, but > > receiver didn't send credit update to make sender happy(free space > > is 0) - sender will sleep. I think both of them are independent. Thanks for the explanation and I agree with your above description. The data structure for credit is per socket. But there is a subtle assumption for STREAM or connection oriented communications. When the client sends the first connect message to the server, it does not know the credit of the server. Current code relies on the pkt_len == 0 to send it out, as the following: /* 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; DGRAM will be different. DGRAM does not require a connect call, and the first message will have a non zero pkt_len. We could somehow improve the current credit mechanism and make it work for DGRAM too, but I don't think it is a strict requirement for DGRAM. Going back to the original question about why adding two more new virtqueues, another reason is for better performance and performance isolation. If DGRAM and STREAM share the same queues, and both have heavy in-flight data, I think the throughput of the STREAM will be decreased. Also, if the DGRAM server somehow does not process packets for a long time, the packets will be kept in the queue and also interfere with the STREAM packets. I think adding two more queues without a credit mechanism is the best option for DGRAM. If there are other reasons to use credit or not adding two more queues, please let me know. Thanks > > > >>>> 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. > >> Yes, just apply patchset to latest kernel. I've solved problem > >> > >> with fragmentation at transport layer. > >> > > Got it. Thanks. I will check it out. > > > >>> 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@xxxxxxxxxxxxx/T/#m81d3ee4ccd54cd301b92c00b3b1bca6e7bc91fc9 > >>> > >>> > >> Thanks, Arseny > >> _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
