Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues

From: Jason Wang <jasowang@redhat.com>
To: Parav Pandit <parav@nvidia.com>, Eli Cohen <elic@nvidia.com>,
	Si-Wei Liu <si-wei.liu@oracle.com>
Cc: "lvivier@redhat.com" <lvivier@redhat.com>,
	"eperezma@redhat.com" <eperezma@redhat.com>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"mst@redhat.com" <mst@redhat.com>
Subject: Re: [PATCH 1/2] vdpa: Allow to configure max data virtqueues
Date: Thu, 2 Dec 2021 11:40:36 +0800	[thread overview]
Message-ID: <1b588960-dd30-3a22-2793-846d55ff5d56@redhat.com> (raw)
In-Reply-To: <DM8PR12MB54808D3B8AE095A6EF145E36DC689@DM8PR12MB5480.namprd12.prod.outlook.com>

在 2021/12/1 下午6:09, Parav Pandit 写道:
>
>> From: Eli Cohen <elic@nvidia.com>
>> Sent: Wednesday, December 1, 2021 3:33 PM
>>
>> On Tue, Nov 30, 2021 at 05:24:03PM -0800, Si-Wei Liu wrote:
>>>
>>> On 11/30/2021 1:48 AM, Eli Cohen wrote:
>>>> Allow to configure the max virtqueues for a device.
>>>>
>>>> Signed-off-by: Eli Cohen <elic@nvidia.com>
>>>> ---
>>>>    drivers/vdpa/vdpa.c  | 16 +++++++++++++++-
>>>>    include/linux/vdpa.h |  1 +
>>>>    2 files changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
>>>> 7332a74a4b00..e185ec2ee851 100644
>>>> --- a/drivers/vdpa/vdpa.c
>>>> +++ b/drivers/vdpa/vdpa.c
>>>> @@ -480,7 +480,8 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff
>> *msg, struct netlink_callback *cb)
>>>>    }
>>>>    #define VDPA_DEV_NET_ATTRS_MASK ((1 <<
>> VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
>>>> -				 (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
>>>> +				 (1 << VDPA_ATTR_DEV_NET_CFG_MTU) | \
>>>> +				 (1 << VDPA_ATTR_DEV_NET_CFG_MAX_VQP))
>>> It seems VDPA_ATTR_DEV_MAX_VQS (u32) is what you want (# of data
>>> virtqueues instead of # of data virtqueue pairs)? Not sure what's
>>> possible use of VDPA_ATTR_DEV_NET_CFG_MAX_VQP, was it to
>> dump/display
>>> the config space max_virtqueue_pairs value (u16, 1-32768) for
>>> virtio-net? Why there's such quasi-duplicate attribute introduced in the first
>> place?
>> VDPA_ATTR_DEV_MAX_VQS currently returns vdev->nvqs which equals
>> whatever passed to _vdpa_register_device(). The latter depends on the value
>> provided by (struct vdpa_dev_set_config).max_virtqueues.
>>
> Max VQs configuration should reuse VDPA_ATTR_DEV_MAX_VQS.
> it indicates what is the max vqs a given vdpa device is using. Until now it was driver's choice, now its users choice if provided.
> So no need for additional attribute.

I think a device specific attribute might be better:

1) max_virt_queue_pairs is part of config space so it should work as mtu 
and mac

2) it's more convenient for the user to specifc qps instead of doing 
2*N+1 calculation

Thanks

>
>> Maybe we should add attributes to add aditional virtqueues like control
>> virtqueue and their index. They could be returned by
>> vdpa_dev_net_config_fill().
> Yes.
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization