From baab8a5d759e03a048bd1e71e12491026a3fb8ab Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 15 Oct 2024 16:09:48 +1100 Subject: [PATCH] Fix TCP dispatches and transport Dispatch needs to know the transport that is being used over the TCP connection to correctly allow for it to be reused. Add a transport parameter to dns_dispatch_createtcp and dns_dispatch_gettcp and use it when selecting a TCP socket for reuse. --- lib/dns/dispatch.c | 26 +++++++++++++++----- lib/dns/include/dns/dispatch.h | 45 +++++++++++++++++++++++++++++++--- lib/dns/request.c | 14 +++++------ lib/dns/resolver.c | 7 +++--- lib/dns/xfrin.c | 2 +- tests/dns/dispatch_test.c | 23 ++++++++++------- 6 files changed, 87 insertions(+), 30 deletions(-) diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index f86d73a7b9..77b04be991 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -113,10 +113,11 @@ struct dns_dispatch { isc_socktype_t socktype; isc_refcount_t references; isc_mem_t *mctx; - dns_dispatchmgr_t *mgr; /*%< dispatch manager */ - isc_nmhandle_t *handle; /*%< netmgr handle for TCP connection */ - isc_sockaddr_t local; /*%< local address */ - isc_sockaddr_t peer; /*%< peer address (TCP) */ + dns_dispatchmgr_t *mgr; /*%< dispatch manager */ + isc_nmhandle_t *handle; /*%< netmgr handle for TCP connection */ + isc_sockaddr_t local; /*%< local address */ + isc_sockaddr_t peer; /*%< peer address (TCP) */ + dns_transport_t *transport; /*%< TCP transport parameters */ dns_dispatchopt_t options; dns_dispatchstate_t state; @@ -1134,6 +1135,7 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, isc_socktype_t type, uint32_t tid, struct dispatch_key { const isc_sockaddr_t *local; const isc_sockaddr_t *peer; + const dns_transport_t *transport; }; static uint32_t @@ -1162,13 +1164,15 @@ dispatch_match(struct cds_lfht_node *node, const void *key0) { } return (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_dispatchopt_t options, dns_dispatch_t **dispp) { + dns_transport_t *transport, dns_dispatchopt_t options, + dns_dispatch_t **dispp) { dns_dispatch_t *disp = NULL; uint32_t tid = isc_tid(); @@ -1179,6 +1183,9 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, disp->options = options; disp->peer = *destaddr; + if (transport != NULL) { + dns_transport_attach(transport, &disp->transport); + } if (localaddr != NULL) { disp->local = *localaddr; @@ -1195,6 +1202,7 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, struct dispatch_key key = { .local = &disp->local, .peer = &disp->peer, + .transport = transport, }; if ((disp->options & DNS_DISPATCHOPT_UNSHARED) == 0) { @@ -1222,7 +1230,8 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr, isc_result_t dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, - const isc_sockaddr_t *localaddr, dns_dispatch_t **dispp) { + const isc_sockaddr_t *localaddr, dns_transport_t *transport, + dns_dispatch_t **dispp) { dns_dispatch_t *disp_connected = NULL; dns_dispatch_t *disp_fallback = NULL; isc_result_t result = ISC_R_NOTFOUND; @@ -1235,6 +1244,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, struct dispatch_key key = { .local = localaddr, .peer = destaddr, + .transport = transport, }; rcu_read_lock(); @@ -1397,6 +1407,9 @@ dispatch_destroy(dns_dispatch_t *disp) { &disp->handle); isc_nmhandle_detach(&disp->handle); } + if (disp->transport != NULL) { + dns_transport_detach(&disp->transport); + } dns_dispatchmgr_detach(&disp->mgr); call_rcu(&disp->rcu_head, dispatch_destroy_rcu); @@ -1426,6 +1439,7 @@ dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop, REQUIRE(sent != NULL); REQUIRE(loop != NULL); REQUIRE(disp->tid == isc_tid()); + REQUIRE(disp->transport == transport); if (disp->state == DNS_DISPATCHSTATE_CANCELED) { return (ISC_R_CANCELED); diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h index f7cd1cfae7..6e35e9533f 100644 --- a/lib/dns/include/dns/dispatch.h +++ b/lib/dns/include/dns/dispatch.h @@ -185,15 +185,27 @@ 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_dispatchopt_t options, dns_dispatch_t **dispp); + dns_transport_t *transport, dns_dispatchopt_t options, + dns_dispatch_t **dispp); /*%< * Create a new TCP dns_dispatch. * + * Note: a NULL transport is different from a non-NULL transport of type + * DNS_TRANSPORT_TCP, though currently their behavior is the same. + * This allows for different types of transactions to be seperated + * in the future if needed. + * * Requires: * *\li mgr is a valid dispatch manager. * - *\li sock is a valid. + *\li dstaddr to be a valid sockaddr. + * + *\li localaddr to be 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. @@ -255,9 +267,31 @@ dns_dispatch_resume(dns_dispentry_t *resp, uint16_t timeout); isc_result_t dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr, - const isc_sockaddr_t *localaddr, dns_dispatch_t **dispp); + const isc_sockaddr_t *localaddr, dns_transport_t *transport, + dns_dispatch_t **dispp); /* - * Attempt to connect to a existing TCP connection. + * 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, @@ -293,6 +327,9 @@ dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop, * *\li "resp" be non-NULL and *resp be NULL * + *\li "transport" to be the same one used with dns_dispatch_createtcp or + * dns_dispatch_gettcp. + * * Ensures: * *\li <id, dest> is a unique tuple. That means incoming messages diff --git a/lib/dns/request.c b/lib/dns/request.c index e4db0e6290..f88cc4f57d 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -338,12 +338,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_dispatch_t **dispatchp) { + dns_transport_t *transport, dns_dispatch_t **dispatchp) { isc_result_t result; if (!newtcp) { result = dns_dispatch_gettcp(requestmgr->dispatchmgr, destaddr, - srcaddr, dispatchp); + srcaddr, transport, dispatchp); if (result == ISC_R_SUCCESS) { char peer[ISC_SOCKADDR_FORMATSIZE]; @@ -355,7 +355,7 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr, } result = dns_dispatch_createtcp(requestmgr->dispatchmgr, srcaddr, - destaddr, 0, dispatchp); + destaddr, transport, 0, dispatchp); return (result); } @@ -391,12 +391,12 @@ 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, const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr, - dns_dispatch_t **dispatchp) { + dns_transport_t *transport, dns_dispatch_t **dispatchp) { isc_result_t result; if (tcp) { result = tcp_dispatch(newtcp, requestmgr, srcaddr, destaddr, - dispatchp); + transport, dispatchp); } else { result = udp_dispatch(requestmgr, srcaddr, destaddr, dispatchp); } @@ -471,7 +471,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, again: result = get_dispatch(tcp, newtcp, requestmgr, srcaddr, destaddr, - &request->dispatch); + transport, &request->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -596,7 +596,7 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, again: result = get_dispatch(tcp, false, requestmgr, srcaddr, destaddr, - &request->dispatch); + transport, &request->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index a78fb15033..9a4443c92e 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2043,9 +2043,10 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, } isc_sockaddr_setport(&addr, 0); - result = dns_dispatch_createtcp( - res->view->dispatchmgr, &addr, &sockaddr, - DNS_DISPATCHOPT_UNSHARED, &query->dispatch); + result = dns_dispatch_createtcp(res->view->dispatchmgr, &addr, + &sockaddr, addrinfo->transport, + DNS_DISPATCHOPT_UNSHARED, + &query->dispatch); if (result != ISC_R_SUCCESS) { goto cleanup_query; } diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index 76d04a8480..f5ebcd94e4 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1241,7 +1241,7 @@ xfrin_start(dns_xfrin_t *xfr) { } else { result = dns_dispatch_createtcp( dispmgr, &xfr->sourceaddr, &xfr->primaryaddr, - DNS_DISPATCHOPT_UNSHARED, &xfr->disp); + xfr->transport, DNS_DISPATCHOPT_UNSHARED, &xfr->disp); dns_dispatchmgr_detach(&dispmgr); if (result != ISC_R_SUCCESS) { goto failure; diff --git a/tests/dns/dispatch_test.c b/tests/dns/dispatch_test.c index 8a94582c4d..b207add205 100644 --- a/tests/dns/dispatch_test.c +++ b/tests/dns/dispatch_test.c @@ -508,7 +508,7 @@ connected_gettcp(isc_result_t eresult ISC_ATTR_UNUSED, }; result = dns_dispatch_gettcp(test2->dispatchmgr, &tcp_server_addr, - &tcp_connect_addr, &test2->dispatch); + &tcp_connect_addr, NULL, &test2->dispatch); assert_int_equal(result, ISC_R_SUCCESS); assert_ptr_equal(test1->dispatch, test2->dispatch); @@ -537,11 +537,11 @@ connected_newtcp(isc_result_t eresult ISC_ATTR_UNUSED, .dispatchmgr = dns_dispatchmgr_ref(test3->dispatchmgr), }; result = dns_dispatch_gettcp(test4->dispatchmgr, &tcp_server_addr, - &tcp_connect_addr, &test4->dispatch); + &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, + test4->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, DNS_DISPATCHOPT_UNSHARED, &test4->dispatch); assert_int_equal(result, ISC_R_SUCCESS); @@ -593,7 +593,8 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_connect) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, 0, &test->dispatch); + &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, @@ -638,7 +639,8 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_response) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, 0, &test->dispatch); + &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, @@ -673,7 +675,8 @@ ISC_LOOP_TEST_IMPL(dispatch_tcp_response) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, 0, &test->dispatch); + &tcp_server_addr, NULL, 0, + &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add( @@ -711,7 +714,8 @@ ISC_LOOP_TEST_IMPL(dispatch_tls_response) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp(test->dispatchmgr, &tls_connect_addr, - &tls_server_addr, 0, &test->dispatch); + &tls_server_addr, tls_transport, 0, + &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add(test->dispatch, isc_loop_main(loopmgr), 0, @@ -816,7 +820,8 @@ ISC_LOOP_TEST_IMPL(dispatch_gettcp) { /* Client */ result = dns_dispatch_createtcp(test->dispatchmgr, &tcp_connect_addr, - &tcp_server_addr, 0, &test->dispatch); + &tcp_server_addr, NULL, 0, + &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_add( @@ -848,7 +853,7 @@ ISC_LOOP_TEST_IMPL(dispatch_newtcp) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_dispatch_createtcp( - test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, + test->dispatchmgr, &tcp_connect_addr, &tcp_server_addr, NULL, DNS_DISPATCHOPT_UNSHARED, &test->dispatch); assert_int_equal(result, ISC_R_SUCCESS);