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: Mon, 10 May 2021 16:50:55 +0200	[thread overview]
Message-ID: <20210510145055.y7mxqaq4zggajz5a@steredhat> (raw)
In-Reply-To: <CAP_N_Z9yZ4ydXaEn1e=70pDh3FyDjrrxgzis4YTfyDoZ5c8k+g@mail.gmail.com>

On Fri, May 07, 2021 at 09:53:19AM -0700, Jiang Wang . wrote:
>Hi guys,
>
>I have one question about adding two new virtqueues for dgram. One new
>thought is that we don't add two new virtqueues but keep using existing
>virtqueues for both stream and dgram.
>
>At the beginning when I first thought about supporting dgram, I thought
>adding two new virtqueues would be easier and have better performance.
>But now, after the prototype is done, I think that to keep using
>existing virtqueues is also not complicated and could in fact be simpler.
>The performance difference may not be very big.

I honestly didn't think it was easier to use two new queues, quite the 
opposite.

>
>Original code has about 3 places which have assumptions about the
>virtqueues are only used by the stream. But we can change those codes.
>One place is to check pkt len. We can check only for stream pkts.
>Another two are in tx and rx code path where if queued replies pkts are
>too much, the code will stop the rx queue and resume later. We can keep
>that same logic. The dgram will be affected a little bit but that should
>be fine I think. Are there any other places that we should fix?

Did you take a look at Arseny's series?
I think he's already found the places where to check the type and it 
seems to me they are the places you listed.

>
>In short, the virtqueues are in a lower level and can support multiple
>flows and socket types. Use existing virtqueues also make it more
>compatible with old versions.

It's not clear to me how compatibility is improved. Can you elaborate on 
this?

>
>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.

Thanks,
Stefano

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