From: Evan Hunt Date: Wed, 20 Nov 2019 21:33:35 +0000 (+0100) Subject: netmgr: make TCP timeouts configurable X-Git-Tag: v9.15.7~75^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=199bd6b62392d3c2111c92c1c3c9d35a5e83bd57;p=thirdparty%2Fbind9.git netmgr: make TCP timeouts configurable - restore support for tcp-initial-timeout, tcp-idle-timeout, tcp-keepalive-timeout and tcp-advertised-timeout configuration options, which were ineffective previously. --- diff --git a/bin/named/server.c b/bin/named/server.c index e2115df9aef..55663d515b0 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8470,8 +8470,8 @@ load_configuration(const char *filename, named_server_t *server, advertised = MAX_TCP_TIMEOUT; } - ns_server_settimeouts(named_g_server->sctx, - initial, idle, keepalive, advertised); + isc_nm_tcp_settimeouts(named_g_nm, initial, idle, + keepalive, advertised); /* * Configure sets of UDP query source ports. @@ -15405,8 +15405,8 @@ named_server_tcptimeouts(isc_lex_t *lex, isc_buffer_t **text) { if (ptr == NULL) return (ISC_R_UNEXPECTEDEND); - ns_server_gettimeouts(named_g_server->sctx, - &initial, &idle, &keepalive, &advertised); + isc_nm_tcp_gettimeouts(named_g_nm, &initial, &idle, + &keepalive, &advertised); /* Look for optional arguments. */ ptr = next_token(lex, NULL); @@ -15445,7 +15445,7 @@ named_server_tcptimeouts(isc_lex_t *lex, isc_buffer_t **text) { result = isc_task_beginexclusive(named_g_server->task); RUNTIME_CHECK(result == ISC_R_SUCCESS); - ns_server_settimeouts(named_g_server->sctx, initial, idle, + isc_nm_tcp_settimeouts(named_g_nm, initial, idle, keepalive, advertised); isc_task_endexclusive(named_g_server->task); diff --git a/bin/tests/system/stop.pl b/bin/tests/system/stop.pl index 1a78ade59f4..a667d446fbc 100644 --- a/bin/tests/system/stop.pl +++ b/bin/tests/system/stop.pl @@ -109,7 +109,7 @@ foreach my $name(@ans) { stop_signal($name, "TERM", 1); } -@ans = wait_for_servers(60, @ans); +@ans = wait_for_servers(1200, @ans); # Pass 3: SIGABRT foreach my $name (@ns) { diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 1696f3cb8c5..97bb1d6d764 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -113,8 +113,20 @@ isc_nmhandle_setdata(isc_nmhandle_t *handle, void *arg, isc_sockaddr_t isc_nmhandle_peeraddr(isc_nmhandle_t *handle); +/*%< + * Return the peer address for the given handle. + */ isc_sockaddr_t isc_nmhandle_localaddr(isc_nmhandle_t *handle); +/*%< + * Return the local address for the given handle. + */ + +isc_nm_t * +isc_nmhandle_netmgr(isc_nmhandle_t *handle); +/*%< + * Return a pointer to the netmgr object for the given handle. + */ typedef void (*isc_nm_recv_cb_t)(isc_nmhandle_t *handle, isc_region_t *region, void *cbarg); @@ -212,7 +224,7 @@ isc_result_t isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, void *cbarg, size_t extrahandlesize, isc_quota_t *quota, - isc_nmsocket_t **rv); + isc_nmsocket_t **sockp); /*%< * Start listening for raw messages over the TCP interface 'iface', using * net manager 'mgr'. @@ -230,8 +242,8 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, * quota. This allows us to enforce TCP client quota limits. * * NOTE: This is currently only called inside isc_nm_listentcpdns(), which - * creates a 'wrapper' socket that sends and receives DNS messages - - * prepended with a two-byte length field - and handles buffering. + * creates a 'wrapper' socket that sends and receives DNS messages + * prepended with a two-byte length field, and handles buffering. */ void @@ -250,8 +262,10 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, * net manager 'mgr'. * * On success, 'sockp' will be updated to contain a new listening TCPDNS - * socket. This is a wrapper around a TCP socket, and handles DNS length - * processing. + * socket. This is a wrapper around a raw TCP socket, which sends and + * receives DNS messages via that socket. It handles message buffering + * and pipelining, and automatically prepends messages with a two-byte + * length field. * * When a complete DNS message is received on the socket, 'cb' will be * called with 'cbarg' as its argument. @@ -259,6 +273,8 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, * When handles are allocated for the socket, 'extrasize' additional bytes * will be allocated along with the handle for an associated object * (typically ns_client). + * + * 'quota' is passed to isc_nm_listentcp() when opening the raw TCP socket. */ void @@ -270,25 +286,60 @@ isc_nm_tcpdns_stoplistening(isc_nmsocket_t *sock); void isc_nm_tcpdns_sequential(isc_nmhandle_t *handle); /*%< - * Disable pipelining on this connection. Each DNS packet - * will be only processed after the previous completes. + * Disable pipelining on this connection. Each DNS packet will be only + * processed after the previous completes. * - * The socket must be unpaused after the query is processed. - * This is done the response is sent, or if we're dropping the - * query, it will be done when a handle is fully dereferenced - * by calling the socket's closehandle_cb callback. + * The socket must be unpaused after the query is processed. This is done + * the response is sent, or if we're dropping the query, it will be done + * when a handle is fully dereferenced by calling the socket's + * closehandle_cb callback. * - * Note: This can only be run while a message is being processed; - * if it is run before any messages are read, no messages will - * be read. + * Note: This can only be run while a message is being processed; if it is + * run before any messages are read, no messages will be read. + * + * Also note: once this has been set, it cannot be reversed for a given + * connection. + */ + +void +isc_nm_tcpdns_keepalive(isc_nmhandle_t *handle); +/*%< + * Enable keepalive on this connection. * - * Also note: once this has been set, it cannot be reversed for a - * given connection. + * When keepalive is active, we switch to using the keepalive timeout + * to determine when to close a connection, rather than the idle timeout. + */ + +void +isc_nm_tcp_settimeouts(isc_nm_t *mgr, uint32_t init, uint32_t idle, + uint32_t keepalive, uint32_t advertised); +/*%< + * Sets the initial, idle, and keepalive timeout values to use for + * TCP connections, and the timeout value to advertise in responses using + * the EDNS TCP Keepalive option (which should ordinarily be the same + * as 'keepalive'), in tenths of seconds. + * + * Requires: + * \li 'mgr' is a valid netmgr. + */ + +void +isc_nm_tcp_gettimeouts(isc_nm_t *mgr, uint32_t *initial, uint32_t *idle, + uint32_t *keepalive, uint32_t *advertised); +/*%< + * Gets the initial, idle, keepalive, or advertised timeout values, + * in tenths of seconds. + * + * Any integer pointer parameter not set to NULL will be updated to + * contain the corresponding timeout value. + * + * Requires: + * \li 'mgr' is a valid netmgr. */ void isc_nm_maxudp(isc_nm_t *mgr, uint32_t maxudp); /*%< - * Simulate a broken firewall that blocks UDP messages larger - * than a given size. + * Simulate a broken firewall that blocks UDP messages larger than a given + * size. */ diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 037ffa145ff..4bea4858005 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -243,6 +243,18 @@ struct isc_nm { * event or wait for the other one to finish if we want to pause. */ atomic_bool interlocked; + + /* + * Timeout values for TCP connections, coresponding to + * tcp-intiial-timeout, tcp-idle-timeout, tcp-keepalive-timeout, + * and tcp-advertised-timeout. Note that these are stored in + * milliseconds so they can be used directly with the libuv timer, + * but they are configured in tenths of seconds. + */ + uint32_t init; + uint32_t idle; + uint32_t keepalive; + uint32_t advertised; }; typedef enum isc_nmsocket_type { @@ -339,6 +351,12 @@ struct isc_nmsocket { */ atomic_bool readpaused; + /*% + * A TCP or TCPDNS socket has been set to use the keepalive + * timeout instead of the default idle timeout. + */ + atomic_bool keepalive; + /*% * 'spare' handles for that can be reused to avoid allocations, * for UDP. diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index e77b7c96f19..9a41171cae4 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -94,6 +94,15 @@ isc_nm_start(isc_mem_t *mctx, uint32_t workers) { atomic_init(&mgr->paused, false); atomic_init(&mgr->interlocked, false); + /* + * Default TCP timeout values. + * May be updated by isc_nm_listentcp(). + */ + mgr->init = 30000; + mgr->idle = 30000; + mgr->keepalive = 30000; + mgr->advertised = 30000; + mgr->workers = isc_mem_get(mctx, workers * sizeof(isc__networker_t)); for (size_t i = 0; i < workers; i++) { int r; @@ -303,6 +312,41 @@ isc_nm_maxudp(isc_nm_t *mgr, uint32_t maxudp) { atomic_store(&mgr->maxudp, maxudp); } +void +isc_nm_tcp_settimeouts(isc_nm_t *mgr, uint32_t init, uint32_t idle, + uint32_t keepalive, uint32_t advertised) +{ + REQUIRE(VALID_NM(mgr)); + + mgr->init = init * 100; + mgr->idle = idle * 100; + mgr->keepalive = keepalive * 100; + mgr->advertised = advertised * 100; +} + +void +isc_nm_tcp_gettimeouts(isc_nm_t *mgr, uint32_t *initial, uint32_t *idle, + uint32_t *keepalive, uint32_t *advertised) +{ + REQUIRE(VALID_NM(mgr)); + + if (initial != NULL) { + *initial = mgr->init / 100; + } + + if (idle != NULL) { + *idle = mgr->idle / 100; + } + + if (keepalive != NULL) { + *keepalive = mgr->keepalive / 100; + } + + if (advertised != NULL) { + *advertised = mgr->advertised / 100; + } +} + /* * nm_thread is a single worker thread, that runs uv_run event loop * until asked to stop. @@ -712,7 +756,6 @@ isc__nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, .inactivehandles = isc_astack_new(mgr->mctx, 60), .inactivereqs = isc_astack_new(mgr->mctx, 60) }; - isc_nm_attach(mgr, &sock->mgr); sock->uv_handle.handle.data = sock; @@ -950,27 +993,27 @@ isc_nmhandle_unref(isc_nmhandle_t *handle) { if (sock->closehandle_cb != NULL) { if (sock->tid == isc_nm_tid()) { sock->closehandle_cb(sock); - - /* - * If we do this asynchronously then - * the async event will clean it up. - */ - if (sock->ah == 0 && - !atomic_load(&sock->active) && - !atomic_load(&sock->destroying)) - { - nmsocket_maybe_destroy(sock); - } } else { - isc__netievent_closecb_t * event = isc__nm_get_ievent(sock->mgr, netievent_closecb); isc_nmsocket_attach(sock, &event->sock); isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *) event); + /* + * If we do this asynchronously then the async event + * will clean the socket, so just exit. + */ + return; } } + + if (atomic_load(&sock->ah) == 0 && + !atomic_load(&sock->active) && + !atomic_load(&sock->destroying)) + { + nmsocket_maybe_destroy(sock); + } } void * @@ -1012,6 +1055,14 @@ isc_nmhandle_localaddr(isc_nmhandle_t *handle) { return (handle->local); } +isc_nm_t * +isc_nmhandle_netmgr(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + return (handle->sock->mgr); +} + isc__nm_uvreq_t * isc__nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock) { isc__nm_uvreq_t *req = NULL; diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 493d82a5aa5..c400866db79 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -130,7 +130,7 @@ isc_result_t isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_cb_t cb, void *cbarg, size_t extrahandlesize, isc_quota_t *quota, - isc_nmsocket_t **rv) + isc_nmsocket_t **sockp) { isc__netievent_tcplisten_t *ievent = NULL; isc_nmsocket_t *nsock = NULL; @@ -163,7 +163,7 @@ isc_nm_listentcp(isc_nm_t *mgr, isc_nmiface_t *iface, ievent->sock = nsock; isc__nm_enqueue_ievent(&mgr->workers[nsock->tid], (isc__netievent_t *) ievent); - *rv = nsock; + *sockp = nsock; return (ISC_R_SUCCESS); } @@ -399,6 +399,11 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) { INSIST(sock->rcb.recv != NULL); sock->rcb.recv(sock->tcphandle, ®ion, sock->rcbarg); + + sock->read_timeout = (atomic_load(&sock->keepalive) + ? sock->mgr->keepalive + : sock->mgr->idle); + if (sock->timer_initialized && sock->read_timeout != 0) { /* The timer will be updated */ uv_timer_start(&sock->timer, readtimeout_cb, @@ -485,7 +490,7 @@ accept_connection(isc_nmsocket_t *ssock) { handle = isc__nmhandle_get(csock, NULL, &local); INSIST(ssock->rcb.accept != NULL); - csock->read_timeout = 1000; + csock->read_timeout = ssock->mgr->init; ssock->rcb.accept(handle, ISC_R_SUCCESS, ssock->rcbarg); isc_nmsocket_detach(&csock); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index a06d5f7b0f6..c3f87a3d4d3 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -86,7 +86,9 @@ timer_close_cb(uv_handle_t *handle) { static void dnstcp_readtimeout(uv_timer_t *timer) { isc_nmsocket_t *sock = (isc_nmsocket_t *) timer->data; + REQUIRE(VALID_NMSOCK(sock)); + isc_nmsocket_detach(&sock->outer); uv_close((uv_handle_t*) &sock->timer, timer_close_cb); } @@ -119,7 +121,7 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { isc_nmsocket_attach(handle->sock, &dnssock->outer); dnssock->peer = handle->sock->peer; dnssock->iface = handle->sock->iface; - dnssock->read_timeout = 5000; + dnssock->read_timeout = handle->sock->mgr->init; dnssock->tid = isc_nm_tid(); dnssock->closehandle_cb = resume_processing; @@ -215,6 +217,10 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) { memmove(dnssock->buf + dnssock->buf_len, base, len); dnssock->buf_len += len; + dnssock->read_timeout = (atomic_load(&dnssock->keepalive) + ? dnssock->mgr->keepalive + : dnssock->mgr->idle); + do { isc_result_t result; isc_nmhandle_t *dnshandle = NULL; @@ -233,7 +239,7 @@ dnslisten_readcb(isc_nmhandle_t *handle, isc_region_t *region, void *arg) { atomic_store(&dnssock->outer->processing, true); uv_timer_stop(&dnssock->timer); - if (dnssock->sequential) { + if (atomic_load(&dnssock->sequential)) { /* * We're in sequential mode and we processed * one packet, so we're done until the next read @@ -270,7 +276,7 @@ isc_result_t isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb, void *cbarg, size_t extrahandlesize, isc_quota_t *quota, - isc_nmsocket_t **rv) + isc_nmsocket_t **sockp) { /* A 'wrapper' socket object with outer set to true TCP socket */ isc_nmsocket_t *dnslistensock = @@ -291,7 +297,8 @@ isc_nm_listentcpdns(isc_nm_t *mgr, isc_nmiface_t *iface, quota, &dnslistensock->outer); atomic_store(&dnslistensock->listening, true); - *rv = dnslistensock; + *sockp = dnslistensock; + return (result); } @@ -331,6 +338,20 @@ isc_nm_tcpdns_sequential(isc_nmhandle_t *handle) { atomic_store(&handle->sock->sequential, true); } +void +isc_nm_tcpdns_keepalive(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + + if (handle->sock->type != isc_nm_tcpdnssocket || + handle->sock->outer == NULL) + { + return; + } + + atomic_store(&handle->sock->keepalive, true); + atomic_store(&handle->sock->outer->keepalive, true); +} + typedef struct tcpsend { isc_mem_t *mctx; isc_nmhandle_t *handle; @@ -363,7 +384,7 @@ resume_processing(void *arg) { * For sequential sockets: Process what's in the buffer, or * if there aren't any messages buffered, resume reading. */ - if (sock->sequential) { + if (atomic_load(&sock->sequential)) { isc_nmhandle_t *handle = NULL; result = processbuffer(sock, &handle); diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 9b3a45d45e9..35faea7d55f 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -438,6 +438,7 @@ isc_netscope_pton isc_nmhandle_getdata isc_nmhandle_getextra isc_nmhandle_is_stream +isc_nmhandle_netmgr isc_nmhandle_localaddr isc_nmhandle_peeraddr isc_nmhandle_ref @@ -450,7 +451,10 @@ isc_nm_listenudp isc_nm_maxudp isc_nm_send isc_nm_start +isc_nm_tcp_gettimeouts +isc_nm_tcp_settimeouts isc_nmsocket_detach +isc_nm_tcpdns_keepalive isc_nm_tcpdns_sequential isc_nm_tcpdns_stoplistening isc_nm_tid diff --git a/lib/ns/client.c b/lib/ns/client.c index 189172f7e33..8cd8226549a 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1023,12 +1023,14 @@ ns_client_addopt(ns_client_t *client, dns_message_t *message, } if (TCP_CLIENT(client) && USEKEEPALIVE(client)) { isc_buffer_t buf; + uint32_t adv; INSIST(count < DNS_EDNSOPTIONS); + isc_nm_tcp_gettimeouts(isc_nmhandle_netmgr(client->handle), + NULL, NULL, NULL, &adv); isc_buffer_init(&buf, advtimo, sizeof(advtimo)); - isc_buffer_putuint16(&buf, - (uint16_t) client->sctx->advertisedtimo); + isc_buffer_putuint16(&buf, (uint16_t) adv); ednsopts[count].code = DNS_OPT_TCP_KEEPALIVE; ednsopts[count].length = 2; ednsopts[count].value = advtimo; @@ -2191,7 +2193,7 @@ get_clientmctx(ns_clientmgr_t *manager, isc_mem_t **mctxp) { #if CLIENT_NMCTXS > 0 LOCK(&manager->lock); - if (isc_nm_tid()>=0) { + if (isc_nm_tid() >= 0) { nextmctx = isc_nm_tid(); } else { nextmctx = manager->nextmctx++; diff --git a/lib/ns/include/ns/server.h b/lib/ns/include/ns/server.h index fa27bdd4461..a8b232340eb 100644 --- a/lib/ns/include/ns/server.h +++ b/lib/ns/include/ns/server.h @@ -92,11 +92,6 @@ struct ns_server { uint32_t options; unsigned int delay; - unsigned int initialtimo; - unsigned int idletimo; - unsigned int keepalivetimo; - unsigned int advertisedtimo; - dns_acl_t *blackholeacl; dns_acl_t *keepresporder; uint16_t udpsize; @@ -174,21 +169,6 @@ ns_server_setserverid(ns_server_t *sctx, const char *serverid); *\li 'sctx' is valid. */ -void -ns_server_settimeouts(ns_server_t *sctx, unsigned int initial, - unsigned int idle, unsigned int keepalive, - unsigned int advertised); -void -ns_server_gettimeouts(ns_server_t *sctx, unsigned int *initial, - unsigned int *idle, unsigned int *keepalive, - unsigned int *advertised); -/*%< - * Set/get tcp-timeout values. - * - * Requires: - *\li 'sctx' is valid. - */ - void ns_server_setoption(ns_server_t *sctx, unsigned int option, bool value); diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 93a59dd9eb5..8096d5ffefb 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -457,7 +457,6 @@ static isc_result_t ns_interface_listentcp(ns_interface_t *ifp) { isc_result_t result; - /* Reserve space for an ns_client_t with the netmgr handle */ result = isc_nm_listentcpdns(ifp->mgr->nm, (isc_nmiface_t *) &ifp->addr, ns__client_request, ifp, diff --git a/lib/ns/server.c b/lib/ns/server.c index 4259cea7967..7cf773d99dd 100644 --- a/lib/ns/server.c +++ b/lib/ns/server.c @@ -87,11 +87,6 @@ ns_server_create(isc_mem_t *mctx, ns_matchview_t matchingview, CHECKFATAL(isc_stats_create(mctx, &sctx->tcpoutstats6, dns_sizecounter_out_max)); - sctx->initialtimo = 300; - sctx->idletimo = 300; - sctx->keepalivetimo = 300; - sctx->advertisedtimo = 300; - sctx->udpsize = 4096; sctx->transfer_tcp_message_size = 20480; @@ -216,34 +211,6 @@ ns_server_setserverid(ns_server_t *sctx, const char *serverid) { return (ISC_R_SUCCESS); } -void -ns_server_settimeouts(ns_server_t *sctx, unsigned int initial, - unsigned int idle, unsigned int keepalive, - unsigned int advertised) -{ - REQUIRE(SCTX_VALID(sctx)); - - sctx->initialtimo = initial; - sctx->idletimo = idle; - sctx->keepalivetimo = keepalive; - sctx->advertisedtimo = advertised; -} - -void -ns_server_gettimeouts(ns_server_t *sctx, unsigned int *initial, - unsigned int *idle, unsigned int *keepalive, - unsigned int *advertised) -{ - REQUIRE(SCTX_VALID(sctx)); - REQUIRE(initial != NULL && idle != NULL && - keepalive != NULL && advertised != NULL); - - *initial = sctx->initialtimo; - *idle = sctx->idletimo; - *keepalive = sctx->keepalivetimo; - *advertised = sctx->advertisedtimo; -} - void ns_server_setoption(ns_server_t *sctx, unsigned int option, bool value) diff --git a/lib/ns/win32/libns.def b/lib/ns/win32/libns.def index 2105a3e4931..d06dbc92a36 100644 --- a/lib/ns/win32/libns.def +++ b/lib/ns/win32/libns.def @@ -87,10 +87,8 @@ ns_server_attach ns_server_create ns_server_detach ns_server_getoption -ns_server_gettimeouts ns_server_setoption ns_server_setserverid -ns_server_settimeouts ns_sortlist_addrorder1 ns_sortlist_addrorder2 ns_sortlist_byaddrsetup