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

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



On 06/16/2011 02:16 PM, Daniel P. Berrange wrote:
On Thu, Jun 16, 2011 at 02:23:07PM +0200, Gerhard Stenzel wrote:

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

  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;
+}
There are already (static) functions in src/util/bridge.c, which are
able to get and set the MAC address on network devices. So I think we
should just make them non-static and call them instead of duplicating
it here.
The problem with bridge.c is the brControl structure that's private.

I think the above 2 functions should be moved to util/interface.c since they can become generic interface functions like the other ones already found there. There they should then be prefixed with 'iface'. If there is duplicate functionality in bridge.c, maybe bridge.c should be built on top of interface.c and then can keep its brControl structure private.

   Stefan


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