Re: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2021/4/7 下午1:10, Parav Pandit 写道:

From: Jason Wang <jasowang@xxxxxxxxxx>
Sent: Wednesday, April 7, 2021 9:26 AM
[..]
   /**
    * struct vdpa_config_ops - operations for configuring a vDPA device.
    * Note: vDPA device drivers are required to implement all of the @@
-164,6 +200,10 @@ struct vdpa_iova_range {
    *				@buf: buffer used to write from
    *				@len: the length to write to
    *				configuration space
+ * @get_ce_config:		Read device specific configuration in
+ *				cpu endianness.
+ *				@vdev: vdpa device
+ *				@config: pointer to config buffer used to
read to


So I wonder what's the reason for having this? If it's only the reason of
endian, we can just use get_config.

Didn't follow your suggestion.
How does in kernel management tool caller get_config  know how to do endianenss conversion?


LE should be used, so it's the responsibility of the vDPA parent (driver) to do the endian conversion.


Or you are proposing to send this whole virtio_config structure as binary data to user space via netlink?
If so, it is not a practice in netlink to return struct.


I don't get here, it should work as mac address I think.



Also making netlink user space to understand __virtio16 doesn't sound correct.
Often orchestration is not written in C. I cannot think of returning virtio_net_config via netlink socket if it is your thought.


I'm not sure I get here. __virtio16 is part of uapi which is defined virtio_types.h so did the virtio_net_config. Duplicating those through dedicated netlink attr looks like burden. E.g you need to introduce new attrs for each field of the config for every virtio devices and keep it up-to-date with the virtio uapis.



And decoding it requires to query features too. Querying features and config are not atomic so parsed value can be wrong.


So I think device should maintain a stable features that will is returned by get_features(), otherwise a lot of things will be broken.



Endianness has to be self-contained in fields return via netlink. With that baseline, lets think further.


Then mandating LE seem self-contained.



We don't plan to expose a legacy or transition device, so we can simply do
the endian conversion in the device.

I am bit confused with Michael's suggestion in v1 [1].

VIRTIO_F_VERSION_1 is set today by mlx5 and ifcvf driver.


I've had some disucssion with Michael, the conclusion is that we should only advertise (or mandate) a modern device to be exposed userspace. Otherwise it could be a lot of burden. Qemu can still present a transtitonal device by doing the endian conversion when necessary in the middle. I'm working on the Qemu patches to do that.

Thanks



Then we can stick to the uapi of virtio_net_config and there's no need for the
VDPA_ATTR_DEV_NET_CFG_XXX?

[1] https://lore.kernel.org/virtualization/20210224020336-mutt-send-email-mst@xxxxxxxxxx/


_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux