Parav Pandit
2022-Jun-14 18:38 UTC
[PATCH V2 3/6] vDPA: allow userspace to query features of a vDPA device
> From: Zhu, Lingshan <lingshan.zhu at intel.com> > Sent: Monday, June 13, 2022 10:53 PM > > > On 6/14/2022 4:42 AM, Parav Pandit wrote: > > > >> From: Zhu Lingshan <lingshan.zhu at intel.com> > >> Sent: Monday, June 13, 2022 6:17 AM > >> device > >> > >> 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 at intel.com> > >> --- > >> drivers/vdpa/vdpa.c | 13 +++++++++---- > >> include/uapi/linux/vdpa.h | 1 + > >> 2 files changed, 10 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index > >> ebf2f363fbe7..9b0e39b2f022 100644 > >> --- a/drivers/vdpa/vdpa.c > >> +++ b/drivers/vdpa/vdpa.c > >> @@ -815,7 +815,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 +832,17 @@ 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); > >> + 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..39f1c3d7c112 100644 > >> --- a/include/uapi/linux/vdpa.h > >> +++ b/include/uapi/linux/vdpa.h > >> @@ -47,6 +47,7 @@ enum vdpa_attr { > >> VDPA_ATTR_DEV_NEGOTIATED_FEATURES, /* u64 */ > >> VDPA_ATTR_DEV_MGMTDEV_MAX_VQS, /* u32 */ > >> VDPA_ATTR_DEV_SUPPORTED_FEATURES, /* u64 */ > > I see now what was done incorrectly with commit cd2629f6df1ca. > > > > Above was done with wrong name prefix that missed MGMTDEV_. :( > Please > > don't add VDPA_ prefix due to one mistake. > > Please reuse this VDPA_ATTR_DEV_SUPPORTED_FEATURES for device > attribute as well. > currently we can reuse VDPA_ATTR_DEV_SUPPORTED_FEATURES to report > device features for sure, however this could confuse the readers since every > attr should has its own unique purpose.VDPA_ATTR_DEV_SUPPORTED_FEATURES is supposed to be VDPA_ATTR_MGMTDEV_SUPPORTED_FEATURES And device specific features is supposed to be named as, VDPA_ATTR_DEV_SUPPORTED_FEATURES. But it was not done this way in commit cd2629f6df1ca. This leads to the finding good name problem now. Given that this new attribute is going to show same or subset of the features of the management device supported features, it is fine to reuse with exception with explicit comment in the UAPI header file.