7:33 a.m.
Use the virNWFilterBindingDefPtr struct in the DHCP address snooping code
directly.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/nwfilter/nwfilter_dhcpsnoop.c | 161 +++++++++----------------
src/nwfilter/nwfilter_dhcpsnoop.h | 7 +-
src/nwfilter/nwfilter_gentech_driver.c | 7 +-
3 files changed, 62 insertions(+), 113 deletions(-)
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 7274b03c2a..533c45f080 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;
+ virNWFilterBindingDefPtr 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;
@@ -484,10 +480,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) {
@@ -497,18 +493,9 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
/* instantiate the filters */
- if (req->ifname) {
- virNWFilterBindingDef binding = {
- .portdevname = req->ifname,
- .linkdevname = req->linkdev,
- .mac = req->macaddr,
- .filter = req->filtername,
- .filterparams = req->vars,
- .ownername = NULL,
- .owneruuid = {0},
- };
+ if (req->binding->portdevname) {
rc = virNWFilterInstantiateFilterLate(req->driver,
- &binding,
+ req->binding,
req->ifindex);
}
@@ -649,10 +636,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);
+ virNWFilterBindingDefFree(req->binding);
virMutexDestroy(&req->lock);
virCondDestroy(&req->threadStatusCond);
@@ -883,30 +867,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) {
- virNWFilterBindingDef binding = {
- .portdevname = req->ifname,
- .linkdevname = req->linkdev,
- .mac = req->macaddr,
- .filter = req->filtername,
- .filterparams = req->vars,
- .ownername = NULL,
- .owneruuid = {0},
- };
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"));
@@ -1036,7 +1013,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;
}
@@ -1198,7 +1175,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);
@@ -1407,13 +1384,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) {
@@ -1422,7 +1400,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,
@@ -1487,11 +1465,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);
@@ -1504,16 +1482,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);
@@ -1539,7 +1518,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;
}
@@ -1552,7 +1531,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;
}
@@ -1563,7 +1542,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;
}
@@ -1574,15 +1553,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();
@@ -1615,12 +1594,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,
+ virNWFilterBindingDefPtr binding,
virNWFilterDriverStatePtr driver)
{
virNWFilterSnoopReqPtr req;
@@ -1631,7 +1605,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);
@@ -1640,9 +1614,8 @@ virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver,
virNWFilterSnoopReqPut(req);
return 0;
}
- /* a recycled req may still have filtername and vars */
- VIR_FREE(req->filtername);
- virHashFree(req->vars);
+ virNWFilterBindingDefFree(req->binding);
+ req->binding = NULL;
} else {
req = virNWFilterSnoopReqNew(ifkey);
if (!req)
@@ -1651,17 +1624,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 = virNWFilterBindingDefCopy(binding)))
goto exit_snoopreqput;
/* check that all tools are available for applying the filters (late) */
@@ -1673,10 +1638,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 "
@@ -1684,20 +1650,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;
}
@@ -1706,7 +1666,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;
}
@@ -1718,7 +1678,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;
}
@@ -1730,14 +1690,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;
}
@@ -1766,7 +1726,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:
@@ -2074,21 +2034,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);
@@ -2191,13 +2151,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);
@@ -2259,12 +2219,7 @@ virNWFilterDHCPSnoopShutdown(void)
int
virNWFilterDHCPSnoopReq(virNWFilterTechDriverPtr techdriver ATTRIBUTE_UNUSED,
- const char *ifname ATTRIBUTE_UNUSED,
- const char *linkdev ATTRIBUTE_UNUSED,
- const unsigned char *vmuuid ATTRIBUTE_UNUSED,
- const virMacAddr *macaddr ATTRIBUTE_UNUSED,
- const char *filtername ATTRIBUTE_UNUSED,
- virHashTablePtr filterparams ATTRIBUTE_UNUSED,
+ virNWFilterBindingDefPtr binding ATTRIBUTE_UNUSED,
virNWFilterDriverStatePtr driver ATTRIBUTE_UNUSED)
{
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.h b/src/nwfilter/nwfilter_dhcpsnoop.h
index a5925de40a..c693e1adbd 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,
+ virNWFilterBindingDefPtr 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 ce45587a44..4b55bd6ca4 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -618,10 +618,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.17.0