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

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

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]