From: Ondřej Surý Date: Mon, 2 Dec 2019 10:42:50 +0000 (+0100) Subject: Refactor the dns_geoip API to use ISC_THREAD_LOCAL X-Git-Tag: v9.15.7~39^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a4ffb6407331cbee1b61434c6f3e9c6ef106b30a;p=thirdparty%2Fbind9.git Refactor the dns_geoip API to use ISC_THREAD_LOCAL Previously, the dns_geoip 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, and creating the local memory context was moved to named and stored in the named_g_geoip global context. --- diff --git a/bin/named/geoip.c b/bin/named/geoip.c index f67960fed5e..d1824579e18 100644 --- a/bin/named/geoip.c +++ b/bin/named/geoip.c @@ -113,8 +113,7 @@ named_geoip_load(char *dir) { #endif } -void -named_geoip_shutdown(void) { +void named_geoip_unload(void) { #ifdef HAVE_GEOIP2 if (named_g_geoip->country != NULL) { MMDB_close(named_g_geoip->country); @@ -136,5 +135,12 @@ named_geoip_shutdown(void) { MMDB_close(named_g_geoip->domain); named_g_geoip->domain = NULL; } +#endif +} + +void +named_geoip_shutdown(void) { +#ifdef HAVE_GEOIP2 + named_geoip_unload(); #endif /* HAVE_GEOIP2 */ } diff --git a/bin/named/include/named/geoip.h b/bin/named/include/named/geoip.h index bb148e5b61d..0b959fa23a2 100644 --- a/bin/named/include/named/geoip.h +++ b/bin/named/include/named/geoip.h @@ -19,5 +19,8 @@ named_geoip_init(void); void named_geoip_load(char *dir); +void +named_geoip_unload(void); + void named_geoip_shutdown(void); diff --git a/bin/named/server.c b/bin/named/server.c index 400bd15b46a..628ed0d28a9 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8342,7 +8342,8 @@ load_configuration(const char *filename, named_server_t *server, /* * Release any previously opened GeoIP2 databases. */ - named_geoip_shutdown(); + named_geoip_unload(); + /* * Initialize GeoIP databases from the configured location. * This should happen before configuring any ACLs, so that we @@ -9744,7 +9745,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { } #if defined(HAVE_GEOIP2) named_geoip_shutdown(); - dns_geoip_shutdown(); #endif /* HAVE_GEOIP2 */ dns_db_detach(&server->in_roothints); @@ -9866,7 +9866,7 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { /* * GeoIP must be initialized before the interface * manager (which includes the ACL environment) - * is created + * is created. */ named_geoip_init(); #endif /* HAVE_GEOIP2 */ diff --git a/lib/dns/geoip2.c b/lib/dns/geoip2.c index c0ab66a2415..098a8844d50 100644 --- a/lib/dns/geoip2.c +++ b/lib/dns/geoip2.c @@ -64,7 +64,6 @@ */ typedef struct geoip_state { - isc_mem_t *mctx; uint16_t subtype; const MMDB_s *db; isc_netaddr_t addr; @@ -72,117 +71,28 @@ typedef struct geoip_state { MMDB_entry_s entry; } geoip_state_t; -static isc_mutex_t key_mutex; -static bool state_key_initialized = false; -static isc_thread_key_t state_key; -static isc_once_t mutex_once = ISC_ONCE_INIT; -static isc_mem_t *state_mctx = NULL; +ISC_THREAD_LOCAL geoip_state_t geoip_state = { 0 }; static void -key_mutex_init(void) { - isc_mutex_init(&key_mutex); -} - -static void -free_state(void *arg) { - geoip_state_t *state = arg; - if (state != NULL) { - isc_mem_putanddetach(&state->mctx, - state, sizeof(geoip_state_t)); - } - isc_thread_key_setspecific(state_key, NULL); -} - -static isc_result_t -state_key_init(void) { - isc_result_t result; - - result = isc_once_do(&mutex_once, key_mutex_init); - if (result != ISC_R_SUCCESS) { - return (result); - } - - if (!state_key_initialized) { - LOCK(&key_mutex); - if (!state_key_initialized) { - int ret; - - if (state_mctx == NULL) { - isc_mem_create(&state_mctx); - } - isc_mem_setname(state_mctx, "geoip_state", NULL); - isc_mem_setdestroycheck(state_mctx, false); - - ret = isc_thread_key_create(&state_key, free_state); - if (ret == 0) { - state_key_initialized = true; - } else { - result = ISC_R_FAILURE; - } - } - UNLOCK(&key_mutex); - } - - return (result); -} - -static isc_result_t set_state(const MMDB_s *db, const isc_netaddr_t *addr, - MMDB_lookup_result_s mmresult, MMDB_entry_s entry, - geoip_state_t **statep) + MMDB_lookup_result_s mmresult, MMDB_entry_s entry) { - geoip_state_t *state = NULL; - isc_result_t result; - - result = state_key_init(); - if (result != ISC_R_SUCCESS) { - return (result); - } - - state = (geoip_state_t *) isc_thread_key_getspecific(state_key); - if (state == NULL) { - state = isc_mem_get(state_mctx, sizeof(geoip_state_t)); - memset(state, 0, sizeof(*state)); - - result = isc_thread_key_setspecific(state_key, state); - if (result != ISC_R_SUCCESS) { - isc_mem_put(state_mctx, state, sizeof(geoip_state_t)); - return (result); - } - - isc_mem_attach(state_mctx, &state->mctx); - } - - state->db = db; - state->addr = *addr; - state->mmresult = mmresult; - state->entry = entry; - - if (statep != NULL) { - *statep = state; - } - - return (ISC_R_SUCCESS); + geoip_state.db = db; + geoip_state.addr = *addr; + geoip_state.mmresult = mmresult; + geoip_state.entry = entry; } static geoip_state_t * get_entry_for(MMDB_s * const db, const isc_netaddr_t *addr) { - isc_result_t result; isc_sockaddr_t sa; - geoip_state_t *state; MMDB_lookup_result_s match; int err; - result = state_key_init(); - if (result != ISC_R_SUCCESS) { - return (NULL); - } - - state = (geoip_state_t *) isc_thread_key_getspecific(state_key); - if (state != NULL) { - if (db == state->db && isc_netaddr_equal(addr, &state->addr)) { - return (state); - } + if (db == geoip_state.db && + isc_netaddr_equal(addr, &geoip_state.addr)) + { + return (&geoip_state); } isc_sockaddr_fromnetaddr(&sa, addr, 0); @@ -191,12 +101,9 @@ get_entry_for(MMDB_s * const db, const isc_netaddr_t *addr) { return (NULL); } - result = set_state(db, addr, match, match.entry, &state); - if (result != ISC_R_SUCCESS) { - return (NULL); - } + set_state(db, addr, match, match.entry); - return (state); + return (&geoip_state); } static dns_geoip_subtype_t @@ -482,10 +389,3 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, */ return (false); } - -void -dns_geoip_shutdown(void) { - if (state_mctx != NULL) { - isc_mem_detach(&state_mctx); - } -} diff --git a/lib/dns/include/dns/geoip.h b/lib/dns/include/dns/geoip.h index 4dc3291c62b..223a457ef9f 100644 --- a/lib/dns/include/dns/geoip.h +++ b/lib/dns/include/dns/geoip.h @@ -105,9 +105,6 @@ dns_geoip_match(const isc_netaddr_t *reqaddr, const dns_geoip_databases_t *geoip, const dns_geoip_elem_t *elt); -void -dns_geoip_shutdown(void); - ISC_LANG_ENDDECLS #endif /* HAVE_GEOIP2 */ diff --git a/lib/dns/tests/geoip_test.c b/lib/dns/tests/geoip_test.c index 7481881a1e2..21a887bae2a 100644 --- a/lib/dns/tests/geoip_test.c +++ b/lib/dns/tests/geoip_test.c @@ -334,17 +334,17 @@ int main(void) { #if defined(HAVE_GEOIP2) const struct CMUnitTest tests[] = { - cmocka_unit_test_setup_teardown(country, _setup, _teardown), - cmocka_unit_test_setup_teardown(country_v6, _setup, _teardown), - cmocka_unit_test_setup_teardown(city, _setup, _teardown), - cmocka_unit_test_setup_teardown(city_v6, _setup, _teardown), - cmocka_unit_test_setup_teardown(asnum, _setup, _teardown), - cmocka_unit_test_setup_teardown(isp, _setup, _teardown), - cmocka_unit_test_setup_teardown(org, _setup, _teardown), - cmocka_unit_test_setup_teardown(domain, _setup, _teardown), + cmocka_unit_test(country), + cmocka_unit_test(country_v6), + cmocka_unit_test(city), + cmocka_unit_test(city_v6), + cmocka_unit_test(asnum), + cmocka_unit_test(isp), + cmocka_unit_test(org), + cmocka_unit_test(domain), }; - return (cmocka_run_group_tests(tests, NULL, NULL)); + return (cmocka_run_group_tests(tests, _setup, _teardown)); #else print_message("1..0 # Skip GeoIP not enabled\n"); #endif diff --git a/lib/dns/win32/libdns.def.in b/lib/dns/win32/libdns.def.in index f90ef411132..d77368debbd 100644 --- a/lib/dns/win32/libdns.def.in +++ b/lib/dns/win32/libdns.def.in @@ -384,7 +384,6 @@ dns_generalstats_dump dns_generalstats_increment @IF GEOIP dns_geoip_match -dns_geoip_shutdown @END GEOIP dns_hashalg_fromtext dns_ipkeylist_clear