diff --git a/bin/named/main.c b/bin/named/main.c index 98fd1aa3e9..f3b2ec80f5 100644 --- a/bin/named/main.c +++ b/bin/named/main.c @@ -122,6 +122,7 @@ extern unsigned int dns_zone_mkey_month; extern unsigned int dns_adb_entrywindow; extern unsigned int dns_adb_cachemin; +extern size_t dns_dispatch_tcppipelining; static bool want_stats = false; static char program_name[NAME_MAX] = "named"; @@ -809,6 +810,13 @@ parse_T_opt(char *option) { dns_adb_entrywindow = atoi(option + 15); } else if (!strncmp(option, "adbcachemin=", 12)) { dns_adb_cachemin = atoi(option + 12); + } else if (!strncmp(option, "tcppipelining=", 14)) { + size_t pipelining = atoi(option + 14); + if (pipelining < 1) { + named_main_earlyfatal("tcppipelining must be at " + "least 1"); + } + dns_dispatch_tcppipelining = pipelining; } else { fprintf(stderr, "unknown -T flag '%s'\n", option); } diff --git a/bin/tests/system/masterformat/ns2/named.args b/bin/tests/system/masterformat/ns2/named.args new file mode 100644 index 0000000000..938e73b772 --- /dev/null +++ b/bin/tests/system/masterformat/ns2/named.args @@ -0,0 +1 @@ +-D masterformat-ns2 -m record -c named.conf -d 99 -g -T maxcachesize=2097152 -T tcppipelining=1 diff --git a/bin/tests/system/tcp/ns2/named.args b/bin/tests/system/tcp/ns2/named.args new file mode 100644 index 0000000000..4934c4fa22 --- /dev/null +++ b/bin/tests/system/tcp/ns2/named.args @@ -0,0 +1 @@ +-D tcp-ns2 -m record -c named.conf -d 99 -g -T maxcachesize=2097152 -T tcppipelining=1 diff --git a/bin/tests/system/tcp/ns3/named.args b/bin/tests/system/tcp/ns3/named.args new file mode 100644 index 0000000000..dce0bc7366 --- /dev/null +++ b/bin/tests/system/tcp/ns3/named.args @@ -0,0 +1 @@ +-D tcp-ns3 -m record -c named.conf -d 99 -g -T maxcachesize=2097152 -T tcppipelining=1 diff --git a/bin/tests/system/tcp/ns4/named.args b/bin/tests/system/tcp/ns4/named.args new file mode 100644 index 0000000000..06e4295518 --- /dev/null +++ b/bin/tests/system/tcp/ns4/named.args @@ -0,0 +1 @@ +-D tcp-ns4 -m record -c named.conf -d 99 -g -T maxcachesize=2097152 -T tcppipelining=1 diff --git a/bin/tests/system/tcp/ns5/named.args b/bin/tests/system/tcp/ns5/named.args new file mode 100644 index 0000000000..a804c55c3f --- /dev/null +++ b/bin/tests/system/tcp/ns5/named.args @@ -0,0 +1 @@ +-D tcp-ns5 -m record -c named.conf -d 99 -g -T maxcachesize=2097152 -T tcppipelining=1 diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index 5ff255b655..2755652a6c 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -45,6 +45,13 @@ #include #include +/* + * Maximum number of queries to pipeline on a single shared TCP dispatch. + * Once reached, the dispatch is removed from the hash table so new queries + * get a fresh connection. Can be overridden via 'named -T tcppipelining=N'. + */ +size_t dns_dispatch_tcppipelining = 256; + typedef ISC_LIST(dns_dispentry_t) dns_displist_t; struct dns_dispatchmgr { @@ -121,6 +128,9 @@ struct dns_dispatch { dns_dispatchopt_t options; dns_dispatchstate_t state; + dns_dispatchtype_t disptype; + + dns_messageid_t nextid; /*%< next sequential QID for TCP */ bool reading; @@ -640,12 +650,9 @@ tcp_recv_oldest(dns_dispatch_t *disp, dns_dispentry_t **respp) { return ISC_R_NOTFOUND; } -/* - * NOTE: Must be RCU read locked! - */ static isc_result_t tcp_recv_success(dns_dispatch_t *disp, isc_region_t *region, - isc_sockaddr_t *peer, dns_dispentry_t **respp) { + dns_dispentry_t **respp) { isc_buffer_t source; dns_messageid_t id; unsigned int flags; @@ -681,37 +688,24 @@ tcp_recv_success(dns_dispatch_t *disp, isc_region_t *region, } /* - * We have a valid response; find the associated dispentry object - * and call the caller back. + * We have a valid response; find the associated dispentry by + * scanning disp->active. With sequential IDs and a bounded + * pipelining limit this is a short linear scan. */ - dns_dispentry_t key = { - .id = id, - .peer = *peer, - .port = isc_sockaddr_getport(&disp->local), - }; - struct cds_lfht_iter iter; - cds_lfht_lookup(disp->mgr->qids, qid_hash(&key), qid_match, &key, - &iter); - - dns_dispentry_t *resp = cds_lfht_entry(cds_lfht_iter_get_node(&iter), - dns_dispentry_t, ht_node); - - /* Skip responses that are not ours */ - if (resp != NULL && resp->disp == disp) { - if (!resp->reading) { - /* - * We already got a message for this QID and weren't - * expecting any more. - */ - result = ISC_R_UNEXPECTED; - } else { - *respp = resp; + dns_dispentry_t *resp = NULL, *r = NULL; + ISC_LIST_FOREACH(disp->active, r, alink) { + if (r->id == id) { + resp = r; + break; } + } + + if (resp != NULL) { + *respp = resp; } else { result = ISC_R_NOTFOUND; } - dispatch_log(disp, ISC_LOG_DEBUG(90), - "search for response in hashtable: %s", + dispatch_log(disp, ISC_LOG_DEBUG(90), "search for response: %s", isc_result_totext(result)); return result; @@ -807,7 +801,7 @@ tcp_recv(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region, break; case ISC_R_SUCCESS: /* We got an answer */ - result = tcp_recv_success(disp, region, &peer, &resp); + result = tcp_recv_success(disp, region, &resp); break; default: @@ -1138,16 +1132,26 @@ struct dispatch_key { const isc_sockaddr_t *local; const isc_sockaddr_t *peer; const dns_transport_t *transport; + const dns_dispatchtype_t disptype; }; static uint32_t dispatch_hash(struct dispatch_key *key) { - uint32_t hashval = isc_sockaddr_hash(key->peer, false); - if (key->local) { - hashval ^= isc_sockaddr_hash(key->local, true); - } + isc_hash32_t hash; - return hashval; + isc_hash32_init(&hash); + + isc_sockaddr_hash_ex(&hash, key->peer, false); + if (key->local != NULL) { + isc_sockaddr_hash_ex(&hash, key->local, true); + } + if (key->transport != NULL) { + uintptr_t transport = (uintptr_t)key->transport; + isc_hash32_hash(&hash, &transport, sizeof(transport), true); + } + isc_hash32_hash(&hash, &key->disptype, sizeof(key->disptype), true); + + return isc_hash32_finalize(&hash); } static int @@ -1165,75 +1169,16 @@ dispatch_match(struct cds_lfht_node *node, const void *key0) { peer = disp->peer; } - return isc_sockaddr_equal(&peer, key->peer) && + return disp->disptype == key->disptype && + isc_sockaddr_equal(&peer, key->peer) && disp->transport == key->transport && (key->local == NULL || isc_sockaddr_equal(&local, key->local)); } -isc_result_t -dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, - const isc_sockaddr_t *destaddr, - dns_transport_t *transport, dns_dispatchopt_t options, - dns_dispatch_t **dispp) { - dns_dispatch_t *disp = NULL; - uint32_t tid = isc_tid(); - - REQUIRE(VALID_DISPATCHMGR(mgr)); - REQUIRE(destaddr != NULL); - - dispatch_allocate(mgr, isc_socktype_tcp, tid, &disp); - - disp->options = options; - disp->peer = *destaddr; - if (transport != NULL) { - dns_transport_attach(transport, &disp->transport); - } - - if (localaddr != NULL) { - disp->local = *localaddr; - } else { - int pf; - pf = isc_sockaddr_pf(destaddr); - isc_sockaddr_anyofpf(&disp->local, pf); - isc_sockaddr_setport(&disp->local, 0); - } - - /* - * Append it to the dispatcher list. - */ - struct dispatch_key key = { - .local = &disp->local, - .peer = &disp->peer, - .transport = transport, - }; - - if ((disp->options & DNS_DISPATCHOPT_UNSHARED) == 0) { - rcu_read_lock(); - cds_lfht_add(mgr->tcps[tid], dispatch_hash(&key), - &disp->ht_node); - rcu_read_unlock(); - } - - if (isc_log_wouldlog(dns_lctx, 90)) { - char addrbuf[ISC_SOCKADDR_FORMATSIZE]; - - isc_sockaddr_format(&disp->local, addrbuf, - ISC_SOCKADDR_FORMATSIZE); - - mgr_log(mgr, ISC_LOG_DEBUG(90), - "dns_dispatch_createtcp: created TCP dispatch %p for " - "%s", - disp, addrbuf); - } - *dispp = disp; - - return ISC_R_SUCCESS; -} - -isc_result_t -dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, - const isc_sockaddr_t *localaddr, dns_transport_t *transport, - dns_dispatch_t **dispp) { +static isc_result_t +dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, + const isc_sockaddr_t *destaddr, dns_transport_t *transport, + dns_dispatchtype_t disptype, dns_dispatch_t **dispp) { dns_dispatch_t *disp_connected = NULL; dns_dispatch_t *disp_fallback = NULL; isc_result_t result = ISC_R_NOTFOUND; @@ -1247,6 +1192,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, .local = localaddr, .peer = destaddr, .transport = transport, + .disptype = disptype, }; rcu_read_lock(); @@ -1263,18 +1209,10 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, /* A dispatch in indeterminate state, skip it */ break; case DNS_DISPATCHSTATE_CONNECTED: - if (ISC_LIST_EMPTY(disp->active)) { - /* Ignore dispatch with no responses */ - break; - } /* We found a connected dispatch */ dns_dispatch_attach(disp, &disp_connected); break; case DNS_DISPATCHSTATE_CONNECTING: - if (ISC_LIST_EMPTY(disp->pending)) { - /* Ignore dispatch with no responses */ - break; - } /* We found "a" dispatch, store it for later */ if (disp_fallback == NULL) { dns_dispatch_attach(disp, &disp_fallback); @@ -1314,6 +1252,106 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, return result; } +static void +dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, + const isc_sockaddr_t *destaddr, dns_transport_t *transport, + dns_dispatchtype_t disptype, dns_dispatchopt_t options, + dns_dispatch_t **dispp) { + dns_dispatch_t *disp = NULL; + uint32_t tid = isc_tid(); + + dispatch_allocate(mgr, isc_socktype_tcp, tid, &disp); + + disp->disptype = disptype; + disp->nextid = isc_random16(); + disp->options = options; + disp->peer = *destaddr; + if (transport != NULL) { + dns_transport_attach(transport, &disp->transport); + } + + if (localaddr != NULL) { + disp->local = *localaddr; + } else { + int pf; + pf = isc_sockaddr_pf(destaddr); + isc_sockaddr_anyofpf(&disp->local, pf); + isc_sockaddr_setport(&disp->local, 0); + } + + /* + * Append it to the dispatcher list. + */ + if ((options & DNS_DISPATCHOPT_FIXEDID) == 0) { + struct dispatch_key key = { + .local = &disp->local, + .peer = &disp->peer, + .transport = transport, + .disptype = disptype, + }; + rcu_read_lock(); + cds_lfht_add(mgr->tcps[tid], dispatch_hash(&key), + &disp->ht_node); + rcu_read_unlock(); + } + + *dispp = disp; +} + +isc_result_t +dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, + const isc_sockaddr_t *destaddr, + dns_transport_t *transport, dns_dispatchtype_t disptype, + dns_dispatchopt_t options, dns_dispatch_t **dispp) { + REQUIRE(VALID_DISPATCHMGR(mgr)); + REQUIRE(destaddr != NULL); + + isc_result_t result; + + if ((options & DNS_DISPATCHOPT_FIXEDID) == 0 && + disptype != DNS_DISPATCHTYPE_XFRIN) + { + result = dispatch_gettcp(mgr, localaddr, destaddr, transport, + disptype, dispp); + if (result == ISC_R_SUCCESS) { + if (isc_log_wouldlog(dns_lctx, 90)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + + isc_sockaddr_format(&(*dispp)->local, addrbuf, + ISC_SOCKADDR_FORMATSIZE); + + mgr_log(mgr, ISC_LOG_DEBUG(90), + "dns_dispatch_createtcp: reused TCP " + "dispatch %p for " + "%s", + *dispp, addrbuf); + } + return result; + } + } + + /* + * Otherwise allocate new TCP dispatch. + */ + + dispatch_createtcp(mgr, localaddr, destaddr, transport, disptype, + options, dispp); + + if (isc_log_wouldlog(dns_lctx, 90)) { + char addrbuf[ISC_SOCKADDR_FORMATSIZE]; + + isc_sockaddr_format(&(*dispp)->local, addrbuf, + ISC_SOCKADDR_FORMATSIZE); + + mgr_log(mgr, ISC_LOG_DEBUG(90), + "dns_dispatch_createtcp: created TCP dispatch %p for " + "%s", + *dispp, addrbuf); + } + + return ISC_R_SUCCESS; +} + isc_result_t dns_dispatch_createudp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, dns_dispatch_t **dispp) { @@ -1391,8 +1429,8 @@ dispatch_destroy(dns_dispatch_t *disp) { disp->magic = 0; - if (disp->socktype == isc_socktype_tcp && - (disp->options & DNS_DISPATCHOPT_UNSHARED) == 0) + if ((disp->options & DNS_DISPATCHOPT_FIXEDID) == 0 && + disp->socktype == isc_socktype_tcp) { (void)cds_lfht_del(mgr->tcps[tid], &disp->ht_node); } @@ -1481,35 +1519,52 @@ dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop, } isc_result_t result = ISC_R_NOMORE; - size_t i = 0; rcu_read_lock(); - do { + + if (disp->socktype == isc_socktype_tcp) { /* - * Try somewhat hard to find a unique ID. Start with - * a random number unless DNS_DISPATCHOPT_FIXEDID is set, - * in which case we start with the ID passed in via *idp. + * TCP dispentries don't use the global QID hash table. + * Responses are matched by scanning disp->active, and + * sequential per-dispatch IDs (bounded by the pipelining + * limit) are guaranteed to be unique within the dispatch. + * FIXEDID TCP dispatches are always fresh and isolated + * (see dns_dispatch_createtcp), so the caller-supplied ID + * can't collide either. */ resp->id = ((options & DNS_DISPATCHOPT_FIXEDID) != 0) ? *idp - : (dns_messageid_t)isc_random16(); + : disp->nextid++; + result = ISC_R_SUCCESS; + } else { + size_t i = 0; + do { + /* + * Try somewhat hard to find a unique random ID + * (or use the fixed ID if DNS_DISPATCHOPT_FIXEDID + * is set). + */ + resp->id = ((options & DNS_DISPATCHOPT_FIXEDID) != 0) + ? *idp + : (dns_messageid_t)isc_random16(); - struct cds_lfht_node *node = - cds_lfht_add_unique(disp->mgr->qids, qid_hash(resp), - qid_match, resp, &resp->ht_node); + struct cds_lfht_node *node = cds_lfht_add_unique( + disp->mgr->qids, qid_hash(resp), qid_match, + resp, &resp->ht_node); - if (node != &resp->ht_node) { - if ((options & DNS_DISPATCHOPT_FIXEDID) != 0) { - /* - * When using fixed ID, we either must - * use it or fail - */ - goto fail; + if (node != &resp->ht_node) { + if ((options & DNS_DISPATCHOPT_FIXEDID) != 0) { + /* + * When using fixed ID, we either + * must use it or fail. + */ + goto fail; + } + } else { + result = ISC_R_SUCCESS; + break; } - } else { - result = ISC_R_SUCCESS; - break; - } - } while (i++ < QID_MAX_TRIES); + } while (i++ < QID_MAX_TRIES); + } fail: if (result != ISC_R_SUCCESS) { isc_mem_put(disp->mctx, resp, sizeof(*resp)); @@ -1531,6 +1586,19 @@ fail: disp->requests++; + /* + * If this shared TCP dispatch has reached the pipelining limit, + * remove it from the hash table so new queries get a fresh + * connection. The dispatch continues to serve its existing + * queries until they complete. + */ + if (disp->socktype == isc_socktype_tcp && + (disp->options & DNS_DISPATCHOPT_FIXEDID) == 0 && + disp->requests >= dns_dispatch_tcppipelining) + { + (void)cds_lfht_del(disp->mgr->tcps[isc_tid()], &disp->ht_node); + } + inc_stats(disp->mgr, (disp->socktype == isc_socktype_udp) ? dns_resstatscounter_disprequdp : dns_resstatscounter_dispreqtcp); @@ -1725,8 +1793,6 @@ tcp_dispentry_cancel(dns_dispentry_t *resp, isc_result_t result) { dec_stats(disp->mgr, dns_resstatscounter_dispreqtcp); - (void)cds_lfht_del(disp->mgr->qids, &resp->ht_node); - resp->state = DNS_DISPATCHSTATE_CANCELED; unlock: diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index 6e35e9533f..3a9845e35f 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -74,9 +74,14 @@ struct dns_dispatchset { typedef enum dns_dispatchopt { DNS_DISPATCHOPT_FIXEDID = 1 << 0, - DNS_DISPATCHOPT_UNSHARED = 1 << 1, /* Don't share this connection */ } dns_dispatchopt_t; +typedef enum dns_dispatchtype { + DNS_DISPATCHTYPE_RESOLVER, + DNS_DISPATCHTYPE_REQUEST, + DNS_DISPATCHTYPE_XFRIN, +} dns_dispatchtype_t; + isc_result_t dns_dispatchmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t *nm, dns_dispatchmgr_t **mgrp); @@ -185,8 +190,8 @@ dns_dispatch_createudp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, isc_result_t dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, const isc_sockaddr_t *destaddr, - dns_transport_t *transport, dns_dispatchopt_t options, - dns_dispatch_t **dispp); + dns_transport_t *transport, dns_dispatchtype_t disptype, + dns_dispatchopt_t options, dns_dispatch_t **dispp); /*%< * Create a new TCP dns_dispatch. * @@ -265,35 +270,6 @@ dns_dispatch_resume(dns_dispentry_t *resp, uint16_t timeout); *\li 'resp' is valid. */ -isc_result_t -dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, - const isc_sockaddr_t *localaddr, dns_transport_t *transport, - dns_dispatch_t **dispp); -/* - * Attempt to connect to a existing TCP connection that was created with - * parameters that match destaddr, localaddr and transport. - * - * If localaddr is NULL, we ignore the dispatch's localaddr when looking - * for a match. However, if transport is NULL, then the matching dispatch - * must also have been created with a NULL transport. - * - * Requires: - *\li mgr to be valid dispatch manager. - * - *\li dstaddr to be a valid sockaddr. - * - *\li localaddr to be NULL or a valid sockaddr. - * - *\li transport is NULL or a valid transport. - * - *\li dispp to be non NULL and *dispp to be NULL - * - * Returns: - *\li ISC_R_SUCCESS -- success. - * - *\li Anything else -- failure. - */ - typedef void (*dispatch_cb_t)(isc_result_t eresult, isc_region_t *region, void *cbarg); diff --git a/lib/dns/request.c b/lib/dns/request.c index 0abddd2fff..5f8edec39c 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -336,27 +336,12 @@ isblackholed(dns_dispatchmgr_t *dispatchmgr, const isc_sockaddr_t *destaddr) { } static isc_result_t -tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr, - const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr, - dns_transport_t *transport, dns_dispatch_t **dispatchp) { - isc_result_t result; - - if (!newtcp) { - result = dns_dispatch_gettcp(requestmgr->dispatchmgr, destaddr, - srcaddr, transport, dispatchp); - if (result == ISC_R_SUCCESS) { - char peer[ISC_SOCKADDR_FORMATSIZE]; - - isc_sockaddr_format(destaddr, peer, sizeof(peer)); - req_log(ISC_LOG_DEBUG(1), - "attached to TCP connection to %s", peer); - return result; - } - } - - result = dns_dispatch_createtcp(requestmgr->dispatchmgr, srcaddr, - destaddr, transport, 0, dispatchp); - return result; +tcp_dispatch(dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, + const isc_sockaddr_t *destaddr, dns_transport_t *transport, + unsigned int dispopt, dns_dispatch_t **dispatchp) { + return dns_dispatch_createtcp( + requestmgr->dispatchmgr, srcaddr, destaddr, transport, + DNS_DISPATCHTYPE_REQUEST, dispopt, dispatchp); } static isc_result_t @@ -389,14 +374,15 @@ udp_dispatch(dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, } static isc_result_t -get_dispatch(bool tcp, bool newtcp, dns_requestmgr_t *requestmgr, +get_dispatch(bool tcp, dns_requestmgr_t *requestmgr, const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr, - dns_transport_t *transport, dns_dispatch_t **dispatchp) { + dns_transport_t *transport, unsigned int dispopt, + dns_dispatch_t **dispatchp) { isc_result_t result; if (tcp) { - result = tcp_dispatch(newtcp, requestmgr, srcaddr, destaddr, - transport, dispatchp); + result = tcp_dispatch(requestmgr, srcaddr, destaddr, transport, + dispopt, dispatchp); } else { result = udp_dispatch(requestmgr, srcaddr, destaddr, dispatchp); } @@ -417,7 +403,6 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, isc_mem_t *mctx = NULL; dns_messageid_t id; bool tcp = false; - bool newtcp = false; isc_region_t r; unsigned int dispopt = 0; @@ -469,29 +454,22 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, goto cleanup; } -again: - result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, - transport, &request->dispatch); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - if ((options & DNS_REQUESTOPT_FIXEDID) != 0) { id = (r.base[0] << 8) | r.base[1]; dispopt |= DNS_DISPATCHOPT_FIXEDID; } + result = get_dispatch(tcp, requestmgr, srcaddr, destaddr, transport, + dispopt, &request->dispatch); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + result = dns_dispatch_add( request->dispatch, loop, dispopt, request->timeout, destaddr, transport, tlsctx_cache, req_connected, req_senddone, req_response, request, &id, &request->dispentry); if (result != ISC_R_SUCCESS) { - if ((options & DNS_REQUESTOPT_FIXEDID) != 0 && !newtcp) { - dns_dispatch_detach(&request->dispatch); - newtcp = true; - goto again; - } - goto cleanup; } @@ -595,8 +573,8 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, } again: - result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, - transport, &request->dispatch); + result = get_dispatch(tcp, requestmgr, srcaddr, destaddr, transport, 0, + &request->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index ca0c62cbbe..6bab8057fb 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2098,7 +2098,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, result = dns_dispatch_createtcp(fctx->dispatchmgr, &addr, &sockaddr, addrinfo->transport, - DNS_DISPATCHOPT_UNSHARED, + DNS_DISPATCHTYPE_RESOLVER, 0, &query->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup_query; diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 9f98632c21..1550d9e577 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1346,7 +1346,7 @@ xfrin_start(dns_xfrin_t *xfr) { } else { result = dns_dispatch_createtcp( dispmgr, &xfr->sourceaddr, &xfr->primaryaddr, - xfr->transport, DNS_DISPATCHOPT_UNSHARED, &xfr->disp); + xfr->transport, DNS_DISPATCHTYPE_XFRIN, 0, &xfr->disp); dns_dispatchmgr_detach(&dispmgr); CHECK(result); } diff --git a/tests/dns/dispatch_test.c b/tests/dns/dispatch_test.c index 5257df79ab..e8ff0ae3b8 100644 --- a/tests/dns/dispatch_test.c +++ b/tests/dns/dispatch_test.c @@ -496,56 +496,23 @@ connected_shutdown(isc_result_t eresult, isc_region_t *region ISC_ATTR_UNUSED, } static void -connected_gettcp(isc_result_t eresult ISC_ATTR_UNUSED, - isc_region_t *region ISC_ATTR_UNUSED, void *arg) { - test_dispatch_t *test1 = arg; - - /* Client 2 */ - isc_result_t result; - test_dispatch_t *test2 = isc_mem_get(mctx, sizeof(*test2)); - *test2 = (test_dispatch_t){ - .dispatchmgr = dns_dispatchmgr_ref(test1->dispatchmgr), - }; - - result = dns_dispatch_gettcp(test2->dispatchmgr, &tcp_server_addr, - &tcp_connect_addr, NULL, &test2->dispatch); - assert_int_equal(result, ISC_R_SUCCESS); - - assert_ptr_equal(test1->dispatch, test2->dispatch); - - result = dns_dispatch_add(test2->dispatch, isc_loop_main(loopmgr), 0, - T_CLIENT_CONNECT, &tcp_server_addr, NULL, - NULL, connected_shutdown, client_senddone, - response_noop, test2, &test2->id, - &test2->dispentry); - assert_int_equal(result, ISC_R_SUCCESS); - - dns_dispatch_connect(test2->dispentry); - - test_dispatch_done(test1); -} - -static void -connected_newtcp(isc_result_t eresult ISC_ATTR_UNUSED, - isc_region_t *region ISC_ATTR_UNUSED, void *arg) { +connected_sharedtcp(isc_result_t eresult ISC_ATTR_UNUSED, + isc_region_t *region ISC_ATTR_UNUSED, void *arg) { test_dispatch_t *test3 = arg; - /* Client - unshared */ + /* Second client — should reuse the first client's TCP dispatch. */ isc_result_t result; test_dispatch_t *test4 = isc_mem_get(mctx, sizeof(*test4)); *test4 = (test_dispatch_t){ .dispatchmgr = dns_dispatchmgr_ref(test3->dispatchmgr), }; - result = dns_dispatch_gettcp(test4->dispatchmgr, &tcp_server_addr, - &tcp_connect_addr, NULL, &test4->dispatch); - assert_int_equal(result, ISC_R_NOTFOUND); result = dns_dispatch_createtcp( test4->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, - DNS_DISPATCHOPT_UNSHARED, &test4->dispatch); + DNS_DISPATCHTYPE_RESOLVER, 0, &test4->dispatch); assert_int_equal(result, ISC_R_SUCCESS); - assert_ptr_not_equal(test3->dispatch, test4->dispatch); + assert_ptr_equal(test3->dispatch, test4->dispatch); result = dns_dispatch_add(test4->dispatch, isc_loop_main(loopmgr), 0, T_CLIENT_CONNECT, &tcp_server_addr, NULL, @@ -592,9 +559,9 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_connect) { &test->dispatchmgr); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, NULL, 0, - &test->dispatch); + result = dns_dispatch_createtcp( + test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, + DNS_DISPATCHTYPE_RESOLVER, 0, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add(test->dispatch, isc_loop_main(loopmgr), 0, @@ -638,9 +605,9 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_response) { &test->dispatchmgr); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, NULL, 0, - &test->dispatch); + result = dns_dispatch_createtcp( + test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, + DNS_DISPATCHTYPE_RESOLVER, 0, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add(test->dispatch, isc_loop_main(loopmgr), 0, @@ -674,9 +641,9 @@ ISC_LOOP_TEST_IMPL(dispatch_tcp_response) { &test->dispatchmgr); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, NULL, 0, - &test->dispatch); + result = dns_dispatch_createtcp( + test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, + DNS_DISPATCHTYPE_RESOLVER, 0, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add( @@ -713,9 +680,9 @@ ISC_LOOP_TEST_IMPL(dispatch_tls_response) { &test->dispatchmgr); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_dispatch_createtcp(test->dispatchmgr, &tls_connect_addr, - &tls_server_addr, tls_transport, 0, - &test->dispatch); + result = dns_dispatch_createtcp( + test->dispatchmgr, &tls_connect_addr, &tls_server_addr, + tls_transport, DNS_DISPATCHTYPE_RESOLVER, 0, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add(test->dispatch, isc_loop_main(loopmgr), 0, @@ -800,7 +767,7 @@ ISC_LOOP_TEST_IMPL(dispatch_getnext) { dns_dispatch_connect(test->dispentry); } -ISC_LOOP_TEST_IMPL(dispatch_gettcp) { +ISC_LOOP_TEST_IMPL(dispatch_sharedtcp) { isc_result_t result; test_dispatch_t *test = isc_mem_get(mctx, sizeof(*test)); *test = (test_dispatch_t){ 0 }; @@ -814,61 +781,28 @@ ISC_LOOP_TEST_IMPL(dispatch_gettcp) { /* ensure we stop listening after the test is done */ isc_loop_teardown(isc_loop_main(loopmgr), stop_listening, sock); - result = dns_dispatchmgr_create(mctx, loopmgr, connect_nm, - &test->dispatchmgr); - assert_int_equal(result, ISC_R_SUCCESS); - - /* Client */ - result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, NULL, 0, - &test->dispatch); - assert_int_equal(result, ISC_R_SUCCESS); - - result = dns_dispatch_add( - test->dispatch, isc_loop_main(loopmgr), 0, T_CLIENT_CONNECT, - &tcp_server_addr, NULL, NULL, connected_gettcp, client_senddone, - response_noop, test, &test->id, &test->dispentry); - assert_int_equal(result, ISC_R_SUCCESS); - - dns_dispatch_connect(test->dispentry); -} - -ISC_LOOP_TEST_IMPL(dispatch_newtcp) { - isc_result_t result; - test_dispatch_t *test = isc_mem_get(mctx, sizeof(*test)); - *test = (test_dispatch_t){ 0 }; - - /* Server */ - result = isc_nm_listenstreamdns( - netmgr, ISC_NM_LISTEN_ONE, &tcp_server_addr, nameserver, NULL, - accept_cb, NULL, 0, NULL, NULL, ISC_NM_PROXY_NONE, &sock); - assert_int_equal(result, ISC_R_SUCCESS); - - /* ensure we stop listening after the test is done */ - isc_loop_teardown(isc_loop_main(loopmgr), stop_listening, sock); - - /* Client - unshared */ + /* First client — creates the shared TCP dispatch. */ result = dns_dispatchmgr_create(mctx, loopmgr, connect_nm, &test->dispatchmgr); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp( test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, - DNS_DISPATCHOPT_UNSHARED, &test->dispatch); + DNS_DISPATCHTYPE_RESOLVER, 0, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_dispatch_add( - test->dispatch, isc_loop_main(loopmgr), 0, T_CLIENT_CONNECT, - &tcp_server_addr, NULL, NULL, connected_newtcp, client_senddone, - response_noop, test, &test->id, &test->dispentry); + result = dns_dispatch_add(test->dispatch, isc_loop_main(loopmgr), 0, + T_CLIENT_CONNECT, &tcp_server_addr, NULL, + NULL, connected_sharedtcp, client_senddone, + response_noop, test, &test->id, + &test->dispentry); assert_int_equal(result, ISC_R_SUCCESS); dns_dispatch_connect(test->dispentry); } ISC_TEST_LIST_START -ISC_TEST_ENTRY_CUSTOM(dispatch_gettcp, setup_test, teardown_test) -ISC_TEST_ENTRY_CUSTOM(dispatch_newtcp, setup_test, teardown_test) +ISC_TEST_ENTRY_CUSTOM(dispatch_sharedtcp, setup_test, teardown_test) ISC_TEST_ENTRY_CUSTOM(dispatch_timeout_udp_response, setup_test, teardown_test) ISC_TEST_ENTRY_CUSTOM(dispatchset_create, setup_test, teardown_test) ISC_TEST_ENTRY_CUSTOM(dispatchset_get, setup_test, teardown_test)