[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH V11 6/7] nwfilter: Add multiple IP address support to DHCP snooping



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);
     }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]