From: Evan Hunt <each@isc.org>
Date: Thu, 2 Jul 2020 14:27:38 +0000 (+0200)
Subject: Use different allocators for UDP and TCP
X-Git-Tag: v9.17.4~10^2~1
X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=38264b6a4d272dbbe948cf66eef9634041d14f98;p=thirdparty%2Fbind9.git

Use different allocators for UDP and TCP

Each worker has a receive buffer with space for 20 DNS messages of up
to 2^16 bytes each, and the allocator function passed to uv_read_start()
or uv_udp_recv_start() will reserve a portion of it for use by sockets.
UDP can use recvmmsg() and so it needs that entire space, but TCP reads
one message at a time.

This commit introduces separate allocator functions for TCP and UDP
setting different buffer size limits, so that libuv will provide the
correct buffer sizes to each of them.
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index b5593a7f59f..6f4d1ff3e77 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -559,16 +559,6 @@ isc__nm_enqueue_ievent(isc__networker_t *worker, isc__netievent_t *event);
  * way to use an isc__networker_t from another thread.)
  */
 
-void
-isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf);
-/*%<
- * Allocator for recv operations.
- *
- * Note that as currently implemented, this doesn't actually
- * allocate anything, it just assigns the the isc__networker's UDP
- * receive buffer to a socket, and marks it as "in use".
- */
-
 void
 isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf);
 /*%<
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 30260afb294..bb10557e9c6 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1003,23 +1003,6 @@ isc__nmsocket_clearcb(isc_nmsocket_t *sock) {
 	sock->accept_cbarg = NULL;
 }
 
-void
-isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) {
-	isc_nmsocket_t *sock = uv_handle_get_data(handle);
-	isc__networker_t *worker = NULL;
-
-	REQUIRE(VALID_NMSOCK(sock));
-	REQUIRE(isc__nm_in_netthread());
-	REQUIRE(size <= ISC_NETMGR_RECVBUF_SIZE);
-
-	worker = &sock->mgr->workers[sock->tid];
-	INSIST(!worker->recvbuf_inuse);
-
-	buf->base = worker->recvbuf;
-	worker->recvbuf_inuse = true;
-	buf->len = ISC_NETMGR_RECVBUF_SIZE;
-}
-
 void
 isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf) {
 	isc__networker_t *worker = NULL;
@@ -1032,7 +1015,7 @@ isc__nm_free_uvbuf(isc_nmsocket_t *sock, const uv_buf_t *buf) {
 	worker = &sock->mgr->workers[sock->tid];
 
 	REQUIRE(worker->recvbuf_inuse);
-	if (buf->base > worker->recvbuf &&
+	if (sock->type == isc_nm_udpsocket && buf->base > worker->recvbuf &&
 	    buf->base <= worker->recvbuf + ISC_NETMGR_RECVBUF_SIZE)
 	{
 		/* Can happen in case of out-of-order recvmmsg in libuv1.36 */
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 558b685caf9..73b8b1fa0b0 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -637,6 +637,30 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 	return (ISC_R_SUCCESS);
 }
 
+/*%<
+ * Allocator for TCP read operations. Limited to size 2^16.
+ *
+ * Note this doesn't actually allocate anything, it just assigns the
+ * worker's receive buffer to a socket, and marks it as "in use".
+ */
+static void
+tcp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) {
+	isc_nmsocket_t *sock = uv_handle_get_data(handle);
+	isc__networker_t *worker = NULL;
+
+	REQUIRE(VALID_NMSOCK(sock));
+	REQUIRE(sock->type == isc_nm_tcpsocket);
+	REQUIRE(isc__nm_in_netthread());
+	REQUIRE(size <= 65536);
+
+	worker = &sock->mgr->workers[sock->tid];
+	INSIST(!worker->recvbuf_inuse);
+
+	buf->base = worker->recvbuf;
+	buf->len = size;
+	worker->recvbuf_inuse = true;
+}
+
 void
 isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) {
 	isc__netievent_startread_t *ievent = (isc__netievent_startread_t *)ev0;
@@ -654,7 +678,7 @@ isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) {
 			       0);
 	}
 
-	r = uv_read_start(&sock->uv_handle.stream, isc__nm_alloc_cb, read_cb);
+	r = uv_read_start(&sock->uv_handle.stream, tcp_alloc_cb, read_cb);
 	if (r != 0) {
 		isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
 	}
diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 544a967ae94..ff981f920fe 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -132,6 +132,32 @@ isc_nm_listenudp(isc_nm_t *mgr, isc_nmiface_t *iface, isc_nm_recv_cb_t cb,
 	return (ISC_R_SUCCESS);
 }
 
+/*%<
+ * Allocator for UDP recv operations. Limited to size 20 * (2^16 + 2),
+ * which allows enough space for recvmmsg() to get multiple messages at
+ * a time.
+ *
+ * Note this doesn't actually allocate anything, it just assigns the
+ * worker's receive buffer to a socket, and marks it as "in use".
+ */
+static void
+udp_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) {
+	isc_nmsocket_t *sock = uv_handle_get_data(handle);
+	isc__networker_t *worker = NULL;
+
+	REQUIRE(VALID_NMSOCK(sock));
+	REQUIRE(sock->type == isc_nm_udpsocket);
+	REQUIRE(isc__nm_in_netthread());
+	REQUIRE(size <= ISC_NETMGR_RECVBUF_SIZE);
+
+	worker = &sock->mgr->workers[sock->tid];
+	INSIST(!worker->recvbuf_inuse);
+
+	buf->base = worker->recvbuf;
+	buf->len = ISC_NETMGR_RECVBUF_SIZE;
+	worker->recvbuf_inuse = true;
+}
+
 /*
  * handle 'udplisten' async call - start listening on a socket.
  */
@@ -193,7 +219,7 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
 	uv_send_buffer_size(&sock->uv_handle.handle,
 			    &(int){ ISC_SEND_BUFFER_SIZE });
 #endif
-	uv_udp_recv_start(&sock->uv_handle.udp, isc__nm_alloc_cb, udp_recv_cb);
+	uv_udp_recv_start(&sock->uv_handle.udp, udp_alloc_cb, udp_recv_cb);
 }
 
 static void