[libvirt] [libvirt PATCHv8 1/1] add DHCP snooping

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 v7: - renamed functions as suggested - collected local state into "virNWFilterSnoopState" struct - cleaned up include file list - misc code cleanups per review comments Signed-off-by: David L Stevens <dlstevens@us.ibm.com> --- docs/formatnwfilter.html.in | 17 + po/POTFILES.in | 1 + src/Makefile.am | 2 + src/nwfilter/nwfilter_dhcpsnoop.c | 1197 ++++++++++++++++++++++++++++++++ src/nwfilter/nwfilter_dhcpsnoop.h | 38 + src/nwfilter/nwfilter_driver.c | 6 + src/nwfilter/nwfilter_gentech_driver.c | 59 ++- 7 files changed, 1307 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/po/POTFILES.in b/po/POTFILES.in index 88be04e..d4b0a86 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -51,6 +51,7 @@ src/node_device/node_device_hal.c src/node_device/node_device_linux_sysfs.c src/node_device/node_device_udev.c src/nodeinfo.c +src/nwfilter/nwfilter_dhcpsnoop.c src/nwfilter/nwfilter_driver.c src/nwfilter/nwfilter_ebiptables_driver.c src/nwfilter/nwfilter_gentech_driver.c 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..8c2ff50 --- /dev/null +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -0,0 +1,1197 @@ +/* + * 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@us.ibm.com> + * Based in part on work by Stefan Berger <stefanb@us.ibm.com> + */ + +#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 "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" + +static struct { +/* lease file */ + int LeaseFD; + int nLeases; /* number of active leases */ + int wLeases; /* number of written leases */ +/* thread management */ + virHashTablePtr SnoopReqs; + virHashTablePtr IfnameToKey; + virHashTablePtr Active; + virMutex SnoopLock; + virMutex ActiveLock; +} virNWFilterSnoopState = { + .LeaseFD = -1, + .SnoopLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, + .ActiveLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, +}; + +#define virNWFilterSnoopLock() \ + { virMutexLock(&virNWFilterSnoopState.SnoopLock); } +#define virNWFilterSnoopUnlock() \ + { virMutexUnlock(&virNWFilterSnoopState.SnoopLock); } +#define virNWFilterSnoopLockActive() \ + { virMutexLock(&virNWFilterSnoopState.ActiveLock); } +#define virNWFilterSnoopUnlockActive() \ + { virMutexUnlock(&virNWFilterSnoopState.ActiveLock); } + +static char * +virNWFilterSnoopActivate(virThreadPtr thread) +{ + char *key, *data; + unsigned long threadID = (unsigned long int)thread->thread; + + if (virAsprintf(&key, "0x%0*lX", (int)sizeof(threadID)*2, threadID) < 0) { + virReportOOMError(); + return NULL; + } + + virNWFilterSnoopLockActive(); + data = strdup("1"); + if (data == NULL) { + virReportOOMError(); + VIR_FREE(key); + } else if (virHashAddEntry(virNWFilterSnoopState.Active, key, data)) { + VIR_FREE(key); + VIR_FREE(data); + } + virNWFilterSnoopUnlockActive(); + return key; +} + +static void +virNWFilterSnoopCancel(char **ThreadKey) +{ + if (*ThreadKey == NULL) + return; + + virNWFilterSnoopLockActive(); + (void) virHashRemoveEntry(virNWFilterSnoopState.Active, *ThreadKey); + *ThreadKey = NULL; + virNWFilterSnoopUnlockActive(); +} + +static bool +virNWFilterSnoopIsActive(char *ThreadKey) +{ + void *entry; + + if (ThreadKey == NULL) + return 0; + virNWFilterSnoopLockActive(); + entry = virHashLookup(virNWFilterSnoopState.Active, ThreadKey); + virNWFilterSnoopUnlockActive(); + return entry != NULL; +} + +#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 virNWFilterSnoopIPLease *start; + struct virNWFilterSnoopIPLease *end; + char *threadkey; +}; + +#define POLL_INTERVAL 10*1000 /* 10 secs */ +#define MAXERRS 25 /* retries on failing device */ + +struct virNWFilterSnoopIPLease { + uint32_t IPAddress; + uint32_t IPServer; + struct virNWFilterSnoopReq *SnoopReq; + unsigned int Timeout; + /* timer list */ + struct virNWFilterSnoopIPLease *prev; + struct virNWFilterSnoopIPLease *next; +}; + +static struct virNWFilterSnoopIPLease * + virNWFilterSnoopGetByIP(struct virNWFilterSnoopIPLease *start, + uint32_t ipaddr); +static void virNWFilterSnoopUpdateLease(struct virNWFilterSnoopIPLease *pl, + time_t timeout); + +static struct virNWFilterSnoopReq *virNWFilterSnoopNewReq(const char *ifkey); + +static void virNWFilterSnoopLeaseFileOpen(void); +static void virNWFilterSnoopLeaseFileClose(void); +static void virNWFilterSnoopLeaseFileLoad(void); +static void virNWFilterSnoopLeaseFileSave(struct virNWFilterSnoopIPLease *ipl); +static void virNWFilterSnoopLeaseFileRestore(struct virNWFilterSnoopReq *req); +static void virNWFilterSnoopLeaseFileRefresh(void); + +/* + * virNWFilterSnoopListAdd - add an IP lease to a list + */ +static void +virNWFilterSnoopListAdd(struct virNWFilterSnoopIPLease *plnew, + struct virNWFilterSnoopIPLease **start, + struct virNWFilterSnoopIPLease **end) +{ + struct virNWFilterSnoopIPLease *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; +} + +/* + * virNWFilterSnoopTimerAdd - add an IP lease to the timer list + */ +static void +virNWFilterSnoopTimerAdd(struct virNWFilterSnoopIPLease *plnew) +{ + struct virNWFilterSnoopReq *req = plnew->SnoopReq; + + virNWFilterSnoopListAdd(plnew, &req->start, &req->end); +} + +/* + * virNWFilterSnoopInstallRule - install rule for a lease + */ +static int +virNWFilterSnoopInstallRule(struct virNWFilterSnoopIPLease *ipl) +{ + char ipbuf[INET_ADDRSTRLEN]; + int rc; + virNWFilterVarValuePtr ipVar; + + if (!inet_ntop(AF_INET, &ipl->IPAddress, ipbuf, sizeof(ipbuf))) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterSnoopInstallRule inet_ntop failed " + " (0x%08X)"), ipl->IPAddress); + return -1; + } + ipVar = virNWFilterVarValueCreateSimpleCopyValue(ipbuf); + if (!ipVar) { + virReportOOMError(); + return -1; + } + if (virNWFilterHashTablePut(ipl->SnoopReq->vars, "IP", ipVar, 1)) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not add variable \"IP\" to hashmap")); + virNWFilterVarValueFree(ipVar); + return -1; + } + rc = virNWFilterInstantiateFilterLate(NULL, + ipl->SnoopReq->ifname, + ipl->SnoopReq->ifindex, + ipl->SnoopReq->linkdev, + ipl->SnoopReq->nettype, + ipl->SnoopReq->macaddr, + ipl->SnoopReq->filtername, + ipl->SnoopReq->vars, + ipl->SnoopReq->driver); + if (rc) + return -1; + return 0; +} + +/* + * virNWFilterSnoopLeaseAdd - create or update an IP lease + */ +static void +virNWFilterSnoopLeaseAdd(struct virNWFilterSnoopIPLease *plnew, + bool update_leasefile) +{ + struct virNWFilterSnoopIPLease *pl; + struct virNWFilterSnoopReq *req = plnew->SnoopReq; + + pl = virNWFilterSnoopGetByIP(req->start, plnew->IPAddress); + if (pl) { + virNWFilterSnoopUpdateLease(pl, plnew->Timeout); + if (update_leasefile) + virNWFilterSnoopLeaseFileSave(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 && virNWFilterSnoopInstallRule(pl) < 0) { + VIR_FREE(pl); + return; + } + virNWFilterSnoopTimerAdd(pl); + virNWFilterSnoopState.nLeases++; + if (update_leasefile) + virNWFilterSnoopLeaseFileSave(pl); +} + +/* + * virNWFilterSnoopListDel - remove an IP lease from a list + */ +static void +virNWFilterSnoopListDel(struct virNWFilterSnoopIPLease *ipl, + struct virNWFilterSnoopIPLease **start, + struct virNWFilterSnoopIPLease **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; +} + +/* + * virNWFilterSnoopTimerDel - remove an IP lease from the timer list + */ +static void +virNWFilterSnoopTimerDel(struct virNWFilterSnoopIPLease *ipl) +{ + struct virNWFilterSnoopReq *req = ipl->SnoopReq; + + virNWFilterSnoopListDel(ipl, &req->start, &req->end); + ipl->Timeout = 0; +} + +/* + * virNWFilterSnoopLeaseDel - delete an IP lease + */ +static void +virNWFilterSnoopLeaseDel(struct virNWFilterSnoopReq *req, + uint32_t ipaddr, bool update_leasefile) +{ + struct virNWFilterSnoopIPLease *ipl; + + ipl = virNWFilterSnoopGetByIP(req->start, ipaddr); + if (ipl == NULL) + return; + + virNWFilterSnoopTimerDel(ipl); + + if (update_leasefile) { + virNWFilterSnoopLeaseFileSave(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, + _("virNWFilterSnoopListDel failed")); + } + VIR_FREE(ipl); + virNWFilterSnoopState.nLeases--; +} + +/* + * virNWFilterSnoopUpdateLease - update the timeout on an IP lease + */ +static void +virNWFilterSnoopUpdateLease(struct virNWFilterSnoopIPLease *ipl, time_t timeout) +{ + if (timeout < ipl->Timeout) + return; /* no take-backs */ + virNWFilterSnoopTimerDel(ipl); + ipl->Timeout = timeout; + virNWFilterSnoopTimerAdd(ipl); + return; +} + +/* + * virNWFilterSnoopGetByIP - lookup IP lease by IP address + */ +static struct virNWFilterSnoopIPLease * +virNWFilterSnoopGetByIP(struct virNWFilterSnoopIPLease *start, uint32_t ipaddr) +{ + struct virNWFilterSnoopIPLease *pl; + + for (pl = start; pl && pl->IPAddress != ipaddr; pl = pl->next) + /* empty */ ; + return pl; +} + +/* + * virNWFilterSnoopTimerRun - run the IP lease timeout list + */ +static unsigned int +virNWFilterSnoopTimerRun(struct virNWFilterSnoopReq *req) +{ + time_t now; + + now = time(0); + while (req->start && req->start->Timeout <= now) + virNWFilterSnoopLeaseDel(req, req->start->IPAddress, 1); + return 0; +} + +typedef unsigned char Eaddr[6]; + +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 + +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 + +static const unsigned char dhcp_magic[4] = { 99, 130, 83, 99 }; + +static int +virNWFilterSnoopDHCPGetOpt(struct virNWFilterSnoopDHCPHdr *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 +virNWFilterSnoopDHCPDecode(struct virNWFilterSnoopReq *req, + struct virNWFilterSnoopEthHdr *pep, + int len) +{ + struct iphdr *pip; + struct udphdr *pup; + struct virNWFilterSnoopDHCPHdr *pd; + struct 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(struct virNWFilterSnoopEthHdr, eh_data); + break; + case ETHERTYPE_VLAN: + if (ntohs(pep->ehv_type) != ETHERTYPE_IP) + return; + pip = (struct iphdr *) pep->ehv_data; + len -= offsetof(struct virNWFilterSnoopEthHdr, ehv_data); + break; + default: + return; + } + pip = (struct iphdr *) pep->eh_data; + len -= sizeof(*pep); + if (len < 0) + return; + pup = (struct udphdr *) ((char *) pip + (pip->ihl << 2)); + len -= pip->ihl << 2; + if (len < 0) + return; + pd = (struct virNWFilterSnoopDHCPHdr *) ((char *) pup + sizeof(*pup)); + len -= sizeof(*pup); + if (len < 0) + return; /* invalid packet length */ + if (virNWFilterSnoopDHCPGetOpt(pd, len, &mtype, &leasetime) < 0) + return; + + 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; + + switch (mtype) { + case DHCPACK: + virNWFilterSnoopLeaseAdd(&ipl, 1); + break; + case DHCPDECLINE: + case DHCPRELEASE: + virNWFilterSnoopLeaseDel(req, ipl.IPAddress, 1); + break; + default: + break; + } +} + +#define PBUFSIZE 576 /* >= IP/TCP/DHCP headers */ +#define TIMEOUT 30 /* secs */ + +static pcap_t * +virNWFilterSnoopDHCPOpen(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)); + pcap_freecode(&fp); + return 0; + } + pcap_freecode(&fp); + return handle; +} + +static void +virNWFilterSnoopReqFree(struct virNWFilterSnoopReq *req) +{ + struct virNWFilterSnoopIPLease *ipl; + + if (!req) + return; + + /* free all leases */ + for (ipl = req->start; ipl; ipl = req->start) + virNWFilterSnoopLeaseDel(req, ipl->IPAddress, 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 virNWFilterSnoopEthHdr *packet; + int ifindex; + int errcount; + char *threadkey; + virThread thread; + + handle = virNWFilterSnoopDHCPOpen(req->ifname); + if (!handle) + return; + + virThreadSelf(&thread); + req->threadkey = virNWFilterSnoopActivate(&thread); + threadkey = strdup(req->threadkey); + if (threadkey == NULL) { + virReportOOMError(); + pcap_close(handle); + return; + } + + ifindex = if_nametoindex(req->ifname); + + virNWFilterSnoopLock(); + virNWFilterSnoopLeaseFileRestore(req); + virNWFilterSnoopUnlock(); + + errcount = 0; + while (1) { + int rv; + + virNWFilterSnoopLock(); + virNWFilterSnoopTimerRun(req); + virNWFilterSnoopUnlock(); + + rv = pcap_next_ex(handle, &hdr, (const u_char **)&packet); + + if (!virNWFilterSnoopIsActive(threadkey)) { + VIR_FREE(threadkey); + pcap_close(handle); + return; + } + 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 = virNWFilterSnoopDHCPOpen(req->ifname); + if (!handle) + break; + } + continue; + } + errcount = 0; + if (rv) { + virNWFilterSnoopLock(); + virNWFilterSnoopDHCPDecode(req, packet, hdr->caplen); + virNWFilterSnoopUnlock(); + } + } + virNWFilterSnoopCancel(&req->threadkey); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, req->ifname); + VIR_FREE(req->ifname); + /* if we still have a valid lease, keep the req for restarts */ + if (!req->start || req->start->Timeout < time(0)) + (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, req->ifkey); + VIR_FREE(threadkey); + pcap_close(handle); + 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) +{ + struct virNWFilterSnoopReq *req; + bool isnewreq; + char ifkey[VIR_IFKEY_LEN]; + virThread thread; + + virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); + virNWFilterSnoopLock(); + req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + isnewreq = req == NULL; + if (!isnewreq) { + if (req->threadkey) { + virNWFilterSnoopUnlock(); + return 0; + } + } else { + req = virNWFilterSnoopNewReq(ifkey); + if (!req) { + virNWFilterSnoopUnlock(); + 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->ifname == NULL || req->filtername == NULL) { + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(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) { + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + virReportOOMError(); + return -1; + } + if (virNWFilterHashTablePutAll(filterparams, req->vars)) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterDHCPSnoopReq: can't copy variables" + " on if %s"), ifkey); + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + return -1; + } + req->driver = driver; + + if (virHashAddEntry(virNWFilterSnoopState.IfnameToKey, ifname, + req->ifkey)) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterDHCPSnoopReq ifname map failed" + " on interface \"%s\" key \"%s\""), ifname, + ifkey); + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + return -1; + } + if (isnewreq && + virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterDHCPSnoopReq req add failed on" + " interface \"%s\" ifkey \"%s\""), ifname, + ifkey); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + return -1; + } + virNWFilterSnoopUnlock(); + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoop, req) != 0) { + virNWFilterSnoopLock(); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); + (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, ifkey); + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterDHCPSnoopReq virThreadCreate " + "failed on interface \"%s\""), ifname); + return -1; + } + return 0; +} + +/* + * virNWFilterSnoopReqRelease - hash table free function to kill a request + */ +static void +virNWFilterSnoopReqRelease(void *req0, const void *name ATTRIBUTE_UNUSED) +{ + struct virNWFilterSnoopReq *req = (struct virNWFilterSnoopReq *) req0; + + if (!req) + return; + + if (req->threadkey) + virNWFilterSnoopCancel(&req->threadkey); + virNWFilterSnoopReqFree(req); +} + +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); +} + +int +virNWFilterDHCPSnoopInit(void) +{ + if (virNWFilterSnoopState.SnoopReqs) + return 0; + + virNWFilterSnoopLock(); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); + virNWFilterSnoopState.SnoopReqs = + virHashCreate(0, virNWFilterSnoopReqRelease); + if (!virNWFilterSnoopState.IfnameToKey || + !virNWFilterSnoopState.SnoopReqs) { + virNWFilterSnoopUnlock(); + virReportOOMError(); + goto errexit; + } + virNWFilterSnoopLeaseFileLoad(); + virNWFilterSnoopLeaseFileOpen(); + + virNWFilterSnoopUnlock(); + + virNWFilterSnoopLockActive(); + virNWFilterSnoopState.Active = virHashCreate(0, 0); + if (!virNWFilterSnoopState.Active) { + virNWFilterSnoopUnlockActive(); + virReportOOMError(); + goto errexit; + } + virNWFilterSnoopUnlockActive(); + return 0; + +errexit: + if (virNWFilterSnoopState.IfnameToKey) { + virHashFree(virNWFilterSnoopState.IfnameToKey); + virNWFilterSnoopState.IfnameToKey = 0; + } + if (virNWFilterSnoopState.SnoopReqs) { + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopState.SnoopReqs = 0; + } + if (virNWFilterSnoopState.Active) { + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopState.Active = 0; + } + return -1; +} + +void +virNWFilterDHCPSnoopEnd(const char *ifname) +{ + char *ifkey = NULL; + + virNWFilterSnoopLock(); + if (!virNWFilterSnoopState.SnoopReqs) { + virNWFilterSnoopUnlock(); + return; + } + + if (ifname) { + ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey,ifname); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); + if (!ifkey) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("ifname \"%s\" not in key map"), ifname); + virNWFilterSnoopUnlock(); + return; + } + } + + if (ifkey) { + struct virNWFilterSnoopReq *req; + + req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + if (!req) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("ifkey \"%s\" has no req"), ifkey); + virNWFilterSnoopUnlock(); + return; + } + if (!req->start || req->start->Timeout < time(0)) { + (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, + req->ifkey); + virNWFilterSnoopUnlock(); + return; + } + /* keep valid lease req; drop interface association */ + virNWFilterSnoopCancel(&req->threadkey); + VIR_FREE(req->ifname); + } else { /* free all of them */ + virNWFilterSnoopLeaseFileClose(); + virHashFree(virNWFilterSnoopState.IfnameToKey); + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, 0); + if (!virNWFilterSnoopState.IfnameToKey) { + virNWFilterSnoopUnlock(); + virReportOOMError(); + return; + } + virNWFilterSnoopState.SnoopReqs = + virHashCreate(0, virNWFilterSnoopReqRelease); + if (!virNWFilterSnoopState.SnoopReqs) { + virHashFree(virNWFilterSnoopState.IfnameToKey); + virNWFilterSnoopUnlock(); + virReportOOMError(); + return; + } + virNWFilterSnoopLeaseFileLoad(); + } + virNWFilterSnoopUnlock(); +} + +static int +virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, + struct virNWFilterSnoopIPLease *ipl) +{ + char lbuf[256], ipstr[INET_ADDRSTRLEN], dhcpstr[INET_ADDRSTRLEN]; + + 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); + 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 +virNWFilterSnoopLeaseFileSave(struct virNWFilterSnoopIPLease *ipl) +{ + struct virNWFilterSnoopReq *req = ipl->SnoopReq; + + if (virNWFilterSnoopState.LeaseFD < 0) + virNWFilterSnoopLeaseFileOpen(); + if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.LeaseFD, + req->ifkey, ipl) < 0) + return; + /* keep dead leases at < ~95% of file size */ + if (++virNWFilterSnoopState.wLeases >= virNWFilterSnoopState.nLeases*20) + virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ +} + +static struct virNWFilterSnoopReq * +virNWFilterSnoopNewReq(const char *ifkey) +{ + struct virNWFilterSnoopReq *req; + + if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN-1) { + virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, + _("virNWFilterSnoopNewReq called with invalid " + "key \"%s\" (%d)"), + ifkey ? ifkey : "", strlen(ifkey)); + return NULL; + } + if (VIR_ALLOC(req) < 0) { + virReportOOMError(); + return NULL; + } + if (virStrcpyStatic(req->ifkey, ifkey) == NULL) + VIR_FREE(req); + + return req; +} + +static void +virNWFilterSnoopSaveIter(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + struct virNWFilterSnoopReq *req = payload; + int tfd = *(int *)data; + struct virNWFilterSnoopIPLease *ipl; + + /* clean up orphaned, expired leases */ + if (!req->threadkey) { + time_t now; + + now = time(0); + for (ipl = req->start; ipl; ipl = ipl->next) + if (ipl->Timeout < now) + virNWFilterSnoopLeaseDel(req, ipl->IPAddress , 0); + if (!req->start) { + virNWFilterSnoopReqFree(req); + return; + } + } + for (ipl = req->start; ipl; ipl = ipl->next) + (void) virNWFilterSnoopLeaseFileWrite(tfd, req->ifkey, ipl); +} + +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) + 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); + } + virNWFilterSnoopState.wLeases = 0; + virNWFilterSnoopLeaseFileOpen(); +} + + +static void +virNWFilterSnoopLeaseFileLoad(void) +{ + char line[256], ifkey[VIR_IFKEY_LEN], ipstr[INET_ADDRSTRLEN], + srvstr[INET_ADDRSTRLEN]; + struct virNWFilterSnoopIPLease 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, + _("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 = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + if (!req) { + req = virNWFilterSnoopNewReq(ifkey); + if (!req) + break; + if (virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { + virNWFilterSnoopReqFree(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); + continue; + } + (void) inet_pton(AF_INET, srvstr, &ipl.IPServer); + ipl.SnoopReq = req; + + if (ipl.Timeout) + virNWFilterSnoopLeaseAdd(&ipl, 0); + else + virNWFilterSnoopLeaseDel(req, ipl.IPAddress, 0); + } + if (fp != NULL) + (void) fclose(fp); + virNWFilterSnoopLeaseFileRefresh(); +} + +static void +virNWFilterSnoopLeaseFileRestore(struct virNWFilterSnoopReq *req) +{ + struct virNWFilterSnoopIPLease *ipl; + + for (ipl=req->start; ipl; ipl=ipl->next) + (void) virNWFilterSnoopInstallRule(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@us.ibm.com> + */ + +#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..07ae33c 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) -- 1.7.6.5

On 03/30/2012 03:07 PM, 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).
I'd accept it with the following changes merged in: Cleanup some parts of the DHCP snooping v8 code addressing - naming consistency - memory leak - if-before-free not being necessary - separate shutdown function (for avoiding 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 Regards, Stefan --- po/POTFILES.in | 1 src/nwfilter/nwfilter_dhcpsnoop.c | 154 ++++++++++++++++++++------------------ src/nwfilter/nwfilter_dhcpsnoop.h | 1 src/nwfilter/nwfilter_driver.c | 6 - 4 files changed, 87 insertions(+), 75 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.c +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c @@ -75,15 +75,15 @@ static struct { { virMutexLock(&virNWFilterSnoopState.SnoopLock); } #define virNWFilterSnoopUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.SnoopLock); } -#define virNWFilterSnoopLockActive() \ +#define virNWFilterSnoopActiveLock() \ { virMutexLock(&virNWFilterSnoopState.ActiveLock); } -#define virNWFilterSnoopUnlockActive() \ +#define virNWFilterSnoopActiveUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.ActiveLock); } static char * virNWFilterSnoopActivate(virThreadPtr thread) { - char *key, *data; + char *key; unsigned long threadID = (unsigned long int)thread->thread; if (virAsprintf(&key, "0x%0*lX", (int)sizeof(threadID)*2, threadID) < 0) { @@ -91,16 +91,11 @@ virNWFilterSnoopActivate(virThreadPtr th return NULL; } - virNWFilterSnoopLockActive(); - data = strdup("1"); - if (data == NULL) { - virReportOOMError(); - VIR_FREE(key); - } else if (virHashAddEntry(virNWFilterSnoopState.Active, key, data)) { + virNWFilterSnoopActiveLock(); + if (virHashAddEntry(virNWFilterSnoopState.Active, key, (void *)0x1)) { VIR_FREE(key); - VIR_FREE(data); } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); return key; } @@ -110,10 +105,10 @@ virNWFilterSnoopCancel(char **ThreadKey) if (*ThreadKey == NULL) return; - virNWFilterSnoopLockActive(); + virNWFilterSnoopActiveLock(); (void) virHashRemoveEntry(virNWFilterSnoopState.Active, *ThreadKey); - *ThreadKey = NULL; - virNWFilterSnoopUnlockActive(); + VIR_FREE(*ThreadKey); + virNWFilterSnoopActiveUnlock(); } static bool @@ -123,9 +118,9 @@ virNWFilterSnoopIsActive(char *ThreadKey if (ThreadKey == NULL) return 0; - virNWFilterSnoopLockActive(); + virNWFilterSnoopActiveLock(); entry = virHashLookup(virNWFilterSnoopState.Active, ThreadKey); - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); return entry != NULL; } @@ -740,7 +735,9 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virThread thread; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); + virNWFilterSnoopLock(); + req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); isnewreq = req == NULL; if (!isnewreq) { @@ -763,11 +760,10 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->ifname = strdup(ifname); memcpy(req->macaddr, macaddr, sizeof(req->macaddr)); req->filtername = strdup(filtername); + if (req->ifname == NULL || req->filtername == NULL) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto err_exit; } if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL, @@ -778,18 +774,14 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->vars = virNWFilterHashTableCreate(0); if (!req->vars) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto err_exit; } if (virNWFilterHashTablePutAll(filterparams, req->vars)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq: can't copy variables" " on if %s"), ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto err_exit; } req->driver = driver; @@ -799,9 +791,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD _("virNWFilterDHCPSnoopReq ifname map failed" " on interface \"%s\" key \"%s\""), ifname, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto err_exit; } if (isnewreq && virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { @@ -810,23 +800,27 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD " interface \"%s\" ifkey \"%s\""), ifname, ifkey); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto err_exit; } - virNWFilterSnoopUnlock(); + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoop, req) != 0) { - virNWFilterSnoopLock(); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq virThreadCreate " "failed on interface \"%s\""), ifname); - return -1; + goto err_exit; } + + virNWFilterSnoopUnlock(); + return 0; + +err_exit: + virNWFilterSnoopUnlock(); + virNWFilterSnoopReqFree(req); + + return -1; } /* @@ -881,29 +875,28 @@ virNWFilterDHCPSnoopInit(void) virNWFilterSnoopUnlock(); - virNWFilterSnoopLockActive(); - virNWFilterSnoopState.Active = virHashCreate(0, 0); + virNWFilterSnoopActiveLock(); + + virNWFilterSnoopState.Active = virHashCreate(0, NULL); if (!virNWFilterSnoopState.Active) { - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); virReportOOMError(); goto errexit; } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); + return 0; errexit: - if (virNWFilterSnoopState.IfnameToKey) { - virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopState.IfnameToKey = 0; - } - if (virNWFilterSnoopState.SnoopReqs) { - virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.SnoopReqs = 0; - } - if (virNWFilterSnoopState.Active) { - virHashFree(virNWFilterSnoopState.Active); - virNWFilterSnoopState.Active = 0; - } + virHashFree(virNWFilterSnoopState.IfnameToKey); + virNWFilterSnoopState.IfnameToKey = NULL; + + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopState.SnoopReqs = NULL; + + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopState.Active = NULL; + return -1; } @@ -913,10 +906,8 @@ virNWFilterDHCPSnoopEnd(const char *ifna char *ifkey = NULL; virNWFilterSnoopLock(); - if (!virNWFilterSnoopState.SnoopReqs) { - virNWFilterSnoopUnlock(); - return; - } + if (!virNWFilterSnoopState.SnoopReqs) + goto cleanup; if (ifname) { ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey,ifname); @@ -924,8 +915,7 @@ virNWFilterDHCPSnoopEnd(const char *ifna if (!ifkey) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifname \"%s\" not in key map"), ifname); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } } @@ -936,14 +926,12 @@ virNWFilterDHCPSnoopEnd(const char *ifna if (!req) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifkey \"%s\" has no req"), ifkey); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } if (!req->start || req->start->Timeout < time(0)) { (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, req->ifkey); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } /* keep valid lease req; drop interface association */ virNWFilterSnoopCancel(&req->threadkey); @@ -952,23 +940,46 @@ virNWFilterDHCPSnoopEnd(const char *ifna virNWFilterSnoopLeaseFileClose(); virHashFree(virNWFilterSnoopState.IfnameToKey); virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.IfnameToKey = virHashCreate(0, 0); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); if (!virNWFilterSnoopState.IfnameToKey) { - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopState.SnoopReqs = virHashCreate(0, virNWFilterSnoopReqRelease); if (!virNWFilterSnoopState.SnoopReqs) { virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopLeaseFileLoad(); } + +cleanup: + virNWFilterSnoopUnlock(); +} + +void +virNWFilterDHCPSnoopShutdown(void) +{ + /* + * free the SnoopReqs before the 'Active' hash since this will already + * clear the key / value pairs in the Active hash. + */ + + virNWFilterSnoopLock(); + + virNWFilterSnoopLeaseFileClose(); + virHashFree(virNWFilterSnoopState.IfnameToKey); + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopUnlock(); + + virNWFilterSnoopActiveLock(); + + virHashFree(virNWFilterSnoopState.Active); + + virNWFilterSnoopActiveUnlock(); } static int @@ -990,7 +1001,7 @@ virNWFilterSnoopLeaseFileWrite(int lfd, /* time intf ip dhcpserver */ snprintf(lbuf, sizeof(lbuf), "%u %s %s %s\n", ipl->Timeout, ifkey, ipstr, dhcpstr); - if (write(lfd, lbuf, strlen(lbuf)) < 0) { + if (safewrite(lfd, lbuf, strlen(lbuf)) != strlen(lbuf)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("lease file write failed: %s"), strerror(errno)); @@ -1023,8 +1034,9 @@ virNWFilterSnoopNewReq(const char *ifkey if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN-1) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopNewReq called with invalid " - "key \"%s\" (%d)"), - ifkey ? ifkey : "", strlen(ifkey)); + "key \"%s\" (%u)"), + ifkey ? ifkey : "", + (unsigned int)strlen(ifkey)); return NULL; } if (VIR_ALLOC(req) < 0) { @@ -1192,6 +1204,6 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("libvirt was not compiled " "with libpcap and \"ip_learning='dhcp'\" requires" " it.")); - return 1; + return -1; } #endif /* HAVE_LIBPCAP */ Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.h +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h @@ -25,6 +25,7 @@ #define __NWFILTER_DHCPSNOOP_H int virNWFilterDHCPSnoopInit(void); +void virNWFilterDHCPSnoopShutdown(void); int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, const char *ifname, const char *linkdev, Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -130,7 +130,7 @@ alloc_err_exit: conf_init_err: virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); return -1; @@ -153,7 +153,7 @@ nwfilterDriverReload(void) { conn = virConnectOpen("qemu:///system"); if (conn) { - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopEnd(NULL); /* shut down all threads -- they will be restarted if necessary */ virNWFilterLearnThreadsTerminate(true); @@ -208,7 +208,7 @@ nwfilterDriverShutdown(void) { virNWFilterConfLayerShutdown(); virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); nwfilterDriverLock(driverState); Index: libvirt-acl/po/POTFILES.in =================================================================== --- libvirt-acl.orig/po/POTFILES.in +++ libvirt-acl/po/POTFILES.in @@ -52,7 +52,6 @@ src/node_device/node_device_hal.c src/node_device/node_device_linux_sysfs.c src/node_device/node_device_udev.c src/nodeinfo.c -src/nwfilter/nwfilter_dhcpsnoop.c src/nwfilter/nwfilter_driver.c src/nwfilter/nwfilter_ebiptables_driver.c src/nwfilter/nwfilter_gentech_driver.c

On 03/30/2012 03:07 PM, 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).
This is the 2nd version to patch on top of this v8. Some more testing revealed a locking problem that I now fixed. Cleanup some parts of the DHCP snooping v8 code addressing - 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() --- po/POTFILES.in | 1 src/nwfilter/nwfilter_dhcpsnoop.c | 360 +++++++++++++++++++++++++------------- src/nwfilter/nwfilter_dhcpsnoop.h | 1 src/nwfilter/nwfilter_driver.c | 6 4 files changed, 246 insertions(+), 122 deletions(-) Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.c +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.c @@ -59,6 +59,7 @@ static struct { int LeaseFD; int nLeases; /* number of active leases */ int wLeases; /* number of written leases */ + int nThreads; /* number of running threads */ /* thread management */ virHashTablePtr SnoopReqs; virHashTablePtr IfnameToKey; @@ -67,23 +68,21 @@ static struct { virMutex ActiveLock; } virNWFilterSnoopState = { .LeaseFD = -1, - .SnoopLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, - .ActiveLock = { .lock=PTHREAD_MUTEX_INITIALIZER }, }; #define virNWFilterSnoopLock() \ { virMutexLock(&virNWFilterSnoopState.SnoopLock); } #define virNWFilterSnoopUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.SnoopLock); } -#define virNWFilterSnoopLockActive() \ +#define virNWFilterSnoopActiveLock() \ { virMutexLock(&virNWFilterSnoopState.ActiveLock); } -#define virNWFilterSnoopUnlockActive() \ +#define virNWFilterSnoopActiveUnlock() \ { virMutexUnlock(&virNWFilterSnoopState.ActiveLock); } static char * virNWFilterSnoopActivate(virThreadPtr thread) { - char *key, *data; + char *key; unsigned long threadID = (unsigned long int)thread->thread; if (virAsprintf(&key, "0x%0*lX", (int)sizeof(threadID)*2, threadID) < 0) { @@ -91,16 +90,11 @@ virNWFilterSnoopActivate(virThreadPtr th return NULL; } - virNWFilterSnoopLockActive(); - data = strdup("1"); - if (data == NULL) { - virReportOOMError(); - VIR_FREE(key); - } else if (virHashAddEntry(virNWFilterSnoopState.Active, key, data)) { + virNWFilterSnoopActiveLock(); + if (virHashAddEntry(virNWFilterSnoopState.Active, key, (void *)0x1)) { VIR_FREE(key); - VIR_FREE(data); } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); return key; } @@ -110,10 +104,10 @@ virNWFilterSnoopCancel(char **ThreadKey) if (*ThreadKey == NULL) return; - virNWFilterSnoopLockActive(); + virNWFilterSnoopActiveLock(); (void) virHashRemoveEntry(virNWFilterSnoopState.Active, *ThreadKey); - *ThreadKey = NULL; - virNWFilterSnoopUnlockActive(); + VIR_FREE(*ThreadKey); + virNWFilterSnoopActiveUnlock(); } static bool @@ -123,15 +117,18 @@ virNWFilterSnoopIsActive(char *ThreadKey if (ThreadKey == NULL) return 0; - virNWFilterSnoopLockActive(); + + virNWFilterSnoopActiveLock(); entry = virHashLookup(virNWFilterSnoopState.Active, ThreadKey); - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); + return entry != NULL; } #define VIR_IFKEY_LEN ((VIR_UUID_STRING_BUFLEN) + (VIR_MAC_STRING_BUFLEN)) struct virNWFilterSnoopReq { + unsigned int refctr; virNWFilterTechDriverPtr techdriver; const char *ifname; int ifindex; @@ -168,6 +165,7 @@ static void virNWFilterSnoopUpdateLease( time_t timeout); static struct virNWFilterSnoopReq *virNWFilterSnoopNewReq(const char *ifkey); +static void virNWFilterSnoopReqRelease(void *req0, const void *name ATTRIBUTE_UNUSED); static void virNWFilterSnoopLeaseFileOpen(void); static void virNWFilterSnoopLeaseFileClose(void); @@ -176,6 +174,56 @@ static void virNWFilterSnoopLeaseFileSav static void virNWFilterSnoopLeaseFileRestore(struct virNWFilterSnoopReq *req); static void virNWFilterSnoopLeaseFileRefresh(void); +static void +virNWFilterSnoopReqGet(struct virNWFilterSnoopReq *req) +{ + req->refctr++; +} + +static struct virNWFilterSnoopReq * +virNWFilterSnoopReqGetByKey(const char *ifkey) +{ + struct virNWFilterSnoopReq *req; + + virNWFilterSnoopLock(); + + req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + if (req) + virNWFilterSnoopReqGet(req); + + virNWFilterSnoopUnlock(); + + return req; +} + +static void +virNWFilterSnoopReqPut(struct virNWFilterSnoopReq *req) +{ + if (!req) + return; + + virNWFilterSnoopLock() + + req->refctr--; + + if (req->refctr == 0) { + /* + * delete the request: + * - if we don't find it on the global list anymore + * 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(); +} + /* * virNWFilterSnoopListAdd - add an IP lease to a list */ @@ -297,7 +345,11 @@ virNWFilterSnoopLeaseAdd(struct virNWFil return; } virNWFilterSnoopTimerAdd(pl); + + virNWFilterSnoopLock(); virNWFilterSnoopState.nLeases++; + virNWFilterSnoopUnlock(); + if (update_leasefile) virNWFilterSnoopLeaseFileSave(pl); } @@ -361,7 +413,10 @@ virNWFilterSnoopLeaseDel(struct virNWFil _("virNWFilterSnoopListDel failed")); } VIR_FREE(ipl); + + virNWFilterSnoopLock(); virNWFilterSnoopState.nLeases--; + virNWFilterSnoopUnlock(); } /* @@ -624,6 +679,9 @@ virNWFilterSnoopReqFree(struct virNWFilt if (!req) return; + if (req->refctr != 0) + return; + /* free all leases */ for (ipl = req->start; ipl; ipl = req->start) virNWFilterSnoopLeaseDel(req, ipl->IPAddress, 0); @@ -633,6 +691,7 @@ virNWFilterSnoopReqFree(struct virNWFilt VIR_FREE(req->linkdev); VIR_FREE(req->filtername); virNWFilterHashTableFree(req->vars); + VIR_FREE(req); } @@ -645,43 +704,38 @@ virNWFilterDHCPSnoop(void *req0) struct virNWFilterSnoopEthHdr *packet; int ifindex; int errcount; - char *threadkey; + char *threadkey = NULL; virThread thread; + /* the thread starting us increased the reference counter for the req */ + handle = virNWFilterSnoopDHCPOpen(req->ifname); if (!handle) - return; + goto exit; virThreadSelf(&thread); req->threadkey = virNWFilterSnoopActivate(&thread); threadkey = strdup(req->threadkey); if (threadkey == NULL) { virReportOOMError(); - pcap_close(handle); - return; + goto exit; } ifindex = if_nametoindex(req->ifname); - virNWFilterSnoopLock(); virNWFilterSnoopLeaseFileRestore(req); - virNWFilterSnoopUnlock(); errcount = 0; while (1) { int rv; - virNWFilterSnoopLock(); virNWFilterSnoopTimerRun(req); - virNWFilterSnoopUnlock(); rv = pcap_next_ex(handle, &hdr, (const u_char **)&packet); - if (!virNWFilterSnoopIsActive(threadkey)) { - VIR_FREE(threadkey); - pcap_close(handle); - return; - } + if (!virNWFilterSnoopIsActive(threadkey)) + goto exit; + if (rv < 0) { if (virNetDevValidateConfig(req->ifname, NULL, ifindex) <= 0) break; @@ -697,20 +751,31 @@ virNWFilterDHCPSnoop(void *req0) continue; } errcount = 0; - if (rv) { - virNWFilterSnoopLock(); + if (rv) virNWFilterSnoopDHCPDecode(req, packet, hdr->caplen); - virNWFilterSnoopUnlock(); - } } virNWFilterSnoopCancel(&req->threadkey); + + virNWFilterSnoopLock(); + (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, req->ifname); + + virNWFilterSnoopUnlock(); + VIR_FREE(req->ifname); - /* if we still have a valid lease, keep the req for restarts */ - if (!req->start || req->start->Timeout < time(0)) - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, req->ifkey); + +exit: + virNWFilterSnoopReqPut(req); + VIR_FREE(threadkey); - pcap_close(handle); + + if (handle) + pcap_close(handle); + + virNWFilterSnoopLock(); + virNWFilterSnoopState.nThreads--; + virNWFilterSnoopUnlock(); + return; } @@ -740,22 +805,23 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virThread thread; virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr); - virNWFilterSnoopLock(); - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); - isnewreq = req == NULL; + + req = virNWFilterSnoopReqGetByKey(ifkey); + isnewreq = (req == NULL); if (!isnewreq) { if (req->threadkey) { - virNWFilterSnoopUnlock(); + virNWFilterSnoopReqPut(req); return 0; } } else { req = virNWFilterSnoopNewReq(ifkey); - if (!req) { - virNWFilterSnoopUnlock(); + if (!req) return -1; - } + + virNWFilterSnoopReqGet(req); } + req->driver = driver; req->techdriver = techdriver; req->ifindex = if_nametoindex(ifname); req->linkdev = linkdev ? strdup(linkdev) : NULL; @@ -763,11 +829,10 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->ifname = strdup(ifname); memcpy(req->macaddr, macaddr, sizeof(req->macaddr)); req->filtername = strdup(filtername); + if (req->ifname == NULL || req->filtername == NULL) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto exit_snoopreqput; } if (techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr, NULL, @@ -778,20 +843,18 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD req->vars = virNWFilterHashTableCreate(0); if (!req->vars) { - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virReportOOMError(); - return -1; + goto exit_snoopreqput; } + if (virNWFilterHashTablePutAll(filterparams, req->vars)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq: can't copy variables" " on if %s"), ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopreqput; } - req->driver = driver; + + virNWFilterSnoopLock(); if (virHashAddEntry(virNWFilterSnoopState.IfnameToKey, ifname, req->ifkey)) { @@ -799,9 +862,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD _("virNWFilterDHCPSnoopReq ifname map failed" " on interface \"%s\" key \"%s\""), ifname, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopunlock; } if (isnewreq && virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { @@ -810,23 +871,30 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD " interface \"%s\" ifkey \"%s\""), ifname, ifkey); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); - return -1; + goto exit_snoopunlock; } - virNWFilterSnoopUnlock(); + + virNWFilterSnoopState.nThreads++; + if (virThreadCreate(&thread, false, virNWFilterDHCPSnoop, req) != 0) { - virNWFilterSnoopLock(); + virNWFilterSnoopState.nThreads--; (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, ifkey); - virNWFilterSnoopUnlock(); - virNWFilterSnoopReqFree(req); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterDHCPSnoopReq virThreadCreate " "failed on interface \"%s\""), ifname); - return -1; + goto exit_snoopunlock; } + + virNWFilterSnoopUnlock(); + return 0; + +exit_snoopunlock: + virNWFilterSnoopUnlock(); +exit_snoopreqput: + virNWFilterSnoopReqPut(req); + + return -1; } /* @@ -842,6 +910,7 @@ virNWFilterSnoopReqRelease(void *req0, c if (req->threadkey) virNWFilterSnoopCancel(&req->threadkey); + virNWFilterSnoopReqFree(req); } @@ -863,13 +932,21 @@ virNWFilterSnoopLeaseFileOpen(void) int virNWFilterDHCPSnoopInit(void) { - if (virNWFilterSnoopState.SnoopReqs) + if (virNWFilterSnoopState.SnoopReqs) { return 0; + } + + if (virMutexInitRecursive(&virNWFilterSnoopState.SnoopLock) < 0) + return -1; + if (virMutexInit(&virNWFilterSnoopState.ActiveLock) < 0) + return -1; virNWFilterSnoopLock(); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); virNWFilterSnoopState.SnoopReqs = virHashCreate(0, virNWFilterSnoopReqRelease); + if (!virNWFilterSnoopState.IfnameToKey || !virNWFilterSnoopState.SnoopReqs) { virNWFilterSnoopUnlock(); @@ -881,29 +958,28 @@ virNWFilterDHCPSnoopInit(void) virNWFilterSnoopUnlock(); - virNWFilterSnoopLockActive(); - virNWFilterSnoopState.Active = virHashCreate(0, 0); + virNWFilterSnoopActiveLock(); + + virNWFilterSnoopState.Active = virHashCreate(0, NULL); if (!virNWFilterSnoopState.Active) { - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); virReportOOMError(); goto errexit; } - virNWFilterSnoopUnlockActive(); + virNWFilterSnoopActiveUnlock(); + return 0; errexit: - if (virNWFilterSnoopState.IfnameToKey) { - virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopState.IfnameToKey = 0; - } - if (virNWFilterSnoopState.SnoopReqs) { - virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.SnoopReqs = 0; - } - if (virNWFilterSnoopState.Active) { - virHashFree(virNWFilterSnoopState.Active); - virNWFilterSnoopState.Active = 0; - } + virHashFree(virNWFilterSnoopState.IfnameToKey); + virNWFilterSnoopState.IfnameToKey = NULL; + + virHashFree(virNWFilterSnoopState.SnoopReqs); + virNWFilterSnoopState.SnoopReqs = NULL; + + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopState.Active = NULL; + return -1; } @@ -913,64 +989,87 @@ virNWFilterDHCPSnoopEnd(const char *ifna char *ifkey = NULL; virNWFilterSnoopLock(); - if (!virNWFilterSnoopState.SnoopReqs) { - virNWFilterSnoopUnlock(); - return; - } + if (!virNWFilterSnoopState.SnoopReqs) + goto cleanup; if (ifname) { - ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey,ifname); + ifkey = (char *)virHashLookup(virNWFilterSnoopState.IfnameToKey, + ifname); (void) virHashRemoveEntry(virNWFilterSnoopState.IfnameToKey, ifname); if (!ifkey) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifname \"%s\" not in key map"), ifname); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } } if (ifkey) { struct virNWFilterSnoopReq *req; - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + req = virNWFilterSnoopReqGetByKey(ifkey); if (!req) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("ifkey \"%s\" has no req"), ifkey); - virNWFilterSnoopUnlock(); - return; - } - if (!req->start || req->start->Timeout < time(0)) { - (void) virHashRemoveEntry(virNWFilterSnoopState.SnoopReqs, - req->ifkey); - virNWFilterSnoopUnlock(); - return; + goto cleanup; } /* keep valid lease req; drop interface association */ virNWFilterSnoopCancel(&req->threadkey); + VIR_FREE(req->ifname); + + virNWFilterSnoopReqPut(req); } else { /* free all of them */ virNWFilterSnoopLeaseFileClose(); virHashFree(virNWFilterSnoopState.IfnameToKey); virHashFree(virNWFilterSnoopState.SnoopReqs); - virNWFilterSnoopState.IfnameToKey = virHashCreate(0, 0); + virNWFilterSnoopState.IfnameToKey = virHashCreate(0, NULL); if (!virNWFilterSnoopState.IfnameToKey) { - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopState.SnoopReqs = virHashCreate(0, virNWFilterSnoopReqRelease); if (!virNWFilterSnoopState.SnoopReqs) { virHashFree(virNWFilterSnoopState.IfnameToKey); - virNWFilterSnoopUnlock(); virReportOOMError(); - return; + goto cleanup; } virNWFilterSnoopLeaseFileLoad(); } + +cleanup: virNWFilterSnoopUnlock(); } +void +virNWFilterDHCPSnoopShutdown(void) +{ + /* + * free the SnoopReqs before the 'Active' hash since this will already + * clear some of the key / value pairs in the Active hash. + */ + + virNWFilterSnoopLock(); + + virNWFilterSnoopLeaseFileClose(); + virHashFree(virNWFilterSnoopState.IfnameToKey); + virHashFree(virNWFilterSnoopState.SnoopReqs); + + virNWFilterSnoopUnlock(); + + virNWFilterSnoopActiveLock(); + virHashFree(virNWFilterSnoopState.Active); + virNWFilterSnoopActiveUnlock(); + + while (1) { + if (virNWFilterSnoopState.nThreads == 0) + break; + + VIR_WARN("Waiting for snooping threads to terminate\n"); + usleep(1000 * 1000); + } +} + static int virNWFilterSnoopLeaseFileWrite(int lfd, const char *ifkey, struct virNWFilterSnoopIPLease *ipl) @@ -990,7 +1089,7 @@ virNWFilterSnoopLeaseFileWrite(int lfd, /* time intf ip dhcpserver */ snprintf(lbuf, sizeof(lbuf), "%u %s %s %s\n", ipl->Timeout, ifkey, ipstr, dhcpstr); - if (write(lfd, lbuf, strlen(lbuf)) < 0) { + if (safewrite(lfd, lbuf, strlen(lbuf)) != strlen(lbuf)) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("lease file write failed: %s"), strerror(errno)); @@ -1005,14 +1104,20 @@ virNWFilterSnoopLeaseFileSave(struct vir { struct virNWFilterSnoopReq *req = ipl->SnoopReq; + virNWFilterSnoopLock(); + if (virNWFilterSnoopState.LeaseFD < 0) - virNWFilterSnoopLeaseFileOpen(); + virNWFilterSnoopLeaseFileOpen(); if (virNWFilterSnoopLeaseFileWrite(virNWFilterSnoopState.LeaseFD, - req->ifkey, ipl) < 0) - return; + req->ifkey, ipl) < 0) { + virNWFilterSnoopUnlock(); + return; + } /* keep dead leases at < ~95% of file size */ - if (++virNWFilterSnoopState.wLeases >= virNWFilterSnoopState.nLeases*20) + if (++virNWFilterSnoopState.wLeases >= virNWFilterSnoopState.nLeases * 20) virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ + + virNWFilterSnoopUnlock(); } static struct virNWFilterSnoopReq * @@ -1023,14 +1128,17 @@ virNWFilterSnoopNewReq(const char *ifkey if (ifkey == NULL || strlen(ifkey) != VIR_IFKEY_LEN-1) { virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopNewReq called with invalid " - "key \"%s\" (%d)"), - ifkey ? ifkey : "", strlen(ifkey)); + "key \"%s\" (%u)"), + ifkey ? ifkey : "", + (unsigned int)strlen(ifkey)); return NULL; } + if (VIR_ALLOC(req) < 0) { virReportOOMError(); return NULL; } + if (virStrcpyStatic(req->ifkey, ifkey) == NULL) VIR_FREE(req); @@ -1077,6 +1185,9 @@ virNWFilterSnoopLeaseFileRefresh(void) TMPLEASEFILE, strerror(errno)); return; } + + virNWFilterSnoopLock(); + if (virNWFilterSnoopState.SnoopReqs) virHashForEach(virNWFilterSnoopState.SnoopReqs, virNWFilterSnoopSaveIter, (void *)&tfd); @@ -1089,6 +1200,8 @@ virNWFilterSnoopLeaseFileRefresh(void) } virNWFilterSnoopState.wLeases = 0; virNWFilterSnoopLeaseFileOpen(); + + virNWFilterSnoopUnlock(); } @@ -1101,7 +1214,7 @@ virNWFilterSnoopLeaseFileLoad(void) struct virNWFilterSnoopReq *req; time_t now; FILE *fp; - int ln = 0; + int ln = 0, tmp; fp = fopen(LEASEFILE, "r"); time(&now); @@ -1123,13 +1236,20 @@ virNWFilterSnoopLeaseFileLoad(void) } if (ipl.Timeout && ipl.Timeout < now) continue; - req = virHashLookup(virNWFilterSnoopState.SnoopReqs, ifkey); + req = virNWFilterSnoopReqGetByKey(ifkey); if (!req) { req = virNWFilterSnoopNewReq(ifkey); if (!req) break; - if (virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req)) { - virNWFilterSnoopReqFree(req); + + virNWFilterSnoopReqGet(req); + + virNWFilterSnoopLock(); + tmp = virHashAddEntry(virNWFilterSnoopState.SnoopReqs, ifkey, req); + virNWFilterSnoopUnlock(); + + if (tmp) { + virNWFilterSnoopReqPut(req); virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("virNWFilterSnoopLeaseFileLoad req add" " failed on interface \"%s\""), ifkey); @@ -1141,6 +1261,7 @@ virNWFilterSnoopLeaseFileLoad(void) virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("line %d corrupt ipaddr \"%s\""), ln, ipstr); + virNWFilterSnoopReqPut(req); continue; } (void) inet_pton(AF_INET, srvstr, &ipl.IPServer); @@ -1150,10 +1271,13 @@ virNWFilterSnoopLeaseFileLoad(void) virNWFilterSnoopLeaseAdd(&ipl, 0); else virNWFilterSnoopLeaseDel(req, ipl.IPAddress, 0); + + virNWFilterSnoopReqPut(req); } if (fp != NULL) (void) fclose(fp); virNWFilterSnoopLeaseFileRefresh(); + } static void @@ -1192,6 +1316,6 @@ virNWFilterDHCPSnoopReq(virNWFilterTechD virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("libvirt was not compiled " "with libpcap and \"ip_learning='dhcp'\" requires" " it.")); - return 1; + return -1; } #endif /* HAVE_LIBPCAP */ Index: libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_dhcpsnoop.h +++ libvirt-acl/src/nwfilter/nwfilter_dhcpsnoop.h @@ -25,6 +25,7 @@ #define __NWFILTER_DHCPSNOOP_H int virNWFilterDHCPSnoopInit(void); +void virNWFilterDHCPSnoopShutdown(void); int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver, const char *ifname, const char *linkdev, Index: libvirt-acl/src/nwfilter/nwfilter_driver.c =================================================================== --- libvirt-acl.orig/src/nwfilter/nwfilter_driver.c +++ libvirt-acl/src/nwfilter/nwfilter_driver.c @@ -130,7 +130,7 @@ alloc_err_exit: conf_init_err: virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); return -1; @@ -153,7 +153,7 @@ nwfilterDriverReload(void) { conn = virConnectOpen("qemu:///system"); if (conn) { - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopEnd(NULL); /* shut down all threads -- they will be restarted if necessary */ virNWFilterLearnThreadsTerminate(true); @@ -208,7 +208,7 @@ nwfilterDriverShutdown(void) { virNWFilterConfLayerShutdown(); virNWFilterTechDriversShutdown(); - virNWFilterDHCPSnoopEnd(0); + virNWFilterDHCPSnoopShutdown(); virNWFilterLearnShutdown(); nwfilterDriverLock(driverState); Index: libvirt-acl/po/POTFILES.in =================================================================== --- libvirt-acl.orig/po/POTFILES.in +++ libvirt-acl/po/POTFILES.in @@ -52,7 +52,6 @@ src/node_device/node_device_hal.c src/node_device/node_device_linux_sysfs.c src/node_device/node_device_udev.c src/nodeinfo.c -src/nwfilter/nwfilter_dhcpsnoop.c src/nwfilter/nwfilter_driver.c src/nwfilter/nwfilter_ebiptables_driver.c src/nwfilter/nwfilter_gentech_driver.c
participants (2)
-
David L Stevens
-
Stefan Berger