[libvirt] [PATCH v4]: set and restore MAC address of a NIC when using PASSTHROUGH mode

Stefan Berger stefanb at linux.vnet.ibm.com
Tue Jun 21 13:44:01 UTC 2011


On 06/21/2011 08:01 AM, Gerhard Stenzel wrote:
> Next try ..
>
> The following patch addresses the problem that when a PASSTHROUGH
> mode DIRECT NIC connection is made the MAC address of the NIC is
> not automatically set and reset to the configured VM MAC and
> back again.
>
> The attached patch fixes this problem by setting and resetting the MAC
> while remembering the previous setting while the VM is running.
> This also works if libvirtd is restarted while the VM is running.
>
> the patch passes make syntax-check
>
> Signed-off-by: Dirk Herrendoerfer<d.herrendoerfer at herrendoerfer.name>
> Signed-off-by: Gerhard Stenzel<gerhard.stenzel at de.ibm.com>
>
> ---
>
>
> Index: libvirt/src/qemu/qemu_command.c
> ===================================================================
> --- libvirt.orig/src/qemu/qemu_command.c
> +++ libvirt/src/qemu/qemu_command.c
> @@ -128,7 +128,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def
>       rc = openMacvtapTap(net->ifname, net->mac, net->data.direct.linkdev,
>                           net->data.direct.mode, vnet_hdr, def->uuid,
>                           &net->data.direct.virtPortProfile,&res_ifname,
> -                        vmop);
> +                        vmop, driver->stateDir);
>       if (rc>= 0) {
>           qemuAuditNetDevice(def, net, res_ifname, true);
>           VIR_FREE(net->ifname);
> @@ -149,7 +149,9 @@ qemuPhysIfaceConnect(virDomainDefPtr def
>               if (err) {
>                   VIR_FORCE_CLOSE(rc);
>                   delMacvtap(net->ifname, net->mac, net->data.direct.linkdev,
> -&net->data.direct.virtPortProfile);
> +                           net->data.direct.mode,
> +&net->data.direct.virtPortProfile,
> +                           driver->stateDir);
>                   VIR_FREE(net->ifname);
>               }
>           }
> Index: libvirt/src/qemu/qemu_process.c
> ===================================================================
> --- libvirt.orig/src/qemu/qemu_process.c
> +++ libvirt/src/qemu/qemu_process.c
> @@ -2876,7 +2876,8 @@ void qemuProcessStop(struct qemud_driver
>           virDomainNetDefPtr net = def->nets[i];
>           if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
>               delMacvtap(net->ifname, net->mac, net->data.direct.linkdev,
> -&net->data.direct.virtPortProfile);
> +                       net->data.direct.mode,
> +&net->data.direct.virtPortProfile, driver->stateDir);
>               VIR_FREE(net->ifname);
>           }
>       }
> Index: libvirt/src/util/macvtap.c
> ===================================================================
> --- libvirt.orig/src/util/macvtap.c
> +++ libvirt/src/util/macvtap.c
> @@ -545,6 +545,104 @@ configMacvtapTap(int tapfd, int vnet_hdr
>       return 0;
>   }
>
> +/**
> + * replaceMacAdress:
> + * @macaddress: new MAC address for interface
> + * @linkdev: name of interface
> + * @stateDir: directory to store old MAC address
> + *
> + * Returns 0 on success, -1 in case of fatal error, error code otherwise.
> + *
> + */
> +static int
> +replaceMacAdress(const unsigned char *macaddress,
> +                 const char *linkdev,
> +                 char *stateDir)
> +{
> +    unsigned char oldmac[6];
> +    int rc;
> +
> +    rc = ifaceGetMacaddr(linkdev, oldmac);
> +
> +    if (rc) {
> +        virReportSystemError(rc,
> +                             _("Getting MAC address from '%s' "
> +                               "to '%02x:%02x:%02x:%02x:%02x:%02x' failed."),
> +                             linkdev,
> +                             oldmac[0], oldmac[1], oldmac[2],
> +                             oldmac[3], oldmac[4], oldmac[5]);
> +    } else {
> +        char *path = NULL;
> +        char macstr[VIR_MAC_STRING_BUFLEN];
> +
> +        if (virAsprintf(&path, "%s/%s",
> +                        stateDir,
> +                        linkdev)<  0) {
> +            virReportOOMError();
> +            return errno;
> +        }
> +        virFormatMacAddr(oldmac, macstr);
> +        if (virFileWriteStr(path, macstr, O_CREAT|O_TRUNC|O_WRONLY)<  0) {
> +            virReportSystemError(errno, _("Unable to preserve mac for %s"),
> +                                 linkdev);
> +            return errno;
> +        }
> +    }
> +
> +    rc = ifaceSetMacaddr(linkdev, macaddress);
> +    if (rc) {
> +        virReportSystemError(errno,
> +                             _("Setting MAC address on  '%s' to "
> +                               "'%02x:%02x:%02x:%02x:%02x:%02x' failed."),
> +                             linkdev,
> +                             macaddress[0], macaddress[1], macaddress[2],
> +                             macaddress[3], macaddress[4], macaddress[5]);
> +    }
> +    return rc;
> +}
> +
> +/**
> + * restoreMacAddress:
> + * @linkdev: name of interface
> + * @stateDir: directory containing old MAC address
> + *
> + * Returns 0 on success, -1 in case of fatal error, error code otherwise.
> + *
> + */
> +static int
> +restoreMacAddress(const char *linkdev,
> +                  char *stateDir)
> +{
> +    int ret;
> +    char *oldmacname = NULL;
> +    char *macstr = NULL;
> +    char *path = NULL;
> +    unsigned char oldmac[6];
> +
> +    if (virAsprintf(&path, "%s/%s",
> +                    stateDir,
> +                    linkdev)<  0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN,&macstr)<  0) {
> +        return errno;
> +    }
> +
> +    if (virParseMacAddr(macstr,&oldmac[0]) != 0) {
> +        macvtapError(VIR_ERR_INTERNAL_ERROR,
> +                             _("Cannot parse MAC address from '%s'"),
> +                             oldmacname);
> +        return -1;
> +    }
> +
> +    /*reset mac and remove file-ignore results*/
> +    ignore_value(ifaceSetMacaddr(linkdev, oldmac));
> +    ignore_value(unlink(path));
> +    VIR_FREE(macstr);
> +    return 0;
> +}
>
>   /**
>    * openMacvtapTap:
> @@ -575,7 +673,8 @@ openMacvtapTap(const char *tgifname,
>                  const unsigned char *vmuuid,
>                  virVirtualPortProfileParamsPtr virtPortProfile,
>                  char **res_ifname,
> -               enum virVMOperationType vmOp)
> +               enum virVMOperationType vmOp,
> +               char *stateDir)
>   {
>       const char *type = "macvtap";
>       int c, rc;
> @@ -589,6 +688,19 @@ openMacvtapTap(const char *tgifname,
>
>       VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__, virVMOperationTypeToString(vmOp));
>
> +    /** Note: When using PASSTHROUGH mode with MACVTAP devices the link
> +     * device's MAC address must be set to the VMs MAC address. In
> +     * order to not confuse the first switch or bridge in line this MAC
> +     * 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.
> +     */
> +    if (mode == VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU) {
> +        if (replaceMacAdress(macaddress, linkdev, stateDir) != 0) {
> +            return -1;
> +        }
> +    }
> +
>       if (tgifname) {
>           if(ifaceGetIndex(false, tgifname,&ifindex) == 0) {
>               if (STRPREFIX(tgifname,
> @@ -684,8 +796,14 @@ void
>   delMacvtap(const char *ifname,
>              const unsigned char *macaddr,
>              const char *linkdev,
> -           virVirtualPortProfileParamsPtr virtPortProfile)
> +           int mode,
> +           virVirtualPortProfileParamsPtr virtPortProfile,
> +           char *stateDir)
>   {
> +    if (mode == VIR_DOMAIN_NETDEV_MACVTAP_MODE_PASSTHRU) {
> +        restoreMacAddress(linkdev, stateDir);
> +    }
> +
>       if (ifname) {
>           vpDisassociatePortProfileId(ifname, macaddr,
>                                       linkdev,
> Index: libvirt/src/util/macvtap.h
> ===================================================================
> --- libvirt.orig/src/util/macvtap.h
> +++ libvirt/src/util/macvtap.h
> @@ -83,12 +83,15 @@ int openMacvtapTap(const char *ifname,
>                      const unsigned char *vmuuid,
>                      virVirtualPortProfileParamsPtr virtPortProfile,
>                      char **res_ifname,
> -                   enum virVMOperationType vmop);
> +                   enum virVMOperationType vmop,
> +                   char *stateDir);
>
>   void delMacvtap(const char *ifname,
>                   const unsigned char *macaddress,
>                   const char *linkdev,
> -                virVirtualPortProfileParamsPtr virtPortProfile);
> +                int mode,
> +                virVirtualPortProfileParamsPtr virtPortProfile,
> +                char *stateDir);
>
>   int vpAssociatePortProfileId(const char *macvtap_ifname,
>                                const unsigned char *macvtap_macaddr,
> Index: libvirt/src/qemu/qemu_hotplug.c
> ===================================================================
> --- libvirt.orig/src/qemu/qemu_hotplug.c
> +++ libvirt/src/qemu/qemu_hotplug.c
> @@ -1610,7 +1610,9 @@ int qemuDomainDetachNetDevice(struct qem
>   #if WITH_MACVTAP
>       if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
>           delMacvtap(detach->ifname, detach->mac, detach->data.direct.linkdev,
> -&detach->data.direct.virtPortProfile);
> +                   detach->data.direct.mode,
> +&detach->data.direct.virtPortProfile,
> +                   driver->stateDir);
>           VIR_FREE(detach->ifname);
>       }
>   #endif
> Index: libvirt/src/util/interface.c
> ===================================================================
> --- libvirt.orig/src/util/interface.c
> +++ libvirt/src/util/interface.c
> @@ -390,3 +390,99 @@ ifaceGetVlanID(const char *vlanifname AT
>       return ENOSYS;
>   }
>   #endif /* __linux__ */
> +
> +/**
> + * ifaceGetMacaddr:
> + * @ifname: interface name to set MTU for
> + * @macaddr: MAC address (VIR_MAC_BUFLEN in size)
> + *
> + * This function gets the @macaddr for a given interface @ifname.
> + *
> + * Returns 0 in case of success or an errno code in case of failure.
> + */
> +#ifdef __linux__
> +int
> +ifaceGetMacaddr(const char *ifname ATTRIBUTE_UNUSED,
> +                unsigned char *macaddr ATTRIBUTE_UNUSED)
> +{
Here the ATTRIBUTE_UNUSED are not necessary.
> +    struct ifreq ifr;
> +    int fd;
> +
> +    if (!ifname)
> +        return EINVAL;
> +
> +    fd = socket(AF_INET, SOCK_STREAM, 0);
> +    if (fd<  0)
> +        return errno;
> +
> +    memset(&ifr, 0, sizeof(struct ifreq));
> +    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
> +        return EINVAL;
> +
> +    if (ioctl(fd, SIOCGIFHWADDR, (char *)&ifr) != 0)
> +        return errno;
> +
> +    memcpy(macaddr, ifr.ifr_ifru.ifru_hwaddr.sa_data, VIR_MAC_BUFLEN);
> +
> +    return 0;
> +}
> +
> +#else
> +
> +int
> +ifaceGetMacaddr(const char *ifname ATTRIBUTE_UNUSED,
> +                unsigned char *macaddr ATTRIBUTE_UNUSED)
> +{
> +    return ENOSYS;
> +}
> +
> +#endif /* __linux__ */
> +
> +/**
> + * ifaceSetMacaddr:
> + * @ifname: interface name to set MTU for
> + * @macaddr: MAC address (VIR_MAC_BUFLEN in size)
> + *
> + * This function sets the @macaddr for a given interface @ifname. This
> + * gets rid of the kernel's automatically assigned random MAC.
> + *
> + * Returns 0 in case of success or an errno code in case of failure.
> + */
> +#ifdef __linux__
> +int
> +ifaceSetMacaddr(const char *ifname ATTRIBUTE_UNUSED,
> +                const unsigned char *macaddr ATTRIBUTE_UNUSED)
> +{
> +    struct ifreq ifr;
> +    int fd;
> +
> +    if (!ifname)
> +        return EINVAL;
> +
> +    fd = socket(AF_INET, SOCK_STREAM, 0);
> +    if (fd<  0)
> +        return errno;
> +
> +    memset(&ifr, 0, sizeof(struct ifreq));
> +    if (virStrcpyStatic(ifr.ifr_name, ifname) == NULL)
> +        return EINVAL;
> +
> +    /* To fill ifr.ifr_hdaddr.sa_family field */
> +    if (ioctl(fd, SIOCGIFHWADDR,&ifr) != 0)
> +        return errno;
> +
> +    memcpy(ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN);
> +
> +    return ioctl(fd, SIOCSIFHWADDR,&ifr) == 0 ? 0 : errno;
> +}
> +
> +#else
> +
> +int
> +ifaceSetMacaddr(const char *ifname ATTRIBUTE_UNUSED,
> +                const unsigned char *macaddr ATTRIBUTE_UNUSED)
> +{
> +    return ENOSYS;
> +}
> +
> +#endif /* __linux__ */
> Index: libvirt/src/util/interface.h
> ===================================================================
> --- libvirt.orig/src/util/interface.h
> +++ libvirt/src/util/interface.h
> @@ -32,4 +32,8 @@ int ifaceGetIndex(bool reportError, cons
>
>   int ifaceGetVlanID(const char *vlanifname, int *vlanid);
>
> +int ifaceSetMacaddr(const char *ifname, const unsigned char *macaddr);
> +
> +int ifaceGetMacaddr(const char *ifname, unsigned char *macaddr);
> +
>   #endif /* __VIR_INTERFACE_H__ */
> Index: libvirt/src/libvirt_private.syms
> ===================================================================
> --- libvirt.orig/src/libvirt_private.syms
> +++ libvirt/src/libvirt_private.syms
> @@ -507,7 +507,9 @@ ifaceCheck;
>   ifaceCtrl;
>   ifaceGetFlags;
>   ifaceGetIndex;
> +ifaceGetMacaddr;
>   ifaceGetVlanID;
> +ifaceSetMacaddr;
>   ifaceIsUp;
>
>
> ===================================================================
>
ACK. Unless someone else opposes, I'd push later today cleaning up what 
I commented about above.

   Stefan




More information about the libvir-list mailing list