On Wed, Mar 03, 2021 at 02:50:11PM +0000, Alex Bennée wrote:
> Make the language about feature negotiation explicitly clear about the
> handling of the VHOST_USER_F_PROTOCOL_FEATURES feature bit. Try and
> avoid the sort of bug introduced in vhost.rs REPLY_ACK processing:
>
> https://github.com/rust-vmm/vhost/pull/24
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
> Cc: Jiang Liu <ge...@linux.alibaba.com>
> Message-Id: <20210226111619.21178-1-alex.ben...@linaro.org>
>
> ---
> v2
> - use Stefan's suggested wording
> - Be super explicit in the message descriptions
> ---
> docs/interop/vhost-user.rst | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 2918d7c757..7c1fb8c209 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -307,6 +307,18 @@ bit was dedicated for this purpose::
>
> #define VHOST_USER_F_PROTOCOL_FEATURES 30
>
> +Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature
> +bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits
> +<https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_.
> +VIRTIO devices do not advertise this feature bit and therefore VIRTIO
> +drivers cannot negotiate it.
> +
> +This reserved feature bit was reused by the vhost-user protocol to add
> +vhost-user protocol feature negotiation in a backwards compatible
> +fashion. Old vhost-user master and slave implementations continue to
> +work even though they are not aware of vhost-user protocol feature
> +negotiation.
> +
> Ring states
> -----------
>
> @@ -865,7 +877,8 @@ Front-end message types
> Get the protocol feature bitmask from the underlying vhost
> implementation. Only legal if feature bit
> ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
> - ``VHOST_USER_GET_FEATURES``.
> + ``VHOST_USER_GET_FEATURES``. It does not need to be acknowledged by
> + ``VHOST_USER_SET_FEATURES``.
>
> .. Note::
> Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must
> @@ -881,7 +894,8 @@ Front-end message types
> Enable protocol features in the underlying vhost implementation.
>
> Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
> - ``VHOST_USER_GET_FEATURES``.
> + ``VHOST_USER_GET_FEATURES``. It does not need to be acknowledged by
> + ``VHOST_USER_SET_FEATURES``.
>
> .. Note::
> Back-ends that report ``VHOST_USER_F_PROTOCOL_FEATURES`` must support
Not really clear what does "It" refer to here.
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 ...
How about: It -> this bit
does not need to be -> before ... has been
so:
Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in
- ``VHOST_USER_GET_FEATURES``, and even before this bit has been
acknowledged by VHOST_USER_SET_FEATURES.
> --
> 2.20.1
