From 4a3d589403dc2c8fc7dbdfe9ef6cf5ff2f2cb5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 2 Dec 2019 11:42:50 +0100 Subject: [PATCH] Refactor the dns_dt API to use ISC_THREAD_LOCAL Previously, the dns_dt API used isc_thread_key API for TLS, which is fairly complicated and requires initialization of memory contexts, etc. This part of code was refactored to use a ISC_THREAD_LOCAL pointer which greatly simplifies the whole code related to storing TLS variables. --- bin/named/server.c | 3 - lib/dns/dnstap.c | 121 ++++++++--------------------------- lib/dns/include/dns/dnstap.h | 7 -- lib/dns/tests/dnstap_test.c | 3 - lib/dns/win32/libdns.def.in | 1 - 5 files changed, 28 insertions(+), 107 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 9321bc1dad..400bd15b46 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9742,9 +9742,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { dns_tsigkey_detach(&named_g_sessionkey); dns_name_free(&named_g_sessionkeyname, server->mctx); } -#ifdef HAVE_DNSTAP - dns_dt_shutdown(); -#endif #if defined(HAVE_GEOIP2) named_geoip_shutdown(); dns_geoip_shutdown(); diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c index 4e7d8e7b48..1f66035ce6 100644 --- a/lib/dns/dnstap.c +++ b/lib/dns/dnstap.c @@ -127,59 +127,16 @@ struct dns_dtenv { goto cleanup; \ } while (0) -static isc_mutex_t dt_mutex; -static bool dt_initialized = false; -static isc_thread_key_t dt_key; -static isc_once_t mutex_once = ISC_ONCE_INIT; -static isc_mem_t *dt_mctx = NULL; -/* - * Change under task exclusive. - */ -static unsigned int generation; +typedef struct ioq { + unsigned int generation; + struct fstrm_iothr_queue *ioq; +} dt__ioq_t; -static void -mutex_init(void) { - isc_mutex_init(&dt_mutex); -} -static void -dtfree(void *arg) { - free(arg); - isc_thread_key_setspecific(dt_key, NULL); -} +ISC_THREAD_LOCAL dt__ioq_t dt_ioq = { 0 }; -static isc_result_t -dt_init(void) { - isc_result_t result; - - result = isc_once_do(&mutex_once, mutex_init); - if (result != ISC_R_SUCCESS) - return (result); - - if (dt_initialized) - return (ISC_R_SUCCESS); - - LOCK(&dt_mutex); - if (!dt_initialized) { - int ret; - - if (dt_mctx == NULL) { - isc_mem_create(&dt_mctx); - } - isc_mem_setname(dt_mctx, "dt", NULL); - isc_mem_setdestroycheck(dt_mctx, false); - - ret = isc_thread_key_create(&dt_key, dtfree); - if (ret == 0) - dt_initialized = true; - else - result = ISC_R_FAILURE; - } - UNLOCK(&dt_mutex); - - return (result); -} +static atomic_uint_fast32_t global_generation; isc_result_t dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path, @@ -202,7 +159,7 @@ dns_dt_create(isc_mem_t *mctx, dns_dtmode_t mode, const char *path, DNS_LOGMODULE_DNSTAP, ISC_LOG_INFO, "opening dnstap destination '%s'", path); - generation++; + atomic_fetch_add_release(&global_generation, 1); env = isc_mem_get(mctx, sizeof(dns_dtenv_t)); @@ -380,7 +337,7 @@ dns_dt_reopen(dns_dtenv_t *env, int roll) { (roll < 0) ? "reopening" : "rolling", env->path); - generation++; + atomic_fetch_add_release(&global_generation, 1); if (env->iothr != NULL) { fstrm_iothr_destroy(&env->iothr); @@ -474,49 +431,33 @@ dns_dt_setversion(dns_dtenv_t *env, const char *version) { return (toregion(env, &env->version, version)); } +static void +set_dt_ioq(unsigned int generation, struct fstrm_iothr_queue *ioq) { + dt_ioq.generation = generation; + dt_ioq.ioq = ioq; +} + static struct fstrm_iothr_queue * dt_queue(dns_dtenv_t *env) { - isc_result_t result; - struct ioq { - unsigned int generation; - struct fstrm_iothr_queue *ioq; - } *ioq; - REQUIRE(VALID_DTENV(env)); - if (env->iothr == NULL) - return (NULL); + unsigned int generation; - result = dt_init(); - if (result != ISC_R_SUCCESS) + if (env->iothr == NULL) { return (NULL); - - ioq = (struct ioq *)isc_thread_key_getspecific(dt_key); - if (ioq != NULL && ioq->generation != generation) { - result = isc_thread_key_setspecific(dt_key, NULL); - if (result != ISC_R_SUCCESS) - return (NULL); - free(ioq); - ioq = NULL; - } - if (ioq == NULL) { - ioq = malloc(sizeof(*ioq)); - if (ioq == NULL) - return (NULL); - ioq->generation = generation; - ioq->ioq = fstrm_iothr_get_input_queue(env->iothr); - if (ioq->ioq == NULL) { - free(ioq); - return (NULL); - } - result = isc_thread_key_setspecific(dt_key, ioq); - if (result != ISC_R_SUCCESS) { - free(ioq); - return (NULL); - } } - return (ioq->ioq); + generation = atomic_load_acquire(&global_generation); + if (dt_ioq.ioq != NULL && dt_ioq.generation != generation) { + set_dt_ioq(0, NULL); + } + if (dt_ioq.ioq == NULL) { + struct fstrm_iothr_queue *ioq = + fstrm_iothr_get_input_queue(env->iothr); + set_dt_ioq(generation, ioq); + } + + return (dt_ioq.ioq); } void @@ -547,7 +488,7 @@ destroy(dns_dtenv_t *env) { "closing dnstap"); env->magic = 0; - generation++; + atomic_fetch_add(&global_generation, 1); if (env->iothr != NULL) fstrm_iothr_destroy(&env->iothr); @@ -927,12 +868,6 @@ dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype, send_dt(view->dtenv, dm.buf, dm.len); } -void -dns_dt_shutdown() { - if (dt_mctx != NULL) - isc_mem_detach(&dt_mctx); -} - static isc_result_t putstr(isc_buffer_t **b, const char *str) { isc_result_t result; diff --git a/lib/dns/include/dns/dnstap.h b/lib/dns/include/dns/dnstap.h index 2e948ebe24..601d3dabbe 100644 --- a/lib/dns/include/dns/dnstap.h +++ b/lib/dns/include/dns/dnstap.h @@ -267,13 +267,6 @@ dns_dt_getstats(dns_dtenv_t *env, isc_stats_t **statsp); *\li ISC_R_NOTFOUND */ -void -dns_dt_shutdown(void); -/*%< - * Shuts down dnstap and frees global resources. This function must only - * be called immediately before server shutdown. - */ - void dns_dt_send(dns_view_t *view, dns_dtmsgtype_t msgtype, isc_sockaddr_t *qaddr, isc_sockaddr_t *dstaddr, diff --git a/lib/dns/tests/dnstap_test.c b/lib/dns/tests/dnstap_test.c index 963f290e71..3c5d407cc1 100644 --- a/lib/dns/tests/dnstap_test.c +++ b/lib/dns/tests/dnstap_test.c @@ -132,8 +132,6 @@ create_test(void **state) { } cleanup(); - - dns_dt_shutdown(); } /* send dnstap messages */ @@ -263,7 +261,6 @@ send_test(void **state) { dns_dt_detach(&view->dtenv); dns_dt_detach(&dtenv); - dns_dt_shutdown(); dns_view_detach(&view); result = dns_dt_open(TAPFILE, dns_dtmode_file, dt_mctx, &handle); diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index 2eef22497c..f90ef41113 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -354,7 +354,6 @@ dns_dt_send dns_dt_setidentity dns_dt_setupfile dns_dt_setversion -dns_dt_shutdown dns_dtdata_free @END NOTYET dns_dumpctx_attach