[libvirt] [PATCH V11 4/7] nwfilter: add DHCP snooping

Daniel Veillard veillard at redhat.com
Thu Apr 19 09:54:31 UTC 2012


On Tue, Apr 17, 2012 at 10:44:05AM -0400, Stefan Berger wrote:
> This patch adds DHCP snooping support to libvirt. The learning method for
> IP addresses is specified by setting the "ip_learning" variable to one of
> "any" [default] (existing IP learning code), "none" (static only addresses)
> or "dhcp" (DHCP snooping).
> 
> Active leases are saved in a lease file and reloaded on restart or HUP.
> 
> The following interface XML activates and uses the DHCP snooping:
> 
>     <interface type='bridge'>
>       <source bridge='virbr0'/>
>       <filterref filter='clean-traffic'>
>         <parameter name='ip_learning' value='dhcp'/>
>       </filterref>
>     </interface>
> 
> All filters containig the variable 'IP' are automatically adjusted when
> the VM receives an IP address via DHCP. However, multiple IP addresses per
> interface are silently ignored in this patch, thus only supporting one IP
> address per interface. Multiple IP address support is added in a later
> patch in this series.
> 
> Signed-off-by: David L Stevens <dlstevens at us.ibm.com>
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> ---
> 
> Changes since v10:
> - using 2 pcap captures now, one for incoming and one for outgoing traffic
>   to prevent libvirt from picking up spoofed DHCP packets and adding bogus
>   leases
> - more cleanups on variable names
> 
> Changes since v9:
> - introduced usage of thread pool for having a worker that evaluates the
>   received DHCP packets and have it instantiate the new filters; it does
>   more time consuming work while the 'normal' thread tries to maximies
>   its time in libpcap packet capture function
> - added rate limitation for evaluation of DCHP packets to prevent
>   flooding the worker thread
> - thread startup synchronization to avoid missing packets on a PXE-booting
>   VM that may send DHCP packets early
> - more work on documentation
> - introducing #defines for reserved variable names
> 
> Changes since v8:
> - naming consistency
> - memory leaks
> - if-before-free not being necessary
> - separate shutdown function (for avoiding a valgrind reporting memory leak)
> - a compilation error ("%d", strlen())
> - pass NULL for a pointer rather than 0
> - use common exits where possible
> - make 'make syntax-check' pass
> - due to a locking bug resulting in a deadlock reworked the locking and
>   introduced a reference counter for the SnoopReq that must be held while
>   the 'req' variable is accessed; it resulred in finer grained locking with
>   the virNWFilterSnoopLock()
> - improved on the documentation
> 
> Changes since v7:
> - renamed functions as suggested
> - collected local state into "virNWFilterSnoopState" struct
> - cleaned up include file list
> - misc code cleanups per review comments
> 
> ---
>  docs/formatnwfilter.html.in            |  143 +-
>  src/Makefile.am                        |    2 
>  src/conf/nwfilter_params.h             |    5 
>  src/nwfilter/nwfilter_dhcpsnoop.c      | 2025 +++++++++++++++++++++++++++++++++
>  src/nwfilter/nwfilter_dhcpsnoop.h      |   39 
>  src/nwfilter/nwfilter_driver.c         |    6 
>  src/nwfilter/nwfilter_gentech_driver.c |   63 -
>  7 files changed, 2238 insertions(+), 45 deletions(-)
>  create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c
>  create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h
> 
> Index: libvirt-acl/docs/formatnwfilter.html.in
> ===================================================================
> --- libvirt-acl.orig/docs/formatnwfilter.html.in
> +++ libvirt-acl/docs/formatnwfilter.html.in
> @@ -371,6 +371,118 @@
>        Further, the notation of $VARIABLE is short-hand for $VARIABLE[@0]. The
>        former notation always assumes the iterator with Id '0'.
>      <p>
> +
> +    <h3><a name="nwfelemsRulesAdvIPAddrDetection">Automatic IP address detection</a></h3>
> +    <p>
> +       The detection of IP addresses used on a virtual machine's interface
> +       is automatically activated if the variable <code>IP</code> is referenced
> +       but not value has been assign to it.
> +       <span class="since">Since 0.9.12</span>
> +       the variable <code>ip_learning</code> can be used to specify
> +       the IP address learning method to use. Valid values are <code>any</code>,
> +       <code>dhcp</code>, or <code>none</code>.
> +       <br/><br/>
> +       The value <code>any</code> means that libvirt may use any packet to
> +       determine the address in use by a virtual machine, which is the default
> +       behavior if the variable <code>ip_learning</code> is not set. This method
> +       will only detect a single IP address on an interface.
> +       Once a VM's IP address has been detected, its IP network traffic
> +       will be locked to that address, if for example IP address spoofing
> +       is prevented by one of its filters. In that case the user of the VM
> +       will not be able to change the IP address on the interface inside
> +       the VM, which would be considered IP address spoofing.
> +       When a VM is migrated to another host or resumed after a suspend operation,
> +       the first packet sent by the VM will again determine the IP address it can
> +       use on a particular interface.
> +       <br/><br>
> +       A value of <code>dhcp</code> specifies that libvirt should only honor DHCP
> +       server-assigned addresses with valid leases. This method supports the detection
> +       and usage of multiple IP address per interface.
> +       When a VM is resumed after a suspend operation, still valid IP address leases
> +       are applied to its filters. Otherwise the VM is expected to again use DHCP to obtain new
> +       IP addresses. The migration of a VM to another physical host requires that
> +       the VM again runs the DHCP protocol.
> +       <br/><br/>
> +       Use of <code>ip_learning=dhcp</code> (DHCP snooping) provides additional
> +       anti-spoofing security, especially when combined with a filter allowing
> +       only trusted DHCP servers to assign addresses. To enable this, set the
> +       variable <code>DHCPSERVER</code> to the IP address of a valid DHCP server
> +       and provide filters that use this variable to filter incoming DHCP responses.
> +       <br/><br/>
> +       When DHCP snooping is enable and the DHCP lease expires,
> +       the VM will no longer be able to use the IP address until it acquires a
> +       new, valid lease from a DHCP server. If the VM is migrated, it must get
> +       a new valid DHCP lease to use an IP address (e.g., by
> +       bringing the VM interface down and up again).
> +       <br/><br/>
> +       Note that automatic DHCP detection listens to the DHCP traffic
> +       the VM exchanges with the DHCP server of the infrastructure. To avoid
> +       denial-of-service attacks on libvirt, the evaluation of those packets
> +       is rate-limited, meaning that a VM sending an excessive number of DHCP
> +       packets per seconds on an interface will not have all of those packets
> +       evaluated and thus filters may not get adapted. Normal DHCP client
> +       behavior is assumed to send a low number of DHCP packets per second.
> +       Further, it is important to setup approriate filters on all VMs in
> +       the infrastructure to avoid them  being able to send DHCP
> +       packets. Therefore VMs must either be prevented from sending UDP and TCP
> +       traffic from port 67 to port 68 or the <code>DHCPSERVER</code>
> +       variable should be used on all VMs to restrict DHCP server messages to
> +       only be allowed to originate from trusted DHCP servers. At the same
> +       time anti-spoofing prevention must be enabled on all VMs in the subnet.
> +       <br/><br/>
> +       If <code>ip_learning</code> is set to <code>none</code>, libvirt does not do
> +       IP address learning and referencing <code>IP</code> without assigning it an
> +       explicit value is an error.
> +       <br/><br/>
> +       The following XML provides an example for the activation of IP address learning
> +       using the DHCP snooping method:
> +    </p>
> +<pre>
> +    <interface type='bridge'>
> +      <source bridge='virbr0'/>
> +      <filterref filter='clean-traffic'>
> +        <parameter name='ip_learning' value='dhcp'/>
> +      </filterref>
> +    </interface>
> +</pre>
> +
> +    <h3><a name="nwfelemsReservedVars">Reserved Variables</a></h3>
> +    <p>
> +      The following table lists reserved variables in use by libvirt.
> +    </p>
> +      <table class="top_table">
> +       <tr>
> +         <th> Variable Name </th>
> +         <th> Semantics </th>
> +       </tr>
> +       <tr>
> +         <td> MAC </td>
> +         <td> The MAC address of the interface </td>
> +       </tr>
> +       <tr>
> +         <td> IP </td>
> +         <td> The list of IP addresses in use by an interface </td>
> +       </tr>
> +       <tr>
> +         <td> IPV6 </td>
> +         <td> Not currently implemented:
> +              the list of IPV6 addresses in use by an interface </td>
> +       </tr>
> +       <tr>
> +         <td> DHCPSERVER </td>
> +         <td> The list of IP addresses of trusted DHCP servers</td>
> +       </tr>
> +       <tr>
> +         <td> DHCPSERVERV6 </td>
> +         <td> Not currently implemented:
> +              The list of IPv6 addresses of trusted DHCP servers</td>
> +       </tr>
> +       <tr>
> +         <td> ip_learning </td>
> +         <td> The choice of the IP address detection mode </td>
> +       </tr>
> +      </table>
> +
>      <h2><a name="nwfelems">Element and attribute overview</a></h2>
>  
>      <p>
> @@ -1629,6 +1741,7 @@
>       The following sections discuss advanced filter configuration
>       topics.
>      </p>
> +
>      <h4><a name="nwfelemsRulesAdvTracking">Connection tracking</a></h4>
>      <p>
>       The network filtering subsystem (on Linux) makes use of the connection
> @@ -2161,36 +2274,6 @@
>       filtering subsystem.
>      </p>
>  
> -    <h3><a name="nwflimitsIP">IP Address Detection</a></h3>
> -     <p>
> -       In case a network filter references the variable
> -       <i>IP</i> and no variable was defined in any higher layer
> -       references to the filter, IP address detection will automatically
> -       be started when the filter is to be instantiated (VM start, interface
> -       hotplug event). Only IPv4
> -       addresses can be detected and only a single IP address
> -       legitimately in use by a VM on a single interface will be detected.
> -       In case a VM was to use multiple IP address on a single interface
> -       (IP aliasing),
> -       the IP addresses would have to be provided explicitly either
> -       in the network filter itself or as variables used in attributes'
> -       values. These
> -       variables must then be defined in a higher level reference to the filter
> -       and each assigned the value of the IP address that the VM is expected
> -       to be using.
> -       Different IP addresses in use by multiple interfaces of a VM
> -       (one IP address each) will be independently detected.
> -       <br/><br/>
> -       Once a VM's IP address has been detected, its IP network traffic
> -       may be locked to that address, if for example IP address spoofing
> -       is prevented by one of its filters. In that case the user of the VM
> -       will not be able to change the IP address on the interface inside
> -       the VM, which would be considered IP address spoofing.
> -       <br/><br/>
> -       In case a VM is resumed after suspension or migrated, IP address
> -       detection will be restarted.
> -     </p>
> -
>      <h3><a name="nwflimitsmigr">VM Migration</a></h3>
>       <p>
>        VM migration is only supported if the whole filter tree
> Index: libvirt-acl/src/Makefile.am
> ===================================================================
> --- libvirt-acl.orig/src/Makefile.am
> +++ libvirt-acl/src/Makefile.am
> @@ -509,6 +509,8 @@ NWFILTER_DRIVER_SOURCES =					\
>  		nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c	\
>  		nwfilter/nwfilter_gentech_driver.c			\
>  		nwfilter/nwfilter_gentech_driver.h			\
> +		nwfilter/nwfilter_dhcpsnoop.c				\
> +		nwfilter/nwfilter_dhcpsnoop.h				\
>  		nwfilter/nwfilter_ebiptables_driver.c			\
>  		nwfilter/nwfilter_ebiptables_driver.h			\
>  		nwfilter/nwfilter_learnipaddr.c				\
> Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -0,0 +1,2025 @@
> +/*
> + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM
> + *                       on an interface
> + *
> + * Copyright (C) 2011,2012 IBM Corp.
> + *
> + * Authors:
> + *    David L Stevens <dlstevens at us.ibm.com>
> + *    Stefan Berger <stefanb at 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
> + *
> + * Based in part on work by Stefan Berger <stefanb at us.ibm.com>
> + */
> +
> +/*
> + * Note about testing:
> + *   On the host run in a shell:
> + *      while :; do kill -SIGHUP `pidof libvirtd` ; echo "HUP $RANDOM"; sleep 20; done
> + *
> + *   Inside a couple of VMs that for example use the 'clean-traffic' filter:
> + *      while :; do kill -SIGTERM `pidof dhclient`; dhclient eth0 ; ifconfig eth0; done
> + *
> + *   On the host check the lease file and that it's periodically shortened:
> + *      cat /var/run/libvirt/network/nwfilter.leases ; date +%s
> + *
> + *   On the host also check that the ebtables rules 'look' ok:
> + *      ebtables -t nat -L
> + */
> +#include <config.h>
> +
> +#ifdef HAVE_LIBPCAP
> +#include <pcap.h>
> +#endif
> +
> +#include <fcntl.h>
> +
> +#include <arpa/inet.h>
> +#include <netinet/ip.h>
> +#include <netinet/udp.h>
> +#include <net/if.h>
> +
> +#include "memory.h"
> +#include "logging.h"
> +#include "datatypes.h"
> +#include "virterror_internal.h"
> +#include "conf/domain_conf.h"
> +#include "nwfilter_gentech_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
> +#include "virnetdev.h"
> +#include "virfile.h"
> +#include "viratomic.h"
> +#include "threadpool.h"
> +#include "configmake.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +#ifdef HAVE_LIBPCAP
> +
> +#define LEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.leases"
> +#define TMPLEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.ltmp"
> +
> +struct virNWFilterSnoopState {
> +    /* lease file */
> +    int                  leaseFD;
> +    virAtomicInt         nLeases; /* number of active leases */
> +    virAtomicInt         wLeases; /* number of written leases */
> +    virAtomicInt         nThreads; /* number of running threads */
> +    /* thread management */
> +    virHashTablePtr      snoopReqs;
> +    virHashTablePtr      ifnameToKey;
> +    virMutex             snoopLock;  /* protects SnoopReqs and IfNameToKey */
> +    virHashTablePtr      active;
> +    virMutex             activeLock; /* protects Active */
> +};
> +
> +#define virNWFilterSnoopLock() \
> +    { virMutexLock(&virNWFilterSnoopState.snoopLock); }
> +#define virNWFilterSnoopUnlock() \
> +    { virMutexUnlock(&virNWFilterSnoopState.snoopLock); }
> +#define virNWFilterSnoopActiveLock() \
> +    { virMutexLock(&virNWFilterSnoopState.activeLock); }
> +#define virNWFilterSnoopActiveUnlock() \
> +    { virMutexUnlock(&virNWFilterSnoopState.activeLock); }
> +
> +#define VIR_IFKEY_LEN   ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN))
> +
> +typedef struct _virNWFilterSnoopReq virNWFilterSnoopReq;
> +typedef virNWFilterSnoopReq *virNWFilterSnoopReqPtr;
> +
> +typedef struct _virNWFilterSnoopIPLease virNWFilterSnoopIPLease;
> +typedef virNWFilterSnoopIPLease *virNWFilterSnoopIPLeasePtr;
> +
> +typedef enum {
> +    THREAD_STATUS_NONE,
> +    THREAD_STATUS_OK,
> +    THREAD_STATUS_FAIL,
> +} virNWFilterSnoopThreadStatus;
> +
> +struct _virNWFilterSnoopReq {
> +    /*
> +     * reference counter: while the req is on the
> +     * publicSnoopReqs hash, the refctr may only
> +     * be modified with the SnoopLock held
> +     */
> +    unsigned int                         refctr;
> +
> +    virNWFilterTechDriverPtr             techdriver;
> +    const char                          *ifname;
> +    int                                  ifindex;
> +    const char                          *linkdev;
> +    enum virDomainNetType                nettype;
> +    char                                 ifkey[VIR_IFKEY_LEN];
> +    unsigned char                        macaddr[VIR_MAC_BUFLEN];
> +    const char                          *filtername;
> +    virNWFilterHashTablePtr              vars;
> +    virNWFilterDriverStatePtr            driver;
> +    /* start and end of lease list, ordered by lease time */
> +    virNWFilterSnoopIPLeasePtr           start;
> +    virNWFilterSnoopIPLeasePtr           end;
> +    char                                *threadkey;
> +
> +    virNWFilterSnoopThreadStatus         threadStatus;
> +    virCond                              threadStatusCond;
> +
> +    int                                  jobCompletionStatus;
> +    /* the number of submitted jobs in the worker's queue */
> +    virAtomicInt                         queueSize;
> +    /*
> +     * protect those members that can change while the
> +     * req is on the public SnoopReq hash and
> +     * at least one reference is held:
> +     * - ifname
> +     * - threadkey
> +     * - start
> +     * - end
> +     * - a lease while it is on the list
> +     * - threadStatus
> +     * (for refctr, see above)
> +     */
> +    virMutex                             lock;
> +};
> +
> +/*
> + * Note about lock-order:
> + * 1st: virNWFilterSnoopLock()
> + * 2nd: virNWFilterSnoopReqLock(req)
> + *
> + * Rationale: Former protects the SnoopReqs hash, latter its contents
> + */
> +
> +struct _virNWFilterSnoopIPLease {
> +    uint32_t                   ipAddress;
> +    uint32_t                   ipServer;
> +    virNWFilterSnoopReqPtr     snoopReq;
> +    unsigned int               timeout;
> +    /* timer list */
> +    virNWFilterSnoopIPLeasePtr prev;
> +    virNWFilterSnoopIPLeasePtr next;
> +};
> +
> +
> +typedef unsigned char Eaddr[6];
> +
> +typedef struct _virNWFilterSnoopEthHdr virNWFilterSnoopEthHdr;
> +typedef virNWFilterSnoopEthHdr *virNWFilterSnoopEthHdrPtr;
> +
> +struct _virNWFilterSnoopEthHdr {
> +    Eaddr eh_dst;
> +    Eaddr eh_src;
> +    unsigned short eh_type;
> +    union {
> +        unsigned char eu_data[0];
> +        struct vlan_hdr {
> +            unsigned short ev_flags;
> +            unsigned short ev_type;
> +            unsigned char  ev_data[0];
> +        } eu_vlh;
> +    } eth_un;
> +} ATTRIBUTE_PACKED;
> +
> +#define eh_data eth_un.eu_data
> +#define ehv_data eth_un.eu_vlh.ev_data
> +#define ehv_type eth_un.eu_vlh.ev_type
> +
> +
> +typedef struct _virNWFilterSnoopDHCPHdr virNWFilterSnoopDHCPHdr;
> +typedef virNWFilterSnoopDHCPHdr *virNWFilterSnoopDHCPHdrPtr;
> +
> +struct _virNWFilterSnoopDHCPHdr {
> +    unsigned char  d_op;
> +    unsigned char  d_htype;
> +    unsigned char  d_hlen;
> +    unsigned char  d_hops;
> +    unsigned int   d_xid;
> +    unsigned short d_secs;
> +    unsigned short d_flags;
> +    unsigned int   d_ciaddr;
> +    unsigned int   d_yiaddr;
> +    unsigned int   d_siaddr;
> +    unsigned int   d_giaddr;
> +    unsigned char  d_chaddr[16];
> +    char           d_sname[64];
> +    char           d_file[128];
> +    unsigned char  d_opts[0];
> +};
> +
> +/* DHCP options */
> +
> +#define DHCPO_PAD         0
> +#define DHCPO_LEASE      51     /* lease time in secs */
> +#define DHCPO_MTYPE      53     /* message type */
> +#define DHCPO_END       255     /* end of options */
> +
> +/* DHCP message types */
> +#define DHCPDECLINE     4
> +#define DHCPACK         5
> +#define DHCPRELEASE     7
> +
> +#define PCAP_PBUFSIZE              576 /* >= IP/TCP/DHCP headers */
> +#define PCAP_POLL_INTERVAL_MS      10*1000 /* 10 secs */
> +#define PCAP_READ_MAXERRS          25 /* retries on failing device */
> +
> +typedef struct _virNWFilterDHCPDecodeJob virNWFilterDHCPDecodeJob;
> +typedef virNWFilterDHCPDecodeJob *virNWFilterDHCPDecodeJobPtr;
> +
> +struct _virNWFilterDHCPDecodeJob {
> +    unsigned char packet[PCAP_PBUFSIZE];
> +    int caplen;
> +    bool fromVM;
> +};
> +
> +#define DHCP_PKT_RATE          10 /* pkts/sec */
> +#define DHCP_PKT_BURST         50 /* pkts/sec */
> +#define DHCP_BURST_INTERVAL_S  10 /* sec */
> +
> +#define MAX_QUEUED_JOBS        (DHCP_PKT_BURST + 2 * DHCP_PKT_RATE)
> +
> +typedef struct _virNWFilterSnoopRateLimitConf virNWFilterSnoopRateLimitConf;
> +typedef virNWFilterSnoopRateLimitConf *virNWFilterSnoopRateLimitConfPtr;
> +
> +struct _virNWFilterSnoopRateLimitConf {
> +    time_t prev;
> +    unsigned int pkt_ctr;
> +    time_t burst;
> +    const unsigned int rate;
> +    const unsigned int burst_rate;
> +    const unsigned int burst_interval;
> +};
> +
> +/* local function prototypes */
> +static int virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
> +                                       uint32_t ipaddr, bool update_leasefile);
> +
> +static void virNWFilterSnoopReqLock(virNWFilterSnoopReqPtr req);
> +static void virNWFilterSnoopReqUnlock(virNWFilterSnoopReqPtr req);
> +
> +static void virNWFilterSnoopLeaseFileLoad(void);
> +static void virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr ipl);
> +
> +/* local variables */
> +static struct virNWFilterSnoopState virNWFilterSnoopState = {
> +    .leaseFD = -1,
> +};
> +
> +static const unsigned char dhcp_magic[4] = { 99, 130, 83, 99 };
> +
> +
> +static char *
> +virNWFilterSnoopActivate(virThreadPtr thread)
> +{
> +    char *key;
> +    unsigned long threadID = (unsigned long int)thread->thread;
> +
> +    if (virAsprintf(&key, "0x%0*lX", (int)sizeof(threadID)*2, threadID) < 0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    virNWFilterSnoopActiveLock();
> +
> +    if (virHashAddEntry(virNWFilterSnoopState.active, key, (void *)0x1) < 0) {
> +        VIR_FREE(key);
> +    }
> +
> +    virNWFilterSnoopActiveUnlock();
> +
> +    return key;
> +}
> +
> +static void
> +virNWFilterSnoopCancel(char **threadKey)
> +{
> +    if (*threadKey == NULL)
> +        return;
> +
> +    virNWFilterSnoopActiveLock();
> +
> +    (void) virHashRemoveEntry(virNWFilterSnoopState.active, *threadKey);
> +    VIR_FREE(*threadKey);
> +
> +    virNWFilterSnoopActiveUnlock();
> +}
> +
> +static bool
> +virNWFilterSnoopIsActive(char *threadKey)
> +{
> +    void *entry;
> +
> +    if (threadKey == NULL)
> +        return 0;
> +
> +    virNWFilterSnoopActiveLock();
> +
> +    entry = virHashLookup(virNWFilterSnoopState.active, threadKey);
> +
> +    virNWFilterSnoopActiveUnlock();
> +
> +    return entry != NULL;
> +}
> +
> +/*
> + * virNWFilterSnoopListAdd - add an IP lease to a list
> + */
> +static void
> +virNWFilterSnoopListAdd(virNWFilterSnoopIPLeasePtr plnew,
> +                        virNWFilterSnoopIPLeasePtr *start,
> +                        virNWFilterSnoopIPLeasePtr *end)
> +{
> +    virNWFilterSnoopIPLeasePtr pl;
> +
> +    plnew->next = plnew->prev = 0;
> +
> +    if (!*start) {
> +        *start = *end = plnew;
> +        return;
> +    }
> +
> +    for (pl = *end; pl && plnew->timeout < pl->timeout;
> +         pl = pl->prev)
> +        /* empty */ ;
> +
> +    if (!pl) {
> +        plnew->next = *start;
> +        *start = plnew;
> +    } else {
> +        plnew->next = pl->next;
> +        pl->next = plnew;
> +    }
> +
> +    plnew->prev = pl;
> +
> +    if (plnew->next)
> +        plnew->next->prev = plnew;
> +    else
> +        *end = plnew;
> +}
> +
> +/*
> + * virNWFilterSnoopListDel - remove an IP lease from a list
> + */
> +static void
> +virNWFilterSnoopListDel(virNWFilterSnoopIPLeasePtr ipl,
> +                        virNWFilterSnoopIPLeasePtr *start,
> +                        virNWFilterSnoopIPLeasePtr *end)
> +{
> +    if (ipl->prev)
> +        ipl->prev->next = ipl->next;
> +    else
> +        *start = ipl->next;
> +
> +    if (ipl->next)
> +        ipl->next->prev = ipl->prev;
> +    else
> +        *end = ipl->prev;
> +
> +    ipl->next = ipl->prev = 0;

 Please use NULL to set pointers to zero

> +}
> +
> +/*
> + * virNWFilterSnoopLeaseTimerAdd - add an IP lease to the timer list
> + */
> +static void
> +virNWFilterSnoopIPLeaseTimerAdd(virNWFilterSnoopIPLeasePtr plnew)
> +{
> +    virNWFilterSnoopReqPtr req = plnew->snoopReq;
> +
> +    /* protect req->start / req->end */
> +    virNWFilterSnoopReqLock(req);
> +
> +    virNWFilterSnoopListAdd(plnew, &req->start, &req->end);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +}
> +
> +/*
> + * virNWFilterSnoopLeaseTimerDel - remove an IP lease from the timer list
> + */
> +static void
> +virNWFilterSnoopIPLeaseTimerDel(virNWFilterSnoopIPLeasePtr ipl)
> +{
> +    virNWFilterSnoopReqPtr req = ipl->snoopReq;
> +
> +    /* protect req->start / req->end */
> +    virNWFilterSnoopReqLock(req);
> +
> +    virNWFilterSnoopListDel(ipl, &req->start, &req->end);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    ipl->timeout = 0;
> +}
> +
> +/*
> + * virNWFilterSnoopInstallRule - install rule for a lease
> + *
> + * @instantiate: when calling this function in a loop, indicate
> + *               the last call with 'true' here so that the
> + *               rules all get instantiated
> + *               Always calling this with 'true' is fine, but less
> + *               efficient.
> + */
> +static int
> +virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
> +                                   bool instantiate)
> +{
> +    char ipbuf[INET_ADDRSTRLEN];
> +    int rc = -1;
> +    virNWFilterVarValuePtr ipVar;
> +    virNWFilterSnoopReqPtr req;
> +
> +    if (!inet_ntop(AF_INET, &ipl->ipAddress, ipbuf, sizeof(ipbuf))) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("%s: inet_ntop failed "
> +                                 " (0x%08X)"),
> +                               __func__, ipl->ipAddress);
> +        return -1;
> +    }
> +    ipVar = virNWFilterVarValueCreateSimpleCopyValue(ipbuf);
> +    if (!ipVar) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (virNWFilterHashTablePut(ipl->snoopReq->vars, NWFILTER_VARNAME_IP,
> +                                ipVar, 1) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Could not add variable \""
> +                                 NWFILTER_VARNAME_IP "\" to hashmap"));
> +        virNWFilterVarValueFree(ipVar);
> +        return -1;
> +    }
> +
> +    if (!instantiate)
> +        return 0;
> +
> +    /* instantiate the filters */
> +    req = ipl->snoopReq;
> +
> +    /* protect req->ifname */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (req->ifname)
> +        rc = virNWFilterInstantiateFilterLate(NULL,
> +                                              req->ifname,
> +                                              req->ifindex,
> +                                              req->linkdev,
> +                                              req->nettype,
> +                                              req->macaddr,
> +                                              req->filtername,
> +                                              req->vars,
> +                                              req->driver);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    return rc;
> +}
> +
> +/*
> + * virNWFilterSnoopIPLeaseUpdate - update the timeout on an IP lease
> + */
> +static void
> +virNWFilterSnoopIPLeaseUpdate(virNWFilterSnoopIPLeasePtr ipl, time_t timeout)
> +{
> +    if (timeout < ipl->timeout)
> +        return;  /* no take-backs */
> +
> +    virNWFilterSnoopIPLeaseTimerDel(ipl);
> +    ipl->timeout = timeout;
> +    virNWFilterSnoopIPLeaseTimerAdd(ipl);
> +}
> +
> +/*
> + * virNWFilterSnoopGetByIP - lookup IP lease by IP address
> + */
> +static virNWFilterSnoopIPLeasePtr
> +virNWFilterSnoopIPLeaseGetByIP(virNWFilterSnoopIPLeasePtr start,
> +                               uint32_t ipaddr)
> +{
> +    virNWFilterSnoopIPLeasePtr pl;
> +
> +    for (pl = start; pl && pl->ipAddress != ipaddr; pl = pl->next)
> +        /* empty */ ;
> +    return pl;
> +}
> +
> +/*
> + * virNWFilterSnoopReqLeaseTimerRun - run the IP lease timeout list
> + */
> +static unsigned int
> +virNWFilterSnoopReqLeaseTimerRun(virNWFilterSnoopReqPtr req)
> +{
> +    time_t now = time(0);
> +
> +    /* protect req->start */
> +    virNWFilterSnoopReqLock(req);
> +
> +    while (req->start && req->start->timeout <= now)
> +        virNWFilterSnoopReqLeaseDel(req, req->start->ipAddress, true);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    return 0;
> +}
> +
> +/*
> + * Get a reference to the given Snoop request
> + */
> +static void
> +virNWFilterSnoopReqGet(virNWFilterSnoopReqPtr req)
> +{
> +    req->refctr++;

  Sounds racy I would lock here, unless this is expected to be called
  while already locked (in which case should be documented, but why
  make a function for it then ?)

> +}
> +
> +/*
> + * Create a new Snoop request. Initialize it with the given
> + * interface key. The caller must release the request with a call
> + * to virNWFilerSnoopReqPut(req).
> + */
> +static virNWFilterSnoopReqPtr
> +virNWFilterSnoopReqNew(const char *ifkey)
> +{
> +    virNWFilterSnoopReqPtr req;
> +
> +    if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN - 1) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterSnoopReqNew called with invalid "
> +                                 "key \"%s\" (%u)"),
> +                               ifkey ? ifkey : "",
> +                               (unsigned int)strlen(ifkey));
> +        return NULL;
> +    }
> +
> +    if (VIR_ALLOC(req) < 0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +
> +    virNWFilterSnoopReqGet(req);
> +
> +    req->threadStatus = THREAD_STATUS_NONE;
> +
> +    if (virMutexInitRecursive(&req->lock) < 0 ||
> +        virStrcpyStatic(req->ifkey, ifkey) == NULL ||
> +        virCondInit(&req->threadStatusCond) < 0 ||
> +        virAtomicIntInit(&req->queueSize) < 0)
> +        VIR_FREE(req);
> +
> +    return req;
> +}
> +
> +/*
> + * Free a snoop request unless it is still referenced.
> + * All its associated leases are also freed.
> + * The lease file is NOT rewritten.
> + */
> +static void
> +virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req)
> +{
> +    virNWFilterSnoopIPLeasePtr ipl;
> +
> +    if (!req)
> +        return;
> +
> +    if (req->refctr != 0)
> +        return;
> +
> +    /* free all leases */
> +    for (ipl = req->start; ipl; ipl = req->start)
> +        virNWFilterSnoopReqLeaseDel(req, ipl->ipAddress, false);
> +
> +    /* free all req data */
> +    VIR_FREE(req->ifname);
> +    VIR_FREE(req->linkdev);
> +    VIR_FREE(req->filtername);
> +    virNWFilterHashTableFree(req->vars);
> +
> +    VIR_FREE(req);
> +}
> +
> +/*
> + * Lock a Snoop request 'req'
> + */
> +static void
> +virNWFilterSnoopReqLock(virNWFilterSnoopReqPtr req)
> +{
> +    virMutexLock(&req->lock);
> +}
> +
> +/*
> + * Unlock a Snoop request 'req'
> + */
> +static void
> +virNWFilterSnoopReqUnlock(virNWFilterSnoopReqPtr req)
> +{
> +    virMutexUnlock(&req->lock);
> +}
> +
> +/*
> + * virNWFilterSnoopReqRelease - hash table free function to kill a request
> + */
> +static void
> +virNWFilterSnoopReqRelease(void *req0, const void *name ATTRIBUTE_UNUSED)
> +{
> +    virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr) req0;
> +
> +    if (!req)
> +        return;
> +
> +    /* protect req->threadkey */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (req->threadkey)
> +        virNWFilterSnoopCancel(&req->threadkey);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    virNWFilterSnoopReqFree(req);
> +}
> +
> +/*
> + * virNWFilterSnoopReqGetByIFKey
> + *
> + * Get a Snoop request given an interface key; caller must release
> + * the Snoop request with a call to virNWFilterSnoopReqPut()
> + */
> +static virNWFilterSnoopReqPtr
> +virNWFilterSnoopReqGetByIFKey(const char *ifkey)
> +{
> +    virNWFilterSnoopReqPtr req;
> +
> +    virNWFilterSnoopLock();
> +
> +    req = virHashLookup(virNWFilterSnoopState.snoopReqs, ifkey);
> +    if (req)
> +        virNWFilterSnoopReqGet(req);
> +
> +    virNWFilterSnoopUnlock();
> +
> +    return req;
> +}
> +
> +/*
> + * Drop the reference to the Snoop request. Don't use the req
> + * after this call.
> + */
> +static void
> +virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req)
> +{
> +    if (!req)
> +        return;
> +
> +    virNWFilterSnoopLock()
> +
> +    req->refctr--;
> +
> +    if (req->refctr == 0) {
> +        /*
> +         * delete the request:
> +         * - if we don't find req on the global list anymore
> +         *   (this happens during SIGHUP)
> +         * we would keep the request:
> +         * - if we still have a valid lease, keep the req for restarts
> +         */
> +        if (virHashLookup(virNWFilterSnoopState.snoopReqs, req->ifkey) != req) {
> +            virNWFilterSnoopReqRelease(req, NULL);
> +        } else if (!req->start || req->start->timeout < time(0)) {
> +            (void) virHashRemoveEntry(virNWFilterSnoopState.snoopReqs,
> +                                      req->ifkey);
> +        }
> +    }
> +
> +    virNWFilterSnoopUnlock();
> +}
> +
> +/*
> + * virNWFilterSnoopReqLeaseAdd - create or update an IP lease
> + */
> +static int
> +virNWFilterSnoopReqLeaseAdd(virNWFilterSnoopReqPtr req,
> +                            virNWFilterSnoopIPLeasePtr plnew,
> +                            bool update_leasefile)
> +{
> +    virNWFilterSnoopIPLeasePtr pl;
> +
> +    plnew->snoopReq = req;
> +
> +    /* protect req->start and the lease */
> +    virNWFilterSnoopReqLock(req);
> +
> +    pl = virNWFilterSnoopIPLeaseGetByIP(req->start, plnew->ipAddress);
> +
> +    if (pl) {
> +        virNWFilterSnoopIPLeaseUpdate(pl, plnew->timeout);
> +
> +        virNWFilterSnoopReqUnlock(req);
> +
> +        goto exit;
> +    }
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    /* support for multiple addresses requires the ability to add filters
> +     * to existing chains, or to instantiate address lists via
> +     * virNWFilterInstantiateFilterLate(). Until one of those capabilities
> +     * is added, don't allow a new address when one is already assigned to
> +     * this interface.
> +     */
> +    if (req->start)
> +         return 0;    /* silently ignore multiple addresses */
> +
> +    if (VIR_ALLOC(pl) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    *pl = *plnew;
> +
> +    /* protect req->threadkey */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (req->threadkey && virNWFilterSnoopIPLeaseInstallRule(pl, true) < 0) {
> +        virNWFilterSnoopReqUnlock(req);
> +        VIR_FREE(pl);
> +        return -1;
> +    }
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    /* put the lease on the req's list */
> +    virNWFilterSnoopIPLeaseTimerAdd(pl);
> +
> +    virAtomicIntAdd(&virNWFilterSnoopState.nLeases, 1);
> +
> +exit:
> +    if (update_leasefile)
> +        virNWFilterSnoopLeaseFileSave(pl);
> +
> +    return 0;
> +}
> +
> +/*
> + * Restore a Snoop request -- walk its list of leases
> + * and re-build the filtering rules with them
> + */
> +static int
> +virNWFilterSnoopReqRestore(virNWFilterSnoopReqPtr req)
> +{
> +    int ret = 0;
> +    virNWFilterSnoopIPLeasePtr ipl;
> +
> +    /* protect req->start */
> +    virNWFilterSnoopReqLock(req);
> +
> +    for (ipl = req->start; ipl; ipl = ipl->next) {
> +        /* instantiate the rules at the last lease */
> +        bool is_last = (ipl->next == NULL);
> +        if (virNWFilterSnoopIPLeaseInstallRule(ipl, is_last) < 0) {
> +            ret = -1;
> +            break;
> +        }
> +    }
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    return ret;
> +}
> +
> +/*
> + * virNWFilterSnoopReqLeaseDel - delete an IP lease
> + *
> + * @update_leasefile: set to 'true' if the lease expired or the lease
> + *                    was returned to the DHCP server and therefore
> + *                    this has to be noted in the lease file.
> + *                    set to 'false' for any other reason such as for
> + *                    example when calling only to free the lease's
> + *                    memory or when calling this function while reading
> + *                    leases from the file.
> + *
> + * Returns 0 on success, -1 if the instantiation of the rules failed
> + */
> +static int
> +virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
> +                            uint32_t ipaddr, bool update_leasefile)
> +{
> +    int ret = 0;
> +    virNWFilterSnoopIPLeasePtr ipl;
> +
> +    /* protect req->start & req->ifname */
> +    virNWFilterSnoopReqLock(req);
> +
> +    ipl = virNWFilterSnoopIPLeaseGetByIP(req->start, ipaddr);
> +    if (ipl == NULL)
> +        goto lease_not_found;
> +
> +    virNWFilterSnoopIPLeaseTimerDel(ipl);
> +
> +    if (update_leasefile) {
> +        const virNWFilterVarValuePtr dhcpsrvrs =
> +            virHashLookup(req->vars->hashTable, NWFILTER_VARNAME_DHCPSERVER);
> +
> +        virNWFilterSnoopLeaseFileSave(ipl);
> +
> +        /*
> +         * for multiple address support, this needs to remove those rules
> +         * referencing "IP" with ipl's ip value.
> +         */
> +        if (req->techdriver &&
> +            req->techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr,
> +                                                dhcpsrvrs, false) < 0) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("virNWFilterSnoopListDel failed"));
> +            ret = -1;
> +        }
> +
> +    }
> +    VIR_FREE(ipl);
> +
> +    virAtomicIntSub(&virNWFilterSnoopState.nLeases, 1);
> +
> +lease_not_found:
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    return ret;
> +}
> +
> +static int
> +virNWFilterSnoopDHCPGetOpt(virNWFilterSnoopDHCPHdrPtr pd, int len,
> +                           int *pmtype, int *pleasetime)
> +{
> +    int oind, olen;
> +    int oend;
> +
> +    olen = len - sizeof(*pd);
> +    oind = 0;
> +
> +    if (olen < 4)               /* bad magic */
> +        return -1;
> +
> +    if (memcmp(dhcp_magic, pd->d_opts, sizeof(dhcp_magic)) != 0)
> +        return -1;              /* bad magic */
> +
> +    oind += sizeof(dhcp_magic);
> +
> +    oend = 0;
> +
> +    *pmtype = *pleasetime = 0;
> +
> +    while (oind < olen) {
> +        switch (pd->d_opts[oind]) {
> +            case DHCPO_LEASE:
> +                if (olen - oind < 6)
> +                    goto malformed;
> +                if (*pleasetime)
> +                    return -1;  /* duplicate lease time */
> +                *pleasetime =
> +                    ntohl(*(unsigned int *) (pd->d_opts + oind + 2));
> +                break;
> +            case DHCPO_MTYPE:
> +                if (olen - oind < 3)
> +                    goto malformed;
> +                if (*pmtype)
> +                    return -1;  /* duplicate message type */
> +                *pmtype = pd->d_opts[oind + 2];
> +                break;
> +            case DHCPO_PAD:
> +                oind++;
> +                continue;
> +
> +            case DHCPO_END:
> +                oend = 1;
> +                break;
> +            default:
> +                if (olen - oind < 2)
> +                    goto malformed;
> +        }
> +        if (oend)
> +            break;
> +        oind += pd->d_opts[oind + 1] + 2;
> +    }
> +    return 0;
> +malformed:
> +    VIR_WARN("got lost in the options!");
> +    return -1;
> +}
> +
> +/*
> + * Decode the DHCP options
> + *
> + * Returns 0 in case of full success.
> + * Returns -2 in case of some error with the packet.
> + * Returns -1 in case of error with the installation of rules
> + */
> +static int
> +virNWFilterSnoopDHCPDecode(virNWFilterSnoopReqPtr req,
> +                           virNWFilterSnoopEthHdrPtr pep,
> +                           int len, bool fromVM)
> +{
> +    struct iphdr *pip;
> +    struct udphdr *pup;
> +    virNWFilterSnoopDHCPHdrPtr pd;
> +    virNWFilterSnoopIPLease ipl;
> +    int mtype, leasetime;
> +
> +    /* go through the protocol headers */
> +    switch (ntohs(pep->eh_type)) {
> +    case ETHERTYPE_IP:
> +        pip = (struct iphdr *) pep->eh_data;
> +        len -= offsetof(virNWFilterSnoopEthHdr, eh_data);
> +        break;
> +    case ETHERTYPE_VLAN:
> +        if (ntohs(pep->ehv_type) != ETHERTYPE_IP)
> +            return -2;
> +        pip = (struct iphdr *) pep->ehv_data;
> +        len -= offsetof(virNWFilterSnoopEthHdr, ehv_data);
> +        break;
> +    default:
> +        return -2;
> +    }
> +
> +    if (len < 0)
> +        return -2;
> +
> +    pip = (struct iphdr *) pep->eh_data;
> +    len -= sizeof(*pep);
> +    if (len < 0)
> +        return -2;
> +
> +    /* check for spoofed or packets not destined for this VM */
> +    if (fromVM) {
> +        if (memcmp(req->macaddr, pep->eh_src, 6) != 0)
> +            return -2;
> +    } else {
> +        /*
> +         * packets not destined for us can occurr while the bridge is
> +         * learning the MAC addresses on ports
> +         */
> +        if (memcmp(req->macaddr, pep->eh_dst, 6) != 0)
> +            return -2;
> +    }
> +
> +    pup = (struct udphdr *) ((char *) pip + (pip->ihl << 2));
> +    len -= pip->ihl << 2;
> +    if (len < 0)
> +        return -2;
> +
> +    pd = (virNWFilterSnoopDHCPHdrPtr) ((char *) pup + sizeof(*pup));
> +    len -= sizeof(*pup);
> +    if (len < 0)
> +        return -2;                 /* invalid packet length */
> +
> +    if (virNWFilterSnoopDHCPGetOpt(pd, len, &mtype, &leasetime) < 0)
> +        return -2;
> +
> +    memset(&ipl, 0, sizeof(ipl));
> +    ipl.ipAddress = pd->d_yiaddr;
> +    ipl.ipServer = pd->d_siaddr;
> +
> +    if (leasetime == ~0)
> +        ipl.timeout = ~0;
> +    else
> +        ipl.timeout = time(0) + leasetime;
> +
> +    ipl.snoopReq = req;
> +
> +    /* check that the type of message comes from the right direction */
> +    switch (mtype) {
> +        case DHCPACK:
> +        case DHCPDECLINE:
> +            if (fromVM)
> +                return -2;
> +            break;
> +        case DHCPRELEASE:
> +            if (!fromVM)
> +                return -2;
> +            break;
> +        default:
> +            break;
> +    }
> +
> +    switch (mtype) {
> +        case DHCPACK:
> +            if (virNWFilterSnoopReqLeaseAdd(req, &ipl, true) < 0)
> +                return -1;
> +            break;
> +        case DHCPDECLINE:
> +        case DHCPRELEASE:
> +            if (virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, true) < 0)
> +                return -1;
> +            break;
> +        default:
> +            return -2;
> +            break;
> +    }
> +    return 0;
> +}
> +
> +static pcap_t *
> +virNWFilterSnoopDHCPOpen(const char *ifname, const char *filter,
> +                         pcap_direction_t dir)
> +{
> +    pcap_t *handle = NULL;
> +    struct bpf_program fp;
> +    char pcap_errbuf[PCAP_ERRBUF_SIZE];
> +
> +    handle = pcap_open_live(ifname, PCAP_PBUFSIZE, 0, PCAP_POLL_INTERVAL_MS,
> +                            pcap_errbuf);
> +
> +    if (handle == NULL) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_open_live: %s"), pcap_errbuf);
> +        goto cleanup;
> +    }
> +
> +    if (pcap_compile(handle, &fp, filter, 1, PCAP_NETMASK_UNKNOWN) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_compile: %s"), pcap_geterr(handle));
> +        goto cleanup;
> +    }
> +
> +    if (pcap_setfilter(handle, &fp) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_setfilter: %s"), pcap_geterr(handle));
> +        goto cleanup_freecode;
> +    }
> +
> +    if (pcap_setdirection(handle, dir) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_setdirection: %s"),
> +                               pcap_geterr(handle));
> +        goto cleanup_freecode;
> +    }
> +
> +    pcap_freecode(&fp);
> +
> +    return handle;
> +
> +cleanup_freecode:
> +    pcap_freecode(&fp);
> +cleanup:
> +    pcap_close(handle);
> +
> +    return NULL;
> +}
> +
> +/*
> + * Worker function to decode the DHCP message and with that
> + * also do the time-consuming work of instantiating the filters
> + */
> +static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque)
> +{
> +    virNWFilterSnoopReqPtr req = opaque;
> +    virNWFilterDHCPDecodeJobPtr job = jobdata;
> +    virNWFilterSnoopEthHdrPtr packet = (virNWFilterSnoopEthHdrPtr)job->packet;
> +
> +    if (virNWFilterSnoopDHCPDecode(req, packet,
> +                                   job->caplen, job->fromVM) == -1) {
> +        req->jobCompletionStatus = -1;
> +
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Instantiation of rules failed on "
> +                                 "interface '%s'"), req->ifname);
> +    }
> +    VIR_FREE(job);
> +    virAtomicIntSub(&req->queueSize, 1);
> +}
> +
> +/*
> + * Submit a job to the worker thread doing the time-consuming work...
> + */
> +static int
> +virNWFilterSnoopDHCPDecodeJobSubmit(virThreadPoolPtr pool,
> +                                    virNWFilterSnoopReqPtr req,
> +                                    virNWFilterSnoopEthHdrPtr pep,
> +                                    int len, pcap_direction_t dir)
> +{
> +    virNWFilterDHCPDecodeJobPtr job;
> +    int ret;
> +
> +    if (len <= 0 || len > sizeof(job->packet))
> +        return 0;
> +
> +    if (VIR_ALLOC(job) < 0) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +
> +    memcpy(job->packet, pep, len);
> +    job->caplen = len;
> +    job->fromVM = (dir == PCAP_D_IN);
> +
> +    ret = virThreadPoolSendJob(pool, 0, job);
> +
> +    if (ret == 0)
> +        virAtomicIntAdd(&req->queueSize, 1);
> +
> +    return ret;
> +}
> +
> +/*
> + * virNWFilterSnoopRateLimit -- limit the rate of jobs submitted to the
> + *                              worker thread
> + *
> + * Help defend the worker thread from being flooded with likely bogus packets
> + * sent by the VM.
> + *
> + * rl: The state of the rate limiter
> + *
> + * Returns the delta of packets compared to the rate, i.e. if the rate
> + * is 4 (pkts/s) and we now have received 5 within a second, it would
> + * return 1. If the number of packets is below the rate, it returns 0.
> + */
> +static int
> +virNWFilterSnoopRateLimit(virNWFilterSnoopRateLimitConfPtr rl)
> +{
> +    time_t now = time(0);
> +    int diff;
> +#define IN_BURST(n,b) ((n)-(b) <= 1) /* bursts span 2 discreet seconds */
> +
> +    if (rl->prev != now && !IN_BURST(now, rl->burst)) {
> +        rl->prev = now;
> +        rl->pkt_ctr = 1;
> +    } else {
> +        rl->pkt_ctr++;
> +        if (rl->pkt_ctr >= rl->rate) {
> +            if (IN_BURST(now, rl->burst)) {
> +                /* in a burst */
> +                diff = rl->pkt_ctr - rl->burst_rate;
> +                if (diff > 0)
> +                    return diff;
> +                return 0;
> +            }
> +            if (rl->prev - rl->burst > rl->burst_interval) {
> +                /* this second will start a new burst */
> +                rl->burst = rl->prev;
> +                return 0;
> +            }
> +            /* previous burst is too close */
> +            return rl->pkt_ctr - rl->rate;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +/*
> + * The DHCP snooping thread. It spends most of its time in the pcap
> + * library and if it gets suitable packets, it submits them to the worker
> + * thread for processing.
> + */
> +static void
> +virNWFilterDHCPSnoopThread(void *req0)
> +{
> +    virNWFilterSnoopReqPtr req = req0;
> +    struct pcap_pkthdr *hdr;
> +    virNWFilterSnoopEthHdrPtr packet;
> +    int ifindex = 0;
> +    int errcount = 0;
> +    int tmp = -1, i, rv, n;
> +    char *threadkey = NULL;
> +    virThreadPoolPtr worker = NULL;
> +    time_t last_displayed = 0, last_displayed_queue = 0;
> +    struct {
> +        pcap_t *handle;
> +        pcap_direction_t dir;
> +        const char *filter;
> +    } pcapConf[] = {
> +        {
> +            .dir = PCAP_D_IN, /* from VM */
> +            .filter = "dst port 67 and src port 68",
> +        }, {
> +            .dir = PCAP_D_OUT, /* to VM */
> +            .filter = "src port 67 and dst port 68",
> +        },
> +    };
> +    struct pollfd fds[] = {
> +        {
> +            /* get a POLLERR if interface goes down or disappears */
> +            .events = POLLIN | POLLERR,
> +        }, {
> +            .events = POLLIN | POLLERR,
> +        },
> +    };
> +    virNWFilterSnoopRateLimitConf rateLimit = {
> +        .prev = time(0),
> +        .pkt_ctr = 0,
> +        .burst = 0,
> +        .rate = DHCP_PKT_RATE,
> +        .burst_rate = DHCP_PKT_BURST,
> +        .burst_interval = DHCP_BURST_INTERVAL_S,
> +    };
> +    bool error = false;
> +
> +    /* whoever started us increased the reference counter for the req for us */
> +
> +    /* protect req->ifname & req->threadkey */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (req->ifname && req->threadkey) {
> +        for (i = 0; i < ARRAY_CARDINALITY(pcapConf); i++) {
> +            pcapConf[i].handle =
> +                virNWFilterSnoopDHCPOpen(req->ifname, pcapConf[i].filter,
> +                                         pcapConf[i].dir);
> +            if (!pcapConf[i].handle) {
> +                error = true;
> +                break;
> +            }
> +            fds[i].fd = pcap_fileno(pcapConf[i].handle);
> +        }
> +        tmp = virNetDevGetIndex(req->ifname, &ifindex);
> +        threadkey = strdup(req->threadkey);
> +        worker = virThreadPoolNew(1, 1, 0,
> +                                  virNWFilterDHCPDecodeWorker,
> +                                  req);
> +    }
> +
> +    /* let creator know how well we initialized */
> +    if (error == true || !threadkey || tmp < 0 || !worker ||
> +        ifindex != req->ifindex)
> +        req->threadStatus = THREAD_STATUS_FAIL;
> +    else
> +        req->threadStatus = THREAD_STATUS_OK;
> +
> +    virCondSignal(&req->threadStatusCond);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    if (req->threadStatus != THREAD_STATUS_OK)
> +        goto exit;
> +
> +    while (!error) {
> +        n = poll(fds, ARRAY_CARDINALITY(fds), 1000 * 10);
> +
> +        if (n < 0)
> +            error = true;
> +
> +        virNWFilterSnoopReqLeaseTimerRun(req);
> +
> +        /*
> +         * Check whether we were cancelled or whether
> +         * a previously submitted job failed.
> +         */
> +        if (!virNWFilterSnoopIsActive(threadkey) ||
> +            req->jobCompletionStatus != 0)
> +            goto exit;
> +
> +        for (i = 0; n > 0 && i < ARRAY_CARDINALITY(fds); i++) {
> +            if (!fds[i].revents)
> +                continue;
> +
> +            fds[i].revents = 0;
> +            n--;
> +
> +            rv = pcap_next_ex(pcapConf[i].handle, &hdr,
> +                              (const u_char **)&packet);
> +
> +            if (rv < 0) {
> +                /* error reading from socket */
> +                tmp = -1;
> +
> +                /* protect req->ifname */
> +                virNWFilterSnoopReqLock(req);
> +
> +                if (req->ifname)
> +                    tmp = virNetDevValidateConfig(req->ifname, NULL, ifindex);
> +
> +                virNWFilterSnoopReqUnlock(req);
> +
> +                if (tmp <= 0) {
> +                    error = true;
> +                    break;
> +                }
> +
> +                if (++errcount > PCAP_READ_MAXERRS) {
> +                    pcap_close(pcapConf[i].handle);
> +                    pcapConf[i].handle = NULL;
> +
> +                    /* protect req->ifname */
> +                    virNWFilterSnoopReqLock(req);
> +
> +                    virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                           _("interface '%s' failing; "
> +                                             "reopening"),
> +                                           req->ifname);
> +                    if (req->ifname)
> +                        pcapConf[i].handle =
> +                            virNWFilterSnoopDHCPOpen(req->ifname,
> +                                                     pcapConf[i].filter,
> +                                                     pcapConf[i].dir);
> +
> +                    virNWFilterSnoopReqUnlock(req);
> +
> +                    if (!pcapConf[i].handle) {
> +                        error = true;
> +                        break;
> +                    }
> +                }
> +                continue;
> +            }
> +
> +            errcount = 0;
> +
> +            if (rv) {
> +                int diff;
> +
> +                /* submit packet to worker thread */
> +                if (virAtomicIntRead(&req->queueSize) > MAX_QUEUED_JOBS) {
> +                    if (last_displayed_queue - time(0) > 10) {
> +                        last_displayed_queue = time(0);
> +                        VIR_WARN("Worker thread for interface '%s' has a "
> +                                 "job queue that is too long\n",
> +                                 req->ifname);
> +                    }
> +                    continue;
> +                }
> +
> +                diff = virNWFilterSnoopRateLimit(&rateLimit);
> +                if (diff > 0) {
> +                    if (diff > DHCP_PKT_RATE) {
> +                        /* twice the allowed rate -- serious flooding */
> +                        usleep(1 * 1000); /* 1 ms */
> +                    }
> +                    /* rate-limited warnings */
> +                    if (time(0) - last_displayed > 10) {
> +                         last_displayed = time(0);
> +                         VIR_WARN("Too many DHCP packets on interface '%s'",
> +                                  req->ifname);
> +                    }
> +                    continue;
> +                }
> +
> +                if (virNWFilterSnoopDHCPDecodeJobSubmit(worker, req, packet,
> +                                                        hdr->caplen,
> +                                                        pcapConf[i].dir) < 0) {
> +                    virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                           _("Job submission failed on "
> +                                           "interface '%s'"), req->ifname);
> +                    error = true;
> +                    break;
> +                }
> +            }
> +        } /* for all fds */
> +    } /* while (!error) */
> +
> +    /* protect IfNameToKey */
> +    virNWFilterSnoopLock();
> +
> +    /* protect req->ifname & req->threadkey */
> +    virNWFilterSnoopReqLock(req);
> +
> +    virNWFilterSnoopCancel(&req->threadkey);
> +
> +    (void) virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, req->ifname);
> +
> +    VIR_FREE(req->ifname);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +    virNWFilterSnoopUnlock();
> +
> +exit:
> +    virThreadPoolFree(worker);
> +
> +    virNWFilterSnoopReqPut(req);
> +
> +    VIR_FREE(threadkey);
> +
> +    for (i = 0; i < ARRAY_CARDINALITY(pcapConf); i++) {
> +        if (pcapConf[i].handle)
> +            pcap_close(pcapConf[i].handle);
> +    }
> +
> +    virAtomicIntSub(&virNWFilterSnoopState.nThreads, 1);
> +
> +    return;
> +}
> +
> +static void
> +virNWFilterSnoopIFKeyFMT(char *ifkey, const unsigned char *vmuuid,
> +                         unsigned const char *macaddr)
> +{
> +    virUUIDFormat(vmuuid, ifkey);
> +    ifkey[VIR_UUID_STRING_BUFLEN - 1] = '-';
> +    virMacAddrFormat(macaddr, ifkey + VIR_UUID_STRING_BUFLEN);
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> +                        const char *ifname,
> +                        const char *linkdev,
> +                        enum virDomainNetType nettype,
> +                        const unsigned char *vmuuid,
> +                        const unsigned char *macaddr,
> +                        const char *filtername,
> +                        virNWFilterHashTablePtr filterparams,
> +                        virNWFilterDriverStatePtr driver)
> +{
> +    virNWFilterSnoopReqPtr req;
> +    bool isnewreq;
> +    char ifkey[VIR_IFKEY_LEN];
> +    int tmp;
> +    virThread thread;
> +    virNWFilterVarValuePtr dhcpsrvrs;
> +
> +    virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr);
> +
> +    req = virNWFilterSnoopReqGetByIFKey(ifkey);
> +    isnewreq = (req == NULL);
> +    if (!isnewreq) {
> +        if (req->threadkey) {
> +            virNWFilterSnoopReqPut(req);
> +            return 0;
> +        }
> +        /* a recycled req may still have filtername and vars */
> +        VIR_FREE(req->filtername);
> +        virNWFilterHashTableFree(req->vars);
> +    } else {
> +        req = virNWFilterSnoopReqNew(ifkey);
> +        if (!req)
> +            return -1;
> +    }
> +
> +    req->driver = driver;
> +    req->techdriver = techdriver;
> +    tmp = virNetDevGetIndex(ifname, &req->ifindex);
> +    req->linkdev = linkdev ? strdup(linkdev) : NULL;
> +    req->nettype = nettype;
> +    req->ifname = strdup(ifname);
> +    memcpy(req->macaddr, macaddr, sizeof(req->macaddr));
> +    req->filtername = strdup(filtername);
> +    req->vars = virNWFilterHashTableCreate(0);
> +
> +    if (!req->ifname || !req->filtername || !req->vars || tmp < 0) {
> +        virReportOOMError();
> +        goto exit_snoopreqput;
> +    }
> +
> +    dhcpsrvrs = virHashLookup(filterparams->hashTable,
> +                              NWFILTER_VARNAME_DHCPSERVER);
> +
> +    if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr,
> +                                       dhcpsrvrs, false) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("applyDHCPOnlyRules "
> +                               "failed - spoofing not protected!"));
> +        goto exit_snoopreqput;
> +    }
> +
> +    if (virNWFilterHashTablePutAll(filterparams, req->vars) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq: can't copy variables"
> +                                " on if %s"), ifkey);
> +        goto exit_snoopreqput;
> +    }
> +
> +    virNWFilterSnoopLock();
> +
> +    if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, ifname,
> +                        req->ifkey) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq ifname map failed"
> +                                 " on interface \"%s\" key \"%s\""), ifname,
> +                               ifkey);
> +        goto exit_snoopunlock;
> +    }
> +
> +    if (isnewreq &&
> +        virHashAddEntry(virNWFilterSnoopState.snoopReqs, ifkey, req) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq req add failed on"
> +                               " interface \"%s\" ifkey \"%s\""), ifname,
> +                               ifkey);
> +        goto exit_rem_ifnametokey;
> +    }
> +
> +    /* prevent thread from holding req */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (virThreadCreate(&thread, false, virNWFilterDHCPSnoopThread,
> +                        req) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq virThreadCreate "
> +                                 "failed on interface '%s'"), ifname);
> +        goto exit_snoopreq_unlock;
> +    }
> +
> +    virAtomicIntAdd(&virNWFilterSnoopState.nThreads, 1);
> +
> +    req->threadkey = virNWFilterSnoopActivate(&thread);
> +    if (!req->threadkey) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Actication of snoop request failed on "
> +                                 "interface '%s'"), req->ifname);
> +        goto exit_snoopreq_unlock;
> +    }
> +
> +    if (virNWFilterSnoopReqRestore(req) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Restoring of leases failed on "
> +                               "interface '%s'"), req->ifname);
> +        goto exit_snoop_cancel;
> +    }
> +
> +    /* sync with thread */
> +    if (virCondWait(&req->threadStatusCond, &req->lock) < 0 ||
> +        req->threadStatus != THREAD_STATUS_OK)
> +        goto exit_snoop_cancel;
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    virNWFilterSnoopUnlock();
> +
> +    /* do not 'put' the req -- the thread will do this */
> +
> +    return 0;
> +
> +exit_snoop_cancel:
> +    virNWFilterSnoopCancel(&req->threadkey);
> +exit_snoopreq_unlock:
> +    virNWFilterSnoopReqUnlock(req);
> +exit_rem_ifnametokey:
> +    virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname);
> +exit_snoopunlock:
> +    virNWFilterSnoopUnlock();
> +exit_snoopreqput:
> +    virNWFilterSnoopReqPut(req);
> +
> +    return -1;
> +}
> +
> +static void
> +virNWFilterSnoopLeaseFileClose(void)
> +{
> +    VIR_FORCE_CLOSE(virNWFilterSnoopState.leaseFD);
> +}
> +
> +static void
> +virNWFilterSnoopLeaseFileOpen(void)
> +{
> +    virNWFilterSnoopLeaseFileClose();
> +
> +    virNWFilterSnoopState.leaseFD = open(LEASEFILE, O_CREAT|O_RDWR|O_APPEND,
> +                                         0644);
> +}
> +
> +/*
> + * Write a single lease to the given file.
> + *
> + */
> +static int
> +virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey,
> +                               virNWFilterSnoopIPLeasePtr ipl)
> +{
> +    char lbuf[256], ipstr[INET_ADDRSTRLEN], dhcpstr[INET_ADDRSTRLEN];
> +    size_t len;
> +
> +    if (inet_ntop(AF_INET, &ipl->ipAddress, ipstr, sizeof(ipstr)) == 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("inet_ntop(0x%08X) failed"), ipl->ipAddress);
> +        return -1;
> +    }
> +    if (inet_ntop(AF_INET, &ipl->ipServer, dhcpstr, sizeof(dhcpstr)) == 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("inet_ntop(0x%08X) failed"), ipl->ipServer);
> +        return -1;
> +    }
> +    /* time intf ip dhcpserver */
> +    snprintf(lbuf, sizeof(lbuf), "%u %s %s %s\n", ipl->timeout,
> +             ifkey, ipstr, dhcpstr);
> +
> +    len = strlen(lbuf);
> +
> +    if (safewrite(lfd, lbuf, len) != len) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("lease file write failed: %s"),
> +                               strerror(errno));
> +        return -1;
> +    }
> +
> +    (void) fsync(lfd);
> +
> +    return 0;
> +}
> +
> +/*
> + * Append a single lease to the end of the lease file.
> + * To keep a limited number of dead leases, re-read the lease
> + * file if the threshold of active leases versus written ones
> + * exceeds a threshold.
> + */
> +static void
> +virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr ipl)
> +{
> +    virNWFilterSnoopReqPtr req = ipl->snoopReq;
> +
> +    virNWFilterSnoopLock();
> +
> +    if (virNWFilterSnoopState.leaseFD < 0)
> +        virNWFilterSnoopLeaseFileOpen();
> +    if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.leaseFD,
> +                                       req->ifkey, ipl) < 0)
> +        goto err_exit;
> +
> +    /* keep dead leases at < ~95% of file size */
> +    if (virAtomicIntAdd(&virNWFilterSnoopState.wLeases, 1) >=
> +        virAtomicIntRead(&virNWFilterSnoopState.nLeases) * 20)
> +        virNWFilterSnoopLeaseFileLoad();   /* load & refresh lease file */
> +
> +err_exit:
> +    virNWFilterSnoopUnlock();
> +}
> +
> +/*
> + * Have requests removed that have no leases.
> + * Remove all expired leases.
> + * Call this function with the SnoopLock held.
> + */
> +static int
> +virNWFilterSnoopPruneIter(const void *payload,
> +                          const void *name ATTRIBUTE_UNUSED,
> +                          const void *data ATTRIBUTE_UNUSED)
> +{
> +    const virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload;
> +    bool del_req;
> +
> +    /* clean up orphaned, expired leases */
> +
> +    /* protect req->threadkey */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (!req->threadkey)
> +        virNWFilterSnoopReqLeaseTimerRun(req);
> +
> +    /*
> +     * have the entry removed if it has no leases and no one holds a ref
> +     */
> +    del_req = ((req->start == NULL) && (req->refctr == 0));
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    return del_req;
> +}
> +
> +/*
> + * Iterator to write all leases of a single request to a file.
> + * Call this function with the SnoopLock held.
> + */
> +static void
> +virNWFilterSnoopSaveIter(void *payload,
> +                         const void *name ATTRIBUTE_UNUSED,
> +                         void *data)
> +{
> +    virNWFilterSnoopReqPtr req = payload;
> +    int  tfd = *(int *)data;
> +    virNWFilterSnoopIPLeasePtr ipl;
> +
> +    /* protect req->start */
> +    virNWFilterSnoopReqLock(req);
> +
> +    for (ipl = req->start; ipl; ipl = ipl->next)
> +        (void) virNWFilterSnoopLeaseFileWrite(tfd, req->ifkey, ipl);
> +
> +    virNWFilterSnoopReqUnlock(req);
> +}
> +
> +/*
> + * Write all valid leases into a temporary file and then
> + * rename the file to the final file.
> + * Call this function with the SnoopLock held.
> + */
> +static void
> +virNWFilterSnoopLeaseFileRefresh(void)
> +{
> +    int tfd;
> +
> +    (void) unlink(TMPLEASEFILE);
> +    /* lease file loaded, delete old one */
> +    tfd = open(TMPLEASEFILE, O_CREAT|O_RDWR|O_TRUNC|O_EXCL, 0644);
> +    if (tfd < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("open(\"%s\"): %s"),
> +                               TMPLEASEFILE, strerror(errno));
> +        return;
> +    }
> +
> +    if (virNWFilterSnoopState.snoopReqs) {
> +        /* clean up the requests */
> +        virHashRemoveSet(virNWFilterSnoopState.snoopReqs,
> +                         virNWFilterSnoopPruneIter, NULL);
> +        /* now save them */
> +        virHashForEach(virNWFilterSnoopState.snoopReqs,
> +                       virNWFilterSnoopSaveIter, (void *)&tfd);
> +    }
> +
> +    VIR_FORCE_CLOSE(tfd);
> +
> +    if (rename(TMPLEASEFILE, LEASEFILE) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("rename(\"%s\", \"%s\"): %s"),
> +                               TMPLEASEFILE, LEASEFILE, strerror(errno));
> +        (void) unlink(TMPLEASEFILE);
> +    }
> +    virAtomicIntSet(&virNWFilterSnoopState.wLeases, 0);
> +    virNWFilterSnoopLeaseFileOpen();
> +}
> +
> +
> +static void
> +virNWFilterSnoopLeaseFileLoad(void)
> +{
> +    char line[256], ifkey[VIR_IFKEY_LEN];
> +    char ipstr[INET_ADDRSTRLEN], srvstr[INET_ADDRSTRLEN];
> +    virNWFilterSnoopIPLease ipl;
> +    virNWFilterSnoopReqPtr req;
> +    time_t now;
> +    FILE *fp;
> +    int ln = 0, tmp;
> +
> +    /* protect the lease file */
> +    virNWFilterSnoopLock();
> +
> +    fp = fopen(LEASEFILE, "r");
> +    time(&now);
> +    while (fp && fgets(line, sizeof(line), fp)) {
> +        if (line[strlen(line)-1] != '\n') {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("virNWFilterSnoopLeaseFileLoad lease file "
> +                                     "line %d corrupt"), ln);
> +            break;
> +        }
> +        ln++;
> +        /* key len 55 = "VMUUID"+'-'+"MAC" */
> +        if (sscanf(line, "%u %55s %16s %16s", &ipl.timeout,
> +                   ifkey, ipstr, srvstr) < 4) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("virNWFilterSnoopLeaseFileLoad lease file "
> +                                     "line %d corrupt"), ln);
> +            break;
> +        }
> +        if (ipl.timeout && ipl.timeout < now)
> +            continue;
> +        req = virNWFilterSnoopReqGetByIFKey(ifkey);
> +        if (!req) {
> +            req = virNWFilterSnoopReqNew(ifkey);
> +            if (!req)
> +               break;
> +
> +            tmp = virHashAddEntry(virNWFilterSnoopState.snoopReqs, ifkey, req);
> +
> +            if (tmp < 0) {
> +                virNWFilterSnoopReqPut(req);
> +                virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                       _("virNWFilterSnoopLeaseFileLoad req add"
> +                                         " failed on interface \"%s\""), ifkey);
> +                continue;
> +            }
> +        }
> +
> +        if (inet_pton(AF_INET, ipstr, &ipl.ipAddress) <= 0) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                  _("line %d corrupt ipaddr \"%s\""),
> +                                  ln, ipstr);
> +            virNWFilterSnoopReqPut(req);
> +            continue;
> +        }
> +        (void) inet_pton(AF_INET, srvstr, &ipl.ipServer);
> +        ipl.snoopReq = req;
> +
> +        if (ipl.timeout)
> +            virNWFilterSnoopReqLeaseAdd(req, &ipl, false);
> +        else
> +            virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, false);
> +
> +        virNWFilterSnoopReqPut(req);
> +    }
> +
> +    VIR_FORCE_FCLOSE(fp);
> +
> +    virNWFilterSnoopLeaseFileRefresh();
> +
> +    virNWFilterSnoopUnlock();
> +}
> +
> +/*
> + * Wait until all threads have ended.
> + */
> +static void
> +virNWFilterSnoopJoinThreads(void)
> +{
> +    while (virAtomicIntRead(&virNWFilterSnoopState.nThreads) != 0) {
> +        VIR_WARN("Waiting for snooping threads to terminate: %u\n",
> +                 virAtomicIntRead(&virNWFilterSnoopState.nThreads));
> +        usleep(1000 * 1000);
> +    }
> +}
> +
> +/*
> + * Iterator to remove a requests, repeatedly called on one
> + * request after another.
> + * The requests' ifname is freed allowing for an association
> + * of the Snoop request's leases with the same VM under a
> + * different interface name at a later time.
> + */
> +static int
> +virNWFilterSnoopRemAllReqIter(const void *payload,
> +                              const void *name ATTRIBUTE_UNUSED,
> +                              const void *data ATTRIBUTE_UNUSED)
> +{
> +    const virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload;
> +
> +    /* protect req->ifname */
> +    virNWFilterSnoopReqLock(req);
> +
> +    if (req->ifname) {
> +        (void) virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
> +                                  req->ifname);
> +
> +        VIR_FREE(req->ifname);
> +    }
> +
> +    virNWFilterSnoopReqUnlock(req);
> +
> +    /* removal will call virNWFilterSnoopCancel() */
> +    return 1;
> +}
> +
> +
> +/*
> + * Terminate all threads; keep the SnoopReqs hash allocated
> + */
> +static void
> +virNWFilterSnoopEndThreads(void)
> +{
> +    virNWFilterSnoopLock();
> +    virHashRemoveSet(virNWFilterSnoopState.snoopReqs,
> +                     virNWFilterSnoopRemAllReqIter,
> +                     NULL);
> +    virNWFilterSnoopUnlock();
> +}
> +
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    if (virNWFilterSnoopState.snoopReqs)
> +        return 0;
> +
> +    if (virMutexInitRecursive(&virNWFilterSnoopState.snoopLock) < 0 ||
> +        virMutexInit(&virNWFilterSnoopState.activeLock) < 0 ||
> +        virAtomicIntInit(&virNWFilterSnoopState.nLeases) < 0 ||
> +        virAtomicIntInit(&virNWFilterSnoopState.wLeases) < 0 ||
> +        virAtomicIntInit(&virNWFilterSnoopState.nThreads) < 0)
> +        return -1;
> +
> +    virNWFilterSnoopState.ifnameToKey = virHashCreate(0, NULL);
> +    virNWFilterSnoopState.active = virHashCreate(0, NULL);
> +    virNWFilterSnoopState.snoopReqs =
> +        virHashCreate(0, virNWFilterSnoopReqRelease);
> +
> +    if (!virNWFilterSnoopState.ifnameToKey ||
> +        !virNWFilterSnoopState.snoopReqs ||
> +        !virNWFilterSnoopState.active) {
> +        virReportOOMError();
> +        goto err_exit;
> +    }
> +
> +    virNWFilterSnoopLeaseFileLoad();
> +    virNWFilterSnoopLeaseFileOpen();
> +
> +    return 0;
> +
> +err_exit:
> +    virHashFree(virNWFilterSnoopState.ifnameToKey);
> +    virNWFilterSnoopState.ifnameToKey = NULL;
> +
> +    virHashFree(virNWFilterSnoopState.snoopReqs);
> +    virNWFilterSnoopState.snoopReqs = NULL;
> +
> +    virHashFree(virNWFilterSnoopState.active);
> +    virNWFilterSnoopState.active = NULL;
> +
> +    return -1;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname)
> +{
> +    char *ifkey = NULL;
> +
> +    virNWFilterSnoopLock();
> +
> +    if (!virNWFilterSnoopState.snoopReqs)
> +        goto cleanup;
> +
> +    if (ifname) {
> +        ifkey = (char *)virHashLookup(virNWFilterSnoopState.ifnameToKey,
> +                                      ifname);
> +        if (!ifkey) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("ifname \"%s\" not in key map"), ifname);
> +            goto cleanup;
> +        }
> +
> +        (void) virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname);
> +    }
> +
> +    if (ifkey) {
> +        virNWFilterSnoopReqPtr req;
> +
> +        req = virNWFilterSnoopReqGetByIFKey(ifkey);
> +        if (!req) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("ifkey \"%s\" has no req"), ifkey);
> +            goto cleanup;
> +        }
> +
> +        /* protect req->ifname & req->threadkey */
> +        virNWFilterSnoopReqLock(req);
> +
> +        /* keep valid lease req; drop interface association */
> +        virNWFilterSnoopCancel(&req->threadkey);
> +
> +        VIR_FREE(req->ifname);
> +
> +        virNWFilterSnoopReqUnlock(req);
> +
> +        virNWFilterSnoopReqPut(req);
> +    } else {                      /* free all of them */
> +        virNWFilterSnoopLeaseFileClose();
> +
> +        virHashRemoveAll(virNWFilterSnoopState.ifnameToKey);
> +
> +        /* tell the threads to terminate */
> +        virNWFilterSnoopEndThreads();
> +
> +        virNWFilterSnoopLeaseFileLoad();
> +    }
> +
> +cleanup:
> +    virNWFilterSnoopUnlock();
> +}
> +
> +void
> +virNWFilterDHCPSnoopShutdown(void)
> +{
> +    virNWFilterSnoopEndThreads();
> +    virNWFilterSnoopJoinThreads();
> +
> +    virNWFilterSnoopLock();
> +
> +    virNWFilterSnoopLeaseFileClose();
> +    virHashFree(virNWFilterSnoopState.ifnameToKey);
> +    virHashFree(virNWFilterSnoopState.snoopReqs);
> +
> +    virNWFilterSnoopUnlock();
> +
> +    virNWFilterSnoopActiveLock();
> +    virHashFree(virNWFilterSnoopState.active);
> +    virNWFilterSnoopActiveUnlock();
> +}
> +
> +#else /* HAVE_LIBPCAP */
> +
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    return -1;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname ATTRIBUTE_UNUSED)
> +{
> +    return;
> +}
> +
> +void
> +virNWFilterDHCPSnoopShutdown(void)
> +{
> +    return;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
> +                        const char *ifname ATTRIBUTE_UNUSED,
> +                        const char *linkdev ATTRIBUTE_UNUSED,
> +                        enum virDomainNetType nettype ATTRIBUTE_UNUSED,
> +                        const unsigned char *vmuuid ATTRIBUTE_UNUSED,
> +                        const unsigned char *macaddr ATTRIBUTE_UNUSED,
> +                        const char *filtername ATTRIBUTE_UNUSED,
> +                        virNWFilterHashTablePtr filterparams ATTRIBUTE_UNUSED,
> +                        virNWFilterDriverStatePtr driver ATTRIBUTE_UNUSED)
> +{
> +    virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("libvirt was not compiled "
> +                           "with libpcap and \"ip_learning='dhcp'\" requires"
> +                           " it."));
> +    return -1;
> +}

  if libpcap wasn't detected shouln't all these dummy functions log an
  error ?

> +#endif /* HAVE_LIBPCAP */
> Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h
> @@ -0,0 +1,39 @@
> +/*
> + * nwfilter_dhcpsnoop.h: support DHCP snooping for a VM on an interface
> + *
> + * Copyright (C) 2010 IBM Corp.
> + * Copyright (C) 2010 David L Stevens
> + *
> + * 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
> + *
> + * Author: David L Stevens <dlstevens at us.ibm.com>
> + */
> +
> +#ifndef __NWFILTER_DHCPSNOOP_H
> +#define __NWFILTER_DHCPSNOOP_H
> +
> +int virNWFilterDHCPSnoopInit(void);
> +void virNWFilterDHCPSnoopShutdown(void);
> +int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> +                            const char *ifname,
> +                            const char *linkdev,
> +                            enum virDomainNetType nettype,
> +                            const unsigned char *vmuuid,
> +                            const unsigned char *macaddr,
> +                            const char *filtername,
> +                            virNWFilterHashTablePtr filterparams,
> +                            virNWFilterDriverStatePtr driver);
> +void virNWFilterDHCPSnoopEnd(const char *ifname);
> +#endif /* __NWFILTER_DHCPSNOOP_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_dhcpsnoop.h"
>  #include "nwfilter_learnipaddr.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NWFILTER
> @@ -66,6 +67,8 @@ static int
>  nwfilterDriverStartup(int privileged) {
>      char *base = NULL;
>  
> +    if (virNWFilterDHCPSnoopInit() < 0)
> +        return -1;
>      if (virNWFilterLearnInit() < 0)
>          return -1;
>  
> @@ -127,6 +130,7 @@ alloc_err_exit:
>  
>  conf_init_err:
>      virNWFilterTechDriversShutdown();
> +    virNWFilterDHCPSnoopShutdown();
>      virNWFilterLearnShutdown();
>  
>      return -1;
> @@ -149,6 +153,7 @@ nwfilterDriverReload(void) {
>      conn = virConnectOpen("qemu:///system");
>  
>      if (conn) {
> +        virNWFilterDHCPSnoopEnd(NULL);
>          /* shut down all threads -- they will be restarted if necessary */
>          virNWFilterLearnThreadsTerminate(true);
>  
> @@ -203,6 +208,7 @@ nwfilterDriverShutdown(void) {
>  
>      virNWFilterConfLayerShutdown();
>      virNWFilterTechDriversShutdown();
> +    virNWFilterDHCPSnoopShutdown();
>      virNWFilterLearnShutdown();
>  
>      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
> @@ -32,6 +32,7 @@
>  #include "virterror_internal.h"
>  #include "nwfilter_gentech_driver.h"
>  #include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
>  #include "nwfilter_learnipaddr.h"
>  #include "virnetdev.h"
>  #include "datatypes.h"
> @@ -39,8 +40,10 @@
>  #define VIR_FROM_THIS VIR_FROM_NWFILTER
>  
>  
> -#define NWFILTER_STD_VAR_MAC "MAC"
> -#define NWFILTER_STD_VAR_IP  "IP"
> +#define NWFILTER_STD_VAR_MAC NWFILTER_VARNAME_MAC
> +#define NWFILTER_STD_VAR_IP  NWFILTER_VARNAME_IP
> +
> +#define NWFILTER_DFLT_LEARN  "any"
>  
>  static int _virNWFilterTeardownFilter(const char *ifname);
>  
> @@ -662,6 +665,9 @@ virNWFilterInstantiate(const unsigned ch
>      void **ptrs = NULL;
>      int instantiate = 1;
>      char *buf;
> +    virNWFilterVarValuePtr lv;
> +    const char *learning;
> +    bool reportIP = false;
>  
>      virNWFilterHashTablePtr missing_vars = virNWFilterHashTableCreate(0);
>      if (!missing_vars) {
> @@ -678,22 +684,47 @@ virNWFilterInstantiate(const unsigned ch
>      if (rc < 0)
>          goto err_exit;
>  
> +    lv = virHashLookup(vars->hashTable, NWFILTER_VARNAME_IP_LEARNING);
> +    if (lv)
> +        learning = virNWFilterVarValueGetNthValue(lv, 0);
> +    else
> +        learning = NULL;
> +
> +    if (learning == NULL)
> +        learning = NWFILTER_DFLT_LEARN;
> +
>      if (virHashSize(missing_vars->hashTable) == 1) {
>          if (virHashLookup(missing_vars->hashTable,
>                            NWFILTER_STD_VAR_IP) != NULL) {
> -            if (virNWFilterLookupLearnReq(ifindex) == NULL) {
> -                rc = virNWFilterLearnIPAddress(techdriver,
> -                                               ifname,
> -                                               ifindex,
> -                                               linkdev,
> -                                               nettype, macaddr,
> -                                               filter->name,
> -                                               vars, driver,
> -                                               DETECT_DHCP|DETECT_STATIC);
> +            if (STRCASEEQ(learning, "none")) {        /* no learning */
> +                reportIP = true;
> +                goto err_unresolvable_vars;
>              }
> -            goto err_exit;
> -        }
> -        goto err_unresolvable_vars;
> +            if (STRCASEEQ(learning, "dhcp")) {
> +                rc = virNWFilterDHCPSnoopReq(techdriver, ifname, linkdev,
> +                                             nettype, vmuuid, macaddr,
> +                                             filter->name, vars, driver);
> +                goto err_exit;
> +            } else if (STRCASEEQ(learning, "any")) {
> +                if (virNWFilterLookupLearnReq(ifindex) == NULL) {
> +                    rc = virNWFilterLearnIPAddress(techdriver,
> +                                                   ifname,
> +                                                   ifindex,
> +                                                   linkdev,
> +                                                   nettype, macaddr,
> +                                                   filter->name,
> +                                                   vars, driver,
> +                                                   DETECT_DHCP|DETECT_STATIC);
> +                }
> +                goto err_exit;
> +            } else {
> +                rc = -1;
> +                virNWFilterReportError(VIR_ERR_PARSE_FAILED, _("filter '%s' "
> +                                       "learning value '%s' invalid."),
> +                                       filter->name, learning);
> +            }
> +        } else
> +             goto err_unresolvable_vars;
>      } else if (virHashSize(missing_vars->hashTable) > 1) {
>          goto err_unresolvable_vars;
>      } else if (!forceWithPendingReq &&
> @@ -761,7 +792,7 @@ err_exit:
>  
>  err_unresolvable_vars:
>  
> -    buf = virNWFilterPrintVars(missing_vars->hashTable, ", ", false, false);
> +    buf = virNWFilterPrintVars(missing_vars->hashTable, ", ", false, reportIP);
>      if (buf) {
>          virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
>                     _("Cannot instantiate filter due to unresolvable "
> @@ -1092,6 +1123,8 @@ _virNWFilterTeardownFilter(const char *i
>          return -1;
>      }
>  
> +    virNWFilterDHCPSnoopEnd(ifname);
> +
>      virNWFilterTerminateLearnReq(ifname);
>  
>      if (virNWFilterLockIface(ifname) < 0)
> Index: libvirt-acl/src/conf/nwfilter_params.h
> ===================================================================
> --- libvirt-acl.orig/src/conf/nwfilter_params.h
> +++ libvirt-acl/src/conf/nwfilter_params.h
> @@ -91,6 +91,11 @@ int virNWFilterHashTablePutAll(virNWFilt
>  # define VALID_VARVALUE \
>    "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
>  
> +# define NWFILTER_VARNAME_IP "IP"
> +# define NWFILTER_VARNAME_MAC "MAC"
> +# define NWFILTER_VARNAME_IP_LEARNING "ip_learning"
> +# define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER"
> +
>  enum virNWFilterVarAccessType {
>      VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0,
>      VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1,

  A fairly big patch ! I have tried to really read everything, but
finding locking issues, especially when there is multiple layers
of locks is nearly impossible by review.
  I think the best is to get this running and tested as much as possible
before the next release,

  ACK

Daniel

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




More information about the libvir-list mailing list