Re: [libvirt] [PATCH] nwfilter: directly use poll to wait for packets instead of pcap_next

[John Ferlan]

On 05/21/2018 08:15 AM, Daniel P. Berrangé wrote:
> 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.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/nwfilter/nwfilter_learnipaddr.c | 37 +++--
>  1 file changed, 35 insertions(+), 2 deletions(-)
> 

FWIW: This patch is related to:

https://www.redhat.com/archives/libvir-list/2018-May/msg01440.html

Reviewed-by: John Ferlan 

I think this needs to be pushed before the release! Since Daniel is
away, I can take care of that.  I will wait for Monday though to ensure
there's no objection...

There's also a companion patch:

https://www.redhat.com/archives/libvir-list/2018-May/msg01429.html

but I have some issues with that and will post an alternative that would
also need to be pushed before the release.

John

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

[libvirt] [PATCH] nwfilter: directly use poll to wait for packets instead of pcap_next

[Daniel P . Berrangé]

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.

Signed-off-by: Daniel P. Berrangé 
---
 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 061b39d72b..e117be9ce2 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -31,6 +31,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -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(>macaddr, macaddr);
 
 switch (req->howDetect) {
@@ -483,17 +488,45 @@ learnIPAddressThread(void *arg)
 pcap_freecode();
 
 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, );
 
 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.17.0

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