netdev - Re: [PATCH V3 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]

Date:   Sat, 23 Jul 2022 19:27:05 +0800
From:   "Zhu, Lingshan" <lingshan.zhu@...el.com>
To:     Parav Pandit <parav@...dia.com>, Jason Wang <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 V3 3/6] vDPA: allow userspace to query features of a vDPA
 device



On 7/6/2022 10:25 AM, Zhu, Lingshan wrote:
>
>
> On 7/6/2022 1:01 AM, Parav Pandit wrote:
>>> From: Zhu, Lingshan <lingshan.zhu@...el.com>
>>> Sent: Tuesday, July 5, 2022 12:56 PM
>>>> Both can be queried simultaneously. Each will return their own 
>>>> feature bits
>>> using same attribute.
>>>> It wont lead to the race.
>>> How? It is just a piece of memory, xxxx[attr], do you see locks in
>>> nla_put_u64_64bit()? It is a typical race condition, data accessed 
>>> by multiple
>>> producers / consumers.
>> No. There is no race condition in here.
>> And new attribute enum by no means avoid any race.
>>
>> Data put using nla_put cannot be accessed until they are transferred.
> How this is guaranteed? Do you see errors when calling nla_put_xxx() 
> twice?
Parav, did you miss this?
>>
>>> And re-use a netlink attr is really confusing.
>> Please put comment for this variable explaining why it is shared for 
>> the exception.
>>
>> Before that lets start, can you share a real world example of when 
>> this feature bitmap will have different value than the mgmt. device 
>> bitmap value?
> For example,
> 1. When migrate the VM to a node which has a more resourceful device. 
> If the source side device does not have MQ, RSS or TSO feature, the 
> vDPA device assigned to the VM does not
> have MQ, RSS or TSO as well. When migrating to a node which has a 
> device with MQ, RSS or TSO, to provide a consistent network device to 
> the guest, to be transparent to the guest,
> we need to mask out MQ, RSS or TSO in the vDPA device when 
> provisioning. This is an example that management device may have 
> different feature bits than the vDPA device.
>
> 2.SIOV, if a virtio device is capable of managing SIOV devices, and it 
> exposes this capability by a feature bit(Like what I am doing in the 
> "transport virtqueue"),
> we don't want the SIOV ADIs have SIOV features, so the ADIs don't have 
> SIOV feature bit.
>
> Thanks
>>
>>>>> IMHO, I don't see any advantages of re-using this attr.
>>>> We don’t want to continue this mess of VDPA_DEV prefix for new
>>> attributes due to previous wrong naming.
>>> as you point out before, is is a wrong naming, we can't re-nmme it 
>>> because
>>> we don't want to break uAPI, so there needs a new attr, if you don't 
>>> like the
>>> name VDPA_ATTR_VDPA_DEV_SUPPORTED_FEATURES, it is more than
>>> welcome to suggest a new one
>>>
>>> Thanks
>