Re: [libvirt PATCHv7 1/1] add DHCP snooping

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

 



Stefan was faster, but some nits I've spotted in addition to his.

On 26.03.2012 22:25, David L Stevens 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.
> 
> Changes since v6:
> - replace pthread_cancel() with synchronous cancelation method
> 
> Signed-off-by: David L Stevens <dlstevens@xxxxxxxxxx>
> ---
>  docs/formatnwfilter.html.in            |   17 +
>  src/Makefile.am                        |    2 +
>  src/nwfilter/nwfilter_dhcpsnoop.c      | 1139 ++++++++++++++++++++++++++++++++
>  src/nwfilter/nwfilter_dhcpsnoop.h      |   38 ++
>  src/nwfilter/nwfilter_driver.c         |    6 +
>  src/nwfilter/nwfilter_gentech_driver.c |   59 ++-
>  6 files changed, 1248 insertions(+), 13 deletions(-)
>  create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c
>  create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h
> 
> diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in
> index 9cb7644..ad10765 100644
> --- a/docs/formatnwfilter.html.in
> +++ b/docs/formatnwfilter.html.in
> @@ -2189,6 +2189,23 @@
>         <br/><br/>
>         In case a VM is resumed after suspension or migrated, IP address
>         detection will be restarted.
> +       <br/><br/>
> +       Variable <i>ip_learning</i> may be used to specify
> +       the IP address learning method. Valid values are <i>any</i>, <i>dhcp</i>,
> +       or <i>none</i>. The default value is <i>any</i>, meaning that libvirt
> +       may use any packet to determine the address in use by a VM. A value of
> +       <i>dhcp</i> specifies that libvirt should only honor DHCP server-assigned
> +       addresses with valid leases. If <i>ip_learning</i> is set to <i>none</i>,
> +       libvirt does not do address learning and referencing <i>IP</i> without
> +       assigning it an explicit value is an error.
> +       <br/><br/>
> +       Use of <i>ip_learning=dhcp</i> (DHCP snooping) provides additional
> +       anti-spoofing security, especially when combined with a filter allowing
> +       only trusted DHCP servers to assign addresses. If 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).
>       </p>
>  
>      <h3><a name="nwflimitsmigr">VM Migration</a></h3>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index a2aae9d..4382caf 100644
> --- a/src/Makefile.am
> +++ b/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				\
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
> new file mode 100644
> index 0000000..8e5dcc5
> --- /dev/null
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -0,0 +1,1139 @@
> +/*
> + * nwfilter_dhcpsnoop.c: support for DHCP snooping used by a VM
> + *                         on an interface
> + *
> + * Copyright (C) 2011 IBM Corp.
> + * Copyright (C) 2011 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@xxxxxxxxxx>
> + * Based in part on work by Stefan Berger <stefanb@xxxxxxxxxx>
> + */
> +
> +#include <config.h>
> +
> +#ifdef HAVE_LIBPCAP
> +#include <pcap.h>
> +#endif
> +
> +#include <fcntl.h>
> +#include <sys/ioctl.h>
> +#include <signal.h>
> +
> +#include <arpa/inet.h>
> +#include <net/ethernet.h>
> +#include <netinet/ip.h>
> +#include <netinet/udp.h>
> +#include <net/if.h>
> +#include <net/if_arp.h>
> +#include <intprops.h>
> +
> +#include "internal.h"
> +
> +#include "buf.h"
> +#include "memory.h"
> +#include "logging.h"
> +#include "datatypes.h"
> +#include "virterror_internal.h"
> +#include "threads.h"
> +#include "conf/nwfilter_params.h"
> +#include "conf/domain_conf.h"
> +#include "nwfilter_gentech_driver.h"
> +#include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
> +#include "virnetdev.h"
> +#include "virfile.h"
> +#include "configmake.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +#ifdef HAVE_LIBPCAP
> + 

trailing whitespace
> +#define LEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.leases"
> +#define TMPLEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.ltmp"
> +static int lease_fd = -1;
> +static int nleases = 0; /* number of active leases */
> +static int wleases = 0; /* number of written leases */
> +
> +static virHashTablePtr SnoopReqs;
> +static virHashTablePtr IfnameToKey;
> +static pthread_mutex_t SnoopLock = PTHREAD_MUTEX_INITIALIZER;
> +
> +/* snooper thread management */
> +
> +static virHashTablePtr Active;
> +static pthread_mutex_t ActiveLock = PTHREAD_MUTEX_INITIALIZER;
> +
> +#define active_lock()    { pthread_mutex_lock(&ActiveLock); }
> +#define active_unlock()  { pthread_mutex_unlock(&ActiveLock); }

Don't we want to use virThread* and virMutex* here?

> +
> +static char *
> +SnoopActivate(pthread_t thread)
> +{
> +    int len;
> +    char *key;
> +
> +    len = sizeof(thread)*2 + 3; /* "0x"+'\0' */
> +    if (VIR_ALLOC_N(key, len)) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +    (void) snprintf(key, len, "0x%0*lX", sizeof(thread)*2,
> +                    (unsigned long int)thread);

* format expects int but sizeof() returns size_t;

just cosmetic - unless function is ATTRIBUTE_RETURN_CHECK I don't see
much value in these (void) typecasts.

> +    key[len-1] = '\0';
> +
> +    active_lock();
> +    if (virHashAddEntry(Active, key, strdup("1")))
> +        VIR_FREE(key);
> +    active_unlock();
> +    return key;
> +}
> +
> +static void
> +SnoopCancel(char **ThreadKey)
> +{
> +    if (*ThreadKey == NULL)
> +        return;
> +
> +    active_lock();
> +    (void) virHashRemoveEntry(Active, *ThreadKey);
> +    *ThreadKey = NULL;
> +    active_unlock();
> +}
> +
> +static bool
> +SnoopIsActive(char *ThreadKey)
> +{
> +    void *entry;
> +
> +    if (ThreadKey == NULL)
> +        return 0;
> +    active_lock();
> +    entry = virHashLookup(Active, ThreadKey);
> +    active_unlock();
> +    return entry != NULL;
> +}
> +
> +#define snoop_lock()    { pthread_mutex_lock(&SnoopLock); }
> +#define snoop_unlock()  { pthread_mutex_unlock(&SnoopLock); }
> +
> +#define VIR_IFKEY_LEN   ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN))
> +
> +struct virNWFilterSnoopReq {
> +    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 */
> +    struct iplease           *start;
> +    struct iplease           *end;
> +    char                     *threadkey;
> +};
> +
> +#define POLL_INTERVAL    10*1000 /* 10 secs */
> +#define MAXERRS               25 /* retries on failing device */
> +
> +struct iplease {
> +    uint32_t                    ipl_ipaddr;
> +    uint32_t                    ipl_server;
> +    struct virNWFilterSnoopReq *ipl_req;
> +    unsigned int                ipl_timeout;
> +    /* timer list */
> +    struct iplease             *ipl_prev;
> +    struct iplease             *ipl_next;
> +};

Do we really need double linked list here?

> +
> +static struct iplease *ipl_getbyip(struct iplease *start, uint32_t ipaddr);
> +static void ipl_update(struct iplease *pl, uint32_t timeout);
> + 

trailing whitespace

> +static struct virNWFilterSnoopReq *newreq(const char *ifkey);
> +
> +static void lease_open(void);
> +static void lease_close(void);
> +static void lease_load(void);
> +static void lease_save(struct iplease *ipl);
> +static void lease_restore(struct virNWFilterSnoopReq *req);
> +static void lease_refresh(void);
> +
> +/*
> + * ipl_ladd - add an IP lease to a list
> + */
> +static void
> +ipl_ladd(struct iplease *plnew, struct iplease **start, struct iplease **end)
> +{
> +    struct iplease             *pl;
> +
> +    plnew->ipl_next = plnew->ipl_prev = 0;
> +    if (!*start) {
> +        *start = *end = plnew;
> +        return;
> +    }
> +    for (pl = *end; pl && plnew->ipl_timeout < pl->ipl_timeout;
> +         pl = pl->ipl_prev)
> +        /* empty */ ;
> +    if (!pl) {
> +        plnew->ipl_next = *start;
> +        *start = plnew;
> +    } else {
> +        plnew->ipl_next = pl->ipl_next;
> +        pl->ipl_next = plnew;
> +    }
> +    plnew->ipl_prev = pl;
> +    if (plnew->ipl_next)
> +        plnew->ipl_next->ipl_prev = plnew;
> +    else
> +        *end = plnew;
> +}
> +
> +/*
> + * ipl_tadd - add an IP lease to the timer list
> + */
> +static void
> +ipl_tadd(struct iplease *plnew)
> +{
> +    struct virNWFilterSnoopReq *req = plnew->ipl_req;
> +
> +    ipl_ladd(plnew, &req->start, &req->end);
> +}
> + 

trailing whitespace

> +/*
> + * ipl_install - install rule for a lease
> + */
> +static int
> +ipl_install(struct iplease *ipl)
> +{
> +    char                     ipbuf[20];    /* dotted decimal IP addr string */
> +    int                      rc;
> +    virNWFilterVarValuePtr   ipVar;
> +
> +    if (!inet_ntop(AF_INET, &ipl->ipl_ipaddr, ipbuf, sizeof(ipbuf))) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("ipl_install inet_ntop " "failed (0x%08X)"),
> +                               ipl->ipl_ipaddr);
> +        return -1;
> +    }
> +    ipVar = virNWFilterVarValueCreateSimpleCopyValue(ipbuf);
> +    if (!ipVar) {
> +        virReportOOMError();
> +        return -1;
> +    }
> +    if (virNWFilterHashTablePut(ipl->ipl_req->vars, "IP", ipVar, 1)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Could not add variable \"IP\" to hashmap"));
> +        virNWFilterVarValueFree(ipVar);
> +        return -1;
> +    }
> +    rc = virNWFilterInstantiateFilterLate(NULL,
> +                                          ipl->ipl_req->ifname,
> +                                          ipl->ipl_req->ifindex,
> +                                          ipl->ipl_req->linkdev,
> +                                          ipl->ipl_req->nettype,
> +                                          ipl->ipl_req->macaddr,
> +                                          ipl->ipl_req->filtername,
> +                                          ipl->ipl_req->vars,
> +                                          ipl->ipl_req->driver);
> +    if (rc)
> +        return -1;
> +    return 0;
> +}
> +
> +/*
> + * ipl_add - create or update an IP lease
> + */
> +static void
> +ipl_add(struct iplease *plnew, bool update_leasefile)
> +{
> +    struct iplease *pl;
> +    struct virNWFilterSnoopReq *req = plnew->ipl_req;
> +
> +    pl = ipl_getbyip(req->start, plnew->ipl_ipaddr);
> +    if (pl) {
> +        ipl_update(pl, plnew->ipl_timeout);
> +        if (update_leasefile)
> +            lease_save(pl);
> +        return;
> +    }
> +    /* 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;    /* silently ignore multiple addresses */
> +
> +    if (VIR_ALLOC(pl) < 0) {
> +        virReportOOMError();
> +        return;
> +    }
> +    *pl = *plnew;
> +
> +    if (req->threadkey && ipl_install(pl) < 0) {
> +        VIR_FREE(pl);
> +        return;
> +    }
> +    ipl_tadd(pl);
> +    nleases++;
> +    if (update_leasefile)
> +        lease_save(pl);
> +}
> +
> +/*
> + * ipl_tdel - remove an IP lease from a list
> + */
> +static void
> +ipl_ldel(struct iplease *ipl, struct iplease **start, struct iplease **end)
> +{
> +    if (ipl->ipl_prev)
> +        ipl->ipl_prev->ipl_next = ipl->ipl_next;
> +    else
> +        *start = ipl->ipl_next;
> +    if (ipl->ipl_next)
> +        ipl->ipl_next->ipl_prev = ipl->ipl_prev;
> +    else
> +        *end = ipl->ipl_prev;
> +    ipl->ipl_next = ipl->ipl_prev = 0;
> +}
> +
> +/*
> + * ipl_tdel - remove an IP lease from the timer list
> + */
> +static void
> +ipl_tdel(struct iplease *ipl)
> +{
> +    struct virNWFilterSnoopReq *req = ipl->ipl_req;
> +
> +    ipl_ldel(ipl, &req->start, &req->end);
> +    ipl->ipl_timeout = 0;
> +}
> +
> +/*
> + * ipl_del - delete an IP lease
> + */
> +static void
> +ipl_del(struct virNWFilterSnoopReq *req, uint32_t ipaddr, bool update_leasefile)
> +{
> +    struct iplease *ipl;
> +
> +    ipl = ipl_getbyip(req->start, ipaddr);
> +    if (ipl == NULL)
> +        return;
> +
> +    ipl_tdel(ipl);
> +
> +    if (update_leasefile) {
> +        lease_save(ipl);
> +
> +        /*
> +         * for multiple address support, this needs to remove those rules
> +         * referencing "IP" with ipl's ip value.
> +         */
> +        if (req->techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr,
> +                                                NULL, false))
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "ipl_ldel failed");
> +    }
> +    VIR_FREE(ipl);
> +    nleases--;
> +}
> +
> +/*
> + * ipl_update - update the timeout on an IP lease
> + */
> +static void
> +ipl_update(struct iplease *ipl, uint32_t timeout)
> +{
> +    if (timeout < ipl->ipl_timeout)
> +        return;  /* no take-backs */
> +    ipl_tdel(ipl);
> +    ipl->ipl_timeout = timeout;
> +    ipl_tadd(ipl);
> +    return;
> +}
> +
> +/*
> + * ipl_getbyip - lookup IP lease by IP address
> + */
> +static struct iplease *
> +ipl_getbyip(struct iplease *start, uint32_t ipaddr)
> +{
> +    struct iplease *pl;
> +
> +    for (pl = start; pl && pl->ipl_ipaddr != ipaddr; pl = pl->ipl_next)
> +        /* empty */ ;
> +    return pl;
> +}
> +
> +/*
> + * ipl_trun - run the IP lease timeout list
> + */
> +static unsigned int
> +ipl_trun(struct virNWFilterSnoopReq *req)
> +{
> +    uint32_t now;
> +
> +    now = time(0);
> +    while (req->start && req->start->ipl_timeout <= now)
> +        ipl_del(req, req->start->ipl_ipaddr, 1);
> +    return 0;
> +}
> +
> +typedef unsigned char Eaddr[6];
> +
> +struct eth {
> +    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
> +
> +struct dhcp {
> +    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
> +
> +unsigned char dhcp_magic[4] = { 99, 130, 83, 99 };
> +
> +static int
> +dhcp_getopt(struct dhcp *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;
> +}
> +
> +static void
> +dhcpdecode(struct virNWFilterSnoopReq *req, struct eth *pep, int len)
> +{
> +    struct iphdr   *pip;
> +    struct udphdr  *pup;
> +    struct dhcp    *pd;
> +    struct iplease  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(struct eth, eh_data);
> +        break;
> +    case ETHERTYPE_VLAN:
> +        if (ntohs(pep->ehv_type) != ETHERTYPE_IP)
> +            return;
> +        pip = (struct iphdr *) pep->ehv_data;
> +        len -= offsetof(struct eth, ehv_data);
> +        break;
> +    default:
> +        return;
> +    }
> +    pip = (struct iphdr *) pep->eh_data;
> +    len -= sizeof(*pep);
> +    pup = (struct udphdr *) ((char *) pip + (pip->ihl << 2));
> +    len -= pip->ihl << 2;
> +    pd = (struct dhcp *) ((char *) pup + sizeof(*pup));
> +    len -= sizeof(*pup);
> +    if (len < 0)
> +        return;                 /* dhcpdecode: invalid packet length */
> +    if (dhcp_getopt(pd, len, &mtype, &leasetime) < 0)
> +        return;
> +
> +    memset(&ipl, 0, sizeof(ipl));
> +    ipl.ipl_ipaddr = pd->d_yiaddr;
> +    ipl.ipl_server = pd->d_siaddr;
> +    if (leasetime == ~0)
> +        ipl.ipl_timeout = ~0;
> +    else
> +        ipl.ipl_timeout = time(0) + leasetime;
> +    ipl.ipl_req = req;
> +
> +    switch (mtype) {
> +        case DHCPACK:
> +            ipl_add(&ipl, 1);
> +            break;
> +        case DHCPDECLINE:
> +        case DHCPRELEASE:
> +            ipl_del(req, ipl.ipl_ipaddr, 1);
> +            break;
> +        default:
> +            break;
> +    }
> +}
> +
> +#define PBUFSIZE        576     /* >= IP/TCP/DHCP headers */
> +#define TIMEOUT          30 /* secs */
> +
> +static pcap_t *
> +dhcpopen(const char *intf)
> +{
> +    pcap_t             *handle = NULL;
> +    struct bpf_program  fp;
> +    char                filter[64];
> +    char                pcap_errbuf[PCAP_ERRBUF_SIZE];
> +    time_t              start;
> +
> +    start = time(0);
> +    while (handle == NULL && time(0) - start < TIMEOUT)
> +        handle = pcap_open_live(intf, PBUFSIZE, 0, POLL_INTERVAL, pcap_errbuf);
> +
> +    if (handle == NULL) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_open_live: %s"), pcap_errbuf);
> +        return 0;
> +    }
> +
> +    sprintf(filter, "port 67 or dst port 68");
> +    if (pcap_compile(handle, &fp, filter, 1, PCAP_NETMASK_UNKNOWN) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_compile: %s"), pcap_geterr(handle));
> +        return 0;
> +    }
> +    if (pcap_setfilter(handle, &fp) != 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("pcap_setfilter: %s"), pcap_geterr(handle));
> +        return 0;
> +    }
> +    pcap_freecode(&fp);
> +    return handle;
> +}
> +
> +static void
> +snoopReqFree(struct virNWFilterSnoopReq *req)
> +{
> +    struct iplease *ipl;
> +
> +    if (!req)
> +        return;
> +
> +    /* free all leases */
> +    for (ipl = req->start; ipl; ipl = req->start)
> +        ipl_del(req, ipl->ipl_ipaddr, 0);
> +
> +    /* free all req data */
> +    VIR_FREE(req->ifname);
> +    VIR_FREE(req->linkdev);
> +    VIR_FREE(req->filtername);
> +    virNWFilterHashTableFree(req->vars);
> +    VIR_FREE(req);
> +}
> +
> +static void *
> +virNWFilterDHCPSnoop(void *req0)
> +{
> +    struct virNWFilterSnoopReq *req = req0;
> +    pcap_t                     *handle;
> +    struct pcap_pkthdr         *hdr;
> +    struct eth                 *packet;
> +    int                         ifindex;
> +    int                         errcount;
> +    char                       *threadkey;
> +
> +    handle = dhcpopen(req->ifname);
> +    if (!handle)
> +        return 0;
> +
> +    req->threadkey = SnoopActivate(pthread_self());
> +    threadkey = strdup(req->threadkey);
> +
> +    ifindex = if_nametoindex(req->ifname);
> +
> +    snoop_lock();
> +    lease_restore(req);
> +    snoop_unlock();
> +
> +    errcount = 0;
> +    while (1) {
> +        int rv;
> +
> +        snoop_lock();
> +        ipl_trun(req);
> +        snoop_unlock();
> +
> +        rv = pcap_next_ex(handle, &hdr, (const u_char **)&packet);
> +
> +        if (!SnoopIsActive(threadkey)) {
> +            VIR_FREE(threadkey);
> +            pcap_close(handle);
> +            return 0;
> +        }
> +        if (rv < 0) {
> +            if (virNetDevValidateConfig(req->ifname, NULL, ifindex) <= 0)
> +                break;
> +            if (++errcount > MAXERRS) {
> +                virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                       _("ifname \"%s\" failing; reopening"),
> +                                       req->ifname);
> +                pcap_close(handle);
> +                handle = dhcpopen(req->ifname);
> +                if (!handle)
> +                    break;
> +            }
> +            continue;
> +        }
> +        errcount = 0;
> +        if (rv) {
> +            snoop_lock();
> +            dhcpdecode(req, packet, hdr->caplen);
> +            snoop_unlock();
> +        }
> +    }
> +    SnoopCancel(&req->threadkey);
> +    (void) virHashRemoveEntry(IfnameToKey, req->ifname);
> +    VIR_FREE(req->ifname);
> +    /* if we still have a valid lease, keep the req for restarts */
> +    if (!req->start || req->start->ipl_timeout < time(0))
> +        (void) virHashRemoveEntry(SnoopReqs, req->ifkey);
> +    VIR_FREE(threadkey);
> +    pcap_close(handle);
> +    return 0;
> +}
> +
> +static void
> +ifkeyFormat(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)
> +{
> +    struct virNWFilterSnoopReq *req;
> +    bool                        isnewreq;
> +    char                        ifkey[VIR_IFKEY_LEN];
> +    pthread_t                   thread;
> +
> +    ifkeyFormat(ifkey, vmuuid, macaddr);
> +    snoop_lock();
> +    req = virHashLookup(SnoopReqs, ifkey);
> +    isnewreq = req == NULL;
> +    if (!isnewreq) {
> +        if (req->threadkey) {
> +            snoop_unlock();
> +            return 0;
> +        }
> +    } else {
> +        req = newreq(ifkey);
> +        if (!req) {
> +            snoop_unlock();
> +            return 1;
> +        }
> +    }
> +
> +    req->techdriver = techdriver;
> +    req->ifindex = if_nametoindex(ifname);
> +    req->linkdev = linkdev ? strdup(linkdev) : NULL;
> +    req->nettype = nettype;
> +    req->ifname = strdup(ifname);
> +    memcpy(req->macaddr, macaddr, sizeof(req->macaddr));
> +    req->filtername = strdup(filtername);
> +    if (req->filtername == NULL) {
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        virReportOOMError();
> +        return 1;
> +    }
> +
> +    if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL,
> +                                       false)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "applyDHCPOnlyRules "
> +                               "failed - spoofing not protected!");
> +    }
> +
> +    req->vars = virNWFilterHashTableCreate(0);
> +    if (!req->vars) {
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        virReportOOMError();
> +        return 1;
> +    }
> +    if (virNWFilterHashTablePutAll(filterparams, req->vars)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq: can't copy variables"
> +                                " on if %s"), ifkey);
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        return 1;
> +    }
> +    req->driver = driver;
> +
> +    if (virHashAddEntry(IfnameToKey, ifname, req->ifkey)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq ifname map failed"
> +                                 " on interface \"%s\" key \"%s\""), ifname,
> +                               ifkey);
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        return 1;
> +    }
> +    if (isnewreq && virHashAddEntry(SnoopReqs, ifkey, req)) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq req add failed on"
> +                               " interface \"%s\" ifkey \"%s\""), ifname,
> +                               ifkey);
> +        (void) virHashRemoveEntry(IfnameToKey, ifname);
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        return 1;
> +    }
> +    snoop_unlock();
> +    if (pthread_create(&thread, NULL, virNWFilterDHCPSnoop, req) != 0) {
> +        snoop_lock();
> +        (void) virHashRemoveEntry(IfnameToKey, ifname);
> +        (void) virHashRemoveEntry(SnoopReqs, ifkey);
> +        snoop_unlock();
> +        snoopReqFree(req);
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("virNWFilterDHCPSnoopReq pthread_create failed"
> +                                " on interface \"%s\""), ifname);
> +        return 1;
> +    }
> +    return 0;
> +}
> +
> +/*
> + * freeReq - hash table free function to kill a request
> + */
> +static void
> +freeReq(void *req0, const void *name ATTRIBUTE_UNUSED)
> +{
> +    struct virNWFilterSnoopReq *req = (struct virNWFilterSnoopReq *) req0;
> +
> +    if (!req)
> +        return;
> +
> +    if (req->threadkey)
> +        SnoopCancel(&req->threadkey);
> +    snoopReqFree(req);
> +}
> +
> +static void
> +lease_close(void)
> +{
> +    VIR_FORCE_CLOSE(lease_fd);
> +}
> +
> +static void
> +lease_open(void)
> +{
> +    lease_close();
> +
> +    lease_fd = open(LEASEFILE, O_CREAT|O_RDWR|O_APPEND, 0644);
> +}
> +
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    if (SnoopReqs)
> +        return 0;
> +
> +    snoop_lock();
> +    IfnameToKey = virHashCreate(0, NULL);
> +    SnoopReqs = virHashCreate(0, freeReq);
> +    if (!SnoopReqs) {
> +        snoop_unlock();
> +        virReportOOMError();
> +        return -1;
> +    }
> +    lease_load();
> +    lease_open();
> +
> +    snoop_unlock();
> +
> +    active_lock();
> +    Active = virHashCreate(0, 0);
> +    if (!Active) {
> +        active_unlock();
> +        virReportOOMError();
> +        return -1;
> +    }
> +    active_unlock();
> +    return 0;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname)
> +{
> +    char *ifkey = NULL;
> + 

trailing whitespace

> +    snoop_lock();
> +    if (!SnoopReqs) {
> +        snoop_unlock();
> +        return;
> +    }
> +
> +    if (ifname) {
> +        ifkey = (char *)virHashLookup(IfnameToKey, ifname);
> +        (void) virHashRemoveEntry(IfnameToKey, ifname);
> +        if (!ifkey) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("ifname \"%s\" not in key map"), ifname);
> +            snoop_unlock();
> +            return;
> +        }
> +    }
> +
> +    if (ifkey) {
> +        struct virNWFilterSnoopReq *req;
> +
> +        req = virHashLookup(SnoopReqs, ifkey);
> +        if (!req) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("ifkey \"%s\" has no req"), ifkey);
> +            snoop_unlock();
> +            return;
> +        }
> +        if (!req->start || req->start->ipl_timeout < time(0)) {
> +            (void) virHashRemoveEntry(SnoopReqs, req->ifkey);
> +            snoop_unlock();
> +            return;
> +        }
> +        /* keep valid lease req; drop interface association */
> +        SnoopCancel(&req->threadkey);
> +        VIR_FREE(req->ifname);
> +    } else {                      /* free all of them */
> +        lease_close();
> +        virHashFree(IfnameToKey);
> +        virHashFree(SnoopReqs);
> +        IfnameToKey = virHashCreate(0, 0);
> +        if (!IfnameToKey) {
> +            snoop_unlock();
> +            virReportOOMError();
> +            return;
> +        }
> +        SnoopReqs = virHashCreate(0, freeReq);
> +        if (!SnoopReqs) {
> +            virHashFree(IfnameToKey);
> +            snoop_unlock();
> +            virReportOOMError();
> +            return;
> +        }
> +        lease_load();
> +    }
> +    snoop_unlock();
> +}
> +
> +static int
> +lease_write(int lfd, const char *ifkey, struct iplease *ipl)
> +{
> +    char lbuf[256],ipstr[16],dhcpstr[16];
> +
> +    if (inet_ntop(AF_INET, &ipl->ipl_ipaddr, ipstr, sizeof ipstr) == 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("inet_ntop(0x%08X) failed"), ipl->ipl_ipaddr);
> +        return -1;
> +    }
> +    if (inet_ntop(AF_INET, &ipl->ipl_server, dhcpstr, sizeof dhcpstr) == 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("inet_ntop(0x%08X) failed"), ipl->ipl_server);
> +        return -1;
> +    }
> +    /* time intf ip dhcpserver */
> +    snprintf(lbuf, sizeof(lbuf), "%u %s %s %s\n", ipl->ipl_timeout,
> +             ifkey, ipstr, dhcpstr);
> +    if (write(lfd, lbuf, strlen(lbuf)) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("lease file write failed: %s"),
> +                               strerror(errno));
> +        return -1;
> +    }
> +    (void) fsync(lfd);
> +    return 0;
> +}
> +
> +static void
> +lease_save(struct iplease *ipl)
> +{
> +    struct virNWFilterSnoopReq *req = ipl->ipl_req;
> +
> +    if (lease_fd < 0)
> +       lease_open();
> +    if (lease_write(lease_fd, req->ifkey, ipl) < 0)
> +       return;
> +    /* keep dead leases at < ~95% of file size */
> +    if (++wleases >= nleases*20)
> +        lease_load();   /* load & refresh lease file */
> +}
> +
> +static struct virNWFilterSnoopReq *
> +newreq(const char *ifkey)
> +{
> +    struct virNWFilterSnoopReq *req;
> +
> +    if (VIR_ALLOC(req) < 0) {
> +        virReportOOMError();
> +        return NULL;
> +    }
> +    strncpy(req->ifkey, ifkey, sizeof req->ifkey);
> +
> +    return req;
> +}
> +
> +static void
> +SaveSnoopReqIter(void *payload,
> +                 const void *name ATTRIBUTE_UNUSED,
> +                 void *data)
> +{
> +    struct virNWFilterSnoopReq *req = payload;
> +    int tfd = (int)data;

sizeof(void *) != sizeof(int) in general ..

> +    struct iplease *ipl;
> +
> +    /* clean up orphaned, expired leases */
> +    if (!req->threadkey) {
> +        uint32_t now;
> +
> +        now = time(0);
> +        for (ipl = req->start; ipl; ipl = ipl->ipl_next)
> +            if (ipl->ipl_timeout < now)
> +                ipl_del(req, ipl->ipl_ipaddr , 0);
> +        if (!req->start) {
> +            snoopReqFree(req);
> +            return;
> +        }
> +    }
> +    for (ipl = req->start; ipl; ipl = ipl->ipl_next)
> +        (void) lease_write(tfd, req->ifkey, ipl);
> +}
> +
> +static void
> +lease_refresh(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 (SnoopReqs)
> +        virHashForEach(SnoopReqs, SaveSnoopReqIter, (void *)tfd);

.. same here. Why not pass &tfd?

> +    (void) close(tfd);
> +    if (rename(TMPLEASEFILE, LEASEFILE) < 0) {
> +        virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("rename(\"%s\", \"%s\"): %s"),
> +                               TMPLEASEFILE, LEASEFILE, strerror(errno));
> +        (void) unlink(TMPLEASEFILE);
> +    }
> +    wleases = 0;
> +    lease_open();
> +}
> +
> +
> +static void
> +lease_load(void)
> +{
> +    char line[256], ifkey[VIR_IFKEY_LEN], ipstr[16], srvstr[16];
> +    struct iplease ipl;
> +    struct virNWFilterSnoopReq *req;
> +    time_t now;
> +    FILE *fp;
> +    int ln = 0;
> +
> +    fp = fopen(LEASEFILE, "r");
> +    time(&now);
> +    while (fp && fgets(line, sizeof(line), fp)) {
> +        if (line[strlen(line)-1] != '\n') {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("lease_load lease file line %d corrupt"),
> +                                   ln);
> +            break;
> +        }
> +        ln++;
> +        /* key len 55 = "VMUUID"+'-'+"MAC" */
> +        if (sscanf(line, "%u %55s %16s %16s", &ipl.ipl_timeout,
> +                   ifkey, ipstr, srvstr) < 4) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("lease_load lease file line %d corrupt"),
> +                                   ln);
> +            break;;
> +        }
> +        if (ipl.ipl_timeout && ipl.ipl_timeout < now)
> +            continue;
> +        req = virHashLookup(SnoopReqs, ifkey);
> +        if (!req) {
> +            req = newreq(ifkey);
> +            if (!req)
> +               break;
> +            if (virHashAddEntry(SnoopReqs, ifkey, req)) {
> +                snoopReqFree(req);
> +                virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                       _("lease_load req add failed on "
> +                                       "interface \"%s\""), ifkey);
> +                continue;
> +            }
> +        }
> +
> +        if (inet_pton(AF_INET, ipstr, &ipl.ipl_ipaddr) <= 0) {
> +            virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> +                                  _("line %d corrupt ipaddr \"%s\""),
> +                                  ln, ipstr);
> +            continue;
> +        }
> +        (void) inet_pton(AF_INET, srvstr, &ipl.ipl_server);
> +        ipl.ipl_req = req;
> +
> +        if (ipl.ipl_timeout)
> +            ipl_add(&ipl, 0);
> +        else
> +            ipl_del(req, ipl.ipl_ipaddr, 0);
> +    }
> +    if (fp != NULL)
> +        (void) fclose(fp);
> +    lease_refresh();
> +}
> +
> +static void
> +lease_restore(struct virNWFilterSnoopReq *req)
> +{
> +    struct iplease *ipl;
> +
> +    for (ipl=req->start; ipl; ipl=ipl->ipl_next)
> +        (void) ipl_install(ipl);
> +}
> +
> +#else /* HAVE_LIBPCAP */
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> +    return -1;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname ATTRIBUTE_UNUSED)
> +{
> +    return;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
> +                        const char *ifname ATTRIBUTE_UNUSED,
> +                        const char *linkdev ATTRIBUTE_UNUSED,
> +                        enum virDomainNetType nettype ATTRIBUTE_UNUSED,
> +                        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;
> +}
> +#endif /* HAVE_LIBPCAP */
> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.h b/src/nwfilter/nwfilter_dhcpsnoop.h
> new file mode 100644
> index 0000000..25500e2
> --- /dev/null
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.h
> @@ -0,0 +1,38 @@
> +/*
> + * 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@xxxxxxxxxx>
> + */
> +
> +#ifndef __NWFILTER_DHCPSNOOP_H
> +#define __NWFILTER_DHCPSNOOP_H
> +
> +int virNWFilterDHCPSnoopInit(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 */
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index ffb4b5d..d014a19 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/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();
> +    virNWFilterDHCPSnoopEnd(0);
>      virNWFilterLearnShutdown();
>  
>      return -1;
> @@ -149,6 +153,7 @@ nwfilterDriverReload(void) {
>      conn = virConnectOpen("qemu:///system");
>  
>      if (conn) {
> +        virNWFilterDHCPSnoopEnd(0);
>          /* shut down all threads -- they will be restarted if necessary */
>          virNWFilterLearnThreadsTerminate(true);
>  
> @@ -203,6 +208,7 @@ nwfilterDriverShutdown(void) {
>  
>      virNWFilterConfLayerShutdown();
>      virNWFilterTechDriversShutdown();
> +    virNWFilterDHCPSnoopEnd(0);
>      virNWFilterLearnShutdown();
>  
>      nwfilterDriverLock(driverState);
> diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
> index fc71e7b..245adb0 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/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"
> @@ -42,6 +43,8 @@
>  #define NWFILTER_STD_VAR_MAC "MAC"
>  #define NWFILTER_STD_VAR_IP  "IP"
>  
> +#define NWFILTER_DFLT_LEARN  "any"
> +
>  static int _virNWFilterTeardownFilter(const char *ifname);
>  
>  
> @@ -662,6 +665,9 @@ virNWFilterInstantiate(const unsigned char *vmuuid ATTRIBUTE_UNUSED,
>      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 char *vmuuid ATTRIBUTE_UNUSED,
>      if (rc < 0)
>          goto err_exit;
>  
> +    lv = virHashLookup(vars->hashTable, "ip_learning");
> +    if (lv && lv->valType == NWFILTER_VALUE_TYPE_SIMPLE)
> +        learning = lv->u.simple.value;
> +    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 (c_strcasecmp(learning, "none") == 0) {        /* no learning */
> +                reportIP = true;
> +                goto err_unresolvable_vars;
>              }
> -            goto err_exit;
> -        }
> -        goto err_unresolvable_vars;
> +            if (c_strcasecmp(learning, "dhcp") == 0) {
> +                rc = virNWFilterDHCPSnoopReq(techdriver, ifname, linkdev,
> +                                             nettype, vmuuid, macaddr,
> +                                             filter->name, vars, driver);
> +                goto err_exit;
> +            } else if (c_strcasecmp(learning, "any") == 0) {
> +                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 *ifname)
>          return -1;
>      }
>  
> +    virNWFilterDHCPSnoopEnd(ifname);
> +
>      virNWFilterTerminateLearnReq(ifname);
>  
>      if (virNWFilterLockIface(ifname) < 0)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]