]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Limit the number of DNS message processed from a single TCP read
authorOndřej Surý <ondrej@isc.org>
Tue, 4 Jun 2024 07:12:45 +0000 (09:12 +0200)
committerNicki Křížek <nicki@isc.org>
Mon, 10 Jun 2024 14:48:54 +0000 (16:48 +0200)
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).

lib/isc/netmgr/netmgr-int.h
lib/isc/netmgr/netmgr.c
lib/isc/netmgr/streamdns.c

index 99a4fdb2e80eeb693333ba3f6c142e65372deccf..f5d8c114189fd5385792dd43f15e6bb6eb58df72 100644 (file)
@@ -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.
         */
index 9fa5c2b9c809ef3577578a06a28db6287a016a1e..ba56506007b9d1dcb30cef6d68fb6c8ce0bef5d1 100644 (file)
@@ -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);
index cab9c22f62cbe4b757d92e416c4f8cf2b8db57e4..35148fed6ce24309d1baa60c7de05f2e7884dcf3 100644 (file)
@@ -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);
 }