From c33b3d26f695d342af3fa81ab404a366bb8ce873 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 3 Jul 2024 13:58:32 +0300 Subject: [PATCH] TCP/TLS DNS: unthrottle only when all input data processing This commit ensures that we restart reading only when all DNS data in the input buffer is processed so the we will not get into the situation when the buffer is overrun. --- lib/isc/netmgr/netmgr.c | 36 ++++++++++++++++++++++++++++++++---- lib/isc/netmgr/tcpdns.c | 23 ++++------------------- lib/isc/netmgr/tlsdns.c | 23 ++++------------------- 3 files changed, 40 insertions(+), 42 deletions(-) diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 01c770d56a..a42ca90e8d 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -2369,14 +2369,41 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) { int_fast32_t ah = atomic_load(&sock->ah); isc_result_t result = processbuffer(sock); switch (result) { - case ISC_R_NOMORE: + case ISC_R_NOMORE: { /* * Don't reset the timer until we have a * full DNS message. */ - result = isc__nm_start_reading(sock); - if (result != ISC_R_SUCCESS) { - return (result); + + /* + * Restart reading if we have less data in the send + * queue than the send buffer size, this means that the + * TCP client has started reading some data again. + * Starting reading when we go under the limit instead + * of waiting for all data has been flushed allows + * faster recovery (in case there was a congestion and + * now there isn't). + */ + size_t write_queue_size = + uv_stream_get_write_queue_size( + &sock->uv_handle.stream); + if (write_queue_size < ISC_NETMGR_TCP_SENDBUF_SIZE) { + if (sock->reading_throttled) { + isc_log_write(isc_lctx, + ISC_LOGCATEGORY_GENERAL, + ISC_LOGMODULE_NETMGR, + ISC_LOG_DEBUG(3), + "resuming TCP " + "connection, the other " + "side is reading the " + "data again (%zu)", + write_queue_size); + sock->reading_throttled = false; + } + result = isc__nm_start_reading(sock); + if (result != ISC_R_SUCCESS) { + return (result); + } } /* * Start the timer only if there are no externally used @@ -2388,6 +2415,7 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) { isc__nmsocket_timer_start(sock); } goto done; + } case ISC_R_CANCELED: isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 639ff7af02..4b5ee58b6a 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -1162,25 +1162,10 @@ tcpdns_maybe_restart_reading(isc_nmsocket_t *sock) { if (!sock->client && sock->reading_throttled && !uv_is_active(&sock->uv_handle.handle)) { - /* - * Restart reading if we have less data in the send queue than - * the send buffer size, this means that the TCP client has - * started reading some data again. Starting reading when we go - * under the limit instead of waiting for all data has been - * flushed allows faster recovery (in case there was a - * congestion and now there isn't). - */ - size_t write_queue_size = - uv_stream_get_write_queue_size(&sock->uv_handle.stream); - if (write_queue_size < ISC_NETMGR_TCP_SENDBUF_SIZE) { - isc_log_write( - isc_lctx, ISC_LOGCATEGORY_GENERAL, - ISC_LOGMODULE_NETMGR, ISC_LOG_DEBUG(3), - "resuming TCP connection, the other side " - "is reading the data again (%zu)", - write_queue_size); - sock->reading_throttled = false; - isc__nm_start_reading(sock); + isc_result_t result = isc__nm_process_sock_buffer(sock); + if (result != ISC_R_SUCCESS) { + atomic_store(&sock->reading, true); + isc__nm_failed_read_cb(sock, result, false); } } } diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index dbcd34b8ed..dcec05347f 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -1828,25 +1828,10 @@ tlsdns_maybe_restart_reading(isc_nmsocket_t *sock) { if (!sock->client && sock->reading_throttled && !uv_is_active(&sock->uv_handle.handle)) { - /* - * Restart reading if we have less data in the send queue than - * the send buffer size, this means that the TCP client has - * started reading some data again. Starting reading when we go - * under the limit instead of waiting for all data has been - * flushed allows faster recovery (in case there was a - * congestion and now there isn't). - */ - size_t write_queue_size = - uv_stream_get_write_queue_size(&sock->uv_handle.stream); - if (write_queue_size < ISC_NETMGR_TCP_SENDBUF_SIZE) { - isc_log_write( - isc_lctx, ISC_LOGCATEGORY_GENERAL, - ISC_LOGMODULE_NETMGR, ISC_LOG_DEBUG(3), - "resuming TCP connection, the other side " - "is reading the data again (%zu)", - write_queue_size); - sock->reading_throttled = false; - isc__nm_start_reading(sock); + isc_result_t result = isc__nm_process_sock_buffer(sock); + if (result != ISC_R_SUCCESS) { + atomic_store(&sock->reading, true); + isc__nm_failed_read_cb(sock, result, false); } } }