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

From: Stefan Hajnoczi <stefanha@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,
	Arseny Krasnov <arseny.krasnov@kaspersky.com>,
	asias@redhat.com
Subject: Re: [External] Re: [RFC PATCH] virtio-vsock: add description for datagram type
Date: Mon, 29 Mar 2021 10:25:49 +0100	[thread overview]
Message-ID: <YGGdHfgJF45GRp5n@stefanha-x1.localdomain> (raw)
In-Reply-To: <CAP_N_Z9g3VUeLGtC0D0EGmHk7D1RxFQvUBMzLDMkct5b=t6x3A@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2070 bytes --]

On Fri, Mar 26, 2021 at 04:40:09PM -0700, Jiang Wang . wrote:
> I thought about this and discussed it with my colleague Cong Wang.
> One idea is to make current asynchronous send_pkt flow to be synchronous,
> then if the virtqueue is full, the function can return  ENOMEM all the way back
> to the caller and the caller can check the return value of sendmsg
> and slow down when necessary.
> 
> In the spec, we can put something like, if the virtqueue is full, the caller
> should be notified with an error etc.
> 
> In terms of implementation, that means we will remove the current
> send_pkt_work for both stream and dgram sockets. Currently, the
> code path uses RCU and a work queue, then grab a mutex in the
> work queue function. Since we cannot grab mutex when in rcu
> critical section, we have to change RCU to a normal reference
> counting mechanism. I think this is doable. The drawback is
> that the reference counting in general spends a little more
> cycles than the RCU, so there is a small price to pay. Another
> option is to use Sleepable RCU and remove the work queue.
> 
> What do you guys think?

I think the tx code path is like this because of reliable delivery.
Maybe a separate datagram rx/tx code path would be simpler?

Take the datagram tx virtqueue lock, try to add the packet into the
virtqueue, and return -ENOBUFS if the virtqueue is full. Then use the
datagram socket's sndbuf accounting to prevent queuing up too many
packets. When a datagram tx virtqueue buffer is used by the device,
select queued packets for transmission. Unlike the stream tx/rx code
path there is no dependency between tx and rx because we don't have the
credit mechanism.

> btw, I will also add some SENDBUF restrictions for the dgram
> sockets, but I don't think it needs to be in the spec.

Yes, the spec doesn't need to explain implementation-specific issues.

If there are common implementation issues then the spec can explain them
in general terms (not referring to Linux internals) to help
implementors.

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

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