Parav Pandit
2022-Jul-26 11:06 UTC
[PATCH V4 3/6] vDPA: allow userspace to query features of a vDPA device
> From: Zhu, Lingshan <lingshan.zhu at intel.com> > Sent: Tuesday, July 26, 2022 7:03 AM > > On 7/24/2022 11:21 PM, Parav Pandit wrote: > > > >> From: Zhu, Lingshan <lingshan.zhu at intel.com> > >> Sent: Saturday, July 23, 2022 7:24 AM > >> > >> > >> On 7/22/2022 9:12 PM, Parav Pandit wrote: > >>>> From: Zhu Lingshan <lingshan.zhu at intel.com> > >>>> 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 > >> 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 */ > >>>> + VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, /* u64 */ > >>>> > >>> 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" I couldn?t find multiple consumers multiple producers working on same nla message.
Zhu Lingshan
2022-Jul-26 11:15 UTC
[PATCH V4 3/6] vDPA: allow userspace to query features of a vDPA device
On 7/26/2022 7:06 PM, Parav Pandit via Virtualization wrote:>> From: Zhu, Lingshan <lingshan.zhu at intel.com> >> Sent: Tuesday, July 26, 2022 7:03 AM >> >> On 7/24/2022 11:21 PM, Parav Pandit wrote: >>>> From: Zhu, Lingshan <lingshan.zhu at intel.com> >>>> Sent: Saturday, July 23, 2022 7:24 AM >>>> >>>> >>>> On 7/22/2022 9:12 PM, Parav Pandit wrote: >>>>>> From: Zhu Lingshan <lingshan.zhu at intel.com> >>>>>> 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 >>>> 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 */ >>>>>> + VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, /* u64 */ >>>>>> >>>>> 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" > I couldn?t find multiple consumers multiple producers working on same nla message.what if two or more iproute2 instance or other userspace tools querying the features of the management device and the vDPA device simultaneously? In such a case, there are multiple consumers in the userspace, and the kernel functions(to fill management device features and vDPA device features) are the multiple producers. And there are no locks.> _______________________________________________ > Virtualization mailing list > Virtualization at lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/virtualization