All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Jiang Liu <gerry@linux.alibaba.com>,
	qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v2] vhost-user.rst: add clarifying language about protocol negotiation
Date: Thu, 04 Mar 2021 18:11:26 +0000	[thread overview]
Message-ID: <87tupqg51c.fsf@linaro.org> (raw)
In-Reply-To: <YEEXhBClR6GRLDu6@stefanha-x1.localdomain>


Stefan Hajnoczi <stefanha@redhat.com> writes:

> On Wed, Mar 03, 2021 at 05:01:05PM -0500, Michael S. Tsirkin wrote:
>> On Wed, Mar 03, 2021 at 02:50:11PM +0000, Alex Bennée wrote:
>> Also, are we sure it's ok to send the messages and then send
>> VHOST_USER_SET_FEATURES with VHOST_USER_F_PROTOCOL_FEATURES clear?
>> Looks more like a violation to me ...
>
> Looking again I agree it would be a violation to omit
> VHOST_USER_F_PROTOCOL_FEATURES in VHOST_USER_SET_FEATURES.
>
> Previously I only looked at VHOST_USER_SET_PROTOCOL_FEATURES where the
> spec says:
>
>   Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
>   ``VHOST_USER_GET_FEATURES``.
>
> So negotiation is *not* necessary for sending
> VHOST_USER_SET_PROTOCOL_FEATURES.
>
> However, I missed that other features *do* require negotiation of
> VHOST_USER_F_PROTOCOL_FEATURES according to the spec. For example:
>
>   If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
>   ring is initialized in an enabled state.
>
> Now I think:
>
> 1. VHOST_USER_F_PROTOCOL_FEATURES *must* be included
>    VHOST_USER_SET_FEATURES if the master supports it.

So added by the master - still invisible to the guest?

>
> 2. VHOST_USER_SET_PROTOCOL_FEATURES does not require negotiation,
>    instead the master just needs to check that
>    VHOST_USER_F_PROTOCOL_FEATURES is included in the
>    VHOST_USER_GET_FEATURES reply. It's an exception.

OK I'm now thoroughly confused but I guess that's a good thing. However
if we make the changes to QEMU to honour this won't we break with
existing vhost-user receivers? We'll also need to track the state of a
SET_FEATURES has happened and then gate the sending of things like
reply_ack requests?

-- 
Alex Bennée


  reply	other threads:[~2021-03-04 18:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-03 14:50 [PATCH v2] vhost-user.rst: add clarifying language about protocol negotiation Alex Bennée
2021-03-03 17:56 ` Stefan Hajnoczi
2021-03-03 22:01 ` Michael S. Tsirkin
2021-03-04 11:00   ` Alex Bennée
2021-03-04 17:23   ` Stefan Hajnoczi
2021-03-04 18:11     ` Alex Bennée [this message]
2021-03-05 17:43       ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tupqg51c.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=gerry@linux.alibaba.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.