From 45a73c113f2982b7171632d75d4bbb51d4e6bb53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 9 Feb 2022 10:59:08 +0100 Subject: [PATCH 1/7] Rename sock->timer to sock->read_timer Before adding the write timer, we have to remove the generic sock->timer to sock->read_timer. We don't touch the function names to limit the impact of the refactoring. --- lib/isc/netmgr/netmgr-int.h | 2 +- lib/isc/netmgr/netmgr.c | 13 +++++++------ lib/isc/netmgr/tcp.c | 23 ++++++++++++----------- lib/isc/netmgr/tcpdns.c | 20 +++++++++++--------- lib/isc/netmgr/tlsdns.c | 22 ++++++++++++---------- lib/isc/netmgr/udp.c | 14 +++++++------- 6 files changed, 50 insertions(+), 44 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 2ce53402b2..4c84047e23 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -950,7 +950,7 @@ struct isc_nmsocket { /*% * TCP read/connect timeout timers. */ - uv_timer_t timer; + uv_timer_t read_timer; uint64_t read_timeout; uint64_t connect_timeout; diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index ecdb26340d..1574ad2a31 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -1977,7 +1977,7 @@ isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, REQUIRE(req->cb.connect != NULL); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); INSIST(atomic_compare_exchange_strong(&sock->connecting, &(bool){ true }, false)); @@ -2108,7 +2108,7 @@ isc__nmsocket_timer_restart(isc_nmsocket_t *sock) { return; } - r = uv_timer_start(&sock->timer, + r = uv_timer_start(&sock->read_timer, isc__nmsocket_connecttimeout_cb, sock->connect_timeout + 10, 0); UV_RUNTIME_CHECK(uv_timer_start, r); @@ -2120,7 +2120,8 @@ isc__nmsocket_timer_restart(isc_nmsocket_t *sock) { return; } - r = uv_timer_start(&sock->timer, isc__nmsocket_readtimeout_cb, + r = uv_timer_start(&sock->read_timer, + isc__nmsocket_readtimeout_cb, sock->read_timeout, 0); UV_RUNTIME_CHECK(uv_timer_start, r); } @@ -2130,7 +2131,7 @@ bool isc__nmsocket_timer_running(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); - return (uv_is_active((uv_handle_t *)&sock->timer)); + return (uv_is_active((uv_handle_t *)&sock->read_timer)); } void @@ -2152,7 +2153,7 @@ isc__nmsocket_timer_stop(isc_nmsocket_t *sock) { /* uv_timer_stop() is idempotent, no need to check if running */ - r = uv_timer_stop(&sock->timer); + r = uv_timer_stop(&sock->read_timer); UV_RUNTIME_CHECK(uv_timer_stop, r); } @@ -2404,7 +2405,7 @@ isc_nmhandle_cleartimeout(isc_nmhandle_t *handle) { default: handle->sock->read_timeout = 0; - if (uv_is_active((uv_handle_t *)&handle->sock->timer)) { + if (uv_is_active((uv_handle_t *)&handle->sock->read_timer)) { isc__nmsocket_timer_stop(handle->sock); } } diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 45339244c7..aef698a79d 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -140,8 +140,9 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); if (r != 0) { @@ -170,7 +171,8 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -231,7 +233,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { REQUIRE(sock->tid == isc_nm_tid()); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (!atomic_load(&sock->connecting)) { return; @@ -527,10 +529,9 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); LOCK(&sock->parent->lock); @@ -974,9 +975,9 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1278,8 +1279,8 @@ tcp_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); } void @@ -1401,7 +1402,7 @@ isc__nm_async_tcpcancel(isc__networker_t *worker, isc__netievent_t *ev0) { REQUIRE(sock->tid == isc_nm_tid()); UNUSED(worker); - uv_timer_stop(&sock->timer); + uv_timer_stop(&sock->read_timer); isc__nm_tcp_failed_read_cb(sock, ISC_R_EOF); } diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index ce31e30c86..ac8fbca33c 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -105,8 +105,9 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -144,7 +145,8 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -205,7 +207,7 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status) { REQUIRE(sock->tid == isc_nm_tid()); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (!atomic_load(&sock->connecting)) { return; @@ -497,9 +499,9 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); LOCK(&sock->parent->lock); @@ -966,9 +968,9 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1320,8 +1322,8 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); } void diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index bd564bac25..fc61e918aa 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -121,9 +121,9 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -161,7 +161,8 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -569,9 +570,9 @@ isc__nm_async_tlsdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); LOCK(&sock->parent->lock); @@ -1107,7 +1108,8 @@ tls_cycle_input(isc_nmsocket_t *sock) { sock->tls.pending_req = NULL; isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + sock); INSIST(atomic_compare_exchange_strong( &sock->connecting, &(bool){ true }, false)); @@ -1476,9 +1478,9 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1889,8 +1891,8 @@ tlsdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); } void diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 3573836785..7d8c5f4b9a 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -265,9 +265,9 @@ route_connect_direct(isc_nmsocket_t *sock) { UV_RUNTIME_CHECK(uv_udp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -441,9 +441,9 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); LOCK(&sock->parent->lock); @@ -857,9 +857,9 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_udp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -1276,7 +1276,7 @@ udp_close_direct(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); } void From cd3b58622c7401cd4150ea0a489596b1504b8869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 9 Feb 2022 12:45:37 +0100 Subject: [PATCH 2/7] Add uv_tcp_close_reset compat The uv_tcp_close_reset() function was added in libuv 1.32.0 and since we support older libuv releases, we have to add a shim uv_tcp_close_reset() implementation loosely based on libuv. --- lib/isc/netmgr/uv-compat.c | 18 ++++++++++++++++++ lib/isc/netmgr/uv-compat.h | 9 +++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/isc/netmgr/uv-compat.c b/lib/isc/netmgr/uv-compat.c index 104ba5118e..b7c0f7b0e2 100644 --- a/lib/isc/netmgr/uv-compat.c +++ b/lib/isc/netmgr/uv-compat.c @@ -42,6 +42,24 @@ isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr) { } #endif /* UV_VERSION_HEX < UV_VERSION(1, 27, 0) */ +#if UV_VERSION_HEX < UV_VERSION(1, 32, 0) +int +uv_tcp_close_reset(uv_tcp_t *handle, uv_close_cb close_cb) { + if (setsockopt(handle->io_watcher.fd, SOL_SOCKET, SO_LINGER, + &(struct linger){ 1, 0 }, sizeof(struct linger)) == -1) + { +#if UV_VERSION_HEX >= UV_VERSION(1, 10, 0) + return (uv_translate_sys_error(errno)); +#else + return (-errno); +#endif /* UV_VERSION_HEX >= UV_VERSION(1, 10, 0) */ + } + + uv_close((uv_handle_t *)handle, close_cb); + return (0); +} +#endif /* UV_VERSION_HEX < UV_VERSION(1, 32, 0) */ + int isc_uv_udp_freebind(uv_udp_t *handle, const struct sockaddr *addr, unsigned int flags) { diff --git a/lib/isc/netmgr/uv-compat.h b/lib/isc/netmgr/uv-compat.h index 6797484a8a..387d3c6940 100644 --- a/lib/isc/netmgr/uv-compat.h +++ b/lib/isc/netmgr/uv-compat.h @@ -23,6 +23,10 @@ #define UV_VERSION(major, minor, patch) ((major << 16) | (minor << 8) | (patch)) +#if !defined(UV__ERR) +#define UV__ERR(x) (-(x)) +#endif + #if UV_VERSION_HEX < UV_VERSION(1, 19, 0) static inline void * uv_handle_get_data(const uv_handle_t *handle) { @@ -45,6 +49,11 @@ uv_req_set_data(uv_req_t *req, void *data) { } #endif /* UV_VERSION_HEX < UV_VERSION(1, 19, 0) */ +#if UV_VERSION_HEX < UV_VERSION(1, 32, 0) +int +uv_tcp_close_reset(uv_tcp_t *handle, uv_close_cb close_cb); +#endif + #if UV_VERSION_HEX < UV_VERSION(1, 34, 0) #define uv_sleep(msec) usleep(msec * 1000) #endif /* UV_VERSION_HEX < UV_VERSION(1, 34, 0) */ From 408b3621696e39ac6dfe58be75fad168a37b31ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 9 Feb 2022 11:21:04 +0100 Subject: [PATCH 3/7] Add TCP, TCPDNS and TLSDNS write timer When the outgoing TCP write buffers are full because the other party is not reading the data, the uv_write() could wait indefinitely on the uv_loop and never calling the callback. Add a new write timer that uses the `tcp-idle-timeout` value to interrupt the TCP connection when we are not able to send data for defined period of time. --- lib/isc/netmgr/netmgr-int.h | 26 +++++++++++++++-- lib/isc/netmgr/netmgr.c | 18 +++++++++++- lib/isc/netmgr/tcp.c | 51 ++++++++++++++++++++++++++++++-- lib/isc/netmgr/tcpdns.c | 56 ++++++++++++++++++++++++++++------- lib/isc/netmgr/tlsdns.c | 58 ++++++++++++++++++++++++++++++------- lib/isc/netmgr/udp.c | 36 +++++++++++++++++++++-- 6 files changed, 215 insertions(+), 30 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 4c84047e23..188d90340e 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -954,6 +954,13 @@ struct isc_nmsocket { uint64_t read_timeout; uint64_t connect_timeout; + /*% + * TCP write timeout timer. + */ + uv_timer_t write_timer; + uint64_t write_timeout; + int64_t writes; + /*% outer socket is for 'wrapped' sockets - e.g. tcpdns in tcp */ isc_nmsocket_t *outer; @@ -2071,12 +2078,25 @@ isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, void isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async); -void -isc__nmsocket_connecttimeout_cb(uv_timer_t *timer); - void isc__nm_accept_connection_log(isc_result_t result, bool can_log_quota); +/* + * Timeout callbacks + */ +void +isc__nmsocket_connecttimeout_cb(uv_timer_t *timer); +void +isc__nmsocket_readtimeout_cb(uv_timer_t *timer); +void +isc__nmsocket_writetimeout_cb(uv_timer_t *timer); + +/*%< + * + * Maximum number of simultaneous handles in flight supported for a single + * connected TCPDNS socket. This value was chosen arbitrarily, and may be + * changed in the future. + */ #define STREAM_CLIENTS_PER_CONN 23 #define UV_RUNTIME_CHECK(func, ret) \ diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 1574ad2a31..b63f990163 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -2070,7 +2070,21 @@ isc__nm_accept_connection_log(isc_result_t result, bool can_log_quota) { isc_result_totext(result)); } -static void +void +isc__nmsocket_writetimeout_cb(uv_timer_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)timer); + + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + + /* The shutdown will be handled in the respective close functions */ + r = uv_tcp_close_reset(&sock->uv_handle.tcp, NULL); + UV_RUNTIME_CHECK(uv_tcp_close_reset, r); + + isc__nmsocket_shutdown(sock); +} + +void isc__nmsocket_readtimeout_cb(uv_timer_t *timer) { isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)timer); @@ -2447,6 +2461,8 @@ isc_nmhandle_keepalive(isc_nmhandle_t *handle, bool value) { atomic_store(&sock->keepalive, value); sock->read_timeout = value ? atomic_load(&sock->mgr->keepalive) : atomic_load(&sock->mgr->idle); + sock->write_timeout = value ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle); break; #if HAVE_LIBNGHTTP2 case isc_nm_tlssocket: diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index aef698a79d..e179bc05b0 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -144,6 +144,10 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); if (r != 0) { isc__nm_closesocket(sock->fd); @@ -533,6 +537,10 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + LOCK(&sock->parent->lock); r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); @@ -979,6 +987,10 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); + r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { result = isc__nm_uverr2result(r); @@ -1072,6 +1084,13 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, const isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tcpsend(sock->mgr, sock, uvreq); isc__nm_maybe_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1082,11 +1101,17 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, const isc_region_t *region, static void tcp_send_cb(uv_write_t *req, int status) { isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; + REQUIRE(VALID_UVREQ(uvreq)); REQUIRE(VALID_NMHANDLE(uvreq->handle)); isc_nmsocket_t *sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + if (status < 0) { isc__nm_incstats(sock, STATID_SENDFAIL); failed_send_cb(sock, uvreq, isc__nm_uverr2result(status)); @@ -1130,6 +1155,11 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { return (ISC_R_CANCELED); } + r = uv_timer_start(&sock->write_timer, isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tcp_send_cb); if (r < 0) { @@ -1193,7 +1223,7 @@ tcp_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1206,6 +1236,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_timer_close_cb(uv_handle_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data(timer); + uv_handle_set_data(timer, NULL); + + REQUIRE(VALID_NMSOCK(sock)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tcp_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tcpsocket); @@ -1258,6 +1299,8 @@ stop_tcp_parent(isc_nmsocket_t *sock) { static void tcp_close_direct(isc_nmsocket_t *sock) { + int r; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1279,8 +1322,10 @@ tcp_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index ac8fbca33c..9244320ec0 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -37,13 +37,6 @@ #include "netmgr-int.h" #include "uv-compat.h" -/*%< - * - * Maximum number of simultaneous handles in flight supported for a single - * connected TCPDNS socket. This value was chosen arbitrarily, and may be - * changed in the future. - */ - static atomic_uint_fast32_t last_tcpdnsquota_log = ATOMIC_VAR_INIT(0); static bool @@ -109,6 +102,10 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; goto error; @@ -503,6 +500,10 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + LOCK(&sock->parent->lock); r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); @@ -972,6 +973,10 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); + r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { result = isc__nm_uverr2result(r); @@ -1088,6 +1093,13 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tcpdnssend(sock->mgr, sock, uvreq); isc__nm_maybe_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1105,6 +1117,11 @@ tcpdns_send_cb(uv_write_t *req, int status) { sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + if (status < 0) { isc__nm_incstats(sock, STATID_SENDFAIL); isc__nm_failed_send_cb(sock, uvreq, @@ -1171,6 +1188,11 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) { goto fail; } + r = uv_timer_start(&sock->write_timer, isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&uvreq->uv_req.write, &sock->uv_handle.stream, bufs, nbufs, tcpdns_send_cb); if (r < 0) { @@ -1240,7 +1262,7 @@ tcpdns_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *timer) { +read_timer_close_cb(uv_handle_t *timer) { isc_nmsocket_t *sock = uv_handle_get_data(timer); uv_handle_set_data(timer, NULL); @@ -1255,6 +1277,17 @@ timer_close_cb(uv_handle_t *timer) { } } +static void +write_timer_close_cb(uv_handle_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data(timer); + uv_handle_set_data(timer, NULL); + + REQUIRE(VALID_NMSOCK(sock)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tcpdns_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tcpdnssocket); @@ -1307,6 +1340,7 @@ stop_tcpdns_parent(isc_nmsocket_t *sock) { static void tcpdns_close_direct(isc_nmsocket_t *sock) { + int r; REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1322,8 +1356,10 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index fc61e918aa..d91e04489d 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -38,13 +38,6 @@ #include "openssl_shim.h" #include "uv-compat.h" -/*%< - * - * Maximum number of simultaneous handles in flight supported for a single - * connected TLSDNS socket. This value was chosen arbitrarily, and may be - * changed in the future. - */ - static atomic_uint_fast32_t last_tlsdnsquota_log = ATOMIC_VAR_INIT(0); static void @@ -125,6 +118,10 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; goto error; @@ -574,6 +571,10 @@ isc__nm_async_tlsdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + LOCK(&sock->parent->lock); r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); @@ -1165,6 +1166,11 @@ tls_write_cb(uv_write_t *req, int status) { isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; isc_nmsocket_t *sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + free_senddata(sock); isc__nm_uvreq_put(&uvreq, sock); @@ -1238,6 +1244,12 @@ tls_cycle_output(isc_nmsocket_t *sock) { break; } + r = uv_timer_start(&sock->write_timer, + isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tls_write_cb); if (r < 0) { @@ -1482,6 +1494,10 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); + r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { result = isc__nm_uverr2result(r); @@ -1629,6 +1645,13 @@ isc__nm_tlsdns_send(isc_nmhandle_t *handle, isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tlsdnssend(sock->mgr, sock, uvreq); isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1806,7 +1829,7 @@ tlsdns_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1821,6 +1844,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_timer_close_cb(uv_handle_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data(timer); + uv_handle_set_data(timer, NULL); + + REQUIRE(VALID_NMSOCK(sock)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tlsdns_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tlsdnssocket); @@ -1874,6 +1908,8 @@ stop_tlsdns_parent(isc_nmsocket_t *sock) { static void tlsdns_close_direct(isc_nmsocket_t *sock) { + int r; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1891,8 +1927,10 @@ tlsdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 7d8c5f4b9a..e8ef4100f1 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -74,7 +74,10 @@ static void udp_close_cb(uv_handle_t *handle); static void -timer_close_cb(uv_handle_t *handle); +read_timer_close_cb(uv_handle_t *handle); + +static void +write_timer_close_cb(uv_handle_t *handle); static void udp_close_direct(isc_nmsocket_t *sock); @@ -269,6 +272,10 @@ route_connect_direct(isc_nmsocket_t *sock) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; goto error; @@ -445,6 +452,10 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + LOCK(&sock->parent->lock); r = uv_udp_open(&sock->uv_handle.udp, sock->fd); @@ -861,6 +872,10 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_timer_init, r); uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; goto error; @@ -1210,7 +1225,7 @@ udp_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1221,6 +1236,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_timer_close_cb(uv_handle_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data(timer); + uv_handle_set_data(timer, NULL); + + REQUIRE(VALID_NMSOCK(sock)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_udp_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_udpsocket); @@ -1273,10 +1299,14 @@ stop_udp_parent(isc_nmsocket_t *sock) { static void udp_close_direct(isc_nmsocket_t *sock) { + int r; REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); - uv_close((uv_handle_t *)&sock->read_timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void From b735182ae0912759f5576557ade7660f4ea9c949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 9 Feb 2022 12:46:29 +0100 Subject: [PATCH 4/7] Add TCP write timeout system test Extend the timeouts system test that bursts the queries for large TXT record and never read any responses back filling up the server TCP write buffer. The test should work with the default wmem_max value on Linux (208k). --- bin/tests/system/timeouts/setup.sh | 5 ++++ bin/tests/system/timeouts/tests-tcp.py | 36 +++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/timeouts/setup.sh b/bin/tests/system/timeouts/setup.sh index 2e8fd6a6ba..c4019d2a27 100644 --- a/bin/tests/system/timeouts/setup.sh +++ b/bin/tests/system/timeouts/setup.sh @@ -20,6 +20,11 @@ copy_setports ns1/named.conf.in ns1/named.conf # tcp-initial-timeout interval # $PYTHON -c " +print('large IN TXT', end=' ') +for a in range(128): + print('\"%s\"' % ('A' * 240), end=' ') +print('') + for a in range(150000): print('%s IN NS a' % (a)) print('%s IN NS b' % (a))" > ns1/large.db diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index 2c5e99cc1c..e1f19608c8 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -51,7 +51,7 @@ def test_initial_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -83,7 +83,7 @@ def test_idle_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -152,7 +152,7 @@ def test_pipelining_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -190,3 +190,33 @@ def test_long_axfr(port): if soa is not None: break assert soa is not None + + +@pytest.mark.dnspython +@pytest.mark.dnspython2 +def test_send_timeout(port): + import dns.query + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.connect(("10.53.0.1", port)) + + # Send and receive single large RDATA over TCP + msg = create_msg("large.example.", "TXT") + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + (response, rtime) = dns.query.receive_tcp(sock, timeout()) + + # Send and receive 28 large (~32k) DNS queries that should + # fill the default maximum 208k TCP send buffer + for n in range(28): + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + + # configure idle interval is 5 seconds, sleep 6 to make sure we are + # above the interval + time.sleep(6) + + with pytest.raises(EOFError): + try: + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + (response, rtime) = dns.query.receive_tcp(sock, timeout()) + except ConnectionError as e: + raise EOFError from e From a89d9e0fa68b8d915c6a1c416543dd157d8b0b5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 9 Feb 2022 19:48:13 +0100 Subject: [PATCH 5/7] Add isc_nmhandle_setwritetimeout() function In some situations (unit test and forthcoming XFR timeouts MR), we need to modify the write timeout independently of the read timeout. Add a isc_nmhandle_setwritetimeout() function that could be called before isc_nm_send() to specify a custom write timeout interval. --- lib/isc/include/isc/netmgr.h | 3 +++ lib/isc/netmgr/netmgr.c | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 6c52f638c8..671880137d 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -697,3 +697,6 @@ isc__nm_force_tid(int tid); * Force the thread ID to 'tid'. This is STRICTLY for use in unit * tests and should not be used in any production code. */ + +void +isc_nmhandle_setwritetimeout(isc_nmhandle_t *handle, uint64_t write_timeout); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index b63f990163..ddcfe23276 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -604,6 +604,14 @@ isc_nm_maxudp(isc_nm_t *mgr, uint32_t maxudp) { atomic_store(&mgr->maxudp, maxudp); } +void +isc_nmhandle_setwritetimeout(isc_nmhandle_t *handle, uint64_t write_timeout) { + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + handle->sock->write_timeout = write_timeout; +} + void isc_nm_settimeouts(isc_nm_t *mgr, uint32_t init, uint32_t idle, uint32_t keepalive, uint32_t advertised) { From ee359d6ffa701e5f9781ec75622af22736506bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 10 Feb 2022 08:42:22 +0100 Subject: [PATCH 6/7] Update writetimeout to be T_IDLE in netmgr_test.c Use the isc_nmhandle_setwritetimeout() function in the netmgr unit test to allow more time for writing and reading the responses because some of the intervals that are used in the unit tests are really small leaving a little room for any delays. --- lib/isc/tests/netmgr_test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/isc/tests/netmgr_test.c b/lib/isc/tests/netmgr_test.c index 824668d4ba..b27c095b7d 100644 --- a/lib/isc/tests/netmgr_test.c +++ b/lib/isc/tests/netmgr_test.c @@ -437,6 +437,7 @@ connect_send(isc_nmhandle_t *handle) { isc_nmhandle_t *sendhandle = NULL; isc_refcount_increment0(&active_csends); isc_nmhandle_attach(handle, &sendhandle); + isc_nmhandle_setwritetimeout(handle, T_IDLE); if (atomic_fetch_sub(&nsends, 1) > 1) { isc_nm_send(sendhandle, (isc_region_t *)&send_msg, connect_send_cb, NULL); @@ -548,6 +549,7 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_nmhandle_t *sendhandle = NULL; isc_nmhandle_attach(handle, &sendhandle); isc_refcount_increment0(&active_ssends); + isc_nmhandle_setwritetimeout(sendhandle, T_IDLE); isc_nm_send(sendhandle, (isc_region_t *)&send_msg, listen_send_cb, cbarg); } From 0c35bda762a97a27c4c580d579851ab0a7f144c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 10 Feb 2022 11:14:58 +0100 Subject: [PATCH 7/7] Add CHANGES and release note for [GL #3132] --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/CHANGES b/CHANGES index 7597fce9d8..6e09a1491e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5807. [bug] Add a TCP "write" timer, and time out writing + connections after the "tcp-idle-timeout" period + has elapsed. [GL #3132] + 5806. [bug] An error in checking the "blackhole" ACL could cause DNS requests sent by named to fail if the destination address or prefix was specifically diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c9ae25023b..18e67bd992 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -70,3 +70,9 @@ Bug Fixes to ``none``. ``blackhole`` worked correctly when it was left unset, or if only positive-match elements were included. This has now been fixed. :gl:`#3157` + +- TCP connections could hang indefinitely if the TCP write buffers + were full because of the other party not reading sent data. This has + been fixed by adding a "write" timer. Connections that are hung + while writing will now time out after the ``tcp-idle-timeout`` period + has elapsed. :gl:`#3132`