9:44 a.m.
With support for multiple IP addresses per interface in place, this patch
now adds support for multiple IP addresses per interface for the DHCP
snooping code.
Testing:
Since the infrastructure I tested this with does not provide multiple IP
addresses per MAC address (anymore), I either had to plug the VM's interface
from the virtual bride connected directly to the infrastructure to virbr0
to get a 2nd IP address from dnsmasq (kill and run dhclient inside the VM)
or changed the lease file (/var/run/libvirt/network/nwfilter.leases) and
restart libvirtd to have a 2nd IP address on an existing interface.
Note that dnsmasq can take a lease timeout parameter as part of the --dhcp-range
command line parameter, so that timeouts can be tested that way
(--dhcp-range 192.168.122.2,192.168.122.254,120). So, terminating and restarting
dnsmasq with that parameter is another choice to watch an IP address disappear
after 120 seconds.
Regards,
Stefan
---
src/nwfilter/nwfilter_dhcpsnoop.c | 102 +++++++++++++++++++++++++-------------
1 file changed, 69 insertions(+), 33 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 @@
#include "conf/domain_conf.h"
#include "nwfilter_gentech_driver.h"
#include "nwfilter_dhcpsnoop.h"
+#include "nwfilter_ipaddrmap.h"
#include "virnetdev.h"
#include "virfile.h"
#include "viratomic.h"
@@ -262,7 +263,8 @@ struct _virNWFilterSnoopRateLimitConf {
/* local function prototypes */
static int virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
- uint32_t ipaddr, bool update_leasefile);
+ uint32_t ipaddr, bool update_leasefile,
+ bool instantiate);
static void virNWFilterSnoopReqLock(virNWFilterSnoopReqPtr req);
static void virNWFilterSnoopReqUnlock(virNWFilterSnoopReqPtr req);
@@ -438,8 +440,8 @@ virNWFilterSnoopIPLeaseInstallRule(virNW
{
char ipbuf[INET_ADDRSTRLEN];
int rc = -1;
- virNWFilterVarValuePtr ipVar;
virNWFilterSnoopReqPtr req;
+ char *ipaddr;
if (!inet_ntop(AF_INET, &ipl->ipAddress, ipbuf, sizeof(ipbuf))) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
@@ -448,17 +450,15 @@ virNWFilterSnoopIPLeaseInstallRule(virNW
__func__, ipl->ipAddress);
return -1;
}
- ipVar = virNWFilterVarValueCreateSimpleCopyValue(ipbuf);
- if (!ipVar) {
+
+ ipaddr = strdup(ipbuf);
+ if (ipaddr == NULL) {
virReportOOMError();
return -1;
}
- if (virNWFilterHashTablePut(ipl->snoopReq->vars, NWFILTER_VARNAME_IP,
- ipVar, 1) < 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("Could not add variable \""
- NWFILTER_VARNAME_IP "\" to hashmap"));
- virNWFilterVarValueFree(ipVar);
+
+ if (virNWFilterIPAddrMapAddIPAddr(ipl->snoopReq->ifname, ipaddr) < 0) {
+ VIR_FREE(ipaddr);
return -1;
}
@@ -522,12 +522,18 @@ static unsigned int
virNWFilterSnoopReqLeaseTimerRun(virNWFilterSnoopReqPtr req)
{
time_t now = time(0);
+ bool is_last = false;
/* protect req->start */
virNWFilterSnoopReqLock(req);
- while (req->start && req->start->timeout <= now)
- virNWFilterSnoopReqLeaseDel(req, req->start->ipAddress, true);
+ while (req->start && req->start->timeout <= now) {
+ if (req->start->next == NULL ||
+ req->start->next->timeout > now)
+ is_last = true;
+ virNWFilterSnoopReqLeaseDel(req, req->start->ipAddress, true,
+ is_last);
+ }
virNWFilterSnoopReqUnlock(req);
@@ -598,7 +604,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoop
/* free all leases */
for (ipl = req->start; ipl; ipl = req->start)
- virNWFilterSnoopReqLeaseDel(req, ipl->ipAddress, false);
+ virNWFilterSnoopReqLeaseDel(req, ipl->ipAddress, false, false);
/* free all req data */
VIR_FREE(req->ifname);
@@ -731,15 +737,6 @@ virNWFilterSnoopReqLeaseAdd(virNWFilterS
virNWFilterSnoopReqUnlock(req);
- /* 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 0; /* silently ignore multiple addresses */
-
if (VIR_ALLOC(pl) < 0) {
virReportOOMError();
return -1;
@@ -807,34 +804,64 @@ virNWFilterSnoopReqRestore(virNWFilterSn
* memory or when calling this function while reading
* leases from the file.
*
+ * @instantiate: when calling this function in a loop, indicate
+ * the last call with 'true' here so that the
+ * rules all get instantiated
+ * Always calling this with 'true' is fine, but less
+ * efficient.
+ *
* Returns 0 on success, -1 if the instantiation of the rules failed
*/
static int
virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
- uint32_t ipaddr, bool update_leasefile)
+ uint32_t ipaddr, bool update_leasefile,
+ bool instantiate)
{
int ret = 0;
virNWFilterSnoopIPLeasePtr ipl;
+ char ipstr[INET_ADDRSTRLEN];
+ int ipAddrLeft;
- /* protect req->start & req->ifname */
+ /* protect req->start, req->ifname and the lease */
virNWFilterSnoopReqLock(req);
ipl = virNWFilterSnoopIPLeaseGetByIP(req->start, ipaddr);
if (ipl == NULL)
goto lease_not_found;
+ if (!inet_ntop(AF_INET, &ipl->ipAddress, ipstr, sizeof(ipstr))) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s: inet_ntop failed (0x%08X)"),
+ __func__, ipl->ipAddress);
+ ret = -1;
+ goto lease_not_found;
+ }
+
virNWFilterSnoopIPLeaseTimerDel(ipl);
+ /* lease is off the list now */
+
+ if (update_leasefile)
+ virNWFilterSnoopLeaseFileSave(ipl);
- if (update_leasefile) {
+ ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->ifname, ipstr);
+
+ if (!req->threadkey || !instantiate)
+ goto skip_instantiate;
+
+ if (ipAddrLeft) {
+ ret = virNWFilterInstantiateFilterLate(NULL,
+ req->ifname,
+ req->ifindex,
+ req->linkdev,
+ req->nettype,
+ req->macaddr,
+ req->filtername,
+ req->vars,
+ req->driver);
+ } else {
const virNWFilterVarValuePtr dhcpsrvrs =
virHashLookup(req->vars->hashTable, NWFILTER_VARNAME_DHCPSERVER);
- virNWFilterSnoopLeaseFileSave(ipl);
-
- /*
- * for multiple address support, this needs to remove those rules
- * referencing "IP" with ipl's ip value.
- */
if (req->techdriver &&
req->techdriver->applyDHCPOnlyRules(req->ifname, req->macaddr,
dhcpsrvrs, false) < 0) {
@@ -844,6 +871,8 @@ virNWFilterSnoopReqLeaseDel(virNWFilterS
}
}
+
+skip_instantiate:
VIR_FREE(ipl);
virAtomicIntSub(&virNWFilterSnoopState.nLeases, 1);
@@ -1015,7 +1044,7 @@ virNWFilterSnoopDHCPDecode(virNWFilterSn
break;
case DHCPDECLINE:
case DHCPRELEASE:
- if (virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, true) < 0)
+ if (virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, true, true) < 0)
return -1;
break;
default:
@@ -1797,7 +1826,7 @@ virNWFilterSnoopLeaseFileLoad(void)
if (ipl.timeout)
virNWFilterSnoopReqLeaseAdd(req, &ipl, false);
else
- virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, false);
+ virNWFilterSnoopReqLeaseDel(req, ipl.ipAddress, false, false);
virNWFilterSnoopReqPut(req);
}
@@ -1843,6 +1872,13 @@ virNWFilterSnoopRemAllReqIter(const void
(void) virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
req->ifname);
+ /*
+ * 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);
+
VIR_FREE(req->ifname);
}