From: Ondřej Surý <ondrej@isc.org>
Date: Tue, 4 Jun 2024 07:12:45 +0000 (+0200)
Subject: Limit the number of DNS message processed from a single TCP read
X-Git-Tag: v9.20.0~9^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4c2ac25a953b6255f41f27998a0d2a084ff9446a;p=thirdparty%2Fbind9.git

Limit the number of DNS message processed from a single TCP read

The single TCP read can create as much as 64k divided by the minimum
size of the DNS message.  This can clog the processing thread and trash
the memory allocator because we need to do as much as ~20k allocations in
a single UV loop tick.

Limit the number of the DNS messages processed in a single UV loop tick
to just single DNS message and limit the number of the outstanding DNS
messages back to 23.  This effectively limits the number of pipelined
DNS messages to that number (this is the limit we already had before).
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 99a4fdb2e80..f5d8c114189 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -85,6 +85,11 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE,
 	      "TCP receive buffer size must be smaller or equal than worker "
 	      "receive buffer size");
 
+/*%
+ * Maximum outstanding DNS message that we process in a single TCP read.
+ */
+#define ISC_NETMGR_MAX_STREAM_CLIENTS_PER_CONN 23
+
 /*%
  * Regular TCP buffer size.
  */
@@ -661,6 +666,9 @@ struct isc_nmsocket {
 	ISC_LIST(isc_nmhandle_t) active_handles;
 	ISC_LIST(isc__nm_uvreq_t) active_uvreqs;
 
+	size_t active_handles_cur;
+	size_t active_handles_max;
+
 	/*%
 	 * Used to pass a result back from listen or connect events.
 	 */
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 9fa5c2b9c80..ba56506007b 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -688,6 +688,7 @@ isc___nmsocket_init(isc_nmsocket_t *sock, isc__networker_t *worker,
 		.inactive_handles = ISC_LIST_INITIALIZER,
 		.result = ISC_R_UNSET,
 		.active_handles = ISC_LIST_INITIALIZER,
+		.active_handles_max = ISC_NETMGR_MAX_STREAM_CLIENTS_PER_CONN,
 		.active_link = ISC_LINK_INITIALIZER,
 		.active = true,
 	};
@@ -853,6 +854,7 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer,
 	}
 
 	ISC_LIST_APPEND(sock->active_handles, handle, active_link);
+	sock->active_handles_cur++;
 
 	switch (sock->type) {
 	case isc_nm_udpsocket:
@@ -967,6 +969,8 @@ nmhandle_destroy(isc_nmhandle_t *handle) {
 	}
 
 	ISC_LIST_UNLINK(sock->active_handles, handle, active_link);
+	INSIST(sock->active_handles_cur > 0);
+	sock->active_handles_cur--;
 
 	if (sock->closehandle_cb == NULL) {
 		nmhandle__destroy(handle);
diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c
index cab9c22f62c..35148fed6ce 100644
--- a/lib/isc/netmgr/streamdns.c
+++ b/lib/isc/netmgr/streamdns.c
@@ -91,6 +91,9 @@ streamdns_try_close_unused(isc_nmsocket_t *sock);
 static bool
 streamdns_closing(isc_nmsocket_t *sock);
 
+static void
+streamdns_resume_processing(void *arg);
+
 static void
 streamdns_resumeread(isc_nmsocket_t *sock, isc_nmhandle_t *transphandle) {
 	if (!sock->streamdns.reading) {
@@ -169,18 +172,32 @@ streamdns_on_complete_dnsmessage(isc_dnsstream_assembler_t *dnsasm,
 		stop = true;
 	}
 
+	if (sock->active_handles_max != 0 &&
+	    (sock->active_handles_cur >= sock->active_handles_max))
+	{
+		stop = true;
+	}
+	INSIST(sock->active_handles_cur <= sock->active_handles_max);
+
 	isc__nmsocket_timer_stop(sock);
-	if (!stop && last_datum) {
+	if (stop) {
+		streamdns_pauseread(sock, transphandle);
+	} else if (last_datum) {
 		/*
 		 * We have processed all data, need to read more.
 		 * The call also restarts the timer.
 		 */
 		streamdns_readmore(sock, transphandle);
-	} else if (stop) {
+	} else {
+		/*
+		 * Process more DNS messages in the next loop tick.
+		 */
 		streamdns_pauseread(sock, transphandle);
+		isc_async_run(sock->worker->loop, streamdns_resume_processing,
+			      sock);
 	}
 
-	return (!stop);
+	return (false);
 }
 
 /*
@@ -670,6 +687,12 @@ streamdns_resume_processing(void *arg) {
 		return;
 	}
 
+	if (sock->active_handles_max != 0 &&
+	    (sock->active_handles_cur >= sock->active_handles_max))
+	{
+		return;
+	}
+
 	streamdns_handle_incoming_data(sock, sock->outerhandle, NULL, 0);
 }