Hi Michael,
> -----Original Message-----
> From: snabb-de...@googlegroups.com [mailto:snabb-
> de...@googlegroups.com] On Behalf Of Michael S. Tsirkin
> Sent: Thursday, August 13, 2015 5:23 PM
> To: Ouyang, Changchun
> Cc: qemu-devel@nongnu.org; snabb-de...@googlegroups.com;
> thibaut.col...@6wind.com; n.nikol...@virtualopensystems.com;
> l...@snabb.co; Long, Thomas
> Subject: [snabb-devel] Re: [PATCH v6 2/2] vhost-user: new protocol feature
> for multi queue
>
> 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 <changchun.ouy...@intel.com>
> > ---
> > 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.
So you suggest it should remove the ' / 2' for the msg.state.index above, as
follows:
msg.state.index = dev->vq_index;
do I understand it correctly?
Thanks
Changchun
>
> > + 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
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "Snabb Switch development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to snabb-devel+unsubscr...@googlegroups.com.
> To post to this group, send an email to snabb-de...@googlegroups.com.
> Visit this group at http://groups.google.com/group/snabb-devel.
