All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] vhost-user.rst: clarify when rings are started
@ 2026-04-29 18:55 Stefan Hajnoczi
  2026-05-07 10:17 ` Stefano Garzarella
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Hajnoczi @ 2026-04-29 18:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: hreitz, Michael S. Tsirkin, gmaglione, Stefano Garzarella,
	Pierrick Bouvier, Pierrick Bouvier, Stefan Hajnoczi,
	Jorge Moreira

Jorge Moreira <jemoreira@google.com> pointed out that the ring state
machine is underspecified. In the discussion that followed, we
discovered that the spec says one thing and implementations do something
else. This patch updates the spec to reflect how things are actually
implemented across widely-used front-ends and back-ends including QEMU,
crosvm, rust-vmm, and DPDK. Do this while taking care not to make any
other existing implementations non-compliant by changing the sepc.

The spec says rings are started when a kick is received but the
implementations actually start rings when VHOST_USER_SET_VRING_KICK is
received.

Reconcile this as follows:
- Clarify that a ring can be stopped and then started again. The
  back-end must resume processing available requests when the ring is
  restarted.
- Update the spec to say rings are started when
  VHOST_USER_SET_VRING_KICK is received.
- Ensure compatibility by saying front-ends SHOULD inject a kick in case
  the back-end strictly implemented the old spec.
- Avoid future back-end dependencies on injected kicks by saying that
  back-ends SHOULD NOT expect a kick to start rings.

This way implementors have clarity on how things work while still
allowing compatibility for existing implementations.

Reported-by: Jorge Moreira <jemoreira@google.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
This is an RFC because I will update hw/virtio/vhost-user.c and
libvhost-user once we've agreed on the spec wording.

 docs/interop/vhost-user.rst | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 137c9f3669..72d792930e 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -462,12 +462,24 @@ Rings have two independent states: started/stopped, and enabled/disabled.
 * started and enabled: The back-end must process the ring normally, i.e.
   process all requests and execute them.
 
-Each ring is initialized in a stopped and disabled state.  The back-end
-must start a ring upon receiving a kick (that is, detecting that file
-descriptor is readable) on the descriptor specified by
-``VHOST_USER_SET_VRING_KICK`` or receiving the in-band message
-``VHOST_USER_VRING_KICK`` if negotiated, and stop a ring upon receiving
-``VHOST_USER_GET_VRING_BASE``.
+Each ring is initialized in a stopped and disabled state.  Rings are started
+with ``VHOST_USER_SET_VRING_KICK`` and stopped with
+``VHOST_USER_GET_VRING_BASE``. A stopped ring enters the started state again
+with ``VHOST_USER_SET_VRING_KICK`` and the back-end resumes processing
+requests.
+
+Note that previous versions of this specification stated that rings start when
+the back-end receives a kick (that is, detecting that file descriptor is
+readable) on the descriptor specified by ``VHOST_USER_SET_VRING_KICK`` or
+receiving the in-band message ``VHOST_USER_VRING_KICK`` if negotiated.
+Widely-used front-ends and back-ends did not implement this behavior and it
+complicates poll mode back-ends that do not rely on the kick file descriptor.
+
+For compatibility with back-ends that implemented the start on kick behavior,
+front-ends SHOULD inject a kick after ``VHOST_USER_SET_VRING_KICK``.  This
+ensures that the back-end processes any available requests in the ring.
+Back-ends SHOULD NOT rely on receiving a kick after
+``VHOST_USER_SET_VRING_KICK``.
 
 Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``.
 
-- 
2.54.0



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC] vhost-user.rst: clarify when rings are started
  2026-04-29 18:55 [RFC] vhost-user.rst: clarify when rings are started Stefan Hajnoczi
@ 2026-05-07 10:17 ` Stefano Garzarella
  2026-05-19 18:29   ` Stefan Hajnoczi
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2026-05-07 10:17 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-devel, hreitz, Michael S. Tsirkin, gmaglione,
	Pierrick Bouvier, Pierrick Bouvier, Jorge Moreira

On Wed, Apr 29, 2026 at 02:55:04PM -0400, Stefan Hajnoczi wrote:
>Jorge Moreira <jemoreira@google.com> pointed out that the ring state
>machine is underspecified. In the discussion that followed, we
>discovered that the spec says one thing and implementations do something
>else. This patch updates the spec to reflect how things are actually
>implemented across widely-used front-ends and back-ends including QEMU,
>crosvm, rust-vmm, and DPDK. Do this while taking care not to make any
>other existing implementations non-compliant by changing the sepc.
>
>The spec says rings are started when a kick is received but the
>implementations actually start rings when VHOST_USER_SET_VRING_KICK is
>received.
>
>Reconcile this as follows:
>- Clarify that a ring can be stopped and then started again. The
>  back-end must resume processing available requests when the ring is
>  restarted.
>- Update the spec to say rings are started when
>  VHOST_USER_SET_VRING_KICK is received.
>- Ensure compatibility by saying front-ends SHOULD inject a kick in case
>  the back-end strictly implemented the old spec.
>- Avoid future back-end dependencies on injected kicks by saying that
>  back-ends SHOULD NOT expect a kick to start rings.
>
>This way implementors have clarity on how things work while still
>allowing compatibility for existing implementations.
>
>Reported-by: Jorge Moreira <jemoreira@google.com>
>Cc: "Michael S . Tsirkin" <mst@redhat.com>
>Cc: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
>This is an RFC because I will update hw/virtio/vhost-user.c and
>libvhost-user once we've agreed on the spec wording.
>
> docs/interop/vhost-user.rst | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
>diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
>index 137c9f3669..72d792930e 100644
>--- a/docs/interop/vhost-user.rst
>+++ b/docs/interop/vhost-user.rst
>@@ -462,12 +462,24 @@ Rings have two independent states: started/stopped, and enabled/disabled.
> * started and enabled: The back-end must process the ring normally, i.e.
>   process all requests and execute them.
>
>-Each ring is initialized in a stopped and disabled state.  The back-end
>-must start a ring upon receiving a kick (that is, detecting that file
>-descriptor is readable) on the descriptor specified by
>-``VHOST_USER_SET_VRING_KICK`` or receiving the in-band message
>-``VHOST_USER_VRING_KICK`` if negotiated, and stop a ring upon receiving
>-``VHOST_USER_GET_VRING_BASE``.
>+Each ring is initialized in a stopped and disabled state.  Rings are started
>+with ``VHOST_USER_SET_VRING_KICK`` and stopped with

In the VHOST_USER_SET_VRING_KICK description we say:

                                    Note that if the protocol feature
   ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` has been negotiated
   this message isn't necessary as the ring is also started on the
   ``VHOST_USER_VRING_KICK`` message, it may however still be used to
   set an event file descriptor (which will be preferred over the
   message) or to enable polling.

I think we need to clarify a bit more what happens if 
VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS is negotiated. In that case, 
I don't think we can necessarily require the frontend to send 
VHOST_USER_SET_VRING_KICK to start the rings.

What about: Rings are started with ``VHOST_USER_SET_VRING_KICK`` (or 
``VHOST_USER_VRING_KICK`` if negotiated) ...

... A stopped ring enters the started state again with 
``VHOST_USER_SET_VRING_KICK`` (or ``VHOST_USER_VRING_KICK`` if 
negotiated) and the back-end resumes processing requests.


>+``VHOST_USER_GET_VRING_BASE``. A stopped ring enters the started state again
>+with ``VHOST_USER_SET_VRING_KICK`` and the back-end resumes processing
>+requests.
>+
>+Note that previous versions of this specification stated that rings start when
>+the back-end receives a kick (that is, detecting that file descriptor is
>+readable) on the descriptor specified by ``VHOST_USER_SET_VRING_KICK`` or
>+receiving the in-band message ``VHOST_USER_VRING_KICK`` if negotiated.
>+Widely-used front-ends and back-ends did not implement this behavior and it
>+complicates poll mode back-ends that do not rely on the kick file descriptor.
>+
>+For compatibility with back-ends that implemented the start on kick behavior,
>+front-ends SHOULD inject a kick after ``VHOST_USER_SET_VRING_KICK``.  This
>+ensures that the back-end processes any available requests in the ring.
>+Back-ends SHOULD NOT rely on receiving a kick after
>+``VHOST_USER_SET_VRING_KICK``.

The rest LGTM.

Thanks,
Stefano

>
> Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``.
>
>-- 
>2.54.0
>



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] vhost-user.rst: clarify when rings are started
  2026-05-07 10:17 ` Stefano Garzarella
@ 2026-05-19 18:29   ` Stefan Hajnoczi
  0 siblings, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2026-05-19 18:29 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: qemu-devel, hreitz, Michael S. Tsirkin, gmaglione,
	Pierrick Bouvier, Pierrick Bouvier, Jorge Moreira

[-- Attachment #1: Type: text/plain, Size: 4066 bytes --]

On Thu, May 07, 2026 at 12:17:14PM +0200, Stefano Garzarella wrote:
> On Wed, Apr 29, 2026 at 02:55:04PM -0400, Stefan Hajnoczi wrote:
> > Jorge Moreira <jemoreira@google.com> pointed out that the ring state
> > machine is underspecified. In the discussion that followed, we
> > discovered that the spec says one thing and implementations do something
> > else. This patch updates the spec to reflect how things are actually
> > implemented across widely-used front-ends and back-ends including QEMU,
> > crosvm, rust-vmm, and DPDK. Do this while taking care not to make any
> > other existing implementations non-compliant by changing the sepc.
> > 
> > The spec says rings are started when a kick is received but the
> > implementations actually start rings when VHOST_USER_SET_VRING_KICK is
> > received.
> > 
> > Reconcile this as follows:
> > - Clarify that a ring can be stopped and then started again. The
> >  back-end must resume processing available requests when the ring is
> >  restarted.
> > - Update the spec to say rings are started when
> >  VHOST_USER_SET_VRING_KICK is received.
> > - Ensure compatibility by saying front-ends SHOULD inject a kick in case
> >  the back-end strictly implemented the old spec.
> > - Avoid future back-end dependencies on injected kicks by saying that
> >  back-ends SHOULD NOT expect a kick to start rings.
> > 
> > This way implementors have clarity on how things work while still
> > allowing compatibility for existing implementations.
> > 
> > Reported-by: Jorge Moreira <jemoreira@google.com>
> > Cc: "Michael S . Tsirkin" <mst@redhat.com>
> > Cc: Stefano Garzarella <sgarzare@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > This is an RFC because I will update hw/virtio/vhost-user.c and
> > libvhost-user once we've agreed on the spec wording.
> > 
> > docs/interop/vhost-user.rst | 24 ++++++++++++++++++------
> > 1 file changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index 137c9f3669..72d792930e 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -462,12 +462,24 @@ Rings have two independent states: started/stopped, and enabled/disabled.
> > * started and enabled: The back-end must process the ring normally, i.e.
> >   process all requests and execute them.
> > 
> > -Each ring is initialized in a stopped and disabled state.  The back-end
> > -must start a ring upon receiving a kick (that is, detecting that file
> > -descriptor is readable) on the descriptor specified by
> > -``VHOST_USER_SET_VRING_KICK`` or receiving the in-band message
> > -``VHOST_USER_VRING_KICK`` if negotiated, and stop a ring upon receiving
> > -``VHOST_USER_GET_VRING_BASE``.
> > +Each ring is initialized in a stopped and disabled state.  Rings are started
> > +with ``VHOST_USER_SET_VRING_KICK`` and stopped with
> 
> In the VHOST_USER_SET_VRING_KICK description we say:
> 
>                                    Note that if the protocol feature
>   ``VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS`` has been negotiated
>   this message isn't necessary as the ring is also started on the
>   ``VHOST_USER_VRING_KICK`` message, it may however still be used to
>   set an event file descriptor (which will be preferred over the
>   message) or to enable polling.
> 
> I think we need to clarify a bit more what happens if
> VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS is negotiated. In that case, I
> don't think we can necessarily require the frontend to send
> VHOST_USER_SET_VRING_KICK to start the rings.
> 
> What about: Rings are started with ``VHOST_USER_SET_VRING_KICK`` (or
> ``VHOST_USER_VRING_KICK`` if negotiated) ...
> 
> ... A stopped ring enters the started state again with
> ``VHOST_USER_SET_VRING_KICK`` (or ``VHOST_USER_VRING_KICK`` if negotiated)
> and the back-end resumes processing requests.

Sounds good to me. I will send a non-RFC version including this change.

Stefan

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-19 18:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-29 18:55 [RFC] vhost-user.rst: clarify when rings are started Stefan Hajnoczi
2026-05-07 10:17 ` Stefano Garzarella
2026-05-19 18:29   ` Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.