[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: vsock: device expects at least 4 KB buffers in the rx virtqueue
On Fri, Nov 15, 2019 at 09:06:36AM -0500, Michael S. Tsirkin wrote: > On Fri, Nov 15, 2019 at 12:31:17PM +0100, Stefano Garzarella wrote: > > Hi, > > following the discussion with Michael [1], the device emulation > > implemented in vhost-vsock made an assumption on buffers at least 4 KB > > in the rx virtqueue. > > > > This patch [2] removed the assumption unintentionally, but vhost-vsock > > modules released before Linux v5.4 still have this limitation. > > > > The current implementation continues to use 4 KB buffers, so for now, we > > are backward compatible, but we should add something in the > > specifications. > > > > I'm not sure how to proceed, my ideas are the following: > > > > 1. Add a note like this: > > diff --git a/virtio-vsock.tex b/virtio-vsock.tex > > index da7e641..4c8f65d 100644 > > --- a/virtio-vsock.tex > > +++ b/virtio-vsock.tex > > @@ -198,6 +198,9 @@ \subsubsection{Receive and Transmit}\label{sec:Device Types / Socket Device / De > > Virtqueue buffers for outgoing packets are read-only. Virtqueue buffers for > > incoming packets are write-only. > > > > +Note: The device implementation (vhost-vsock) in Linux prior to v5.4 requires > > +that the receive buffers in the rx virtqueue MUST be at least 4KBytes. > > + > > Question is how big a fix it is for all devices? The only device affected seems to be the vhost-vsock implementation (the other implementation that I found is the Firecracker emulation, see later) and the fix [2] should easily be applied on stable branches, since that code hasn't changed much since the introduction. This is the diffstat of the patch: drivers/vhost/vsock.c | 66 +++++++++++++++++++++++++++++++++++------------- net/vmw_vsock/virtio_transport_common.c | 15 ++++++++--- 2 files changed, 60 insertions(+), 21 deletions(-) > It is attractive to fix it in the correct place, which is host kernel. > If we are to fix it I'd start with putting the patch in stable, > and red hat kernels over which we have control. Okay, I'll ask Dave to consider the patch also for stable branches, and I'll look for RH kernels. > > Also, there are implementations outside vhost. Can you check these > please? Did they make the 4K buffer assumption too? The other sole implementation that I found is in Firecracker [3] and looking at the code [4], they don't do any assumption and look at the buffer size taken from the RX virtqueue. > > > \drivernormative{\paragraph}{Device Operation: Receive and Transmit}{Device Types / Socket Device / Device Operation / Receive and Transmit} > > > > The \field{guest_cid} configuration field MUST be used as the source CID when > > > > 2. Add a new feature to tell that the device supports any buffer size in > > the rx queue > > > > 3. Add a new feature to support mergeable buffers (in my plans), > > avoiding the feature at point 2 > > Assuming we make it a new feature, I think either 2 or 3. > Okay, maybe 3 would be cleaner and less related to implementation in vhost-vsock. Thanks, Stefano > [1] https://lkml.org/lkml/2019/9/2/676 > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6dbd3e66e7785a2f055bf84d98de9b8fd31ff3f5 [3] https://github.com/firecracker-microvm/firecracker/tree/master/devices/src/virtio/vsock [4] https://github.com/firecracker-microvm/firecracker/blob/81592a1e21084c4f8e02bfafc9b1122c9db4254e/devices/src/virtio/vsock/csm/connection.rs#L217
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]