[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC] vhost-user: protocol extensions
From: |
Olivier MATZ |
Subject: |
Re: [Qemu-devel] [PATCH RFC] vhost-user: protocol extensions |
Date: |
Mon, 20 Apr 2015 14:42:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.5.0 |
Hi Michael,
On 04/06/2015 04:48 PM, Michael S. Tsirkin wrote:
> This adds several extensions to the vhost user protocol:
> - protocol feature negotiation similar to virtio features
> - ability to report request failures
> - ability to start/stop specific rings
>
> I went over all vhost-user implementations I could find,
> and this seems to be compatible with them all.
>
> One thing that still bothers me is that the protocol
> as defined is not a transport in the virtio spec sense:
> for example, there is no concept of status bits,
> or config space. I'm still mulling whether this makes
> any sense.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> docs/specs/vhost-user.txt | 105
> +++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 95 insertions(+), 10 deletions(-)
>
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 650bb18..025ffe9 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -2,6 +2,7 @@ Vhost-user Protocol
> ===================
>
> Copyright (c) 2014 Virtual Open Systems Sarl.
> +Copyright (c) 2015 Red Hat, Inc.
>
> This work is licensed under the terms of the GNU GPL, version 2 or later.
> See the COPYING file in the top-level directory.
> @@ -35,11 +36,28 @@ consists of 3 header fields and a payload:
>
> * Request: 32-bit type of the request
> * Flags: 32-bit bit field:
> - - Lower 2 bits are the version (currently 0x01)
> + - Lower 2 bits are the version (currently 0x1 or 0x2)
> - Bit 2 is the reply flag - needs to be sent on each reply from the slave
> * Size - 32-bit size of the payload
>
>
> +Version negotiation:
> +---------------------
> +
> +Requester can set any version (at the moment 0x1 or 0x2) on all requests.
> +Responder has two options:
> +1. ignore version on requests, set version 0x1 on all replies. Such responder
> + is allowed to assume it will only get request codes that were allowed
> + to use version 0x1 (though requests themselves can use any version).
> +2. retrieve version from request and respond with same version in reply.
> + Such responder must ignore unexpected requests (and respond with
> + setting VHOST_USER_MESSAGE_ERROR if appropriate).
> +
> +In both cases, responder must ignore version for all other purposes,
> +for example, it must not check it and terminate the connection,
> +or ignore a message with an unexpected version value.
> +
If I understand well, it is the role of the requester to determine the
version of the responder. This would be done as soon as possible, for
instance in the VHOST_USER_GET_FEATURES message by setting the version=2
in the message and checking its value in the answer.
If the responder is version 1, the requester won't send any version=2
message (case 1.). As the VHOST_USER_MESSAGE_ERROR does not exist in
version 1, what is the expected reply from the responder for this first
message if it only supports version 1?
> +
> Depending on the request type, payload can be:
>
> * A single 64-bit integer
> @@ -110,26 +128,45 @@ implementing vhost-user have an equivalent ioctl to the
> kernel implementation.
>
> The communication consists of master sending message requests and slave
> sending
> message replies. Most of the requests don't require replies. Here is a list
> of
> -the ones that do:
> +the ones that always do:
> +
> + * VHOST_USER_GET_FEATURES
> + * VHOST_USER_GET_VRING_BASE
> + * VHOST_USER_GET_PROTOCOL_FEATURES
> + * VHOST_USER_GET_RUNNING
> +
> +If slave is unable to respond, it must mirror the request received,
> +and set VHOST_USER_MESSAGE_ERROR bit in the request field.
>
> - * VHOST_GET_FEATURES
> - * VHOST_GET_VRING_BASE
> +If VHOST_USER_PROTOCOL_F_REPLY_ALL protocol feature bit is negotiated, slave
> +must respond to all other messages by mirroring the requests received, and
> +setting the high bit VHOST_USER_MESSAGE_ERROR in Request field if the request
> +received is unsupported.
> + VHOST_USER_MESSAGE_ERROR 31
>
> There are several messages that the master sends with file descriptors passed
> in the ancillary data:
>
> - * VHOST_SET_MEM_TABLE
> - * VHOST_SET_LOG_FD
> - * VHOST_SET_VRING_KICK
> - * VHOST_SET_VRING_CALL
> - * VHOST_SET_VRING_ERR
> + * VHOST_USER_SET_MEM_TABLE
> + * VHOST_USER_SET_LOG_FD
> + * VHOST_USER_SET_VRING_KICK
> + * VHOST_USER_SET_VRING_CALL
> + * VHOST_USER_SET_VRING_ERR
>
> If Master is unable to send the full message or receives a wrong reply it
> will
> close the connection. An optional reconnection mechanism can be implemented.
>
> -Message types
> +Ring processing
> -------------
> +virtio ring processing is documented in the virtio spec.
> +slave starts processing a specific ring after detecting
> VHOST_USER_SET_RUNNING
> +request with VHOST_USER_RUNNING value, or (if VHOST_USER_PROTOCOL_F_MUST_RUN
> +has not been negotiated) after detecting a ring kick, and stops ring
> processing
> +after detecting VHOST_USER_RESET_OWNER, or VHOST_USER_SET_RUNNING with 0x0
> +value.
So the role of VHOST_USER_SET_RUNNING is to enable or disable the
processing of a ring. Which component and what event would generate
this message? Does it come from the VM or from qemu?
- VM -> qemu -> vhost-user-app?
- or qemu -> vhost-user-app?
My underlying question is to understand how this flag will be used.
>
> +Message types
> +-------------
> * VHOST_USER_GET_FEATURES
>
> Id: 1
> @@ -264,3 +301,51 @@ Message types
> Bits (0-7) of the payload contain the vring index. Bit 8 is the
> invalid FD flag. This flag is set when there is no file descriptor
> in the ancillary data.
> +
> + * VHOST_USER_GET_RUNNING
> + Id: 15
> + Ioctl: VHOST_NET_SET_BACKEND
> + Master payload: vring state description
> +
> + Not valid under protocol version 0x1.
> + Only valid if VHOST_USER_PROTOCOL_F_MUST_RUN has been
> + negotiated.
> + Report whether backend is processing descriptors from a given queue.
> + Values: num = 0x1 - VHOST_USER_RUNNING; num = 0x0 - not running
> +
> + * VHOST_USER_SET_RUNNING
> + Id: 16
> + Ioctl: VHOST_NET_SET_BACKEND
> + Master payload: vring state description
> +
> + Not valid under protocol version 0x1.
> + Only valid if VHOST_USER_PROTOCOL_F_MUST_RUN has been
> + negotiated.
> + Stop/start processing descriptors from a given queue.
> + Values: num = 0x1 - VHOST_USER_RUNNING; num = 0x0 - not running
> +
> + * VHOST_USER_GET_PROTOCOL_FEATURES
> +
> + Id: 17
> + Equivalent ioctl: N/A
> + Master payload: N/A
> + Slave payload: u64
> +
> + Get the protocol features bitmask.
> + Not valid under protocol version 0x1.
> + At the moment two feature masks are defined:
> + VHOST_USER_PROTOCOL_F_REPLY_ALL 0 - causes slave to reply to all
> messages
I'm not sure to understand why such flag is needed. Could you
please give some more explanations?
> + VHOST_USER_PROTOCOL_F_MUST_RUN 1 - slave won't process rings unless
> + VHOST_USER_SET_RUNNING is called.
> +
> + * VHOST_USER_SET_PROTOCOL_FEATURES
> +
> + Id: 18
> + Ioctl: VHOST_SET_FEATURES
> + Master payload: u64
> +
> + Get the protocol features bitmask.
> + Not valid under protocol version 0x1.
> + At the moment a single feature mask bit is defined:
> + VHOST_USER_PROTOCOL_F_REPLY_ALL 0
> + If set, all messages will get a reply.
So VHOST_USER_PROTOCOL_F_MUST_RUN is a read-only property of the
responder, as it cannot be set in this command. Or is it a typo?
We can see above in the document "Only valid if
VHOST_USER_PROTOCOL_F_MUST_RUN has been negaciated".
Regards,
Olivier