From 56e2c5e8471704b3f28210a96f3e5ef7e8557b97 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 28 Jul 2016 17:23:43 +0200 Subject: [PATCH] lib: AgentX: use threads instead of eventloop hack AgentX fd/timeout handling is rather hackishly monkeyed into thread.c. Replace with code that uses plain thread_* functions. NB: Net-SNMP's API rivals Quagga's in terms of age and absence of documentation. netsnmp_check_outstanding_agent_requests() in particular seems to be unused and is therefore untested. The most useful documentation on this is actually the blog post Vincent Bernat wrote when he originally integrated this into lldpd and Quagga: https://vincent.bernat.im/en/blog/2012-snmp-event-loop.html Signed-off-by: David Lamparter --- lib/agentx.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++- lib/thread.c | 82 +--------------------------------------- 2 files changed, 106 insertions(+), 81 deletions(-) diff --git a/lib/agentx.c b/lib/agentx.c index 9dc5b47de3..5996b34a0f 100644 --- a/lib/agentx.c +++ b/lib/agentx.c @@ -24,12 +24,110 @@ #if defined HAVE_SNMP && defined SNMP_AGENTX #include #include +#include +#include #include "command.h" #include "smux.h" #include "memory.h" +#include "linklist.h" -int agentx_enabled = 0; +static int agentx_enabled = 0; + +static struct thread_master *agentx_tm; +static struct thread *timeout_thr = NULL; +static struct list *events = NULL; + +static void agentx_events_update(void); + +static int +agentx_timeout(struct thread *t) +{ + timeout_thr = NULL; + + snmp_timeout (); + run_alarms (); + netsnmp_check_outstanding_agent_requests (); + agentx_events_update (); + return 0; +} + +static int +agentx_read(struct thread *t) +{ + fd_set fds; + struct listnode *ln = THREAD_ARG (t); + list_delete_node (events, ln); + + FD_ZERO (&fds); + FD_SET (THREAD_FD (t), &fds); + snmp_read (&fds); + + netsnmp_check_outstanding_agent_requests (); + agentx_events_update (); + return 0; +} + +static void +agentx_events_update(void) +{ + int maxfd = 0; + int block = 1; + struct timeval timeout = { .tv_sec = 0, .tv_usec = 0 }; + fd_set fds; + struct listnode *ln; + struct thread *thr; + int fd, thr_fd; + + THREAD_OFF (timeout_thr); + + FD_ZERO (&fds); + snmp_select_info (&maxfd, &fds, &timeout, &block); + + if (!block) + timeout_thr = thread_add_timer_tv (agentx_tm, agentx_timeout, NULL, &timeout); + + ln = listhead (events); + thr = ln ? listgetdata (ln) : NULL; + thr_fd = thr ? THREAD_FD (thr) : -1; + + /* "two-pointer" / two-list simultaneous iteration + * ln/thr/thr_fd point to the next existing event listener to hit while + * fd counts to catch up */ + for (fd = 0; fd < maxfd; fd++) + { + /* caught up */ + if (thr_fd == fd) + { + struct listnode *nextln = listnextnode (ln); + if (!FD_ISSET (fd, &fds)) + { + thread_cancel (thr); + list_delete_node (events, ln); + } + ln = nextln; + thr = ln ? listgetdata (ln) : NULL; + thr_fd = thr ? THREAD_FD (thr) : -1; + } + /* need listener, but haven't hit one where it would be */ + else if (FD_ISSET (fd, &fds)) + { + struct listnode *newln; + thr = thread_add_read (agentx_tm, agentx_read, NULL, fd); + newln = listnode_add_before (events, ln, thr); + thr->arg = newln; + } + } + + /* leftover event listeners at this point have fd > maxfd, delete them */ + while (ln) + { + struct listnode *nextln = listnextnode (ln); + thread_cancel (listgetdata (ln)); + list_delete_node (events, ln); + ln = nextln; + } +} /* AgentX node. */ static struct cmd_node agentx_node = @@ -78,6 +176,8 @@ DEFUN (agentx_enable, if (!agentx_enabled) { init_snmp("quagga"); + events = list_new(); + agentx_events_update (); agentx_enabled = 1; return CMD_SUCCESS; } @@ -100,6 +200,8 @@ DEFUN (no_agentx, void smux_init (struct thread_master *tm) { + agentx_tm = tm; + netsnmp_enable_subagent (); snmp_disable_log (); snmp_enable_calllog (); @@ -208,6 +310,7 @@ smux_trap (struct variable *vp, size_t vp_len, send_v2trap (notification_vars); snmp_free_varbind (notification_vars); + agentx_events_update (); return 1; } diff --git a/lib/thread.c b/lib/thread.c index f3b0bdf8a5..a128786c3d 100644 --- a/lib/thread.c +++ b/lib/thread.c @@ -32,27 +32,6 @@ #include "command.h" #include "sigevent.h" -#if defined HAVE_SNMP && defined SNMP_AGENTX - -#ifdef HAVE_POLL -#define QUAGGA_HAVE_POLL -#endif - -#include -#include -#include -#include - -#ifdef HAVE_POLL -#undef HAVE_POLL -#endif -#ifdef QUAGGA_HAVE_POLL -#define HAVE_POLL -#endif - -extern int agentx_enabled; -#endif - #if defined(__APPLE__) #include #include @@ -1312,12 +1291,7 @@ thread_fetch (struct thread_master *m, struct thread *fetch) while (1) { int num = 0; -#if defined HAVE_SNMP && defined SNMP_AGENTX - struct timeval snmp_timer_wait; - int snmpblock = 0; - int fdsetsize; -#endif - + /* Signals pre-empt everything */ quagga_sigevent_process (); @@ -1353,35 +1327,7 @@ thread_fetch (struct thread_master *m, struct thread *fetch) (!timer_wait || (timeval_cmp (*timer_wait, *timer_wait_bg) > 0))) timer_wait = timer_wait_bg; } - -#if defined HAVE_SNMP && defined SNMP_AGENTX - /* When SNMP is enabled, we may have to select() on additional - FD. snmp_select_info() will add them to `readfd'. The trick - with this function is its last argument. We need to set it to - 0 if timer_wait is not NULL and we need to use the provided - new timer only if it is still set to 0. */ - if (agentx_enabled) - { - fdsetsize = FD_SETSIZE; - snmpblock = 1; - if (timer_wait) - { - snmpblock = 0; - memcpy(&snmp_timer_wait, timer_wait, sizeof(struct timeval)); - } -#if defined(HAVE_POLL) - /* clear fdset since there are no other fds in fd_set, - then add injected fds from snmp_select_info into pollset */ - FD_ZERO(&readfd); -#endif - snmp_select_info(&fdsetsize, &readfd, &snmp_timer_wait, &snmpblock); -#if defined(HAVE_POLL) - add_snmp_pollfds(m, &readfd, fdsetsize); -#endif - if (snmpblock == 0) - timer_wait = &snmp_timer_wait; - } -#endif + num = fd_select (m, FD_SETSIZE, &readfd, &writefd, &exceptfd, timer_wait); /* Signals should get quick treatment */ @@ -1393,30 +1339,6 @@ thread_fetch (struct thread_master *m, struct thread *fetch) return NULL; } -#if defined HAVE_SNMP && defined SNMP_AGENTX -#if defined(HAVE_POLL) - /* re-enter pollfds in fd_set for handling in snmp_read */ - FD_ZERO(&readfd); - nfds_t i; - for (i = m->handler.pfdcount; i < m->handler.pfdcountsnmp; ++i) - { - if (m->handler.pfds[i].revents == POLLIN) - FD_SET(m->handler.pfds[i].fd, &readfd); - } -#endif - if (agentx_enabled) - { - if (num > 0) - snmp_read(&readfd); - else if (num == 0) - { - snmp_timeout(); - run_alarms(); - } - netsnmp_check_outstanding_agent_requests(); - } -#endif - /* Check foreground timers. Historically, they have had higher priority than I/O threads, so let's push them onto the ready list in front of the I/O threads. */