Add option to mark TCP dispatch as unshared

The current dispatch code could reuse the TCP connection when
dns_dispatch_gettcp() would be used first.  This is problematic as the
dns_resolver doesn't use TCP connection sharing, but dns_request could
get the TCP stream that was created outside of the dns_request.

Add new DNS_DISPATCHOPT_UNSHARED option to dns_dispatch_createtcp() that
would prevent the TCP stream to be reused.  Use that option in the
dns_resolver call to dns_dispatch_createtcp() to prevent dns_request
from reusing the TCP connections created by dns_resolver.

Additionally, the dns_xfrin unit added TCP connection sharing for
incoming transfers.  While interleaving *xfr streams on a TCP connection
should work this should be a deliberate change and be property of the
server that can be controlled.  Additionally some level of parallel TCP
streams is desirable.  Revert to the old behaviour by removing the
dns_dispatch_gettcp() calls from dns_xfrin and use the new option to
prevent from sharing the transfer streams with dns_request.
This commit is contained in:
Ondřej Surý 2023-10-20 08:14:27 +02:00
parent efbdee5b3a
commit f213f644ed
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
6 changed files with 53 additions and 67 deletions

View file

@ -117,6 +117,7 @@ struct dns_dispatch {
isc_sockaddr_t local; /*%< local address */
isc_sockaddr_t peer; /*%< peer address (TCP) */
dns_dispatchopt_t options;
dns_dispatchstate_t state;
bool reading;
@ -1156,7 +1157,8 @@ dispatch_match(struct cds_lfht_node *node, const void *key0) {
isc_result_t
dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr,
const isc_sockaddr_t *destaddr, dns_dispatch_t **dispp) {
const isc_sockaddr_t *destaddr,
dns_dispatchopt_t options, dns_dispatch_t **dispp) {
dns_dispatch_t *disp = NULL;
uint32_t tid = isc_tid();
@ -1165,6 +1167,7 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr,
dispatch_allocate(mgr, isc_socktype_tcp, tid, &disp);
disp->options = options;
disp->peer = *destaddr;
if (localaddr != NULL) {
@ -1184,9 +1187,12 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *localaddr,
.peer = &disp->peer,
};
rcu_read_lock();
cds_lfht_add(mgr->tcps[tid], dispatch_hash(&key), &disp->ht_node);
rcu_read_unlock();
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];
@ -1363,7 +1369,9 @@ dispatch_destroy(dns_dispatch_t *disp) {
disp->magic = 0;
if (disp->socktype == isc_socktype_tcp) {
if (disp->socktype == isc_socktype_tcp &&
(disp->options & DNS_DISPATCHOPT_UNSHARED) == 0)
{
(void)cds_lfht_del(mgr->tcps[tid], &disp->ht_node);
}
@ -1391,12 +1399,12 @@ ISC_REFCOUNT_IMPL(dns_dispatch, dispatch_destroy);
#endif
isc_result_t
dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop, unsigned int options,
unsigned int timeout, const isc_sockaddr_t *dest,
dns_transport_t *transport, isc_tlsctx_cache_t *tlsctx_cache,
dispatch_cb_t connected, dispatch_cb_t sent,
dispatch_cb_t response, void *arg, dns_messageid_t *idp,
dns_dispentry_t **respp) {
dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop,
dns_dispatchopt_t options, unsigned int timeout,
const isc_sockaddr_t *dest, dns_transport_t *transport,
isc_tlsctx_cache_t *tlsctx_cache, dispatch_cb_t connected,
dispatch_cb_t sent, dispatch_cb_t response, void *arg,
dns_messageid_t *idp, dns_dispentry_t **respp) {
REQUIRE(VALID_DISPATCH(disp));
REQUIRE(dest != NULL);
REQUIRE(respp != NULL && *respp == NULL);

View file

@ -72,9 +72,10 @@ struct dns_dispatchset {
uint32_t ndisp;
};
/*
*/
#define DNS_DISPATCHOPT_FIXEDID 0x00000001U
typedef enum dns_dispatchopt {
DNS_DISPATCHOPT_FIXEDID = 1 << 0,
DNS_DISPATCHOPT_UNSHARED = 1 << 1, /* Don't share this connection */
} dns_dispatchopt_t;
isc_result_t
dns_dispatchmgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, isc_nm_t *nm,
@ -183,7 +184,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_dispatch_t **dispp);
const isc_sockaddr_t *destaddr,
dns_dispatchopt_t options, dns_dispatch_t **dispp);
/*%<
* Create a new TCP dns_dispatch.
*
@ -262,12 +264,12 @@ typedef void (*dispatch_cb_t)(isc_result_t eresult, isc_region_t *region,
void *cbarg);
isc_result_t
dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop, unsigned int options,
unsigned int timeout, const isc_sockaddr_t *dest,
dns_transport_t *transport, isc_tlsctx_cache_t *tlsctx_cache,
dispatch_cb_t connected, dispatch_cb_t sent,
dispatch_cb_t response, void *arg, dns_messageid_t *idp,
dns_dispentry_t **resp);
dns_dispatch_add(dns_dispatch_t *disp, isc_loop_t *loop,
dns_dispatchopt_t options, unsigned int timeout,
const isc_sockaddr_t *dest, dns_transport_t *transport,
isc_tlsctx_cache_t *tlsctx_cache, dispatch_cb_t connected,
dispatch_cb_t sent, dispatch_cb_t response, void *arg,
dns_messageid_t *idp, dns_dispentry_t **resp);
/*%<
* Add a response entry for this dispatch.
*

View file

@ -350,7 +350,7 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr,
}
result = dns_dispatch_createtcp(requestmgr->dispatchmgr, srcaddr,
destaddr, dispatchp);
destaddr, 0, dispatchp);
return (result);
}

View file

@ -2093,8 +2093,9 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
}
isc_sockaddr_setport(&addr, 0);
result = dns_dispatch_createtcp(res->view->dispatchmgr, &addr,
&sockaddr, &query->dispatch);
result = dns_dispatch_createtcp(
res->view->dispatchmgr, &addr, &sockaddr,
DNS_DISPATCHOPT_UNSHARED, &query->dispatch);
if (result != ISC_R_SUCCESS) {
goto cleanup_query;
}

View file

@ -1227,46 +1227,21 @@ xfrin_start(dns_xfrin_t *xfr) {
dns_xfrin_ref(xfr);
/*
* Reuse an existing TCP connection if possible. For XoT, we can't
* do this because other connections could be using a different
* certificate, so we just create a new dispatch every time.
*/
if (xfr->disp == NULL &&
(xfr->transport == NULL ||
dns_transport_get_type(xfr->transport) == DNS_TRANSPORT_TCP))
{
dns_dispatchmgr_t *dispmgr = dns_view_getdispatchmgr(xfr->view);
if (dispmgr == NULL) {
result = ISC_R_SHUTTINGDOWN;
} else {
result = dns_dispatch_gettcp(dispmgr, &xfr->primaryaddr,
&xfr->sourceaddr,
&xfr->disp);
dns_dispatchmgr_detach(&dispmgr);
}
}
if (result == ISC_R_SUCCESS) {
char peer[ISC_SOCKADDR_FORMATSIZE];
isc_sockaddr_format(&xfr->primaryaddr, peer, sizeof(peer));
xfrin_log(xfr, ISC_LOG_DEBUG(1),
"attached to TCP connection to %s", peer);
} else if (xfr->disp == NULL) {
dns_dispatchmgr_t *dispmgr = dns_view_getdispatchmgr(xfr->view);
if (dispmgr == NULL) {
result = ISC_R_SHUTTINGDOWN;
} else {
result = dns_dispatch_createtcp(
dispmgr, &xfr->sourceaddr, &xfr->primaryaddr,
&xfr->disp);
dns_dispatchmgr_detach(&dispmgr);
}
CHECK(result);
}
/* If this is a retry, we need to cancel the previous dispentry */
if (xfr->dispentry != NULL) {
dns_dispatch_done(&xfr->dispentry);
xfrin_cancelio(xfr);
dns_dispatchmgr_t *dispmgr = dns_view_getdispatchmgr(xfr->view);
if (dispmgr == NULL) {
result = ISC_R_SHUTTINGDOWN;
goto failure;
} else {
result = dns_dispatch_createtcp(
dispmgr, &xfr->sourceaddr, &xfr->primaryaddr,
DNS_DISPATCHOPT_UNSHARED, &xfr->disp);
dns_dispatchmgr_detach(&dispmgr);
if (result != ISC_R_SUCCESS) {
goto failure;
}
}
LIBDNS_XFRIN_START(xfr, xfr->info);

View file

@ -470,7 +470,7 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_connect) {
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_dispatch_createtcp(dispatchmgr, &tcp_connect_addr,
&tcp_server_addr, &dispatch);
&tcp_server_addr, 0, &dispatch);
assert_int_equal(result, ISC_R_SUCCESS);
dns_dispatchmgr_detach(&dispatchmgr);
@ -516,7 +516,7 @@ ISC_LOOP_TEST_IMPL(dispatch_timeout_tcp_response) {
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_dispatch_createtcp(dispatchmgr, &tcp_connect_addr,
&tcp_server_addr, &dispatch);
&tcp_server_addr, 0, &dispatch);
assert_int_equal(result, ISC_R_SUCCESS);
dns_dispatchmgr_detach(&dispatchmgr);
@ -551,7 +551,7 @@ ISC_LOOP_TEST_IMPL(dispatch_tcp_response) {
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_dispatch_createtcp(dispatchmgr, &tcp_connect_addr,
&tcp_server_addr, &dispatch);
&tcp_server_addr, 0, &dispatch);
assert_int_equal(result, ISC_R_SUCCESS);
dns_dispatchmgr_detach(&dispatchmgr);
@ -589,7 +589,7 @@ ISC_LOOP_TEST_IMPL(dispatch_tls_response) {
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_dispatch_createtcp(dispatchmgr, &tls_connect_addr,
&tls_server_addr, &dispatch);
&tls_server_addr, 0, &dispatch);
assert_int_equal(result, ISC_R_SUCCESS);
dns_dispatchmgr_detach(&dispatchmgr);