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.

diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c
index f86d73a7b9bec69514609043de16fb0d52bddfcc..77b04be991b5beb0f98e12df0d2df19e654f0b6e 100644 (file)
--- 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 f7cd1cfae75f7f628b6dc73bd0df968c94f99c5b..6e35e9533ff273570648b9d0e2d86bc84122d697 100644 (file)
--- 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  &lt;id, dest> is a unique tuple.  That means incoming messages

diff --git a/lib/dns/request.c b/lib/dns/request.c
index e4db0e629045c8feb3195178a6f1db3f7c472c56..f88cc4f57d1334bf07629aaf7150ecc61a5743c7 100644 (file)
--- 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 a78fb1503359843a64d633f70a272ceb1318c019..9a4443c92e829b3c6c3fe934b65a3315e3632a9c 100644 (file)
--- 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 76d04a848020cded43d05cd092febd8b7de15c48..f5ebcd94e45ebf8b4998f5f87d69faa3b6603ceb 100644 (file)
--- 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 8a94582c4da079d9fce6137f797939489ae92bb2..b207add20559e6253a92995aeb2ebe0fec4f8abf 100644 (file)
--- 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);