From: Ondřej Surý Date: Tue, 2 Jul 2024 07:22:54 +0000 (+0200) Subject: Don't open route socket if we don't need it X-Git-Tag: alessio/regression/026024a6ae~4^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b26079fdaff782ee5d43639d7caa99f1e2c37399;p=thirdparty%2Fbind9.git Don't open route socket if we don't need it When automatic-interface-scan is disabled, the route socket was still being opened. Add new API to connect / disconnect from the route socket only as needed. Additionally, move the block that disables periodic interface rescans to a place where it actually have access to the configuration values. Previously, the values were being checked before the configuration was loaded. --- diff --git a/bin/delv/delv.c b/bin/delv/delv.c index ef078bed095..549d1bde5ae 100644 --- a/bin/delv/delv.c +++ b/bin/delv/delv.c @@ -2154,7 +2154,7 @@ run_server(void *arg) { isc_sockaddr_any(&any); CHECK(dns_dispatch_createudp(dispatchmgr, &any, &dispatch)); CHECK(ns_interfacemgr_create(mctx, sctx, loopmgr, netmgr, dispatchmgr, - NULL, false, &interfacemgr)); + NULL, &interfacemgr)); CHECK(dns_view_create(mctx, dispatchmgr, dns_rdataclass_in, "_default", &view)); diff --git a/bin/named/server.c b/bin/named/server.c index ce8a0ae3c98..6f68406196b 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -8897,15 +8897,6 @@ load_configuration(const char *filename, named_server_t *server, result = named_config_get(maps, "interface-interval", &obj); INSIST(result == ISC_R_SUCCESS); interface_interval = cfg_obj_asduration(obj); - if (server->interface_timer != NULL) { - if (interface_interval == 0) { - isc_timer_stop(server->interface_timer); - } else if (server->interface_interval != interface_interval) { - isc_interval_set(&interval, interface_interval, 0); - isc_timer_start(server->interface_timer, - isc_timertype_ticker, &interval); - } - } server->interface_interval = interface_interval; /* @@ -8916,6 +8907,35 @@ load_configuration(const char *filename, named_server_t *server, INSIST(result == ISC_R_SUCCESS); server->sctx->interface_auto = cfg_obj_asboolean(obj); + if (server->sctx->interface_auto) { + if (ns_interfacemgr_dynamic_updates_are_reliable() && + server->interface_interval != 0) + { + /* + * In some cases the user might expect a certain + * behaviour from the rescan timer, let's try to deduce + * that from the configuration options. + */ + isc_log_write( + named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, + "Disabling periodic interface re-scans timer"); + server->interface_interval = 0; + } + + ns_interfacemgr_routeconnect(server->interfacemgr); + } else { + ns_interfacemgr_routedisconnect(server->interfacemgr); + } + + if (server->interface_interval == 0) { + isc_timer_stop(server->interface_timer); + } else { + isc_interval_set(&interval, interface_interval, 0); + isc_timer_start(server->interface_timer, isc_timertype_ticker, + &interval); + } + /* * Configure the dialup heartbeat timer. */ @@ -9916,26 +9936,12 @@ run_server(void *arg) { CHECKFATAL(ns_interfacemgr_create(named_g_mctx, server->sctx, named_g_loopmgr, named_g_netmgr, - named_g_dispatchmgr, geoip, true, + named_g_dispatchmgr, geoip, &server->interfacemgr), "creating interface manager"); - /* - * In some cases the user might expect a certain behaviour from - * the rescan timer, let's try to deduce that from the - * configuration options. - */ - if ((ns_interfacemgr_dynamic_updates_are_reliable() && - server->interface_auto) || - (server->interface_interval == 0)) - { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "Disabling periodic interface re-scans timer"); - } else { - isc_timer_create(named_g_mainloop, interface_timer_tick, server, - &server->interface_timer); - } + isc_timer_create(named_g_mainloop, interface_timer_tick, server, + &server->interface_timer); isc_timer_create(named_g_mainloop, heartbeat_timer_tick, server, &server->heartbeat_timer); @@ -10053,9 +10059,7 @@ shutdown_server(void *arg) { isc_mem_put(server->mctx, nsc, sizeof(*nsc)); } - if (server->interface_timer != NULL) { - isc_timer_destroy(&server->interface_timer); - } + isc_timer_destroy(&server->interface_timer); isc_timer_destroy(&server->heartbeat_timer); isc_timer_destroy(&server->pps_timer); isc_timer_destroy(&server->tat_timer); diff --git a/lib/ns/include/ns/interfacemgr.h b/lib/ns/include/ns/interfacemgr.h index c9c051cb3ad..35cd8f7f960 100644 --- a/lib/ns/include/ns/interfacemgr.h +++ b/lib/ns/include/ns/interfacemgr.h @@ -101,8 +101,7 @@ isc_result_t ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_loopmgr_t *loopmgr, isc_nm_t *nm, dns_dispatchmgr_t *dispatchmgr, - dns_geoip_databases_t *geoip, bool scan, - ns_interfacemgr_t **mgrp); + dns_geoip_databases_t *geoip, ns_interfacemgr_t **mgrp); /*%< * Create a new interface manager. * @@ -111,14 +110,28 @@ ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx, * to set nonempty listen-on lists. */ +ISC_REFCOUNT_DECL(ns_interfacemgr); + void -ns_interfacemgr_attach(ns_interfacemgr_t *source, ns_interfacemgr_t **target); +ns_interfacemgr_shutdown(ns_interfacemgr_t *mgr); void -ns_interfacemgr_detach(ns_interfacemgr_t **targetp); +ns_interfacemgr_routeconnect(ns_interfacemgr_t *mgr); +/*% + * Connect to the route socket. + * + * NOTE: This function is idempotent. Calling it on an ns_interfacemgr_t object + * with route socket already connected will do nothing. + */ void -ns_interfacemgr_shutdown(ns_interfacemgr_t *mgr); +ns_interfacemgr_routedisconnect(ns_interfacemgr_t *mgr); +/*% + * Disconnect the route socket. + * + * NOTE: This function is idempotent. Calling it on an ns_interfacemgr_t object + * that has no routing socket will do nothing. + */ void ns_interfacemgr_setbacklog(ns_interfacemgr_t *mgr, int backlog); diff --git a/lib/ns/interfacemgr.c b/lib/ns/interfacemgr.c index 965399f63ae..fc061345240 100644 --- a/lib/ns/interfacemgr.c +++ b/lib/ns/interfacemgr.c @@ -210,16 +210,18 @@ route_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, return; } - if (eresult != ISC_R_SUCCESS) { - if (eresult != ISC_R_CANCELED && eresult != ISC_R_SHUTTINGDOWN) - { - isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR, - "automatic interface scanning " - "terminated: %s", - isc_result_totext(eresult)); - } - isc_nmhandle_detach(&mgr->route); - ns_interfacemgr_detach(&mgr); + switch (eresult) { + case ISC_R_SUCCESS: + break; + default: + isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR, + "automatic interface scanning terminated: %s", + isc_result_totext(eresult)); + FALLTHROUGH; + case ISC_R_CANCELED: + case ISC_R_SHUTTINGDOWN: + case ISC_R_EOF: + ns_interfacemgr_routedisconnect(mgr); return; } @@ -271,8 +273,7 @@ isc_result_t ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_loopmgr_t *loopmgr, isc_nm_t *nm, dns_dispatchmgr_t *dispatchmgr, - dns_geoip_databases_t *geoip, bool scan, - ns_interfacemgr_t **mgrp) { + dns_geoip_databases_t *geoip, ns_interfacemgr_t **mgrp) { isc_result_t result; ns_interfacemgr_t *mgr = NULL; @@ -328,20 +329,6 @@ ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx, RUNTIME_CHECK(result == ISC_R_SUCCESS); } - if (scan) { - ns_interfacemgr_t *imgr = NULL; - - ns_interfacemgr_attach(mgr, &imgr); - - result = isc_nm_routeconnect(nm, route_connected, imgr); - if (result != ISC_R_SUCCESS) { - isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_INFO, - "unable to open route socket: %s", - isc_result_totext(result)); - ns_interfacemgr_detach(&imgr); - } - } - return (ISC_R_SUCCESS); cleanup_lock: @@ -351,8 +338,43 @@ cleanup_lock: return (result); } +void +ns_interfacemgr_routeconnect(ns_interfacemgr_t *mgr) { + REQUIRE(NS_INTERFACEMGR_VALID(mgr)); + REQUIRE(isc_tid() == 0); + + if (mgr->route != NULL) { + return; + } + + ns_interfacemgr_ref(mgr); + + isc_result_t result = isc_nm_routeconnect(mgr->nm, route_connected, + mgr); + if (result != ISC_R_SUCCESS) { + isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_INFO, + "unable to open route socket: %s", + isc_result_totext(result)); + ns_interfacemgr_unref(mgr); + } +} + +void +ns_interfacemgr_routedisconnect(ns_interfacemgr_t *mgr) { + REQUIRE(NS_INTERFACEMGR_VALID(mgr)); + REQUIRE(isc_tid() == 0); + + if (mgr->route == NULL) { + return; + } + + isc_nmhandle_close(mgr->route); + isc_nmhandle_detach(&mgr->route); + ns_interfacemgr_detach(&mgr); +} + static void -ns_interfacemgr_destroy(ns_interfacemgr_t *mgr) { +ns_interfacemgr__destroy(ns_interfacemgr_t *mgr) { REQUIRE(NS_INTERFACEMGR_VALID(mgr)); isc_refcount_destroy(&mgr->references); @@ -396,23 +418,7 @@ ns_interfacemgr_getaclenv(ns_interfacemgr_t *mgr) { return (aclenv); } -void -ns_interfacemgr_attach(ns_interfacemgr_t *source, ns_interfacemgr_t **target) { - REQUIRE(NS_INTERFACEMGR_VALID(source)); - isc_refcount_increment(&source->references); - *target = source; -} - -void -ns_interfacemgr_detach(ns_interfacemgr_t **targetp) { - ns_interfacemgr_t *target = *targetp; - *targetp = NULL; - REQUIRE(target != NULL); - REQUIRE(NS_INTERFACEMGR_VALID(target)); - if (isc_refcount_decrement(&target->references) == 1) { - ns_interfacemgr_destroy(target); - } -} +ISC_REFCOUNT_IMPL(ns_interfacemgr, ns_interfacemgr__destroy); void ns_interfacemgr_shutdown(ns_interfacemgr_t *mgr) { diff --git a/tests/libtest/ns.c b/tests/libtest/ns.c index b6ee7490e45..f45c35c2205 100644 --- a/tests/libtest/ns.c +++ b/tests/libtest/ns.c @@ -98,8 +98,7 @@ setup_server(void **state) { } result = ns_interfacemgr_create(mctx, sctx, loopmgr, netmgr, - dispatchmgr, NULL, false, - &interfacemgr); + dispatchmgr, NULL, &interfacemgr); if (result != ISC_R_SUCCESS) { goto cleanup; }