Consolidate interface related functions in interface.c

Changes from v1 to v2:
- changed function name prefixes to 'iface' from previous 'Iface'

- Further to make make syntax-check pass:
 - indentation fix in interface.h
 - added entry to POTFILES.in

I am consolidating network interface related functions used in nwfilter
and macvtap code in utils/interface.c. All function names are prefixed
with 'Iface'. The following functions are now available through
interface.h:

int ifaceCtrl(const char *name, bool up);
int ifaceUp(const char *name);
int ifaceDown(const char *name);

int ifaceCheck(bool reportError, const char *ifname,
               const unsigned char *macaddr, int ifindex);

int ifaceGetIndex(bool reportError, const char *ifname, int *ifindex);

I added 'int ifindex' as parameter to ifaceCheck to the original
function and modified the code accordingly.

po/POTFILES.in

@@ -72,6 +72,7 @@ src/util/bridge.c
 src/util/conf.c
 src/util/hooks.c
 src/util/hostusb.c
+src/util/interface.c
 src/util/json.c
 src/util/logging.c
 src/util/macvtap.c

src/Makefile.am

@@ -67,6 +67,7 @@ UTIL_SOURCES =							\
 		util/processinfo.c util/processinfo.h		\
 		util/hostusb.c util/hostusb.h			\
 		util/network.c util/network.h			\
+		util/interface.c util/interface.h		\
 		util/qparams.c util/qparams.h			\
 		util/stats_linux.c util/stats_linux.h		\
 		util/storage_file.c util/storage_file.h		\

src/libvirt_private.syms

@@ -681,6 +681,10 @@ virFileWaitForDevices;
 virFileMatchesNameSuffix;
 virArgvToString;
 
+# interface.h
+ifaceCtrl;
+ifaceCheck;
+ifaceGetIndex;
 
 # usb.h
 usbGetDevice;

src/nwfilter/nwfilter_gentech_driver.c

@@ -23,16 +23,11 @@
 
 #include <config.h>
 
-#include <stdint.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <linux/if.h>
-
 #include "internal.h"
 
 #include "memory.h"
 #include "logging.h"
-#include "datatypes.h"
+#include "interface.h"
 #include "domain_conf.h"
 #include "virterror_internal.h"
 #include "nwfilter_gentech_driver.h"
@@ -792,117 +787,6 @@ _virNWFilterInstantiateFilter(virConnectPtr conn,
 }
 
 
-// FIXME: move chgIfFlags, ifUp, checkIf into common file & share w/ macvtap.c
-
-/*
- * chgIfFlags: Change flags on an interface
- * @ifname : name of the interface
- * @flagclear : the flags to clear
- * @flagset : the flags to set
- *
- * The new flags of the interface will be calculated as
- * flagmask = (~0 ^ flagclear)
- * newflags = (curflags & flagmask) | flagset;
- *
- * Returns 0 on success, errno on failure.
- */
-static int chgIfFlags(const char *ifname, short flagclear, short flagset) {
-    struct ifreq ifr;
-    int rc = 0;
-    int flags;
-    short flagmask = (~0 ^ flagclear);
-    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
-
-    if (fd < 0)
-        return errno;
-
-    if (virStrncpy(ifr.ifr_name,
-                   ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) {
-        rc = ENODEV;
-        goto err_exit;
-    }
-
-    if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
-        rc = errno;
-        goto err_exit;
-    }
-
-    flags = (ifr.ifr_flags & flagmask) | flagset;
-
-    if (ifr.ifr_flags != flags) {
-        ifr.ifr_flags = flags;
-
-        if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0)
-            rc = errno;
-    }
-
-err_exit:
-    close(fd);
-    return rc;
-}
-
-/*
- * ifUp
- * @name: name of the interface
- * @up: 1 for up, 0 for down
- *
- * Function to control if an interface is activated (up, 1) or not (down, 0)
- *
- * Returns 0 in case of success or an errno code in case of failure.
- */
-static int
-ifUp(const char *name, int up)
-{
-    return chgIfFlags(name,
-                      (up) ? 0      : IFF_UP,
-                      (up) ? IFF_UP : 0);
-}
-
-
-/**
- * checkIf
- *
- * @ifname: Name of the interface
- * @macaddr: expected MAC address of the interface
- *
- * FIXME: the interface's index is another good parameter to check
- *
- * Determine whether a given interface is still available. If so,
- * it must have the given MAC address.
- *
- * Returns an error code ENODEV in case the interface does not exist
- * anymore or its MAC address is different, 0 otherwise.
- */
-int
-checkIf(const char *ifname, const unsigned char *macaddr)
-{
-    struct ifreq ifr;
-    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
-    int rc = 0;
-
-    if (fd < 0)
-        return errno;
-
-    if (virStrncpy(ifr.ifr_name,
-                   ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) {
-        rc = ENODEV;
-        goto err_exit;
-    }
-
-    if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
-        rc = errno;
-        goto err_exit;
-    }
-
-    if (memcmp(&ifr.ifr_hwaddr.sa_data, macaddr, 6) != 0)
-        rc = ENODEV;
-
- err_exit:
-    close(fd);
-    return rc;
-}
-
-
 int
 virNWFilterInstantiateFilterLate(virConnectPtr conn,
                                  const char *ifname,
@@ -926,7 +810,7 @@ virNWFilterInstantiateFilterLate(virConnectPtr conn,
                                         driver);
     if (rc) {
         //something went wrong... 'DOWN' the interface
-        if (ifUp(ifname ,0)) {
+        if (ifaceDown(ifname)) {
             // assuming interface disappeared...
             _virNWFilterTeardownFilter(ifname);
         }

src/nwfilter/nwfilter_gentech_driver.h

@@ -63,6 +63,4 @@ void virNWFilterDomainFWUpdateCB(void *payload,
                                  const char *name ATTRIBUTE_UNUSED,
                                  void *data);
 
-int checkIf(const char *ifname, const unsigned char *macaddr);
-
 #endif

src/nwfilter/nwfilter_learnipaddr.c

@@ -43,6 +43,7 @@
 #include "memory.h"
 #include "logging.h"
 #include "datatypes.h"
+#include "interface.h"
 #include "virterror_internal.h"
 #include "threads.h"
 #include "conf/nwfilter_params.h"
@@ -339,7 +340,7 @@ learnIPAddressThread(void *arg)
                 break;
             }
             /* listening on linkdev, check whether VM's dev is still there */
-            if (checkIf(req->ifname, req->macaddr)) {
+            if (ifaceCheck(false, req->ifname, req->macaddr, -1)) {
                 req->status = ENODEV;
                 break;
             }

src/util/interface.c

@@ -0,0 +1,226 @@
+/*
+ * interface.c: interface support functions
+ *
+ * Copyright (C) 2010 IBM Corp.
+ * Copyright (C) 2010 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ * chgIfaceFlags originated from bridge.c
+ *
+ * Author: Stefan Berger <stefanb@us.ibm.com>
+ */
+
+#include <config.h>
+
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <linux/if.h>
+
+#include "internal.h"
+
+#include "util.h"
+#include "interface.h"
+#include "virterror_internal.h"
+
+#define ifaceError(code, ...) \
+        virReportErrorHelper(NULL, VIR_FROM_NET, code, __FILE__, \
+                             __FUNCTION__, __LINE__, __VA_ARGS__)
+
+/*
+ * chgIfFlags: Change flags on an interface
+ *
+ * @ifname : name of the interface
+ * @flagclear : the flags to clear
+ * @flagset : the flags to set
+ *
+ * The new flags of the interface will be calculated as
+ * flagmask = (~0 ^ flagclear)
+ * newflags = (curflags & flagmask) | flagset;
+ *
+ * Returns 0 on success, errno on failure.
+ */
+static int chgIfaceFlags(const char *ifname, short flagclear, short flagset) {
+    struct ifreq ifr;
+    int rc = 0;
+    int flags;
+    short flagmask = (~0 ^ flagclear);
+    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
+
+    if (fd < 0)
+        return errno;
+
+    if (virStrncpy(ifr.ifr_name,
+                   ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) {
+        rc = ENODEV;
+        goto err_exit;
+    }
+
+    if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
+        rc = errno;
+        goto err_exit;
+    }
+
+    flags = (ifr.ifr_flags & flagmask) | flagset;
+
+    if (ifr.ifr_flags != flags) {
+        ifr.ifr_flags = flags;
+
+        if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0)
+            rc = errno;
+    }
+
+err_exit:
+    close(fd);
+    return rc;
+}
+
+
+/*
+ * ifaceCtrl
+ * @name: name of the interface
+ * @up: true (1) for up, false (0) for down
+ *
+ * Function to control if an interface is activated (up, 1) or not (down, 0)
+ *
+ * Returns 0 in case of success or an errno code in case of failure.
+ */
+int
+ifaceCtrl(const char *name, bool up)
+{
+    return chgIfaceFlags(name,
+                         (up) ? 0      : IFF_UP,
+                         (up) ? IFF_UP : 0);
+}
+
+
+/**
+ * ifaceCheck
+ *
+ * @reportError: whether to report errors or keep silent
+ * @ifname: Name of the interface
+ * @macaddr: expected MAC address of the interface; not checked if NULL
+ * @ifindex: expected index of the interface; not checked if '-1'
+ *
+ * Determine whether a given interface is still available. If so,
+ * it must have the given MAC address and if an interface index is
+ * passed, it must also match the interface index.
+ *
+ * Returns 0 on success, an error code on failure.
+ *   ENODEV : if interface with given name does not exist or its interface
+ *            index is different than the one passed
+ *   EINVAL : if interface name is invalid (too long)
+ */
+int
+ifaceCheck(bool reportError, const char *ifname,
+           const unsigned char *macaddr, int ifindex)
+{
+    struct ifreq ifr;
+    int fd = -1;
+    int rc = 0;
+    int idx;
+
+    if (macaddr != NULL) {
+        fd = socket(PF_PACKET, SOCK_DGRAM, 0);
+        if (fd < 0)
+            return errno;
+
+        if (virStrncpy(ifr.ifr_name,
+                       ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) {
+            if (reportError)
+                ifaceError(VIR_ERR_INTERNAL_ERROR,
+                           _("invalid interface name %s"),
+                           ifname);
+            rc = EINVAL;
+            goto err_exit;
+        }
+
+        if (ioctl(fd, SIOCGIFHWADDR, &ifr) < 0) {
+            if (reportError)
+                ifaceError(VIR_ERR_INTERNAL_ERROR,
+                           _("coud not get MAC address of interface %s"),
+                           ifname);
+            rc = errno;
+            goto err_exit;
+        }
+
+        if (memcmp(&ifr.ifr_hwaddr.sa_data, macaddr, VIR_MAC_BUFLEN) != 0) {
+            rc = ENODEV;
+            goto err_exit;
+        }
+    }
+
+    if (ifindex != -1) {
+        rc = ifaceGetIndex(reportError, ifname, &idx);
+        if (rc == 0 && idx != ifindex)
+            rc = ENODEV;
+    }
+
+ err_exit:
+    if (fd >= 0)
+        close(fd);
+
+    return rc;
+}
+
+
+/**
+ * ifaceGetIndex
+ *
+ * @reportError: whether to report errors or keep silent
+ * @ifname : Name of the interface whose index is to be found
+ * @ifindex: Pointer to int where the index will be written into
+ *
+ * Get the index of an interface given its name.
+ *
+ * Returns 0 on success, an error code on failure.
+ *   ENODEV : if interface with given name does not exist
+ *   EINVAL : if interface name is invalid (too long)
+ */
+int
+ifaceGetIndex(bool reportError, const char *ifname, int *ifindex)
+{
+    int rc = 0;
+    struct ifreq ifreq;
+    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
+
+    if (fd < 0)
+        return errno;
+
+    if (virStrncpy(ifreq.ifr_name, ifname, strlen(ifname),
+                   sizeof(ifreq.ifr_name)) == NULL) {
+        if (reportError)
+            ifaceError(VIR_ERR_INTERNAL_ERROR,
+                       _("invalid interface name %s"),
+                       ifname);
+        rc = EINVAL;
+        goto err_exit;
+    }
+
+    if (ioctl(fd, SIOCGIFINDEX, &ifreq) >= 0)
+        *ifindex = ifreq.ifr_ifindex;
+    else {
+        if (reportError)
+            ifaceError(VIR_ERR_INTERNAL_ERROR,
+                       _("interface %s does not exist"),
+                       ifname);
+        rc = ENODEV;
+    }
+
+err_exit:
+    close(fd);
+
+    return rc;
+}

src/util/interface.h

@@ -0,0 +1,30 @@
+/*
+ * interface.h: interface helper APIs for libvirt
+ *
+ * Copyright (C) 2010 IBM Corporation, Inc.
+ *
+ * See COPYING.LIB for the License of this software
+ *
+ * Stefan Berger <stefanb@us.ibm.com>
+ */
+#ifndef __VIR_INTERFACE_H__
+# define __VIR_INTERFACE_H__
+
+# include "datatypes.h"
+
+int ifaceCtrl(const char *name, bool up);
+
+static inline int ifaceUp(const char *name) {
+    return ifaceCtrl(name, true);
+}
+
+static inline int ifaceDown(const char *name) {
+    return ifaceCtrl(name, false);
+}
+
+int ifaceCheck(bool reportError, const char *ifname,
+               const unsigned char *macaddr, int ifindex);
+
+int ifaceGetIndex(bool reportError, const char *ifname, int *ifindex);
+
+#endif /* __VIR_INTERFACE_H__ */

src/util/macvtap.c

@@ -44,6 +44,7 @@
 # include "util.h"
 # include "memory.h"
 # include "macvtap.h"
+# include "interface.h"
 # include "conf/domain_conf.h"
 # include "virterror_internal.h"
 
@@ -192,109 +193,6 @@ nlAppend(struct nlmsghdr *nlm, int totlen, const void *data, int datalen)
 }
 
 
-static int
-getIfIndex(bool reportError,
-           const char *ifname,
-           int *idx)
-{
-    int rc = 0;
-    struct ifreq ifreq;
-    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
-
-    if (fd < 0)
-        return errno;
-
-    if (virStrncpy(ifreq.ifr_name, ifname, strlen(ifname),
-                   sizeof(ifreq.ifr_name)) == NULL) {
-        if (reportError)
-            macvtapError(VIR_ERR_INTERNAL_ERROR,
-                         _("invalid interface name %s"),
-                         ifname);
-        rc = EINVAL;
-        goto err_exit;
-    }
-    if (ioctl(fd, SIOCGIFINDEX, &ifreq) >= 0)
-        *idx = ifreq.ifr_ifindex;
-    else {
-        if (reportError)
-            macvtapError(VIR_ERR_INTERNAL_ERROR,
-                         _("interface %s does not exist"),
-                         ifname);
-        rc = ENODEV;
-    }
-
-err_exit:
-    close(fd);
-
-    return rc;
-}
-
-
-/*
- * chgIfFlags: Change flags on an interface
- * @ifname : name of the interface
- * @flagclear : the flags to clear
- * @flagset : the flags to set
- *
- * The new flags of the interface will be calculated as
- * flagmask = (~0 ^ flagclear)
- * newflags = (curflags & flagmask) | flagset;
- *
- * Returns 0 on success, errno on failure.
- */
-static int chgIfFlags(const char *ifname, short flagclear, short flagset) {
-    struct ifreq ifr;
-    int rc = 0;
-    int flags;
-    short flagmask = (~0 ^ flagclear);
-    int fd = socket(PF_PACKET, SOCK_DGRAM, 0);
-
-    if (fd < 0)
-        return errno;
-
-    if (virStrncpy(ifr.ifr_name,
-                   ifname, strlen(ifname), sizeof(ifr.ifr_name)) == NULL) {
-        rc = ENODEV;
-        goto err_exit;
-    }
-
-    if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
-        rc = errno;
-        goto err_exit;
-    }
-
-    flags = (ifr.ifr_flags & flagmask) | flagset;
-
-    if (ifr.ifr_flags != flags) {
-        ifr.ifr_flags = flags;
-
-        if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0)
-            rc = errno;
-    }
-
-err_exit:
-    close(fd);
-    return rc;
-}
-
-/*
- * ifUp
- * @name: name of the interface
- * @up: 1 for up, 0 for down
- *
- * Function to control if an interface is activated (up, 1) or not (down, 0)
- *
- * Returns 0 in case of success or an errno code in case of failure.
- */
-static int
-ifUp(const char *name, int up)
-{
-    return chgIfFlags(name,
-                      (up) ? 0      : IFF_UP,
-                      (up) ? IFF_UP : 0);
-}
-
-
 static int
 link_add(const char *type,
          const unsigned char *macaddress, int macaddrsize,
@@ -314,7 +212,7 @@ link_add(const char *type,
     char *recvbuf = NULL;
     int recvbuflen;
 
-    if (getIfIndex(true, srcdev, &ifindex) != 0)
+    if (ifaceGetIndex(true, srcdev, &ifindex) != 0)
         return -1;
 
     *retry = 0;
@@ -708,7 +606,7 @@ openMacvtapTap(const char *tgifname,
     *res_ifname = NULL;
 
     if (tgifname) {
-        if(getIfIndex(false, tgifname, &ifindex) == 0) {
+        if(ifaceGetIndex(false, tgifname, &ifindex) == 0) {
             if (STRPREFIX(tgifname,
                           MACVTAP_NAME_PREFIX)) {
                 goto create_name;
@@ -727,7 +625,7 @@ create_name:
         retries = 5;
         for (c = 0; c < 8192; c++) {
             snprintf(ifname, sizeof(ifname), MACVTAP_NAME_PATTERN, c);
-            if (getIfIndex(false, ifname, &ifindex) == ENODEV) {
+            if (ifaceGetIndex(false, ifname, &ifindex) == ENODEV) {
                 rc = link_add(type, macaddress, 6, ifname, linkdev,
                               macvtapMode, &do_retry);
                 if (rc == 0)
@@ -741,7 +639,7 @@ create_name:
         cr_ifname = ifname;
     }
 
-    rc = ifUp(cr_ifname, 1);
+    rc = ifaceUp(cr_ifname);
     if (rc != 0) {
         virReportSystemError(errno,
                              _("cannot 'up' interface %s -- another "