[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 2/2] vhost-user: new protocol feature for mul
From: |
Yuanhan Liu |
Subject: |
Re: [Qemu-devel] [PATCH v6 2/2] vhost-user: new protocol feature for multi queue |
Date: |
Tue, 1 Sep 2015 19:42:33 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Sep 01, 2015 at 01:09:15PM +0300, Michael S. Tsirkin wrote:
> 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 <address@hidden>
> > > > ---
> > > > 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;
Here queue_index is meant to the index of queue pair, while vq_index is
index of virtio net queue, which simply equals to 2 * queue_index.
Then, maybe we could do as Changchun suggested:
msg.state.index = dev->vp_index;
So that we could get rid of the '/2' hack(btw, is that what you worried
about?), and let the backend do the virtio queue index to queue pair index
convertion, aka, divide by 2.
--yliu