Re: [External] Re: vsock virtio: questions about supporting DGRAM type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 16, 2021 at 12:23:19AM -0800, Jiang Wang . wrote:
On Tue, Feb 16, 2021 at 12:09 AM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:

On Mon, Feb 15, 2021 at 08:50:36PM -0800, Jiang Wang . wrote:
>On Mon, Feb 15, 2021 at 12:31 AM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote:
>>
>> 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@xxxxxxxxxx> 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.
>>
>
>I think we already have a limit for the allocation. If a DGRAM client
>sends packets while no socket is bound on the server side ,
>the packet will be dropped when looking for the socket. If the socket is
>bound on the server side, but the server program somehow does not
>call recv or similar functions, the packets will be dropped when the
>buffer is full as in your previous patch at here:
>https://lore.kernel.org/patchwork/patch/1141083/
>If there are still other cases that we don't have protection, let me know and
>I can add some threshold.

Maybe I misunderstood, but I thought you didn't use credit for DGRAM
messages.

I don't use credit for DGRAM. But the dropping code I mentioned does not
rely on credit too. Just to make it clear, following is one part of code that
I referred to:

static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
struct virtio_vsock_pkt *pkt)
{
if (vvs->rx_bytes + pkt->len > vvs->buf_alloc)
return false;

vvs->rx_bytes += pkt->len;
return true;
}

Okay, now it's clear. The transmitter doesn't care about the receiver's credit, but the receiver uses that part of the credit mechanism to discard packets.

I think that's fine, but when you send the patches, explain it well in the cover letter/commit message.



If you use it to limit buffer occupancy, then it should be okay, but
again, I still have to look at the code :-)

Sure. I think you mean you need to look at my final patches. I will
send it later.  Just a friendly reminder, if you just want to get some
idea of my patches, you could check this link :
https://github.com/Jiang1155/linux/commit/4e89736e0bce15496460ff411cb4694b143d1c3d

It's my fault because I didn't have time, I saw the link. :-)

Remember that we should also plan changes to the VIRTIO spec for the DGRAM support.

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux