On 8/16/2022 10:07 AM, Parav Pandit wrote:
From: Zhu, Lingshan <lingshan.zhu@xxxxxxxxx>
Sent: Monday, August 15, 2022 9:49 PM
On 8/16/2022 2:15 AM, Si-Wei Liu wrote:
On 8/15/2022 2:26 AM, Zhu Lingshan wrote:
This commit adds a new vDPA netlink attribution
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES. Userspace can query
features
of vDPA devices through this new attr.
Signed-off-by: Zhu Lingshan <lingshan.zhu@xxxxxxxxx>
---
drivers/vdpa/vdpa.c | 17 +++++++++++++----
include/uapi/linux/vdpa.h | 3 +++
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
c06c02704461..efb55a06e961 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -491,6 +491,8 @@ static int vdpa_mgmtdev_fill(const struct
vdpa_mgmt_dev *mdev, struct sk_buff *m
err = -EMSGSIZE;
goto msg_err;
}
+
+ /* report features of a vDPA management device through
VDPA_ATTR_DEV_SUPPORTED_FEATURES */
if (nla_put_u64_64bit(msg, VDPA_ATTR_DEV_SUPPORTED_FEATURES,
mdev->supported_features, VDPA_ATTR_PAD)) {
err = -EMSGSIZE;
@@ -815,7 +817,7 @@ static int vdpa_dev_net_mq_config_fill(struct
vdpa_device *vdev,
static int vdpa_dev_net_config_fill(struct vdpa_device *vdev,
struct sk_buff *msg)
{
struct virtio_net_config config = {};
- u64 features;
+ u64 features_device, features_driver;
u16 val_u16;
vdpa_get_config_unlocked(vdev, 0, &config, sizeof(config));
@@ -832,12 +834,19 @@ static int vdpa_dev_net_config_fill(struct
vdpa_device *vdev, struct sk_buff *ms
if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
return -EMSGSIZE;
- features = vdev->config->get_driver_features(vdev);
- if (nla_put_u64_64bit(msg,
VDPA_ATTR_DEV_NEGOTIATED_FEATURES,
features,
+ features_driver = vdev->config->get_driver_features(vdev);
+ if (nla_put_u64_64bit(msg,
VDPA_ATTR_DEV_NEGOTIATED_FEATURES,
features_driver,
+ VDPA_ATTR_PAD))
+ return -EMSGSIZE;
+
+ features_device = vdev->config->get_device_features(vdev);
+
+ /* report features of a vDPA device through
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES */
+ if (nla_put_u64_64bit(msg,
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, features_device,
VDPA_ATTR_PAD))
return -EMSGSIZE;
- return vdpa_dev_net_mq_config_fill(vdev, msg, features,
&config);
+ return vdpa_dev_net_mq_config_fill(vdev, msg, features_driver,
&config);
}
static int
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 25c55cab3d7c..d171b92ef522 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -46,7 +46,10 @@ enum vdpa_attr {
VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */
VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */
+ /* features of a vDPA management device */
Say comment as.
/* Features of the vDPA management device matching the virtio feature bits 0 to 63 when queried on the mgmt. device.
* When returned on the vdpa device, it indicates virtio feature bits 0 to 63 of the vdpa device
*/
This still suggests to re-use the attr VDPA_ATTR_DEV_SUPPORTED_FEATURES,
I think a new attr should be better and easier
for others.
VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */
+ /* features of a vDPA device, e.g., /dev/vhost-vdpa0 */
+ VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, /* u64 */
Append to the end, please. Otherwise it breaks userspace ABI.
OK, will fix it in V2
I have read Se-Wei comment in the v4.
However I disagree, we don’t need to continue the past mistake done with the naming.
Please add a comment that VDPA_ATTR_DEV_SUPPORTED_FEATURES like above about the exception.
We established that there is no race condition either.
So no need to add new UAPI for some past history.
Yes, no race conditions, the thing is, it is not a good practice to
re-use a netlink attr,
every attr should has its own purpose, no need to confuse others, I
think we had this discussion before.
Thanks
