Re: [RFC v2] virtio-vsock: add description for datagram type

From: Stefano Garzarella <sgarzare@redhat.com>
To: "Jiang Wang ." <jiang.wang@bytedance.com>
Cc: cong.wang@bytedance.com,
	Xiongchun Duan <duanxiongchun@bytedance.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	cohuck@redhat.com, virtualization@lists.linux-foundation.org,
	xieyongji@bytedance.com, Stefan Hajnoczi <stefanha@redhat.com>,
	Arseny Krasnov <arseny.krasnov@kaspersky.com>
Subject: Re: [RFC v2] virtio-vsock: add description for datagram type
Date: Fri, 14 May 2021 17:17:01 +0200	[thread overview]
Message-ID: <20210514151701.6fp27qanjseom4tl@steredhat> (raw)
In-Reply-To: <CAP_N_Z94Pi4k8Dv6cHR0CZ9RTLJeQ3VWQoQgLTCWE4k+A01xbg@mail.gmail.com>

On Thu, May 13, 2021 at 04:26:03PM -0700, Jiang Wang . wrote:
>On Mon, May 10, 2021 at 7:52 AM Stefano Garzarella <sgarzare@redhat.com> wrote:
>> On Fri, May 07, 2021 at 09:53:19AM -0700, Jiang Wang . wrote:

[...]

>I was thinking if we don't add two new virtqueues, then maybe we don't
>need to add new feature bit too? If the other end does not support
>dgram, then the packets will be just dropped. What do you think? Do
>we still need to add dgram feature bits? I can have a feature bit for
>mergeable buffer.

With seqpacket, where we reuse stream queues, we decided to add the new 
feature bit, so I guess we should do the same for dgram.

In this way the driver knows if the protocol is supported and we can 
avoid for example to open a listening socket.

Without the feature bit this would not be possible. I mean, the sender 
will get an error, but the receiver will never know if it can receive or 
not.

>> >What do you guys think? I remember Stefano mentioned that we should 
>> >add
>> >two new virtqueues for dgram. Stefano, do you have some specific reasons
>> >for that? Could we just keep using existing virtqueues? Thanks.
>>
>> My biggest concern was about the credit mechanism for datagrams. I mean
>> avoiding datagrams from crowding the queue without limits, preventing
>> streams from communicating.
>>
>> If you've found a way to limit datagram traffic, then maybe it's doable.
>
>I see. I will add some limit to dgram packets. Also, when the virtqueues
>are shared between stream and dgram, both of them need to grab a lock
>before using the virtqueue, so one will not completely block another one.

I'm not worried about the concurrent access that we definitely need to 
handle with a lock, but more about the uncontrolled packet sending that 
dgram might have, flooding the queues and preventing others from 
communicating.

So having 2 dedicated queues could avoid a credit mechanism at all for 
connection-less sockets, and simply the receiver discards packets that 
it can't handle.

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization