From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWiVK-00079L-0R for qemu-devel@nongnu.org; Tue, 01 Sep 2015 06:09:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWiVE-0000ri-Um for qemu-devel@nongnu.org; Tue, 01 Sep 2015 06:09:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47403) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWiVE-0000ra-Lu for qemu-devel@nongnu.org; Tue, 01 Sep 2015 06:09:20 -0400 Date: Tue, 1 Sep 2015 13:09:15 +0300 From: "Michael S. Tsirkin" Message-ID: <20150901130858-mutt-send-email-mst@redhat.com> References: <1432776186-24515-1-git-send-email-changchun.ouyang@intel.com> <1439360742-2186-1-git-send-email-changchun.ouyang@intel.com> <1439360742-2186-3-git-send-email-changchun.ouyang@intel.com> <20150813121932-mutt-send-email-mst@redhat.com> <20150901091631.GB3054@yliu-dev.sh.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150901091631.GB3054@yliu-dev.sh.intel.com> Subject: Re: [Qemu-devel] [PATCH v6 2/2] vhost-user: new protocol feature for multi queue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yuanhan Liu Cc: snabb-devel@googlegroups.com, thibaut.collet@6wind.com, qemu-devel@nongnu.org, n.nikolaev@virtualopensystems.com, luke@snabb.co, thomas.long@intel.com, Ouyang Changchun On Tue, Sep 01, 2015 at 05:16:31PM +0800, Yuanhan Liu wrote: > On Thu, Aug 13, 2015 at 12:22:50PM +0300, Michael S. Tsirkin wrote: > > On Wed, Aug 12, 2015 at 02:25:42PM +0800, Ouyang Changchun wrote: > > > This patch is based on top of "vhost-user: protocol updates" series > > > proposed earlier by Michael S. Tsirkin. > > > > > > Use new message VHOST_USER_SET_VRING_FLAG to enable and disable an > > > actual virt queue, which is similar to attach/detach queue for tap device. > > > virtio driver on guest doesn't have to use max virt queue pair, it could > > > enable any number of virt queue ranging from 1 to max virt queue pair. > > > > > > It requires that VHOST_USER_F_PROTOCOL_FEATURES is present. > > > > > > Signed-off-by: Changchun Ouyang > > > --- > > > This is added since v5 > > > > > > docs/specs/vhost-user.txt | 17 +++++++++++++++++ > > > hw/net/vhost_net.c | 18 ++++++++++++++++++ > > > hw/net/virtio-net.c | 2 ++ > > > hw/virtio/vhost-user.c | 35 +++++++++++++++++++++++++++++++++-- > > > include/hw/virtio/vhost-backend.h | 2 ++ > > > include/net/vhost_net.h | 1 + > > > 6 files changed, 73 insertions(+), 2 deletions(-) > > > > > > diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt > > > index 9390f89..cca3e5b 100644 > > > --- a/docs/specs/vhost-user.txt > > > +++ b/docs/specs/vhost-user.txt > > > @@ -135,6 +135,10 @@ As older slaves don't support negotiating protocol features, > > > a feature bit was dedicated for this purpose: > > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > > > > > +The Slave uses vring flag to notify the vhost-user whether one virtq is enabled > > > +or not. This request doesn't require replies: > > > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 2 > > > + > > > Multi queue support > > > ------------------- > > > The protocol supports multiple queues by setting all index fields in the sent > > > @@ -306,3 +310,16 @@ 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_SET_VRING_FLAG > > > + > > > + Id: 18 > > > + Equivalent ioctl: N/A > > > + Master payload: vring state description > > > + > > > + Set the flag(enable or disable) in the vring, the vhost user backend > > > + enable or disable the vring according to state.num. Olny legal if feature > > > + bit VHOST_USER_F_PROTOCOL_FEATURES is present in VHOST_USER_GET_FEATURE > > > + and feature bit VHOST_USER_PROTOCOL_F_VRING_FLAG is present in > > > + VHOST_USER_GET_PROTOCOL_FEATURES. The vring is enabled when state.num is > > > + 1, otherwise, the vring is disabled. > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > > > index 9cd6c05..5fa341c 100644 > > > --- a/hw/net/vhost_net.c > > > +++ b/hw/net/vhost_net.c > > > @@ -405,6 +405,19 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > > > > return vhost_net; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, unsigned int enable) > > > +{ > > > + if (nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) { > > > + struct vhost_net *net = get_vhost_net(nc); > > > + const VhostOps *vhost_ops = net->dev.vhost_ops; > > > + if (vhost_ops->vhost_backend_mq_set_vring_flag) > > > + return vhost_ops->vhost_backend_mq_set_vring_flag(&net->dev, enable); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > #else > > > struct vhost_net *vhost_net_init(VhostNetOptions *options) > > > { > > > @@ -455,4 +468,9 @@ VHostNetState *get_vhost_net(NetClientState *nc) > > > { > > > return 0; > > > } > > > + > > > +int vhost_set_vring_flag(NetClientState *nc, unsigned int enable) > > > +{ > > > + return 0; > > > +} > > > #endif > > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > > > index 3af6faf..272b77d 100644 > > > --- a/hw/net/virtio-net.c > > > +++ b/hw/net/virtio-net.c > > > @@ -396,6 +396,7 @@ static int peer_attach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 1); > > > return 0; > > > } > > > > > > @@ -411,6 +412,7 @@ static int peer_detach(VirtIONet *n, int index) > > > } > > > > > > if (nc->peer->info->type != NET_CLIENT_OPTIONS_KIND_TAP) { > > > + vhost_set_vring_flag(nc->peer, 0); > > > return 0; > > > } > > > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > > index fb11d4c..d806ce2 100644 > > > --- a/hw/virtio/vhost-user.c > > > +++ b/hw/virtio/vhost-user.c > > > @@ -25,7 +25,8 @@ > > > > > > #define VHOST_MEMORY_MAX_NREGIONS 8 > > > #define VHOST_USER_F_PROTOCOL_FEATURES 30 > > > -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x0ULL > > > +#define VHOST_USER_PROTOCOL_F_VRING_FLAG 2 > > > +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL > > > > > > typedef enum VhostUserRequest { > > > VHOST_USER_NONE = 0, > > > @@ -45,6 +46,7 @@ typedef enum VhostUserRequest { > > > VHOST_USER_SET_VRING_ERR = 14, > > > VHOST_USER_GET_PROTOCOL_FEATURES = 15, > > > VHOST_USER_SET_PROTOCOL_FEATURES = 16, > > > + VHOST_USER_SET_VRING_FLAG = 18, > > > VHOST_USER_MAX > > > } VhostUserRequest; > > > > > > @@ -399,6 +401,34 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) > > > return 0; > > > } > > > > > > +static int vhost_user_set_vring_flag(struct vhost_dev *dev, unsigned int enable) > > > +{ > > > + VhostUserMsg msg = { 0 }; > > > + int err; > > > + > > > + assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > + > > > + if ((dev->backend_features & ( 1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) == 0) > > > + return -1; > > > + > > > + if ((dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_VRING_FLAG)) == 0) > > > + return -1; > > > + > > > + msg.request = VHOST_USER_SET_VRING_FLAG; > > > + msg.flags = VHOST_USER_VERSION; > > > + /* Set the virt queue pair index */ > > > + msg.state.index = dev->vq_index / 2; > > > > Thinking about this, this means that the index value is specific to virtio net. > > This seems wrong. > > How about adding a new field queue_index to vhost_dev structure? So that > we could get rid of it. > > --yliu don't we have it already? /* the first virtqueue which would be used by this vhost dev */ int vq_index; > > > > > + msg.state.num = enable; > > > + msg.size = sizeof msg.state; > > > + > > > + err = vhost_user_write(dev, &msg, NULL, 0); > > > + if (err < 0) { > > > + return err; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static int vhost_user_cleanup(struct vhost_dev *dev) > > > { > > > assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); > > > @@ -412,5 +442,6 @@ const VhostOps user_ops = { > > > .backend_type = VHOST_BACKEND_TYPE_USER, > > > .vhost_call = vhost_user_call, > > > .vhost_backend_init = vhost_user_init, > > > - .vhost_backend_cleanup = vhost_user_cleanup > > > + .vhost_backend_cleanup = vhost_user_cleanup, > > > + .vhost_backend_mq_set_vring_flag = vhost_user_set_vring_flag, > > > }; > > > diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h > > > index e472f29..c394147 100644 > > > --- a/include/hw/virtio/vhost-backend.h > > > +++ b/include/hw/virtio/vhost-backend.h > > > @@ -24,12 +24,14 @@ typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request, > > > void *arg); > > > typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); > > > typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); > > > +typedef int (*vhost_backend_mq_set_vring_flag)(struct vhost_dev *dev, unsigned int enable); > > > > > > typedef struct VhostOps { > > > VhostBackendType backend_type; > > > vhost_call vhost_call; > > > vhost_backend_init vhost_backend_init; > > > vhost_backend_cleanup vhost_backend_cleanup; > > > + vhost_backend_mq_set_vring_flag vhost_backend_mq_set_vring_flag; > > > } VhostOps; > > > > > > extern const VhostOps user_ops; > > > diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h > > > index b1c18a3..9b76548 100644 > > > --- a/include/net/vhost_net.h > > > +++ b/include/net/vhost_net.h > > > @@ -29,4 +29,5 @@ bool vhost_net_virtqueue_pending(VHostNetState *net, int n); > > > void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, > > > int idx, bool mask); > > > VHostNetState *get_vhost_net(NetClientState *nc); > > > +int vhost_set_vring_flag(NetClientState * nc, unsigned int enable); > > > #endif > > > -- > > > 1.8.4.2 > > >