diff --git a/bin/named/server.c b/bin/named/server.c index 0abefe8407..7957591281 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10003,7 +10003,7 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { ns_interfacemgr_detach(&server->interfacemgr); - dns_dispatchmgr_destroy(&named_g_dispatchmgr); + dns_dispatchmgr_detach(&named_g_dispatchmgr); dns_zonemgr_shutdown(server->zonemgr); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index a3bb130a7e..0690676db4 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -735,7 +735,7 @@ doshutdown(void) { } ddebug("Shutting down dispatch manager"); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } static void diff --git a/bin/tests/system/pipelined/pipequeries.c b/bin/tests/system/pipelined/pipequeries.c index 6b34dd808c..e41f14a566 100644 --- a/bin/tests/system/pipelined/pipequeries.c +++ b/bin/tests/system/pipelined/pipequeries.c @@ -286,7 +286,7 @@ main(int argc, char *argv[]) { dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); diff --git a/bin/tests/system/tkey/keycreate.c b/bin/tests/system/tkey/keycreate.c index 9ede9a65b5..b83d613f82 100644 --- a/bin/tests/system/tkey/keycreate.c +++ b/bin/tests/system/tkey/keycreate.c @@ -255,7 +255,7 @@ main(int argc, char *argv[]) { dns_requestmgr_shutdown(requestmgr); dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); isc_managers_destroy(&netmgr, &taskmgr, NULL, NULL); diff --git a/bin/tests/system/tkey/keydelete.c b/bin/tests/system/tkey/keydelete.c index c76d816132..9b32e2930e 100644 --- a/bin/tests/system/tkey/keydelete.c +++ b/bin/tests/system/tkey/keydelete.c @@ -198,7 +198,7 @@ main(int argc, char **argv) { dns_requestmgr_shutdown(requestmgr); dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchv4); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); isc_managers_destroy(&netmgr, &taskmgr, NULL, NULL); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index f3d37da52c..ca3acb8eac 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -2175,7 +2175,7 @@ main(int argc, char *argv[]) { dns_requestmgr_detach(&requestmgr); dns_dispatch_detach(&dispatchvx); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); isc_task_shutdown(task); isc_task_detach(&task); diff --git a/lib/dns/client.c b/lib/dns/client.c index d33be3708f..030fa035b6 100644 --- a/lib/dns/client.c +++ b/lib/dns/client.c @@ -268,7 +268,6 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr6) { isc_result_t result; dns_client_t *client = NULL; - dns_dispatchmgr_t *dispatchmgr = NULL; dns_dispatch_t *dispatchv4 = NULL; dns_dispatch_t *dispatchv6 = NULL; dns_view_t *view = NULL; @@ -293,12 +292,11 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, goto cleanup_lock; } - result = dns_dispatchmgr_create(mctx, nm, &dispatchmgr); + result = dns_dispatchmgr_create(mctx, nm, &client->dispatchmgr); if (result != ISC_R_SUCCESS) { goto cleanup_task; } - client->dispatchmgr = dispatchmgr; - (void)setsourceports(mctx, dispatchmgr); + (void)setsourceports(mctx, client->dispatchmgr); /* * If only one address family is specified, use it. @@ -306,7 +304,7 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, */ client->dispatchv4 = NULL; if (localaddr4 != NULL || localaddr6 == NULL) { - result = getudpdispatch(AF_INET, dispatchmgr, taskmgr, + result = getudpdispatch(AF_INET, client->dispatchmgr, taskmgr, &dispatchv4, localaddr4); if (result == ISC_R_SUCCESS) { client->dispatchv4 = dispatchv4; @@ -315,7 +313,7 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, client->dispatchv6 = NULL; if (localaddr6 != NULL || localaddr4 == NULL) { - result = getudpdispatch(AF_INET6, dispatchmgr, taskmgr, + result = getudpdispatch(AF_INET6, client->dispatchmgr, taskmgr, &dispatchv6, localaddr6); if (result == ISC_R_SUCCESS) { client->dispatchv6 = dispatchv6; @@ -332,8 +330,8 @@ dns_client_create(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr, /* Create the default view for class IN */ result = createview(mctx, dns_rdataclass_in, taskmgr, RESOLVER_NTASKS, - nm, timermgr, dispatchmgr, dispatchv4, dispatchv6, - &view); + nm, timermgr, client->dispatchmgr, dispatchv4, + dispatchv6, &view); if (result != ISC_R_SUCCESS) { goto cleanup_references; } @@ -366,7 +364,7 @@ cleanup_dispatchmgr: if (dispatchv6 != NULL) { dns_dispatch_detach(&dispatchv6); } - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&client->dispatchmgr); cleanup_task: isc_task_detach(&client->task); cleanup_lock: @@ -394,7 +392,7 @@ destroyclient(dns_client_t *client) { dns_dispatch_detach(&client->dispatchv6); } - dns_dispatchmgr_destroy(&client->dispatchmgr); + dns_dispatchmgr_detach(&client->dispatchmgr); isc_task_detach(&client->task); diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index 3f36a52223..75803f210d 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -41,8 +41,6 @@ typedef ISC_LIST(dns_dispentry_t) dns_displist_t; -typedef struct dispsocket dispsocket_t; - typedef struct dns_qid { unsigned int magic; isc_mutex_t lock; @@ -54,6 +52,7 @@ typedef struct dns_qid { struct dns_dispatchmgr { /* Unlocked. */ unsigned int magic; + isc_refcount_t references; isc_mem_t *mctx; dns_acl_t *blackhole; isc_stats_t *stats; @@ -69,8 +68,6 @@ struct dns_dispatchmgr { isc_mutex_t buffer_lock; unsigned int buffers; - isc_refcount_t irefs; - in_port_t *v4ports; /*%< available ports for IPv4 */ unsigned int nv4ports; /*%< # of available ports for IPv4 */ in_port_t *v6ports; /*%< available ports for IPv4 */ @@ -82,22 +79,25 @@ struct dns_dispatchmgr { struct dns_dispentry { unsigned int magic; + isc_refcount_t references; dns_dispatch_t *disp; - dns_messageid_t id; - in_port_t port; + isc_task_t *task; + isc_nmhandle_t *handle; /*%< netmgr handle for UDP connection */ unsigned int bucket; unsigned int timeout; + isc_sockaddr_t local; isc_sockaddr_t peer; - isc_task_t *task; + in_port_t port; + dns_messageid_t id; isc_nm_cb_t connected; isc_nm_cb_t sent; isc_nm_cb_t timedout; isc_taskaction_t action; void *arg; - bool item_out; - dispsocket_t *dispsocket; + bool canceled; ISC_LIST(dns_dispatchevent_t) items; ISC_LINK(dns_dispentry_t) link; + ISC_LINK(dns_dispentry_t) alink; }; /*% @@ -107,17 +107,6 @@ struct dns_dispentry { #define DNS_DISPATCH_UDPBUFSIZE 4096 #endif /* ifndef DNS_DISPATCH_UDPBUFSIZE */ -struct dispsocket { - unsigned int magic; - isc_nmhandle_t *handle; - dns_dispatch_t *disp; - isc_sockaddr_t local; - isc_sockaddr_t peer; - dns_dispentry_t *resp; - in_port_t port; - ISC_LINK(dispsocket_t) link; -}; - struct dns_dispatch { /* Unlocked. */ unsigned int magic; /*%< magic */ @@ -127,9 +116,6 @@ struct dns_dispatch { isc_sockaddr_t local; /*%< local address */ in_port_t localport; /*%< local UDP port */ isc_sockaddr_t peer; /*%< peer address (TCP) */ - isc_event_t *ctlevent; - - isc_mem_t *sepool; /*%< pool for socket events */ /*% Locked by mgr->lock. */ ISC_LINK(dns_dispatch_t) link; @@ -138,13 +124,13 @@ struct dns_dispatch { isc_mutex_t lock; /*%< locks all below */ isc_socktype_t socktype; unsigned int attributes; - isc_refcount_t refcount; - dns_dispatchevent_t *failsafe_ev; /*%< failsafe cancel event */ - unsigned int shutting_down : 1, shutdown_out : 1, recv_pending : 1; - isc_result_t shutdown_why; - ISC_LIST(dispsocket_t) activesockets; - ISC_LIST(dispsocket_t) inactivesockets; + isc_refcount_t references; + dns_dispatchevent_t failsafe_ev; /*%< failsafe cancel event */ + unsigned int shutdown_out : 1, recv_pending : 1; + + ISC_LIST(dns_dispentry_t) active; unsigned int nsockets; + unsigned int requests; /*%< how many requests we have */ unsigned int tcpbuffers; /*%< allocated buffers */ }; @@ -164,17 +150,6 @@ struct dns_dispatch { #define DNS_DISPATCHMGR_MAGIC ISC_MAGIC('D', 'M', 'g', 'r') #define VALID_DISPATCHMGR(e) ISC_MAGIC_VALID((e), DNS_DISPATCHMGR_MAGIC) -/*% - * Maximum number of dispatch sockets that can be pooled for reuse. The - * appropriate value may vary, but experiments have shown a busy caching server - * may need more than 1000 sockets concurrently opened. The maximum allowable - * number of dispatch sockets (per manager) will be set to the double of this - * value. - */ -#ifndef DNS_DISPATCH_POOLSOCKS -#define DNS_DISPATCH_POOLSOCKS 2048 -#endif /* ifndef DNS_DISPATCH_POOLSOCKS */ - /*% * Quota to control the number of UDP dispatch sockets. If a dispatch has * more than the quota of sockets, new queries will purge oldest ones, so @@ -194,14 +169,6 @@ struct dns_dispatch { #define DNS_DISPATCH_MAXBUFFERS 37268 #endif /* ifndef DNS_DISPATCH_MAXBUFFERS */ -/*% - * Number of dispatch sockets available for all dispatches in the - * socket memory pool. - */ -#ifndef DNS_DISPATCH_MAXSOCKETS -#define DNS_DISPATCH_MAXSOCKETS 32768 -#endif /* ifndef DNS_DISPATCH_MAXSOCKETS */ - /*% * Quota to control the number of concurrent requests that can be handled * by each TCP dispatch. (UDP dispatches do not currently support socket @@ -227,17 +194,12 @@ struct dns_dispatch { /* * Statics. */ +static void +dispatchmgr_destroy(dns_dispatchmgr_t *mgr); + static dns_dispentry_t * entry_search(dns_qid_t *, const isc_sockaddr_t *, dns_messageid_t, in_port_t, unsigned int); -static bool -destroy_disp_ok(dns_dispatch_t *); -static void -destroy_disp(isc_task_t *task, isc_event_t *event); -static void -destroy_dispsocket(dns_dispatch_t *, dispsocket_t **); -static void -deactivate_dispsocket(dns_dispatch_t *, dispsocket_t *); static void udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg); @@ -251,31 +213,19 @@ free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev); static inline dns_dispatchevent_t * allocate_devent(dns_dispatch_t *disp); static void -do_cancel(dns_dispatch_t *disp); -static dns_dispentry_t * -linear_first(dns_qid_t *disp); -static dns_dispentry_t * -linear_next(dns_qid_t *disp, dns_dispentry_t *resp); -static void dispatch_free(dns_dispatch_t **dispp); static isc_result_t dispatch_createudp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, unsigned int attributes, dns_dispatch_t **dispp); -static bool -destroy_mgr_ok(dns_dispatchmgr_t *mgr); -static void -destroy_mgr(dns_dispatchmgr_t **mgrp); static void qid_allocate(dns_dispatchmgr_t *mgr, dns_qid_t **qidp); static void qid_destroy(isc_mem_t *mctx, dns_qid_t **qidp); -static isc_nmhandle_t * -gethandle(dns_dispatch_t *disp); -static isc_nmhandle_t * +static inline isc_nmhandle_t * getentryhandle(dns_dispentry_t *resp); static void -startrecv(dns_dispatch_t *disp, dispsocket_t *dispsocket); +startrecv(dns_dispatch_t *disp, dns_dispentry_t *resp); #define LVL(x) ISC_LOG_DEBUG(x) @@ -345,7 +295,6 @@ static void request_log(dns_dispatch_t *disp, dns_dispentry_t *resp, int level, const char *fmt, ...) { char msgbuf[2048]; - char peerbuf[256]; va_list ap; if (!isc_log_wouldlog(dns_lctx, level)) { @@ -357,6 +306,7 @@ request_log(dns_dispatch_t *disp, dns_dispentry_t *resp, int level, va_end(ap); if (VALID_RESPONSE(resp)) { + char peerbuf[256]; isc_sockaddr_format(&resp->peer, peerbuf, sizeof(peerbuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_DISPATCH, DNS_LOGMODULE_DISPATCH, level, @@ -387,136 +337,17 @@ dns_hash(dns_qid_t *qid, const isc_sockaddr_t *dest, dns_messageid_t id, return (ret); } -/* - * Find the first entry in 'qid'. Returns NULL if there are no entries. - */ -static dns_dispentry_t * -linear_first(dns_qid_t *qid) { - dns_dispentry_t *ret = NULL; - unsigned int bucket = 0; - - while (bucket < qid->qid_nbuckets) { - ret = ISC_LIST_HEAD(qid->qid_table[bucket]); - if (ret != NULL) { - return (ret); - } - bucket++; - } - - return (NULL); -} - -/* - * Find the next entry after 'resp' in 'qid'. Return NULL if there are - * no more entries. - */ -static dns_dispentry_t * -linear_next(dns_qid_t *qid, dns_dispentry_t *resp) { - dns_dispentry_t *ret = NULL; - unsigned int bucket; - - ret = ISC_LIST_NEXT(resp, link); - if (ret != NULL) { - return (ret); - } - - bucket = resp->bucket + 1; - while (bucket < qid->qid_nbuckets) { - ret = ISC_LIST_HEAD(qid->qid_table[bucket]); - if (ret != NULL) { - return (ret); - } - bucket++; - } - - return (NULL); -} - -/* - * The dispatch must be locked. - */ -static bool -destroy_disp_ok(dns_dispatch_t *disp) { - if (isc_refcount_current(&disp->refcount) != 0) { - return (false); - } - - if (disp->recv_pending != 0) { - return (false); - } - - if (!ISC_LIST_EMPTY(disp->activesockets)) { - return (false); - } - - if (disp->shutting_down == 0) { - return (false); - } - - return (true); -} - -/* - * Called when refcount reaches 0 (and safe to destroy). - * - * The dispatcher must be locked. - * The manager must not be locked. - */ -static void -destroy_disp(isc_task_t *task, isc_event_t *event) { - dns_dispatch_t *disp = NULL; - dns_dispatchmgr_t *mgr = NULL; - bool killmgr; - dispsocket_t *dispsocket = NULL; - - INSIST(event->ev_type == DNS_EVENT_DISPATCHCONTROL); - - UNUSED(task); - - disp = event->ev_arg; - mgr = disp->mgr; - - LOCK(&mgr->lock); - ISC_LIST_UNLINK(mgr->list, disp, link); - - dispatch_log(disp, LVL(90), "shutting down; detaching from handle %p", - disp->handle); - - if (disp->sepool != NULL) { - isc_mem_destroy(&disp->sepool); - } - - if (disp->handle != NULL) { - isc_nmhandle_detach(&disp->handle); - } - while ((dispsocket = ISC_LIST_HEAD(disp->inactivesockets)) != NULL) { - ISC_LIST_UNLINK(disp->inactivesockets, dispsocket, link); - destroy_dispsocket(disp, &dispsocket); - } - isc_task_detach(&disp->task); - isc_event_free(&event); - - dispatch_free(&disp); - - killmgr = destroy_mgr_ok(mgr); - UNLOCK(&mgr->lock); - if (killmgr) { - destroy_mgr(&mgr); - } -} - /*% - * Make a new socket for a single dispatch with a random port number. + * Choose a random port number for a dispatch entry. * The caller must hold the disp->lock */ static isc_result_t -get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, - dispsocket_t **dispsockp, in_port_t *portp) { +setup_socket(dns_dispatch_t *disp, dns_dispentry_t *resp, + const isc_sockaddr_t *dest, in_port_t *portp) { dns_dispatchmgr_t *mgr = disp->mgr; - in_port_t port; - dispsocket_t *dispsock = NULL; unsigned int nports; in_port_t *ports = NULL; + in_port_t port; if (isc_sockaddr_pf(&disp->local) == AF_INET) { nports = mgr->nv4ports; @@ -529,81 +360,34 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, return (ISC_R_ADDRNOTAVAIL); } - dispsock = ISC_LIST_HEAD(disp->inactivesockets); - if (dispsock != NULL) { - ISC_LIST_UNLINK(disp->inactivesockets, dispsock, link); - } else { - dispsock = isc_mem_get(mgr->mctx, sizeof(*dispsock)); - disp->nsockets++; + disp->nsockets++; - *dispsock = (dispsocket_t){ .disp = disp }; - ISC_LINK_INIT(dispsock, link); - dispsock->magic = DISPSOCK_MAGIC; - } + resp->local = disp->local; + resp->peer = *dest; - dispsock->local = disp->local; - dispsock->peer = *dest; - - /* Pick a random UDP port */ port = ports[isc_random_uniform(nports)]; - isc_sockaddr_setport(&dispsock->local, port); - dispsock->port = port; + isc_sockaddr_setport(&resp->local, port); + resp->port = port; - *dispsockp = dispsock; *portp = port; return (ISC_R_SUCCESS); } /*% - * Destroy a dedicated dispatch socket. + * Deactivate the socket for a dispatch entry. * The dispatch must be locked. */ static void -destroy_dispsocket(dns_dispatch_t *disp, dispsocket_t **dispsockp) { - dispsocket_t *dispsock = NULL; - - REQUIRE(dispsockp != NULL && *dispsockp != NULL); - - dispsock = *dispsockp; - *dispsockp = NULL; - - REQUIRE(!ISC_LINK_LINKED(dispsock, link)); - +deactivate_dispentry(dns_dispatch_t *disp, dns_dispentry_t *resp) { + if (ISC_LINK_LINKED(resp, alink)) { + ISC_LIST_UNLINK(disp->active, resp, alink); + } + if (resp->handle != NULL) { + isc_nm_cancelread(resp->handle); + isc_nmhandle_detach(&resp->handle); + } disp->nsockets--; - dispsock->magic = 0; - if (dispsock->handle != NULL) { - isc_nm_cancelread(dispsock->handle); - isc_nmhandle_detach(&dispsock->handle); - } - - isc_mem_put(disp->mgr->mctx, dispsock, sizeof(*dispsock)); -} - -/*% - * Deactivate a dedicated dispatch socket. Move it to the inactive list for - * future reuse unless the total number of sockets are exceeding the maximum. - * The dispatch must be locked. - */ -static void -deactivate_dispsocket(dns_dispatch_t *disp, dispsocket_t *dispsock) { - ISC_LIST_UNLINK(disp->activesockets, dispsock, link); - if (dispsock->resp != NULL) { - INSIST(dispsock->resp->dispsocket == dispsock); - dispsock->resp->dispsocket = NULL; - dispsock->resp = NULL; - } - - if (disp->nsockets > DNS_DISPATCH_POOLSOCKS) { - destroy_dispsocket(disp, &dispsock); - } else { - if (dispsock->handle != NULL) { - isc_nm_cancelread(dispsock->handle); - isc_nmhandle_detach(&dispsock->handle); - } - - ISC_LIST_APPEND(disp->inactivesockets, dispsock, link); - } } /* @@ -671,7 +455,8 @@ allocate_udp_buffer(dns_dispatch_t *disp) { static inline void free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev) { - if (disp->failsafe_ev == ev) { + /* failsafe_ev is not allocated */ + if (&(disp->failsafe_ev) == ev) { INSIST(disp->shutdown_out == 1); disp->shutdown_out = 0; @@ -684,8 +469,9 @@ free_devent(dns_dispatch_t *disp, dns_dispatchevent_t *ev) { ev->region.length = 0; } - isc_refcount_decrement(&disp->mgr->irefs); isc_mem_put(disp->mgr->mctx, ev, sizeof(*ev)); + + dns_dispatch_detach(&disp); } static inline dns_dispatchevent_t * @@ -693,14 +479,95 @@ allocate_devent(dns_dispatch_t *disp) { dns_dispatchevent_t *ev = NULL; ev = isc_mem_get(disp->mgr->mctx, sizeof(*ev)); - isc_refcount_increment0(&disp->mgr->irefs); *ev = (dns_dispatchevent_t){ 0 }; + + dns_dispatch_attach(disp, &ev->dispatch); + ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, 0, NULL, NULL, NULL, NULL, NULL); return (ev); } +#define dispentry_attach(r, rp) \ + __dispentry_attach(r, rp, __func__, __FILE__, __LINE__) + +static void +__dispentry_attach(dns_dispentry_t *resp, dns_dispentry_t **respp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + + REQUIRE(VALID_RESPONSE(resp)); + REQUIRE(respp != NULL && *respp == NULL); + + ref = isc_refcount_increment(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, resp, respp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + + *respp = resp; +} + +static void +__dispentry_destroy(dns_dispentry_t *resp) { + dns_dispatch_t *disp = resp->disp; + + resp->magic = 0; + + if (resp->handle != NULL) { + isc_nmhandle_detach(&resp->handle); + } + + isc_refcount_destroy(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_detach(%p) -> %p\n", __FILE__, + __LINE__, __func__, &resp->task, resp->task); +#endif /* DISPATCH_TRACE */ + + isc_task_detach(&resp->task); + isc_mem_put(disp->mgr->mctx, resp, sizeof(*resp)); + + dns_dispatch_detach(&disp); +} + +#define dispentry_detach(rp) \ + __dispentry_detach(rp, __func__, __FILE__, __LINE__) + +static void +__dispentry_detach(dns_dispentry_t **respp, const char *func, const char *file, + unsigned int line) { + dns_dispentry_t *resp = NULL; + uint_fast32_t ref; + + REQUIRE(respp != NULL && VALID_RESPONSE(*respp)); + + resp = *respp; + *respp = NULL; + + ref = isc_refcount_decrement(&resp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, resp, respp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + if (ref == 1) { + __dispentry_destroy(resp); + } +} + /* * General flow: * @@ -718,60 +585,43 @@ allocate_devent(dns_dispatch_t *disp) { static void udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { - dispsocket_t *dispsock = (dispsocket_t *)arg; + dns_dispentry_t *resp = (dns_dispentry_t *)arg; dns_dispatch_t *disp = NULL; dns_messageid_t id; isc_result_t dres; isc_buffer_t source; unsigned int flags; - dns_dispentry_t *resp = NULL; dns_dispatchevent_t *rev = NULL; - bool killit; - bool queue_response; - dns_dispatchmgr_t *mgr = NULL; isc_sockaddr_t peer; isc_netaddr_t netaddr; int match; + isc_taskaction_t action = NULL; + bool nomore = true; - REQUIRE(VALID_DISPSOCK(dispsock)); + REQUIRE(VALID_RESPONSE(resp)); - disp = dispsock->disp; + disp = resp->disp; LOCK(&disp->lock); - mgr = disp->mgr; - - LOCK(&disp->mgr->buffer_lock); - dispatch_log(disp, LVL(90), "got packet: requests %d, recvs %d", - disp->requests, disp->recv_pending); - UNLOCK(&disp->mgr->buffer_lock); - - if (eresult == ISC_R_CANCELED || dispsock->resp == NULL) { - /* - * dispsock->resp can be NULL if this transaction was canceled - * just after receiving a response. So we can just move on. - */ - dispsock = NULL; + if (isc_log_wouldlog(dns_lctx, LVL(90))) { + LOCK(&disp->mgr->buffer_lock); + dispatch_log(disp, LVL(90), "got packet: requests %d, recvs %d", + disp->requests, disp->recv_pending); + UNLOCK(&disp->mgr->buffer_lock); } - if (disp->shutting_down == 1) { + if (eresult == ISC_R_CANCELED) { /* * This dispatcher is shutting down. */ - killit = destroy_disp_ok(disp); - UNLOCK(&disp->lock); - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); - } - - return; + goto unlock; } - if (dispsock == NULL) { - goto next; + if (!ISC_LINK_LINKED(resp, alink)) { + goto unlock; } - resp = dispsock->resp; id = resp->id; peer = isc_nmhandle_peeraddr(handle); @@ -780,7 +630,8 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, if (eresult == ISC_R_TIMEDOUT && resp->timedout != NULL) { resp->timedout(handle, ISC_R_TIMEDOUT, resp->arg); if (isc_nmhandle_timer_running(handle)) { - goto next; + nomore = false; + goto unlock; } } @@ -810,7 +661,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dispatch_log(disp, LVL(10), "blackholed packet from %s", netaddrstr); } - goto next; + goto unlock; } /* @@ -821,7 +672,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dres = dns_message_peekheader(&source, &id, &flags); if (dres != ISC_R_SUCCESS) { dispatch_log(disp, LVL(10), "got garbage packet"); - goto next; + goto unlock; } dispatch_log(disp, LVL(92), @@ -834,7 +685,7 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, */ if ((flags & DNS_MESSAGEFLAG_QR) == 0) { /* query */ - goto next; + goto unlock; } /* @@ -842,13 +693,12 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, */ if (resp->id != id || !isc_sockaddr_equal(&peer, &resp->peer)) { dispatch_log(disp, LVL(90), "response doesn't match"); - inc_stats(mgr, dns_resstatscounter_mismatch); - goto next; + inc_stats(disp->mgr, dns_resstatscounter_mismatch); + goto unlock; } sendevent: rev = allocate_devent(disp); - queue_response = resp->item_out; /* * At this point, rev contains the event we want to fill in, and @@ -856,7 +706,6 @@ sendevent: * Send the event off. */ rev->result = eresult; - resp->item_out = true; if (region != NULL) { rev->region.base = allocate_udp_buffer(disp); rev->region.length = DNS_DISPATCH_UDPBUFSIZE; @@ -866,23 +715,26 @@ sendevent: isc_buffer_add(&rev->buffer, rev->region.length); } - if (queue_response) { - ISC_LIST_APPEND(resp->items, rev, ev_link); - } else { - ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - request_log(disp, resp, LVL(90), - "[a] Sent event %p buffer %p len %d to task %p", - rev, rev->buffer.base, rev->buffer.length, - resp->task); - isc_task_send(resp->task, ISC_EVENT_PTR(&rev)); + ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, + resp->action, resp->arg, resp, NULL, NULL); + + request_log(disp, resp, LVL(90), + "[a] Sent event %p buffer %p len %d to task %p", rev, + rev->buffer.base, rev->buffer.length, resp->task); + + action = resp->action; + +unlock: + UNLOCK(&disp->lock); + + if (action != NULL) { + action(resp->task, (isc_event_t *)rev); } - UNLOCK(&disp->lock); + if (nomore) { + dispentry_detach(&resp); + } return; - -next: - UNLOCK(&disp->lock); } /* @@ -907,17 +759,15 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, dns_messageid_t id; isc_result_t dres; unsigned int flags; - dispsocket_t *dispsock = NULL; dns_dispentry_t *resp = NULL; dns_dispatchevent_t *rev = NULL; unsigned int bucket; - bool killit; - bool queue_response; dns_qid_t *qid = NULL; int level; char buf[ISC_SOCKADDR_FORMATSIZE]; isc_buffer_t source; isc_sockaddr_t peer; + isc_taskaction_t action = NULL; REQUIRE(VALID_DISPATCH(disp)); @@ -932,79 +782,48 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, INSIST(disp->recv_pending != 0); disp->recv_pending = 0; - if (isc_refcount_current(&disp->refcount) == 0) { - /* - * This dispatcher is shutting down. Force cancellation. - */ - eresult = ISC_R_CANCELED; - } - peer = isc_nmhandle_peeraddr(handle); - isc_nmhandle_detach(&handle); if (eresult != ISC_R_SUCCESS) { - disp->shutdown_why = eresult; - switch (eresult) { case ISC_R_CANCELED: dispatch_log(disp, LVL(90), "shutting down on cancel"); - do_cancel(disp); break; case ISC_R_EOF: dispatch_log(disp, LVL(90), "shutting down on EOF"); - do_cancel(disp); break; - case ISC_R_CONNECTIONRESET: - level = ISC_LOG_INFO; - goto logit; - case ISC_R_TIMEDOUT: /* * Time out the first active response for which * no event has already been sent. */ - for (dispsock = ISC_LIST_HEAD(disp->activesockets); - dispsock != NULL; - dispsock = ISC_LIST_NEXT(dispsock, link)) + for (resp = ISC_LIST_HEAD(disp->active); resp != NULL; + resp = ISC_LIST_NEXT(resp, alink)) { - resp = dispsock->resp; - if (resp->item_out) { - continue; - } - ISC_LIST_UNLINK(disp->activesockets, dispsock, - link); - ISC_LIST_APPEND(disp->activesockets, dispsock, - link); + ISC_LIST_UNLINK(disp->active, resp, alink); + ISC_LIST_APPEND(disp->active, resp, alink); goto sendevent; } - INSIST(0); - ISC_UNREACHABLE(); + break; default: - level = ISC_LOG_ERROR; - logit: + if (eresult == ISC_R_CONNECTIONRESET) { + level = ISC_LOG_INFO; + } else { + level = ISC_LOG_ERROR; + } + isc_sockaddr_format(&peer, buf, sizeof(buf)); dispatch_log(disp, level, "shutting down due to TCP " "receive error: %s: %s", buf, isc_result_totext(eresult)); - do_cancel(disp); break; } - disp->shutting_down = 1; - - /* - * If the recv() was canceled pass the word on. - */ - killit = destroy_disp_ok(disp); - UNLOCK(&disp->lock); - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); - } - return; + goto unlock; } dispatch_log(disp, LVL(90), "result %d, length == %d, addr = %p", @@ -1055,7 +874,6 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, } sendevent: - queue_response = resp->item_out; rev = allocate_devent(disp); /* @@ -1064,7 +882,6 @@ sendevent: * Send the event off. */ rev->result = eresult; - resp->item_out = true; if (region != NULL) { disp->tcpbuffers++; rev->region.base = isc_mem_get(disp->mgr->mctx, region->length); @@ -1075,84 +892,26 @@ sendevent: isc_buffer_add(&rev->buffer, rev->region.length); } - if (queue_response) { - ISC_LIST_APPEND(resp->items, rev, ev_link); - } else { - ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - request_log(disp, resp, LVL(90), - "[b] Sent event %p buffer %p len %d to task %p", - rev, rev->buffer.base, rev->buffer.length, - resp->task); - isc_task_send(resp->task, ISC_EVENT_PTR(&rev)); - } + ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, DNS_EVENT_DISPATCH, + resp->action, resp->arg, resp, NULL, NULL); + request_log(disp, resp, LVL(90), + "[b] Sent event %p buffer %p len %d to task %p", rev, + rev->buffer.base, rev->buffer.length, resp->task); + + action = resp->action; next: startrecv(disp, NULL); + +unlock: + isc_nmhandle_detach(&handle); UNLOCK(&disp->lock); -} -/* - * Mgr must be locked when calling this function. - */ -static bool -destroy_mgr_ok(dns_dispatchmgr_t *mgr) { - mgr_log(mgr, LVL(90), - "destroy_mgr_ok: shuttingdown=%d, listnonempty=%d, ", - MGR_IS_SHUTTINGDOWN(mgr), !ISC_LIST_EMPTY(mgr->list)); - if (!MGR_IS_SHUTTINGDOWN(mgr)) { - return (false); + dns_dispatch_detach(&disp); + + if (action != NULL) { + action(resp->task, (isc_event_t *)rev); } - if (!ISC_LIST_EMPTY(mgr->list)) { - return (false); - } - if (isc_refcount_current(&mgr->irefs) != 0) { - return (false); - } - - return (true); -} - -/* - * Mgr must be unlocked when calling this function. - */ -static void -destroy_mgr(dns_dispatchmgr_t **mgrp) { - dns_dispatchmgr_t *mgr = NULL; - - mgr = *mgrp; - *mgrp = NULL; - - mgr->magic = 0; - isc_mutex_destroy(&mgr->lock); - mgr->state = 0; - - if (mgr->qid != NULL) { - qid_destroy(mgr->mctx, &mgr->qid); - } - - isc_mutex_destroy(&mgr->buffer_lock); - - if (mgr->blackhole != NULL) { - dns_acl_detach(&mgr->blackhole); - } - - if (mgr->stats != NULL) { - isc_stats_detach(&mgr->stats); - } - - if (mgr->v4ports != NULL) { - isc_mem_put(mgr->mctx, mgr->v4ports, - mgr->nv4ports * sizeof(in_port_t)); - } - if (mgr->v6ports != NULL) { - isc_mem_put(mgr->mctx, mgr->v6ports, - mgr->nv6ports * sizeof(in_port_t)); - } - - isc_nm_detach(&mgr->nm); - - isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(dns_dispatchmgr_t)); } /*% @@ -1231,7 +990,7 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, mgr = isc_mem_get(mctx, sizeof(dns_dispatchmgr_t)); *mgr = (dns_dispatchmgr_t){ .magic = 0 }; - isc_refcount_init(&mgr->irefs, 0); + isc_refcount_init(&mgr->references, 1); isc_mem_attach(mctx, &mgr->mctx); isc_nm_attach(nm, &mgr->nm); @@ -1256,6 +1015,56 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, return (ISC_R_SUCCESS); } +void +dns__dispatchmgr_attach(dns_dispatchmgr_t *mgr, dns_dispatchmgr_t **mgrp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + + REQUIRE(VALID_DISPATCHMGR(mgr)); + REQUIRE(mgrp != NULL && *mgrp == NULL); + + ref = isc_refcount_increment(&mgr->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, mgr, mgrp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + + *mgrp = mgr; +} + +void +dns__dispatchmgr_detach(dns_dispatchmgr_t **mgrp, const char *func, + const char *file, unsigned int line) { + dns_dispatchmgr_t *mgr = NULL; + uint_fast32_t ref; + + REQUIRE(mgrp != NULL && VALID_DISPATCHMGR(*mgrp)); + + mgr = *mgrp; + *mgrp = NULL; + + ref = isc_refcount_decrement(&mgr->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, mgr, mgrp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + if (ref == 1) { + dispatchmgr_destroy(mgr); + } +} + void dns_dispatchmgr_setblackhole(dns_dispatchmgr_t *mgr, dns_acl_t *blackhole) { REQUIRE(VALID_DISPATCHMGR(mgr)); @@ -1278,27 +1087,40 @@ dns_dispatchmgr_setavailports(dns_dispatchmgr_t *mgr, isc_portset_t *v4portset, return (setavailports(mgr, v4portset, v6portset)); } -void -dns_dispatchmgr_destroy(dns_dispatchmgr_t **mgrp) { - dns_dispatchmgr_t *mgr = NULL; - bool killit; +static void +dispatchmgr_destroy(dns_dispatchmgr_t *mgr) { + REQUIRE(VALID_DISPATCHMGR(mgr)); - REQUIRE(mgrp != NULL); - REQUIRE(VALID_DISPATCHMGR(*mgrp)); + isc_refcount_destroy(&mgr->references); - mgr = *mgrp; - *mgrp = NULL; + mgr->magic = 0; + isc_mutex_destroy(&mgr->lock); + mgr->state = 0; - LOCK(&mgr->lock); - mgr->state |= MGR_SHUTTINGDOWN; - killit = destroy_mgr_ok(mgr); - UNLOCK(&mgr->lock); + qid_destroy(mgr->mctx, &mgr->qid); - mgr_log(mgr, LVL(90), "destroy: killit=%d", killit); + isc_mutex_destroy(&mgr->buffer_lock); - if (killit) { - destroy_mgr(&mgr); + if (mgr->blackhole != NULL) { + dns_acl_detach(&mgr->blackhole); } + + if (mgr->stats != NULL) { + isc_stats_detach(&mgr->stats); + } + + if (mgr->v4ports != NULL) { + isc_mem_put(mgr->mctx, mgr->v4ports, + mgr->nv4ports * sizeof(in_port_t)); + } + if (mgr->v6ports != NULL) { + isc_mem_put(mgr->mctx, mgr->v6ports, + mgr->nv6ports * sizeof(in_port_t)); + } + + isc_nm_detach(&mgr->nm); + + isc_mem_putanddetach(&mgr->mctx, mgr, sizeof(dns_dispatchmgr_t)); } void @@ -1366,15 +1188,12 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, isc_socktype_t type, int pf, */ disp = isc_mem_get(mgr->mctx, sizeof(*disp)); - isc_refcount_increment0(&mgr->irefs); + *disp = (dns_dispatch_t){ .socktype = type }; - *disp = (dns_dispatch_t){ .mgr = mgr, - .socktype = type, - .shutdown_why = ISC_R_UNEXPECTED }; - isc_refcount_init(&disp->refcount, 1); + dns_dispatchmgr_attach(mgr, &disp->mgr); + isc_refcount_init(&disp->references, 1); ISC_LINK_INIT(disp, link); - ISC_LIST_INIT(disp->activesockets); - ISC_LIST_INIT(disp->inactivesockets); + ISC_LIST_INIT(disp->active); switch (type) { case isc_socktype_tcp: @@ -1409,7 +1228,11 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, isc_socktype_t type, int pf, disp->attributes |= attributes; isc_mutex_init(&disp->lock); - disp->failsafe_ev = allocate_devent(disp); + + /* failsafe_ev is not allocated */ + ISC_EVENT_INIT(&disp->failsafe_ev, sizeof(disp->failsafe_ev), 0, NULL, + 0, NULL, NULL, NULL, NULL, NULL); + disp->magic = DISPATCH_MAGIC; *dispp = disp; @@ -1427,26 +1250,17 @@ dispatch_free(dns_dispatch_t **dispp) { disp = *dispp; *dispp = NULL; + disp->magic = 0; + mgr = disp->mgr; REQUIRE(VALID_DISPATCHMGR(mgr)); INSIST(disp->requests == 0); INSIST(disp->recv_pending == 0); - INSIST(ISC_LIST_EMPTY(disp->activesockets)); - INSIST(ISC_LIST_EMPTY(disp->inactivesockets)); + INSIST(ISC_LIST_EMPTY(disp->active)); - isc_refcount_decrement(&mgr->irefs); - - if (disp->failsafe_ev != NULL) { - isc_mem_put(mgr->mctx, disp->failsafe_ev, - sizeof(*disp->failsafe_ev)); - disp->failsafe_ev = NULL; - } - - disp->mgr = NULL; isc_mutex_destroy(&disp->lock); - disp->magic = 0; - isc_refcount_decrement(&mgr->irefs); + isc_mem_put(mgr->mctx, disp, sizeof(*disp)); } @@ -1479,14 +1293,16 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, } result = isc_task_create(taskmgr, 50, &disp->task); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_create() -> %p\n", __FILE__, + __LINE__, __func__, disp->task); +#endif /* DISPATCH_TRACE */ + if (result != ISC_R_SUCCESS) { goto cleanup; } - disp->ctlevent = - isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL, - destroy_disp, disp, sizeof(isc_event_t)); - isc_task_setname(disp->task, "tcpdispatch", disp); /* @@ -1520,10 +1336,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, const isc_sockaddr_t *localaddr, bool *connected, dns_dispatch_t **dispp) { dns_dispatch_t *disp = NULL; - isc_sockaddr_t peeraddr; - isc_sockaddr_t sockname; unsigned int attributes, mask; - bool match = false; REQUIRE(VALID_DISPATCHMGR(mgr)); REQUIRE(destaddr != NULL); @@ -1535,24 +1348,32 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, DNS_DISPATCHATTR_CONNECTED; LOCK(&mgr->lock); +again: disp = ISC_LIST_HEAD(mgr->list); - while (disp != NULL && !match) { + while (disp != NULL && *dispp == NULL) { + isc_sockaddr_t sockname; + isc_sockaddr_t peeraddr; + LOCK(&disp->lock); - if ((disp->shutting_down == 0) && - ATTRMATCH(disp->attributes, attributes, mask) && + + if (disp->handle != NULL) { + sockname = isc_nmhandle_localaddr(disp->handle); + peeraddr = isc_nmhandle_peeraddr(disp->handle); + } else { + sockname = disp->local; + peeraddr = disp->peer; + } + + if (ATTRMATCH(disp->attributes, attributes, mask) && (localaddr == NULL || isc_sockaddr_eqaddr(localaddr, &disp->local))) { - sockname = isc_nmhandle_localaddr(disp->handle); - peeraddr = isc_nmhandle_peeraddr(disp->handle); if (isc_sockaddr_equal(destaddr, &peeraddr) && (localaddr == NULL || isc_sockaddr_eqaddr(localaddr, &sockname))) { /* attach */ - isc_refcount_increment(&disp->refcount); - *dispp = disp; - match = true; + dns_dispatch_attach(disp, dispp); if (connected != NULL) { *connected = true; } @@ -1561,34 +1382,21 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, UNLOCK(&disp->lock); disp = ISC_LIST_NEXT(disp, link); } - if (match || connected == NULL) { + + if (*dispp != NULL) { UNLOCK(&mgr->lock); - return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND); + return (ISC_R_SUCCESS); } - /* Second pass, only if connected != NULL */ - attributes = DNS_DISPATCHATTR_TCP; - - disp = ISC_LIST_HEAD(mgr->list); - while (disp != NULL && !match) { - LOCK(&disp->lock); - if ((disp->shutting_down == 0) && - ATTRMATCH(disp->attributes, attributes, mask) && - (localaddr == NULL || - isc_sockaddr_eqaddr(localaddr, &disp->local)) && - isc_sockaddr_equal(destaddr, &disp->peer)) - { - /* attach */ - isc_refcount_increment(&disp->refcount); - *dispp = disp; - match = true; - } - UNLOCK(&disp->lock); - disp = ISC_LIST_NEXT(disp, link); + if (connected != NULL) { + /* Second pass, only if connected != NULL */ + attributes = DNS_DISPATCHATTR_TCP; + connected = NULL; + goto again; } + UNLOCK(&mgr->lock); - - return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND); + return (ISC_R_NOTFOUND); } isc_result_t @@ -1648,18 +1456,16 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, disp->local = *localaddr; result = isc_task_create(taskmgr, 0, &disp->task); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_create() -> %p\n", __FILE__, + __LINE__, __func__, disp->task); +#endif /* DISPATCH_TRACE */ + if (result != ISC_R_SUCCESS) { goto cleanup; } - disp->ctlevent = - isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL, - destroy_disp, disp, sizeof(isc_event_t)); - - disp->sepool = NULL; - isc_mem_create(&disp->sepool); - isc_mem_setname(disp->sepool, "disp_sepool"); - /* * Append it to the dispatcher list. */ @@ -1681,53 +1487,90 @@ cleanup: return (result); } +static void +dns_dispatch_destroy(dns_dispatch_t *disp) { + dns_dispatchmgr_t *mgr = disp->mgr; + + LOCK(&mgr->lock); + ISC_LIST_UNLINK(mgr->list, disp, link); + UNLOCK(&mgr->lock); + + dispatch_log(disp, LVL(90), "shutting down; detaching from handle %p", + disp->handle); + + if (disp->handle != NULL) { + isc_nmhandle_detach(&disp->handle); + } + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_detach(%p) -> %p\n", __FILE__, + __LINE__, __func__, &disp->task, disp->task); +#endif /* DISPATCH_TRACE */ + + isc_task_detach(&disp->task); + + dispatch_free(&disp); + + /* Because dispatch uses mgr->mctx, we must detach after freeing + * dispatch, not before + */ + dns_dispatchmgr_detach(&mgr); +} + void -dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp) { +dns__dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp, + const char *func, const char *file, unsigned int line) { + uint_fast32_t ref; + REQUIRE(VALID_DISPATCH(disp)); REQUIRE(dispp != NULL && *dispp == NULL); - isc_refcount_increment(&disp->refcount); + ref = isc_refcount_increment(&disp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, disp, dispp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* DISPATCH_TRACE */ + *dispp = disp; } void -dns_dispatch_detach(dns_dispatch_t **dispp) { +dns__dispatch_detach(dns_dispatch_t **dispp, const char *func, const char *file, + unsigned int line) { dns_dispatch_t *disp = NULL; - dispsocket_t *dispsock = NULL; - bool killit = false; + uint_fast32_t ref; REQUIRE(dispp != NULL && VALID_DISPATCH(*dispp)); disp = *dispp; *dispp = NULL; - if (isc_refcount_decrement(&disp->refcount) == 1) { + ref = isc_refcount_decrement(&disp->references); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, disp, dispp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* DISPATCH_TRACE */ + + dispatch_log(disp, LVL(90), "detach: refcount %" PRIuFAST32, ref - 1); + + if (ref == 1) { LOCK(&disp->lock); - if (disp->recv_pending != 0 && disp->handle != NULL) { - isc_nm_cancelread(disp->handle); - } - if (disp->handle != NULL) { - isc_nmhandle_detach(&disp->handle); - } - for (dispsock = ISC_LIST_HEAD(disp->activesockets); - dispsock != NULL; dispsock = ISC_LIST_NEXT(dispsock, link)) - { - if (dispsock->handle != NULL) { - isc_nmhandle_detach(&dispsock->handle); - } - } - disp->shutting_down = 1; - do_cancel(disp); - - killit = destroy_disp_ok(disp); + REQUIRE(disp->recv_pending == 0); + REQUIRE(ISC_LIST_EMPTY(disp->active)); UNLOCK(&disp->lock); - } - dispatch_log(disp, LVL(90), "detach: refcount %" PRIuFAST32, - isc_refcount_current(&disp->refcount)); - - if (killit) { - isc_task_send(disp->task, &disp->ctlevent); + dns_dispatch_destroy(disp); } } @@ -1738,15 +1581,16 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, isc_nm_cb_t sent, isc_taskaction_t action, isc_nm_cb_t timedout, void *arg, dns_messageid_t *idp, dns_dispentry_t **resp) { - isc_result_t result; dns_dispentry_t *res = NULL; - dispsocket_t *dispsocket = NULL; dns_qid_t *qid = NULL; in_port_t localport = 0; dns_messageid_t id; unsigned int bucket; bool ok = false; int i = 0; + isc_taskaction_t oldest_action = NULL; + isc_task_t *oldest_task = NULL; + isc_event_t *oldest_ev = NULL; REQUIRE(VALID_DISPATCH(disp)); REQUIRE(task != NULL); @@ -1758,11 +1602,6 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, LOCK(&disp->lock); - if (disp->shutting_down == 1) { - UNLOCK(&disp->lock); - return (ISC_R_SHUTTINGDOWN); - } - if (disp->requests >= DNS_DISPATCH_MAXREQUESTS) { UNLOCK(&disp->lock); return (ISC_R_QUOTA); @@ -1773,42 +1612,51 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, if (disp->socktype == isc_socktype_udp && disp->nsockets > DNS_DISPATCH_SOCKSQUOTA) { - dispsocket_t *oldestsocket = NULL; - dns_dispentry_t *oldestresp = NULL; + dns_dispentry_t *oldest = NULL; dns_dispatchevent_t *rev = NULL; /* * Kill oldest outstanding query if the number of sockets * exceeds the quota to keep the room for new queries. */ - oldestsocket = ISC_LIST_HEAD(disp->activesockets); - oldestresp = oldestsocket->resp; - if (oldestresp != NULL && !oldestresp->item_out) { - rev = allocate_devent(oldestresp->disp); + oldest = ISC_LIST_HEAD(disp->active); + if (oldest != NULL) { + rev = allocate_devent(oldest->disp); rev->buffer.base = NULL; rev->result = ISC_R_CANCELED; ISC_EVENT_INIT(rev, sizeof(*rev), 0, NULL, - DNS_EVENT_DISPATCH, oldestresp->action, - oldestresp->arg, oldestresp, NULL, NULL); - oldestresp->item_out = true; - isc_task_send(oldestresp->task, ISC_EVENT_PTR(&rev)); + DNS_EVENT_DISPATCH, oldest->action, + oldest->arg, oldest, NULL, NULL); + + oldest_action = oldest->action; + oldest_task = oldest->task; + oldest_ev = (isc_event_t *)rev; + inc_stats(disp->mgr, dns_resstatscounter_dispabort); } - - /* - * Move this entry to the tail so that it won't (easily) be - * examined before actually being canceled. - */ - ISC_LIST_UNLINK(disp->activesockets, oldestsocket, link); - ISC_LIST_APPEND(disp->activesockets, oldestsocket, link); } + res = isc_mem_get(disp->mgr->mctx, sizeof(*res)); + + *res = (dns_dispentry_t){ .port = localport, + .timeout = timeout, + .peer = *dest, + .connected = connected, + .sent = sent, + .timedout = timedout, + .action = action, + .arg = arg }; + + isc_refcount_init(&res->references, 1); + + ISC_LIST_INIT(res->items); + ISC_LINK_INIT(res, link); + ISC_LINK_INIT(res, alink); + if (disp->socktype == isc_socktype_udp) { - /* - * Get a separate UDP socket with a random port number. - */ - result = get_dispsocket(disp, dest, &dispsocket, &localport); + isc_result_t result = setup_socket(disp, res, dest, &localport); if (result != ISC_R_SUCCESS) { + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); UNLOCK(&disp->lock); inc_stats(disp->mgr, dns_resstatscounter_dispsockfail); return (result); @@ -1841,38 +1689,26 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, UNLOCK(&qid->lock); if (!ok) { + isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); UNLOCK(&disp->lock); return (ISC_R_NOMORE); } - res = isc_mem_get(disp->mgr->mctx, sizeof(*res)); - isc_refcount_increment0(&disp->mgr->irefs); - - *res = (dns_dispentry_t){ .id = id, - .port = localport, - .bucket = bucket, - .timeout = timeout, - .peer = *dest, - .connected = connected, - .sent = sent, - .timedout = timedout, - .action = action, - .arg = arg, - .dispsocket = dispsocket }; - dns_dispatch_attach(disp, &res->disp); + +#ifdef DISPATCH_TRACE + fprintf(stderr, "%s:%d:%s:isc_task_attach(%p, %p)\n", __FILE__, + __LINE__, __func__, task, &res->task); +#endif /* DISPATCH_TRACE */ + isc_task_attach(task, &res->task); - ISC_LIST_INIT(res->items); - ISC_LINK_INIT(res, link); + res->id = id; + res->bucket = bucket; res->magic = RESPONSE_MAGIC; disp->requests++; - if (dispsocket != NULL) { - dispsocket->resp = res; - } - LOCK(&qid->lock); ISC_LIST_APPEND(qid->qid_table[bucket], res, link); UNLOCK(&qid->lock); @@ -1883,13 +1719,13 @@ dns_dispatch_addresponse(dns_dispatch_t *disp, unsigned int options, ? dns_resstatscounter_disprequdp : dns_resstatscounter_dispreqtcp); - if (dispsocket != NULL) { - ISC_LIST_APPEND(disp->activesockets, dispsocket, link); - } + ISC_LIST_APPEND(disp->active, res, alink); UNLOCK(&disp->lock); - INSIST(disp->socktype == isc_socktype_tcp || res->dispsocket != NULL); + if (oldest_action != NULL) { + oldest_action(oldest_task, oldest_ev); + } *idp = id; *resp = res; @@ -1901,6 +1737,7 @@ isc_result_t dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { dns_dispatch_t *disp = NULL; dns_dispatchevent_t *ev = NULL; + isc_taskaction_t action = NULL; REQUIRE(VALID_RESPONSE(resp)); REQUIRE(sockevent != NULL && *sockevent != NULL); @@ -1913,16 +1750,8 @@ dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { LOCK(&disp->lock); - REQUIRE(resp->item_out); - resp->item_out = false; - free_devent(disp, ev); - if (disp->shutting_down == 1) { - UNLOCK(&disp->lock); - return (ISC_R_SHUTTINGDOWN); - } - ev = ISC_LIST_HEAD(resp->items); if (ev != NULL) { ISC_LIST_UNLINK(resp->items, ev, ev_link); @@ -1931,40 +1760,44 @@ dns_dispatch_getnext(dns_dispentry_t *resp, dns_dispatchevent_t **sockevent) { request_log(disp, resp, LVL(90), "[c] Sent event %p buffer %p len %d to task %p", ev, ev->buffer.base, ev->buffer.length, resp->task); - resp->item_out = true; - isc_task_send(resp->task, ISC_EVENT_PTR(&ev)); + + action = resp->action; } - startrecv(disp, resp->dispsocket); + startrecv(disp, resp); UNLOCK(&disp->lock); + if (action != NULL) { + action(resp->task, (isc_event_t *)ev); + } + return (ISC_R_SUCCESS); } void -dns_dispatch_removeresponse(dns_dispentry_t **resp, +dns_dispatch_removeresponse(dns_dispentry_t **respp, dns_dispatchevent_t **sockevent) { dns_dispatchmgr_t *mgr = NULL; dns_dispatch_t *disp = NULL; - dns_dispentry_t *res = NULL; + dns_dispentry_t *resp = NULL; dns_dispatchevent_t *ev = NULL; - unsigned int bucket; - isc_eventlist_t events; dns_qid_t *qid = NULL; - REQUIRE(resp != NULL); - REQUIRE(VALID_RESPONSE(*resp)); + REQUIRE(respp != NULL); - res = *resp; - *resp = NULL; + resp = *respp; + + REQUIRE(VALID_RESPONSE(resp)); + + disp = resp->disp; - disp = res->disp; - res->disp = NULL; REQUIRE(VALID_DISPATCH(disp)); mgr = disp->mgr; + REQUIRE(VALID_DISPATCHMGR(mgr)); + qid = mgr->qid; if (sockevent != NULL) { @@ -1978,95 +1811,66 @@ dns_dispatch_removeresponse(dns_dispentry_t **resp, LOCK(&disp->lock); INSIST(disp->requests > 0); disp->requests--; + dec_stats(disp->mgr, (disp->socktype == isc_socktype_udp) ? dns_resstatscounter_disprequdp : dns_resstatscounter_dispreqtcp); - if (res->dispsocket != NULL) { - deactivate_dispsocket(disp, res->dispsocket); - } - UNLOCK(&disp->lock); - - bucket = res->bucket; + deactivate_dispentry(disp, resp); LOCK(&qid->lock); - ISC_LIST_UNLINK(qid->qid_table[bucket], res, link); + ISC_LIST_UNLINK(qid->qid_table[resp->bucket], resp, link); UNLOCK(&qid->lock); - - if (ev == NULL && res->item_out) { - unsigned int n; - - /* - * We've posted our event, but the caller hasn't gotten it - * yet. Take it back. - */ - ISC_LIST_INIT(events); - n = isc_task_unsend(res->task, res, DNS_EVENT_DISPATCH, NULL, - &events); - /* - * We had better have gotten it back. - */ - INSIST(n == 1); - ev = (dns_dispatchevent_t *)ISC_LIST_HEAD(events); - } + UNLOCK(&disp->lock); if (ev != NULL) { - REQUIRE(res->item_out); - res->item_out = false; free_devent(disp, ev); } - request_log(disp, res, LVL(90), "detaching from task %p", res->task); - isc_task_detach(&res->task); - /* * Free any buffered responses as well */ - ev = ISC_LIST_HEAD(res->items); + ev = ISC_LIST_HEAD(resp->items); while (ev != NULL) { - ISC_LIST_UNLINK(res->items, ev, ev_link); + ISC_LIST_UNLINK(resp->items, ev, ev_link); free_devent(disp, ev); - ev = ISC_LIST_HEAD(res->items); + ev = ISC_LIST_HEAD(resp->items); } - res->magic = 0; - isc_refcount_decrement(&disp->mgr->irefs); - isc_mem_put(disp->mgr->mctx, res, sizeof(*res)); - dns_dispatch_detach(&disp); + dispentry_detach(respp); } /* * disp must be locked. */ static void -startrecv(dns_dispatch_t *disp, dispsocket_t *dispsocket) { +startrecv(dns_dispatch_t *disp, dns_dispentry_t *resp) { isc_nmhandle_t *handle = NULL; - - if (disp->shutting_down == 1) { - return; - } - - if (dispsocket == NULL) { - if (disp->socktype == isc_socktype_udp || - disp->recv_pending != 0) { - return; - } - isc_nmhandle_attach(disp->handle, &handle); - } else { - handle = dispsocket->handle; - } + dns_dispentry_t *tmp_resp = NULL; + dns_dispatch_t *tmp_disp = NULL; switch (disp->socktype) { case isc_socktype_udp: - isc_nm_read(handle, udp_recv, dispsocket); - break; + REQUIRE(resp != NULL); + REQUIRE(resp->handle != NULL); + dispentry_attach(resp, &tmp_resp); + + /* resp->handle is detached in _removeresponse() */ + isc_nm_read(resp->handle, udp_recv, resp); + + break; case isc_socktype_tcp: + if (resp != NULL) { + REQUIRE(resp->handle == NULL); + } + + isc_nmhandle_attach(disp->handle, &handle); + dns_dispatch_attach(disp, &tmp_disp); isc_nm_read(handle, tcp_recv, disp); INSIST(disp->recv_pending == 0); disp->recv_pending = 1; break; - default: INSIST(0); ISC_UNREACHABLE(); @@ -2077,36 +1881,64 @@ static void disp_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { dns_dispentry_t *resp = (dns_dispentry_t *)arg; dns_dispatch_t *disp = resp->disp; - dispsocket_t *dispsocket = NULL; + if (resp->canceled && eresult == ISC_R_SUCCESS) { + eresult = ISC_R_CANCELED; + goto detach; + } + + if (MGR_IS_SHUTTINGDOWN(disp->mgr)) { + eresult = ISC_R_SHUTTINGDOWN; + } + + /* + * Start listening on success + */ if (eresult == ISC_R_SUCCESS) { - if (disp->socktype == isc_socktype_udp) { - dispsocket = resp->dispsocket; - isc_nmhandle_attach(handle, &dispsocket->handle); - } else if (disp->handle == NULL) { - disp->attributes |= DNS_DISPATCHATTR_CONNECTED; - isc_nmhandle_attach(handle, &disp->handle); + switch (disp->socktype) { + case isc_socktype_udp: + isc_nmhandle_attach(handle, &resp->handle); + startrecv(disp, resp); + break; + case isc_socktype_tcp: + if (disp->handle == NULL) { + LOCK(&disp->lock); + isc_nmhandle_attach(handle, &disp->handle); + disp->attributes |= DNS_DISPATCHATTR_CONNECTED; + UNLOCK(&disp->lock); + startrecv(disp, resp); + } + break; + default: + INSIST(0); + ISC_UNREACHABLE(); } - - startrecv(disp, dispsocket); } if (resp->connected != NULL) { resp->connected(handle, eresult, resp->arg); } + +detach: + dispentry_detach(&resp); } isc_result_t dns_dispatch_connect(dns_dispentry_t *resp) { dns_dispatch_t *disp = NULL; + dns_dispentry_t *tmp = NULL; REQUIRE(VALID_RESPONSE(resp)); disp = resp->disp; + dispentry_attach(resp, &tmp); /* detached in disp_connected */ + switch (disp->socktype) { case isc_socktype_tcp: + INSIST(disp->handle == NULL); if (disp->handle != NULL) { + /* We are already connected */ break; } @@ -2114,9 +1946,8 @@ dns_dispatch_connect(dns_dispentry_t *resp) { disp_connected, resp, resp->timeout, 0); break; case isc_socktype_udp: - isc_nm_udpconnect(disp->mgr->nm, &resp->dispsocket->local, - &resp->dispsocket->peer, disp_connected, resp, - resp->timeout, 0); + isc_nm_udpconnect(disp->mgr->nm, &resp->local, &resp->peer, + disp_connected, resp, resp->timeout, 0); break; default: return (ISC_R_NOTIMPLEMENTED); @@ -2125,9 +1956,25 @@ dns_dispatch_connect(dns_dispentry_t *resp) { return (ISC_R_SUCCESS); } +static void +send_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { + dns_dispentry_t *resp = (dns_dispentry_t *)cbarg; + + REQUIRE(VALID_RESPONSE(resp)); + + resp->sent(handle, result, resp->arg); + + if (result != ISC_R_SUCCESS) { + isc_nm_cancelread(handle); + } + + dispentry_detach(&resp); +} + void dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) { isc_nmhandle_t *handle = NULL; + dns_dispentry_t *tmp = NULL; REQUIRE(VALID_RESPONSE(resp)); @@ -2149,108 +1996,38 @@ dns_dispatch_send(dns_dispentry_t *resp, isc_region_t *r, isc_dscp_t dscp) { } #endif - isc_nm_send(handle, r, resp->sent, resp->arg); + dispentry_attach(resp, &tmp); /* detached in send_done() */ + isc_nm_send(handle, r, send_done, resp); } void -dns_dispatch_cancel(dns_dispatch_t *disp, dns_dispentry_t *resp, bool sending, - bool connecting) { +dns_dispatch_cancel(dns_dispentry_t *resp) { + REQUIRE(VALID_RESPONSE(resp)); + + resp->canceled = true; + + if (resp->handle) { + isc_nm_cancelread(resp->handle); + } else if (resp->disp->handle != NULL) { + isc_nm_cancelread(resp->disp->handle); + } +} + +static inline isc_nmhandle_t * +getentryhandle(dns_dispentry_t *resp) { isc_nmhandle_t *handle = NULL; - REQUIRE(disp != NULL || resp != NULL); - - if (resp != NULL) { - REQUIRE(VALID_RESPONSE(resp)); - handle = getentryhandle(resp); - } else if (disp != NULL) { - REQUIRE(VALID_DISPATCH(disp)); - handle = gethandle(disp); - } else { - INSIST(0); - ISC_UNREACHABLE(); - } - - if (handle == NULL) { - return; - } - - if (connecting) { - // isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_CONNECT); - } - - if (sending) { - // isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND); - } -} - -/* - * disp must be locked. - */ -static void -do_cancel(dns_dispatch_t *disp) { - dns_dispatchevent_t *ev = NULL; - dns_dispentry_t *resp = NULL; - dns_qid_t *qid = disp->mgr->qid; - - if (disp->shutdown_out == 1) { - return; - } - - /* - * Search for the first response handler without packets outstanding - * unless a specific handler is given. - */ - LOCK(&qid->lock); - for (resp = linear_first(qid); resp != NULL && resp->item_out; - /* Empty. */) - { - resp = linear_next(qid, resp); - } - - /* - * No one to send the cancel event to, so nothing to do. - */ - if (resp == NULL) { - goto unlock; - } - - /* - * Send the shutdown failsafe event to this resp. - */ - ev = disp->failsafe_ev; - disp->failsafe_ev = NULL; - ISC_EVENT_INIT(ev, sizeof(*ev), 0, NULL, DNS_EVENT_DISPATCH, - resp->action, resp->arg, resp, NULL, NULL); - ev->result = disp->shutdown_why; - ev->buffer.base = NULL; - ev->buffer.length = 0; - disp->shutdown_out = 1; - request_log(disp, resp, LVL(10), "cancel: failsafe event %p -> task %p", - ev, resp->task); - resp->item_out = true; - isc_task_send(resp->task, ISC_EVENT_PTR(&ev)); -unlock: - UNLOCK(&qid->lock); -} - -static isc_nmhandle_t * -gethandle(dns_dispatch_t *disp) { - REQUIRE(VALID_DISPATCH(disp)); - - return (disp->handle); -} - -static isc_nmhandle_t * -getentryhandle(dns_dispentry_t *resp) { REQUIRE(VALID_RESPONSE(resp)); if (resp->disp->socktype == isc_socktype_tcp) { - return (resp->disp->handle); - } else if (resp->dispsocket != NULL) { - return (resp->dispsocket->handle); + handle = resp->disp->handle; } else { - return (NULL); + handle = resp->handle; } + + INSIST(handle != NULL); + + return (handle); } isc_result_t @@ -2275,8 +2052,8 @@ dns_dispentry_getlocaladdress(dns_dispentry_t *resp, isc_sockaddr_t *addrp) { return (ISC_R_SUCCESS); } - if (resp->dispsocket != NULL && resp->dispsocket->handle != NULL) { - *addrp = isc_nmhandle_localaddr(resp->dispsocket->handle); + if (resp->handle != NULL) { + *addrp = isc_nmhandle_localaddr(resp->handle); return (ISC_R_SUCCESS); } @@ -2288,8 +2065,8 @@ dns_dispatch_getattributes(dns_dispatch_t *disp) { REQUIRE(VALID_DISPATCH(disp)); /* - * We don't bother locking disp here; it's the caller's responsibility - * to use only non volatile flags. + * We don't bother locking disp here; it's the caller's + * responsibility to use only non volatile flags. */ return (disp->attributes); } diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index 762a979ad8..01054a1dc0 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -79,9 +79,10 @@ ISC_LANG_BEGINDECLS struct dns_dispatchevent { ISC_EVENT_COMMON(dns_dispatchevent_t); /*%< standard event common */ - isc_result_t result; /*%< result code */ - isc_region_t region; /*%< data region */ - isc_buffer_t buffer; /*%< data buffer */ + isc_result_t result; /*%< result code */ + isc_region_t region; /*%< data region */ + isc_buffer_t buffer; /*%< data buffer */ + dns_dispatch_t *dispatch; }; /*% @@ -146,8 +147,19 @@ dns_dispatchmgr_create(isc_mem_t *mctx, isc_nm_t *nm, dns_dispatchmgr_t **mgrp); *\li anything else -- failure */ +#define dns_dispatchmgr_attach(mgr, mgrp) \ + dns__dispatchmgr_attach(mgr, mgrp, __func__, __FILE__, __LINE__) + void -dns_dispatchmgr_destroy(dns_dispatchmgr_t **mgrp); +dns__dispatchmgr_attach(dns_dispatchmgr_t *mgr, dns_dispatchmgr_t **mgrp, + const char *func, const char *file, unsigned int line); + +#define dns_dispatchmgr_detach(mgrp) \ + dns__dispatchmgr_detach(mgrp, __func__, __FILE__, __LINE__) + +void +dns__dispatchmgr_detach(dns_dispatchmgr_t **mgrp, const char *func, + const char *file, unsigned int line); /*%< * Destroys the dispatchmgr when it becomes empty. This could be * immediately. @@ -246,8 +258,12 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_taskmgr_t *taskmgr, *\li Anything else -- failure. */ +#define dns_dispatch_attach(d, dp) \ + dns__dispatch_attach(d, dp, __func__, __FILE__, __LINE__) + void -dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp); +dns__dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp, + const char *func, const char *file, unsigned int line); /*%< * Attach to a dispatch handle. * @@ -257,8 +273,12 @@ dns_dispatch_attach(dns_dispatch_t *disp, dns_dispatch_t **dispp); *\li dispp != NULL && *dispp == NULL */ +#define dns_dispatch_detach(dp) \ + dns__dispatch_detach(dp, __func__, __FILE__, __LINE__) + void -dns_dispatch_detach(dns_dispatch_t **dispp); +dns__dispatch_detach(dns_dispatch_t **dispp, const char *func, const char *file, + unsigned int line); /*%< * Detaches from the dispatch. * @@ -277,15 +297,13 @@ dns_dispatch_connect(dns_dispentry_t *resp); */ void -dns_dispatch_cancel(dns_dispatch_t *disp, dns_dispentry_t *resp, bool sending, - bool connecting); +dns_dispatch_cancel(dns_dispentry_t *resp); /*%< - * Cancel pending sends (if 'sending' is true) and connects (if - * 'connecting' is true) in 'resp' or 'disp'. + * Cancel pending connects in 'resp', by setting a flag so that + * a read is not started when the connect handler runs. * * Requires: - *\li 'resp' is NULL and 'disp' is valid, or - *\li 'disp' is NULL and 'resp' is valid. + *\li 'resp' is valid. */ void diff --git a/lib/dns/include/dns/request.h b/lib/dns/include/dns/request.h index 9554603b50..fd8e08cebe 100644 --- a/lib/dns/include/dns/request.h +++ b/lib/dns/include/dns/request.h @@ -124,8 +124,12 @@ dns_requestmgr_shutdown(dns_requestmgr_t *requestmgr); *\li 'requestmgr' is a valid requestmgr. */ +#define dns_requestmgr_attach(source, targetp) \ + dns__requestmgr_attach(source, targetp, __FILE__, __LINE__, __func__) + void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp); +dns__requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp, + const char *file, unsigned int line, const char *func); /*%< * Attach to the request manager. dns_requestmgr_shutdown() must not * have been called on 'source' prior to calling dns_requestmgr_attach(). @@ -137,8 +141,12 @@ dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp); *\li 'targetp' to be non NULL and '*targetp' to be NULL. */ +#define dns_requestmgr_detach(requestmgrp) \ + dns__requestmgr_detach(requestmgrp, __FILE__, __LINE__, __func__) + void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp); +dns__requestmgr_detach(dns_requestmgr_t **requestmgrp, const char *file, + unsigned int line, const char *func); /*%< * Detach from the given requestmgr. If this is the final detach * requestmgr will be destroyed. dns_requestmgr_shutdown() must diff --git a/lib/dns/request.c b/lib/dns/request.c index fcb378ed6a..7d2b938fa3 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -32,6 +32,8 @@ #include #include +#define REQ_TRACE + #define REQUESTMGR_MAGIC ISC_MAGIC('R', 'q', 'u', 'M') #define VALID_REQUESTMGR(mgr) ISC_MAGIC_VALID(mgr, REQUESTMGR_MAGIC) @@ -44,17 +46,17 @@ typedef ISC_LIST(dns_request_t) dns_requestlist_t; struct dns_requestmgr { unsigned int magic; + isc_refcount_t references; + isc_mutex_t lock; isc_mem_t *mctx; /* locked */ - int32_t eref; - int32_t iref; isc_taskmgr_t *taskmgr; dns_dispatchmgr_t *dispatchmgr; dns_dispatch_t *dispatchv4; dns_dispatch_t *dispatchv6; - bool exiting; + atomic_bool exiting; isc_eventlist_t whenshutdown; unsigned int hash; isc_mutex_t locks[DNS_REQUEST_NLOCKS]; @@ -63,6 +65,8 @@ struct dns_requestmgr { struct dns_request { unsigned int magic; + isc_refcount_t references; + unsigned int hash; isc_mem_t *mctx; int32_t flags; @@ -97,8 +101,6 @@ struct dns_request { static void mgr_destroy(dns_requestmgr_t *requestmgr); -static void -mgr_shutdown(dns_requestmgr_t *requestmgr); static unsigned int mgr_gethash(dns_requestmgr_t *requestmgr); static void @@ -118,12 +120,23 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg); static void req_timeout(isc_nmhandle_t *handle, isc_result_t eresult, void *arg); static void +__req_attach(dns_request_t *source, dns_request_t **targetp, const char *file, + unsigned int line, const char *func); +static void +__req_detach(dns_request_t **requestp, const char *file, unsigned int line, + const char *func); +static void req_destroy(dns_request_t *request); static void req_log(int level, const char *fmt, ...) ISC_FORMAT_PRINTF(2, 3); void request_cancel(dns_request_t *request); +#define req_attach(source, targetp) \ + __req_attach(source, targetp, __FILE__, __LINE__, __func__) +#define req_detach(requestp) \ + __req_detach(requestp, __FILE__, __LINE__, __func__) + /*** *** Public ***/ @@ -171,11 +184,10 @@ dns_requestmgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, } requestmgr->mctx = NULL; isc_mem_attach(mctx, &requestmgr->mctx); - requestmgr->eref = 1; /* implicit attach */ - requestmgr->iref = 0; + isc_refcount_init(&requestmgr->references, 1); ISC_LIST_INIT(requestmgr->whenshutdown); ISC_LIST_INIT(requestmgr->requests); - requestmgr->exiting = false; + atomic_init(&requestmgr->exiting, false); requestmgr->hash = 0; requestmgr->magic = REQUESTMGR_MAGIC; @@ -201,7 +213,7 @@ dns_requestmgr_whenshutdown(dns_requestmgr_t *requestmgr, isc_task_t *task, LOCK(&requestmgr->lock); - if (requestmgr->exiting) { + if (atomic_load_acquire(&requestmgr->exiting)) { /* * We're already shutdown. Send the event. */ @@ -218,126 +230,95 @@ dns_requestmgr_whenshutdown(dns_requestmgr_t *requestmgr, isc_task_t *task, void dns_requestmgr_shutdown(dns_requestmgr_t *requestmgr) { + dns_request_t *request; + REQUIRE(VALID_REQUESTMGR(requestmgr)); req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_shutdown: %p", requestmgr); - LOCK(&requestmgr->lock); - mgr_shutdown(requestmgr); - UNLOCK(&requestmgr->lock); -} - -static void -mgr_shutdown(dns_requestmgr_t *requestmgr) { - dns_request_t *request; - - if (!requestmgr->exiting) { - requestmgr->exiting = true; - for (request = ISC_LIST_HEAD(requestmgr->requests); - request != NULL; request = ISC_LIST_NEXT(request, link)) - { - dns_request_cancel(request); - } - if (requestmgr->iref == 0) { - INSIST(ISC_LIST_EMPTY(requestmgr->requests)); - send_shutdown_events(requestmgr); - } + if (!atomic_compare_exchange_strong(&requestmgr->exiting, + &(bool){ false }, true)) + { + return; } -} - -static void -requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp) { - /* - * Locked by caller. - */ - - REQUIRE(VALID_REQUESTMGR(source)); - REQUIRE(targetp != NULL && *targetp == NULL); - - REQUIRE(!source->exiting); - - source->iref++; - *targetp = source; - - req_log(ISC_LOG_DEBUG(3), "requestmgr_attach: %p: eref %d iref %d", - source, source->eref, source->iref); -} - -static void -requestmgr_detach(dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr; - bool need_destroy = false; - - REQUIRE(requestmgrp != NULL); - requestmgr = *requestmgrp; - *requestmgrp = NULL; - REQUIRE(VALID_REQUESTMGR(requestmgr)); LOCK(&requestmgr->lock); - INSIST(requestmgr->iref > 0); - requestmgr->iref--; + for (request = ISC_LIST_HEAD(requestmgr->requests); request != NULL; + request = ISC_LIST_NEXT(request, link)) + { + dns_request_cancel(request); + } - req_log(ISC_LOG_DEBUG(3), "requestmgr_detach: %p: eref %d iref %d", - requestmgr, requestmgr->eref, requestmgr->iref); - - if (requestmgr->iref == 0 && requestmgr->exiting) { - INSIST(ISC_LIST_HEAD(requestmgr->requests) == NULL); + if (ISC_LIST_EMPTY(requestmgr->requests)) { send_shutdown_events(requestmgr); - if (requestmgr->eref == 0) { - need_destroy = true; - } } - UNLOCK(&requestmgr->lock); - if (need_destroy) { - mgr_destroy(requestmgr); - } + UNLOCK(&requestmgr->lock); } void -dns_requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp) { +dns__requestmgr_attach(dns_requestmgr_t *source, dns_requestmgr_t **targetp, + const char *file, unsigned int line, const char *func) { + uint_fast32_t ref; + REQUIRE(VALID_REQUESTMGR(source)); REQUIRE(targetp != NULL && *targetp == NULL); - REQUIRE(!source->exiting); - LOCK(&source->lock); - source->eref++; + REQUIRE(!atomic_load_acquire(&source->exiting)); + + ref = isc_refcount_increment(&source->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + req_log(ISC_LOG_DEBUG(3), + "dns_requestmgr_attach: %p: references = %" PRIuFAST32, source, + ref + 1); + *targetp = source; - UNLOCK(&source->lock); - - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_attach: %p: eref %d iref %d", - source, source->eref, source->iref); } void -dns_requestmgr_detach(dns_requestmgr_t **requestmgrp) { - dns_requestmgr_t *requestmgr; - bool need_destroy = false; +dns__requestmgr_detach(dns_requestmgr_t **requestmgrp, const char *file, + unsigned int line, const char *func) { + dns_requestmgr_t *requestmgr = NULL; + uint_fast32_t ref; + + REQUIRE(requestmgrp != NULL && VALID_REQUESTMGR(*requestmgrp)); - REQUIRE(requestmgrp != NULL); requestmgr = *requestmgrp; *requestmgrp = NULL; - REQUIRE(VALID_REQUESTMGR(requestmgr)); - LOCK(&requestmgr->lock); - INSIST(requestmgr->eref > 0); - requestmgr->eref--; + ref = isc_refcount_decrement(&requestmgr->references); - req_log(ISC_LOG_DEBUG(3), "dns_requestmgr_detach: %p: eref %d iref %d", - requestmgr, requestmgr->eref, requestmgr->iref); +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, requestmgr, requestmgrp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ - if (requestmgr->eref == 0 && requestmgr->iref == 0) { - INSIST(requestmgr->exiting && - ISC_LIST_HEAD(requestmgr->requests) == NULL); - need_destroy = true; - } - UNLOCK(&requestmgr->lock); + req_log(ISC_LOG_DEBUG(3), + "dns_requestmgr_detach: %p: references = %" PRIuFAST32, + requestmgr, ref - 1); - if (need_destroy) { + if (ref == 1) { + INSIST(ISC_LIST_EMPTY(requestmgr->requests)); mgr_destroy(requestmgr); } } +/* FIXME */ static void send_shutdown_events(dns_requestmgr_t *requestmgr) { isc_event_t *event, *next_event; @@ -365,8 +346,7 @@ mgr_destroy(dns_requestmgr_t *requestmgr) { req_log(ISC_LOG_DEBUG(3), "mgr_destroy"); - REQUIRE(requestmgr->eref == 0); - REQUIRE(requestmgr->iref == 0); + isc_refcount_destroy(&requestmgr->references); isc_mutex_destroy(&requestmgr->lock); for (i = 0; i < DNS_REQUEST_NLOCKS; i++) { @@ -409,12 +389,13 @@ req_send(dns_request_t *request) { static isc_result_t new_request(isc_mem_t *mctx, dns_request_t **requestp) { - dns_request_t *request; + dns_request_t *request = NULL; request = isc_mem_get(mctx, sizeof(*request)); *request = (dns_request_t){ .dscp = -1 }; ISC_LINK_INIT(request, link); + isc_refcount_init(&request->references, 1); isc_mem_attach(mctx, &request->mctx); request->magic = REQUEST_MAGIC; @@ -426,25 +407,25 @@ static bool isblackholed(dns_dispatchmgr_t *dispatchmgr, const isc_sockaddr_t *destaddr) { dns_acl_t *blackhole; isc_netaddr_t netaddr; - int match; - bool drop = false; char netaddrstr[ISC_NETADDR_FORMATSIZE]; + int match; + isc_result_t result; blackhole = dns_dispatchmgr_getblackhole(dispatchmgr); - if (blackhole != NULL) { - isc_netaddr_fromsockaddr(&netaddr, destaddr); - if (dns_acl_match(&netaddr, NULL, blackhole, NULL, &match, - NULL) == ISC_R_SUCCESS && - match > 0) - { - drop = true; - } + if (blackhole == NULL) { + return (false); } - if (drop) { - isc_netaddr_format(&netaddr, netaddrstr, sizeof(netaddrstr)); - req_log(ISC_LOG_DEBUG(10), "blackholed address %s", netaddrstr); + + isc_netaddr_fromsockaddr(&netaddr, destaddr); + result = dns_acl_match(&netaddr, NULL, blackhole, NULL, &match, NULL); + if (result != ISC_R_SUCCESS || match == 0) { + return (false); } - return (drop); + + isc_netaddr_format(&netaddr, netaddrstr, sizeof(netaddrstr)); + req_log(ISC_LOG_DEBUG(10), "blackholed address %s", netaddrstr); + + return (true); } static isc_result_t @@ -553,6 +534,10 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, req_log(ISC_LOG_DEBUG(3), "dns_request_createraw"); + if (atomic_load_acquire(&requestmgr->exiting)) { + return (ISC_R_SHUTTINGDOWN); + } + if (isblackholed(requestmgr->dispatchmgr, destaddr)) { return (DNS_R_BLACKHOLED); } @@ -593,6 +578,12 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, request->timeout = udptimeout * 1000; } + isc_buffer_allocate(mctx, &request->query, r.length + (tcp ? 2 : 0)); + result = isc_buffer_copyregion(request->query, &r); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + again: result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, dscp, &connected, &request->dispatch); @@ -605,6 +596,9 @@ again: dispopt |= DNS_DISPATCHOPT_FIXEDID; } + dns_request_t *tmp = NULL; + req_attach(request, &tmp); + result = dns_dispatch_addresponse( request->dispatch, dispopt, request->timeout, destaddr, task, req_connected, req_senddone, req_response, req_timeout, request, @@ -619,24 +613,13 @@ again: goto cleanup; } - isc_buffer_allocate(mctx, &request->query, r.length + (tcp ? 2 : 0)); - result = isc_buffer_copyregion(request->query, &r); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - /* Add message ID. */ isc_buffer_usedregion(request->query, &r); r.base[0] = (id >> 8) & 0xff; r.base[1] = id & 0xff; LOCK(&requestmgr->lock); - if (requestmgr->exiting) { - UNLOCK(&requestmgr->lock); - result = ISC_R_SHUTTINGDOWN; - goto cleanup; - } - requestmgr_attach(requestmgr, &request->requestmgr); + dns_requestmgr_attach(requestmgr, &request->requestmgr); request->hash = mgr_gethash(requestmgr); ISC_LIST_APPEND(requestmgr->requests, request, link); UNLOCK(&requestmgr->lock); @@ -665,7 +648,7 @@ cleanup: if (tclone != NULL) { isc_task_detach(&tclone); } - req_destroy(request); + req_detach(&request); req_log(ISC_LOG_DEBUG(3), "dns_request_createraw: failed %s", dns_result_totext(result)); return (result); @@ -697,7 +680,6 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, isc_mem_t *mctx; dns_messageid_t id; bool tcp = false; - bool settsigkey = true; bool connected = false; REQUIRE(VALID_REQUESTMGR(requestmgr)); @@ -712,6 +694,10 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, req_log(ISC_LOG_DEBUG(3), "dns_request_createvia"); + if (atomic_load_acquire(&requestmgr->exiting)) { + return (ISC_R_SHUTTINGDOWN); + } + if (srcaddr != NULL && isc_sockaddr_pf(srcaddr) != isc_sockaddr_pf(destaddr)) { return (ISC_R_FAMILYMISMATCH); @@ -740,7 +726,11 @@ dns_request_createvia(dns_requestmgr_t *requestmgr, dns_message_t *message, dns_tsigkey_attach(key, &request->tsigkey); } -use_tcp: + result = dns_message_settsigkey(message, request->tsigkey); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + if ((options & DNS_REQUESTOPT_TCP) != 0) { tcp = true; request->timeout = timeout * 1000; @@ -754,12 +744,16 @@ use_tcp: request->timeout = udptimeout * 1000; } +use_tcp: result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, dscp, &connected, &request->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } + dns_request_t *tmp = NULL; + req_attach(request, &tmp); + result = dns_dispatch_addresponse( request->dispatch, 0, request->timeout, destaddr, task, req_connected, req_senddone, req_response, req_timeout, request, @@ -769,14 +763,8 @@ use_tcp: } message->id = id; - if (settsigkey) { - result = dns_message_settsigkey(message, request->tsigkey); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - } result = req_render(message, &request->query, options, mctx); - if (result == DNS_R_USETCP && (options & DNS_REQUESTOPT_TCP) == 0) { + if (result == DNS_R_USETCP && !tcp) { /* * Try again using TCP. */ @@ -784,7 +772,7 @@ use_tcp: dns_dispatch_removeresponse(&request->dispentry, NULL); dns_dispatch_detach(&request->dispatch); options |= DNS_REQUESTOPT_TCP; - settsigkey = false; + tcp = true; goto use_tcp; } if (result != ISC_R_SUCCESS) { @@ -797,12 +785,7 @@ use_tcp: } LOCK(&requestmgr->lock); - if (requestmgr->exiting) { - UNLOCK(&requestmgr->lock); - result = ISC_R_SHUTTINGDOWN; - goto cleanup; - } - requestmgr_attach(requestmgr, &request->requestmgr); + dns_requestmgr_attach(requestmgr, &request->requestmgr); request->hash = mgr_gethash(requestmgr); ISC_LIST_APPEND(requestmgr->requests, request, link); UNLOCK(&requestmgr->lock); @@ -831,7 +814,7 @@ cleanup: if (tclone != NULL) { isc_task_detach(&tclone); } - req_destroy(request); + req_detach(&request); req_log(ISC_LOG_DEBUG(3), "dns_request_createvia: failed %s", dns_result_totext(result)); return (result); @@ -956,9 +939,7 @@ request_cancel(dns_request_t *request) { request->flags &= ~DNS_REQUEST_F_CONNECTING; if (request->dispentry != NULL) { - dns_dispatch_cancel(NULL, request->dispentry, - DNS_REQUEST_SENDING(request), - DNS_REQUEST_CONNECTING(request)); + dns_dispatch_cancel(request->dispentry); dns_dispatch_removeresponse(&request->dispentry, NULL); } @@ -1047,7 +1028,7 @@ dns_request_destroy(dns_request_t **requestp) { INSIST(request->dispentry == NULL); INSIST(request->dispatch == NULL); - req_destroy(request); + req_detach(&request); } /*** @@ -1059,15 +1040,17 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { UNUSED(handle); + req_log(ISC_LOG_DEBUG(3), "req_connected: request %p: %s", request, + isc_result_totext(eresult)); + if (eresult == ISC_R_CANCELED) { + req_detach(&request); return; } REQUIRE(VALID_REQUEST(request)); REQUIRE(DNS_REQUEST_CONNECTING(request)); - req_log(ISC_LOG_DEBUG(3), "req_connected: request %p", request); - LOCK(&request->requestmgr->locks[request->hash]); request->flags &= ~DNS_REQUEST_F_CONNECTING; @@ -1084,6 +1067,8 @@ req_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { send_if_done(request, ISC_R_CANCELED); } UNLOCK(&request->requestmgr->locks[request->hash]); + + req_detach(&request); } static void @@ -1195,12 +1180,76 @@ req_sendevent(dns_request_t *request, isc_result_t result) { isc_task_sendanddetach(&task, (isc_event_t **)&request->event); } +static void +__req_attach(dns_request_t *source, dns_request_t **targetp, const char *file, + unsigned int line, const char *func) { + uint_fast32_t ref; + + REQUIRE(VALID_REQUEST(source)); + REQUIRE(targetp != NULL && *targetp == NULL); + + ref = isc_refcount_increment(&source->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + *targetp = source; +} + +static void +__req_detach(dns_request_t **requestp, const char *file, unsigned int line, + const char *func) { + dns_request_t *request = NULL; + uint_fast32_t ref; + + REQUIRE(requestp != NULL && VALID_REQUEST(*requestp)); + + request = *requestp; + *requestp = NULL; + + ref = isc_refcount_decrement(&request->references); + +#ifdef REQ_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, request, requestp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* REQ_TRACE */ + + if (request->requestmgr != NULL && + atomic_load_acquire(&request->requestmgr->exiting)) + { + /* We are shutting down and this was last request */ + LOCK(&request->requestmgr->lock); + if (ISC_LIST_EMPTY(request->requestmgr->requests)) { + send_shutdown_events(request->requestmgr); + } + UNLOCK(&request->requestmgr->lock); + } + + if (ref == 1) { + req_destroy(request); + } +} + static void req_destroy(dns_request_t *request) { REQUIRE(VALID_REQUEST(request)); req_log(ISC_LOG_DEBUG(3), "req_destroy: request %p", request); + isc_refcount_destroy(&request->references); + request->magic = 0; if (request->query != NULL) { isc_buffer_free(&request->query); @@ -1224,7 +1273,7 @@ req_destroy(dns_request_t *request) { dns_tsigkey_detach(&request->tsigkey); } if (request->requestmgr != NULL) { - requestmgr_detach(&request->requestmgr); + dns_requestmgr_detach(&request->requestmgr); } isc_mem_putanddetach(&request->mctx, request, sizeof(*request)); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index d140628fb0..2a65090906 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -61,6 +61,11 @@ #include #include #include + +#define RESOLVER_TRACE 1 + +#define WANT_QUERYTRACE 1 + #ifdef WANT_QUERYTRACE #define RTRACE(m) \ isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER, \ @@ -222,6 +227,7 @@ typedef struct fetchctx fetchctx_t; typedef struct query { /* Locked by task event serialization. */ unsigned int magic; + isc_refcount_t references; fetchctx_t *fctx; dns_message_t *rmessage; isc_mem_t *mctx; @@ -240,8 +246,6 @@ typedef struct query { int ednsversion; unsigned int options; isc_sockeventattr_t attributes; - unsigned int sends; - unsigned int connects; unsigned int udpsize; unsigned char data[512]; } resquery_t; @@ -543,7 +547,7 @@ struct dns_resolver { /* Locked by lock. */ isc_eventlist_t whenshutdown; - unsigned int activebuckets; + isc_refcount_t activebuckets; unsigned int spillat; /* clients-per-query */ dns_badcache_t *badcache; /* Bad cache. */ @@ -824,7 +828,7 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, static void rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo); -static void +static isc_result_t rctx_next(respctx_t *rctx); static void @@ -1122,19 +1126,27 @@ munge: } static inline void -resquery_destroy(resquery_t **queryp) { +resquery_destroy(resquery_t *query) { dns_resolver_t *res; bool empty; - resquery_t *query; fetchctx_t *fctx; unsigned int bucket; - REQUIRE(queryp != NULL); - query = *queryp; - *queryp = NULL; REQUIRE(!ISC_LINK_LINKED(query, link)); - INSIST(query->dispatch == NULL); + if (query->tsig != NULL) { + isc_buffer_free(&query->tsig); + } + + if (query->tsigkey != NULL) { + dns_tsigkey_detach(&query->tsigkey); + } + + if (query->dispatch != NULL) { + dns_dispatch_detach(&query->dispatch); + } + + isc_refcount_destroy(&query->references); fctx = query->fctx; res = fctx->res; @@ -1157,6 +1169,61 @@ resquery_destroy(resquery_t **queryp) { } } +#define resquery_attach(s, t) \ + __resquery_attach(s, t, __FILE__, __LINE__, __func__) + +static void __attribute__((unused)) +__resquery_attach(resquery_t *source, resquery_t **targetp, const char *file, + unsigned int line, const char *func) { + uint_fast32_t ref; + + REQUIRE(VALID_QUERY(source)); + REQUIRE(targetp != NULL && *targetp == NULL); + + ref = isc_refcount_increment(&source->references); + +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, source, targetp, ref + 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); + UNUSED(ref); +#endif /* RESOLVER_TRACE */ + + *targetp = source; +} + +#define resquery_detach(q) __resquery_detach(q, __FILE__, __LINE__, __func__) + +static void +__resquery_detach(resquery_t **queryp, const char *file, unsigned int line, + const char *func) { + uint_fast32_t ref; + resquery_t *query = NULL; + + REQUIRE(queryp != NULL && VALID_QUERY(*queryp)); + + query = *queryp; + *queryp = NULL; + + ref = isc_refcount_decrement(&query->references); + +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(%p, %p) = %" PRIuFAST32 "\n", func, file, + line, __func__, query, queryp, ref - 1); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* RESOLVER_TRACE */ + + if (ref == 1) { + resquery_destroy(query); + } +} + /*% * Update EDNS statistics for a server after not getting a response to a UDP * query sent to it. @@ -1176,23 +1243,36 @@ update_edns_stats(resquery_t *query) { } } +#define fctx_cancelquery(q, d, f, n, a) \ + __fctx_cancelquery(q, d, f, n, a, __FILE__, __LINE__, __func__) + static void -fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, - isc_time_t *finish, bool no_response, bool age_untried) { +__fctx_cancelquery(resquery_t *query, dns_dispatchevent_t **deventp, + isc_time_t *finish, bool no_response, bool age_untried, + const char *file, unsigned int line, const char *func) { fetchctx_t *fctx = NULL; - resquery_t *query = NULL; unsigned int rtt, rttms; unsigned int factor; dns_adbfind_t *find = NULL; dns_adbaddrinfo_t *addrinfo; isc_stdtime_t now; - query = *queryp; fctx = query->fctx; +#ifdef RESOLVER_TRACE + fprintf(stderr, "%s:%s:%u:%s(query = %p, ...)\n", func, file, line, + __func__, query); +#else + UNUSED(func); + UNUSED(file); + UNUSED(line); +#endif /* RESOLVER_TRACE */ + FCTXTRACE("cancelquery"); - REQUIRE(!RESQUERY_CANCELED(query)); + if (RESQUERY_CANCELED(query)) { + return; + } query->attributes |= RESQUERY_ATTR_CANCELED; @@ -1352,16 +1432,14 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, } /* - * Check for any outstanding socket events. If they exist, cancel - * them and let the event handlers finish the cleanup. (XXX: - * Currently the resolver, rather than dispatch, tracks whether - * it's sending or connecting; this will be moved into dispatch - * later.) + * Check for any outstanding socket events. If they exist, + * cancel them and let the event handlers finish the cleanup. + * (XXX: Currently the resolver, rather than dispatch, tracks + * whether it's sending or connecting; this will be moved into + * dispatch later.) */ - dns_dispatch_cancel(query->dispatch, query->dispentry, - RESQUERY_SENDING(query), - RESQUERY_CONNECTING(query)); if (query->dispentry != NULL) { + dns_dispatch_cancel(query->dispentry); dns_dispatch_removeresponse(&query->dispentry, deventp); } @@ -1369,24 +1447,8 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, ISC_LIST_UNLINK(fctx->queries, query, link); } - if (query->tsig != NULL) { - isc_buffer_free(&query->tsig); - } - - if (query->tsigkey != NULL) { - dns_tsigkey_detach(&query->tsigkey); - } - - if (query->dispatch != NULL) { - dns_dispatch_detach(&query->dispatch); - } - - if (!(RESQUERY_CONNECTING(query) || RESQUERY_SENDING(query))) { - /* - * It's safe to destroy the query now. - */ - resquery_destroy(&query); - } + /* This is the final detach matching the "init" */ + resquery_detach(&query); } static void @@ -1398,7 +1460,7 @@ fctx_cancelqueries(fetchctx_t *fctx, bool no_response, bool age_untried) { for (query = ISC_LIST_HEAD(fctx->queries); query != NULL; query = next_query) { next_query = ISC_LIST_NEXT(query, link); - fctx_cancelquery(&query, NULL, NULL, no_response, age_untried); + fctx_cancelquery(query, NULL, NULL, no_response, age_untried); } } @@ -1734,61 +1796,53 @@ fctx_done(fetchctx_t *fctx, isc_result_t result, int line) { static void resquery_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resquery_t *query = (resquery_t *)arg; - bool destroy_query = false; fetchctx_t *fctx = NULL; QTRACE("senddone"); UNUSED(handle); - INSIST(RESQUERY_SENDING(query)); - - query->sends--; - fctx = query->fctx; if (RESQUERY_CANCELED(query)) { - if (query->sends == 0 && query->connects == 0) { - destroy_query = true; - } - } else { - switch (eresult) { - case ISC_R_SUCCESS: - break; - - case ISC_R_HOSTUNREACH: - case ISC_R_NETUNREACH: - case ISC_R_NOPERM: - case ISC_R_ADDRNOTAVAIL: - case ISC_R_CONNREFUSED: - FCTXTRACE3("query canceled in resquery_senddone(): " - "no route to host; no response", - eresult); - - /* - * No route to remote. - */ - add_bad(fctx, query->rmessage, query->addrinfo, eresult, - badns_unreachable); - fctx_cancelquery(&query, NULL, NULL, true, false); - - FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); - break; - - default: - FCTXTRACE3("query canceled in resquery_senddone() " - "due to unexpected result; responding", - eresult); - fctx_cancelquery(&query, NULL, NULL, false, false); - fctx_done(fctx, eresult, __LINE__); - break; - } + goto detach; } - if (destroy_query) { - resquery_destroy(&query); + switch (eresult) { + case ISC_R_SUCCESS: + /* Don't detach from resquery */ + return; + + case ISC_R_HOSTUNREACH: + case ISC_R_NETUNREACH: + case ISC_R_NOPERM: + case ISC_R_ADDRNOTAVAIL: + case ISC_R_CONNREFUSED: + FCTXTRACE3("query canceled in resquery_senddone(): " + "no route to host; no response", + eresult); + + /* + * No route to remote. + */ + add_bad(fctx, query->rmessage, query->addrinfo, eresult, + badns_unreachable); + fctx_cancelquery(query, NULL, NULL, true, false); + + FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); + fctx_try(fctx, true, false); + break; + default: + FCTXTRACE3("query canceled in resquery_senddone() " + "due to unexpected result; responding", + eresult); + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, eresult, __LINE__); + break; } + +detach: + resquery_detach(&query); } static inline isc_result_t @@ -1961,9 +2015,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, srtt = addrinfo->srtt; /* - * Allow an additional second for the kernel to resend the SYN (or - * SYN without ECN in the case of stupid firewalls blocking ECN - * negotiation) over the current RTT estimate. + * Allow an additional second for the kernel to resend the SYN + * (or SYN without ECN in the case of stupid firewalls blocking + * ECN negotiation) over the current RTT estimate. */ if ((options & DNS_FETCHOPT_TCP) != 0) { srtt += US_PER_SEC; @@ -1992,9 +2046,11 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, .addrinfo = addrinfo, .dispatchmgr = res->dispatchmgr }; + isc_refcount_init(&query->references, 1); + /* - * Note that the caller MUST guarantee that 'addrinfo' will remain - * valid until this query is canceled. + * Note that the caller MUST guarantee that 'addrinfo' will + * remain valid until this query is canceled. */ dns_message_create(fctx->mctx, DNS_MESSAGE_INTENTPARSE, @@ -2112,9 +2168,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, /* * We should always have a valid dispatcher here. If we * don't support a protocol family, then its dispatcher - * will be NULL, but we shouldn't be finding addresses for - * protocol types we don't support, so the dispatcher - * we found should never be NULL. + * will be NULL, but we shouldn't be finding addresses + * for protocol types we don't support, so the + * dispatcher we found should never be NULL. */ INSIST(query->dispatch != NULL); } @@ -2137,6 +2193,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, fctx->nqueries++; UNLOCK(&res->buckets[fctx->bucketnum].lock); + resquery_t *tmp = NULL; + resquery_attach(query, &tmp); + /* Set up the dispatch and set the query ID */ result = dns_dispatch_addresponse( query->dispatch, 0, isc_interval_ms(&fctx->interval), @@ -2148,8 +2207,8 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, } /* Connect the socket */ - query->connects++; fctx_increference(fctx); + result = dns_dispatch_connect(query->dispentry); return (result); @@ -2159,11 +2218,10 @@ cleanup_dispatch: } cleanup_query: - if (query->connects == 0) { - query->magic = 0; - dns_message_detach(&query->rmessage); - isc_mem_put(fctx->mctx, query, sizeof(*query)); - } + + query->magic = 0; + dns_message_detach(&query->rmessage); + isc_mem_put(fctx->mctx, query, sizeof(*query)); return (result); } @@ -2271,7 +2329,8 @@ compute_cc(const resquery_t *query, uint8_t *cookie, const size_t len) { INSIST(len >= CLIENT_COOKIE_SIZE); STATIC_ASSERT(sizeof(query->fctx->res->view->secret) >= ISC_SIPHASH24_KEY_LENGTH, - "The view->secret size can't fit SipHash 2-4 key length"); + "The view->secret size can't fit SipHash 2-4 key " + "length"); uint8_t buf[16] ISC_NONSTRING = { 0 }; size_t buflen = add_serveraddr(buf, sizeof(buf), query); @@ -2361,8 +2420,8 @@ resquery_send(resquery_t *query) { qrdataset = NULL; /* - * Set RD if the client has requested that we do a recursive query, - * or if we're sending to a forwarder. + * Set RD if the client has requested that we do a recursive + * query, or if we're sending to a forwarder. */ if ((query->options & DNS_FETCHOPT_RECURSIVE) != 0 || ISFORWARDER(query->addrinfo)) @@ -2426,8 +2485,8 @@ resquery_send(resquery_t *query) { (void)dns_peerlist_peerbyaddr(fctx->res->view->peers, &ipaddr, &peer); /* - * The ADB does not know about servers with "edns no". Check this, - * and then inform the ADB for future use. + * The ADB does not know about servers with "edns no". Check + * this, and then inform the ADB for future use. */ if ((query->addrinfo->flags & DNS_FETCHOPT_NOEDNS0) == 0 && peer != NULL && @@ -2449,12 +2508,13 @@ resquery_send(resquery_t *query) { struct tried *tried; /* - * If this is the first timeout for this server in this fetch - * context, try setting EDNS UDP buffer size to the largest UDP - * response size we have seen from this server so far. + * If this is the first timeout for this server in this + * fetch context, try setting EDNS UDP buffer size to + * the largest UDP response size we have seen from this + * server so far. * - * If this server has already timed out twice or more in this - * fetch context, force TCP. + * If this server has already timed out twice or more in + * this fetch context, force TCP. */ if ((tried = triededns(fctx, sockaddr)) != NULL) { if (tried->count == 1U) { @@ -2468,8 +2528,8 @@ resquery_send(resquery_t *query) { fctx->timeout = false; /* - * Use EDNS0, unless the caller doesn't want it, or we know that the - * remote server doesn't like it. + * Use EDNS0, unless the caller doesn't want it, or we know that + * the remote server doesn't like it. */ if ((query->options & DNS_FETCHOPT_NOEDNS0) == 0) { if ((query->addrinfo->flags & DNS_FETCHOPT_NOEDNS0) == 0) { @@ -2483,26 +2543,26 @@ resquery_send(resquery_t *query) { uint16_t padding = 0; /* - * Set the default UDP size to what was configured as - * 'edns-buffer-size' + * Set the default UDP size to what was + * configured as 'edns-buffer-size' */ udpsize = res->udpsize; /* - * This server timed out for the first time in this - * fetch context and we received a response from it - * before (either in this fetch context or in a - * different one). Set 'udpsize' to the size of the - * largest UDP response we have received from this - * server so far. + * This server timed out for the first time in + * this fetch context and we received a response + * from it before (either in this fetch context + * or in a different one). Set 'udpsize' to the + * size of the largest UDP response we have + * received from this server so far. */ if (hint != 0U) { udpsize = hint; } /* - * If a fixed EDNS UDP buffer size is configured for - * this server, make sure we obey that. + * If a fixed EDNS UDP buffer size is configured + * for this server, make sure we obey that. */ if (peer != NULL) { (void)dns_peer_getudpsize(peer, &peerudpsize); @@ -2516,7 +2576,8 @@ resquery_send(resquery_t *query) { version >>= DNS_FETCHOPT_EDNSVERSIONSHIFT; } - /* Request NSID/COOKIE/VERSION for current peer? */ + /* Request NSID/COOKIE/VERSION for current peer? + */ if (peer != NULL) { uint8_t ednsversion; (void)dns_peer_getrequestnsid(peer, &reqnsid); @@ -2576,7 +2637,8 @@ resquery_send(resquery_t *query) { ednsopt++; } - /* Add PAD for current peer? Require TCP for now */ + /* Add PAD for current peer? Require TCP for now + */ if ((peer != NULL) && tcp) { (void)dns_peer_getpadding(peer, &padding); } @@ -2595,9 +2657,9 @@ resquery_send(resquery_t *query) { query->options |= DNS_FETCHOPT_WANTNSID; } else if (result != ISC_R_SUCCESS) { /* - * We couldn't add the OPT, but we'll press on. - * We're not using EDNS0, so set the NOEDNS0 - * bit. + * We couldn't add the OPT, but we'll + * press on. We're not using EDNS0, so + * set the NOEDNS0 bit. */ query->options |= DNS_FETCHOPT_NOEDNS0; query->ednsversion = -1; @@ -2606,8 +2668,8 @@ resquery_send(resquery_t *query) { } else { /* * We know this server doesn't like EDNS0, so we - * won't use it. Set the NOEDNS0 bit since we're - * not using EDNS0. + * won't use it. Set the NOEDNS0 bit since + * we're not using EDNS0. */ query->options |= DNS_FETCHOPT_NOEDNS0; query->ednsversion = -1; @@ -2622,7 +2684,8 @@ resquery_send(resquery_t *query) { query->udpsize = udpsize; /* - * If we need EDNS0 to do this query and aren't using it, we lose. + * If we need EDNS0 to do this query and aren't using it, we + * lose. */ if (NEEDEDNS0(fctx) && (query->options & DNS_FETCHOPT_NOEDNS0) != 0) { result = DNS_R_SERVFAIL; @@ -2704,7 +2767,6 @@ resquery_send(resquery_t *query) { isc_buffer_usedregion(&buffer, &r); dns_dispatch_send(query->dispentry, &r, query->dscp); - query->sends++; QTRACE("sent"); @@ -2755,6 +2817,7 @@ cleanup_temps: static void resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { resquery_t *query = (resquery_t *)arg; + resquery_t *connquery = query; isc_result_t result; fetchctx_t *fctx = NULL; dns_resolver_t *res = NULL; @@ -2768,7 +2831,6 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { UNUSED(handle); - query->connects--; fctx = query->fctx; res = fctx->res; @@ -2779,78 +2841,77 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { } if (RESQUERY_CANCELED(query)) { + goto detach; + } + + switch (eresult) { + case ISC_R_SUCCESS: /* - * This query was canceled while the connect() was - * in progress. + * We are connected. Update the dispatcher and + * send the query. */ - resquery_destroy(&query); - } else { - switch (eresult) { - case ISC_R_SUCCESS: - /* - * We are connected. Update the dispatcher and - * send the query. - */ - dns_dispatch_changeattributes( - query->dispatch, DNS_DISPATCHATTR_CONNECTED, - DNS_DISPATCHATTR_CONNECTED); + dns_dispatch_changeattributes(query->dispatch, + DNS_DISPATCHATTR_CONNECTED, + DNS_DISPATCHATTR_CONNECTED); - result = resquery_send(query); - if (result != ISC_R_SUCCESS) { - FCTXTRACE("query canceled: " - "resquery_send() failed; responding"); + result = resquery_send(query); + if (result != ISC_R_SUCCESS) { + FCTXTRACE("query canceled: " + "resquery_send() failed; " + "responding"); - fctx_cancelquery(&query, NULL, NULL, false, - false); - fctx_done(fctx, result, __LINE__); - } - - fctx->querysent++; - - ISC_LIST_APPEND(fctx->queries, query, link); - pf = isc_sockaddr_pf(&query->addrinfo->sockaddr); - if (pf == PF_INET) { - inc_stats(res, dns_resstatscounter_queryv4); - } else { - inc_stats(res, dns_resstatscounter_queryv6); - } - if (res->view->resquerystats != NULL) { - dns_rdatatypestats_increment( - res->view->resquerystats, fctx->type); - } - break; - - case ISC_R_NETUNREACH: - case ISC_R_HOSTUNREACH: - case ISC_R_CONNREFUSED: - case ISC_R_NOPERM: - case ISC_R_ADDRNOTAVAIL: - case ISC_R_CONNECTIONRESET: - case ISC_R_TIMEDOUT: - FCTXTRACE3("query canceled in resquery_connected(): " - "no route to host; no response", - eresult); - - /* - * Do not query this server again in this fetch context - * if the server is unavailable over TCP. - */ - add_bad(fctx, query->rmessage, query->addrinfo, eresult, - badns_unreachable); - fctx_cancelquery(&query, NULL, NULL, true, false); - - FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); - fctx_try(fctx, true, false); - break; - - default: - FCTXTRACE3("query canceled in resquery_connected() " - "due to unexpected result; responding", - eresult); - - fctx_cancelquery(&query, NULL, NULL, false, false); - fctx_done(fctx, eresult, __LINE__); + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, result, __LINE__); } + + fctx->querysent++; + + ISC_LIST_APPEND(fctx->queries, query, link); + pf = isc_sockaddr_pf(&query->addrinfo->sockaddr); + if (pf == PF_INET) { + inc_stats(res, dns_resstatscounter_queryv4); + } else { + inc_stats(res, dns_resstatscounter_queryv6); + } + if (res->view->resquerystats != NULL) { + dns_rdatatypestats_increment(res->view->resquerystats, + fctx->type); + } + break; + + case ISC_R_NETUNREACH: + case ISC_R_HOSTUNREACH: + case ISC_R_CONNREFUSED: + case ISC_R_NOPERM: + case ISC_R_ADDRNOTAVAIL: + case ISC_R_CONNECTIONRESET: + case ISC_R_TIMEDOUT: + FCTXTRACE3("query canceled in " + "resquery_connected(): " + "no route to host; no response", + eresult); + + /* + * Do not query this server again in this fetch + * context if the server is unavailable over + * TCP. + */ + add_bad(fctx, query->rmessage, query->addrinfo, eresult, + badns_unreachable); + fctx_cancelquery(query, NULL, NULL, true, false); + + FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); + fctx_try(fctx, true, false); + break; + + default: + FCTXTRACE3("query canceled in " + "resquery_connected() " + "due to unexpected result; responding", + eresult); + + fctx_cancelquery(query, NULL, NULL, false, false); + fctx_done(fctx, eresult, __LINE__); } LOCK(&res->buckets[bucketnum].lock); @@ -2859,6 +2920,9 @@ resquery_connected(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { if (bucket_empty) { empty_bucket(res); } + +detach: + resquery_detach(&connquery); } static void @@ -2899,9 +2963,9 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) { fctx->findfail++; if (fctx->pending == 0) { /* - * We've got nothing else to wait for and don't - * know the answer. There's nothing to do but - * fail the fctx. + * We've got nothing else to wait for + * and don't know the answer. There's + * nothing to do but fail the fctx. */ FCTX_ATTR_CLR(fctx, FCTX_ATTR_ADDRWAIT); want_done = true; @@ -2923,7 +2987,8 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) { if (want_try) { fctx_try(fctx, true, false); } else if (want_done) { - FCTXTRACE("fetch failed in finddone(); return ISC_R_FAILURE"); + FCTXTRACE("fetch failed in finddone(); return " + "ISC_R_FAILURE"); fctx_done(fctx, ISC_R_FAILURE, __LINE__); } else if (dodestroy) { fctx_destroy(fctx); @@ -3057,8 +3122,8 @@ add_bad(fetchctx_t *fctx, dns_message_t *rmessage, dns_adbaddrinfo_t *addrinfo, break; /* counted as 'valfail' */ case badns_forwarder: /* - * We were called to prevent the given forwarder from - * being used again for this fetch context. + * We were called to prevent the given forwarder + * from being used again for this fetch context. */ break; } @@ -3304,9 +3369,11 @@ findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port, fctx->quotacount++; /* quota exceeded */ } else if ((find->options & DNS_ADBFIND_LAMEPRUNED) != 0) { - fctx->lamecount++; /* cached lame server */ + fctx->lamecount++; /* cached lame server + */ } else { - fctx->adberr++; /* unreachable server, etc. */ + fctx->adberr++; /* unreachable server, + etc. */ } /* @@ -3384,8 +3451,8 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { /* * If we have DNS_FETCHOPT_NOFORWARD set and forwarding policy * allows us to not forward - skip forwarders and go straight - * to NSes. This is currently used to make sure that priming query - * gets root servers' IP addresses in ADDITIONAL section. + * to NSes. This is currently used to make sure that priming + * query gets root servers' IP addresses in ADDITIONAL section. */ if ((fctx->options & DNS_FETCHOPT_NOFORWARD) != 0 && (fctx->fwdpolicy != dns_fwdpolicy_only)) @@ -3468,8 +3535,8 @@ fctx_getaddresses(fetchctx_t *fctx, bool badcache) { } /* - * If the forwarding policy is "only", we don't need the addresses - * of the nameservers. + * If the forwarding policy is "only", we don't need the + * addresses of the nameservers. */ if (fctx->fwdpolicy == dns_fwdpolicy_only) { goto out; @@ -3489,8 +3556,9 @@ normal_nses: * can use. * * We don't want to set this option all the time, since - * if fctx->restarts > 1, we've clearly been having trouble - * with the addresses we had, so getting more could help. + * if fctx->restarts > 1, we've clearly been having + * trouble with the addresses we had, so getting more + * could help. */ stdoptions |= DNS_ADBFIND_AVOIDFETCHES; } @@ -3598,8 +3666,9 @@ out: */ if (fctx->pending > 0) { /* - * We're fetching the addresses, but don't have any - * yet. Tell the caller to wait for an answer. + * We're fetching the addresses, but don't have + * any yet. Tell the caller to wait for an + * answer. */ result = DNS_R_WAIT; } else { @@ -3607,8 +3676,8 @@ out: isc_interval_t i; /* * We've lost completely. We don't know any - * addresses, and the ADB has told us it can't get - * them. + * addresses, and the ADB has told us it can't + * get them. */ FCTXTRACE("no addresses"); isc_interval_set(&i, DNS_RESOLVER_BADCACHETTL(fctx), 0); @@ -3626,8 +3695,8 @@ out: /* * If all of the addresses found were over the - * fetches-per-server quota, return the configured - * response. + * fetches-per-server quota, return the + * configured response. */ if (all_spilled) { result = res->quotaresp[dns_quotatype_server]; @@ -3636,8 +3705,8 @@ out: } } else { /* - * We've found some addresses. We might still be looking - * for more addresses. + * We've found some addresses. We might still be + * looking for more addresses. */ sort_finds(&fctx->finds, res->view->v6bias); sort_finds(&fctx->altfinds, 0); @@ -3958,9 +4027,9 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) { if (fctx->minimized && !fctx->forwarding) { unsigned int options = fctx->options; /* - * Also clear DNS_FETCHOPT_TRYSTALE_ONTIMEOUT here, otherwise - * every query minimization step will activate the try-stale - * timer again. + * Also clear DNS_FETCHOPT_TRYSTALE_ONTIMEOUT here, + * otherwise every query minimization step will activate + * the try-stale timer again. */ options &= ~(DNS_FETCHOPT_QMINIMIZE | DNS_FETCHOPT_TRYSTALE_ONTIMEOUT); @@ -4119,7 +4188,8 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { fctx->qmin_labels = DNS_MAX_LABELS + 1; /* * We store the result. If we succeed in the end - * we'll issue a warning that the server is broken. + * we'll issue a warning that the server is + * broken. */ fctx->qmin_warning = result; } else { @@ -4140,9 +4210,9 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { &fctx->nameservers, NULL); /* - * DNS_R_NXDOMAIN here means we have not loaded the root zone mirror - * yet - but DNS_R_NXDOMAIN is not a valid return value when doing - * recursion, we need to patch it. + * DNS_R_NXDOMAIN here means we have not loaded the root zone + * mirror yet - but DNS_R_NXDOMAIN is not a valid return value + * when doing recursion, we need to patch it. */ if (result == DNS_R_NXDOMAIN) { result = DNS_R_SERVFAIL; @@ -4174,9 +4244,10 @@ resume_qmin(isc_task_t *task, isc_event_t *event) { if (!fctx->minimized) { /* - * We have finished minimizing, but fctx->finds was filled at - * the beginning of the run - now we need to clear it before - * sending the final query to use proper nameservers. + * We have finished minimizing, but fctx->finds was + * filled at the beginning of the run - now we need to + * clear it before sending the final query to use proper + * nameservers. */ fctx_cancelqueries(fctx, false, false); fctx_cleanupall(fctx); @@ -4299,7 +4370,8 @@ fctx_shutdown(fetchctx_t *fctx) { isc_event_t *cevent; /* - * Start the shutdown process for fctx, if it isn't already underway. + * Start the shutdown process for fctx, if it isn't already + * underway. */ FCTXTRACE("shutdown"); @@ -4352,7 +4424,8 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) { /* * Cancel all pending validators. Note that this must be done - * without the bucket lock held, since that could cause deadlock. + * without the bucket lock held, since that could cause + * deadlock. */ validator = ISC_LIST_HEAD(fctx->validators); while (validator != NULL) { @@ -4429,8 +4502,8 @@ fctx_start(isc_task_t *task, isc_event_t *event) { INSIST(fctx->state == fetchstate_init); if (fctx->want_shutdown) { /* - * We haven't started this fctx yet, and we've been requested - * to shut it down. + * We haven't started this fctx yet, and we've been + * requested to shut it down. */ FCTX_ATTR_SET(fctx, FCTX_ATTR_SHUTTINGDOWN); fctx->state = fetchstate_done; @@ -4456,8 +4529,8 @@ fctx_start(isc_task_t *task, isc_event_t *event) { */ fctx->state = fetchstate_active; /* - * Reset the control event for later use in shutting down - * the fctx. + * Reset the control event for later use in shutting + * down the fctx. */ ISC_EVENT_INIT(event, sizeof(*event), 0, NULL, DNS_EVENT_FETCHCONTROL, fctx_doshutdown, fctx, @@ -4491,8 +4564,8 @@ fctx_add_event(fetchctx_t *fctx, isc_task_t *task, const isc_sockaddr_t *client, /* * We store the task we're going to send this event to in the - * sender field. We'll make the fetch the sender when we actually - * send the event. + * sender field. We'll make the fetch the sender when we + * actually send the event. */ isc_task_attach(task, &tclone); event = (dns_fetchevent_t *)isc_event_allocate(fctx->res->mctx, tclone, @@ -4510,8 +4583,8 @@ fctx_add_event(fetchctx_t *fctx, isc_task_t *task, const isc_sockaddr_t *client, event->foundname = dns_fixedname_initname(&event->fname); /* - * Store the sigrdataset in the first event in case it is needed by - * any of the events. + * Store the sigrdataset in the first event in case it is needed + * by any of the events. */ if (event->sigrdataset != NULL) { ISC_LIST_PREPEND(fctx->events, event, ev_link); @@ -4567,7 +4640,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, size_t p; /* - * Caller must be holding the lock for bucket number 'bucketnum'. + * Caller must be holding the lock for bucket number + * 'bucketnum'. */ REQUIRE(fctxp != NULL && *fctxp == NULL); @@ -4682,8 +4756,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, /* * The caller didn't supply a query domain and - * nameservers, and we're not in forward-only mode, - * so find the best nameservers to use. + * nameservers, and we're not in forward-only + * mode, so find the best nameservers to use. */ if (dns_rdatatype_atparent(fctx->type)) { findoptions |= DNS_DBFIND_NOEXACT; @@ -4702,7 +4776,8 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, fctx->ns_ttl_ok = true; } else { /* - * We're in forward-only mode. Set the query domain. + * We're in forward-only mode. Set the query + * domain. */ dns_name_copy(fname, fctx->domain); dns_name_copy(fname, fctx->qmindcname); @@ -4757,15 +4832,15 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type, } /* - * Default retry interval initialization. We set the interval now - * mostly so it won't be uninitialized. It will be set to the - * correct value before a query is issued. + * Default retry interval initialization. We set the interval + * now mostly so it won't be uninitialized. It will be set to + * the correct value before a query is issued. */ isc_interval_set(&fctx->interval, 2, 0); /* - * If stale answers are enabled, compute an expiration time after - * which stale data will be served, if the target RRset is + * If stale answers are enabled, compute an expiration time + * after which stale data will be served, if the target RRset is * available in cache. */ if ((options & DNS_FETCHOPT_TRYSTALE_ONTIMEOUT) != 0) { @@ -5023,7 +5098,8 @@ clone_results(fetchctx_t *fctx) { for (event = ISC_LIST_HEAD(fctx->events); event != NULL; event = ISC_LIST_NEXT(event, ev_link)) { - /* This is the the head event; keep a pointer and move on */ + /* This is the the head event; keep a pointer and move + * on */ if (hevent == NULL) { hevent = ISC_LIST_HEAD(fctx->events); continue; @@ -5032,17 +5108,18 @@ clone_results(fetchctx_t *fctx) { if (event->ev_type == DNS_EVENT_TRYSTALE) { /* * We don't need to clone resulting data to this - * type of event, as its associated callback is only - * called when stale-answer-client-timeout triggers, - * and the logic in there doesn't expect any result - * as input, as it will itself lookup for stale data - * in cache to use as result, if any is available. + * type of event, as its associated callback is + * only called when stale-answer-client-timeout + * triggers, and the logic in there doesn't + * expect any result as input, as it will itself + * lookup for stale data in cache to use as + * result, if any is available. * - * Also, if we reached this point, then the whole fetch - * context is done, it will cancel timers, process - * associated callbacks of type DNS_EVENT_FETCHDONE, and - * silently remove/free events of type - * DNS_EVENT_TRYSTALE. + * Also, if we reached this point, then the + * whole fetch context is done, it will cancel + * timers, process associated callbacks of type + * DNS_EVENT_FETCHDONE, and silently remove/free + * events of type DNS_EVENT_TRYSTALE. */ continue; } @@ -5086,7 +5163,8 @@ clone_results(fetchctx_t *fctx) { * '*fctx' is shutting down. * * Returns: - * true if the resolver is exiting and this is the last fctx in the bucket. + * true if the resolver is exiting and this is the last fctx in the + *bucket. */ static bool maybe_destroy(fetchctx_t *fctx, bool locked) { @@ -5205,8 +5283,8 @@ validated(isc_task_t *task, isc_event_t *event) { /* * If shutting down, ignore the results. Check to see if we're - * done waiting for validator completions and ADB pending events; if - * so, destroy the fctx. + * done waiting for validator completions and ADB pending + * events; if so, destroy the fctx. */ if (SHUTTINGDOWN(fctx) && !sentresponse) { bool bucket_empty; @@ -5221,8 +5299,8 @@ validated(isc_task_t *task, isc_event_t *event) { isc_stdtime_get(&now); /* - * If chaining, we need to make sure that the right result code is - * returned, and that the rdatasets are bound. + * If chaining, we need to make sure that the right result code + * is returned, and that the rdatasets are bound. */ if (vevent->result == ISC_R_SUCCESS && !negative && vevent->rdataset != NULL && CHAINING(vevent->rdataset)) @@ -5239,9 +5317,9 @@ validated(isc_task_t *task, isc_event_t *event) { } /* - * Either we're not shutting down, or we are shutting down but want - * to cache the result anyway (if this was a validation started by - * a query with cd set) + * Either we're not shutting down, or we are shutting down but + * want to cache the result anyway (if this was a validation + * started by a query with cd set) */ hevent = ISC_LIST_HEAD(fctx->events); @@ -5289,7 +5367,8 @@ validated(isc_task_t *task, isc_event_t *event) { } if (fctx->vresult == DNS_R_BROKENCHAIN && !negative) { /* - * Cache the data as pending for later validation. + * Cache the data as pending for later + * validation. */ result = ISC_R_NOTFOUND; if (vevent->rdataset != NULL) { @@ -5320,7 +5399,8 @@ validated(isc_task_t *task, isc_event_t *event) { if (fctx->validator != NULL) { dns_validator_send(fctx->validator); } else if (sentresponse) { - fctx_done(fctx, result, __LINE__); /* Locks bucket. */ + fctx_done(fctx, result, __LINE__); /* Locks + bucket. */ } else if (result == DNS_R_BROKENCHAIN) { isc_result_t tresult; isc_time_t expire; @@ -5336,7 +5416,8 @@ validated(isc_task_t *task, isc_event_t *event) { dns_resolver_addbadcache(res, fctx->name, fctx->type, &expire); } - fctx_done(fctx, result, __LINE__); /* Locks bucket. */ + fctx_done(fctx, result, __LINE__); /* Locks + bucket. */ } else { fctx_try(fctx, true, true); /* Locks bucket. */ } @@ -5458,8 +5539,8 @@ validated(isc_task_t *task, isc_event_t *event) { if (sentresponse) { bool bucket_empty = false; /* - * If we only deferred the destroy because we wanted to cache - * the data, destroy now. + * If we only deferred the destroy because we wanted to + * cache the data, destroy now. */ dns_db_detachnode(fctx->cache, &node); if (SHUTTINGDOWN(fctx)) { @@ -5823,11 +5904,12 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, anodep = &event->node; /* - * If this is an ANY, SIG or RRSIG query, we're not - * going to return any rdatasets, unless we encountered - * a CNAME or DNAME as "the answer". In this case, - * we're going to return DNS_R_CNAME or DNS_R_DNAME - * and we must set up the rdatasets. + * If this is an ANY, SIG or RRSIG query, we're + * not going to return any rdatasets, unless we + * encountered a CNAME or DNAME as "the answer". + * In this case, we're going to return + * DNS_R_CNAME or DNS_R_DNAME and we must set up + * the rdatasets. */ if ((fctx->type != dns_rdatatype_any && fctx->type != dns_rdatatype_rrsig && @@ -5920,21 +6002,21 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * If this RRset is in a secure domain, is in bailiwick, - * and is not glue, attempt DNSSEC validation. (We do not - * attempt to validate glue or out-of-bailiwick data--even - * though there might be some performance benefit to doing - * so--because it makes it simpler and safer to ensure that - * records from a secure domain are only cached if validated - * within the context of a query to the domain that owns - * them.) + * and is not glue, attempt DNSSEC validation. (We do + * not attempt to validate glue or out-of-bailiwick + * data--even though there might be some performance + * benefit to doing so--because it makes it simpler and + * safer to ensure that records from a secure domain are + * only cached if validated within the context of a + * query to the domain that owns them.) */ if (secure_domain && rdataset->trust != dns_trust_glue && !EXTERNAL(rdataset)) { dns_trust_t trust; /* - * RRSIGs are validated as part of validating the - * type they cover. + * RRSIGs are validated as part of validating + * the type they cover. */ if (rdataset->type == dns_rdatatype_rrsig) { continue; @@ -5944,7 +6026,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, !ANSWER(rdataset)) { /* * Ignore unrelated non-answer - * rdatasets that are missing signatures. + * rdatasets that are missing + * signatures. */ continue; } @@ -5969,9 +6052,9 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, /* * Cache this rdataset/sigrdataset pair as - * pending data. Track whether it was additional - * or not. If this was a priming query, additional - * should be cached as glue. + * pending data. Track whether it was + * additional or not. If this was a priming + * query, additional should be cached as glue. */ if (rdataset->trust == dns_trust_additional) { trust = dns_trust_pending_additional; @@ -6016,11 +6099,13 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, ardataset != NULL && NEGATIVE(ardataset)) { /* - * The answer in the cache is - * better than the answer we - * found, and is a negative - * cache entry, so we must set - * eresult appropriately. + * The answer in the + * cache is better than + * the answer we found, + * and is a negative + * cache entry, so we + * must set eresult + * appropriately. */ if (NXDOMAIN(ardataset)) { eresult = @@ -6030,10 +6115,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, DNS_R_NCACHENXRRSET; } /* - * We have a negative response - * from the cache so don't - * attempt to add the RRSIG - * rrset. + * We have a negative + * response from the + * cache so don't + * attempt to add the + * RRSIG rrset. */ continue; } @@ -6065,9 +6151,10 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, { /* * This is The Answer. We will - * validate it, but first we cache - * the rest of the response - it may - * contain useful keys. + * validate it, but first we + * cache the rest of the + * response - it may contain + * useful keys. */ INSIST(valrdataset == NULL && valsigrdataset == NULL); @@ -6124,10 +6211,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, rdataset->covers == dns_rdatatype_ns))) { /* - * If the trust level is 'dns_trust_glue' - * then we are adding data from a referral - * we got while executing the search algorithm. - * New referral data always takes precedence + * If the trust level is + * 'dns_trust_glue' then we are adding + * data from a referral we got while + * executing the search algorithm. New + * referral data always takes precedence * over the existing cache contents. */ options = DNS_DBADD_FORCE; @@ -6162,10 +6250,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, if (ANSWER(rdataset) && ardataset != NULL && NEGATIVE(ardataset)) { /* - * The answer in the cache is better - * than the answer we found, and is - * a negative cache entry, so we - * must set eresult appropriately. + * The answer in the cache is + * better than the answer we + * found, and is a negative + * cache entry, so we must set + * eresult appropriately. */ if (NXDOMAIN(ardataset)) { eresult = DNS_R_NCACHENXDOMAIN; @@ -6199,7 +6288,8 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, FCTX_ATTR_SET(fctx, FCTX_ATTR_HAVEANSWER); if (event != NULL) { /* - * Negative results must be indicated in event->result. + * Negative results must be indicated in + * event->result. */ if (dns_rdataset_isassociated(event->rdataset) && NEGATIVE(event->rdataset)) { @@ -6268,7 +6358,8 @@ cache_message(fetchctx_t *fctx, dns_message_t *message, } /* - * Do what dns_ncache_addoptout() does, and then compute an appropriate eresult. + * Do what dns_ncache_addoptout() does, and then compute an appropriate + * eresult. */ static isc_result_t ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, @@ -6308,10 +6399,10 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node, } else { /* * Either we don't care about the nature of the - * cache rdataset (because no fetch is interested - * in the outcome), or the cache rdataset is not - * a negative cache entry. Whichever case it is, - * we can return success. + * cache rdataset (because no fetch is + * interested in the outcome), or the cache + * rdataset is not a negative cache entry. + * Whichever case it is, we can return success. * * XXXRTH There's a CNAME/DNAME problem here. */ @@ -6413,9 +6504,9 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message, NULL, NULL, valoptions, res->buckets[fctx->bucketnum].task); /* - * If validation is necessary, return now. Otherwise continue - * to process the message, letting the validation complete - * in its own good time. + * If validation is necessary, return now. Otherwise + * continue to process the message, letting the + * validation complete in its own good time. */ return (result); } @@ -6618,8 +6709,8 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, } /* - * If the owner name matches one in the exclusion list, either exactly - * or partially, allow it. + * If the owner name matches one in the exclusion list, either + * exactly or partially, allow it. */ if (view->answeracl_exclude != NULL) { dns_rbtnode_t *node = NULL; @@ -6633,9 +6724,9 @@ is_answeraddress_allowed(dns_view_t *view, dns_name_t *name, } /* - * Otherwise, search the filter list for a match for each address - * record. If a match is found, the address should be filtered, - * so should the entire answer. + * Otherwise, search the filter list for a match for each + * address record. If a match is found, the address should be + * filtered, so should the entire answer. */ for (result = dns_rdataset_first(rdataset); result == ISC_R_SUCCESS; result = dns_rdataset_next(rdataset)) @@ -6748,8 +6839,8 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, } /* - * If the owner name matches one in the exclusion list, either exactly - * or partially, allow it. + * If the owner name matches one in the exclusion list, either + * exactly or partially, allow it. */ if (view->answernames_exclude != NULL) { result = dns_rbt_findnode(view->answernames_exclude, qname, @@ -6760,12 +6851,13 @@ is_answertarget_allowed(fetchctx_t *fctx, dns_name_t *qname, dns_name_t *rname, } /* - * If the target name is a subdomain of the search domain, allow it. + * If the target name is a subdomain of the search domain, allow + * it. * - * Note that if BIND is configured as a forwarding DNS server, the - * search domain will always match the root domain ("."), so we - * must also check whether forwarding is enabled so that filters - * can be applied; see GL #1574. + * Note that if BIND is configured as a forwarding DNS server, + * the search domain will always match the root domain ("."), so + * we must also check whether forwarding is enabled so that + * filters can be applied; see GL #1574. */ if (!fctx->forwarding && dns_name_issubdomain(tname, fctx->domain)) { return (true); @@ -6956,7 +7048,8 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) { dns_rdataset_t *nsrdataset = NULL; /* - * Retrieve state from fctx->nsfetch before we destroy it. + * Retrieve state from fctx->nsfetch before we destroy + * it. */ domain = dns_fixedname_initname(&fixed); dns_name_copy(fctx->nsfetch->private->domain, domain); @@ -7283,9 +7376,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * Is the question the same as the one we asked? - * NOERROR/NXDOMAIN/YXDOMAIN/REFUSED/SERVFAIL/BADCOOKIE must have - * the same question. - * FORMERR/NOTIMP if they have a question section then it must match. + * NOERROR/NXDOMAIN/YXDOMAIN/REFUSED/SERVFAIL/BADCOOKIE must + * have the same question. FORMERR/NOTIMP if they have a + * question section then it must match. */ switch (query->rmessage->rcode) { case dns_rcode_notimp: @@ -7328,21 +7421,15 @@ resquery_response(isc_task_t *task, isc_event_t *event) { } /* - * The dispatcher should ensure we only get responses with QR set. + * The dispatcher should ensure we only get responses with QR + * set. */ INSIST((query->rmessage->flags & DNS_MESSAGEFLAG_QR) != 0); - /* - * INSIST() that the message comes from the place we sent it to, - * since the dispatch code should ensure this. - * - * INSIST() that the message id is correct (this should also be - * ensured by the dispatch code). - */ /* - * If we have had a server cookie and don't get one retry over TCP. - * This may be a misconfigured anycast server or an attempt to send - * a spoofed response. Skip if we have a valid tsig. + * If we have had a server cookie and don't get one retry over + * TCP. This may be a misconfigured anycast server or an attempt + * to send a spoofed response. Skip if we have a valid tsig. */ if (dns_message_gettsig(query->rmessage, NULL) == NULL && !query->rmessage->cc_ok && !query->rmessage->cc_bad && @@ -7359,7 +7446,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { isc_log_write( dns_lctx, DNS_LOGCATEGORY_RESOLVER, DNS_LOGMODULE_RESOLVER, ISC_LOG_INFO, - "missing expected cookie from %s", + "missing expected cookie " + "from %s", addrbuf); } rctx.retryopts |= DNS_FETCHOPT_TCP; @@ -7367,10 +7455,6 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_done(&rctx, result); return; } - /* - * XXXMPA When support for DNS COOKIE becomes ubiquitous, fall - * back to TCP for all non-COOKIE responses. - */ } rctx_edns(&rctx); @@ -7459,8 +7543,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_delonly_zone(&rctx); /* - * Optionally call dns_rdata_checkowner() and dns_rdata_checknames() - * to validate the names in the response message. + * Optionally call dns_rdata_checkowner() and + * dns_rdata_checknames() to validate the names in the response + * message. */ if ((fctx->res->options & DNS_RESOLVER_CHECKNAMES) != 0) { checknames(query->rmessage); @@ -7497,7 +7582,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { case DNS_R_CHASEDSSERVERS: break; case DNS_R_DELEGATION: - /* With NOFOLLOW we want to pass the result code */ + /* With NOFOLLOW we want to pass the result code + */ if ((fctx->options & DNS_FETCHOPT_NOFOLLOW) == 0) { result = ISC_R_SUCCESS; } @@ -7531,8 +7617,8 @@ resquery_response(isc_task_t *task, isc_event_t *event) { rctx_additional(&rctx); /* - * Cache the cacheable parts of the message. This may also cause - * work to be queued to the DNSSEC validator. + * Cache the cacheable parts of the message. This may also + * cause work to be queued to the DNSSEC validator. */ if (WANTCACHE(fctx)) { isc_result_t tresult; @@ -7556,8 +7642,9 @@ resquery_response(isc_task_t *task, isc_event_t *event) { /* * rctx_respinit(): - * Initialize the response context structure 'rctx' to all zeroes, then set - * the task, event, query and fctx information from resquery_response(). + * Initialize the response context structure 'rctx' to all zeroes, then + * set the task, event, query and fctx information from + * resquery_response(). */ static void rctx_respinit(isc_task_t *task, dns_dispatchevent_t *devent, resquery_t *query, @@ -7579,8 +7666,8 @@ rctx_respinit(isc_task_t *task, dns_dispatchevent_t *devent, resquery_t *query, * rctx_answer_init(): * Clear and reinitialize those portions of 'rctx' that will be needed * when scanning the answer section of the response message. This can be - * called more than once if scanning needs to be restarted (though currently - * there are no cases in which this occurs). + * called more than once if scanning needs to be restarted (though + * currently there are no cases in which this occurs). */ static void rctx_answer_init(respctx_t *rctx) { @@ -7646,8 +7733,8 @@ rctx_dispfail(respctx_t *rctx) { (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) { /* - * The problem might be that they don't understand EDNS0. - * Turn it off and try again. + * The problem might be that they don't understand + * EDNS0. Turn it off and try again. */ rctx->retryopts |= DNS_FETCHOPT_NOEDNS0; rctx->resend = true; @@ -7659,9 +7746,9 @@ rctx_dispfail(respctx_t *rctx) { rctx->next_server = true; /* * If this is a network error, mark the server as bad so - * that we won't try it for this fetch again. Also adjust - * finish and no_response so that we penalize this address - * in SRTT adjustment later. + * that we won't try it for this fetch again. Also + * adjust finish and no_response so that we penalize + * this address in SRTT adjustment later. */ if (devent->result == ISC_R_HOSTUNREACH || devent->result == ISC_R_NETUNREACH || @@ -7920,15 +8007,16 @@ rctx_edns(respctx_t *rctx) { (rctx->retryopts & DNS_FETCHOPT_NOEDNS0) == 0) { /* - * We didn't get a OPT record in response to a EDNS query. + * We didn't get a OPT record in response to a EDNS + * query. * * Old versions of named incorrectly drop the OPT record - * when there is a signed, truncated response so we check - * that TC is not set. + * when there is a signed, truncated response so we + * check that TC is not set. * - * Record that the server is not talking EDNS. While this - * should be safe to do for any rcode we limit it to NOERROR - * and NXDOMAIN. + * Record that the server is not talking EDNS. While + * this should be safe to do for any rcode we limit it + * to NOERROR and NXDOMAIN. */ dns_message_logpacket( query->rmessage, "received packet (no opt) from", @@ -8122,8 +8210,8 @@ rctx_answer_positive(respctx_t *rctx) { } /* - * We didn't end with an incomplete chain, so the rcode should be - * "no error". + * We didn't end with an incomplete chain, so the rcode should + * be "no error". */ if (rctx->query->rmessage->rcode != dns_rcode_noerror) { log_formerr(fctx, "CNAME/DNAME chain complete, but RCODE " @@ -8152,8 +8240,8 @@ rctx_answer_positive(respctx_t *rctx) { /* * rctx_answer_scan(): * Perform a single pass over the answer section of a response, looking - * for an answer that matches QNAME/QTYPE, or a CNAME matching QNAME, or a - * covering DNAME. If more than one rdataset is found matching these + * for an answer that matches QNAME/QTYPE, or a CNAME matching QNAME, or + * a covering DNAME. If more than one rdataset is found matching these * criteria, then only one is kept. Order of preference is 1) the * shortest DNAME, 2) the first matching answer, or 3) the first CNAME. */ @@ -8213,8 +8301,9 @@ rctx_answer_scan(respctx_t *rctx) { } /* - * We are looking for the shortest DNAME if there - * are multiple ones (which there shouldn't be). + * We are looking for the shortest DNAME if + * there are multiple ones (which there + * shouldn't be). */ for (rdataset = ISC_LIST_HEAD(name->list); rdataset != NULL; @@ -8236,9 +8325,9 @@ rctx_answer_scan(respctx_t *rctx) { /* * If a DNAME was found, then any CNAME or other answer matching - * QNAME that may also have been found must be ignored. Similarly, - * if a matching answer was found along with a CNAME, the CNAME - * must be ignored. + * QNAME that may also have been found must be ignored. + * Similarly, if a matching answer was found along with a CNAME, + * the CNAME must be ignored. */ if (rctx->dname != NULL) { rctx->aname = NULL; @@ -8497,9 +8586,9 @@ rctx_answer_dname(respctx_t *rctx) { /* * rctx_authority_positive(): * Examine the records in the authority section (if there are any) for a - * positive answer. We expect the names for all rdatasets in this section - * to be subdomains of the domain being queried; any that are not are - * skipped. We expect to find only *one* owner name; any names + * positive answer. We expect the names for all rdatasets in this + * section to be subdomains of the domain being queried; any that are + * not are skipped. We expect to find only *one* owner name; any names * after the first one processed are ignored. We expect to find only * rdatasets of type NS, RRSIG, or SIG; all others are ignored. Whatever * remains can be cached at trust level authauthority or additional @@ -8554,8 +8643,8 @@ rctx_authority_positive(respctx_t *rctx) { rctx->ns_rdataset = rdataset; } /* - * Mark any additional data related - * to this rdataset. + * Mark any additional data + * related to this rdataset. */ (void)dns_rdataset_additionaldata( rdataset, name, check_related, @@ -8587,8 +8676,8 @@ rctx_answer_none(respctx_t *rctx) { rctx_answer_init(rctx); /* - * Sometimes we can tell if its a negative response by looking at - * the message header. + * Sometimes we can tell if its a negative response by looking + * at the message header. */ if (rctx->query->rmessage->rcode == dns_rcode_nxdomain || (rctx->query->rmessage->counts[DNS_SECTION_ANSWER] == 0 && @@ -8908,7 +8997,8 @@ rctx_authority_dnssec(respctx_t *rctx) { } if (!dns_name_issubdomain(name, fctx->domain)) { - /* Invalid name found; preserve it for logging later */ + /* Invalid name found; preserve it for logging + * later */ rctx->found_name = name; rctx->found_type = ISC_LIST_HEAD(name->list)->type; continue; @@ -8947,16 +9037,17 @@ rctx_authority_dnssec(respctx_t *rctx) { rdataset->trust = dns_trust_additional; } /* - * No additional data needs to be marked. + * No additional data needs to be + * marked. */ break; case dns_rdatatype_ds: /* * DS or SIG DS. * - * These should only be here if this is a - * referral, and there should only be one - * DS RRset. + * These should only be here if this is + * a referral, and there should only be + * one DS RRset. */ if (rctx->ns_name == NULL) { log_formerr(fctx, "DS with no " @@ -9127,8 +9218,8 @@ rctx_referral(respctx_t *rctx) { /* * Reinitialize 'rctx' to prepare for following the delegation: - * set the get_nameservers and next_server flags appropriately and - * reset the fetch context counters. + * set the get_nameservers and next_server flags appropriately + * and reset the fetch context counters. * */ if ((rctx->fctx->options & DNS_FETCHOPT_NOFOLLOW) == 0) { @@ -9241,7 +9332,8 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, } if (!dns_name_issubdomain(fname, fctx->domain)) { /* - * The best nameservers are now above our QDOMAIN. + * The best nameservers are now above our + * QDOMAIN. */ FCTXTRACE("nameservers now above QDOMAIN"); fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); @@ -9273,9 +9365,10 @@ rctx_nextserver(respctx_t *rctx, dns_message_t *message, /* * rctx_resend(): * - * Resend the query, probably with the options changed. Calls fctx_query(), - * passing rctx->retryopts (which is based on query->options, but may have been - * updated since the last time fctx_query() was called). + * Resend the query, probably with the options changed. Calls + * fctx_query(), passing rctx->retryopts (which is based on + * query->options, but may have been updated since the last time + * fctx_query() was called). */ static void rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) { @@ -9305,12 +9398,12 @@ rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) { /* * rctx_next(): - * We got what appeared to be a response but it didn't match the question - * or the cookie; it may have been meant for someone else, or it may be a - * spoofing attack. Drop it and continue listening for the response we - * wanted. + * We got what appeared to be a response but it didn't match the + * question or the cookie; it may have been meant for someone else, or + * it may be a spoofing attack. Drop it and continue listening for the + * response we wanted. */ -static void +static isc_result_t rctx_next(respctx_t *rctx) { #ifdef WANT_QUERYTRACE fetchctx_t *fctx = rctx->fctx; @@ -9325,11 +9418,14 @@ rctx_next(respctx_t *rctx) { if (result != ISC_R_SUCCESS) { fctx_done(rctx->fctx, result, __LINE__); } + + return (result); } /* * rctx_chaseds(): - * Look up the parent zone's NS records so that DS records can be fetched. + * Look up the parent zone's NS records so that DS records can be + * fetched. */ static void rctx_chaseds(respctx_t *rctx, dns_message_t *message, @@ -9363,10 +9459,11 @@ rctx_chaseds(respctx_t *rctx, dns_message_t *message, /* * rctx_done(): - * This resolver query response is finished, either because we encountered - * a problem or because we've gotten all the information from it that we - * can. We either wait for another response, resend the query to the - * same server, resend to a new server, or clean up and shut down the fetch. + * This resolver query response is finished, either because we + * encountered a problem or because we've gotten all the information + * from it that we can. We either wait for another response, resend the + * query to the same server, resend to a new server, or clean up and + * shut down the fetch. */ static void rctx_done(respctx_t *rctx, isc_result_t result) { @@ -9390,7 +9487,7 @@ rctx_done(respctx_t *rctx, isc_result_t result) { * Cancel the query. */ if (!rctx->nextitem) { - fctx_cancelquery(&query, &rctx->devent, rctx->finish, + fctx_cancelquery(query, &rctx->devent, rctx->finish, rctx->no_response, false); } @@ -9399,7 +9496,7 @@ rctx_done(respctx_t *rctx, isc_result_t result) { (rctx->next_server || rctx->resend || rctx->nextitem)) { if (rctx->nextitem) { - fctx_cancelquery(&query, &rctx->devent, rctx->finish, + fctx_cancelquery(query, &rctx->devent, rctx->finish, rctx->no_response, false); } fctx_done(fctx, DNS_R_SERVFAIL, __LINE__); @@ -9412,7 +9509,9 @@ rctx_done(respctx_t *rctx, isc_result_t result) { } else if (rctx->resend) { rctx_resend(rctx, addrinfo); } else if (rctx->nextitem) { - rctx_next(rctx); + if (rctx_next(rctx) != ISC_R_SUCCESS) { + resquery_detach(&query); + } } else if (result == DNS_R_CHASEDSSERVERS) { rctx_chaseds(rctx, message, addrinfo, result); } else if (result == ISC_R_SUCCESS && !HAVE_ANSWER(fctx)) { @@ -9759,15 +9858,11 @@ static void empty_bucket(dns_resolver_t *res) { RTRACE("empty_bucket"); - LOCK(&res->lock); - - INSIST(res->activebuckets > 0); - res->activebuckets--; - if (res->activebuckets == 0) { + if (isc_refcount_decrement(&res->activebuckets) == 1) { + LOCK(&res->lock); send_shutdown_events(res); + UNLOCK(&res->lock); } - - UNLOCK(&res->lock); } static void @@ -9847,10 +9942,11 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, .maxdepth = DEFAULT_RECURSION_DEPTH, .maxqueries = DEFAULT_MAX_QUERIES, .nbuckets = ntasks, - .activebuckets = ntasks, .querydscp4 = -1, .querydscp6 = -1 }; + atomic_init(&res->activebuckets, ntasks); + res->quotaresp[dns_quotatype_zone] = DNS_R_DROP; res->quotaresp[dns_quotatype_server] = DNS_R_SERVFAIL; isc_refcount_init(&res->references, 1); @@ -9877,8 +9973,8 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr, isc_mutex_init(&res->buckets[i].lock); /* - * Since we have a pool of tasks we bind them to task queues - * to spread the load evenly + * Since we have a pool of tasks we bind them to task + * queues to spread the load evenly */ res->buckets[i].task = NULL; result = isc_task_create_bound(taskmgr, 0, @@ -10048,12 +10144,12 @@ dns_resolver_prime(dns_resolver_t *res) { if (want_priming) { /* * To avoid any possible recursive locking problems, we - * start the priming fetch like any other fetch, and holding - * no resolver locks. No one else will try to start it - * because we're the ones who set res->priming to true. - * Any other callers of dns_resolver_prime() while we're - * running will see that res->priming is already true and - * do nothing. + * start the priming fetch like any other fetch, and + * holding no resolver locks. No one else will try to + * start it because we're the ones who set res->priming + * to true. Any other callers of dns_resolver_prime() + * while we're running will see that res->priming is + * already true and do nothing. */ RTRACE("priming"); rdataset = isc_mem_get(res->mctx, sizeof(*rdataset)); @@ -10117,7 +10213,9 @@ dns_resolver_whenshutdown(dns_resolver_t *res, isc_task_t *task, LOCK(&res->lock); - if (atomic_load_acquire(&res->exiting) && res->activebuckets == 0) { + if (atomic_load_acquire(&res->exiting) && + atomic_load_acquire(&res->activebuckets) == 0) + { /* * We're already shutdown. Send the event. */ @@ -10139,6 +10237,7 @@ dns_resolver_shutdown(dns_resolver_t *res) { fetchctx_t *fctx; isc_result_t result; bool is_false = false; + bool is_done = false; REQUIRE(VALID_RESOLVER(res)); @@ -10157,12 +10256,14 @@ dns_resolver_shutdown(dns_resolver_t *res) { } atomic_store(&res->buckets[i].exiting, true); if (ISC_LIST_EMPTY(res->buckets[i].fctxs)) { - INSIST(res->activebuckets > 0); - res->activebuckets--; + if (isc_refcount_decrement( + &res->activebuckets) == 1) { + is_done = true; + } } UNLOCK(&res->buckets[i].lock); } - if (res->activebuckets == 0) { + if (is_done) { send_shutdown_events(res); } result = isc_timer_reset(res->spillattimer, @@ -10185,10 +10286,8 @@ dns_resolver_detach(dns_resolver_t **resp) { RTRACE("detach"); if (isc_refcount_decrement(&res->references) == 1) { - LOCK(&res->lock); + isc_refcount_destroy(&res->activebuckets); INSIST(atomic_load_acquire(&res->exiting)); - INSIST(res->activebuckets == 0); - UNLOCK(&res->lock); destroy(res); } } @@ -10255,7 +10354,8 @@ fctx_minimize_qname(fetchctx_t *fctx) { * boundaries at /16, /32, /48, /56, /64 and /128 * In 'label count' terms that's equal to * 7 11 15 17 19 35 - * We fix fctx->qmin_labels to point to the nearest boundary + * We fix fctx->qmin_labels to point to the nearest + * boundary */ if (fctx->qmin_labels < 7) { fctx->qmin_labels = 7; @@ -10452,8 +10552,9 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name, isc_task_send(res->buckets[bucketnum].task, &event); } else { /* - * We don't care about the result of fctx_unlink() - * since we know we're not exiting. + * We don't care about the result of + * fctx_unlink() since we know we're not + * exiting. */ (void)fctx_unlink(fctx); dodestroy = true; @@ -10787,7 +10888,8 @@ dns_resolver_disable_algorithm(dns_resolver_t *resolver, const dns_name_t *name, memmove(tmp, algorithms, *algorithms); } tmp[len - 1] |= mask; - /* 'tmp[0]' should contain the length of 'tmp'. */ + /* 'tmp[0]' should contain the length of 'tmp'. + */ *tmp = len; node->data = tmp; /* Free the older bitfield. */ @@ -11155,7 +11257,9 @@ dns_resolver_dumpfetches(dns_resolver_t *resolver, isc_statsformat_t format, fc = ISC_LIST_NEXT(fc, link)) { dns_name_print(fc->domain, fp); - fprintf(fp, ": %u active (%u spilled, %u allowed)\n", + fprintf(fp, + ": %u active (%u spilled, %u " + "allowed)\n", fc->count, fc->dropped, fc->allowed); } UNLOCK(&resolver->dbuckets[i].lock); diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 65a2f3d441..b8d083b687 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -168,7 +168,7 @@ reset(void) { dns_dispatchset_destroy(&dset); } if (dispatchmgr != NULL) { - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } } @@ -393,7 +393,7 @@ dispatch_getnext(void **state) { * Shutdown the dispatch. */ dns_dispatch_detach(&dispatch); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } int diff --git a/lib/dns/tests/resolver_test.c b/lib/dns/tests/resolver_test.c index 2989af3041..2c8b6f4e9a 100644 --- a/lib/dns/tests/resolver_test.c +++ b/lib/dns/tests/resolver_test.c @@ -70,7 +70,7 @@ _teardown(void **state) { dns_dispatch_detach(&dispatch); dns_view_detach(&view); - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); dns_test_end(); return (0); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 9b26c61ffe..d2e1b75bbf 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -14011,8 +14011,8 @@ refresh_callback(isc_task_t *task, isc_event_t *event) { result = dns_request_getresponse(revent->request, msg, 0); if (result != ISC_R_SUCCESS) { dns_zone_log(zone, ISC_LOG_INFO, - "refresh: failure trying master " - "%s (source %s): %s", + "refresh: unable to get response, master " + "%s, source %s: %s", master, source, dns_result_totext(result)); goto next_master; } diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 8ece586163..8f49e53047 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -165,7 +165,7 @@ shutdown_managers(isc_task_t *task, isc_event_t *event) { } if (dispatchmgr != NULL) { - dns_dispatchmgr_destroy(&dispatchmgr); + dns_dispatchmgr_detach(&dispatchmgr); } atomic_store(&shutdown_done, true);