From: Remi Gacogne Date: Tue, 30 May 2017 20:34:26 +0000 (+0200) Subject: SNMP: Use our FDMultiplexer instead of select() X-Git-Tag: auth-4.1.0-rc1~3^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0e663c3b059c1618c99dec6f19f3a3bad32d10c6;p=thirdparty%2Fpdns.git SNMP: Use our FDMultiplexer instead of select() --- diff --git a/m4/pdns_with_net_snmp.m4 b/m4/pdns_with_net_snmp.m4 index 4134fc454e..8040672e6c 100644 --- a/m4/pdns_with_net_snmp.m4 +++ b/m4/pdns_with_net_snmp.m4 @@ -11,6 +11,15 @@ AC_DEFUN([PDNS_WITH_NET_SNMP], [ AS_IF([test "x$with_net_snmp" = "xyes" -o "x$with_net_snmp" = "xauto"], [ AC_CHECK_PROG([NET_SNMP_CFLAGS], [net-snmp-config], [`net-snmp-config --cflags`]) AC_CHECK_PROG([NET_SNMP_LIBS], [net-snmp-config], [`net-snmp-config --agent-libs`]) + AC_CHECK_DECLS([snmp_select_info2], [ : ], [ : ], + [AC_INCLUDES_DEFAULT + #include + #include + #include + #include + #include + #include + ]) ]) ]) AS_IF([test "x$with_net_snmp" = "xyes"], [ diff --git a/pdns/devpollmplexer.cc b/pdns/devpollmplexer.cc index 54543787a6..5e11404d1c 100644 --- a/pdns/devpollmplexer.cc +++ b/pdns/devpollmplexer.cc @@ -35,7 +35,6 @@ #include "misc.hh" #include "syncres.hh" -#include "namespaces.hh" #include "namespaces.hh" class DevPollFDMultiplexer : public FDMultiplexer @@ -47,7 +46,7 @@ public: close(d_devpollfd); } - virtual int run(struct timeval* tv); + virtual int run(struct timeval* tv, int timeout=500); virtual void addFD(callbackmap_t& cbmap, int fd, callbackfunc_t toDo, const funcparam_t& parameter); virtual void removeFD(callbackmap_t& cbmap, int fd); @@ -113,7 +112,7 @@ void DevPollFDMultiplexer::removeFD(callbackmap_t& cbmap, int fd) } } -int DevPollFDMultiplexer::run(struct timeval* now) +int DevPollFDMultiplexer::run(struct timeval* now, int timeout) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); @@ -121,7 +120,7 @@ int DevPollFDMultiplexer::run(struct timeval* now) struct dvpoll dvp; dvp.dp_nfds = d_readCallbacks.size() + d_writeCallbacks.size(); dvp.dp_fds = new pollfd[dvp.dp_nfds]; - dvp.dp_timeout = 500; + dvp.dp_timeout = timeout; int ret=ioctl(d_devpollfd, DP_POLL, &dvp); gettimeofday(now,0); // MANDATORY! @@ -151,7 +150,7 @@ int DevPollFDMultiplexer::run(struct timeval* now) } delete[] dvp.dp_fds; d_inrun=false; - return 0; + return ret; } #if 0 diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index 5ae8da3154..3193fc1a88 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -47,7 +47,11 @@ EXTRA_DIST=dnslabeltext.rl \ bpf-filter.main.ebpf \ bpf-filter.qname.ebpf \ bpf-filter.ebpf.src \ - DNSDIST-MIB.txt + DNSDIST-MIB.txt \ + devpollmplexer.cc \ + epollmplexer.cc \ + kqueuemplexer.cc \ + portsmplexer.cc bin_PROGRAMS = dnsdist @@ -93,15 +97,17 @@ dnsdist_SOURCES = \ ednscookies.cc ednscookies.hh \ ednssubnet.cc ednssubnet.hh \ gettime.cc gettime.hh \ + htmlfiles.h \ iputils.cc iputils.hh \ lock.hh \ misc.cc misc.hh \ - htmlfiles.h \ + mplexer.hh \ namespaces.hh \ pdnsexception.hh \ protobuf.cc protobuf.hh \ qtype.cc qtype.hh \ remote_logger.cc remote_logger.hh \ + selectmplexer.cc \ sholder.hh \ snmp-agent.cc snmp-agent.hh \ sodcrypto.cc sodcrypto.hh \ @@ -152,6 +158,20 @@ dnsdist.$(OBJEXT): dnsmessage.pb.cc endif endif +if HAVE_FREEBSD +dnsdist_SOURCES += kqueuemplexer.cc +endif + +if HAVE_LINUX +dnsdist_SOURCES += epollmplexer.cc +endif + +if HAVE_SOLARIS +dnsdist_SOURCES += \ + devpollmplexer.cc \ + portsmplexer.cc +endif + testrunner_SOURCES = \ base64.hh \ dns.hh \ diff --git a/pdns/dnsdistdist/devpollmplexer.cc b/pdns/dnsdistdist/devpollmplexer.cc new file mode 120000 index 0000000000..ab437856ba --- /dev/null +++ b/pdns/dnsdistdist/devpollmplexer.cc @@ -0,0 +1 @@ +../devpollmplexer.cc \ No newline at end of file diff --git a/pdns/dnsdistdist/epollmplexer.cc b/pdns/dnsdistdist/epollmplexer.cc new file mode 120000 index 0000000000..b796a57808 --- /dev/null +++ b/pdns/dnsdistdist/epollmplexer.cc @@ -0,0 +1 @@ +../epollmplexer.cc \ No newline at end of file diff --git a/pdns/dnsdistdist/kqueuemplexer.cc b/pdns/dnsdistdist/kqueuemplexer.cc new file mode 120000 index 0000000000..0824bd91df --- /dev/null +++ b/pdns/dnsdistdist/kqueuemplexer.cc @@ -0,0 +1 @@ +../kqueuemplexer.cc \ No newline at end of file diff --git a/pdns/dnsdistdist/mplexer.hh b/pdns/dnsdistdist/mplexer.hh new file mode 120000 index 0000000000..abb3c51302 --- /dev/null +++ b/pdns/dnsdistdist/mplexer.hh @@ -0,0 +1 @@ +../mplexer.hh \ No newline at end of file diff --git a/pdns/dnsdistdist/portsmplexer.cc b/pdns/dnsdistdist/portsmplexer.cc new file mode 120000 index 0000000000..d5e7107b4e --- /dev/null +++ b/pdns/dnsdistdist/portsmplexer.cc @@ -0,0 +1 @@ +../portsmplexer.cc \ No newline at end of file diff --git a/pdns/dnsdistdist/selectmplexer.cc b/pdns/dnsdistdist/selectmplexer.cc new file mode 120000 index 0000000000..85b38bcdce --- /dev/null +++ b/pdns/dnsdistdist/selectmplexer.cc @@ -0,0 +1 @@ +../selectmplexer.cc \ No newline at end of file diff --git a/pdns/epollmplexer.cc b/pdns/epollmplexer.cc index 9ab4ebf48e..2deae83b76 100644 --- a/pdns/epollmplexer.cc +++ b/pdns/epollmplexer.cc @@ -31,7 +31,6 @@ #include #endif -#include "namespaces.hh" #include "namespaces.hh" class EpollFDMultiplexer : public FDMultiplexer @@ -43,7 +42,7 @@ public: close(d_epollfd); } - virtual int run(struct timeval* tv); + virtual int run(struct timeval* tv, int timeout=500); virtual void addFD(callbackmap_t& cbmap, int fd, callbackfunc_t toDo, const funcparam_t& parameter); virtual void removeFD(callbackmap_t& cbmap, int fd); @@ -124,13 +123,13 @@ void EpollFDMultiplexer::removeFD(callbackmap_t& cbmap, int fd) throw FDMultiplexerException("Removing fd from epoll set: "+stringerror()); } -int EpollFDMultiplexer::run(struct timeval* now) +int EpollFDMultiplexer::run(struct timeval* now, int timeout) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); } - int ret=epoll_wait(d_epollfd, d_eevents.get(), s_maxevents, 500); + int ret=epoll_wait(d_epollfd, d_eevents.get(), s_maxevents, timeout); gettimeofday(now,0); // MANDATORY if(ret < 0 && errno!=EINTR) @@ -154,7 +153,7 @@ int EpollFDMultiplexer::run(struct timeval* now) } } d_inrun=false; - return 0; + return ret; } #if 0 diff --git a/pdns/kqueuemplexer.cc b/pdns/kqueuemplexer.cc index e0eefc8765..b3510c501f 100644 --- a/pdns/kqueuemplexer.cc +++ b/pdns/kqueuemplexer.cc @@ -27,14 +27,12 @@ #include #include #include "misc.hh" -#include "syncres.hh" #include #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) #include #endif #include -#include "namespaces.hh" #include "namespaces.hh" class KqueueFDMultiplexer : public FDMultiplexer @@ -46,7 +44,7 @@ public: close(d_kqueuefd); } - virtual int run(struct timeval* tv); + virtual int run(struct timeval* tv, int timeout=500); virtual void addFD(callbackmap_t& cbmap, int fd, callbackfunc_t toDo, const boost::any& parameter); virtual void removeFD(callbackmap_t& cbmap, int fd); @@ -105,15 +103,15 @@ void KqueueFDMultiplexer::removeFD(callbackmap_t& cbmap, int fd) throw FDMultiplexerException("Removing fd from kqueue set: "+stringerror()); } -int KqueueFDMultiplexer::run(struct timeval* now) +int KqueueFDMultiplexer::run(struct timeval* now, int timeout) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); } struct timespec ts; - ts.tv_sec=0; - ts.tv_nsec=500000000U; + ts.tv_sec=timeout/1000; + ts.tv_nsec=(timeout % 1000) * 1000000; int ret=kevent(d_kqueuefd, 0, 0, d_kevents.get(), s_maxevents, &ts); gettimeofday(now,0); // MANDATORY! @@ -141,7 +139,7 @@ int KqueueFDMultiplexer::run(struct timeval* now) } d_inrun=false; - return 0; + return ret; } #if 0 diff --git a/pdns/mplexer.hh b/pdns/mplexer.hh index 7ab348151b..3e9c8cd3eb 100644 --- a/pdns/mplexer.hh +++ b/pdns/mplexer.hh @@ -30,7 +30,6 @@ #include #include #include -#include "utility.hh" class FDMultiplexerException : public std::runtime_error { @@ -50,7 +49,6 @@ public: class FDMultiplexer { public: - // typedef boost::variant funcparam_t; typedef boost::any funcparam_t; protected: @@ -68,7 +66,9 @@ public: virtual ~FDMultiplexer() {} - virtual int run(struct timeval* tv) = 0; + /* tv will be updated to 'now' before run returns */ + /* timeout is in ms */ + virtual int run(struct timeval* tv, int timeout=500) = 0; //! Add an fd to the read watch list - currently an fd can only be on one list at a time! virtual void addReadFD(int fd, callbackfunc_t toDo, const funcparam_t& parameter=funcparam_t()) @@ -131,7 +131,6 @@ public: virtual std::string getName() = 0; - protected: typedef std::map callbackmap_t; callbackmap_t d_readCallbacks, d_writeCallbacks; @@ -168,7 +167,7 @@ public: virtual ~SelectFDMultiplexer() {} - virtual int run(struct timeval* tv); + virtual int run(struct timeval* tv, int timeout=500); virtual void addFD(callbackmap_t& cbmap, int fd, callbackfunc_t toDo, const funcparam_t& parameter); virtual void removeFD(callbackmap_t& cbmap, int fd); diff --git a/pdns/pollmplexer.cc b/pdns/pollmplexer.cc index 08f126eb4e..5f1a6ffb8e 100644 --- a/pdns/pollmplexer.cc +++ b/pdns/pollmplexer.cc @@ -6,9 +6,6 @@ #include #include #include "misc.hh" -#include "syncres.hh" -#include "utility.hh" -#include "namespaces.hh" #include "namespaces.hh" @@ -49,7 +46,7 @@ bool pollfdcomp(const struct pollfd& a, const struct pollfd& b) return a.fd < b.fd; } -int PollFDMultiplexer::run(struct timeval* now) +int PollFDMultiplexer::run(struct timeval* now, int timeout=500) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); @@ -70,15 +67,15 @@ int PollFDMultiplexer::run(struct timeval* now) pollfds.push_back(pollfd); } - int ret=poll(&pollfds[0], pollfds.size(), 500); - Utility::gettimeofday(now, 0); // MANDATORY! + int ret=poll(&pollfds[0], pollfds.size(), timeout); + gettimeofday(now, 0); // MANDATORY! if(ret < 0 && errno!=EINTR) throw FDMultiplexerException("poll returned error: "+stringerror()); d_iter=d_readCallbacks.end(); d_inrun=true; - + for(unsigned int n = 0; n < pollfds.size(); ++n) { if(pollfds[n].revents == POLLIN) { d_iter=d_readCallbacks.find(pollfds[n].fd); @@ -97,7 +94,7 @@ int PollFDMultiplexer::run(struct timeval* now) } } d_inrun=false; - return 0; + return ret; } #if 0 diff --git a/pdns/portsmplexer.cc b/pdns/portsmplexer.cc index 58d05f7eee..c6e91e60fc 100644 --- a/pdns/portsmplexer.cc +++ b/pdns/portsmplexer.cc @@ -11,9 +11,7 @@ #include #include "misc.hh" -#include "syncres.hh" -#include "namespaces.hh" #include "namespaces.hh" class PortsFDMultiplexer : public FDMultiplexer @@ -25,7 +23,7 @@ public: close(d_portfd); } - virtual int run(struct timeval* tv); + virtual int run(struct timeval* tv, int timeout=500); virtual void addFD(callbackmap_t& cbmap, int fd, callbackfunc_t toDo, const boost::any& parameter); virtual void removeFD(callbackmap_t& cbmap, int fd); @@ -80,16 +78,17 @@ void PortsFDMultiplexer::removeFD(callbackmap_t& cbmap, int fd) throw FDMultiplexerException("Removing fd from port set: "+stringerror()); } -int PortsFDMultiplexer::run(struct timeval* now) +int PortsFDMultiplexer::run(struct timeval* now, int timeout) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); } - struct timespec timeout; - timeout.tv_sec=0; timeout.tv_nsec=500000000; + struct timespec timeoutspec; + timeoutspec.tv_sec = time / 1000; + timeoutspec.tv_nsec = (time % 1000) * 1000000; unsigned int numevents=1; - int ret= port_getn(d_portfd, d_pevents.get(), min(PORT_MAX_LIST, s_maxevents), &numevents, &timeout); + int ret= port_getn(d_portfd, d_pevents.get(), min(PORT_MAX_LIST, s_maxevents), &numevents, &timeoutspec); /* port_getn has an unusual API - (ret == -1, errno == ETIME) can mean partial success; you must check (*numevents) in this case @@ -133,7 +132,7 @@ int PortsFDMultiplexer::run(struct timeval* now) } d_inrun=false; - return 0; + return numevents; } #if 0 diff --git a/pdns/selectmplexer.cc b/pdns/selectmplexer.cc index 857dec6570..9e3d6f7c90 100644 --- a/pdns/selectmplexer.cc +++ b/pdns/selectmplexer.cc @@ -5,10 +5,7 @@ #include "sstuff.hh" #include #include "misc.hh" -#include "utility.hh" - -#include "namespaces.hh" #include "namespaces.hh" static FDMultiplexer* make() @@ -43,7 +40,7 @@ void SelectFDMultiplexer::removeFD(callbackmap_t& cbmap, int fd) throw FDMultiplexerException("Tried to remove unlisted fd "+std::to_string(fd)+ " from multiplexer"); } -int SelectFDMultiplexer::run(struct timeval* now) +int SelectFDMultiplexer::run(struct timeval* now, int timeout) { if(d_inrun) { throw FDMultiplexerException("FDMultiplexer::run() is not reentrant!\n"); @@ -64,9 +61,9 @@ int SelectFDMultiplexer::run(struct timeval* now) fdmax=max(i->first, fdmax); } - struct timeval tv={0,500000}; + struct timeval tv={timeout / 1000 , (timeout % 1000) * 1000}; int ret=select(fdmax + 1, &readfds, &writefds, 0, &tv); - Utility::gettimeofday(now, 0); // MANDATORY! + gettimeofday(now, 0); // MANDATORY! if(ret < 0 && errno!=EINTR) throw FDMultiplexerException("select returned error: "+stringerror()); @@ -76,12 +73,14 @@ int SelectFDMultiplexer::run(struct timeval* now) d_iter=d_readCallbacks.end(); d_inrun=true; - + + int got = 0; for(callbackmap_t::iterator i=d_readCallbacks.begin(); i != d_readCallbacks.end() && i->first <= fdmax; ) { d_iter=i++; if(FD_ISSET(d_iter->first, &readfds)) { d_iter->second.d_callback(d_iter->first, d_iter->second.d_parameter); + got++; continue; // so we don't refind ourselves as writable } } @@ -90,11 +89,12 @@ int SelectFDMultiplexer::run(struct timeval* now) d_iter=i++; if(FD_ISSET(d_iter->first, &writefds)) { d_iter->second.d_callback(d_iter->first, d_iter->second.d_parameter); + got++; } } d_inrun=false; - return 0; + return got; } #if 0 diff --git a/pdns/snmp-agent.cc b/pdns/snmp-agent.cc index ad76f1d87a..eecb36ebf8 100644 --- a/pdns/snmp-agent.cc +++ b/pdns/snmp-agent.cc @@ -3,6 +3,22 @@ #ifdef HAVE_NET_SNMP +#ifndef HAVE_SNMP_SELECT_INFO2 +/* that's terrible, because it means we are going to have trouble with large + FD numbers at some point.. */ +# define netsnmp_large_fd_set fd_set +# define snmp_read2 snmp_read +# define snmp_select_info2 snmp_select_info +# define netsnmp_large_fd_set_init(...) +# define netsnmp_large_fd_set_cleanup(...) +# define NETSNMP_LARGE_FD_SET FD_SET +# define NETSNMP_LARGE_FD_CLR FD_CLR +# define NETSNMP_LARGE_FD_ZERO FD_ZERO +# define NETSNMP_LARGE_FD_ISSET FD_ISSET +#else +# include +#endif + const oid SNMPAgent::snmpTrapOID[] = { 1, 3, 6, 1, 6, 3, 1, 1, 4, 1, 0 }; const size_t SNMPAgent::snmpTrapOIDLen = OID_LENGTH(SNMPAgent::snmpTrapOID); @@ -31,7 +47,7 @@ bool SNMPAgent::sendTrap(int fd, return true; } -void SNMPAgent::handleTraps() +void SNMPAgent::handleTrapsEvent() { netsnmp_variable_list* varList = nullptr; ssize_t got = 0; @@ -46,44 +62,104 @@ void SNMPAgent::handleTraps() } while (got > 0); } + +void SNMPAgent::handleSNMPQueryEvent(int fd) +{ + netsnmp_large_fd_set fdset; + netsnmp_large_fd_set_init(&fdset, FD_SETSIZE); + NETSNMP_LARGE_FD_ZERO(&fdset); + NETSNMP_LARGE_FD_SET(fd, &fdset); + snmp_read2(&fdset); +} + +void SNMPAgent::handleTrapsCB(int fd, FDMultiplexer::funcparam_t& var) +{ + SNMPAgent** agent = boost::any_cast(&var); + if (!agent || !*agent) + throw std::runtime_error("Invalid value received in SNMP trap callback"); + + (*agent)->handleTrapsEvent(); +} + +void SNMPAgent::handleSNMPQueryCB(int fd, FDMultiplexer::funcparam_t& var) +{ + SNMPAgent** agent = boost::any_cast(&var); + if (!agent || !*agent) + throw std::runtime_error("Invalid value received in SNMP trap callback"); + + (*agent)->handleSNMPQueryEvent(fd); +} + +static FDMultiplexer* getMultiplexer() +{ + FDMultiplexer* ret = nullptr; + for(const auto& i : FDMultiplexer::getMultiplexerMap()) { + try { + ret = i.second(); + return ret; + } + catch(const FDMultiplexerException& fe) { + } + catch(...) { + } + } + return ret; +} + #endif /* HAVE_NET_SNMP */ void SNMPAgent::worker() { #ifdef HAVE_NET_SNMP - int numfds = 0; + FDMultiplexer* mplexer = getMultiplexer(); + if (mplexer == nullptr) { + throw std::runtime_error("No FD multiplexer found for the SNMP agent!"); + } + + int maxfd = 0; int block = 1; - fd_set fdset; + netsnmp_large_fd_set fdset; struct timeval timeout = { 0, 0 }; + struct timeval now; + + /* we want to be notified if a trap is waiting + to be sent */ + mplexer->addReadFD(d_trapPipe[0], &handleTrapsCB, this); while(true) { - numfds = FD_SETSIZE; + netsnmp_large_fd_set_init(&fdset, FD_SETSIZE); + NETSNMP_LARGE_FD_ZERO(&fdset); - FD_ZERO(&fdset); - FD_SET(d_trapPipe[0], &fdset); block = 1; timeout = { 0, 0 }; - snmp_select_info(&numfds, &fdset, &timeout, &block); + snmp_select_info2(&maxfd, &fdset, &timeout, &block); - int res = select(FD_SETSIZE, &fdset, nullptr, nullptr, block ? nullptr : &timeout); - - if (res == 2) { - FD_CLR(d_trapPipe[0], &fdset); - snmp_read(&fdset); - handleTraps(); - } - else if (res == 1) - { - if (FD_ISSET(d_trapPipe[0], &fdset)) { - handleTraps(); - } else { - snmp_read(&fdset); + for (int fd = 0; fd < maxfd; fd++) { + if (NETSNMP_LARGE_FD_ISSET(fd, &fdset)) { + mplexer->addReadFD(fd, &handleSNMPQueryCB, this); } } - else if (res == 0) { + + /* run updates now */ + int res = mplexer->run(&now, (timeout.tv_sec * 1000) + (timeout.tv_usec / 1000)); + + /* we handle timeouts here, the rest has already been handled by callbacks */ + if (res == 0) { snmp_timeout(); run_alarms(); } + + for (int fd = 0; fd < maxfd; fd++) { + if (NETSNMP_LARGE_FD_ISSET(fd, &fdset)) { + try { + mplexer->removeReadFD(fd); + } + catch(const FDMultiplexerException& e) { + /* we might get an exception when removing a closed file descriptor, + just ignore it */ + } + } + } } #endif /* HAVE_NET_SNMP */ } diff --git a/pdns/snmp-agent.hh b/pdns/snmp-agent.hh index 4e2afe2466..5d693bb6bc 100644 --- a/pdns/snmp-agent.hh +++ b/pdns/snmp-agent.hh @@ -17,6 +17,8 @@ #undef INET6 /* SRSLY? */ #endif /* HAVE_NET_SNMP */ +#include "mplexer.hh" + class SNMPAgent { public: @@ -49,12 +51,14 @@ protected: static bool sendTrap(int fd, netsnmp_variable_list* varList); - void handleTraps(); - int d_trapPipe[2] = { -1, -1}; #endif /* HAVE_NET_SNMP */ private: void worker(); + static void handleTrapsCB(int fd, FDMultiplexer::funcparam_t& var); + static void handleSNMPQueryCB(int fd, FDMultiplexer::funcparam_t& var); + void handleTrapsEvent(); + void handleSNMPQueryEvent(int fd); std::thread d_thread; };