> From: Zhu, Lingshan <lingshan.zhu@xxxxxxxxx> > Sent: Tuesday, July 5, 2022 3:59 AM > > > On 7/4/2022 8:53 PM, Parav Pandit wrote: > >> From: Jason Wang <jasowang@xxxxxxxxxx> > >> Sent: Monday, July 4, 2022 12:47 AM > >> > >> > >> 在 2022/7/2 06:02, Parav Pandit 写道: > >>>> From: Zhu Lingshan <lingshan.zhu@xxxxxxxxx> > >>>> Sent: Friday, July 1, 2022 9:28 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. > >>>> > >>>> Fixes: a64917bc2e9b vdpa: (Provide interface to read driver > >>>> feature) > >>> Missing the "" in the line. > >>> I reviewed the patches again. > >>> > >>> However, this is not the fix. > >>> A fix cannot add a new UAPI. > >>> > >>> Code is already considering negotiated driver features to return the > >>> device > >> config space. > >>> Hence it is fine. > >>> > >>> This patch intents to provide device features to user space. > >>> First what vdpa device are capable of, are already returned by > >>> features > >> attribute on the management device. > >>> This is done in commit [1]. > >>> > >>> The only reason to have it is, when one management device indicates > >>> that > >> feature is supported, but device may end up not supporting this > >> feature if such feature is shared with other devices on same physical device. > >>> For example all VFs may not be symmetric after large number of them > >>> are > >> in use. In such case features bit of management device can differ > >> (more > >> features) than the vdpa device of this VF. > >>> Hence, showing on the device is useful. > >>> > >>> As mentioned before in V2, commit [1] has wrongly named the > >>> attribute to > >> VDPA_ATTR_DEV_SUPPORTED_FEATURES. > >>> It should have been, > >> VDPA_ATTR_DEV_MGMTDEV_SUPPORTED_FEATURES. > >>> Because it is in UAPI, and since we don't want to break compilation > >>> of iproute2, It cannot be renamed anymore. > >>> > >>> Given that, we do not want to start trend of naming device > >>> attributes with > >> additional _VDPA_ to it as done in this patch. > >>> Error in commit [1] was exception. > >>> > >>> Hence, please reuse VDPA_ATTR_DEV_SUPPORTED_FEATURES to return > >> for device features too. > >> > >> > >> This will probably break or confuse the existing userspace? > >> > > It shouldn't break, because its new attribute on the device. > > All attributes are per command, so old one will not be confused either. > A netlink attr should has its own and unique purpose, that's why we don't need > locks for the attrs, only one consumer and only one producer. > I am afraid re-using (for both management device and the vDPA device) the attr > VDPA_ATTR_DEV_SUPPORTED_FEATURES would lead to new race condition. > E.g., There are possibilities of querying FEATURES of a management device and > a vDPA device simultaneously, or can there be a syncing issue in a tick? Both can be queried simultaneously. Each will return their own feature bits using same attribute. It wont lead to the race. > > 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. _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
