RE: [PATCH v5 10/13] vdpa: Support reporting max device virtqueues

[Parav Pandit via Virtualization]

> From: Si-Wei Liu <si-wei....@oracle.com>
> Sent: Wednesday, December 22, 2021 7:31 AM
> 
> On 12/21/2021 9:20 AM, Eli Cohen wrote:
> > Add max_supported_vqs field to struct vdpa_mgmt_dev. Upstream drivers
> > need to feel this value according to the device capabilities.
> >
> > This value is reported back in a netlink message when showing
> > management devices.
> >
> > Example:
> >
> > $ vdpa dev show
> s/dev/mgmtdev/
> 
> and,
> > vdpa mgmtdev show
> remove this line.
> > auxiliary/mlx5_core.sf.1:
> >    supported_classes net
> >    max_supported_vqs 256
It should be in same line.
Also please show the JSON output.

> Not consistent with the example in patch #11 in the series.
> >
> > Signed-off-by: Eli Cohen <e...@nvidia.com>
> > ---
> >   drivers/vdpa/vdpa.c       | 3 +++
> >   include/linux/vdpa.h      | 1 +
> >   include/uapi/linux/vdpa.h | 1 +
> >   3 files changed, 5 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > eb223bec5209..4b649125a038 100644
> > --- a/drivers/vdpa/vdpa.c
> > +++ b/drivers/vdpa/vdpa.c
> > @@ -507,6 +507,9 @@ static int vdpa_mgmtdev_fill(const struct
> vdpa_mgmt_dev *mdev, struct sk_buff *m
> >             err = -EMSGSIZE;
> >             goto msg_err;
> >     }
> > +   if (nla_put_u16(msg, VDPA_ATTR_DEV_MGMTDEV_MAX_VQS,
> > +                   mdev->max_supported_vqs))
> It still needs a default value when the field is not explicitly filled in by 
> the
> driver.
> 
Unlikely. This can be optional field to help user decide device max limit.
When max_supported_vqs is set to zero. Vdpa should omit exposing it to user 
space.

> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -447,6 +447,7 @@ struct vdpa_mgmt_dev {
> >     const struct vdpa_mgmtdev_ops *ops;
> >     const struct virtio_device_id *id_table;
> >     u64 config_attr_mask;
> > +   u16 max_supported_vqs;
This breaks the natural alignment and create holes in the struct.
Please move it at the last entry in the struct after list.

There has been some talk/patches of rdma virtio device.
I anticipate such device to support more than 64K queues by nature of rdma.
It is better to keep max_supported_vqs as u32.

> >     struct list_head list;
> >   };
> >
> > diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
> > index db3738ef3beb..995257c6bf2a 100644
> > --- a/include/uapi/linux/vdpa.h
> > +++ b/include/uapi/linux/vdpa.h
> > @@ -44,6 +44,7 @@ enum vdpa_attr {
> >     VDPA_ATTR_DEV_NET_CFG_MTU,              /* u16 */
> >
> >     VDPA_ATTR_DEV_NEGOTIATED_FEATURES,      /* u64 */
> > +   VDPA_ATTR_DEV_MGMTDEV_MAX_VQS,          /* u16 */
> >     /* new attributes must be added above here */
> >     VDPA_ATTR_MAX,
> >   };

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