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. Signed-off-by: Stefan Berger <stefanb@us.ibm.com> --- src/Makefile.am | 1 src/libvirt_private.syms | 4 src/nwfilter/nwfilter_gentech_driver.c | 120 ----------------- src/nwfilter/nwfilter_gentech_driver.h | 2 src/nwfilter/nwfilter_learnipaddr.c | 3 src/util/interface.c | 224 +++++++++++++++++++++++++++++++++ src/util/interface.h | 31 ++++ src/util/macvtap.c | 112 ---------------- 8 files changed, 269 insertions(+), 228 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c +++ libvirt-acl/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(virConnect } -// 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(virConn driver); if (rc) { //something went wrong... 'DOWN' the interface - if (ifUp(ifname ,0)) { + if (IfaceDown(ifname)) { // assuming interface disappeared... _virNWFilterTeardownFilter(ifname); } Index: libvirt-acl/src/util/interface.c =================================================================== --- /dev/null +++ libvirt-acl/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; +} Index: libvirt-acl/src/util/interface.h =================================================================== --- /dev/null +++ libvirt-acl/src/util/interface.h @@ -0,0 +1,31 @@ +/* + * 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__ */ + Index: libvirt-acl/src/Makefile.am =================================================================== --- libvirt-acl.orig/src/Makefile.am +++ libvirt-acl/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 \ Index: libvirt-acl/src/libvirt_private.syms =================================================================== --- libvirt-acl.orig/src/libvirt_private.syms +++ libvirt-acl/src/libvirt_private.syms @@ -681,6 +681,10 @@ virFileWaitForDevices; virFileMatchesNameSuffix; virArgvToString; +# interface.h +IfaceCtrl; +IfaceCheck; +IfaceGetIndex; # usb.h usbGetDevice; Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c +++ libvirt-acl/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; } Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/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" @@ -193,109 +194,6 @@ nlAppend(struct nlmsghdr *nlm, int totle 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, const char *ifname, @@ -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 " Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.h +++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.h @@ -63,6 +63,4 @@ void virNWFilterDomainFWUpdateCB(void *p const char *name ATTRIBUTE_UNUSED, void *data); -int checkIf(const char *ifname, const unsigned char *macaddr); - #endif