From: Wouter Wijngaards Date: Tue, 8 May 2012 12:08:55 +0000 (+0000) Subject: - Fix for accept spinning reported by OpenBSD. X-Git-Tag: release-1.4.17rc1~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=13a839d624d8fadf79368b4a1c00264f12585b10;p=thirdparty%2Funbound.git - Fix for accept spinning reported by OpenBSD. git-svn-id: file:///svn/unbound/trunk@2663 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/daemon/remote.c b/daemon/remote.c index 6f0625e16..38ca15c85 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -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)) { diff --git a/daemon/remote.h b/daemon/remote.h index 1edc1b887..5919be4f2 100644 --- a/daemon/remote.h +++ b/daemon/remote.h @@ -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). diff --git a/daemon/worker.c b/daemon/worker.c index 3c85fa37f..e84c54b59 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -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), diff --git a/daemon/worker.h b/daemon/worker.h index 96c04676a..c510ebfd7 100644 --- a/daemon/worker.h +++ b/daemon/worker.h @@ -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 */ diff --git a/doc/Changelog b/doc/Changelog index 2d2e5bc28..c6b6d17d6 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -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. diff --git a/libunbound/libworker.c b/libunbound/libworker.c index c4e71cf6a..917a9106d 100644 --- a/libunbound/libworker.c +++ b/libunbound/libworker.c @@ -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); diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index ea7ec3a66..59ca1991e 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -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); + } + } +} + diff --git a/services/listen_dnsport.h b/services/listen_dnsport.h index 22fa82804..4d37aca5e 100644 --- a/services/listen_dnsport.h +++ b/services/listen_dnsport.h @@ -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. diff --git a/smallapp/worker_cb.c b/smallapp/worker_cb.c index 8e86caaeb..bc37e3307 100644 --- a/smallapp/worker_cb.c +++ b/smallapp/worker_cb.c @@ -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 */ diff --git a/testcode/fake_event.c b/testcode/fake_event.c index 159ea3089..26dfaa8b0 100644 --- a/testcode/fake_event.c +++ b/testcode/fake_event.c @@ -62,6 +62,7 @@ #include "util/fptr_wlist.h" #include 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 ***********/ diff --git a/util/fptr_wlist.c b/util/fptr_wlist.c index cf3ebf6fc..6bb95a531 100644 --- a/util/fptr_wlist.c +++ b/util/fptr_wlist.c @@ -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 diff --git a/util/fptr_wlist.h b/util/fptr_wlist.h index 8ec823dd6..d204e9238 100644 --- a/util/fptr_wlist.h +++ b/util/fptr_wlist.h @@ -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. diff --git a/util/netevent.c b/util/netevent.c index c026e1daf..ce8c10207 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -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 || diff --git a/util/netevent.h b/util/netevent.h index 1d8665603..0ea4cf041 100644 --- a/util/netevent.h +++ b/util/netevent.h @@ -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