Re: [Qemu-devel] [PATCH v6 1/2] vhost-user: add multi queue support

[Marcel Apfelbaum <marcel@redhat.com>]

On 09/07/2015 03:26 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 07, 2015 at 02:07:38PM +0300, Marcel Apfelbaum wrote:
>> On 08/13/2015 12:18 PM, Michael S. Tsirkin wrote:
>>> On Wed, Aug 12, 2015 at 02:25:41PM +0800, Ouyang Changchun wrote:
>>>> Based on patch by Nikolay Nikolaev:
>>>> Vhost-user will implement the multi queue support in a similar way
>>>> to what vhost already has - a separate thread for each queue.
>>>> To enable the multi queue functionality - a new command line parameter
>>>> "queues" is introduced for the vhost-user netdev.
>>>>
>>>> The RESET_OWNER change is based on commit:
>>>>     294ce717e0f212ed0763307f3eab72b4a1bdf4d0
>>>> If it is reverted, the patch need update for it accordingly.
>>>>
>>>> Signed-off-by: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
>>>> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
>>>> ---
>>>> Changes since v5:
>>>>   - fix the message descption for VHOST_RESET_OWNER in vhost-user txt
>>>>
>>>> Changes since v4:
>>>>   - remove the unnecessary trailing '\n'
>>>>
>>>> Changes since v3:
>>>>   - fix one typo and wrap one long line
>>>>
>>>> Changes since v2:
>>>>   - fix vq index issue for set_vring_call
>>>>     When it is the case of VHOST_SET_VRING_CALL, The vq_index is not initialized before it is used,
>>>>     thus it could be a random value. The random value leads to crash in vhost after passing down
>>>>     to vhost, as vhost use this random value to index an array index.
>>>>   - fix the typo in the doc and description
>>>>   - address vq index for reset_owner
>>>>
>>>> Changes since v1:
>>>>   - use s->nc.info_str when bringing up/down the backend
>>>>
>>>>   docs/specs/vhost-user.txt |  7 ++++++-
>>>>   hw/net/vhost_net.c        |  3 ++-
>>>>   hw/virtio/vhost-user.c    | 11 ++++++++++-
>>>>   net/vhost-user.c          | 37 ++++++++++++++++++++++++-------------
>>>>   qapi-schema.json          |  6 +++++-
>>>>   qemu-options.hx           |  5 +++--
>>>>   6 files changed, 50 insertions(+), 19 deletions(-)
>>>>
>>
>> [...]
>>
>>>
>>>
>>> There are two problems here:
>>>
>>> 1. we don't really know that the backend
>>>     is able to support the requested number of queues.
>>>     If not, everything will fail, silently.
>>>     A new message to query the # of queues could help, though
>>>     I'm not sure what can be done on failure. Fail connection?
>>>
>>> 2. each message (e.g. set memory table) is sent multiple times,
>>>     on the same socket.
>>
>> Hi,
>>
>> Actually each queue has its own vhost_dev device which in turn has his own
>> memory mappings. Because of this VHOST_SET_MEM_TABLE should be sent for each queue.
>>
>> Should we change it to VHOST_SET_VRING_MEM_TABLE? Or maybe I got this wrong...
>>
>> Thanks,
>> Marcel
>>
>
> You got it wrong, the table is the same for all rings.

OK, thanks. So the backend, in this case DPDK, maps it different per each queue.
Here is an example for 2 queues:

  VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
  VHOST_CONFIG: mapped region 0 fd:165 to 0x2aac40000000 sz:0xa0000 off:0x0
  VHOST_CONFIG: mapped region 1 fd:166 to 0x2aac80000000 sz:0x80000000 off:0xc0000
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
  VHOST_CONFIG: vring kick idx:0 file:167 VHOST_CONFIG: virtio isn't ready for processing.
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
  VHOST_CONFIG: vring kick idx:1 file:168
  VHOST_CONFIG: virtio isn't ready for processing.
  VHOST_CONFIG: read message VHOST_USER_SET_FEATURES
  VHOST_CONFIG: read message VHOST_USER_SET_MEM_TABLE
  VHOST_CONFIG: mapped region 0 fd:169 to 0x2aad00000000 sz:0xa0000 off:0x0
  VHOST_CONFIG: mapped region 1 fd:170 to 0x2aad40000000 sz:0x80000000 off:0xc0000
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
  VHOST_CONFIG: vring kick idx:2 file:171
  VHOST_CONFIG: virtio isn't ready for processing.
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_NUM
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_BASE
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_ADDR
  VHOST_CONFIG: read message VHOST_USER_SET_VRING_KICK
  VHOST_CONFIG: vring kick idx:3 file:172
  VHOST_CONFIG: virtio is now ready for processing.

DPDK expects this message per queue and maps it to a different address.
Trying to send it only once will leave the second queue without the mapping.

I'll try to fix this on dpdk side.

Thanks,
Marcel


>
>>
>>
>>>
>>>
>>>
>>
>> [...]