]> The Tcpdump Group git mirrors - libpcap/commitdiff
Close the eventfd if we are non-blocking 1113/head
authorBill Fenner <[email protected]>
Thu, 26 May 2022 18:41:43 +0000 (11:41 -0700)
committerBill Fenner <[email protected]>
Thu, 25 Aug 2022 15:29:07 +0000 (08:29 -0700)
The eventfd is used to break out of a poll() before it
times out, used by pcap_breakloop().  If we are non-blocking,
then the eventfd is never needed, so we close it.  (And
open a new eventfd if we switch to blocking).

pcap-linux.c
testprogs/Makefile.in
testprogs/nonblocktest.c [new file with mode: 0644]

index 8dbf89bca393ca596b98dee24e837fd9c22cea43..42ad9249d64030fa31c16c02029789c4816acf1e 100644 (file)
@@ -842,7 +842,10 @@ static void        pcap_cleanup_linux( pcap_t *handle )
                handlep->device = NULL;
        }
 
-       close(handlep->poll_breakloop_fd);
+       if (handlep->poll_breakloop_fd != -1) {
+               close(handlep->poll_breakloop_fd);
+               handlep->poll_breakloop_fd = -1;
+       }
        pcap_cleanup_live_common(handle);
 }
 
@@ -941,7 +944,8 @@ static void pcap_breakloop_linux(pcap_t *handle)
 
        uint64_t value = 1;
        /* XXX - what if this fails? */
-       (void)write(handlep->poll_breakloop_fd, &value, sizeof(value));
+       if (handlep->poll_breakloop_fd != -1)
+               (void)write(handlep->poll_breakloop_fd, &value, sizeof(value));
 }
 
 /*
@@ -3375,7 +3379,23 @@ pcap_setnonblock_linux(pcap_t *handle, int nonblock)
                         */
                        handlep->timeout = ~handlep->timeout;
                }
+               if (handlep->poll_breakloop_fd != -1) {
+                       /* Close the eventfd; we do not need it in nonblock mode. */
+                       close(handlep->poll_breakloop_fd);
+                       handlep->poll_breakloop_fd = -1;
+               }
        } else {
+               if (handlep->poll_breakloop_fd == -1) {
+                       /* If we did not have an eventfd, open one now that we are blocking. */
+                       if ( ( handlep->poll_breakloop_fd = eventfd(0, EFD_NONBLOCK) ) == -1 ) {
+                               int save_errno = errno;
+                               snprintf(handle->errbuf, PCAP_ERRBUF_SIZE,
+                                               "Could not open eventfd: %s",
+                                               strerror(errno));
+                               errno = save_errno;
+                               return -1;
+                       }
+               }
                if (handlep->timeout < 0) {
                        handlep->timeout = ~handlep->timeout;
                }
@@ -3419,10 +3439,24 @@ static int pcap_wait_for_frames_mmap(pcap_t *handle)
        struct ifreq ifr;
        int ret;
        struct pollfd pollinfo[2];
+       int numpollinfo;
        pollinfo[0].fd = handle->fd;
        pollinfo[0].events = POLLIN;
-       pollinfo[1].fd = handlep->poll_breakloop_fd;
-       pollinfo[1].events = POLLIN;
+       if ( handlep->poll_breakloop_fd == -1 ) {
+               numpollinfo = 1;
+               pollinfo[1].revents = 0;
+               /*
+                * We set pollinfo[1].revents to zero, even though
+                * numpollinfo = 1 meaning that poll() doesn't see
+                * pollinfo[1], so that we do not have to add a
+                * conditional of numpollinfo > 1 below when we
+                * test pollinfo[1].revents.
+                */
+       } else {
+               pollinfo[1].fd = handlep->poll_breakloop_fd;
+               pollinfo[1].events = POLLIN;
+               numpollinfo = 2;
+       }
 
        /*
         * Keep polling until we either get some packets to read, see
@@ -3487,7 +3521,7 @@ static int pcap_wait_for_frames_mmap(pcap_t *handle)
                        if (timeout != 0)
                                timeout = 1;
                }
-               ret = poll(pollinfo, 2, timeout);
+               ret = poll(pollinfo, numpollinfo, timeout);
                if (ret < 0) {
                        /*
                         * Error.  If it's not EINTR, report it.
index 45e1c73e7524eb97a1eda28a71f9c42b1de03754..293ed88827147db253c0d0cc383e9c5cc103f613 100644 (file)
@@ -85,6 +85,7 @@ SRC = @VALGRINDTEST_SRC@ \
        findalldevstest-perf.c \
        findalldevstest.c \
        opentest.c \
+       nonblocktest.c \
        reactivatetest.c \
        selpolltest.c \
        threadsignaltest.c \
@@ -126,6 +127,10 @@ opentest: $(srcdir)/opentest.c ../libpcap.a
        $(CC) $(FULL_CFLAGS) -I. -L. -o opentest $(srcdir)/opentest.c \
            ../libpcap.a $(LIBS)
 
+nonblocktest: $(srcdir)/nonblocktest.c ../libpcap.a
+       $(CC) $(FULL_CFLAGS) -I. -L. -o nonblocktest $(srcdir)/nonblocktest.c \
+           ../libpcap.a $(LIBS)
+
 reactivatetest: $(srcdir)/reactivatetest.c ../libpcap.a
        $(CC) $(FULL_CFLAGS) -I. -L. -o reactivatetest \
            $(srcdir)/reactivatetest.c ../libpcap.a $(LIBS)
diff --git a/testprogs/nonblocktest.c b/testprogs/nonblocktest.c
new file mode 100644 (file)
index 0000000..72700a3
--- /dev/null
@@ -0,0 +1,187 @@
+/*
+ * Copyright (c) 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 2000
+ *     The Regents of the University of California.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that: (1) source code distributions
+ * retain the above copyright notice and this paragraph in its entirety, (2)
+ * distributions including binary code include the above copyright notice and
+ * this paragraph in its entirety in the documentation or other materials
+ * provided with the distribution, and (3) all advertising materials mentioning
+ * features or use of this software display the following acknowledgement:
+ * ``This product includes software developed by the University of California,
+ * Lawrence Berkeley Laboratory and its contributors.'' Neither the name of
+ * the University nor the names of its contributors may be used to endorse
+ * or promote products derived from this software without specific prior
+ * written permission.
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+ */
+
+#include "varattrs.h"
+
+/*
+ * Tests for pcap_set_nonblock / pcap_get_nonblock:
+ * - idempotency
+ * - set/get are symmetric
+ * - get returns the same before/after activate
+ * - pcap_breakloop works after setting nonblock on and then off
+ *
+ * Really this is meant to
+ * be run manually under strace, to check for extra
+ * calls to eventfd or close.
+ */
+#include <pcap.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+static pcap_t *pd;
+static char *program_name = "nonblocktest";
+/* Forwards */
+static void PCAP_NORETURN usage(void);
+static void PCAP_NORETURN error(const char *, ...) PCAP_PRINTFLIKE(1, 2);
+static void warning(const char *, ...) PCAP_PRINTFLIKE(1, 2);
+
+/* VARARGS */
+static void
+error(const char *fmt, ...)
+{
+       va_list ap;
+
+       (void)fprintf(stderr, "%s: ", program_name);
+       va_start(ap, fmt);
+       (void)vfprintf(stderr, fmt, ap);
+       va_end(ap);
+       if (*fmt) {
+               fmt += strlen(fmt);
+               if (fmt[-1] != '\n')
+                       (void)fputc('\n', stderr);
+       }
+       exit(1);
+       /* NOTREACHED */
+}
+
+/* VARARGS */
+static void
+warning(const char *fmt, ...)
+{
+       va_list ap;
+
+       (void)fprintf(stderr, "%s: WARNING: ", program_name);
+       va_start(ap, fmt);
+       (void)vfprintf(stderr, fmt, ap);
+       va_end(ap);
+       if (*fmt) {
+               fmt += strlen(fmt);
+               if (fmt[-1] != '\n')
+                       (void)fputc('\n', stderr);
+       }
+}
+
+static void
+usage(void)
+{
+       (void)fprintf(stderr, "Usage: %s [ -i interface ]\n",
+           program_name);
+       exit(1);
+}
+
+static void
+breakme(u_char *user _U_, const struct pcap_pkthdr *h _U_, const u_char *sp _U_)
+{
+       warning("using pcap_breakloop()");
+       pcap_breakloop(pd);
+}
+
+int
+main(int argc, char **argv)
+{
+       int status, op, i, ret;
+       char *device;
+       pcap_if_t *devlist;
+       char ebuf[PCAP_ERRBUF_SIZE];
+
+       device = NULL;
+       while ((op = getopt(argc, argv, "i:sptnq")) != -1) {
+               switch (op) {
+
+               case 'i':
+                       device = optarg;
+                       break;
+
+               default:
+                       usage();
+                       /* NOTREACHED */
+               }
+       }
+       if (device == NULL) {
+               if (pcap_findalldevs(&devlist, ebuf) == -1)
+                       error("%s", ebuf);
+               if (devlist == NULL)
+                       error("no interfaces available for capture");
+               device = strdup(devlist->name);
+               warning("listening on %s", device);
+               pcap_freealldevs(devlist);
+       }
+       *ebuf = '\0';
+       pd = pcap_create(device, ebuf);
+       if (pd == NULL)
+               error("%s", ebuf);
+       else if (*ebuf)
+               warning("%s", ebuf);
+       /* set nonblock before activate */
+       if (pcap_setnonblock(pd, 1, ebuf) < 0)
+               error("pcap_setnonblock failed: %s", ebuf);
+       /* getnonblock just returns "not activated yet" */
+       ret = pcap_getnonblock(pd, ebuf);
+       if (ret != PCAP_ERROR_NOT_ACTIVATED)
+               error("pcap_getnonblock unexpectedly succeeded");
+       if ((status = pcap_activate(pd)) < 0)
+               error("pcap_activate failed");
+       ret = pcap_getnonblock(pd, ebuf);
+       if (ret != 1)
+               error( "pcap_getnonblock did not return nonblocking" );
+
+       /* Set nonblock multiple times, ensure with strace that it's a noop */
+       for (i=0; i<10; i++) {
+               if (pcap_setnonblock(pd, 1, ebuf) < 0)
+                       error("pcap_setnonblock failed: %s", ebuf);
+               ret = pcap_getnonblock(pd, ebuf);
+               if (ret != 1)
+                       error( "pcap_getnonblock did not return nonblocking" );
+       }
+       /* Set block multiple times, ensure with strace that it's a noop */
+       for (i=0; i<10; i++) {
+               if (pcap_setnonblock(pd, 0, ebuf) < 0)
+                       error("pcap_setnonblock failed: %s", ebuf);
+               ret = pcap_getnonblock(pd, ebuf);
+               if (ret != 0)
+                       error( "pcap_getnonblock did not return blocking" );
+       }
+
+       /* Now pcap_loop forever, with a callback that
+        * uses pcap_breakloop to get out of forever */
+       pcap_loop(pd, -1, breakme, NULL);
+
+        /* Now test that pcap_setnonblock fails if we can't open the
+         * eventfd. */
+        if (pcap_setnonblock(pd, 1, ebuf) < 0)
+                error("pcap_setnonblock failed: %s", ebuf);
+        while (1) {
+                ret = open("/dev/null", O_RDONLY);
+                if (ret < 0)
+                        break;
+        }
+        ret = pcap_setnonblock(pd, 0, ebuf);
+        if (ret == 0)
+                error("pcap_setnonblock succeeded even though file table is full");
+        else
+                warning("pcap_setnonblock failed as expected: %s", ebuf);
+}