[libvirt] [PATCH] util: Don't save/set MAC address for macvtap+passthrough+802.1Qbh
Before libvirt sets the MAC address of the physdev (the physical ethernet device) linked to a macvtap passthrough device, it always saves the previous MAC address to restore when the guest is finished (following a "leave nothing behind" policy). It has even done this for macvtap devices that have an 802.1Qbh port profile attached to them. It turns out that this is unnecessary, because the port profile Associate/Disassociate operations do that for us. Beyond that, with a recent change to the way we retrieve the MAC address (commit cb3fe38c), all attempts to start a macvtap passthrough device with an 802.1Qbh port profile attached to a Cisco VMFEX card (which uses the "enic" driver in the kernel) to fail. This patch puts extra qualifiers around both the save/set and the restore of the physdev address, so that it isn't done if there is an 802.1Qbh port profile associated with it. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1257004 --- Stefan - do you know if this save/restore of MAC address is also unnecessary/erroneous for 802.1Qbg? If so, I'll just disable it any time a virtportprofile is present, since those are the only two port profile types that are valid with macvtap anyway. src/util/virnetdevmacvlan.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 213b8eb..9c4da1f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -777,11 +777,17 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, * address must be reset when the VM is shut down. * This is especially important when using SRIOV capable cards that * emulate their switch in firmware. + * + * Note that this saving/setting of the MAC address must *NOT* be + * done if the interface is setup with an 802.1Qbh. In that case, + * the 802.1Qbh "port associate" operation will take care of + * saving/setting the MAC address. */ - if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { - if (virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) < 0) - return -1; - } + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU && + !(virtPortProfile && + virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH) && + virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) < 0) + return -1; if (tgifname) { if ((ret = virNetDevExists(tgifname)) < 0) @@ -913,7 +919,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, int ret = 0; int vf = -1; - if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU && + !(virtPortProfile && + virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH)) ignore_value(virNetDevRestoreNetConfig(linkdev, vf, stateDir)); if (ifname) { -- 2.1.0
On Wed, Aug 26, 2015 at 12:29:20AM -0400, Laine Stump wrote:
Before libvirt sets the MAC address of the physdev (the physical ethernet device) linked to a macvtap passthrough device, it always saves the previous MAC address to restore when the guest is finished (following a "leave nothing behind" policy). It has even done this for macvtap devices that have an 802.1Qbh port profile attached to them. It turns out that this is unnecessary, because the port profile Associate/Disassociate operations do that for us.
Beyond that, with a recent change to the way we retrieve the MAC address (commit cb3fe38c), all attempts to start a macvtap passthrough device with an 802.1Qbh port profile attached to a Cisco VMFEX card (which uses the "enic" driver in the kernel) to fail.
This patch puts extra qualifiers around both the save/set and the restore of the physdev address, so that it isn't done if there is an 802.1Qbh port profile associated with it.
This resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1257004 ---
Stefan - do you know if this save/restore of MAC address is also unnecessary/erroneous for 802.1Qbg? If so, I'll just disable it any time a virtportprofile is present, since those are the only two port profile types that are valid with macvtap anyway.
src/util/virnetdevmacvlan.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 213b8eb..9c4da1f 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -777,11 +777,17 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname, * address must be reset when the VM is shut down. * This is especially important when using SRIOV capable cards that * emulate their switch in firmware. + * + * Note that this saving/setting of the MAC address must *NOT* be + * done if the interface is setup with an 802.1Qbh. In that case, + * the 802.1Qbh "port associate" operation will take care of + * saving/setting the MAC address. */ - if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) { - if (virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) < 0) - return -1; - } + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU && + !(virtPortProfile && + virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH) && + virNetDevReplaceNetConfig(linkdev, -1, macaddress, -1, stateDir) < 0) + return -1;
if (tgifname) { if ((ret = virNetDevExists(tgifname)) < 0) @@ -913,7 +919,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname, int ret = 0; int vf = -1;
- if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) + if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU && + !(virtPortProfile && + virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH)) ignore_value(virNetDevRestoreNetConfig(linkdev, vf, stateDir));
I was thinking this could be dealt with in the Replace/Restore functions, but that would require more work and, more importantly, it is already dealt with on other callers of these functions, so... ACK
if (ifname) { -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Laine Stump -
Martin Kletzander