Re: [libvirt] PATCH: pci-subsystem: ixgbe: SR-IOV: kernel way to order driver's virtfn -entries is odd causing libvirt failures.
On 10/24/2013 11:12 AM, Daniel P. Berrange wrote:
On Thu, Oct 24, 2013 at 09:52:31AM +0300, Niilona wrote:
> Hi.
>
> As Bjorn Helgaas recommend, this might be the item to discuss in the wider area.
> -----------------------------------
>
> There is a behavior effecting virtfn -entries in sysfs, when amount of
> them increases over 10.
> Run VM's through LIBVIRT -> QEMU/KVM, this causes :
> - MAC address setting by LIBVIRT disordered ie. setting targeted to wrong VF.
> - VLAN setting by LIBVIRT overall failed
>
> Basics of this are in "/libvirt-x.x.x/src/util/virpci.c" ; in function
below,
> which don't order virtfn entries correctly.
>
> /*
> * Returns virtual functions of a physical function
> */
> int
> virPCIGetVirtualFunctions(const char *sysfs_path,
> virPCIDeviceAddressPtr **virtual_functions,
> unsigned int *num_virtual_functions)
> {
>
> But I let you to decide which is best way to fix this, as if every
> application reads "virtfn" entries from PF's directory, they all need
> to sort entries in alphabet. order to avoid this
> influence.
> So personally I did get over this by adding pre-zeroes to names to
> have them in sorted order in PF's directory.
Libvirt has to work correctly with all existing released kernels,
so we have to fix libvirt to deal with ordering correctly. Thus
changing the kernel naming here doesn't really help. We need to
fix libvirt to deal with this.
Right. Now that there is enough information to understand the problem
wrt. libvirt, I can see that the main place this improper ordering of
the device links causes an issue is in virPCIGetVirtualFunctionIndex()
(and callers) - it assumes that the VF indexes will exactly match the
index of each PCI device in the list returned by
virPCIGetVirtualFunction(). (We've never made any guarantees about the
ordering of virtual functions in the output of nodedev-dumpxml, but the
return value from virPCIGetVirtualFunctionIndex is used by callers to
fill in the netlink command to get/set a VF's mac address and vlan tag.)
To fix this we need to modify virPCIGetVirtualFunctions to gather all
virtfn%d entries in the PF's directory and sort them numerically (right
now we just grab them in whatever order readdir() provides and put them
in the list in that order). This will automatically fix
virPCIGetVirtualFunctionIndex() and coincidentally make the ordering in
nodedev-dumpxml "proper".
Of course even this will only work as long as lists of virtual functions
are guaranteed to never be "sparse" (e.g. there can't be a virtfn22
unless there is also a virtfn21). My guess is that this is always true,
but we should check for that in the new function and log an appropriate
error in the case that we encounter a sparse list.