Re: [virtio-dev] [RFC PATCH] docs/interop: define STANDALONE protocol feature for vhost-user

[Stefan Hajnoczi <stefanha@gmail.com>]

On Wed, 26 Jul 2023 at 11:42, Erik Schilling <erik.schilling@linaro.org> wrote:
>
> On Tue Jul 4, 2023 at 4:54 PM CEST, Stefano Garzarella wrote:
> > On Tue, Jul 04, 2023 at 01:36:00PM +0100, Alex Bennée wrote:
> > >Currently QEMU has to know some details about the back-end to be able
> > >to setup the guest. While various parts of the setup can be delegated
> > >to the backend (for example config handling) this is a very piecemeal
> > >approach.
> > >
> > >This patch suggests a new feature flag (VHOST_USER_PROTOCOL_F_STANDALONE)
> > >which the back-end can advertise which allows a probe message to be
> > >sent to get all the details QEMU needs to know in one message.
> > >
> > >Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > >
> > >---
> > >Initial RFC for discussion. I intend to prototype this work with QEMU
> > >and one of the rust-vmm vhost-user daemons.
> >
> > Thanks for starting this discussion!
> >
> > I'm comparing with vhost-vdpa IOCTLs, so my questions may be
> > superficial, but they help me understand the differences.
> >
> > >---
> > > docs/interop/vhost-user.rst | 37 +++++++++++++++++++++++++++++++++++++
> > > hw/virtio/vhost-user.c      |  8 ++++++++
> > > 2 files changed, 45 insertions(+)
> > >
> > >diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > >index 5a070adbc1..85b1b1583a 100644
> > >--- a/docs/interop/vhost-user.rst
> > >+++ b/docs/interop/vhost-user.rst
> > >@@ -275,6 +275,21 @@ Inflight description
> > >
> > > :queue size: a 16-bit size of virtqueues
> > >
> > >+Backend specifications
> > >+^^^^^^^^^^^^^^^^^^^^^^
> > >+
> > >++-----------+-------------+------------+------------+
> > >+| device id | config size |   min_vqs  |   max_vqs  |
> > >++-----------+-------------+------------+------------+
> > >+
> > >+:device id: a 32-bit value holding the VirtIO device ID
> > >+
> > >+:config size: a 32-bit value holding the config size (see ``VHOST_USER_GET_CONFIG``)
> > >+
> > >+:min_vqs: a 32-bit value holding the minimum number of vqs supported
> >
> > Why do we need the minimum?
> >
> > >+
> > >+:max_vqs: a 32-bit value holding the maximum number of vqs supported, must be >= min_vqs
> >
> > Is this overlap with VHOST_USER_GET_QUEUE_NUM?
>
> While fiddeling with a rust-vmm implementation of this I wondered:
>
> Would a standalone daemon even need VHOST_USER_PROTOCOL_F_MQ if
> VHOST_USER_PROTOCOL_F_CONFIG is required anyway? It looks like
> all full virtio devices provide config information that allows to
> derive the exact or maximum number of queues already. So wouldn't
> VHOST_USER_PROTOCOL_F_MQ just provide a second, redundant way to get
> the information? (And this would be a third?)
>
> Am I missing something here? Otherwise, I think we could drop dependency
> on VHOST_USER_PROTOCOL_F_MQ and get the info from the device config for
> standalone daemons?

vhost (in general and that includes vhost-user) was not designed to be
a full VIRTIO device, just an interface for offloading specific
virtqueues. Now that vhost-user is being extended to support full
VIRTIO devices ("standalone"), some of the vhost-user protocol
features are redundant.

However, actually dropping the redundant features must be done
carefully because they might be used in ways that you don't expect by
existing vhost-user implementations. For example, maybe existing
vhost-user-net code doesn't look at the VIRTIO Configuration Space
field because it queries the number of virtqueues via
VHOST_USER_PROTOCOL_F_MQ + VHOST_USER_GET_QUEUE_NUM. So you might be
forced to keep them for compatibility.

I think the answer to your questions is: yes, but carefully because
you might break existing implementations or make it hard for them to
remain compatible.

Stefan