There are multiple bugs filed against both libvirt and the kernel
related to incorrect restoration of MAC addresses for SR-IOV VFs, both
when used via VFIO device assignment and when used for macvtap
passthrough mode
https://bugzilla.redhat.com/1415609 - libvirt
https://bugzilla.redhat.com/1341248 - kernel (igb)
https://bugzilla.redhat.com/1415609 - kernel (ixgbe)
https://bugzilla.redhat.com/1302166 - kernel (mlx, fixed)
Beyond that, there are other problems with incorrect setting and
restoration of VF MAC addresses that don't have their own bug reports
(although they may be partially described in the comments of the BZes
mentioned above). This patch series attempts (and I hope succeeds!) to
fix all of these problems, which can be summarized in these three
points:
* The VF's own MAC address was not being set prior to using it for
macvtap passthrough mode. Instead, due to serious misconception on
my part, the "admin MAC" for that VF was set in the PF. The problem
is that the admin MAC isn't put into use on the VF immediately;
rather it is unused until the next time the VF driver is initialized
(on either the host or in a guest) so setting it had no practical
effect, meaning that the macvtap device's MAC didn't match the MAC
of the VF device it was attached to - this meant that traffic
wouldn't pass unless the device was in promiscuous mode (and
apparently it was back when the code was changed to behave this
way?).
* The VF's own MAC address was never restored after it had been used
(for both VFIO device assignment and for macvtap passthrough mode).
Only the "admin MAC" address (which, again, won't take effect
until/unless the VF driver is reloaded/reinitialized) was restored.
* If the original admin MAC was 00:00:00:00:00:00, libvirt would save
that, then attempt to restore it when the guest was finished with
the device, but for some/many SRIOV-capable net drivers, an all 0
MAC is not allowed to be set (even though that is its initial
value). This would result in the MAC not being changed (of course,
since this is the admin MAC, that didn't actually matter, but the
combination of the error message and the VF's own MAC still being
set to the value used by the guest, users mistakenly believed this
was the source of networking problems (e.g. when the guest moved to
another host, but the old host still had an interface showing its
MAC address).
There are lots of details in the patches. 01 - 07 just clean up and
slightly modify some existing utility functions. 08 - 11 define some
functions to save/restore netdev MAC/vlan settings, and 12-14 change
the existing setup/teardown code for macvtap and hostdev to use the
new functions, then 15 and 17-19 modify their behavior further, while
16 just removes functions that are no longer used.
In the end, the VF's own MAC *and* admin MAC are saved for all SRIOV
VFs when we begin using a VF, and the VF's MAC is restored when
finished. Since the admin MAC doesn't actually have any immediate
practical effect, we don't concern ourselves with assuring it is
restored in all cases (in particular, when use use the VF for VFIO
assignment, we only restore the VF's MAC, but not the admin MAC during
teardown, and when using the VF for macvtap passthrough we avoid
restoring the admin MAC because that would unnecessarily turn on the
"administratively set" flag in the PF driver (described in the commit
log for one of these patches). I might decide to fix the former later,
but for now it just unnecessarily complicates the code for no real
gain).
Laine Stump (19):
util: permit querying a VF MAC address or VLAN tag by itself
util: remove unused args from virNetDevSetVfConfig()
util: use cleanup label consistently in virHostdevNetConfigReplace()
util: eliminate useless local variable
util: make virMacAddrParse more versatile
util: change virPCIGetNetName() to not return error if device has no
net name
util: make virPCIGetDeviceAddressFromSysfsLink() public
util: new function virPCIDeviceRebind()
util: new internal function to permit silent failure of
virNetDevSetMAC()
util: new function virNetDevPFGetVF()
util: new functions virNetDev(Save|Read|Set)NetConfig()
util: use new virNetDev*NetConfig() functions for macvtap
setup/teardown
util: use new virNetDev*NetConfig() functions for hostdev
setup/teardown
util: replace virHostdevNetConfigReplace with ...(Save|Set)NetConfig()
util: save hostdev network device config before unbinding from host
driver
util: after hostdev assignment, restore VF MAC address via setting
admin MAC
util: remove unused functions from virnetdev.c
util: if setting admin MAC to 00:00:00:00:00:00 fails, try
02:00:00:00:00:00
util: try *really* hard to set the MAC address of an SRIOV VF
src/libvirt_private.syms | 10 +-
src/util/virhostdev.c | 171 ++++++--
src/util/virmacaddr.c | 2 +-
src/util/virnetdev.c | 937 +++++++++++++++++++++++++++++++-------------
src/util/virnetdev.h | 25 ++
src/util/virnetdevmacvlan.c | 45 ++-
src/util/virpci.c | 65 ++-
src/util/virpci.h | 4 +
8 files changed, 933 insertions(+), 326 deletions(-)
Okay, I've gone through the patches and they look okay. ACK to all
except for 11/19 where I'd like to see the file having some format (e.g.
JSON). Now let me grab a six pack and clean my head :-).
Michal