From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37446) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZGA9h-0001iq-DD for qemu-devel@nongnu.org; Fri, 17 Jul 2015 14:14:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZGA9c-0006Rj-DB for qemu-devel@nongnu.org; Fri, 17 Jul 2015 14:14:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56539) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZGA9c-0006Rb-5r for qemu-devel@nongnu.org; Fri, 17 Jul 2015 14:14:36 -0400 Date: Fri, 17 Jul 2015 15:14:33 -0300 From: Flavio Leitner Message-ID: <20150717181433.GD2626@x240.home> References: <1437142095-20217-1-git-send-email-mst@redhat.com> <1437142095-20217-4-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1437142095-20217-4-git-send-email-mst@redhat.com> Subject: Re: [Qemu-devel] [PATCH 3/4] vhost-user: add protocol feature negotiation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: thibaut.collet@6wind.com, changchun.ouyang@intel.com, marcandre.lureau@gmail.com, qemu-devel@nongnu.org, haifeng.lin@huawei.com On Fri, Jul 17, 2015 at 05:09:38PM +0300, Michael S. Tsirkin wrote: > Support a separate bitmask for vhost-user protocol features, > and messages to get/set protocol features. > > Invoke them at init. > > No features are defined yet. > > Signed-off-by: Michael S. Tsirkin > --- > include/hw/virtio/vhost.h | 1 + > hw/net/vhost_net.c | 2 ++ > hw/virtio/vhost-user.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ > docs/specs/vhost-user.txt | 37 +++++++++++++++++++++++++++++++++ > 4 files changed, 92 insertions(+) > > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h > index dd51050..6467c73 100644 > --- a/include/hw/virtio/vhost.h > +++ b/include/hw/virtio/vhost.h > @@ -47,6 +47,7 @@ struct vhost_dev { > unsigned long long features; > unsigned long long acked_features; > unsigned long long backend_features; > + unsigned long long protocol_features; > bool started; > bool log_enabled; > unsigned long long log_size; > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > index 5c1d11f..c864237 100644 > --- a/hw/net/vhost_net.c > +++ b/hw/net/vhost_net.c > @@ -152,8 +152,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) > net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend) > ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR); > net->backend = r; > + net->dev.protocol_features = 0; > } else { > net->dev.backend_features = 0; > + net->dev.protocol_features = 0; > net->backend = -1; > } > net->nc = options->net_backend; > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 5a83d00..c4428a1 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -25,6 +25,9 @@ > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > +#define VHOST_USER_F_PROTOCOL_FEATURES 30 > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL > + > typedef enum VhostUserRequest { > VHOST_USER_NONE = 0, > VHOST_USER_GET_FEATURES = 1, > @@ -41,6 +44,8 @@ typedef enum VhostUserRequest { > VHOST_USER_SET_VRING_KICK = 12, > VHOST_USER_SET_VRING_CALL = 13, > VHOST_USER_SET_VRING_ERR = 14, > + VHOST_USER_GET_PROTOCOL_FEATURES = 15, > + VHOST_USER_SET_PROTOCOL_FEATURES = 26, I think you meant 16 here. fbl > VHOST_USER_MAX > } VhostUserRequest; > > @@ -332,10 +337,57 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, > > static int vhost_user_init(struct vhost_dev *dev, void *opaque) > { > + VhostUserMsg msg = { 0 }; > + int err; > + > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > dev->opaque = opaque; > > + msg.request = VHOST_USER_GET_FEATURES; > + msg.flags = VHOST_USER_VERSION; > + msg.size = 0; > + > + err = vhost_user_write(dev, &msg, NULL, 0); > + if (err < 0) { > + return err; > + } > + > + err = vhost_user_read(dev, &msg); > + if (err < 0) { > + return err; > + } > + > + if (__virtio_has_feature(msg.u64, VHOST_USER_F_PROTOCOL_FEATURES)) { > + dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES; > + > + msg.request = VHOST_USER_GET_PROTOCOL_FEATURES; > + msg.flags = VHOST_USER_VERSION; > + msg.size = 0; > + > + err = vhost_user_write(dev, &msg, NULL, 0); > + if (err < 0) { > + return err; > + } > + > + err = vhost_user_read(dev, &msg); > + if (err < 0) { > + return err; > + } > + > + dev->protocol_features = msg.u64 & VHOST_USER_PROTOCOL_FEATURE_MASK; > + > + msg.request = VHOST_USER_SET_PROTOCOL_FEATURES; > + msg.flags = VHOST_USER_VERSION; > + msg.u64 = dev->protocol_features; > + msg.size = sizeof msg.u64; > + > + err = vhost_user_write(dev, &msg, NULL, 0); > + if (err < 0) { > + return err; > + } > + } > + > return 0; > } > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > index 650bb18..0062baa 100644 > --- a/docs/specs/vhost-user.txt > +++ b/docs/specs/vhost-user.txt > @@ -113,6 +113,7 @@ message replies. Most of the requests don't require replies. Here is a list of > the ones that do: > > * VHOST_GET_FEATURES > + * VHOST_GET_PROTOCOL_FEATURES > * VHOST_GET_VRING_BASE > > There are several messages that the master sends with file descriptors passed > @@ -127,6 +128,13 @@ in the ancillary data: > 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. > > +Any protocol extensions are gated by protocol feature bits, > +which allows full backwards compatibility on both master > +and slave. > +As older slaves don't support negotiating protocol features, > +a feature bit was dedicated for this purpose: > +#define VHOST_USER_F_PROTOCOL_FEATURES 30 > + > Message types > ------------- > > @@ -138,6 +146,8 @@ Message types > Slave payload: u64 > > Get from the underlying vhost implementation the features bitmask. > + Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals slave support for > + VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES. > > * VHOST_USER_SET_FEATURES > > @@ -146,6 +156,33 @@ Message types > Master payload: u64 > > Enable features in the underlying vhost implementation using a bitmask. > + Feature bit VHOST_USER_F_PROTOCOL_FEATURES signals master support for > + VHOST_USER_GET_PROTOCOL_FEATURES and VHOST_USER_SET_PROTOCOL_FEATURES. > + > + * VHOST_USER_GET_PROTOCOL_FEATURES > + > + Id: 15 > + Equivalent ioctl: VHOST_GET_FEATURES > + Master payload: N/A > + Slave payload: u64 > + > + 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. > + Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support > + this message even before VHOST_USER_SET_FEATURES was called. > + > + * VHOST_USER_SET_PROTOCOL_FEATURES > + > + Id: 16 > + Ioctl: VHOST_SET_FEATURES > + Master payload: u64 > + > + 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. > + Note: slave that reported VHOST_USER_F_PROTOCOL_FEATURES must support > + this message even before VHOST_USER_SET_FEATURES was called. > > * VHOST_USER_SET_OWNER > > -- > MST >