On 7/6/2022 10:25 AM, Zhu, Lingshan wrote:

On 7/6/2022 1:01 AM, Parav Pandit wrote:

quoted

quoted

From: Zhu, Lingshan <redacted>
Sent: Tuesday, July 5, 2022 12:56 PM

quoted

Both can be queried simultaneously. Each will return their own 
feature bits

using same attribute.

quoted

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?

quoted

quoted

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

quoted

quoted

quoted

quoted

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