Thread (25 messages) 25 messages, 5 authors, 2022-08-11

RE: [PATCH V4 3/6] vDPA: allow userspace to query features of a vDPA device

From: Parav Pandit <hidden>
Date: 2022-08-09 19:27:18
Also in: virtualization

From: Zhu, Lingshan <redacted>
Sent: Wednesday, July 27, 2022 2:02 AM


On 7/26/2022 7:06 PM, Parav Pandit wrote:
quoted
quoted
From: Zhu, Lingshan <redacted>
Sent: Tuesday, July 26, 2022 7:03 AM

On 7/24/2022 11:21 PM, Parav Pandit wrote:
quoted
quoted
From: Zhu, Lingshan <redacted>
Sent: Saturday, July 23, 2022 7:24 AM


On 7/22/2022 9:12 PM, Parav Pandit wrote:
quoted
quoted
From: Zhu Lingshan <redacted>
Sent: Friday, July 22, 2022 7:53 AM

This commit adds a new vDPA netlink attribution
VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES. Userspace can
query
quoted
quoted
features
quoted
quoted
of vDPA devices through this new attr.

Signed-off-by: Zhu Lingshan <redacted>
---
    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
quoted
quoted
quoted
quoted
quoted
quoted
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))
quoted
quoted
quoted
quoted
quoted
quoted
    		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);
quoted
quoted
quoted
quoted
quoted
quoted
+	return vdpa_dev_net_mq_config_fill(vdev, msg,
features_driver,
quoted
quoted
quoted
quoted
quoted
quoted
+&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 */
quoted
quoted
quoted
quoted
quoted
quoted
    	VDPA_ATTR_DEV_SUPPORTED_FEATURES,	/* u64 */
+	VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES,	/*
u64 */
quoted
quoted
quoted
quoted
quoted
quoted
I have answered in previous emails.
I disagree with the change.
Please reuse VDPA_ATTR_DEV_SUPPORTED_FEATURES.
I believe we have already discussed this before in the V3 thread.
I have told you that reusing this attr will lead to a new race condition.
Returning attribute cannot lead to any race condition.
Please refer to our discussion in the V3 series, I have explained if
re-use this attr, it will be a multiple consumers and multiple
produces model, it is a typical racing condition.
I read the emails with subject = " Re: [PATCH V3 3/6] vDPA: allow userspace
to query features of a vDPA device"
quoted
I couldn’t find multiple consumers multiple producers working on same nla
message.
If this attr is reused, then there can be multiple iproute2 instances or other
applications querying feature bits of the management device and the vDPA
device simultaneously, and both kernel side management feature bits filler
function and vDPA device feature bits filler function can write the NLA
message at the same time. That's the multiple consumers and producers,
and no locks
No. Each filling up happens in each process context. There is no race here.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help