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.

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index 9871c15c34a39304d755ccd8c254c24c70945dfd..b68692fbb918e3747bad0b1f3ccfbcd6176220fe 100644 (file)
--- a/lib/dns/dispatch.c
+++ b/lib/dns/dispatch.c
@@ -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);

diff --git a/lib/dns/include/dns/dispatch.h b/lib/dns/include/dns/dispatch.h
index 80ef3c9d775c17753ccb8a92829de318b159927f..f7cd1cfae75f7f628b6dc73bd0df968c94f99c5b 100644 (file)
--- a/lib/dns/include/dns/dispatch.h
+++ b/lib/dns/include/dns/dispatch.h
@@ -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.
  *

diff --git a/lib/dns/request.c b/lib/dns/request.c
index dc8aaaec648c58e4632db674d02eb4aad411015f..c7e6a4061d9330d1f403ac89e1bf0bb63a05dc1a 100644 (file)
--- a/lib/dns/request.c
+++ b/lib/dns/request.c
@@ -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);
 }
 

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index a3327fc9614629e9123e866b98c6aa45f521df56..781237ad674da53e35716a77383b630c1e4d40fb 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -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;
                }

diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c
index e91226456d68a7492684fcb99c052c0befd78e77..148449ce626a3ca88d2462c351fc9c96bf8e2ece 100644 (file)
--- a/lib/dns/xfrin.c
+++ b/lib/dns/xfrin.c
@@ -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);

diff --git a/tests/dns/dispatch_test.c b/tests/dns/dispatch_test.c
index bce2565d82b798116d79a02e5603926ddcb5b4fc..c8950069cd0d5c325c24819b611c8492165cd768 100644 (file)
--- a/tests/dns/dispatch_test.c
+++ b/tests/dns/dispatch_test.c
@@ -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);