]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
ns_client: reuse TCP send buffers
authorAram Sargsyan <aram@isc.org>
Tue, 12 Mar 2024 15:29:51 +0000 (15:29 +0000)
committerNicki Křížek <nicki@isc.org>
Mon, 10 Jun 2024 14:48:53 +0000 (16:48 +0200)
Constantly allocating, reallocating and deallocating 64K TCP send
buffers by 'ns_client' instances takes too much CPU time.

There is an existing mechanism to reuse the ns_clent_t structure
associated with the handle using 'isc_nmhandle_getdata/_setdata'
(see ns_client_request()), but it doesn't work with TCP, because
every time ns_client_request() is called it gets a new handle even
for the same TCP connection, see the comments in
streamdns_on_complete_dnsmessage().

To solve the problem, we introduce an array of available (unused)
TCP buffers stored in ns_clientmgr_t structure so that a 'client'
working via TCP can have a chance to reuse one (if there is one)
instead of allocating a new one every time.

lib/ns/client.c
lib/ns/include/ns/client.h

index f6ccdd0c5bd6e4659441b5c09119cacbde7cafc9..411a03cb3c7ceb5b9e5dc4b882f6a3b49ce02596 100644 (file)
@@ -98,6 +98,9 @@
 #define COOKIE_SIZE 24U /* 8 + 4 + 4 + 8 */
 #define ECS_SIZE    20U /* 2 + 1 + 1 + [0..16] */
 
+#define TCPBUFFERS_FILLCOUNT 1U
+#define TCPBUFFERS_FREEMAX   8U
+
 #define WANTNSID(x)    (((x)->attributes & NS_CLIENTATTR_WANTNSID) != 0)
 #define WANTEXPIRE(x)  (((x)->attributes & NS_CLIENTATTR_WANTEXPIRE) != 0)
 #define WANTPAD(x)     (((x)->attributes & NS_CLIENTATTR_WANTPAD) != 0)
@@ -369,6 +372,30 @@ client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
        isc_nmhandle_detach(&handle);
 }
 
+static void
+client_setup_tcp_buffer(ns_client_t *client) {
+       REQUIRE(client->tcpbuf == NULL);
+
+       client->tcpbuf = isc_mempool_get(client->manager->tcp_buffers);
+       client->tcpbuf_size = NS_CLIENT_TCP_BUFFER_SIZE;
+}
+
+static void
+client_put_tcp_buffer(ns_client_t *client) {
+       if (client->tcpbuf == NULL) {
+               return;
+       }
+
+       if (client->tcpbuf_size == NS_CLIENT_TCP_BUFFER_SIZE) {
+               isc_mempool_put(client->manager->tcp_buffers, client->tcpbuf);
+       } else {
+               isc_mem_put(client->manager->send_mctx, client->tcpbuf,
+                           client->tcpbuf_size);
+       }
+
+       client->tcpbuf_size = 0;
+}
+
 static void
 client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
                    unsigned char **datap) {
@@ -378,12 +405,9 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
        REQUIRE(datap != NULL);
 
        if (TCP_CLIENT(client)) {
-               INSIST(client->tcpbuf == NULL);
-               client->tcpbuf = isc_mem_get(client->manager->send_mctx,
-                                            NS_CLIENT_TCP_BUFFER_SIZE);
-               client->tcpbuf_size = NS_CLIENT_TCP_BUFFER_SIZE;
+               client_setup_tcp_buffer(client);
                data = client->tcpbuf;
-               isc_buffer_init(buffer, data, NS_CLIENT_TCP_BUFFER_SIZE);
+               isc_buffer_init(buffer, data, client->tcpbuf_size);
        } else {
                data = client->sendbuf;
                if ((client->attributes & NS_CLIENTATTR_HAVECOOKIE) == 0) {
@@ -416,11 +440,56 @@ client_sendpkg(ns_client_t *client, isc_buffer_t *buffer) {
 
        if (isc_buffer_base(buffer) == client->tcpbuf) {
                size_t used = isc_buffer_usedlength(buffer);
-               client->tcpbuf = isc_mem_creget(
-                       client->manager->send_mctx, client->tcpbuf,
-                       client->tcpbuf_size, used, sizeof(char));
-               client->tcpbuf_size = used;
-               r.base = client->tcpbuf;
+               const size_t threshold = (3 * NS_CLIENT_TCP_BUFFER_SIZE) / 4;
+
+               INSIST(client->tcpbuf_size == NS_CLIENT_TCP_BUFFER_SIZE);
+               /*
+                * Copy the data into a smaller buffer before sending,
+                * and keep the original big TCP send buffer for reuse
+                * by other clients.
+                */
+               if (used > threshold) {
+                       /*
+                        * The data in the buffer is very large, so there is
+                        * no point in using a smaller buffer.
+                        */
+                       r.base = buffer->base;
+               } else if (used > NS_CLIENT_SEND_BUFFER_SIZE) {
+                       /*
+                        * We can save space by allocating a new buffer with a
+                        * correct size and freeing the big buffer.
+                        */
+                       unsigned char *new_tcpbuf =
+                               isc_mem_get(client->manager->send_mctx, used);
+                       memmove(new_tcpbuf, buffer->base, used);
+
+                       /*
+                        * Put the big buffer so we can replace the pointer
+                        * and the size with the new ones.
+                        */
+                       client_put_tcp_buffer(client);
+
+                       /*
+                        * Keep the new buffer's information so it can be freed.
+                        */
+                       client->tcpbuf = new_tcpbuf;
+                       client->tcpbuf_size = used;
+
+                       r.base = new_tcpbuf;
+               } else {
+                       /*
+                        * The data fits in the available space in
+                        * 'sendbuf', there is no need for a new buffer.
+                        */
+                       memmove(client->sendbuf, buffer->base, used);
+
+                       /*
+                        * Put the big buffer, we don't need a dynamic buffer.
+                        */
+                       client_put_tcp_buffer(client);
+
+                       r.base = client->sendbuf;
+               }
                r.length = used;
        } else {
                isc_buffer_usedregion(buffer, &r);
@@ -498,8 +567,7 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
        return;
 done:
        if (client->tcpbuf != NULL) {
-               isc_mem_put(client->manager->send_mctx, client->tcpbuf,
-                           client->tcpbuf_size);
+               client_put_tcp_buffer(client);
        }
 
        ns_client_drop(client, result);
@@ -785,8 +853,7 @@ renderend:
 
 cleanup:
        if (client->tcpbuf != NULL) {
-               isc_mem_put(client->manager->send_mctx, client->tcpbuf,
-                           client->tcpbuf_size);
+               client_put_tcp_buffer(client);
        }
 
        if (cleanup_cctx) {
@@ -1625,8 +1692,7 @@ ns__client_reset_cb(void *client0) {
 
        ns_client_endrequest(client);
        if (client->tcpbuf != NULL) {
-               isc_mem_put(client->manager->send_mctx, client->tcpbuf,
-                           client->tcpbuf_size);
+               client_put_tcp_buffer(client);
        }
 
        if (client->keytag != NULL) {
@@ -2373,6 +2439,7 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
 
                client->sendbuf = isc_mem_get(client->manager->send_mctx,
                                              NS_CLIENT_SEND_BUFFER_SIZE);
+
                /*
                 * Set magic earlier than usual because ns_query_init()
                 * and the functions it calls will require it.
@@ -2447,6 +2514,8 @@ clientmgr_destroy_cb(void *arg) {
 
        dns_message_destroypools(&manager->rdspool, &manager->namepool);
 
+       isc_mempool_destroy(&manager->tcp_buffers);
+
        isc_mem_detach(&manager->send_mctx);
 
        isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager));
@@ -2538,6 +2607,13 @@ ns_clientmgr_create(ns_server_t *sctx, isc_loopmgr_t *loopmgr,
         */
        (void)isc_mem_arena_set_muzzy_decay_ms(manager->send_mctx, 0);
 
+       isc_mempool_create(manager->send_mctx,
+                          (size_t)NS_CLIENT_TCP_BUFFER_SIZE,
+                          &manager->tcp_buffers);
+       isc_mempool_setfillcount(manager->tcp_buffers, TCPBUFFERS_FILLCOUNT);
+       isc_mempool_setfreemax(manager->tcp_buffers, TCPBUFFERS_FREEMAX);
+       isc_mempool_setname(manager->tcp_buffers, "ns_clientmgr_tcp");
+
        manager->magic = MANAGER_MAGIC;
 
        MTRACE("create");
index 345f5e778248fe0f523a17fa8fc7542d40e7233c..753cbe677ee7872c285cd42ba9d0afeae2bf9201 100644 (file)
@@ -158,6 +158,8 @@ struct ns_clientmgr {
        /* Lock covers the recursing list */
        isc_mutex_t   reclock;
        client_list_t recursing; /*%< Recursing clients */
+
+       isc_mempool_t *tcp_buffers;
 };
 
 /*% nameserver client structure */