]> git.ipfire.org Git - thirdparty/unbound.git/commitdiff
- Fix for accept spinning reported by OpenBSD.
authorWouter Wijngaards <wouter@nlnetlabs.nl>
Tue, 8 May 2012 12:08:55 +0000 (12:08 +0000)
committerWouter Wijngaards <wouter@nlnetlabs.nl>
Tue, 8 May 2012 12:08:55 +0000 (12:08 +0000)
git-svn-id: file:///svn/unbound/trunk@2663 be551aaa-1e26-0410-a405-d3ace91eadb9

14 files changed:
daemon/remote.c
daemon/remote.h
daemon/worker.c
daemon/worker.h
doc/Changelog
libunbound/libworker.c
services/listen_dnsport.c
services/listen_dnsport.h
smallapp/worker_cb.c
testcode/fake_event.c
util/fptr_wlist.c
util/fptr_wlist.h
util/netevent.c
util/netevent.h

index 6f0625e16dd63bec5814d96fb3df25d9b35aa4e1..38ca15c85cdbb4f2b64bf523090c981d3be9c890 100644 (file)
@@ -362,6 +362,22 @@ int daemon_remote_open_accept(struct daemon_remote* rc,
        return 1;
 }
 
+void daemon_remote_stop_accept(struct daemon_remote* rc)
+{
+       struct listen_list* p;
+       for(p=rc->accept_list; p; p=p->next) {
+               comm_point_stop_listening(p->com);      
+       }
+}
+
+void daemon_remote_start_accept(struct daemon_remote* rc)
+{
+       struct listen_list* p;
+       for(p=rc->accept_list; p; p=p->next) {
+               comm_point_start_listening(p->com, -1, -1);     
+       }
+}
+
 int remote_accept_callback(struct comm_point* c, void* arg, int err, 
        struct comm_reply* ATTR_UNUSED(rep))
 {
index 1edc1b887bd6af682ae86370fc9695a5a31a9b26..5919be4f2a3e2c484aad869e3b534fe92475d7b7 100644 (file)
@@ -135,6 +135,18 @@ struct listen_port* daemon_remote_open_ports(struct config_file* cfg);
 int daemon_remote_open_accept(struct daemon_remote* rc, 
        struct listen_port* ports, struct worker* worker);
 
+/**
+ * Stop accept handlers for TCP (until enabled again)
+ * @param rc: state
+ */
+void daemon_remote_stop_accept(struct daemon_remote* rc);
+
+/**
+ * Stop accept handlers for TCP (until enabled again)
+ * @param rc: state
+ */
+void daemon_remote_start_accept(struct daemon_remote* rc);
+
 /**
  * Handle nonthreaded remote cmd execution.
  * @param worker: this worker (the remote worker).
index 3c85fa37f1f43b62672f0398bacaed2226a66eaa..e84c54b59d7a791b82125cabb5f732baa3dcf132 100644 (file)
@@ -1057,6 +1057,8 @@ worker_init(struct worker* worker, struct config_file *cfg,
                worker_delete(worker);
                return 0;
        }
+       comm_base_set_slow_accept_handlers(worker->base, &worker_stop_accept,
+               &worker_start_accept, worker);
        if(do_sigs) {
 #ifdef SIGHUP
                ub_thread_sig_unblock(SIGHUP);
@@ -1290,6 +1292,22 @@ void worker_stats_clear(struct worker* worker)
        worker->back->unwanted_replies = 0;
 }
 
+void worker_start_accept(void* arg)
+{
+       struct worker* worker = (struct worker*)arg;
+       listen_start_accept(worker->front);
+       if(worker->thread_num == 0)
+               daemon_remote_start_accept(worker->daemon->rc);
+}
+
+void worker_stop_accept(void* arg)
+{
+       struct worker* worker = (struct worker*)arg;
+       listen_stop_accept(worker->front);
+       if(worker->thread_num == 0)
+               daemon_remote_stop_accept(worker->daemon->rc);
+}
+
 /* --- fake callbacks for fptr_wlist to work --- */
 struct outbound_entry* libworker_send_query(uint8_t* ATTR_UNUSED(qname), 
        size_t ATTR_UNUSED(qnamelen), uint16_t ATTR_UNUSED(qtype), 
index 96c04676a11bce660891819ce1b1da9a75312fe2..c510ebfd735cb67bc050451e7245620c78ed5812 100644 (file)
@@ -225,4 +225,10 @@ void worker_stat_timer_cb(void* arg);
 /** probe timer callback handler */
 void worker_probe_timer_cb(void* arg);
 
+/** start accept callback handler */
+void worker_start_accept(void* arg);
+
+/** stop accept callback handler */
+void worker_stop_accept(void* arg);
+
 #endif /* DAEMON_WORKER_H */
index 2d2e5bc28f15dfa6fdff92189cfeeed88036eaa9..c6b6d17d62bf68d7aa85b4fe2abbf45cad7cd3d5 100644 (file)
@@ -1,3 +1,6 @@
+8 May 2012: Wouter
+       - Fix for accept spinning reported by OpenBSD.
+
 2 May 2012: Wouter
        - Fix validation of nodata for DS query in NSEC zones, reported by
          Ondrej Mikle.
index c4e71cf6ac89c5029c151871621008954aa73d72..917a9106d078546ea73e573e9188619747548198 100644 (file)
@@ -871,6 +871,16 @@ void worker_probe_timer_cb(void* ATTR_UNUSED(arg))
        log_assert(0);
 }
 
+void worker_start_accept(void* ATTR_UNUSED(arg))
+{
+       log_assert(0);
+}
+
+void worker_stop_accept(void* ATTR_UNUSED(arg))
+{
+       log_assert(0);
+}
+
 int order_lock_cmp(const void* ATTR_UNUSED(e1), const void* ATTR_UNUSED(e2))
 {
        log_assert(0);
index ea7ec3a66035aaca165adb8660bacb36ae9492cb..59ca1991eb12141cfc9fd1b442f546bc907c11ff 100644 (file)
@@ -915,3 +915,30 @@ size_t listen_get_mem(struct listen_dnsport* listen)
        }
        return s;
 }
+
+void listen_stop_accept(struct listen_dnsport* listen)
+{
+       /* do not stop the ones that have no tcp_free list
+        * (they have already stopped listening) */
+       struct listen_list* p;
+       for(p=listen->cps; p; p=p->next) {
+               if(p->com->type == comm_tcp_accept &&
+                       p->com->tcp_free != NULL) {
+                       comm_point_stop_listening(p->com);
+               }
+       }
+}
+
+void listen_start_accept(struct listen_dnsport* listen)
+{
+       /* do not start the ones that have no tcp_free list, it is no
+        * use to listen to them because they have no free tcp handlers */
+       struct listen_list* p;
+       for(p=listen->cps; p; p=p->next) {
+               if(p->com->type == comm_tcp_accept &&
+                       p->com->tcp_free != NULL) {
+                       comm_point_start_listening(p->com, -1, -1);
+               }
+       }
+}
+
index 22fa82804535a1db1938b8095e7d18f855e34467..4d37aca5eee476897ffaa47cf372af73de680892 100644 (file)
@@ -153,6 +153,18 @@ void listen_list_delete(struct listen_list* list);
  */
 size_t listen_get_mem(struct listen_dnsport* listen);
 
+/**
+ * stop accept handlers for TCP (until enabled again)
+ * @param listen: listening structure.
+ */
+void listen_stop_accept(struct listen_dnsport* listen);
+
+/**
+ * start accept handlers for TCP (was stopped before)
+ * @param listen: listening structure.
+ */
+void listen_start_accept(struct listen_dnsport* listen);
+
 /**
  * Create and bind nonblocking UDP socket
  * @param family: for socket call.
index 8e86caaeb5741fc20c173c271664462a8ab8cd3b..bc37e3307ae432816db3870da6ddabcf5f16c343 100644 (file)
@@ -195,6 +195,16 @@ void worker_probe_timer_cb(void* ATTR_UNUSED(arg))
        log_assert(0);
 }
 
+void worker_start_accept(void* ATTR_UNUSED(arg))
+{
+       log_assert(0);
+}
+
+void worker_stop_accept(void* ATTR_UNUSED(arg))
+{
+       log_assert(0);
+}
+
 /** keep track of lock id in lock-verify application */
 struct order_id {
         /** the thread id that created it */
index 159ea30896d2c0b1d7ae8a25198d2234f326fa04..26dfaa8b068bda638eb255ba0d2bb8024b101408 100644 (file)
@@ -62,6 +62,7 @@
 #include "util/fptr_wlist.h"
 #include <signal.h>
 struct worker;
+struct daemon_remote;
 
 /** Global variable: the scenario. Saved here for when event_init is done. */
 static struct replay_scenario* saved_scenario = NULL;
@@ -1286,6 +1287,12 @@ void comm_point_raw_handle_callback(int ATTR_UNUSED(fd),
        log_assert(0);
 }
 
+void comm_base_handle_slow_accept(int ATTR_UNUSED(fd), 
+       short ATTR_UNUSED(event), void* ATTR_UNUSED(arg))
+{
+       log_assert(0);
+}
+
 int serviced_udp_callback(struct comm_point* ATTR_UNUSED(c), 
        void* ATTR_UNUSED(arg), int ATTR_UNUSED(error),
         struct comm_reply* ATTR_UNUSED(reply_info))
@@ -1368,6 +1375,15 @@ void comm_timer_delete(struct comm_timer* timer)
        free(timer);
 }
 
+void comm_base_set_slow_accept_handlers(struct comm_base* ATTR_UNUSED(b),
+       void (*stop_acc)(void*), void (*start_acc)(void*),
+       void* ATTR_UNUSED(arg))
+{
+       /* ignore this */
+       (void)stop_acc;
+       (void)start_acc;
+}
+
 struct event_base* comm_base_internal(struct comm_base* ATTR_UNUSED(b))
 {
        /* no pipe comm possible in testbound */
@@ -1378,4 +1394,20 @@ void daemon_remote_exec(struct worker* ATTR_UNUSED(worker))
 {
 }
 
+void listen_start_accept(struct listen_dnsport* ATTR_UNUSED(listen))
+{
+}
+
+void listen_stop_accept(struct listen_dnsport* ATTR_UNUSED(listen))
+{
+}
+
+void daemon_remote_start_accept(struct daemon_remote* ATTR_UNUSED(rc))
+{
+}
+
+void daemon_remote_stop_accept(struct daemon_remote* ATTR_UNUSED(rc))
+{
+}
+
 /*********** End of Dummy routines ***********/
index cf3ebf6fcf9b9995bfcfdc4f37c10e937764522f..6bb95a5318bfa36b5e6d3d0ce1bec19fc2cb6b37 100644 (file)
@@ -119,6 +119,18 @@ fptr_whitelist_comm_signal(void (*fptr)(int, void*))
        return 0;
 }
 
+int fptr_whitelist_start_accept(void (*fptr)(void*))
+{
+       if(fptr == &worker_start_accept) return 1;
+       return 0;
+}
+
+int fptr_whitelist_stop_accept(void (*fptr)(void*))
+{
+       if(fptr == &worker_stop_accept) return 1;
+       return 0;
+}
+
 int 
 fptr_whitelist_event(void (*fptr)(int, short, void *))
 {
@@ -131,6 +143,7 @@ fptr_whitelist_event(void (*fptr)(int, short, void *))
        else if(fptr == &comm_point_local_handle_callback) return 1;
        else if(fptr == &comm_point_raw_handle_callback) return 1;
        else if(fptr == &tube_handle_signal) return 1;
+       else if(fptr == &comm_base_handle_slow_accept) return 1;
 #ifdef UB_ON_WINDOWS
        else if(fptr == &worker_win_stop_cb) return 1;
 #endif
index 8ec823dd6f1224792f2b2523a9b56db4ef02ade4..d204e923884901a038dbf5d771945c273d8470ef 100644 (file)
@@ -106,6 +106,22 @@ int fptr_whitelist_comm_timer(void (*fptr)(void*));
  */
 int fptr_whitelist_comm_signal(void (*fptr)(int, void*));
 
+/**
+ * Check function pointer whitelist for start_accept callback values.
+ *
+ * @param fptr: function pointer to check.
+ * @return false if not in whitelist.
+ */
+int fptr_whitelist_start_accept(void (*fptr)(void*));
+
+/**
+ * Check function pointer whitelist for stop_accept callback values.
+ *
+ * @param fptr: function pointer to check.
+ * @return false if not in whitelist.
+ */
+int fptr_whitelist_stop_accept(void (*fptr)(void*));
+
 /**
  * Check function pointer whitelist for event structure callback values.
  * This is not called by libevent itself, but checked by netevent.
index c026e1dafd396a616ca9d5e357253ecc92a9b822..ce8c1020787caf4821fc4b31578008e6a706d693 100644 (file)
@@ -115,6 +115,10 @@ struct internal_base {
        uint32_t secs;
        /** timeval with current time */
        struct timeval now;
+       /** the event used for slow_accept timeouts */
+       struct event slow_accept;
+       /** true if slow_accept is enabled */
+       int slow_accept_enabled;
 };
 
 /**
@@ -225,6 +229,11 @@ comm_base_delete(struct comm_base* b)
 {
        if(!b)
                return;
+       if(b->eb->slow_accept_enabled) {
+               if(event_del(&b->eb->slow_accept) != 0) {
+                       log_err("could not event_del slow_accept");
+               }
+       }
 #ifdef USE_MINI_EVENT
        event_base_free(b->eb->base);
 #elif defined(HAVE_EVENT_BASE_FREE) && defined(HAVE_EVENT_BASE_ONCE)
@@ -263,6 +272,14 @@ void comm_base_exit(struct comm_base* b)
        }
 }
 
+void comm_base_set_slow_accept_handlers(struct comm_base* b,
+       void (*stop_acc)(void*), void (*start_acc)(void*), void* arg)
+{
+       b->stop_accept = stop_acc;
+       b->start_accept = start_acc;
+       b->cb_arg = arg;
+}
+
 struct event_base* comm_base_internal(struct comm_base* b)
 {
        return b->eb->base;
@@ -655,6 +672,18 @@ setup_tcp_handler(struct comm_point* c, int fd)
        comm_point_start_listening(c, fd, TCP_QUERY_TIMEOUT);
 }
 
+void comm_base_handle_slow_accept(int fd, short event, void* arg)
+{
+       struct comm_base* b = (struct comm_base*)arg;
+       /* timeout for the slow accept, re-enable accepts again */
+       if(b->start_accept) {
+               verbose(VERB_ALGO, "wait is over, slow accept disabled");
+               fptr_ok(fptr_whitelist_start_accept(b->start_accept));
+               (*b->start_accept)(b->cb_arg);
+               b->eb->slow_accept_enabled = 0;
+       }
+}
+
 int comm_point_perform_accept(struct comm_point* c,
        struct sockaddr_storage* addr, socklen_t* addrlen)
 {
@@ -676,6 +705,38 @@ int comm_point_perform_accept(struct comm_point* c,
 #endif /* EPROTO */
                        )
                        return -1;
+#if defined(ENFILE) && defined(EMFILE)
+               if(errno == ENFILE || errno == EMFILE) {
+                       /* out of file descriptors, likely outside of our
+                        * control. stop accept() calls for some time */
+                       if(c->ev->base->stop_accept) {
+                               struct comm_base* b = c->ev->base;
+                               struct timeval tv;
+                               verbose(VERB_ALGO, "out of file descriptors: "
+                                       "slow accept");
+                               b->eb->slow_accept_enabled = 1;
+                               fptr_ok(fptr_whitelist_stop_accept(
+                                       b->stop_accept));
+                               (*b->stop_accept)(b->cb_arg);
+                               /* set timeout, no mallocs */
+                               tv.tv_sec = NETEVENT_SLOW_ACCEPT_TIME/1000;
+                               tv.tv_usec = NETEVENT_SLOW_ACCEPT_TIME%1000;
+                               event_set(&b->eb->slow_accept, -1, EV_TIMEOUT, 
+                                       comm_base_handle_slow_accept, b);
+                               if(event_base_set(b->eb->base,
+                                       &b->eb->slow_accept) != 0) {
+                                       /* we do not want to log here, because
+                                        * that would spam the logfiles.
+                                        * error: "event_base_set failed." */
+                               }
+                               if(event_add(&b->eb->slow_accept, &tv) != 0) {
+                                       /* we do not want to log here,
+                                        * error: "event_add failed." */
+                               }
+                       }
+                       return -1;
+               }
+#endif
                log_err("accept failed: %s", strerror(errno));
 #else /* USE_WINSOCK */
                if(WSAGetLastError() == WSAEINPROGRESS ||
index 1d866560372583d8c67330c64aefd0484977ecbc..0ea4cf04145e05568417d05af6c2e16022e599a0 100644 (file)
@@ -83,12 +83,23 @@ typedef int comm_point_callback_t(struct comm_point*, void*, int,
 /** to pass fallback from capsforID to callback function; 0x20 failed */
 #define NETEVENT_CAPSFAIL -3
 
+/** timeout to slow accept calls when not possible, in msec. */
+#define NETEVENT_SLOW_ACCEPT_TIME 2000
+
 /**
  * A communication point dispatcher. Thread specific.
  */
 struct comm_base {
        /** behind the scenes structure. with say libevent info. alloced */
        struct internal_base* eb;
+       /** callback to stop listening on accept sockets,
+        * performed when accept() will not function properly */
+       void (*stop_accept)(void*);
+       /** callback to start listening on accept sockets, performed
+        * after stop_accept() then a timeout has passed. */
+       void (*start_accept)(void*);
+       /** user argument for stop_accept and start_accept functions */
+       void* cb_arg;
 };
 
 /**
@@ -311,6 +322,17 @@ void comm_base_dispatch(struct comm_base* b);
  */
 void comm_base_exit(struct comm_base* b);
 
+/**
+ * Set the slow_accept mode handlers.  You can not provide these if you do
+ * not perform accept() calls.
+ * @param b: comm base
+ * @param stop_accept: function that stops listening to accept fds.
+ * @param start_accept: function that resumes listening to accept fds.
+ * @param arg: callback arg to pass to the functions.
+ */
+void comm_base_set_slow_accept_handlers(struct comm_base* b,
+       void (*stop_accept)(void*), void (*start_accept)(void*), void* arg);
+
 /**
  * Access internal data structure (for util/tube.c on windows)
  * @param b: comm base
@@ -636,6 +658,16 @@ void comm_point_local_handle_callback(int fd, short event, void* arg);
  */
 void comm_point_raw_handle_callback(int fd, short event, void* arg);
 
+/**
+ * This routine is published for checks and tests, and is only used internally.
+ * libevent callback for timeout on slow accept.
+ * @param fd: file descriptor.
+ * @param event: event bits from libevent: 
+ *     EV_READ, EV_WRITE, EV_SIGNAL, EV_TIMEOUT.
+ * @param arg: the comm_point structure.
+ */
+void comm_base_handle_slow_accept(int fd, short event, void* arg);
+
 #ifdef USE_WINSOCK
 /**
  * Callback for openssl BIO to on windows detect WSAEWOULDBLOCK and notify