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

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



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


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