On Thu, 2020-08-20 at 18:56 -0400, Laine Stump wrote:
On 8/18/20 2:37 PM, Jonathon Jongsma wrote:
vDPA network devices allow high-performance networking in a virtual machine by providing a wire-speed data path. These devices require a vendor-specific host driver but the data path follows the virtio specification.
The support for vDPA devices was recently added to qemu. This allows libvirt to support these devices. It requires that the device is configured on the host with the appropriate vendor-specific driver. This will create a chardev on the host at e.g. /dev/vhost-vdpa-0. That chardev path can then be used to define a new interface with type='vdpa'. ---
[snip]
+ +:: + + ... + <devices> + <interface type='vdpa'> + <source dev='/dev/vhost-vdpa-0'/>
(The above device is created (I just learned this from you in IRC!) by unbinding a VF from its NIC driver on the host, and re-binding it to a special VDPA-VF driver.)
As we were just discussing online, on one hand it could be nice if libvirt could automatically handle rebinding the VF to the vdpa host driver (given the PCI address of the VF), to make it easier to use (because no advance setup would be needed), similar to what's already done with hostdev devices (and <interface type='hostdev'>) when managed='yes' (which is the default setting).
On the other hand, it is exactly that managed='yes' functionality that has created more "libvirt-but-not-really-libvirt" bug reports than any other aspect of vfio device assignment, because the process of unbinding and rebinding drivers is timing-sensitive and causes code that's usually run only once at host boot-time to be run hundreds of times thus making it more likely to expose infrequently-hit bugs.
I just bring this up in advance of someone suggesting the addition of managed='yes' here to put in my vote for *not* doing it, and instead using that same effort to provide some sort of API in the node- device driver for easily creating one or more VDPA devices from VFs, which could be done once at host boot time, and thus avoid the level of "libvirt-not-libvirt" bug reports for VDPA. (and after that maybe even an API to allocate a device from that pool to be used by a guest). But that's for later.
I'd really love to hear some additional opinions on this topic from some more of the libvirt "old-timers". I intentionally kept the initial vdpa support simple by requiring that the device is already setup and bound to the appropriate driver. But I want to make sure that we can add additional capabilities later (as Laine suggested) with this same API/schema. Anybody else have thoughts on this topic?
+ </interface> + </devices> + ... + :anchor:`<a id="elementsTeaming"/>`
Teaming a virtio/hostdev NIC pair diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0d0dcbc5ce..17f74490f4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3108,6 +3108,21 @@ <ref name="interface-options"/> </interleave> </group> + + <group> + <attribute name="type"> + <value>vdpa</value> + </attribute> + <interleave> + <element name="source"> + <attribute name="dev"> + <ref name="deviceName"/> + </attribute> + </element> + <ref name="interface-options"/> + </interleave> + </group> + </choice> <optional> <attribute name="trustGuestRxFilters">
[snip]
diff --git a/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args b/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args new file mode 100644 index 0000000000..8e76ac7794 --- /dev/null +++ b/tests/qemuxml2argvdata/net-vdpa.x86_64-latest.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i386 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-add-fd set=0,fd=1732 \ +-netdev vhost-vdpa,vhostdev=/dev/fdset/0,id=hostnet0 \
Okay, I'm feeling too lazy to parse through the code above an see how you arrived at "vhostdev='/dev/fdset/0'", but that doesn't look right. Shouldn't you be ending up with "-netdev vhost-vdpa,fd=NN,..."? The document I have shows that syntax is supported, so there shouldn't be any need to do the add-fd stuff in this case.
The initial proposal for vdpa in qemu supported both vhostdev= and fd= parameters, but the final implementation does not actually support an fd= parameter here. The recommended way to pass an pre-opened fd to qemu in this scenario is to use the 'add-fd' and /dev/fdset/ method as shown above. As far as I understand from reading qemu code, /dev/fdset/N is not actually a file that exists in the filesystem. Instead, when you call 'add-fd', qemu adds that fd to an internal mapping that maps from the fdset specified by set=N to the passed fd. Whenever qemu tries to open a file, qemu_open() has special handling for filenames that start with "/dev/fdset/": it looks up the fd associated with that fdset id and returns that instead of attempting to open the file path. So I think the above should be correct.
I think the next step should be to find some hardware and give this a smoke test! :-)
Indeed, I'm working with Cindy Lu (who implemented the qemu vdpa support) to try to get this tested properly. Unfortunately I've been unable to get vdpa_sim working and I don't have access to hardware at the moment. Thanks for the review! Jonathon