[libvirt] [libvirt PATCHv4 1/2] add DHCP snooping
Stefan Berger
stefanb at linux.vnet.ibm.com
Tue Oct 25 12:50:09 UTC 2011
On 10/24/2011 07:12 PM, David L Stevens wrote:
> This patch adds DHCP Snooping support to libvirt.
>
> Signed-off-by: David L Stevens<dlstevens at us.ibm.com>
> ---
> examples/xml/nwfilter/no-ip-spoofing.xml | 5 +
> src/Makefile.am | 2 +
> src/nwfilter/nwfilter_dhcpsnoop.c | 705 ++++++++++++++++++++++++++++++
> src/nwfilter/nwfilter_dhcpsnoop.h | 38 ++
> src/nwfilter/nwfilter_driver.c | 5 +
> src/nwfilter/nwfilter_gentech_driver.c | 51 ++-
> 6 files changed, 793 insertions(+), 13 deletions(-)
> create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.c
> create mode 100644 src/nwfilter/nwfilter_dhcpsnoop.h
>
> diff --git a/examples/xml/nwfilter/no-ip-spoofing.xml b/examples/xml/nwfilter/no-ip-spoofing.xml
> index b8c94c8..e5969a0 100644
> --- a/examples/xml/nwfilter/no-ip-spoofing.xml
> +++ b/examples/xml/nwfilter/no-ip-spoofing.xml
> @@ -4,4 +4,9 @@
> <rule action='drop' direction='out'>
> <ip match='no' srcipaddr='$IP' />
> </rule>
> +<!-- allow DHCP requests -->
> +<rule action='return' direction='out'>
> +<ip match='yes' srcipaddr='0.0.0.0' protocol='udp' srcportstart='68'
> + srcportend='68' />
> +</rule>
> </filter>
Is this change necessary? Is it needed for the first instantiation of
the rules or is it needed to support the case when all IP addresses have
expired? I am asking because below I saw you calling
+ if (req->techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL)) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "ipl_ldel failed");
+ }
which does a similiar thing. If it was necessary, couldn't you just
reference the allow-dhcp filter?
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 87d91ed..c948cbf 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -481,6 +481,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..b77e2b0
> --- /dev/null
> +++ b/src/nwfilter/nwfilter_dhcpsnoop.c
> @@ -0,0 +1,705 @@
> +/*
> + * 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 at us.ibm.com>
> + * Based in part on work by Stefan Berger<stefanb at us.ibm.com>
> + */
> +
> +#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 "interface.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"
> +
> +#define VIR_FROM_THIS VIR_FROM_NWFILTER
> +
> +#ifdef HAVE_LIBPCAP
> +
> +static virHashTablePtr SnoopReqs;
> +static pthread_mutex_t SnoopLock;
> +
> +#define snoop_lock() { pthread_mutex_lock(&SnoopLock); }
> +#define snoop_unlock() { pthread_mutex_unlock(&SnoopLock); }
> +
> +struct virNWFilterSnoopReq {
> + virNWFilterTechDriverPtr techdriver;
> + const char *ifname;
> + int ifindex;
> + const char *linkdev;
> + enum virDomainNetType nettype;
> + unsigned char macaddr[VIR_MAC_STRING_BUFLEN];
> + const char *filtername;
> + virNWFilterHashTablePtr vars;
> + virNWFilterDriverStatePtr driver;
> + pthread_t thread;
> + /* start and end of lease list, ordered by lease time */
> + struct iplease *start;
> + struct iplease *end;
> + bool die;
> +};
> +
> +#define POLL_INTERVAL 10*1000 /* 10 secs */
> +
> +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;
> +};
> +
> +static struct iplease *ipl_getbyip(struct iplease *start, uint32_t ipaddr);
> +static void ipl_update(struct iplease *pl, uint32_t timeout);
> +
> +
> +/*
> + * 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) {
> + plnew->ipl_prev = plnew->ipl_next = 0;
> + *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);
> +}
> +
> +/*
> + * ipl_add - create or update an IP lease
> + */
> +static void
> +ipl_add(struct iplease *plnew)
> +{
> + struct iplease *pl;
> + int rc;
> + char ipbuf[20]; /* dotted decimal IP addr string */
> + char *ipstr;
> +
> + pl = ipl_getbyip(plnew->ipl_req->start, plnew->ipl_ipaddr);
> + if (pl) {
> + ipl_update(pl, plnew->ipl_timeout);
> + 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 (plnew->ipl_req->start)
> + return; /* silently ignore multiple addresses */
> +
> + if (VIR_ALLOC(pl)< 0) {
> + virReportOOMError();
> + return;
> + }
> + *pl = *plnew;
> +
> + if (!inet_ntop(AF_INET,&pl->ipl_ipaddr, ipbuf, sizeof(ipbuf))) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("ipl_add inet_ntop " "failed (0x%08X)"),
> + pl->ipl_ipaddr);
> + VIR_FREE(pl);
> + return;
> +
> + }
> +
> + ipstr = strdup(ipbuf);
> + if (!ipstr) {
> + VIR_FREE(pl);
> + virReportOOMError();
> + return;
> + }
> + if (virNWFilterHashTablePut(pl->ipl_req->vars, "IP", ipstr, 1)) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not add variable \"IP\" to hashmap"));
> + VIR_FREE(ipstr);
> + VIR_FREE(pl);
> + return;
> + }
> + rc = virNWFilterInstantiateFilterLate(NULL,
> + pl->ipl_req->ifname,
> + pl->ipl_req->ifindex,
> + pl->ipl_req->linkdev,
> + pl->ipl_req->nettype,
> + pl->ipl_req->macaddr,
> + pl->ipl_req->filtername,
> + pl->ipl_req->vars,
> + pl->ipl_req->driver);
> + if (rc) {
> + VIR_FREE(ipstr);
> + VIR_FREE(pl);
> + return;
> + }
> + ipl_tadd(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 iplease *ipl)
> +{
> + struct virNWFilterSnoopReq *req;
> +
> + if (!ipl)
> + return;
> +
> + req = ipl->ipl_req;
> +
> + ipl_tdel(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)) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "ipl_ldel failed");
> + }
> + VIR_FREE(ipl);
> +}
> +
> +/*
> + * ipl_update - update the timeout on an IP lease
> + */
> +static void
> +ipl_update(struct iplease *ipl, uint32_t timeout)
> +{
> + 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;
> +}
> +
> +#define GRACE 5
> +
> +/*
> + * 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->start);
> + return 0;
> +}
> +
> +typedef unsigned char Eaddr[6];
> +
> +struct eth {
> + Eaddr eh_dst;
> + Eaddr eh_src;
> + unsigned short eh_type;
> + unsigned char eh_data[0];
> +};
The below algorithm also seems to assume that the VM does not send
802.1Q (VLAN) headers. I remember having had problems when trying to
receive 802.1Q headers when the VM's interface is on a bridge and the
remote traffic comes through the network. I wonder whether it generally
doesn't work. I think at least in the code you should look at the
header, check for ETHERTYPE_IPv4 and then use eh_data[0], otherwise
either discard it or if ETHERTYPE_VLAN (0x8100) is used read the
encapsulated protocol ID and make sure ETHERTYPE_IPv4 is found there and
then use eh_data[4] for further processing.
> +
> +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;
> + for (oind = 0; oind< sizeof dhcp_magic; ++oind)
> + if (pd->d_opts[oind] != dhcp_magic[oind])
> + return -1; /* bad magic */
> +
memcmp?
> + 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, *pipl;
> + int mtype, leasetime;
> +
> + /* go through the protocol headers */
> + 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);
> + break;
> + case DHCPDECLINE:
> + case DHCPRELEASE:
> + pipl = ipl_getbyip(req->start, ipl.ipl_ipaddr);
> + ipl_del(pipl);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +#define PBUFSIZE 576 /*>= IP/TCP/DHCP headers */
> +
> +static pcap_t *
> +dhcpopen(const char *intf)
> +{
> + pcap_t *handle;
> + struct bpf_program fp;
> + char filter[64];
> + char pcap_errbuf[PCAP_ERRBUF_SIZE];
> +
> + 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;
> +
> + /* free all leases */
> + snoop_lock();
> + for (ipl = req->start; ipl; ipl = req->start)
> + ipl_del(ipl);
> + snoop_unlock();
> +
> + /* free all req data */
> + if (req->ifname)
> + VIR_FREE(req->ifname);
> + if (req->linkdev)
> + VIR_FREE(req->linkdev);
> + if (req->filtername)
> + VIR_FREE(req->filtername);
> + if (req->vars)
> + virNWFilterHashTableFree(req->vars);
A 'make syntax-check' will probably flag those... don't check for NULL
before free().
> + VIR_FREE(req);
> +}
> +
> +static void *
> +virNWFilterDHCPSnoop(void *req0)
> +{
> + struct virNWFilterSnoopReq *req = req0;
> + pcap_t *handle;
> + struct pcap_pkthdr hdr;
> + struct eth *packet;
> + int ifindex;
> +
> + handle = dhcpopen(req->ifname);
> + if (!handle)
> + return 0;
> +
> + ifindex = if_nametoindex(req->ifname);
> +
> + while (1) {
> + if (req->die)
> + break;
> + snoop_lock();
> + ipl_trun(req);
> + snoop_unlock();
> +
> + packet = (struct eth *) pcap_next(handle,&hdr);
> +
> + if (!packet) {
> + if (ifaceCheck(false, req->ifname, NULL, ifindex))
> + virNWFilterDHCPSnoopEnd(req->ifname);
> + continue;
> + }
> +
> + snoop_lock();
> + dhcpdecode(req, packet, hdr.caplen);
> + snoop_unlock();
> + }
> + snoopReqFree(req);
> + return 0;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> + const char *ifname,
> + int ifindex,
> + const char *linkdev,
> + enum virDomainNetType nettype,
> + const unsigned char *macaddr,
> + const char *filtername,
> + virNWFilterHashTablePtr filterparams,
> + virNWFilterDriverStatePtr driver)
> +{
> + struct virNWFilterSnoopReq *req;
> +
> + snoop_lock();
> + req = virHashLookup(SnoopReqs, ifname);
> + snoop_unlock();
> + if (req)
> + return 0;
> +
> + if (VIR_ALLOC(req)< 0) {
> + virReportOOMError();
> + return 1;
> + }
> +
> + req->techdriver = techdriver;
> + req->ifname = strdup(ifname);
> + if (req->ifname == 0) {
compare a string against NULL rather than 0.
> + snoopReqFree(req);
> + virReportOOMError();
> + return 1;
> + }
> + req->ifindex = ifindex;
> + req->linkdev = linkdev ? strdup(linkdev) : 0;
> + req->nettype = nettype;
> + memcpy(req->macaddr, macaddr, sizeof(req->macaddr));
> + req->filtername = strdup(filtername);
> + if (req->filtername == 0) {
> + snoopReqFree(req);
> + virReportOOMError();
> + return 1;
> + }
> +
> + if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL)) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, "applyDHCPOnlyRules "
> + "failed - spoofing not protected!");
> + }
> +
> + req->vars = virNWFilterHashTableCreate(0);
> + if (!req->vars) {
> + snoopReqFree(req);
> + virReportOOMError();
> + return 1;
> + }
> + if (virNWFilterHashTablePutAll(filterparams, req->vars)) {
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("virNWFilterDHCPSnoopReq: can't copy variables"
> + " on if %s"), ifname);
> + snoopReqFree(req);
> + return 1;
> + }
> + req->driver = driver;
> +
> + snoop_lock();
> + if (virHashAddEntry(SnoopReqs, ifname, req)) {
> + snoop_unlock();
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("virNWFilterDHCPSnoopReq req add failed on"
> + "interface \"%s\""), ifname);
> + snoopReqFree(req);
> + return 1;
> + }
> + snoop_unlock();
> + if (pthread_create(&req->thread, NULL, virNWFilterDHCPSnoop, req) != 0) {
> + (void) virHashRemoveEntry(SnoopReqs, ifname);
> + snoopReqFree(req);
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("virNWFilterDHCPSnoopReq pthread_create failed"
> + " oninterface \"%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;
> +
> + req->die = 1;
> +}
> +
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> + if (SnoopReqs)
> + return 0;
> +
> + if (pthread_mutex_init(&SnoopLock, 0)< 0)
> + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
> + _("pthread_mutex_init: %s"),
> + strerror(errno));
Return -1 in case of failure?
> + snoop_lock();
> + SnoopReqs = virHashCreate(0, freeReq);
> + if (!SnoopReqs) {
> + snoop_unlock();
> + virReportOOMError();
> + return -1;
> + }
> + snoop_unlock();
> + return 0;
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname)
> +{
> + snoop_lock();
> + if (!SnoopReqs) {
> + snoop_unlock();
> + return;
> + }
> + if (ifname)
> + virHashRemoveEntry(SnoopReqs, ifname);
> + else /* free all of them */
> + virHashFree(SnoopReqs);
> + snoop_unlock();
> +}
> +
> +#else /* HAVE_LIBPCAP */
> +int
> +virNWFilterDHCPSnoopInit(void)
> +{
> + return;
Should return a value. Please test it.
> +}
> +
> +void
> +virNWFilterDHCPSnoopEnd(const char *ifname ATTRIBUTE_UNUSED)
> +{
> + return;
> +}
> +
> +int
> +virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
> + const char *ifname ATTRIBUTE_UNUSED,
> + int ifindex ATTRIBUTE_UNUSED,
> + const char *linkdev ATTRIBUTE_UNUSED,
> + enum virDomainNetType nettype 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..3393157
> --- /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 at us.ibm.com>
> + */
> +
> +#ifndef __NWFILTER_DHCPSNOOP_H
> +# define __NWFILTER_DHCPSNOOP_H
> +
> +int virNWFilterDHCPSnoopInit(void);
> +int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
> + const char *ifname,
> + int ifindex,
> + const char *linkdev,
> + enum virDomainNetType nettype,
> + 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 a735059..57eb52c 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;
> @@ -201,6 +205,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 7891983..2a9773c 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.c
> +++ b/src/nwfilter/nwfilter_gentech_driver.c
> @@ -33,6 +33,7 @@
> #include "virterror_internal.h"
> #include "nwfilter_gentech_driver.h"
> #include "nwfilter_ebiptables_driver.h"
> +#include "nwfilter_dhcpsnoop.h"
> #include "nwfilter_learnipaddr.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);
>
>
> @@ -638,6 +641,8 @@ virNWFilterInstantiate(virConnectPtr conn,
> void **ptrs = NULL;
> int instantiate = 1;
> char *buf;
> + const char *learning = NWFILTER_DFLT_LEARN;
> + bool reportIP = false;
>
> virNWFilterHashTablePtr missing_vars = virNWFilterHashTableCreate(0);
> if (!missing_vars) {
> @@ -655,22 +660,40 @@ virNWFilterInstantiate(virConnectPtr conn,
> if (rc)
> goto err_exit;
>
> + learning = virHashLookup(vars->hashTable, "ip_learning");
> +
Can you add this as documentation to the nwfilter documentation page?
> 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 (strcasecmp(learning, "none") == 0) { /* no learning */
> + reportIP = true;
> + goto err_unresolvable_vars;
> }
> - goto err_exit;
> - }
> - goto err_unresolvable_vars;
> + if (strcasecmp(learning, "dhcp") == 0) {
> + rc = virNWFilterDHCPSnoopReq(techdriver, ifname, ifindex,
> + linkdev, nettype, macaddr,
> + filter->name, vars, driver);
> + goto err_exit;
> + } else if (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&&
> @@ -738,7 +761,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 "
> @@ -1070,6 +1093,8 @@ _virNWFilterTeardownFilter(const char *ifname)
> return 1;
> }
>
> + virNWFilterDHCPSnoopEnd(ifname);
> +
> virNWFilterTerminateLearnReq(ifname);
>
> if (virNWFilterLockIface(ifname))
I'll try it.
Stefan
More information about the libvir-list
mailing list