10:25 a.m.
Use the virNWFilterBinding struct in the DHCP address snooping code
directly.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/nwfilter/nwfilter_dhcpsnoop.c | 150 +++++++++++++--------------------
src/nwfilter/nwfilter_dhcpsnoop.h | 7 +-
src/nwfilter/nwfilter_gentech_driver.c | 7 +-
3 files changed, 61 insertions(+), 103 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index dc4e3cb834..e67cea40ab 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -135,13 +135,9 @@ struct _virNWFilterSnoopReq {
int refctr;
virNWFilterTechDriverPtr techdriver;
- char *ifname;
+ virNWFilterBindingPtr binding;
int ifindex;
- char *linkdev;
char ifkey[VIR_IFKEY_LEN];
- virMacAddr macaddr;
- char *filtername;
- virHashTablePtr vars;
virNWFilterDriverStatePtr driver;
/* start and end of lease list, ordered by lease time */
virNWFilterSnoopIPLeasePtr start;
@@ -473,10 +469,10 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
req = ipl->snoopReq;
- /* protect req->ifname */
+ /* protect req->binding->portdevname */
virNWFilterSnoopReqLock(req);
- if (virNWFilterIPAddrMapAddIPAddr(req->ifname, ipaddr) < 0)
+ if (virNWFilterIPAddrMapAddIPAddr(req->binding->portdevname, ipaddr) < 0)
goto exit_snooprequnlock;
if (!instantiate) {
@@ -486,16 +482,9 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
/* instantiate the filters */
- if (req->ifname) {
- virNWFilterBinding binding = {
- .portdevname = req->ifname,
- .linkdevname = req->linkdev,
- .mac = req->macaddr,
- .filter = req->filtername,
- .filterparams = req->vars,
- };
+ if (req->binding->portdevname) {
rc = virNWFilterInstantiateFilterLate(req->driver,
- &binding,
+ req->binding,
req->ifindex);
}
@@ -636,10 +625,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req)
virNWFilterSnoopReqLeaseDel(req, &ipl->ipAddress, false, false);
/* free all req data */
- VIR_FREE(req->ifname);
- VIR_FREE(req->linkdev);
- VIR_FREE(req->filtername);
- virHashFree(req->vars);
+ virNWFilterBindingFree(req->binding);
virMutexDestroy(&req->lock);
virCondDestroy(&req->threadStatusCond);
@@ -870,28 +856,23 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
if (update_leasefile)
virNWFilterSnoopLeaseFileSave(ipl);
- ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->ifname, ipstr);
+ ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, ipstr);
if (!req->threadkey || !instantiate)
goto skip_instantiate;
if (ipAddrLeft) {
- virNWFilterBinding binding = {
- .portdevname = req->ifname,
- .linkdevname = req->linkdev,
- .mac = req->macaddr,
- .filter = req->filtername,
- .filterparams = req->vars,
- };
ret = virNWFilterInstantiateFilterLate(req->driver,
- &binding,
+ req->binding,
req->ifindex);
} else {
virNWFilterVarValuePtr dhcpsrvrs =
- virHashLookup(req->vars, NWFILTER_VARNAME_DHCPSERVER);
+ virHashLookup(req->binding->filterparams,
+ NWFILTER_VARNAME_DHCPSERVER);
if (req->techdriver &&
- req->techdriver->applyDHCPOnlyRules(req->ifname,
&req->macaddr,
+ req->techdriver->applyDHCPOnlyRules(req->binding->portdevname,
+ &req->binding->mac,
dhcpsrvrs, false) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("virNWFilterSnoopListDel failed"));
@@ -1021,7 +1002,7 @@ virNWFilterSnoopDHCPDecode(virNWFilterSnoopReqPtr req,
* inside the DHCP response
*/
if (!fromVM) {
- if (virMacAddrCmpRaw(&req->macaddr,
+ if (virMacAddrCmpRaw(&req->binding->mac,
(unsigned char *)&pd->d_chaddr) != 0)
return -2;
}
@@ -1178,7 +1159,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void
*opaque)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Instantiation of rules failed on "
- "interface '%s'"), req->ifname);
+ "interface '%s'"),
req->binding->portdevname);
}
virAtomicIntDecAndTest(job->qCtr);
VIR_FREE(job);
@@ -1387,13 +1368,14 @@ virNWFilterDHCPSnoopThread(void *req0)
/* whoever started us increased the reference counter for the req for us */
- /* protect req->ifname & req->threadkey */
+ /* protect req->binding->portdevname & req->threadkey */
virNWFilterSnoopReqLock(req);
- if (req->ifname && req->threadkey) {
+ if (req->binding->portdevname && req->threadkey) {
for (i = 0; i < ARRAY_CARDINALITY(pcapConf); i++) {
pcapConf[i].handle =
- virNWFilterSnoopDHCPOpen(req->ifname, &req->macaddr,
+ virNWFilterSnoopDHCPOpen(req->binding->portdevname,
+ &req->binding->mac,
pcapConf[i].filter,
pcapConf[i].dir);
if (!pcapConf[i].handle) {
@@ -1402,7 +1384,7 @@ virNWFilterDHCPSnoopThread(void *req0)
}
fds[i].fd = pcap_fileno(pcapConf[i].handle);
}
- tmp = virNetDevGetIndex(req->ifname, &ifindex);
+ tmp = virNetDevGetIndex(req->binding->portdevname, &ifindex);
ignore_value(VIR_STRDUP(threadkey, req->threadkey));
worker = virThreadPoolNew(1, 1, 0,
virNWFilterDHCPDecodeWorker,
@@ -1467,11 +1449,11 @@ virNWFilterDHCPSnoopThread(void *req0)
/* error reading from socket */
tmp = -1;
- /* protect req->ifname */
+ /* protect req->binding->portdevname */
virNWFilterSnoopReqLock(req);
- if (req->ifname)
- tmp = virNetDevValidateConfig(req->ifname, NULL, ifindex);
+ if (req->binding->portdevname)
+ tmp = virNetDevValidateConfig(req->binding->portdevname, NULL,
ifindex);
virNWFilterSnoopReqUnlock(req);
@@ -1484,16 +1466,17 @@ virNWFilterDHCPSnoopThread(void *req0)
pcap_close(pcapConf[i].handle);
pcapConf[i].handle = NULL;
- /* protect req->ifname */
+ /* protect req->binding->portdevname */
virNWFilterSnoopReqLock(req);
virReportError(VIR_ERR_INTERNAL_ERROR,
_("interface '%s' failing; "
"reopening"),
- req->ifname);
- if (req->ifname)
+ req->binding->portdevname);
+ if (req->binding->portdevname)
pcapConf[i].handle =
- virNWFilterSnoopDHCPOpen(req->ifname,
&req->macaddr,
+ virNWFilterSnoopDHCPOpen(req->binding->portdevname,
+ &req->binding->mac,
pcapConf[i].filter,
pcapConf[i].dir);
@@ -1519,7 +1502,7 @@ virNWFilterDHCPSnoopThread(void *req0)
last_displayed_queue = time(0);
VIR_WARN("Worker thread for interface '%s' has a
"
"job queue that is too long",
- req->ifname);
+ req->binding->portdevname);
}
continue;
}
@@ -1532,7 +1515,7 @@ virNWFilterDHCPSnoopThread(void *req0)
if (time(0) - last_displayed > 10) {
last_displayed = time(0);
VIR_WARN("Too many DHCP packets on interface
'%s'",
- req->ifname);
+ req->binding->portdevname);
}
continue;
}
@@ -1543,7 +1526,7 @@ virNWFilterDHCPSnoopThread(void *req0)
&pcapConf[i].qCtr) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Job submission failed on "
- "interface '%s'"),
req->ifname);
+ "interface '%s'"),
req->binding->portdevname);
error = true;
break;
}
@@ -1554,15 +1537,15 @@ virNWFilterDHCPSnoopThread(void *req0)
/* protect IfNameToKey */
virNWFilterSnoopLock();
- /* protect req->ifname & req->threadkey */
+ /* protect req->binding->portdevname & req->threadkey */
virNWFilterSnoopReqLock(req);
virNWFilterSnoopCancel(&req->threadkey);
ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
- req->ifname));
+ req->binding->portdevname));
- VIR_FREE(req->ifname);
+ VIR_FREE(req->binding->portdevname);
virNWFilterSnoopReqUnlock(req);
virNWFilterSnoopUnlock();
@@ -1595,12 +1578,7 @@ virNWFilterSnoopIFKeyFMT(char *ifkey, const unsigned char *vmuuid,
int
virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
- const char *ifname,
- const char *linkdev,
- const unsigned char *vmuuid,
- const virMacAddr *macaddr,
- const char *filtername,
- virHashTablePtr filterparams,
+ virNWFilterBindingPtr binding,
virNWFilterDriverStatePtr driver)
{
virNWFilterSnoopReqPtr req;
@@ -1611,7 +1589,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
virNWFilterVarValuePtr dhcpsrvrs;
bool threadPuts = false;
- virNWFilterSnoopIFKeyFMT(ifkey, vmuuid, macaddr);
+ virNWFilterSnoopIFKeyFMT(ifkey, binding->owneruuid, &binding->mac);
req = virNWFilterSnoopReqGetByIFKey(ifkey);
isnewreq = (req == NULL);
@@ -1620,9 +1598,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
virNWFilterSnoopReqPut(req);
return 0;
}
- /* a recycled req may still have filtername and vars */
- VIR_FREE(req->filtername);
- virHashFree(req->vars);
+ virNWFilterBindingFree(req->binding);
+ req->binding = NULL;
} else {
req = virNWFilterSnoopReqNew(ifkey);
if (!req)
@@ -1631,17 +1608,9 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
req->driver = driver;
req->techdriver = techdriver;
- tmp = virNetDevGetIndex(ifname, &req->ifindex);
- virMacAddrSet(&req->macaddr, macaddr);
- req->vars = virNWFilterHashTableCreate(0);
- req->linkdev = NULL;
-
- if (VIR_STRDUP(req->ifname, ifname) < 0 ||
- VIR_STRDUP(req->filtername, filtername) < 0 ||
- VIR_STRDUP(req->linkdev, linkdev) < 0)
+ if ((tmp = virNetDevGetIndex(binding->portdevname, &req->ifindex)) < 0)
goto exit_snoopreqput;
-
- if (!req->vars || tmp < 0)
+ if (!(req->binding = virNWFilterBindingCopy(binding)))
goto exit_snoopreqput;
/* check that all tools are available for applying the filters (late) */
@@ -1653,10 +1622,11 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
goto exit_snoopreqput;
}
- dhcpsrvrs = virHashLookup(filterparams,
+ dhcpsrvrs = virHashLookup(binding->filterparams,
NWFILTER_VARNAME_DHCPSERVER);
- if (techdriver->applyDHCPOnlyRules(req->ifname, &req->macaddr,
+ if (techdriver->applyDHCPOnlyRules(req->binding->portdevname,
+ &req->binding->mac,
dhcpsrvrs, false) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("applyDHCPOnlyRules "
@@ -1664,20 +1634,14 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
goto exit_snoopreqput;
}
- if (virNWFilterHashTablePutAll(filterparams, req->vars) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("virNWFilterDHCPSnoopReq: can't copy variables"
- " on if %s"), ifkey);
- goto exit_snoopreqput;
- }
-
virNWFilterSnoopLock();
- if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey, ifname,
+ if (virHashAddEntry(virNWFilterSnoopState.ifnameToKey,
+ req->binding->portdevname,
req->ifkey) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("virNWFilterDHCPSnoopReq ifname map failed"
- " on interface \"%s\" key
\"%s\""), ifname,
+ " on interface \"%s\" key
\"%s\""), binding->portdevname,
ifkey);
goto exit_snoopunlock;
}
@@ -1686,7 +1650,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
virHashAddEntry(virNWFilterSnoopState.snoopReqs, ifkey, req) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("virNWFilterDHCPSnoopReq req add failed on"
- " interface \"%s\" ifkey \"%s\""),
ifname,
+ " interface \"%s\" ifkey \"%s\""),
binding->portdevname,
ifkey);
goto exit_rem_ifnametokey;
}
@@ -1698,7 +1662,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
req) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("virNWFilterDHCPSnoopReq virThreadCreate "
- "failed on interface '%s'"), ifname);
+ "failed on interface '%s'"),
binding->portdevname);
goto exit_snoopreq_unlock;
}
@@ -1710,14 +1674,14 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
if (!req->threadkey) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Activation of snoop request failed on "
- "interface '%s'"), req->ifname);
+ "interface '%s'"),
req->binding->portdevname);
goto exit_snoopreq_unlock;
}
if (virNWFilterSnoopReqRestore(req) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Restoring of leases failed on "
- "interface '%s'"), req->ifname);
+ "interface '%s'"),
req->binding->portdevname);
goto exit_snoop_cancel;
}
@@ -1746,7 +1710,7 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
exit_snoopreq_unlock:
virNWFilterSnoopReqUnlock(req);
exit_rem_ifnametokey:
- virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, ifname);
+ virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey, binding->portdevname);
exit_snoopunlock:
virNWFilterSnoopUnlock();
exit_snoopreqput:
@@ -2054,21 +2018,21 @@ virNWFilterSnoopRemAllReqIter(const void *payload,
{
virNWFilterSnoopReqPtr req = (virNWFilterSnoopReqPtr)payload;
- /* protect req->ifname */
+ /* protect req->binding->portdevname */
virNWFilterSnoopReqLock(req);
- if (req->ifname) {
+ if (req->binding->portdevname) {
ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
- req->ifname));
+ req->binding->portdevname));
/*
* Remove all IP addresses known to be associated with this
* interface so that a new thread will be started on this
* interface
*/
- virNWFilterIPAddrMapDelIPAddr(req->ifname, NULL);
+ virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, NULL);
- VIR_FREE(req->ifname);
+ VIR_FREE(req->binding->portdevname);
}
virNWFilterSnoopReqUnlock(req);
@@ -2171,13 +2135,13 @@ virNWFilterDHCPSnoopEnd(const char *ifname)
goto cleanup;
}
- /* protect req->ifname & req->threadkey */
+ /* protect req->binding->portdevname & req->threadkey */
virNWFilterSnoopReqLock(req);
/* keep valid lease req; drop interface association */
virNWFilterSnoopCancel(&req->threadkey);
- VIR_FREE(req->ifname);
+ VIR_FREE(req->binding->portdevname);
virNWFilterSnoopReqUnlock(req);
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.h b/src/nwfilter/nwfilter_dhcpsnoop.h
index a5925de40a..0c047fd5a1 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.h
+++ b/src/nwfilter/nwfilter_dhcpsnoop.h
@@ -30,12 +30,7 @@
int virNWFilterDHCPSnoopInit(void);
void virNWFilterDHCPSnoopShutdown(void);
int virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
- const char *ifname,
- const char *linkdev,
- const unsigned char *vmuuid,
- const virMacAddr *macaddr,
- const char *filtername,
- virHashTablePtr filterparams,
+ virNWFilterBindingPtr binding,
virNWFilterDriverStatePtr driver);
void virNWFilterDHCPSnoopEnd(const char *ifname);
#endif /* __NWFILTER_DHCPSNOOP_H */
diff --git a/src/nwfilter/nwfilter_gentech_driver.c
b/src/nwfilter/nwfilter_gentech_driver.c
index 514315f781..0dc51d16c5 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -645,10 +645,9 @@ virNWFilterDoInstantiate(virNWFilterTechDriverPtr techdriver,
goto err_unresolvable_vars;
}
if (STRCASEEQ(learning, "dhcp")) {
- rc = virNWFilterDHCPSnoopReq(techdriver, binding->portdevname,
- binding->linkdevname,
- binding->owneruuid,
&binding->mac,
- filter->name, binding->filterparams,
driver);
+ rc = virNWFilterDHCPSnoopReq(techdriver,
+ binding,
+ driver);
goto err_exit;
} else if (STRCASEEQ(learning, "any")) {
if (!virNWFilterHasLearnReq(ifindex)) {
--
2.14.3