From: Ondřej Surý <ondrej@isc.org>
Date: Wed, 23 Nov 2022 13:03:23 +0000 (+0100)
Subject: Make the netmgr read callback to be asynchronous only when needed
X-Git-Tag: v9.19.8~42^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5ca49942a332a7d973ddeede679c9c8155e497cb;p=thirdparty%2Fbind9.git

Make the netmgr read callback to be asynchronous only when needed

Previously, the read callback would be synchronous only on success or
timeout.  Add an option (similar to what other callbacks have) to decide
whether we need the asynchronous read callback on a higher level.

On a general level, we need the asynchronous callbacks to happen only
when we are invoking the callback from the public API.  If the path to
the callback went through the libuv callback or netmgr callback, we are
already on asynchronous path, and there's no need to make the call to
the callback asynchronous again.

For the read callback, this means we need the asynchronous path for
failure paths inside the isc_nm_read() (which calls isc__nm_udp_read(),
isc__nm_tcp_read(), etc...) - all other invocations of the read callback
could be synchronous, because those are called from the respective libuv
or netmgr read callbacks.
---

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index da6d28836d0..d17c18d81ba 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -1224,7 +1224,7 @@ isc__nm_async_connectcb(isc__networker_t *worker, isc__netievent_t *ev0);
 
 void
 isc__nm_readcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
-	       isc_result_t eresult);
+	       isc_result_t eresult, bool async);
 void
 isc__nm_async_readcb(isc__networker_t *worker, isc__netievent_t *ev0);
 
@@ -1581,7 +1581,8 @@ isc__nmhandle_tls_setwritetimeout(isc_nmhandle_t *handle,
 				  uint64_t write_timeout);
 
 void
-isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async);
 
 void
 isc__nm_http_stoplistening(isc_nmsocket_t *sock);
@@ -1894,11 +1895,14 @@ NETIEVENT_SOCKET_TLSCTX_DECL(settlsctx);
 NETIEVENT_SOCKET_DECL(sockstop);
 
 void
-isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async);
 void
-isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async);
 void
-isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result);
+isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			      bool async);
 void
 isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 			      bool async);
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 5efd4be5737..7583e531f5a 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1392,20 +1392,20 @@ isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	switch (sock->type) {
 	case isc_nm_udpsocket:
-		isc__nm_udp_failed_read_cb(sock, result);
+		isc__nm_udp_failed_read_cb(sock, result, async);
 		return;
 	case isc_nm_tcpsocket:
-		isc__nm_tcp_failed_read_cb(sock, result);
+		isc__nm_tcp_failed_read_cb(sock, result, async);
 		return;
 	case isc_nm_tcpdnssocket:
-		isc__nm_tcpdns_failed_read_cb(sock, result);
+		isc__nm_tcpdns_failed_read_cb(sock, result, async);
 		return;
 	case isc_nm_tlsdnssocket:
 		isc__nm_tlsdns_failed_read_cb(sock, result, async);
 		return;
 #ifdef HAVE_LIBNGHTTP2
 	case isc_nm_tlssocket:
-		isc__nm_tls_failed_read_cb(sock, result);
+		isc__nm_tls_failed_read_cb(sock, result, async);
 		return;
 #endif
 	default:
@@ -1497,7 +1497,7 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) {
 
 		if (sock->recv_cb != NULL) {
 			isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
-			isc__nm_readcb(sock, req, ISC_R_TIMEDOUT);
+			isc__nm_readcb(sock, req, ISC_R_TIMEDOUT, false);
 		}
 
 		if (!isc__nmsocket_timer_running(sock)) {
@@ -2212,24 +2212,24 @@ isc__nm_async_connectcb(isc__networker_t *worker, isc__netievent_t *ev0) {
 
 void
 isc__nm_readcb(isc_nmsocket_t *sock, isc__nm_uvreq_t *uvreq,
-	       isc_result_t eresult) {
+	       isc_result_t eresult, bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(VALID_UVREQ(uvreq));
 	REQUIRE(VALID_NMHANDLE(uvreq->handle));
 
-	if (eresult == ISC_R_SUCCESS || eresult == ISC_R_TIMEDOUT) {
+	if (!async) {
 		isc__netievent_readcb_t ievent = { .type = netievent_readcb,
 						   .sock = sock,
 						   .req = uvreq,
 						   .result = eresult };
 
 		isc__nm_async_readcb(NULL, (isc__netievent_t *)&ievent);
-	} else {
-		isc__netievent_readcb_t *ievent = isc__nm_get_netievent_readcb(
-			sock->worker, sock, uvreq, eresult);
-		isc__nm_enqueue_ievent(sock->worker,
-				       (isc__netievent_t *)ievent);
+		return;
 	}
+
+	isc__netievent_readcb_t *ievent = isc__nm_get_netievent_readcb(
+		sock->worker, sock, uvreq, eresult);
+	isc__nm_enqueue_ievent(sock->worker, (isc__netievent_t *)ievent);
 }
 
 void
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 0283596e012..9b2e9380035 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -698,7 +698,8 @@ isc__nm_async_tcpstop(isc__networker_t *worker, isc__netievent_t *ev0) {
 }
 
 void
-isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 
@@ -713,7 +714,7 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 	if (sock->recv_cb != NULL) {
 		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
 		isc__nmsocket_clearcb(sock);
-		isc__nm_readcb(sock, req, result);
+		isc__nm_readcb(sock, req, result, async);
 	}
 
 destroy:
@@ -769,18 +770,15 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 	return;
 failure:
 	sock->reading = true;
-	isc__nm_tcp_failed_read_cb(sock, result);
+	isc__nm_tcp_failed_read_cb(sock, result, true);
 }
 
 void
 isc__nm_tcp_read_stop(isc_nmhandle_t *handle) {
-	isc_nmsocket_t *sock = NULL;
-
 	REQUIRE(VALID_NMHANDLE(handle));
+	REQUIRE(VALID_NMSOCK(handle->sock));
 
-	sock = handle->sock;
-
-	REQUIRE(VALID_NMSOCK(sock));
+	isc_nmsocket_t *sock = handle->sock;
 
 	isc__nmsocket_timer_stop(sock);
 	isc__nm_stop_reading(sock);
@@ -802,7 +800,7 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
 	netmgr = sock->worker->netmgr;
 
 	if (isc__nmsocket_closing(sock)) {
-		isc__nm_tcp_failed_read_cb(sock, ISC_R_CANCELED);
+		isc__nm_tcp_failed_read_cb(sock, ISC_R_CANCELED, false);
 		goto free;
 	}
 
@@ -811,7 +809,8 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
 			isc__nm_incstats(sock, STATID_RECVFAIL);
 		}
 
-		isc__nm_tcp_failed_read_cb(sock, isc_uverr2result(nread));
+		isc__nm_tcp_failed_read_cb(sock, isc_uverr2result(nread),
+					   false);
 
 		goto free;
 	}
@@ -832,7 +831,7 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
 					      : atomic_load(&netmgr->idle));
 	}
 
-	isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+	isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
 
 	/* The readcb could have paused the reading */
 	if (sock->reading) {
diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 069b5e748de..e1016bea497 100644
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -671,7 +671,8 @@ isc__nm_async_tcpdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) {
 }
 
 void
-isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			      bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 
@@ -686,7 +687,7 @@ isc__nm_tcpdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 	if (sock->recv_cb != NULL) {
 		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
 		isc__nmsocket_clearcb(sock);
-		isc__nm_readcb(sock, req, result);
+		isc__nm_readcb(sock, req, result, async);
 	}
 
 destroy:
@@ -848,7 +849,7 @@ isc__nm_tcpdns_processbuffer(isc_nmsocket_t *sock) {
 	 */
 	REQUIRE(sock->processing == false);
 	sock->processing = true;
-	isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+	isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
 	sock->processing = false;
 
 	len += 2;
diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index e5737c253d9..528e6d442bb 100644
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -891,7 +891,7 @@ isc__nm_tlsdns_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
 	if (sock->recv_cb != NULL) {
 		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
 		isc__nmsocket_clearcb(sock);
-		isc__nm_readcb(sock, req, result);
+		isc__nm_readcb(sock, req, result, async);
 	}
 
 destroy:
@@ -1054,7 +1054,7 @@ isc__nm_tlsdns_processbuffer(isc_nmsocket_t *sock) {
 	 */
 	REQUIRE(sock->processing == false);
 	sock->processing = true;
-	isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+	isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
 	sock->processing = false;
 
 	len += 2;
diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c
index 35fa09fec2c..e564d61fe47 100644
--- a/lib/isc/netmgr/tlsstream.c
+++ b/lib/isc/netmgr/tlsstream.c
@@ -218,7 +218,10 @@ tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) {
 }
 
 void
-isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async) {
+	UNUSED(async);
+
 	if (!inactive(sock) && sock->tlsstream.state == TLS_IO) {
 		tls_do_bio(sock, NULL, NULL, true);
 	} else if (sock->reading) {
@@ -871,10 +874,12 @@ isc__nm_tls_read_stop(isc_nmhandle_t *handle) {
 	REQUIRE(VALID_NMHANDLE(handle));
 	REQUIRE(VALID_NMSOCK(handle->sock));
 
-	handle->sock->reading = false;
+	isc_nmsocket_t *sock = handle->sock;
+
+	sock->reading = false;
 
-	if (handle->sock->outerhandle != NULL) {
-		isc_nm_read_stop(handle->sock->outerhandle);
+	if (sock->outerhandle != NULL) {
+		isc_nm_read_stop(sock->outerhandle);
 	}
 }
 
diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 3900372173e..0c5d4cf07d4 100644
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -617,7 +617,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
 
 	REQUIRE(!sock->processing);
 	sock->processing = true;
-	isc__nm_readcb(sock, req, ISC_R_SUCCESS);
+	isc__nm_readcb(sock, req, ISC_R_SUCCESS, false);
 	sock->processing = false;
 
 free:
@@ -879,7 +879,8 @@ isc_nm_udpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer,
 }
 
 void
-isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result,
+			   bool async) {
 	REQUIRE(VALID_NMSOCK(sock));
 	REQUIRE(result != ISC_R_SUCCESS);
 	REQUIRE(sock->tid == isc_tid());
@@ -892,14 +893,15 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 		if (!sock->recv_read) {
 			goto destroy;
 		}
-		sock->recv_read = false;
 
 		if (sock->recv_cb != NULL) {
 			isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
 			isc__nmsocket_clearcb(sock);
-			isc__nm_readcb(sock, req, result);
+			isc__nm_readcb(sock, req, result, async);
 		}
 
+		sock->recv_read = false;
+
 	destroy:
 		isc__nmsocket_prep_destroy(sock);
 		return;
@@ -919,7 +921,7 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
 
 	if (sock->recv_cb != NULL) {
 		isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL);
-		isc__nm_readcb(sock, req, result);
+		isc__nm_readcb(sock, req, result, async);
 	}
 }
 
@@ -966,7 +968,7 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
 
 fail:
 	sock->reading = true; /* required by the next call */
-	isc__nm_failed_read_cb(sock, result, false);
+	isc__nm_failed_read_cb(sock, result, true);
 }
 
 static void