Re: [PATCH] util: Don't save/set MAC address for macvtap+passthrough+802.1Qbh

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

 



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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]