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@xxxxxxxxxxxxxxxxxx>
> + *
> + * 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@xxxxxxxxxxxxxxxxxx>
> + *
> + * 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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list