[1/4] vhost-user: fix multiple queue specification

Message ID 20180112145658.17121-2-maxime.coquelin@redhat.com
State New
Headers show
Series
  • vhost-user: notify backend with number of queues setup
Related show

Commit Message

Maxime Coquelin Jan. 12, 2018, 2:56 p.m.
The number of queues supported by the slave is queried with
message VHOST_USER_GET_QUEUE_NUM, not with message
VHOST_USER_GET_PROTOCOL_FEATURES.

Also, looking at master and slave implemntations, the payload
returned by the slave is the number of queue pairs supported
by the slave, not the number of queues.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 docs/interop/vhost-user.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Michael S. Tsirkin Jan. 16, 2018, 3 a.m. | #1
On Fri, Jan 12, 2018 at 03:56:55PM +0100, Maxime Coquelin wrote:
> The number of queues supported by the slave is queried with
> message VHOST_USER_GET_QUEUE_NUM, not with message
> VHOST_USER_GET_PROTOCOL_FEATURES.
> 
> Also, looking at master and slave implemntations, the payload
> returned by the slave is the number of queue pairs supported
> by the slave, not the number of queues.

virtio doesn't have a concept of queue pairs. virtio net does
have a concept of a tx/rx pair for purposes of steering.

Would this be a slave bug then?

I've applied the 1st chunk for now.

> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  docs/interop/vhost-user.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
> index d49444e037..8a14191a1e 100644
> --- a/docs/interop/vhost-user.txt
> +++ b/docs/interop/vhost-user.txt
> @@ -214,8 +214,8 @@ Multiple queue is treated as a protocol extension, hence the slave has to
>  implement protocol features first. The multiple queues feature is supported
>  only when the protocol feature VHOST_USER_PROTOCOL_F_MQ (bit 0) is set.
>  
> -The max number of queues the slave supports can be queried with message
> -VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
> +The max number of queue pairs the slave supports can be queried with message
> +VHOST_USER_GET_QUEUE_NUM. Master should stop when the number of
>  requested queues is bigger than that.
>  
>  As all queues share one connection, the master uses a unique index for each
> @@ -537,7 +537,7 @@ Master message types
>        Master payload: N/A
>        Slave payload: u64
>  
> -      Query how many queues the backend supports. This request should be
> +      Query how many queue pairs the backend supports. This request should be
>        sent only when VHOST_USER_PROTOCOL_F_MQ is set in queried protocol
>        features by VHOST_USER_GET_PROTOCOL_FEATURES.
>  
> -- 
> 2.14.3
Maxime Coquelin Jan. 16, 2018, 12:35 p.m. | #2
On 01/16/2018 04:00 AM, Michael S. Tsirkin wrote:
> On Fri, Jan 12, 2018 at 03:56:55PM +0100, Maxime Coquelin wrote:
>> The number of queues supported by the slave is queried with
>> message VHOST_USER_GET_QUEUE_NUM, not with message
>> VHOST_USER_GET_PROTOCOL_FEATURES.
>>
>> Also, looking at master and slave implemntations, the payload
>> returned by the slave is the number of queue pairs supported
>> by the slave, not the number of queues.
> 
> virtio doesn't have a concept of queue pairs. virtio net does
> have a concept of a tx/rx pair for purposes of steering.

Ok, thanks for the clarification. I will have a look at how vhost-user
SCSI implements it.

> Would this be a slave bug then?

If I'm not mistaken, the bug is in QEMU:

VHOST_USER_GET_QUEUE_NUM is stored in (struct vhost_dev).max_queues.
vhost_net_get_max_queues() returns (struct vhost_dev).max_queues.
And vhost_user_start() from net/vhost-user.c calls
vhost_net_get_max_queues() to get the max number of tx/rx pairs.

If we want to fix QEMU, I think we will need a new flag for
compatibility with older/current backends that assume it represent a
queue pair.

> I've applied the 1st chunk for now.

Thanks.

>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   docs/interop/vhost-user.txt | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
>> index d49444e037..8a14191a1e 100644
>> --- a/docs/interop/vhost-user.txt
>> +++ b/docs/interop/vhost-user.txt
>> @@ -214,8 +214,8 @@ Multiple queue is treated as a protocol extension, hence the slave has to
>>   implement protocol features first. The multiple queues feature is supported
>>   only when the protocol feature VHOST_USER_PROTOCOL_F_MQ (bit 0) is set.
>>   
>> -The max number of queues the slave supports can be queried with message
>> -VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
>> +The max number of queue pairs the slave supports can be queried with message
>> +VHOST_USER_GET_QUEUE_NUM. Master should stop when the number of
>>   requested queues is bigger than that.
>>   
>>   As all queues share one connection, the master uses a unique index for each
>> @@ -537,7 +537,7 @@ Master message types
>>         Master payload: N/A
>>         Slave payload: u64
>>   
>> -      Query how many queues the backend supports. This request should be
>> +      Query how many queue pairs the backend supports. This request should be
>>         sent only when VHOST_USER_PROTOCOL_F_MQ is set in queried protocol
>>         features by VHOST_USER_GET_PROTOCOL_FEATURES.
>>   
>> -- 
>> 2.14.3

Patch

diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt
index d49444e037..8a14191a1e 100644
--- a/docs/interop/vhost-user.txt
+++ b/docs/interop/vhost-user.txt
@@ -214,8 +214,8 @@  Multiple queue is treated as a protocol extension, hence the slave has to
 implement protocol features first. The multiple queues feature is supported
 only when the protocol feature VHOST_USER_PROTOCOL_F_MQ (bit 0) is set.
 
-The max number of queues the slave supports can be queried with message
-VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of
+The max number of queue pairs the slave supports can be queried with message
+VHOST_USER_GET_QUEUE_NUM. Master should stop when the number of
 requested queues is bigger than that.
 
 As all queues share one connection, the master uses a unique index for each
@@ -537,7 +537,7 @@  Master message types
       Master payload: N/A
       Slave payload: u64
 
-      Query how many queues the backend supports. This request should be
+      Query how many queue pairs the backend supports. This request should be
       sent only when VHOST_USER_PROTOCOL_F_MQ is set in queried protocol
       features by VHOST_USER_GET_PROTOCOL_FEATURES.