Avoid indefinite send re-scheduling in TLS DNS

When a peer is not reading the data we are sending it was for the TLS
DNS code to end up in a situation when it would indefinitely
reschedule send requests, effectively turning the 'uv_loop' into a
busy loop that would consume CPU cycles in endless efforts to send
outgoing data.

The main reason for that was only one send buffer dedicated for sends:
the code would re-queue sends until it is empty - that would never
happen when the remote side is not reading data.

That seems like an omission from the older day of the Network Manager
as it is quiet simple to make the code use multiple buffers for
sends. That ultimately breaks the cycle of futile send request
rescheduling.

As a side effect, this commit also gets rid of one memory copying on a
hot path.

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 75b221e33881a0fb06786dcb8992716bfac8491f..cc635e3f5f987ddb4b99f8ff0079f14b6aae3d8e 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -378,9 +378,10 @@ struct isc__nm_uvreq {
        int magic;
        isc_nmsocket_t *sock;
        isc_nmhandle_t *handle;
-       char tcplen[2];        /* The TCP DNS message length */
-       uv_buf_t uvbuf;        /* translated isc_region_t, to be
-                               * sent or received */
+       char tcplen[2]; /* The TCP DNS message length */
+       uv_buf_t uvbuf; /* translated isc_region_t, to be
+                        * sent or received */
+       isc_region_t userbuf;
        isc_sockaddr_t local;  /* local address */
        isc_sockaddr_t peer;   /* peer address */
        isc__nm_cb_t cb;       /* callback */
@@ -995,7 +996,6 @@ struct isc_nmsocket {
                        TLS_STATE_ERROR,
                        TLS_STATE_CLOSING
                } state;
-               isc_region_t senddata;
                ISC_LIST(isc__nm_uvreq_t) sendreqs;
                bool cycle;
                isc_result_t pending_error;

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index 32e685724cee2d9bb7092173bfc477a039509cfb..b41c35384242a7cb4d6a30398d350d20781a2502 100644 (file)
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -1277,17 +1277,17 @@ call_pending_send_callbacks(isc_nmsocket_t *sock, const isc_result_t result) {
 }
 
 static void
-free_senddata(isc_nmsocket_t *sock, const isc_result_t result) {
+free_senddata(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
+             const isc_result_t result) {
        REQUIRE(VALID_NMSOCK(sock));
-       REQUIRE(sock->tls.senddata.base != NULL);
-       REQUIRE(sock->tls.senddata.length > 0);
+       REQUIRE(req != NULL && req->userbuf.base != NULL &&
+               req->userbuf.length > 0);
 
-       isc_mem_put(sock->mgr->mctx, sock->tls.senddata.base,
-                   sock->tls.senddata.length);
-       sock->tls.senddata.base = NULL;
-       sock->tls.senddata.length = 0;
+       isc_mem_put(sock->mgr->mctx, req->userbuf.base, req->userbuf.length);
 
        call_pending_send_callbacks(sock, result);
+
+       isc__nm_uvreq_put(&req, sock);
 }
 
 static void
@@ -1300,9 +1300,7 @@ tls_write_cb(uv_write_t *req, int status) {
        isc_nm_timer_stop(uvreq->timer);
        isc_nm_timer_detach(&uvreq->timer);
 
-       free_senddata(sock, result);
-
-       isc__nm_uvreq_put(&uvreq, sock);
+       free_senddata(sock, uvreq, result);
 
        if (status != 0) {
                if (!sock->client &&
@@ -1339,23 +1337,18 @@ tls_cycle_output(isc_nmsocket_t *sock) {
                int rv;
                int r;
 
-               if (sock->tls.senddata.base != NULL ||
-                   sock->tls.senddata.length > 0)
-               {
-                       break;
-               }
-
                if (pending > (int)ISC_NETMGR_TCP_RECVBUF_SIZE) {
                        pending = (int)ISC_NETMGR_TCP_RECVBUF_SIZE;
                }
 
-               sock->tls.senddata.base = isc_mem_get(sock->mgr->mctx, pending);
-               sock->tls.senddata.length = pending;
-
                /* It's a bit misnomer here, but it does the right thing */
                req = isc__nm_get_read_req(sock, NULL);
-               req->uvbuf.base = (char *)sock->tls.senddata.base;
-               req->uvbuf.len = sock->tls.senddata.length;
+
+               req->userbuf.base = isc_mem_get(sock->mgr->mctx, pending);
+               req->userbuf.length = (size_t)pending;
+
+               req->uvbuf.base = (char *)req->userbuf.base;
+               req->uvbuf.len = (size_t)req->userbuf.length;
 
                rv = BIO_read_ex(sock->tls.app_rbio, req->uvbuf.base,
                                 req->uvbuf.len, &bytes);
@@ -1367,23 +1360,20 @@ tls_cycle_output(isc_nmsocket_t *sock) {
 
                if (r == pending) {
                        /* Wrote everything, restart */
-                       isc__nm_uvreq_put(&req, sock);
-                       free_senddata(sock, ISC_R_SUCCESS);
+                       free_senddata(sock, req, ISC_R_SUCCESS);
                        continue;
                }
 
                if (r > 0) {
                        /* Partial write, send rest asynchronously */
-                       memmove(req->uvbuf.base, req->uvbuf.base + r,
-                               req->uvbuf.len - r);
-                       req->uvbuf.len = req->uvbuf.len - r;
+                       req->uvbuf.base += r;
+                       req->uvbuf.len -= r;
                } else if (r == UV_ENOSYS || r == UV_EAGAIN) {
                        /* uv_try_write is not supported, send
                         * asynchronously */
                } else {
                        result = isc__nm_uverr2result(r);
-                       isc__nm_uvreq_put(&req, sock);
-                       free_senddata(sock, result);
+                       free_senddata(sock, req, result);
                        break;
                }
 
@@ -1391,8 +1381,7 @@ tls_cycle_output(isc_nmsocket_t *sock) {
                             &req->uvbuf, 1, tls_write_cb);
                if (r < 0) {
                        result = isc__nm_uverr2result(r);
-                       isc__nm_uvreq_put(&req, sock);
-                       free_senddata(sock, result);
+                       free_senddata(sock, req, result);
                        break;
                }