From ab4591f1f4d8a57a8e1bce3fde125376a0524d66 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 21 May 2018 12:46:00 +0100
Subject: [PATCH] nwfilter: directly use poll to wait for packets instead of
 pcap_next
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

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>
---
 src/nwfilter/nwfilter_learnipaddr.c | 37 +++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

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;
-- 
2.47.2