From 4f30b679e7c561f73ff6824fbdea09521748a61b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 18 Dec 2020 14:59:50 -0800 Subject: [PATCH] Creating TCP dispatch now creates/binds the socket Previously, creation of TCP dispatches differed from UDP in that a TCP dispatch was created to attach to an existing socket, whereas a UDP dispatch would be created in a vacuum and sockets would be opened on demand when a transaction was initiated. We are moving as much socket code as possible into the dispatch module, so that it can be replaced with a netmgr version as easily as possible. (This will also have the side effect of making TCP and UDP dispatches more similar.) As a step in that direction, this commit changes dns_dispatch_createtcp() so that it creates the TCP socket. --- lib/dns/dispatch.c | 178 +++++++++++++++------------------ lib/dns/include/dns/dispatch.h | 4 +- lib/dns/request.c | 31 +----- lib/dns/resolver.c | 128 +++++++----------------- 4 files changed, 125 insertions(+), 216 deletions(-) diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index cbb84220ee..6cdd138a7a 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -631,6 +631,7 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest, continue; } UNLOCK(&qid->lock); + result = open_socket(sockmgr, &localaddr, ISC_SOCKET_REUSEADDRESS, &sock); if (result == ISC_R_SUCCESS) { @@ -1652,8 +1653,9 @@ qid_destroy(isc_mem_t *mctx, dns_qid_t **qidp) { /* * Allocate and set important limits. */ -static isc_result_t -dispatch_allocate(dns_dispatchmgr_t *mgr, dns_dispatch_t **dispp) { +static void +dispatch_allocate(dns_dispatchmgr_t *mgr, isc_sockettype_t type, int pf, + unsigned int attributes, dns_dispatch_t **dispp) { dns_dispatch_t *disp = NULL; REQUIRE(VALID_DISPATCHMGR(mgr)); @@ -1668,18 +1670,50 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, dns_dispatch_t **dispp) { isc_refcount_increment0(&mgr->irefs); *disp = (dns_dispatch_t){ .mgr = mgr, + .socktype = type, .shutdown_why = ISC_R_UNEXPECTED }; isc_refcount_init(&disp->refcount, 1); ISC_LINK_INIT(disp, link); ISC_LIST_INIT(disp->activesockets); ISC_LIST_INIT(disp->inactivesockets); + switch (type) { + case isc_sockettype_tcp: + disp->attributes |= DNS_DISPATCHATTR_TCP; + break; + case isc_sockettype_udp: + disp->attributes |= DNS_DISPATCHATTR_UDP; + break; + default: + INSIST(0); + ISC_UNREACHABLE(); + } + + switch (pf) { + case PF_INET: + disp->attributes |= DNS_DISPATCHATTR_IPV4; + break; + case PF_INET6: + disp->attributes |= DNS_DISPATCHATTR_IPV6; + break; + default: + INSIST(0); + ISC_UNREACHABLE(); + } + + /* + * Set whatever attributes were passed in that haven't been + * reset automatically by the code above. + */ + attributes &= ~(DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP | + DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6); + disp->attributes |= attributes; + isc_mutex_init(&disp->lock); disp->failsafe_ev = allocate_devent(disp); disp->magic = DISPATCH_MAGIC; *dispp = disp; - return (ISC_R_SUCCESS); } /* @@ -1720,30 +1754,38 @@ dispatch_free(dns_dispatch_t **dispp) { } isc_result_t -dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock, +dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, const isc_sockaddr_t *destaddr, unsigned int attributes, - dns_dispatch_t **dispp) { + isc_dscp_t dscp, dns_dispatch_t **dispp) { isc_result_t result; dns_dispatch_t *disp = NULL; + isc_sockaddr_t src; int pf; REQUIRE(VALID_DISPATCHMGR(mgr)); - REQUIRE(isc_socket_gettype(sock) == isc_sockettype_tcp); - - attributes |= DNS_DISPATCHATTR_TCP; - attributes &= ~DNS_DISPATCHATTR_UDP; + REQUIRE(sockmgr != NULL); + REQUIRE(destaddr != NULL); LOCK(&mgr->lock); - result = dispatch_allocate(mgr, &disp); - if (result != ISC_R_SUCCESS) { - UNLOCK(&mgr->lock); - return (result); - } + pf = isc_sockaddr_pf(destaddr); + dispatch_allocate(mgr, isc_sockettype_tcp, pf, attributes, &disp); - disp->socktype = isc_sockettype_tcp; - isc_socket_attach(sock, &disp->socket); + disp->peer = *destaddr; + + if (localaddr != NULL) { + disp->local = *localaddr; + } else { + switch (pf) { + case AF_INET: + isc_sockaddr_any(&disp->local); + break; + case AF_INET6: + isc_sockaddr_any6(&disp->local); + break; + } + } disp->ntasks = 1; disp->task[0] = NULL; @@ -1752,6 +1794,26 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock, goto cleanup; } + result = isc_socket_create(sockmgr, isc_sockaddr_pf(destaddr), + isc_sockettype_tcp, &disp->socket); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + if (localaddr == NULL) { + isc_sockaddr_anyofpf(&src, pf); + } else { + src = *localaddr; + isc_sockaddr_setport(&src, 0); + } + + result = isc_socket_bind(disp->socket, &src, 0); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + isc_socket_dscp(disp->socket, dscp); + disp->ctlevent = isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL, destroy_disp, disp, sizeof(isc_event_t)); @@ -1761,48 +1823,6 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock, dns_tcpmsg_init(mgr->mctx, disp->socket, &disp->tcpmsg); disp->tcpmsg_valid = 1; - if (destaddr == NULL) { - (void)isc_socket_getpeername(sock, &disp->peer); - attributes |= DNS_DISPATCHATTR_PRIVATE; - } else { - disp->peer = *destaddr; - } - - pf = isc_sockaddr_pf(&disp->peer); - - if (localaddr == NULL) { - if (destaddr != NULL) { - switch (pf) { - case AF_INET: - isc_sockaddr_any(&disp->local); - break; - case AF_INET6: - isc_sockaddr_any6(&disp->local); - break; - } - } else { - (void)isc_socket_getsockname(sock, &disp->local); - } - } else { - disp->local = *localaddr; - } - - switch (pf) { - case PF_INET: - attributes |= DNS_DISPATCHATTR_IPV4; - break; - - case PF_INET6: - attributes |= DNS_DISPATCHATTR_IPV6; - break; - - default: - result = ISC_R_NOTIMPLEMENTED; - goto cleanup; - } - - disp->attributes = attributes; - /* * Append it to the dispatcher list. */ @@ -1908,6 +1928,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, disp = ISC_LIST_NEXT(disp, link); } UNLOCK(&mgr->lock); + return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND); } @@ -1924,43 +1945,29 @@ dns_dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, REQUIRE(taskmgr != NULL); REQUIRE(dispp != NULL && *dispp == NULL); - attributes |= DNS_DISPATCHATTR_UDP; - attributes &= ~DNS_DISPATCHATTR_TCP; - LOCK(&mgr->lock); result = dispatch_createudp(mgr, sockmgr, taskmgr, localaddr, attributes, &disp); - if (result == ISC_R_SUCCESS) { *dispp = disp; } - UNLOCK(&mgr->lock); - return (ISC_R_SUCCESS); + + return (result); } static isc_result_t dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, unsigned int attributes, dns_dispatch_t **dispp) { - isc_result_t result; + isc_result_t result = ISC_R_SUCCESS; dns_dispatch_t *disp = NULL; isc_socket_t *sock = NULL; isc_sockaddr_t sa_any; int pf, i = 0; - /* - * dispatch_allocate() checks mgr for us. - */ - disp = NULL; - result = dispatch_allocate(mgr, &disp); - if (result != ISC_R_SUCCESS) { - return (result); - } - pf = isc_sockaddr_pf(localaddr); - - disp->socktype = isc_sockettype_udp; + dispatch_allocate(mgr, isc_sockettype_udp, pf, attributes, &disp); /* * For dispatches with a specified source address, we open a @@ -2014,25 +2021,6 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, isc_mem_create(&disp->sepool); isc_mem_setname(disp->sepool, "disp_sepool"); - attributes &= ~DNS_DISPATCHATTR_TCP; - attributes |= DNS_DISPATCHATTR_UDP; - - switch (pf) { - case PF_INET: - attributes |= DNS_DISPATCHATTR_IPV4; - break; - - case PF_INET6: - attributes |= DNS_DISPATCHATTR_IPV6; - break; - - default: - result = ISC_R_NOTIMPLEMENTED; - goto kill_socket; - } - - disp->attributes = attributes; - /* * Append it to the dispatcher list. */ diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index c9bdc42954..69e0d81c62 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -223,10 +223,10 @@ dns_dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, */ isc_result_t -dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock, +dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr, isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr, const isc_sockaddr_t *destaddr, unsigned int attributes, - dns_dispatch_t **dispp); + isc_dscp_t dscp, dns_dispatch_t **dispp); /*%< * Create a new dns_dispatch and attach it to the provided isc_socket_t. * diff --git a/lib/dns/request.c b/lib/dns/request.c index fed16798ee..960ff04d95 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -498,9 +498,6 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr, isc_dscp_t dscp, bool *connected, dns_dispatch_t **dispatchp) { isc_result_t result; - isc_socket_t *sock = NULL; - isc_sockaddr_t src; - isc_sockaddr_t bind_any; if (!newtcp) { result = dns_dispatch_gettcp(requestmgr->dispatchmgr, destaddr, @@ -517,31 +514,9 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr, } } - result = isc_socket_create(requestmgr->socketmgr, - isc_sockaddr_pf(destaddr), - isc_sockettype_tcp, &sock); - if (result != ISC_R_SUCCESS) { - return (result); - } - if (srcaddr == NULL) { - isc_sockaddr_anyofpf(&bind_any, isc_sockaddr_pf(destaddr)); - result = isc_socket_bind(sock, &bind_any, 0); - } else { - src = *srcaddr; - isc_sockaddr_setport(&src, 0); - result = isc_socket_bind(sock, &src, 0); - } - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - isc_socket_dscp(sock, dscp); - result = dns_dispatch_createtcp(requestmgr->dispatchmgr, sock, - requestmgr->taskmgr, srcaddr, destaddr, - 0, dispatchp); - -cleanup: - isc_socket_detach(&sock); + result = dns_dispatch_createtcp( + requestmgr->dispatchmgr, requestmgr->socketmgr, + requestmgr->taskmgr, srcaddr, destaddr, 0, dscp, dispatchp); return (result); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 9a39175319..5d156e5edc 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -228,7 +228,6 @@ typedef struct query { dns_dispatchmgr_t *dispatchmgr; dns_dispatch_t *dispatch; dns_adbaddrinfo_t *addrinfo; - isc_socket_t *tcpsocket; isc_time_t start; dns_messageid_t id; dns_dispentry_t *dispentry; @@ -1200,7 +1199,7 @@ resquery_destroy(resquery_t **queryp) { *queryp = NULL; REQUIRE(!ISC_LINK_LINKED(query, link)); - INSIST(query->tcpsocket == NULL); + INSIST(query->dispatch == NULL); fctx = query->fctx; res = fctx->res; @@ -1245,13 +1244,13 @@ update_edns_stats(resquery_t *query) { static void fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, isc_time_t *finish, bool no_response, bool age_untried) { - fetchctx_t *fctx; - resquery_t *query; + fetchctx_t *fctx = NULL; + resquery_t *query = NULL; unsigned int rtt, rttms; unsigned int factor; - dns_adbfind_t *find; + dns_adbfind_t *find = NULL; + isc_socket_t *sock = NULL; dns_adbaddrinfo_t *addrinfo; - isc_socket_t *sock; isc_stdtime_t now; query = *queryp; @@ -1423,33 +1422,20 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp, * only needs to worry about managing the connect and send events; * the dispatcher manages the recv events. */ - if (RESQUERY_CONNECTING(query)) { - /* - * Cancel the connect. - */ - if (query->tcpsocket != NULL) { - isc_socket_cancel(query->tcpsocket, NULL, - ISC_SOCKCANCEL_CONNECT); - } else if (query->dispentry != NULL) { - sock = dns_dispatch_getentrysocket(query->dispentry); - if (sock != NULL) { - isc_socket_cancel(sock, NULL, - ISC_SOCKCANCEL_CONNECT); - } - } + if (query->dispentry != NULL) { + sock = dns_dispatch_getentrysocket(query->dispentry); + } else { + sock = dns_dispatch_getsocket(query->dispatch); } - if (RESQUERY_SENDING(query)) { - /* - * Cancel the pending send. - */ - if (query->dispentry != NULL) { - sock = dns_dispatch_getentrysocket(query->dispentry); - } else { - sock = dns_dispatch_getsocket(query->dispatch); - } - if (sock != NULL) { - isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND); - } + + /* Cancel the connect. */ + if (sock != NULL && RESQUERY_CONNECTING(query)) { + isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_CONNECT); + } + + /* Cancel the pending send. */ + if (sock != NULL && RESQUERY_SENDING(query)) { + isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND); } if (query->dispentry != NULL) { @@ -1829,19 +1815,12 @@ process_sendevent(resquery_t *query, isc_event_t *event) { bool destroy_query = false; bool retry = false; isc_result_t result; - fetchctx_t *fctx; + fetchctx_t *fctx = NULL; fctx = query->fctx; if (RESQUERY_CANCELED(query)) { if (query->sends == 0 && query->connects == 0) { - /* - * This query was canceled while the - * isc_socket_sendto/connect() was in progress. - */ - if (query->tcpsocket != NULL) { - isc_socket_detach(&query->tcpsocket); - } destroy_query = true; } } else { @@ -2007,10 +1986,10 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) { static isc_result_t fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, unsigned int options) { - dns_resolver_t *res; - isc_task_t *task; + dns_resolver_t *res = NULL; + isc_task_t *task = NULL; isc_result_t result; - resquery_t *query; + resquery_t *query = NULL; isc_sockaddr_t addr; bool have_addr = false; unsigned int srtt; @@ -2123,21 +2102,12 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, query->dscp = dscp; } - result = isc_socket_create(res->socketmgr, pf, - isc_sockettype_tcp, - &query->tcpsocket); + result = dns_dispatch_createtcp( + res->dispatchmgr, res->socketmgr, res->taskmgr, &addr, + &addrinfo->sockaddr, 0, query->dscp, &query->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup_query; } - - result = isc_socket_bind(query->tcpsocket, &addr, 0); - if (result != ISC_R_SUCCESS) { - goto cleanup_socket; - } - - /* - * A dispatch will be created once the connect succeeds. - */ } else { if (have_addr) { switch (isc_sockaddr_pf(&addr)) { @@ -2195,19 +2165,16 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, query->magic = QUERY_MAGIC; if ((query->options & DNS_FETCHOPT_TCP) != 0) { + isc_socket_t *sock = NULL; + /* * Connect to the remote server. - * - * XXXRTH Should we attach to the socket? */ - if (query->dscp != -1) { - isc_socket_dscp(query->tcpsocket, query->dscp); - } - result = isc_socket_connect(query->tcpsocket, - &addrinfo->sockaddr, task, + sock = dns_dispatch_getsocket(query->dispatch); + result = isc_socket_connect(sock, &addrinfo->sockaddr, task, resquery_connected, query); if (result != ISC_R_SUCCESS) { - goto cleanup_socket; + goto cleanup_dispatch; } query->connects++; QTRACE("connecting via TCP"); @@ -2244,9 +2211,6 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, return (ISC_R_SUCCESS); -cleanup_socket: - isc_socket_detach(&query->tcpsocket); - cleanup_dispatch: if (query->dispatch != NULL) { dns_dispatch_detach(&query->dispatch); @@ -2961,11 +2925,8 @@ resquery_connected(isc_task_t *task, isc_event_t *event) { * This query was canceled while the connect() was in * progress. */ - isc_socket_detach(&query->tcpsocket); resquery_destroy(&query); } else { - int attrs = 0; - switch (sevent->result) { case ISC_R_SUCCESS: @@ -2989,27 +2950,16 @@ resquery_connected(isc_task_t *task, isc_event_t *event) { fctx_done(fctx, result, __LINE__); break; } + /* - * We are connected. Create a dispatcher and + * We are connected. Update the dispatcher and * send the query. */ - attrs = DNS_DISPATCHATTR_CONNECTED; - result = dns_dispatch_createtcp( - query->dispatchmgr, query->tcpsocket, - query->fctx->res->taskmgr, NULL, NULL, attrs, - &query->dispatch); - - /* - * Regardless of whether dns_dispatch_create() - * succeeded or not, we don't need our reference - * to the socket anymore. - */ - isc_socket_detach(&query->tcpsocket); - - if (result == ISC_R_SUCCESS) { - result = resquery_send(query); - } + 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"); @@ -3030,10 +2980,6 @@ resquery_connected(isc_task_t *task, isc_event_t *event) { "no route to host; no response", sevent->result); - /* - * No route to remote. - */ - isc_socket_detach(&query->tcpsocket); /* * Do not query this server again in this fetch context * if the server is unavailable over TCP. @@ -3049,7 +2995,7 @@ resquery_connected(isc_task_t *task, isc_event_t *event) { "unexpected event result; responding", sevent->result); - isc_socket_detach(&query->tcpsocket); + dns_dispatch_detach(&query->dispatch); fctx_cancelquery(&query, NULL, NULL, false, false); break; }