Parav Pandit
2021-Dec-01 10:09 UTC
[PATCH 1/2] vdpa: Allow to configure max data virtqueues
> From: Eli Cohen <elic at 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 at 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.> 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.
? 2021/12/1 ??6:09, Parav Pandit ??:> >> From: Eli Cohen <elic at 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 at 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. >