Re: vsock: device expects at least 4 KB buffers in the rx virtqueue

["Michael S. Tsirkin" <mst@redhat.com>]

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

Also, there are implementations outside vhost. Can you check these
please? Did they make the 4K buffer assumption too?

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


> Any suggestion?
> 
> 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