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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d9d6022-5d49-6f6e-a1ff-562d088ad03c@intel.com>
Date:   Tue, 26 Jul 2022 19:02:50 +0800
From:   "Zhu, Lingshan" <lingshan.zhu@...el.com>
To:     Parav Pandit <parav@...dia.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "mst@...hat.com" <mst@...hat.com>
Cc:     "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "xieyongji@...edance.com" <xieyongji@...edance.com>,
        "gautam.dawar@....com" <gautam.dawar@....com>
Subject: Re: [PATCH V4 3/6] vDPA: allow userspace to query features of a vDPA
 device



On 7/24/2022 11:21 PM, Parav Pandit wrote:
>
>> From: Zhu, Lingshan <lingshan.zhu@...el.com>
>> Sent: Saturday, July 23, 2022 7:24 AM
>>
>>
>> On 7/22/2022 9:12 PM, Parav Pandit wrote:
>>>> From: Zhu Lingshan <lingshan.zhu@...el.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@...el.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.

>
>   
>> Pleas refer to the previous thread, and you did not answer my questions in
>> that thread.
>>
>> Thanks,
>> Zhu Lingshan
>>> MST,
>>> I nack this patch.
>>> As mentioned in the previous versions, also it is missing the example
>> output in the commit log.
>>> Please include example output.
>>>
>>>>    	VDPA_ATTR_DEV_QUEUE_INDEX,              /* u32 */
>>>>    	VDPA_ATTR_DEV_VENDOR_ATTR_NAME,		/* string */
>>>> --
>>>> 2.31.1