From: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon, 21 May 2018 11:46:00 +0000 (+0100)
Subject: nwfilter: directly use poll to wait for packets instead of pcap_next
X-Git-Tag: v4.5.0-rc1~208
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ab4591f1f4d8a57a8e1bce3fde125376a0524d66;p=thirdparty%2Flibvirt.git

nwfilter: directly use poll to wait for packets instead of pcap_next

When a QEMU VM shuts down its TAP device gets deleted while nwfilter
IP address learning thread is still capturing packets. It is seen that
with TPACKET_V3 support in libcap, the pcap_next() call will not always
exit its poll() when the NIC is removed. This prevents the learning
thread from exiting which blocks the rest of libvirtd waiting on mutex
acquisition. By switching to do poll() in libvirt code, we can ensure
that we always exit the poll() at a time that is right for libvirt.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_learnipaddr.c
index 085af7892e..52adc37389 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -31,6 +31,7 @@
 
 #include <fcntl.h>
 #include <sys/ioctl.h>
+#include <poll.h>
 
 #include <arpa/inet.h>
 #include <net/ethernet.h>
@@ -414,6 +415,7 @@ learnIPAddressThread(void *arg)
     bool showError = true;
     enum howDetect howDetected = 0;
     virNWFilterTechDriverPtr techdriver = req->techdriver;
+    struct pollfd fds[1];
 
     if (virNWFilterLockIface(req->ifname) < 0)
        goto err_no_lock;
@@ -435,6 +437,9 @@ learnIPAddressThread(void *arg)
         goto done;
     }
 
+    fds[0].fd = pcap_fileno(handle);
+    fds[0].events = POLLIN | POLLERR;
+
     virMacAddrFormat(&req->macaddr, macaddr);
 
     if (req->howDetect == DETECT_DHCP) {
@@ -480,17 +485,45 @@ learnIPAddressThread(void *arg)
     pcap_freecode(&fp);
 
     while (req->status == 0 && vmaddr == 0) {
+        int n = poll(fds, ARRAY_CARDINALITY(fds), PKT_TIMEOUT_MS);
+
+        if (n < 0) {
+            if (errno == EAGAIN || errno == EINTR)
+                continue;
+
+            req->status = errno;
+            showError = true;
+            break;
+        }
+
+        if (n == 0) {
+            if (threadsTerminate || req->terminate) {
+                VIR_DEBUG("Terminate request seen, cancelling pcap");
+                req->status = ECANCELED;
+                showError = false;
+                break;
+            }
+            continue;
+        }
+
+        if (fds[0].revents & (POLLHUP | POLLERR)) {
+            VIR_DEBUG("Error from FD probably dev deleted");
+            req->status = ENODEV;
+            showError = false;
+            break;
+        }
+
         packet = pcap_next(handle, &header);
 
         if (!packet) {
-
+            /* Already handled with poll, but lets be sure */
             if (threadsTerminate || req->terminate) {
                 req->status = ECANCELED;
                 showError = false;
                 break;
             }
 
-            /* check whether VM's dev is still there */
+            /* Again, already handled above, but lets be sure */
             if (virNetDevValidateConfig(req->ifname, NULL, req->ifindex) <= 0) {
                 virResetLastError();
                 req->status = ENODEV;