From 5ca49942a332a7d973ddeede679c9c8155e497cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 23 Nov 2022 14:03:23 +0100 Subject: [PATCH] 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. --- lib/isc/netmgr/netmgr-int.h | 14 +++++++++----- lib/isc/netmgr/netmgr.c | 24 ++++++++++++------------ lib/isc/netmgr/tcp.c | 21 ++++++++++----------- lib/isc/netmgr/tcpdns.c | 7 ++++--- lib/isc/netmgr/tlsdns.c | 4 ++-- lib/isc/netmgr/tlsstream.c | 13 +++++++++---- lib/isc/netmgr/udp.c | 14 ++++++++------ 7 files changed, 54 insertions(+), 43 deletions(-) diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index da6d28836d..d17c18d81b 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 5efd4be573..7583e531f5 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 0283596e01..9b2e938003 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 069b5e748d..e1016bea49 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 e5737c253d..528e6d442b 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 35fa09fec2..e564d61fe4 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; - if (handle->sock->outerhandle != NULL) { - isc_nm_read_stop(handle->sock->outerhandle); + sock->reading = false; + + 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 3900372173..0c5d4cf07d 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