From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34974) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cxwEx-0004Fr-7h for qemu-devel@nongnu.org; Tue, 11 Apr 2017 09:53:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cxwEt-0004hd-Ah for qemu-devel@nongnu.org; Tue, 11 Apr 2017 09:53:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46018) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cxwEt-0004hP-1T for qemu-devel@nongnu.org; Tue, 11 Apr 2017 09:53:47 -0400 References: <20170411101002.28451-1-maxime.coquelin@redhat.com> <20170411101002.28451-2-maxime.coquelin@redhat.com> From: Maxime Coquelin Message-ID: <7dfc220c-b22b-bd7d-53af-caf49275b66c@redhat.com> Date: Tue, 11 Apr 2017 15:53:37 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC 1/2] spec/vhost-user: Introduce secondary channel for slave initiated requests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , mst@redhat.com, vkaplans@redhat.com, jasowang@redhat.com, wexu@redhat.com, peterx@redhat.com, yuanhan.liu@linux.intel.com, virtio-comment@lists.oasis-open.org, qemu-devel@nongnu.org Hi Marc-Andr=C3=A9, On 04/11/2017 03:06 PM, Marc-Andr=C3=A9 Lureau wrote: > Hi > > On Tue, Apr 11, 2017 at 12:10 PM Maxime Coquelin > > wrote: > > This vhost-user specification update aims at enabling the > slave to send requests to the master using a dedicated socket > created by the master. > > It can be used for example when the slave implements a device > IOTLB to send cache miss requests to the master. > > The message types list is updated with an "Initiator" field to > indicate for each type whether the master and/or slave can > initiate the request. > > Signed-off-by: Maxime Coquelin > > > > This is very similar to a patch I proposed for shutdown slave initiated > requests: > https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg00095.html Indeed, thanks for pointing this out, I wasn't aware of your series. I find your proposal of having dedicated messages types (VHOST_USER_SLAVE_*) cleaner. Are you ok if I handover your patch, and replace VHOST_USER_SET_SLAVE_FD to VHOST_USER_SET_SLAVE_REQ_FD? > > > --- > docs/specs/vhost-user.txt | 38 +++++++++++++++++++++++++++++++++++= +++ > 1 file changed, 38 insertions(+) > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 036890f..b365047 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -139,6 +139,7 @@ in the ancillary data: > * VHOST_USER_SET_VRING_KICK > * VHOST_USER_SET_VRING_CALL > * VHOST_USER_SET_VRING_ERR > + * VHOST_USER_SET_SLAVE_REQ_FD > > > I like "slave-req-fd" better than "slave-fd" > > > 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. > @@ -150,6 +151,11 @@ As older slaves don't support negotiating > protocol features, > a feature bit was dedicated for this purpose: > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > +If the slave supports VHOST_USER_PROTOCOL_F_SLAVE_REQ protocol > feature, the > +master may create a secondary Unix domain socket and send its file > descriptor > +to the slave using VHOST_USER_SET_SLAVE_REQ_FD request. This new > channel enables > +the slave to send message requests and master to send message repl= ies. > + > Starting and stopping rings > ---------------------- > Client must only process each ring when it is started. > @@ -260,6 +266,7 @@ Protocol features > #define VHOST_USER_PROTOCOL_F_RARP 2 > #define VHOST_USER_PROTOCOL_F_REPLY_ACK 3 > #define VHOST_USER_PROTOCOL_F_MTU 4 > +#define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 > > Message types > ------------- > @@ -268,6 +275,7 @@ Message types > > Id: 1 > Equivalent ioctl: VHOST_GET_FEATURES > + Initiator: Master > Master payload: N/A > Slave payload: u64 > > @@ -279,6 +287,7 @@ Message types > > Id: 2 > Ioctl: VHOST_SET_FEATURES > + Initiator: Master > Master payload: u64 > > Enable features in the underlying vhost implementation using > a bitmask. > @@ -289,6 +298,7 @@ Message types > > Id: 15 > Equivalent ioctl: VHOST_GET_FEATURES > + Initiator: Master > Master payload: N/A > Slave payload: u64 > > @@ -302,6 +312,7 @@ Message types > > Id: 16 > Ioctl: VHOST_SET_FEATURES > + Initiator: Master > Master payload: u64 > > Enable protocol features in the underlying vhost implementat= ion. > @@ -314,6 +325,7 @@ Message types > > Id: 3 > Equivalent ioctl: VHOST_SET_OWNER > + Initiator: Master > Master payload: N/A > > Issued when a new connection is established. It sets the > current Master > @@ -323,6 +335,7 @@ Message types > * VHOST_USER_RESET_OWNER > > Id: 4 > + Initiator: Master > Master payload: N/A > > This is no longer used. Used to be sent to request disabling > @@ -335,6 +348,7 @@ Message types > > Id: 5 > Equivalent ioctl: VHOST_SET_MEM_TABLE > + Initiator: Master > Master payload: memory regions description > > Sets the memory map regions on the slave so it can translate > the vring > @@ -346,6 +360,7 @@ Message types > > Id: 6 > Equivalent ioctl: VHOST_SET_LOG_BASE > + Initiator: Master > Master payload: u64 > Slave payload: N/A > > @@ -360,6 +375,7 @@ Message types > > Id: 7 > Equivalent ioctl: VHOST_SET_LOG_FD > + Initiator: Master > Master payload: N/A > > Sets the logging file descriptor, which is passed as > ancillary data. > @@ -368,6 +384,7 @@ Message types > > Id: 8 > Equivalent ioctl: VHOST_SET_VRING_NUM > + Initiator: Master > Master payload: vring state description > > Set the size of the queue. > @@ -376,6 +393,7 @@ Message types > > Id: 9 > Equivalent ioctl: VHOST_SET_VRING_ADDR > + Initiator: Master > Master payload: vring address description > Slave payload: N/A > > @@ -385,6 +403,7 @@ Message types > > Id: 10 > Equivalent ioctl: VHOST_SET_VRING_BASE > + Initiator: Master > Master payload: vring state description > > Sets the base offset in the available vring. > @@ -393,6 +412,7 @@ Message types > > Id: 11 > Equivalent ioctl: VHOST_USER_GET_VRING_BASE > + Initiator: Master > Master payload: vring state description > Slave payload: vring state description > > @@ -402,6 +422,7 @@ Message types > > Id: 12 > Equivalent ioctl: VHOST_SET_VRING_KICK > + Initiator: Master > Master payload: u64 > > Set the event file descriptor for adding buffers to the vrin= g. It > @@ -415,6 +436,7 @@ Message types > > Id: 13 > Equivalent ioctl: VHOST_SET_VRING_CALL > + Initiator: Master > Master payload: u64 > > Set the event file descriptor to signal when buffers are use= d. It > @@ -428,6 +450,7 @@ Message types > > Id: 14 > Equivalent ioctl: VHOST_SET_VRING_ERR > + Initiator: Master > Master payload: u64 > > Set the event file descriptor to signal when error occurs. I= t > @@ -440,6 +463,7 @@ Message types > > Id: 17 > Equivalent ioctl: N/A > + Initiator: Master > Master payload: N/A > Slave payload: u64 > > @@ -451,6 +475,7 @@ Message types > > Id: 18 > Equivalent ioctl: N/A > + Initiator: Master > Master payload: vring state description > > Signal slave to enable or disable corresponding vring. > @@ -461,6 +486,7 @@ Message types > > Id: 19 > Equivalent ioctl: N/A > + Initiator: Master > Master payload: u64 > > Ask vhost user backend to broadcast a fake RARP to notify th= e > migration > @@ -475,6 +501,7 @@ Message types > > Id: 20 > Equivalent ioctl: N/A > + Initiator: Master > Master payload: u64 > > Set host MTU value exposed to the guest. > @@ -486,6 +513,17 @@ Message types > If VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated, slave must > respond > with zero in case the specified MTU is valid, or non-zero > otherwise. > > + * VHOST_USER_SET_SLAVE_REQ_FD > + > + Id: 21 > + Equivalent ioctl: N/A > + Initiator: Master > + > + Set the socket file descriptor for slave initiated requests. > + This request should be sent only when > VHOST_USER_F_PROTOCOL_FEATURES > + has been negotiated, and protocol feature bit > VHOST_USER_PROTOCOL_F_SLAVE_REQ > + bit is present in VHOST_USER_GET_PROTOCOL_FEATURES. > + > > > looks good to me > > > VHOST_USER_PROTOCOL_F_REPLY_ACK: > ------------------------------- > The original vhost-user specification only demands replies for cer= tain > -- > 2.9.3 > > > -- > Marc-Andr=C3=A9 Lureau