From 7b02848865131dd9636ce25a4a4d9d7ca705c2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 13:05:46 +0200 Subject: [PATCH 1/9] Cleanup the uv_import check The uv_import() is not needed anymore, so we can remove the autoconf check for it. --- config.h.win32 | 7 ++----- configure.ac | 2 +- util/copyrights | 1 - win32utils/Configure | 16 ---------------- win32utils/libuv.diff | 27 --------------------------- 5 files changed, 3 insertions(+), 50 deletions(-) delete mode 100644 win32utils/libuv.diff diff --git a/config.h.win32 b/config.h.win32 index bd583dbe90..c76993dd8a 100644 --- a/config.h.win32 +++ b/config.h.win32 @@ -379,13 +379,10 @@ typedef __int64 off_t; #define SSL_CTX_UP_REF 1 /* Define to 1 if you have the `uv_handle_get_data' function. */ -@HAVE_UV_HANDLE_GET_DATA@ +#define HAVE_UV_HANDLE_GET_DATA 1 /* Define to 1 if you have the `uv_handle_set_data' function. */ -@HAVE_UV_HANDLE_SET_DATA@ - -/* Define to 1 if you have the `uv_import' function. */ -@HAVE_UV_IMPORT@ +#define HAVE_UV_HANDLE_SET_DATA 1 /* GSSAPI Related defines */ @HAVE_GSSAPI@ diff --git a/configure.ac b/configure.ac index 9b99141419..467c7d14d7 100644 --- a/configure.ac +++ b/configure.ac @@ -600,7 +600,7 @@ LIBS="$LIBS $LIBUV_LIBS" # Those functions are only provided in newer versions of libuv, we'll be emulating them # for now -AC_CHECK_FUNCS([uv_handle_get_data uv_handle_set_data uv_import uv_udp_connect uv_translate_sys_error uv_sleep]) +AC_CHECK_FUNCS([uv_handle_get_data uv_handle_set_data uv_udp_connect uv_translate_sys_error uv_sleep]) AX_RESTORE_FLAGS([libuv]) # libnghttp2 diff --git a/util/copyrights b/util/copyrights index 22a6cbc908..9e4beecd80 100644 --- a/util/copyrights +++ b/util/copyrights @@ -2260,4 +2260,3 @@ ./win32utils/Configure PERL 2013,2014,2015,2016,2017,2018,2019,2020,2021 ./win32utils/GeoIP.diff X 2013,2018,2019,2020,2021 ./win32utils/bind9.sln.in X 2013,2014,2015,2016,2017,2018,2019,2020 -./win32utils/libuv.diff X 2020,2021 diff --git a/win32utils/Configure b/win32utils/Configure index c5870ea78a..c3903f0a84 100644 --- a/win32utils/Configure +++ b/win32utils/Configure @@ -172,9 +172,6 @@ my @substdefh = ("PACKAGE_VERSION_MAJOR", "WITH_IDN", "CPU_RELAX", "VALIDATION_DEFAULT", - "HAVE_UV_HANDLE_GET_DATA", - "HAVE_UV_HANDLE_SET_DATA", - "HAVE_UV_IMPORT", ); # for platform.h @@ -1276,13 +1273,6 @@ if ($use_libuv eq "auto") { if ($use_libuv eq "auto") { die "can't find an libuv built directory at sibling root\n"; } - - # When a libuv version exposing uv_import() and uv_export() is released, the - # following three config.h macros will need to be conditionally defined for - # that libuv version and all later ones. - # $configdefh{"HAVE_UV_HANDLE_SET_DATA"} = 1; - # $configdefh{"HAVE_UV_HANDLE_GET_DATA"} = 1; - # $configdefh{"HAVE_UV_IMPORT"} = 1; } # falls into (so no else) if ($use_libuv eq "yes") { @@ -1309,12 +1299,6 @@ if ($use_libuv eq "yes") { $configinc{"LIBUV_INC"} = "$libuv_inc"; $configlib{"LIBUV_LIB"} = "$libuv_lib"; $configdll{"LIBUV_DLL"} = "$libuv_dll"; - # When a libuv version exposing uv_import() and uv_export() is released, the - # following three config.h macros will need to be conditionally defined for - # that libuv version and all later ones. - # $configdefh{"HAVE_UV_HANDLE_SET_DATA"} = 1; - # $configdefh{"HAVE_UV_HANDLE_GET_DATA"} = 1; - # $configdefh{"HAVE_UV_IMPORT"} = 1; } # with-nghttp2 diff --git a/win32utils/libuv.diff b/win32utils/libuv.diff deleted file mode 100644 index 41f3796ccc..0000000000 --- a/win32utils/libuv.diff +++ /dev/null @@ -1,27 +0,0 @@ -To make TCP listening properly multithreaded, we need to have the -uv_export() and uv_import() functions that were removed from libuv. -The alternative is passing sockets over IPC, which is complicated and -error prone. - -To make it simple, we export two internal functions from libuv; they will -be used in lib/isc/netmgr/uv-compat.c by our versions of the uv_export() -and uv_import() functions. - -diff --git a/src/win/internal.h b/src/win/internal.h -index 058ddb8e..a9dc4168 100644 ---- a/src/win/internal.h -+++ b/src/win/internal.h -@@ -92,11 +92,11 @@ void uv_process_tcp_connect_req(uv_loop_t* loop, uv_tcp_t* handle, - void uv_tcp_close(uv_loop_t* loop, uv_tcp_t* tcp); - void uv_tcp_endgame(uv_loop_t* loop, uv_tcp_t* handle); - --int uv__tcp_xfer_export(uv_tcp_t* handle, -+UV_EXTERN int uv__tcp_xfer_export(uv_tcp_t* handle, - int pid, - uv__ipc_socket_xfer_type_t* xfer_type, - uv__ipc_socket_xfer_info_t* xfer_info); --int uv__tcp_xfer_import(uv_tcp_t* tcp, -+UV_EXTERN int uv__tcp_xfer_import(uv_tcp_t* tcp, - uv__ipc_socket_xfer_type_t xfer_type, - uv__ipc_socket_xfer_info_t* xfer_info); - From f752840db39eaceb93094e56a378444fba676794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 11:46:38 +0200 Subject: [PATCH 2/9] Add uv_req_get_data() and uv_req_set_data() compatibility shims The uv_req_get_data() and uv_req_set_data() functions were introduced in libuv >= 1.19.0, so we need to add compatibility shims with older libuv versions. --- config.h.win32 | 6 ++++++ configure.ac | 4 +++- lib/isc/netmgr/uv-compat.h | 14 ++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/config.h.win32 b/config.h.win32 index c76993dd8a..abb3ba2957 100644 --- a/config.h.win32 +++ b/config.h.win32 @@ -384,6 +384,12 @@ typedef __int64 off_t; /* Define to 1 if you have the `uv_handle_set_data' function. */ #define HAVE_UV_HANDLE_SET_DATA 1 +/* Define to 1 if you have the `uv_req_get_data' function. */ +#define HAVE_UV_REQ_GET_DATA 1 + +/* Define to 1 if you have the `uv_req_set_data' function. */ +#define HAVE_UV_REQ_SET_DATA 1 + /* GSSAPI Related defines */ @HAVE_GSSAPI@ @HAVE_GSSAPI_H@ diff --git a/configure.ac b/configure.ac index 467c7d14d7..dc458b4172 100644 --- a/configure.ac +++ b/configure.ac @@ -600,7 +600,9 @@ LIBS="$LIBS $LIBUV_LIBS" # Those functions are only provided in newer versions of libuv, we'll be emulating them # for now -AC_CHECK_FUNCS([uv_handle_get_data uv_handle_set_data uv_udp_connect uv_translate_sys_error uv_sleep]) +AC_CHECK_FUNCS([uv_handle_get_data uv_handle_set_data]) +AC_CHECK_FUNCS([uv_req_get_data uv_req_set_data]) +AC_CHECK_FUNCS([uv_udp_connect uv_translate_sys_error uv_sleep]) AX_RESTORE_FLAGS([libuv]) # libnghttp2 diff --git a/lib/isc/netmgr/uv-compat.h b/lib/isc/netmgr/uv-compat.h index 8cd4b160f5..9d9b437294 100644 --- a/lib/isc/netmgr/uv-compat.h +++ b/lib/isc/netmgr/uv-compat.h @@ -33,6 +33,20 @@ uv_handle_set_data(uv_handle_t *handle, void *data) { } #endif /* ifndef HAVE_UV_HANDLE_SET_DATA */ +#ifndef HAVE_UV_REQ_GET_DATA +static inline void * +uv_req_get_data(const uv_req_t *req) { + return (req->data); +} +#endif /* ifndef HAVE_UV_REQ_GET_DATA */ + +#ifndef HAVE_UV_REQ_SET_DATA +static inline void +uv_req_set_data(uv_req_t *req, void *data) { + req->data = data; +} +#endif /* ifndef HAVE_UV_REQ_SET_DATA */ + #ifndef HAVE_UV_SLEEP #define uv_sleep(msec) usleep(msec * 1000) #endif From 7477d1b2edb6c63c6818bbae60c043b0158a05cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 12:34:06 +0200 Subject: [PATCH 3/9] Add uv_os_getenv() and uv_os_setenv() compatibility shims The uv_os_getenv() and uv_os_setenv() functions were introduced in the libuv >= 1.12.0. Add simple compatibility shims for older versions. --- config.h.win32 | 6 ++++++ configure.ac | 1 + lib/isc/netmgr/uv-compat.h | 30 ++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/config.h.win32 b/config.h.win32 index abb3ba2957..6ebb165b5c 100644 --- a/config.h.win32 +++ b/config.h.win32 @@ -384,6 +384,12 @@ typedef __int64 off_t; /* Define to 1 if you have the `uv_handle_set_data' function. */ #define HAVE_UV_HANDLE_SET_DATA 1 +/* Define to 1 if you have the `uv_os_getenv' function. */ +#define HAVE_UV_OS_GETENV 1 + +/* Define to 1 if you have the `uv_os_setenv' function. */ +#define HAVE_UV_OS_SETENV 1 + /* Define to 1 if you have the `uv_req_get_data' function. */ #define HAVE_UV_REQ_GET_DATA 1 diff --git a/configure.ac b/configure.ac index dc458b4172..9b947ff4d8 100644 --- a/configure.ac +++ b/configure.ac @@ -603,6 +603,7 @@ LIBS="$LIBS $LIBUV_LIBS" AC_CHECK_FUNCS([uv_handle_get_data uv_handle_set_data]) AC_CHECK_FUNCS([uv_req_get_data uv_req_set_data]) AC_CHECK_FUNCS([uv_udp_connect uv_translate_sys_error uv_sleep]) +AC_CHECK_FUNCS([uv_os_getenv uv_os_setenv]) AX_RESTORE_FLAGS([libuv]) # libnghttp2 diff --git a/lib/isc/netmgr/uv-compat.h b/lib/isc/netmgr/uv-compat.h index 9d9b437294..2020689c71 100644 --- a/lib/isc/netmgr/uv-compat.h +++ b/lib/isc/netmgr/uv-compat.h @@ -55,6 +55,36 @@ uv_req_set_data(uv_req_t *req, void *data) { #define isc_uv_udp_connect uv_udp_connect #else +#ifndef HAVE_UV_OS_GETENV +#include +#include + +static inline int +uv_os_getenv(const char *name, char *buffer, size_t *size) { + size_t len; + char *buf = getenv(name); + + if (buf == NULL) { + return (UV_ENOENT); + } + + len = strlen(buf) + 1; + if (len > *size) { + *size = len; + return (UV_ENOBUFS); + } + + *size = len; + memmove(buffer, buf, len); + + return (0); +} +#endif /* HAVE_UV_OS_GETENV */ + +#ifndef HAVE_UV_OS_SETENV +#define uv_os_setenv(name, value) setenv(name, value, 0) +#endif /* HAVE_UV_OS_SETENV */ + int isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr); /*%< From 211bfefbaa7c709ef8dc368720dd363316982e41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 13:38:21 +0200 Subject: [PATCH 4/9] Use UV_VERSION_HEX to decide whether we need libuv shim functions Instead of having a configure check for every missing function that has been added in later version of libuv, we now use UV_VERSION_HEX to decide whether we need the shim or not. --- config.h.win32 | 18 -------------- lib/isc/netmgr/uv-compat.c | 4 ++-- lib/isc/netmgr/uv-compat.h | 48 ++++++++++++++++---------------------- 3 files changed, 22 insertions(+), 48 deletions(-) diff --git a/config.h.win32 b/config.h.win32 index 6ebb165b5c..16aab18194 100644 --- a/config.h.win32 +++ b/config.h.win32 @@ -378,24 +378,6 @@ typedef __int64 off_t; /* Define to 1 if you have the `SSL_CTX_up_ref' function. */ #define SSL_CTX_UP_REF 1 -/* Define to 1 if you have the `uv_handle_get_data' function. */ -#define HAVE_UV_HANDLE_GET_DATA 1 - -/* Define to 1 if you have the `uv_handle_set_data' function. */ -#define HAVE_UV_HANDLE_SET_DATA 1 - -/* Define to 1 if you have the `uv_os_getenv' function. */ -#define HAVE_UV_OS_GETENV 1 - -/* Define to 1 if you have the `uv_os_setenv' function. */ -#define HAVE_UV_OS_SETENV 1 - -/* Define to 1 if you have the `uv_req_get_data' function. */ -#define HAVE_UV_REQ_GET_DATA 1 - -/* Define to 1 if you have the `uv_req_set_data' function. */ -#define HAVE_UV_REQ_SET_DATA 1 - /* GSSAPI Related defines */ @HAVE_GSSAPI@ @HAVE_GSSAPI_H@ diff --git a/lib/isc/netmgr/uv-compat.c b/lib/isc/netmgr/uv-compat.c index a0e1add2d7..89d7ac025c 100644 --- a/lib/isc/netmgr/uv-compat.c +++ b/lib/isc/netmgr/uv-compat.c @@ -16,7 +16,7 @@ #include "netmgr-int.h" -#ifndef HAVE_UV_UDP_CONNECT +#if UV_VERSION_HEX < UV_VERSION(1, 27, 0) int isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr) { int err = 0; @@ -46,7 +46,7 @@ isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr) { return (0); } -#endif /* ifndef HAVE_UV_UDP_CONNECT */ +#endif /* UV_VERSION_HEX < UV_VERSION(1, 27, 0) */ int isc_uv_udp_freebind(uv_udp_t *handle, const struct sockaddr *addr, diff --git a/lib/isc/netmgr/uv-compat.h b/lib/isc/netmgr/uv-compat.h index 2020689c71..c7037353f5 100644 --- a/lib/isc/netmgr/uv-compat.h +++ b/lib/isc/netmgr/uv-compat.h @@ -19,43 +19,49 @@ * library version. */ -#ifndef HAVE_UV_HANDLE_GET_DATA +#define UV_VERSION(major, minor, patch) ((major << 16) | (minor << 8) | (patch)) + +#if UV_VERSION_HEX < UV_VERSION(1, 19, 0) static inline void * uv_handle_get_data(const uv_handle_t *handle) { return (handle->data); } -#endif /* ifndef HAVE_UV_HANDLE_GET_DATA */ -#ifndef HAVE_UV_HANDLE_SET_DATA static inline void uv_handle_set_data(uv_handle_t *handle, void *data) { handle->data = data; } -#endif /* ifndef HAVE_UV_HANDLE_SET_DATA */ -#ifndef HAVE_UV_REQ_GET_DATA static inline void * uv_req_get_data(const uv_req_t *req) { return (req->data); } -#endif /* ifndef HAVE_UV_REQ_GET_DATA */ -#ifndef HAVE_UV_REQ_SET_DATA static inline void uv_req_set_data(uv_req_t *req, void *data) { req->data = data; } -#endif /* ifndef HAVE_UV_REQ_SET_DATA */ +#endif /* UV_VERSION_HEX < UV_VERSION(1, 19, 0) */ -#ifndef HAVE_UV_SLEEP +#if UV_VERSION_HEX < UV_VERSION(1, 34, 0) #define uv_sleep(msec) usleep(msec * 1000) -#endif +#endif /* UV_VERSION_HEX < UV_VERSION(1, 34, 0) */ -#ifdef HAVE_UV_UDP_CONNECT +#if UV_VERSION_HEX < UV_VERSION(1, 27, 0) +int +isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr); +/*%< + * Associate the UDP handle to a remote address and port, so every message sent + * by this handle is automatically sent to that destination. + * + * NOTE: This is just a limited shim for uv_udp_connect() as it requires the + * handle to be bound. + */ +#else /* UV_VERSION_HEX < UV_VERSION(1, 27, 0) */ #define isc_uv_udp_connect uv_udp_connect -#else +#endif /* UV_VERSION_HEX < UV_VERSION(1, 27, 0) */ -#ifndef HAVE_UV_OS_GETENV +#if UV_VERSION_HEX < UV_VERSION(1, 12, 0) #include #include @@ -79,23 +85,9 @@ uv_os_getenv(const char *name, char *buffer, size_t *size) { return (0); } -#endif /* HAVE_UV_OS_GETENV */ -#ifndef HAVE_UV_OS_SETENV #define uv_os_setenv(name, value) setenv(name, value, 0) -#endif /* HAVE_UV_OS_SETENV */ - -int -isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr); -/*%< - * Associate the UDP handle to a remote address and port, so every message sent - * by this handle is automatically sent to that destination. - * - * NOTE: This is just a limited shim for uv_udp_connect() as it requires the - * handle to be bound. - */ - -#endif +#endif /* UV_VERSION_HEX < UV_VERSION(1, 12, 0) */ int isc_uv_udp_freebind(uv_udp_t *handle, const struct sockaddr *addr, From 87fe97ed91df2db33b311ef26c680f8fa7a0aff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 09:45:07 +0200 Subject: [PATCH 5/9] Add asynchronous work API to the network manager The libuv has a support for running long running tasks in the dedicated threadpools, so it doesn't affect networking IO. This commit adds isc_nm_work_enqueue() wrapper that would wraps around the libuv API and runs it on top of associated worker loop. The only limitation is that the function must be called from inside network manager thread, so the call to the function should be wrapped inside a (bound) task. --- lib/isc/include/isc/netmgr.h | 19 +++++++ lib/isc/netmgr/netmgr-int.h | 11 ++++ lib/isc/netmgr/netmgr.c | 97 ++++++++++++++++++++++++++++++++++-- lib/isc/win32/libisc.def.in | 2 + 4 files changed, 125 insertions(+), 4 deletions(-) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index ca40fc6bcd..b8f19bf15a 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -68,6 +68,12 @@ typedef void (*isc_nm_opaquecb_t)(void *arg); * callbacks. */ +typedef void (*isc_nm_workcb_t)(void *arg); +typedef void (*isc_nm_after_workcb_t)(void *arg, isc_result_t result); +/*%< + * Callback functions for libuv threadpool work (see uv_work_t) + */ + void isc_nm_attach(isc_nm_t *mgr, isc_nm_t **dst); void @@ -529,6 +535,19 @@ isc_nm_task_enqueue(isc_nm_t *mgr, isc_task_t *task, int threadid); * maximum number of 'workers' as specifed in isc_nm_start() */ +void +isc_nm_work_offload(isc_nm_t *mgr, isc_nm_workcb_t work_cb, + isc_nm_after_workcb_t after_work_cb, void *data); +/*%< + * Schedules a job to be handled by the libuv thread pool (see uv_work_t). + * The function specified in `work_cb` will be run by a thread in the + * thread pool; when complete, the `after_work_cb` function will run. + * + * Requires: + * \li 'mgr' is a valid netmgr object. + * \li We are currently running in a network manager thread. + */ + void isc__nm_force_tid(int tid); /*%< diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index dece53bfec..9e5445664b 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -646,6 +646,17 @@ typedef union { isc__netievent_tlsconnect_t nitc; } isc__netievent_storage_t; +/* + * Work item for a uv_work threadpool. + */ +typedef struct isc__nm_work { + isc_nm_t *netmgr; + uv_work_t req; + isc_nm_workcb_t cb; + isc_nm_after_workcb_t after_cb; + void *data; +} isc__nm_work_t; + /* * Network manager */ diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 9692be6a4b..0b708b57d5 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -40,6 +40,7 @@ #include "netmgr-int.h" #include "netmgr_p.h" #include "openssl_shim.h" +#include "trampoline_p.h" #include "uv-compat.h" /*% @@ -199,6 +200,14 @@ static void isc__nm_async_detach(isc__networker_t *worker, isc__netievent_t *ev0); static void isc__nm_async_close(isc__networker_t *worker, isc__netievent_t *ev0); + +static void +isc__nm_threadpool_initialize(uint32_t workers); +static void +isc__nm_work_cb(uv_work_t *req); +static void +isc__nm_after_work_cb(uv_work_t *req, int status); + /*%< * Issue a 'handle closed' callback on the socket. */ @@ -256,6 +265,17 @@ isc__nm_winsock_destroy(void) { } #endif /* WIN32 */ +static void +isc__nm_threadpool_initialize(uint32_t workers) { + char buf[11]; + int r = uv_os_getenv("UV_THREADPOOL_SIZE", buf, + &(size_t){ sizeof(buf) }); + if (r == UV_ENOENT) { + snprintf(buf, sizeof(buf), "%" PRIu32, workers); + uv_os_setenv("UV_THREADPOOL_SIZE", buf); + } +} + void isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { isc_nm_t *mgr = NULL; @@ -267,6 +287,8 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { isc__nm_winsock_initialize(); #endif /* WIN32 */ + isc__nm_threadpool_initialize(workers); + mgr = isc_mem_get(mctx, sizeof(*mgr)); *mgr = (isc_nm_t){ .nworkers = workers }; @@ -280,8 +302,6 @@ isc__netmgr_create(isc_mem_t *mctx, uint32_t workers, isc_nm_t **netmgrp) { atomic_init(&mgr->workers_paused, 0); atomic_init(&mgr->paused, false); atomic_init(&mgr->closing, false); - atomic_init(&mgr->idle, false); - atomic_init(&mgr->keepalive, false); atomic_init(&mgr->recv_tcp_buffer_size, 0); atomic_init(&mgr->send_tcp_buffer_size, 0); atomic_init(&mgr->recv_udp_buffer_size, 0); @@ -818,7 +838,8 @@ async_cb(uv_async_t *handle) { isc__networker_t *worker = (isc__networker_t *)handle->loop->data; if (process_all_queues(worker)) { - /* If we didn't process all the events, we need to enqueue + /* + * If we didn't process all the events, we need to enqueue * async_cb to be run in the next iteration of the uv_loop */ uv_async_send(handle); @@ -3242,6 +3263,73 @@ isc__nm_set_network_buffers(isc_nm_t *nm, uv_handle_t *handle) { } } +static isc_threadresult_t +isc__nm_work_run(isc_threadarg_t arg) { + isc__nm_work_t *work = (isc__nm_work_t *)arg; + + work->cb(work->data); + + return ((isc_threadresult_t)0); +} + +static void +isc__nm_work_cb(uv_work_t *req) { + isc__nm_work_t *work = uv_req_get_data((uv_req_t *)req); + + if (isc_tid_v == SIZE_MAX) { + isc__trampoline_t *trampoline_arg = + isc__trampoline_get(isc__nm_work_run, work); + (void)isc__trampoline_run(trampoline_arg); + } else { + (void)isc__nm_work_run((isc_threadarg_t)work); + } +} + +static void +isc__nm_after_work_cb(uv_work_t *req, int status) { + isc_result_t result = ISC_R_SUCCESS; + isc__nm_work_t *work = uv_req_get_data((uv_req_t *)req); + isc_nm_t *netmgr = work->netmgr; + + if (status != 0) { + result = isc__nm_uverr2result(status); + } + + work->after_cb(work->data, result); + + isc_mem_put(netmgr->mctx, work, sizeof(*work)); + + isc_nm_detach(&netmgr); +} + +void +isc_nm_work_offload(isc_nm_t *netmgr, isc_nm_workcb_t work_cb, + isc_nm_after_workcb_t after_work_cb, void *data) { + isc__networker_t *worker = NULL; + isc__nm_work_t *work = NULL; + int r; + + REQUIRE(isc__nm_in_netthread()); + REQUIRE(VALID_NM(netmgr)); + + worker = &netmgr->workers[isc_nm_tid()]; + + work = isc_mem_get(netmgr->mctx, sizeof(*work)); + *work = (isc__nm_work_t){ + .cb = work_cb, + .after_cb = after_work_cb, + .data = data, + }; + + isc_nm_attach(netmgr, &work->netmgr); + + uv_req_set_data((uv_req_t *)&work->req, work); + + r = uv_queue_work(&worker->loop, &work->req, isc__nm_work_cb, + isc__nm_after_work_cb); + RUNTIME_CHECK(r == 0); +} + #ifdef NETMGR_TRACE /* * Dump all active sockets in netmgr. We output to stderr @@ -3302,7 +3390,8 @@ nmsocket_dump(isc_nmsocket_t *sock) { nmsocket_type_totext(sock->type), isc_refcount_current(&sock->references)); fprintf(stderr, - "Parent %p, listener %p, server %p, statichandle = %p\n", + "Parent %p, listener %p, server %p, statichandle = " + "%p\n", sock->parent, sock->listener, sock->server, sock->statichandle); fprintf(stderr, "Flags:%s%s%s%s%s\n", atomic_load(&sock->active) ? " active" : "", diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index fdf402a572..664ff6c065 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -474,11 +474,13 @@ isc_nm_tcpdnsconnect isc_nm_gettimeouts isc_nm_setnetbuffers isc_nm_settimeouts +isc_nm_task_enqueue isc_nm_tcpdns_keepalive isc_nm_tcpdns_sequential isc_nm_tid isc_nm_tlsdnsconnect isc_nm_udpconnect +isc_nm_work_offload isc_nmsocket_close isc__nm_acquire_interlocked isc__nm_drop_interlocked From 7670f9837791482cac694069418cec1ed349238d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 09:45:07 +0200 Subject: [PATCH 6/9] Add isc_task_getnetmgr() function Add a function to pull the attached netmgr from inside the executed task. This is needed for any task that needs to call the netmgr API. --- lib/isc/include/isc/task.h | 3 +++ lib/isc/task.c | 7 +++++++ lib/isc/win32/libisc.def.in | 1 + 3 files changed, 11 insertions(+) diff --git a/lib/isc/include/isc/task.h b/lib/isc/include/isc/task.h index f1facae397..fc33330267 100644 --- a/lib/isc/include/isc/task.h +++ b/lib/isc/include/isc/task.h @@ -511,6 +511,9 @@ isc_task_getname(isc_task_t *task); * */ +isc_nm_t * +isc_task_getnetmgr(isc_task_t *task); + void * isc_task_gettag(isc_task_t *task); /*%< diff --git a/lib/isc/task.c b/lib/isc/task.c index c85ac76197..e93c5d5856 100644 --- a/lib/isc/task.c +++ b/lib/isc/task.c @@ -774,6 +774,13 @@ isc_task_gettag(isc_task_t *task) { return (task->tag); } +isc_nm_t * +isc_task_getnetmgr(isc_task_t *task) { + REQUIRE(VALID_TASK(task)); + + return (task->manager->netmgr); +} + /*** *** Task Manager. ***/ diff --git a/lib/isc/win32/libisc.def.in b/lib/isc/win32/libisc.def.in index 664ff6c065..743d2cf6b8 100644 --- a/lib/isc/win32/libisc.def.in +++ b/lib/isc/win32/libisc.def.in @@ -100,6 +100,7 @@ isc_socketmgr_getmaxsockets isc_socketmgr_setreserved isc_socketmgr_setstats isc_task_getname +isc_task_getnetmgr isc_task_gettag isc_task_ready isc_task_run From 8a5c62de83ab1388648360be4fcba248013e46db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 09:45:07 +0200 Subject: [PATCH 7/9] Refactor zone dumping code to use netmgr async threadpools Previously, dumping the zones to the files were quantized, so it doesn't slow down network IO processing. With the introduction of network manager asynchronous threadpools, we can move the IO intensive work to use that API and we don't have to quantize the work anymore as it the file IO won't block anything except other zone dumping processes. --- bin/named/server.c | 4 +- lib/dns/include/dns/masterdump.h | 32 +++---- lib/dns/masterdump.c | 146 +++++++++++++------------------ lib/dns/win32/libdns.def.in | 4 +- lib/dns/zone.c | 2 +- 5 files changed, 78 insertions(+), 110 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 07f853fa95..ba0757348e 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -11487,7 +11487,7 @@ resume: ";\n; Cache dump of view '%s' (cache %s)\n;\n", dctx->view->view->name, dns_cache_getname(dctx->view->view->cache)); - result = dns_master_dumptostreaminc( + result = dns_master_dumptostreamasync( dctx->mctx, dctx->cache, NULL, style, dctx->fp, dctx->task, dumpdone, dctx, &dctx->mdctx); if (result == DNS_R_CONTINUE) { @@ -11547,7 +11547,7 @@ resume: goto nextzone; } dns_db_currentversion(dctx->db, &dctx->version); - result = dns_master_dumptostreaminc( + result = dns_master_dumptostreamasync( dctx->mctx, dctx->db, dctx->version, style, dctx->fp, dctx->task, dumpdone, dctx, &dctx->mdctx); diff --git a/lib/dns/include/dns/masterdump.h b/lib/dns/include/dns/masterdump.h index fcfc2e2e74..dbb8ec8026 100644 --- a/lib/dns/include/dns/masterdump.h +++ b/lib/dns/include/dns/masterdump.h @@ -243,11 +243,11 @@ dns_dumpctx_db(dns_dumpctx_t *dctx); /*@{*/ isc_result_t -dns_master_dumptostreaminc(isc_mem_t *mctx, dns_db_t *db, - dns_dbversion_t * version, - const dns_master_style_t *style, FILE *f, - isc_task_t *task, dns_dumpdonefunc_t done, - void *done_arg, dns_dumpctx_t **dctxp); +dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, + dns_dbversion_t * version, + const dns_master_style_t *style, FILE *f, + isc_task_t *task, dns_dumpdonefunc_t done, + void *done_arg, dns_dumpctx_t **dctxp); isc_result_t dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, @@ -259,11 +259,6 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, * 'format'. If the format is dns_masterformat_text (the RFC1035 format), * 'style' specifies the file style (e.g., &dns_master_style_default). * - * dns_master_dumptostream() is an old form of dns_master_dumptostream3(), - * which always specifies the dns_masterformat_text format. - * dns_master_dumptostream2() is an old form which always specifies - * a NULL header. - * * If 'format' is dns_masterformat_raw, then 'header' can contain * information to be written to the file header. * @@ -276,7 +271,6 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, * * Returns: *\li ISC_R_SUCCESS - *\li ISC_R_CONTINUE dns_master_dumptostreaminc() only. *\li ISC_R_NOMEMORY *\li Any database or rrset iterator error. *\li Any dns_rdata_totext() error code. @@ -286,11 +280,11 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, /*@{*/ isc_result_t -dns_master_dumpinc(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, - const dns_master_style_t *style, const char *filename, - isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, - dns_dumpctx_t **dctxp, dns_masterformat_t format, - dns_masterrawheader_t *header); +dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, + const dns_master_style_t *style, const char *filename, + isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, + dns_dumpctx_t **dctxp, dns_masterformat_t format, + dns_masterrawheader_t *header); isc_result_t dns_master_dump(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, @@ -302,11 +296,6 @@ dns_master_dump(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, * 'format'. If the format is dns_masterformat_text (the RFC1035 format), * 'style' specifies the file style (e.g., &dns_master_style_default). * - * dns_master_dumpinc() and dns_master_dump() are old forms of _dumpinc3() - * and _dump3(), respectively, which always specify the dns_masterformat_text - * format. dns_master_dumpinc2() and dns_master_dump2() are old forms which - * always specify a NULL header. - * * If 'format' is dns_masterformat_raw, then 'header' can contain * information to be written to the file header. * @@ -314,7 +303,6 @@ dns_master_dump(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, * * Returns: *\li ISC_R_SUCCESS - *\li ISC_R_CONTINUE dns_master_dumpinc() only. *\li ISC_R_NOMEMORY *\li Any database or rrset iterator error. *\li Any dns_rdata_totext() error code. diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index c66eb7cceb..0c8a400196 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -265,8 +265,8 @@ struct dns_dumpctx { isc_task_t *task; dns_dumpdonefunc_t done; void *done_arg; - unsigned int nodes; - /* dns_master_dumpinc() */ + /* dns_master_dumpasync() */ + isc_result_t result; char *file; char *tmpfile; dns_masterformat_t format; @@ -1343,7 +1343,7 @@ dump_rdatasets_map(isc_mem_t *mctx, const dns_name_t *name, static const int initial_buffer_length = 1200; static isc_result_t -dumptostreaminc(dns_dumpctx_t *dctx); +dumptostream(dns_dumpctx_t *dctx); static void dumpctx_destroy(dns_dumpctx_t *dctx) { @@ -1486,27 +1486,23 @@ closeandrename(FILE *f, isc_result_t result, const char *temp, return (result); } +/* + * This will run in a libuv threadpool thread. + */ static void -dump_quantum(isc_task_t *task, isc_event_t *event) { - isc_result_t result; - isc_result_t tresult; - dns_dumpctx_t *dctx; - - REQUIRE(event != NULL); - dctx = event->ev_arg; +master_dump_cb(void *data) { + isc_result_t result = ISC_R_UNSET; + dns_dumpctx_t *dctx = data; REQUIRE(DNS_DCTX_VALID(dctx)); + if (atomic_load_acquire(&dctx->canceled)) { result = ISC_R_CANCELED; } else { - result = dumptostreaminc(dctx); - } - if (result == DNS_R_CONTINUE) { - event->ev_arg = dctx; - isc_task_send(task, &event); - return; + result = dumptostream(dctx); } if (dctx->file != NULL) { + isc_result_t tresult = ISC_R_UNSET; tresult = closeandrename(dctx->f, result, dctx->tmpfile, dctx->file); if (tresult != ISC_R_SUCCESS && result == ISC_R_SUCCESS) { @@ -1515,17 +1511,51 @@ dump_quantum(isc_task_t *task, isc_event_t *event) { } else { result = flushandsync(dctx->f, result, NULL); } + + dctx->result = result; +} + +/* + * This will run in a network/task manager thread when the dump is complete. + */ +static void +master_dump_done_cb(void *data, isc_result_t result) { + dns_dumpctx_t *dctx = data; + + if (result == ISC_R_SUCCESS && dctx->result != ISC_R_SUCCESS) { + result = dctx->result; + } + (dctx->done)(dctx->done_arg, result); - isc_event_free(&event); dns_dumpctx_detach(&dctx); } +/* + * This must be run from a network/task manager thread. + */ +static void +setup_dump(isc_task_t *task, isc_event_t *event) { + dns_dumpctx_t *dctx = NULL; + + REQUIRE(isc_nm_tid() >= 0); + REQUIRE(event != NULL); + + dctx = event->ev_arg; + + REQUIRE(DNS_DCTX_VALID(dctx)); + + isc_nm_work_offload(isc_task_getnetmgr(task), master_dump_cb, + master_dump_done_cb, dctx); + + isc_event_free(&event); +} + static isc_result_t task_send(dns_dumpctx_t *dctx) { isc_event_t *event; event = isc_event_allocate(dctx->mctx, NULL, DNS_EVENT_DUMPQUANTUM, - dump_quantum, dctx, sizeof(*event)); + setup_dump, dctx, sizeof(*event)); isc_task_send(dctx->task, &event); return (ISC_R_SUCCESS); } @@ -1548,7 +1578,6 @@ dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, dctx->done = NULL; dctx->done_arg = NULL; dctx->task = NULL; - dctx->nodes = 0; dctx->first = true; atomic_init(&dctx->canceled, false); dctx->file = NULL; @@ -1702,13 +1731,12 @@ writeheader(dns_dumpctx_t *dctx) { } static isc_result_t -dumptostreaminc(dns_dumpctx_t *dctx) { +dumptostream(dns_dumpctx_t *dctx) { isc_result_t result = ISC_R_SUCCESS; isc_buffer_t buffer; char *bufmem; dns_name_t *name; dns_fixedname_t fixname; - unsigned int nodes; isc_time_t start; bufmem = isc_mem_get(dctx->mctx, initial_buffer_length); @@ -1742,9 +1770,8 @@ dumptostreaminc(dns_dumpctx_t *dctx) { result = ISC_R_SUCCESS; } - nodes = dctx->nodes; isc_time_now(&start); - while (result == ISC_R_SUCCESS && (dctx->nodes == 0 || nodes--)) { + while (result == ISC_R_SUCCESS) { dns_rdatasetiter_t *rdsiter = NULL; dns_dbnode_t *node = NULL; @@ -1780,52 +1807,7 @@ dumptostreaminc(dns_dumpctx_t *dctx) { result = dns_dbiterator_next(dctx->dbiter); } - /* - * Work out how many nodes can be written in the time between - * two requests to the nameserver. Smooth the resulting number and - * use it as a estimate for the number of nodes to be written in the - * next iteration. - */ - if (dctx->nodes != 0 && result == ISC_R_SUCCESS) { - unsigned int pps = dns_pps; /* packets per second */ - unsigned int interval; - uint64_t usecs; - isc_time_t end; - - isc_time_now(&end); - if (pps < 100) { - pps = 100; - } - interval = 1000000 / pps; /* interval in usecs */ - if (interval == 0) { - interval = 1; - } - usecs = isc_time_microdiff(&end, &start); - if (usecs == 0) { - dctx->nodes = dctx->nodes * 2; - if (dctx->nodes > 1000) { - dctx->nodes = 1000; - } - } else { - nodes = dctx->nodes * interval; - nodes /= (unsigned int)usecs; - if (nodes == 0) { - nodes = 1; - } else if (nodes > 1000) { - nodes = 1000; - } - - /* Smooth and assign. */ - dctx->nodes = (nodes + dctx->nodes * 7) / 8; - - isc_log_write(dns_lctx, ISC_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTERDUMP, - ISC_LOG_DEBUG(1), - "dumptostreaminc(%p) new nodes -> %d", - dctx, dctx->nodes); - } - result = DNS_R_CONTINUE; - } else if (result == ISC_R_NOMORE) { + if (result == ISC_R_NOMORE) { result = ISC_R_SUCCESS; } cleanup: @@ -1835,11 +1817,11 @@ cleanup: } isc_result_t -dns_master_dumptostreaminc(isc_mem_t *mctx, dns_db_t *db, - dns_dbversion_t *version, - const dns_master_style_t *style, FILE *f, - isc_task_t *task, dns_dumpdonefunc_t done, - void *done_arg, dns_dumpctx_t **dctxp) { +dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, + dns_dbversion_t *version, + const dns_master_style_t *style, FILE *f, + isc_task_t *task, dns_dumpdonefunc_t done, + void *done_arg, dns_dumpctx_t **dctxp) { dns_dumpctx_t *dctx = NULL; isc_result_t result; @@ -1855,7 +1837,6 @@ dns_master_dumptostreaminc(isc_mem_t *mctx, dns_db_t *db, isc_task_attach(task, &dctx->task); dctx->done = done; dctx->done_arg = done_arg; - dctx->nodes = 100; result = task_send(dctx); if (result == ISC_R_SUCCESS) { @@ -1881,7 +1862,7 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, return (result); } - result = dumptostreaminc(dctx); + result = dumptostream(dctx); INSIST(result != DNS_R_CONTINUE); dns_dumpctx_detach(&dctx); @@ -1927,11 +1908,11 @@ cleanup: } isc_result_t -dns_master_dumpinc(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, - const dns_master_style_t *style, const char *filename, - isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, - dns_dumpctx_t **dctxp, dns_masterformat_t format, - dns_masterrawheader_t *header) { +dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, + const dns_master_style_t *style, const char *filename, + isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, + dns_dumpctx_t **dctxp, dns_masterformat_t format, + dns_masterrawheader_t *header) { FILE *f = NULL; isc_result_t result; char *tempname = NULL; @@ -1956,7 +1937,6 @@ dns_master_dumpinc(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, isc_task_attach(task, &dctx->task); dctx->done = done; dctx->done_arg = done_arg; - dctx->nodes = 100; dctx->file = file; file = NULL; dctx->tmpfile = tempname; @@ -2001,7 +1981,7 @@ dns_master_dump(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, goto cleanup; } - result = dumptostreaminc(dctx); + result = dumptostream(dctx); INSIST(result != DNS_R_CONTINUE); dns_dumpctx_detach(&dctx); diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index ab01b696df..97208946ef 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -493,11 +493,11 @@ dns_lookup_cancel dns_lookup_create dns_lookup_destroy dns_master_dump -dns_master_dumpinc +dns_master_dumpasync dns_master_dumpnode dns_master_dumpnodetostream dns_master_dumptostream -dns_master_dumptostreaminc +dns_master_dumptostreamasync dns_master_initrawheader dns_master_loadbuffer dns_master_loadbufferinc diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 6424343613..c4788ea203 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -2549,7 +2549,7 @@ zone_gotwritehandle(isc_task_t *task, isc_event_t *event) { } else { output_style = &dns_master_style_default; } - result = dns_master_dumpinc( + result = dns_master_dumpasync( zone->mctx, db, version, output_style, zone->masterfile, zone->task, dump_done, zone, &zone->dctx, zone->masterformat, &rawdata); From e83b6569dabe12024f30184dd54d5c9737a39f6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 11:04:37 +0200 Subject: [PATCH 8/9] Indicate to the kernel that we won't be needing the zone dumps Add a call to posix_fadvise() to indicate to the kernel, that `named` won't be needing the dumped zone files any time soon with: * POSIX_FADV_DONTNEED - The specified data will not be accessed in the near future. Notes: POSIX_FADV_DONTNEED attempts to free cached pages associated with the specified region. This is useful, for example, while streaming large files. A program may periodically request the kernel to free cached data that has already been used, so that more useful cached pages are not discarded instead. --- lib/dns/masterdump.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 0c8a400196..c5f00bd1c3 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -1898,6 +1898,11 @@ opentmp(isc_mem_t *mctx, dns_masterformat_t format, const char *file, isc_result_totext(result)); goto cleanup; } + +#if defined(POSIX_FADV_DONTNEED) + posix_fadvise(fileno(f), 0, 0, POSIX_FADV_DONTNEED); +#endif + *tempp = tempname; *fp = f; return (ISC_R_SUCCESS); From 3e433b87fbca6bcbb2334b359b059ca5490dd598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 27 May 2021 10:34:57 +0200 Subject: [PATCH 9/9] Add CHANGES and release note for [GL #2732] --- CHANGES | 3 +++ doc/notes/notes-current.rst | 3 +++ 2 files changed, 6 insertions(+) diff --git a/CHANGES b/CHANGES index 2838b49b5b..9b37ab14fe 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5651. [func] Refactor zone dumping to be processed asynchronously + via the uv_work_t thread pool API. [GL #2732] + 5650. [bug] Prevent a crash that could occur if serve-stale was enabled and a prefetch was triggered during a query restart. [GL #2733] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 04f1e8db86..4694b063b5 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -33,6 +33,9 @@ New Features became clogged up with queries that are too old and have already timeouted on the receiving side. :gl:`#2313` +- Run zone dumping tasks on separate asynchronous thread pools. This change + makes zone dumping no longer block networking I/O. :gl:`#2732` + Removed Features ~~~~~~~~~~~~~~~~