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

Martin Kletzander mkletzan at redhat.com
Wed Aug 26 08:19:16 UTC 2015


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 at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150826/1e8add6e/attachment-0001.sig>


More information about the libvir-list mailing list