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

Re: [libvirt] [PATCH V11 5/7] nwfilter: move code for IP address map into separate file



On Tue, Apr 17, 2012 at 10:44:06AM -0400, Stefan Berger wrote:
> The goal of this patch is to prepare for support for multiple IP
> addresses per interface in the DHCP snooping code.
> 
> Move the code for the IP address map that maps interface names to
> IP addresses into their own file. Rename the functions on the way
> but otherwise leave the code as-is. Initialize this new layer
> separately before dependent layers (iplearning, dhcpsnooping)
> and shut it down after them.
> 
> ---
>  src/Makefile.am                        |    4 
>  src/conf/nwfilter_ipaddrmap.c          |  167 +++++++++++++++++++++++++++++++++
>  src/conf/nwfilter_ipaddrmap.h          |   37 +++++++
>  src/libvirt_private.syms               |    8 +
>  src/nwfilter/nwfilter_driver.c         |   11 +-
>  src/nwfilter/nwfilter_gentech_driver.c |    5 
>  src/nwfilter/nwfilter_learnipaddr.c    |  126 ------------------------
>  src/nwfilter/nwfilter_learnipaddr.h    |    3 
>  8 files changed, 229 insertions(+), 132 deletions(-)
> 
> Index: libvirt-acl/src/Makefile.am
> ===================================================================
> --- libvirt-acl.orig/src/Makefile.am
> +++ libvirt-acl/src/Makefile.am
> @@ -159,9 +159,11 @@ NETWORK_CONF_SOURCES =						\
>  # Network filter driver generic impl APIs
>  NWFILTER_PARAM_CONF_SOURCES =					\
>  		conf/nwfilter_params.c conf/nwfilter_params.h	\
> +		conf/nwfilter_ipaddrmap.c			\
> +		conf/nwfilter_ipaddrmap.h			\
>  		conf/nwfilter_conf.h
>  
> -NWFILTER_CONF_SOURCES =					\
> +NWFILTER_CONF_SOURCES =						\
>  		$(NWFILTER_PARAM_CONF_SOURCES)			\
>  		conf/nwfilter_conf.c conf/nwfilter_conf.h
>  
> Index: libvirt-acl/src/nwfilter/nwfilter_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c
> +++ libvirt-acl/src/nwfilter/nwfilter_driver.c
> @@ -39,6 +39,7 @@
>  #include "nwfilter_gentech_driver.h"
>  #include "configmake.h"
>  
> +#include "nwfilter_ipaddrmap.h"
>  #include "nwfilter_dhcpsnoop.h"
>  #include "nwfilter_learnipaddr.h"
>  
> @@ -67,10 +68,12 @@ static int
>  nwfilterDriverStartup(int privileged) {
>      char *base = NULL;
>  
> -    if (virNWFilterDHCPSnoopInit() < 0)
> +    if (virNWFilterIPAddrMapInit() < 0)
>          return -1;
>      if (virNWFilterLearnInit() < 0)
> -        return -1;
> +        goto err_exit_ipaddrmapshutdown;
> +    if (virNWFilterDHCPSnoopInit() < 0)
> +        goto err_exit_learnshutdown;
>  
>      virNWFilterTechDriversInit(privileged);
>  
> @@ -131,7 +134,10 @@ alloc_err_exit:
>  conf_init_err:
>      virNWFilterTechDriversShutdown();
>      virNWFilterDHCPSnoopShutdown();
> +err_exit_learnshutdown:
>      virNWFilterLearnShutdown();
> +err_exit_ipaddrmapshutdown:
> +    virNWFilterIPAddrMapShutdown();
>  
>      return -1;
>  }
> @@ -210,6 +216,7 @@ nwfilterDriverShutdown(void) {
>      virNWFilterTechDriversShutdown();
>      virNWFilterDHCPSnoopShutdown();
>      virNWFilterLearnShutdown();
> +    virNWFilterIPAddrMapShutdown();
>  
>      nwfilterDriverLock(driverState);
>  
> 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
> @@ -33,6 +33,7 @@
>  #include "nwfilter_gentech_driver.h"
>  #include "nwfilter_ebiptables_driver.h"
>  #include "nwfilter_dhcpsnoop.h"
> +#include "nwfilter_ipaddrmap.h"
>  #include "nwfilter_learnipaddr.h"
>  #include "virnetdev.h"
>  #include "datatypes.h"
> @@ -870,7 +871,7 @@ __virNWFilterInstantiateFilter(const uns
>          goto err_exit;
>      }
>  
> -    ipaddr = virNWFilterGetIpAddrForIfname(ifname);
> +    ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname);
>  
>      vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr);
>      if (!vars1) {
> @@ -1132,7 +1133,7 @@ _virNWFilterTeardownFilter(const char *i
>  
>      techdriver->allTeardown(ifname);
>  
> -    virNWFilterDelIpAddrForIfname(ifname, NULL);
> +    virNWFilterIPAddrMapDelIPAddr(ifname, NULL);
>  
>      virNWFilterUnlockIface(ifname);
>  
> Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
> ===================================================================
> --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c
> +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
> @@ -52,6 +52,7 @@
>  #include "conf/domain_conf.h"
>  #include "nwfilter_gentech_driver.h"
>  #include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_ipaddrmap.h"
>  #include "nwfilter_learnipaddr.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NWFILTER
> @@ -118,9 +119,6 @@ struct ether_vlan_header
>  static virMutex pendingLearnReqLock;
>  static virHashTablePtr pendingLearnReq;
>  
> -static virMutex ipAddressMapLock;
> -static virNWFilterHashTablePtr ipAddressMap;
> -
>  static virMutex ifaceMapLock;
>  static virHashTablePtr ifaceLockMap;
>  
> @@ -310,113 +308,8 @@ virNWFilterDeregisterLearnReq(int ifinde
>      return res;
>  }
>  
> -/* Add an IP address to the list of IP addresses an interface is
> - * known to use. This function feeds the per-interface cache that
> - * is used to instantiate filters with variable '$IP'.
> - *
> - * @ifname: The name of the (tap) interface
> - * @addr: An IPv4 address in dotted decimal format that the (tap)
> - *        interface is known to use.
> - *
> - * This function returns 0 on success, -1 otherwise
> - */
> -static int
> -virNWFilterAddIpAddrForIfname(const char *ifname, char *addr)
> -{
> -    int ret = -1;
> -    virNWFilterVarValuePtr val;
> -
> -    virMutexLock(&ipAddressMapLock);
> -
> -    val = virHashLookup(ipAddressMap->hashTable, ifname);
> -    if (!val) {
> -        val = virNWFilterVarValueCreateSimple(addr);
> -        if (!val) {
> -            virReportOOMError();
> -            goto cleanup;
> -        }
> -        ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1);
> -        goto cleanup;
> -    } else {
> -        if (virNWFilterVarValueAddValue(val, addr) < 0)
> -            goto cleanup;
> -    }
> -
> -    ret = 0;
> -
> -cleanup:
> -    virMutexUnlock(&ipAddressMapLock);
> -
> -    return ret;
> -}
>  #endif
>  
> -/* Delete all or a specific IP address from an interface. After this
> - * call either all or the given IP address will not be associated
> - * with the interface anymore.
> - *
> - * @ifname: The name of the (tap) interface
> - * @addr: An IPv4 address in dotted decimal format that the (tap)
> - *        interface is not using anymore; provide NULL to remove all IP
> - *        addresses associated with the given interface
> - *
> - * This function returns the number of IP addresses that are still
> - * known to be associated with this interface, in case of an error
> - * -1 is returned. Error conditions are:
> - * - IP addresses is not known to be associated with the interface
> - */
> -int
> -virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr)
> -{
> -    int ret = -1;
> -    virNWFilterVarValuePtr val = NULL;
> -
> -    virMutexLock(&ipAddressMapLock);
> -
> -    if (ipaddr != NULL) {
> -        val = virHashLookup(ipAddressMap->hashTable, ifname);
> -        if (val) {
> -            if (virNWFilterVarValueGetCardinality(val) == 1 &&
> -                STREQ(ipaddr,
> -                      virNWFilterVarValueGetNthValue(val, 0)))
> -                goto remove_entry;
> -            virNWFilterVarValueDelValue(val, ipaddr);
> -            ret = virNWFilterVarValueGetCardinality(val);
> -        }
> -    } else {
> -remove_entry:
> -        /* remove whole entry */
> -        val = virNWFilterHashTableRemoveEntry(ipAddressMap, ifname);
> -        virNWFilterVarValueFree(val);
> -        ret = 0;
> -    }
> -
> -    virMutexUnlock(&ipAddressMapLock);
> -
> -    return ret;
> -}
> -
> -/* Get the list of IP addresses known to be in use by an interface
> - *
> - * This function returns NULL in case no IP address is known to be
> - * associated with the interface, a virNWFilterVarValuePtr otherwise
> - * that then can contain one or multiple entries.
> - */
> -virNWFilterVarValuePtr
> -virNWFilterGetIpAddrForIfname(const char *ifname)
> -{
> -    virNWFilterVarValuePtr res;
> -
> -    virMutexLock(&ipAddressMapLock);
> -
> -    res = virHashLookup(ipAddressMap->hashTable, ifname);
> -
> -    virMutexUnlock(&ipAddressMapLock);
> -
> -    return res;
> -}
> -
> -
>  #ifdef HAVE_LIBPCAP
>  
>  static void
> @@ -699,7 +592,7 @@ learnIPAddressThread(void *arg)
>          char *inetaddr;
>  
>          if ((inetaddr = virSocketAddrFormat(&sa)) != NULL) {
> -            if (virNWFilterAddIpAddrForIfname(req->ifname, inetaddr) < 0) {
> +            if (virNWFilterIPAddrMapAddIPAddr(req->ifname, inetaddr) < 0) {
>                  VIR_ERROR(_("Failed to add IP address %s to IP address "
>                            "cache for interface %s"), inetaddr, req->ifname);
>              }
> @@ -901,18 +794,6 @@ virNWFilterLearnInit(void) {
>          return -1;
>      }
>  
> -    ipAddressMap = virNWFilterHashTableCreate(0);
> -    if (!ipAddressMap) {
> -        virReportOOMError();
> -        virNWFilterLearnShutdown();
> -        return -1;
> -    }
> -
> -    if (virMutexInit(&ipAddressMapLock) < 0) {
> -        virNWFilterLearnShutdown();
> -        return -1;
> -    }
> -
>      ifaceLockMap = virHashCreate(0, freeIfaceLock);
>      if (!ifaceLockMap) {
>          virNWFilterLearnShutdown();
> @@ -954,9 +835,6 @@ virNWFilterLearnShutdown(void)
>      virHashFree(pendingLearnReq);
>      pendingLearnReq = NULL;
>  
> -    virNWFilterHashTableFree(ipAddressMap);
> -    ipAddressMap = NULL;
> -
>      virHashFree(ifaceLockMap);
>      ifaceLockMap = NULL;
>  }
> Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h
> ===================================================================
> --- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.h
> +++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.h
> @@ -65,9 +65,6 @@ int virNWFilterLearnIPAddress(virNWFilte
>  virNWFilterIPAddrLearnReqPtr virNWFilterLookupLearnReq(int ifindex);
>  int virNWFilterTerminateLearnReq(const char *ifname);
>  
> -int virNWFilterDelIpAddrForIfname(const char *ifname, const char *ipaddr);
> -virNWFilterVarValuePtr virNWFilterGetIpAddrForIfname(const char *ifname);
> -
>  int virNWFilterLockIface(const char *ifname) ATTRIBUTE_RETURN_CHECK;
>  void virNWFilterUnlockIface(const char *ifname);
>  
> Index: libvirt-acl/src/libvirt_private.syms
> ===================================================================
> --- libvirt-acl.orig/src/libvirt_private.syms
> +++ libvirt-acl/src/libvirt_private.syms
> @@ -853,6 +853,14 @@ virNWFilterTestUnassignDef;
>  virNWFilterUnlockFilterUpdates;
>  
>  
> +# nwfilter_ipaddrmap
> +virNWFilterIPAddrMapAddIPAddr;
> +virNWFilterIPAddrMapDelIPAddr;
> +virNWFilterIPAddrMapGetIPAddr;
> +virNWFilterIPAddrMapInit;
> +virNWFilterIPAddrMapShutdown;
> +
> +
>  # nwfilter_params.h
>  virNWFilterHashTableCreate;
>  virNWFilterHashTableFree;

  Weird I would have expected some of the renamed/moved functions
to be removed from the libvirt_private.syms, I'm surprized to see
only additions :)

> Index: libvirt-acl/src/conf/nwfilter_ipaddrmap.c
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/conf/nwfilter_ipaddrmap.c
> @@ -0,0 +1,167 @@
> +/*
> + * nwfilter_ipaddrmap.c: IP address map for mapping interfaces to their
> + *                       detected/expected IP addresses
> + *
> + * Copyright (C) 2010, 2012 IBM Corp.
> + *
> + * Author:
> + *     Stefan Berger <stefanb linux vnet ibm com>
> + *
> + * 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
> + */
> +
> +#include <config.h>
> +
> +#include "internal.h"
> +
> +#include "virterror_internal.h"
> +#include "datatypes.h"
> +#include "nwfilter_params.h"
> +#include "nwfilter_ipaddrmap.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +static virMutex ipAddressMapLock;
> +static virNWFilterHashTablePtr ipAddressMap;
> +
> +
> +/* Add an IP address to the list of IP addresses an interface is
> + * known to use. This function feeds the per-interface cache that
> + * is used to instantiate filters with variable '$IP'.
> + *
> + * @ifname: The name of the (tap) interface
> + * @addr: An IPv4 address in dotted decimal format that the (tap)
> + *        interface is known to use.
> + *
> + * This function returns 0 on success, -1 otherwise
> + */
> +int
> +virNWFilterIPAddrMapAddIPAddr(const char *ifname, char *addr)
> +{
> +    int ret = -1;
> +    virNWFilterVarValuePtr val;
> +
> +    virMutexLock(&ipAddressMapLock);
> +
> +    val = virHashLookup(ipAddressMap->hashTable, ifname);
> +    if (!val) {
> +        val = virNWFilterVarValueCreateSimple(addr);
> +        if (!val) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +        ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1);
> +        goto cleanup;
> +    } else {
> +        if (virNWFilterVarValueAddValue(val, addr) < 0)
> +            goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    virMutexUnlock(&ipAddressMapLock);
> +
> +    return ret;
> +}
> +
> +/* Delete all or a specific IP address from an interface. After this
> + * call either all or the given IP address will not be associated
> + * with the interface anymore.
> + *
> + * @ifname: The name of the (tap) interface
> + * @addr: An IPv4 address in dotted decimal format that the (tap)
> + *        interface is not using anymore; provide NULL to remove all IP
> + *        addresses associated with the given interface
> + *
> + * This function returns the number of IP addresses that are still
> + * known to be associated with this interface, in case of an error
> + * -1 is returned. Error conditions are:
> + * - IP addresses is not known to be associated with the interface
> + */
> +int
> +virNWFilterIPAddrMapDelIPAddr(const char *ifname, const char *ipaddr)
> +{
> +    int ret = -1;
> +    virNWFilterVarValuePtr val = NULL;
> +
> +    virMutexLock(&ipAddressMapLock);
> +
> +    if (ipaddr != NULL) {
> +        val = virHashLookup(ipAddressMap->hashTable, ifname);
> +        if (val) {
> +            if (virNWFilterVarValueGetCardinality(val) == 1 &&
> +                STREQ(ipaddr,
> +                      virNWFilterVarValueGetNthValue(val, 0)))
> +                goto remove_entry;
> +            virNWFilterVarValueDelValue(val, ipaddr);
> +            ret = virNWFilterVarValueGetCardinality(val);
> +        }
> +    } else {
> +remove_entry:
> +        /* remove whole entry */
> +        val = virNWFilterHashTableRemoveEntry(ipAddressMap, ifname);
> +        virNWFilterVarValueFree(val);
> +        ret = 0;
> +    }
> +
> +    virMutexUnlock(&ipAddressMapLock);
> +
> +    return ret;
> +}
> +
> +/* Get the list of IP addresses known to be in use by an interface
> + *
> + * This function returns NULL in case no IP address is known to be
> + * associated with the interface, a virNWFilterVarValuePtr otherwise
> + * that then can contain one or multiple entries.
> + */
> +virNWFilterVarValuePtr
> +virNWFilterIPAddrMapGetIPAddr(const char *ifname)
> +{
> +    virNWFilterVarValuePtr res;
> +
> +    virMutexLock(&ipAddressMapLock);
> +
> +    res = virHashLookup(ipAddressMap->hashTable, ifname);
> +
> +    virMutexUnlock(&ipAddressMapLock);
> +
> +    return res;
> +}
> +
> +int
> +virNWFilterIPAddrMapInit(void)
> +{
> +    ipAddressMap = virNWFilterHashTableCreate(0);
> +    if (!ipAddressMap) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    if (virMutexInit(&ipAddressMapLock) < 0) {
> +        virNWFilterIPAddrMapShutdown();
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +void
> +virNWFilterIPAddrMapShutdown(void)
> +{
> +    virNWFilterHashTableFree(ipAddressMap);
> +    ipAddressMap = NULL;
> +}
> Index: libvirt-acl/src/conf/nwfilter_ipaddrmap.h
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/conf/nwfilter_ipaddrmap.h
> @@ -0,0 +1,37 @@
> +/*
> + * nwfilter_ipaddrmap.h: IP address map for mapping interfaces to their
> + *                       detected/expected IP addresses
> + *
> + * Copyright (C) 2010, 2012 IBM Corp.
> + *
> + * Author:
> + *     Stefan Berger <stefanb linux vnet ibm com>
> + *
> + * 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
> + *
> + */
> +
> +#ifndef __VIR_NWFILTER_IPADDRMAP_H
> +# define __VIR_NWFILTER_IPADDRMAP_H
> +
> +int virNWFilterIPAddrMapInit(void);
> +void virNWFilterIPAddrMapShutdown(void);
> +
> +int virNWFilterIPAddrMapAddIPAddr(const char *ifname, char *addr);
> +int virNWFilterIPAddrMapDelIPAddr(const char *ifname,
> +                                  const char *ipaddr);
> +virNWFilterVarValuePtr virNWFilterIPAddrMapGetIPAddr(const char *ifname);
> +
> +#endif /* __VIR_NWFILTER_IPADDRMAP_H */
> 

  ACK, but double check the syms, thanks,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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