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

Stefan Berger stefanb at linux.vnet.ibm.com
Sun Jun 19 16:10:02 UTC 2011


On 06/17/2011 10:08 AM, Gerhard Stenzel wrote:
> This is a another rework of the patch Dirk sent out last week
> taking into account most propsosed changes
>
> 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/libvirt_macvtap.syms
> ===================================================================
> --- libvirt.orig/src/libvirt_macvtap.syms
> +++ libvirt/src/libvirt_macvtap.syms
> @@ -5,6 +5,8 @@
>
>   # macvtap.h
>   delMacvtap;
> +getMacaddr;
>   openMacvtapTap;
> +setMacaddr;
>   vpAssociatePortProfileId;
>   vpDisassociatePortProfileId;
> 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,7 @@ 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
> @@ -2707,7 +2707,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
> @@ -87,6 +87,7 @@
>
>   # define LLDPAD_PID_FILE  "/var/run/lldpad.pid"
>
> +#define MACADDRSIZE 6
Consider ETH_ALEN via <netinet/ether.h>.
>   enum virVirtualPortOp {
>       ASSOCIATE = 0x1,
> @@ -191,6 +192,149 @@ err_exit:
>
>   # if WITH_MACVTAP
>
> +/**
> + * getMacaddr:
> + * Get the MAC address of a network device
> + *
> + * @macaddress: Pointer where the MAC address will be stored
> + * @srcdev: The interface name of the NIC to get the MAC from
> + *
> + * Returns zero in case of success,
> + * negative value otherwise with error reported.
> + *
> + */
> +int
> +getMacaddr(const unsigned char *macaddress, const char *srcdev )
> +{
> +    int sockfd;
> +    int io;
> +    struct ifreq ifr;
> +
> +    strcpy(ifr.ifr_name, srcdev);
> +
> +    sockfd = socket(AF_INET, SOCK_STREAM, 0);
> +    if(sockfd<  0){
> +        return -1;
> +    }
> +
> +    io = ioctl(sockfd, SIOCGIFHWADDR, (char *)&ifr);
> +    if(io<  0){
> +        return -1;
> +    }
> +
> +    memcpy(macaddress, ifr.ifr_ifru.ifru_hwaddr.sa_data, MACADDRSIZE);
> +
> +    return 0;
> +}
> +
> +/**
> + * setMacaddr:
> + * Set the MAC address of a network device
> + *
> + * @macaddress: MAC address to assign to the NIC
> + * @srcdev: The interface name of the NIC
> + *
> + * Returns zero in case of success,
> + * negative value otherwise with error reported.
> + *
> + */
> +int
> +setMacaddr(const unsigned char *macaddress, const char *srcdev )
> +{
> +    int rc = 0;
> +    struct nlmsghdr *resp;
> +    struct nlmsgerr *err;
> +    struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
> +    int ifindex;
> +    unsigned char *recvbuf = NULL;
> +    unsigned int recvbuflen;
> +    struct nl_msg *nl_msg;
> +
> +    if (ifaceGetIndex(true, srcdev,&ifindex) != 0)
> +        return -1;
> +
> +    nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST);
> +
> +    if (!nl_msg) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    if (nlmsg_append(nl_msg,&ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO)<  0)
> +        goto buffer_too_small;
> +
> +    if (nla_put_u32(nl_msg, IFLA_LINK, ifindex)<  0)
> +        goto buffer_too_small;
> +
> +    if (nla_put(nl_msg, IFLA_ADDRESS, MACADDRSIZE, macaddress)<  0)
> +        goto buffer_too_small;
> +
> +    if (srcdev&&
> +        nla_put(nl_msg, IFLA_IFNAME, strlen(srcdev)+1, srcdev)<  0)
> +        goto buffer_too_small;
> +
> +    if (nlComm(nl_msg,&recvbuf,&recvbuflen, 0)<  0) {
> +        rc = -1;
> +        goto err_exit;
> +    }
> +
> +    if (recvbuflen<  NLMSG_LENGTH(0) || recvbuf == NULL)
> +        goto malformed_resp;
> +
> +    resp = (struct nlmsghdr *)recvbuf;
> +
> +    switch (resp->nlmsg_type) {
> +    case NLMSG_ERROR:
> +        err = (struct nlmsgerr *)NLMSG_DATA(resp);
> +        if (resp->nlmsg_len<  NLMSG_LENGTH(sizeof(*err)))
> +            goto malformed_resp;
> +
> +        switch (err->error) {
> +
> +        case 0:
> +            break;
> +
> +        case -EEXIST:
> +            rc = -1;
> +            break;
> +
> +        default:
> +            macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                         _("error setting device mac address"));
> +            rc = -1;
> +        }
> +        break;
> +
> +    case NLMSG_DONE:
> +        break;
> +
> +    default:
> +        goto malformed_resp;
> +    }
> +
> +err_exit:
> +    nlmsg_free(nl_msg);
> +
> +    VIR_FREE(recvbuf);
> +
> +    return rc;
> +
> +malformed_resp:
> +    nlmsg_free(nl_msg);
> +
> +    macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                 _("malformed netlink response message"));
> +    VIR_FREE(recvbuf);
> +    return -1;
> +
> +buffer_too_small:
> +    nlmsg_free(nl_msg);
> +
> +    macvtapError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                 _("allocated netlink buffer is too small"));
> +    return -1;
> +}
> +
>   static int
>   link_add(const char *type,
>            const unsigned char *macaddress, int macaddrsize,
> @@ -575,7 +719,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 +734,54 @@ 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) {
> +        unsigned char oldmac[6];
> +
> +        rc = getMacaddr(&oldmac, linkdev);
> +        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 = setMacaddr(macaddress, linkdev);
> +        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]);
> +        }
Can you put this part into a function of its own?
> +    }
> +
> +
>       if (tgifname) {
>           if(ifaceGetIndex(false, tgifname,&ifindex) == 0) {
>               if (STRPREFIX(tgifname,
> @@ -684,8 +886,43 @@ 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) {
> +                int ret;
> +                char *oldmacname = NULL;
> +                char *macstr = NULL;
> +                char *path = NULL;
> +                unsigned char oldmac[6];
> +
> +                if (virAsprintf(&path, "%s/%s",
> +                                stateDir,
> +                                linkdev)<  0) {
> +                    virReportOOMError();
> +                    goto disassociate_exit;
> +                }
> +
> +                if (virFileReadAll(path, VIR_MAC_STRING_BUFLEN,&macstr)<  0) {
> +                    virReportSystemError(errno, _("Unable to preserve mac for %s"),
> +                                         linkdev);
> +                    goto disassociate_exit;
> +                }
> +
> +                if (virParseMacAddr(macstr,&oldmac[0]) != 0) {
> +                    virReportSystemError(VIR_ERR_INTERNAL_ERROR,
> +                                         _("Cannot parse MAC address from '%s'"),
> +                                         oldmacname);
> +                }
> +
> +                /*reset mac and remove file-ignore results*/
> +                ret = setMacaddr(oldmac, linkdev);
> +                ret = unlink(path);
> +                VIR_FREE(macstr);
and also put this part into a function of its own?
> +            }
> +
> +disassociate_exit:
>       if (ifname) {
>           vpDisassociatePortProfileId(ifname, macaddr,
>                                       linkdev,
> Index: libvirt/src/util/macvtap.h
> ===================================================================
> --- libvirt.orig/src/util/macvtap.h
> +++ libvirt/src/util/macvtap.h
> @@ -75,6 +75,10 @@ enum virVMOperationType {
>
>   #  include "internal.h"
>
> +int getMacaddr(const unsigned char *macaddress, const char *srcdev );
> +
> +int setMacaddr(const unsigned char *macaddress, const char *srcdev );
> +
>   int openMacvtapTap(const char *ifname,
>                      const unsigned char *macaddress,
>                      const char *linkdev,
> @@ -83,12 +87,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,7 @@ 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
> ===================================================================
Stefan

> Best regards,
>
> Gerhard Stenzel
> -------------------------------------------------------------------------------------
> IBM Deutschland Research&  Development GmbH
> Vorsitzender des Aufsichtsrats: Martin Jetter
> Geschaeftsfuehrung: Dirk Wittkopp
> Sitz der Gesellschaft: Boeblingen
> Registergericht: Amtsgericht Stuttgart, HRB 243294
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list