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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list