From ca428233ba1957e93edf778cb7a38aaa484c15ce Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Wed, 18 Nov 2020 22:24:18 -0500 Subject: [PATCH] Revert "Try another optimization in LinuxEthernetTap." This reverts commit a390629371970be9dffa9db7cd8af2a7004ee50c. --- osdep/LinuxEthernetTap.cpp | 102 +++++++++++++++++++++++-------------- osdep/LinuxEthernetTap.hpp | 1 + 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/osdep/LinuxEthernetTap.cpp b/osdep/LinuxEthernetTap.cpp index 4659df556..25ed52edf 100644 --- a/osdep/LinuxEthernetTap.cpp +++ b/osdep/LinuxEthernetTap.cpp @@ -182,7 +182,14 @@ LinuxEthernetTap::LinuxEthernetTap( _dev = ifr.ifr_name; ::fcntl(_fd,F_SETFD,fcntl(_fd,F_GETFD) | FD_CLOEXEC); + (void)::pipe(_shutdownSignalPipe); + _tapReaderThread = std::thread([this]{ + fd_set readfds,nullfds; + int n,nfds,r; + void *buf = nullptr; + std::vector buffers; + { struct ifreq ifr; memset(&ifr,0,sizeof(ifr)); @@ -225,51 +232,67 @@ LinuxEthernetTap::LinuxEthernetTap( return; } + fcntl(_fd,F_SETFL,O_NONBLOCK); + ::close(sock); } - std::vector buffers; - void *buf = nullptr; - for(int r=0;;) { - if (!buf) { - // To reduce use of the mutex, we keep a local buffer vector and - // swap (which is a pointer swap) with the global one when it's - // empty. This retrieves a batch of buffers to use. - if (buffers.empty()) { - std::lock_guard l(_buffers_l); - buffers.swap(_buffers); - } - if (buffers.empty()) { - buf = malloc(ZT_TAP_BUF_SIZE); - if (!buf) - break; - } else { - buf = buffers.back(); - buffers.pop_back(); - } - } + FD_ZERO(&readfds); + FD_ZERO(&nullfds); + nfds = (int)std::max(_shutdownSignalPipe[0],_fd) + 1; - const int n = (int)::read(_fd,reinterpret_cast(buf) + r,ZT_TAP_BUF_SIZE - r); + r = 0; + for(;;) { + FD_SET(_shutdownSignalPipe[0],&readfds); + FD_SET(_fd,&readfds); + select(nfds,&readfds,&nullfds,&nullfds,(struct timeval *)0); - if (n > 0) { - // Some tap drivers like to send the ethernet frame and the - // payload in two chunks, so handle that by accumulating - // data until we have at least a frame. - r += n; - if (r > 14) { - if (r > ((int)_mtu + 14)) // sanity check for weird TAP behavior on some platforms - r = _mtu + 14; + if (FD_ISSET(_shutdownSignalPipe[0],&readfds)) // writes to shutdown pipe terminate thread + break; - if (_enabled) { - _tapq.post(std::pair(buf,r)); - buf = nullptr; + if (FD_ISSET(_fd,&readfds)) { + for(;;) { // read until there are no more packets, then return to outer select() loop + if (!buf) { + // To reduce use of the mutex, we keep a local buffer vector and + // swap (which is a pointer swap) with the global one when it's + // empty. This retrieves a batch of buffers to use. + if (buffers.empty()) { + std::lock_guard l(_buffers_l); + buffers.swap(_buffers); + } + if (buffers.empty()) { + buf = malloc(ZT_TAP_BUF_SIZE); + if (!buf) + break; + } else { + buf = buffers.back(); + buffers.pop_back(); + } } - r = 0; + n = (int)::read(_fd,reinterpret_cast(buf) + r,ZT_TAP_BUF_SIZE - r); + + if (n > 0) { + // Some tap drivers like to send the ethernet frame and the + // payload in two chunks, so handle that by accumulating + // data until we have at least a frame. + r += n; + if (r > 14) { + if (r > ((int)_mtu + 14)) // sanity check for weird TAP behavior on some platforms + r = _mtu + 14; + + if (_enabled) { + _tapq.post(std::pair(buf,r)); + buf = nullptr; + } + + r = 0; + } + } else { + r = 0; + break; + } } - } else { - r = 0; - break; } } }); @@ -297,9 +320,12 @@ LinuxEthernetTap::LinuxEthernetTap( LinuxEthernetTap::~LinuxEthernetTap() { - _tapq.post(std::pair(nullptr,0)); - ::shutdown(_fd, SHUT_RDWR); + (void)::write(_shutdownSignalPipe[1],"\0",1); // causes reader thread to exit + _tapq.post(std::pair(nullptr,0)); // causes processor thread to exit + ::close(_fd); + ::close(_shutdownSignalPipe[0]); + ::close(_shutdownSignalPipe[1]); _tapReaderThread.join(); _tapProcessorThread.join(); diff --git a/osdep/LinuxEthernetTap.hpp b/osdep/LinuxEthernetTap.hpp index f296ec693..9e9206ead 100644 --- a/osdep/LinuxEthernetTap.hpp +++ b/osdep/LinuxEthernetTap.hpp @@ -68,6 +68,7 @@ private: std::vector _multicastGroups; unsigned int _mtu; int _fd; + int _shutdownSignalPipe[2]; std::atomic_bool _enabled; std::thread _tapReaderThread; std::thread _tapProcessorThread;