Re: [PATCH V2 5/6] vDPA: answer num of queue pairs = 1 to userspace when VIRTIO_NET_F_MQ == 0

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 6/14/2022 4:36 AM, Parav Pandit wrote:
>
>> From: Zhu Lingshan <lingshan.zhu@intel.com>
>> Sent: Monday, June 13, 2022 6:17 AM
>> To: jasowang@redhat.com; mst@redhat.com
>> Cc: virtualization@lists.linux-foundation.org; netdev@vger.kernel.org; Parav
>> Pandit <parav@nvidia.com>; xieyongji@bytedance.com;
>> gautam.dawar@amd.com; Zhu Lingshan <lingshan.zhu@intel.com>
>> Subject: [PATCH V2 5/6] vDPA: answer num of queue pairs = 1 to userspace
>> when VIRTIO_NET_F_MQ == 0
>>
>> If VIRTIO_NET_F_MQ == 0, the virtio device should have one queue pair, so
>> when userspace querying queue pair numbers, it should return mq=1 than
>> zero.
>>
>> Function vdpa_dev_net_config_fill() fills the attributions of the vDPA
>> devices, so that it should call vdpa_dev_net_mq_config_fill() so the
>> parameter in vdpa_dev_net_mq_config_fill() should be feature_device than
>> feature_driver for the vDPA devices themselves
>>
>> Before this change, when MQ = 0, iproute2 output:
>> $vdpa dev config show vdpa0
>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 0
>> mtu 1500
>>
> Max_vq_pairs should not be printed when _MQ feature is not negotiated.
> Existing code that returns 0 is correct following this line of the spec.
>
> " The following driver-read-only field, max_virtqueue_pairs only exists if VIRTIO_NET_F_MQ or VIRTIO_-
> NET_F_RSS is set."
> The field doesn't exist when _MQ is not there. Hence, it should not be printed.
> Is _RSS offered and is that why you see it?
>
> If not a fix in the iproute2/vdpa should be done.
IMHO, The spec says:
The following driver-read-only field, max_virtqueue_pairs only exists if 
VIRTIO_NET_F_MQ is *set*

The spec doesn't say:
The following driver-read-only field, max_virtqueue_pairs only exists if 
VIRTIO_NET_F_MQ is *negotiated*

If a device is capable of of multi-queues, this capability does not 
depend on the driver. We are querying the device, not the driver.

If there is MQ, we print the onboard implemented total number of the 
queue pairs.
If MQ is not set, we will not read the onboard mq number, because it is 
not there as the spec says.
But there should be at least one queue pair to be a functional 
virtio-net, so 1 is printed.

Thanks,
Zhu Lingshan

>
>
>> After applying this commit, when MQ = 0, iproute2 output:
>> $vdpa dev config show vdpa0
>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false max_vq_pairs 1
>> mtu 1500
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>> ---
>>   drivers/vdpa/vdpa.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
>> d76b22b2f7ae..846dd37f3549 100644
>> --- a/drivers/vdpa/vdpa.c
>> +++ b/drivers/vdpa/vdpa.c
>> @@ -806,9 +806,10 @@ static int vdpa_dev_net_mq_config_fill(struct
>> vdpa_device *vdev,
>>   	u16 val_u16;
>>
>>   	if ((features & BIT_ULL(VIRTIO_NET_F_MQ)) == 0)
>> -		return 0;
>> +		val_u16 = 1;
>> +	else
>> +		val_u16 = __virtio16_to_cpu(true, config-
>>> max_virtqueue_pairs);
>> -	val_u16 = le16_to_cpu(config->max_virtqueue_pairs);
>>   	return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP,
>> val_u16);  }
>>
>> @@ -842,7 +843,7 @@ static int vdpa_dev_net_config_fill(struct
>> vdpa_device *vdev, struct sk_buff *ms
>>   			      VDPA_ATTR_PAD))
>>   		return -EMSGSIZE;
>>
>> -	return vdpa_dev_net_mq_config_fill(vdev, msg, features_driver,
>> &config);
>> +	return vdpa_dev_net_mq_config_fill(vdev, msg, features_device,
>> +&config);
>>   }
>>
>>   static int
>> --
>> 2.31.1