]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Creating TCP dispatch now creates/binds the socket
authorEvan Hunt <each@isc.org>
Fri, 18 Dec 2020 22:59:50 +0000 (14:59 -0800)
committerEvan Hunt <each@isc.org>
Sat, 2 Oct 2021 18:39:34 +0000 (11:39 -0700)
Previously, creation of TCP dispatches differed from UDP in that a TCP
dispatch was created to attach to an existing socket, whereas a UDP
dispatch would be created in a vacuum and sockets would be opened on
demand when a transaction was initiated.

We are moving as much socket code as possible into the dispatch module,
so that it can be replaced with a netmgr version as easily as
possible. (This will also have the side effect of making TCP and UDP
dispatches more similar.)

As a step in that direction, this commit changes
dns_dispatch_createtcp() so that it creates the TCP socket.

lib/dns/dispatch.c
lib/dns/include/dns/dispatch.h
lib/dns/request.c
lib/dns/resolver.c

index cbb84220eec06d6a4b085391ee93ae4e3ed6514b..6cdd138a7a7f5d5a33672005c1770da95ff54595 100644 (file)
@@ -631,6 +631,7 @@ get_dispsocket(dns_dispatch_t *disp, const isc_sockaddr_t *dest,
                        continue;
                }
                UNLOCK(&qid->lock);
+
                result = open_socket(sockmgr, &localaddr,
                                     ISC_SOCKET_REUSEADDRESS, &sock);
                if (result == ISC_R_SUCCESS) {
@@ -1652,8 +1653,9 @@ qid_destroy(isc_mem_t *mctx, dns_qid_t **qidp) {
 /*
  * Allocate and set important limits.
  */
-static isc_result_t
-dispatch_allocate(dns_dispatchmgr_t *mgr, dns_dispatch_t **dispp) {
+static void
+dispatch_allocate(dns_dispatchmgr_t *mgr, isc_sockettype_t type, int pf,
+                 unsigned int attributes, dns_dispatch_t **dispp) {
        dns_dispatch_t *disp = NULL;
 
        REQUIRE(VALID_DISPATCHMGR(mgr));
@@ -1668,18 +1670,50 @@ dispatch_allocate(dns_dispatchmgr_t *mgr, dns_dispatch_t **dispp) {
        isc_refcount_increment0(&mgr->irefs);
 
        *disp = (dns_dispatch_t){ .mgr = mgr,
+                                 .socktype = type,
                                  .shutdown_why = ISC_R_UNEXPECTED };
        isc_refcount_init(&disp->refcount, 1);
        ISC_LINK_INIT(disp, link);
        ISC_LIST_INIT(disp->activesockets);
        ISC_LIST_INIT(disp->inactivesockets);
 
+       switch (type) {
+       case isc_sockettype_tcp:
+               disp->attributes |= DNS_DISPATCHATTR_TCP;
+               break;
+       case isc_sockettype_udp:
+               disp->attributes |= DNS_DISPATCHATTR_UDP;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
+       }
+
+       switch (pf) {
+       case PF_INET:
+               disp->attributes |= DNS_DISPATCHATTR_IPV4;
+               break;
+       case PF_INET6:
+               disp->attributes |= DNS_DISPATCHATTR_IPV6;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
+       }
+
+       /*
+        * Set whatever attributes were passed in that haven't been
+        * reset automatically by the code above.
+        */
+       attributes &= ~(DNS_DISPATCHATTR_UDP | DNS_DISPATCHATTR_TCP |
+                       DNS_DISPATCHATTR_IPV4 | DNS_DISPATCHATTR_IPV6);
+       disp->attributes |= attributes;
+
        isc_mutex_init(&disp->lock);
        disp->failsafe_ev = allocate_devent(disp);
        disp->magic = DISPATCH_MAGIC;
 
        *dispp = disp;
-       return (ISC_R_SUCCESS);
 }
 
 /*
@@ -1720,30 +1754,38 @@ dispatch_free(dns_dispatch_t **dispp) {
 }
 
 isc_result_t
-dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock,
+dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
                       isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr,
                       const isc_sockaddr_t *destaddr, unsigned int attributes,
-                      dns_dispatch_t **dispp) {
+                      isc_dscp_t dscp, dns_dispatch_t **dispp) {
        isc_result_t result;
        dns_dispatch_t *disp = NULL;
+       isc_sockaddr_t src;
        int pf;
 
        REQUIRE(VALID_DISPATCHMGR(mgr));
-       REQUIRE(isc_socket_gettype(sock) == isc_sockettype_tcp);
-
-       attributes |= DNS_DISPATCHATTR_TCP;
-       attributes &= ~DNS_DISPATCHATTR_UDP;
+       REQUIRE(sockmgr != NULL);
+       REQUIRE(destaddr != NULL);
 
        LOCK(&mgr->lock);
 
-       result = dispatch_allocate(mgr, &disp);
-       if (result != ISC_R_SUCCESS) {
-               UNLOCK(&mgr->lock);
-               return (result);
-       }
+       pf = isc_sockaddr_pf(destaddr);
+       dispatch_allocate(mgr, isc_sockettype_tcp, pf, attributes, &disp);
 
-       disp->socktype = isc_sockettype_tcp;
-       isc_socket_attach(sock, &disp->socket);
+       disp->peer = *destaddr;
+
+       if (localaddr != NULL) {
+               disp->local = *localaddr;
+       } else {
+               switch (pf) {
+               case AF_INET:
+                       isc_sockaddr_any(&disp->local);
+                       break;
+               case AF_INET6:
+                       isc_sockaddr_any6(&disp->local);
+                       break;
+               }
+       }
 
        disp->ntasks = 1;
        disp->task[0] = NULL;
@@ -1752,56 +1794,34 @@ dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock,
                goto cleanup;
        }
 
-       disp->ctlevent =
-               isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL,
-                                  destroy_disp, disp, sizeof(isc_event_t));
-
-       isc_task_setname(disp->task[0], "tcpdispatch", disp);
-
-       dns_tcpmsg_init(mgr->mctx, disp->socket, &disp->tcpmsg);
-       disp->tcpmsg_valid = 1;
-
-       if (destaddr == NULL) {
-               (void)isc_socket_getpeername(sock, &disp->peer);
-               attributes |= DNS_DISPATCHATTR_PRIVATE;
-       } else {
-               disp->peer = *destaddr;
+       result = isc_socket_create(sockmgr, isc_sockaddr_pf(destaddr),
+                                  isc_sockettype_tcp, &disp->socket);
+       if (result != ISC_R_SUCCESS) {
+               goto cleanup;
        }
 
-       pf = isc_sockaddr_pf(&disp->peer);
-
        if (localaddr == NULL) {
-               if (destaddr != NULL) {
-                       switch (pf) {
-                       case AF_INET:
-                               isc_sockaddr_any(&disp->local);
-                               break;
-                       case AF_INET6:
-                               isc_sockaddr_any6(&disp->local);
-                               break;
-                       }
-               } else {
-                       (void)isc_socket_getsockname(sock, &disp->local);
-               }
+               isc_sockaddr_anyofpf(&src, pf);
        } else {
-               disp->local = *localaddr;
+               src = *localaddr;
+               isc_sockaddr_setport(&src, 0);
        }
 
-       switch (pf) {
-       case PF_INET:
-               attributes |= DNS_DISPATCHATTR_IPV4;
-               break;
-
-       case PF_INET6:
-               attributes |= DNS_DISPATCHATTR_IPV6;
-               break;
-
-       default:
-               result = ISC_R_NOTIMPLEMENTED;
+       result = isc_socket_bind(disp->socket, &src, 0);
+       if (result != ISC_R_SUCCESS) {
                goto cleanup;
        }
 
-       disp->attributes = attributes;
+       isc_socket_dscp(disp->socket, dscp);
+
+       disp->ctlevent =
+               isc_event_allocate(mgr->mctx, disp, DNS_EVENT_DISPATCHCONTROL,
+                                  destroy_disp, disp, sizeof(isc_event_t));
+
+       isc_task_setname(disp->task[0], "tcpdispatch", disp);
+
+       dns_tcpmsg_init(mgr->mctx, disp->socket, &disp->tcpmsg);
+       disp->tcpmsg_valid = 1;
 
        /*
         * Append it to the dispatcher list.
@@ -1908,6 +1928,7 @@ dns_dispatch_gettcp(dns_dispatchmgr_t *mgr, const isc_sockaddr_t *destaddr,
                disp = ISC_LIST_NEXT(disp, link);
        }
        UNLOCK(&mgr->lock);
+
        return (match ? ISC_R_SUCCESS : ISC_R_NOTFOUND);
 }
 
@@ -1924,43 +1945,29 @@ dns_dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
        REQUIRE(taskmgr != NULL);
        REQUIRE(dispp != NULL && *dispp == NULL);
 
-       attributes |= DNS_DISPATCHATTR_UDP;
-       attributes &= ~DNS_DISPATCHATTR_TCP;
-
        LOCK(&mgr->lock);
        result = dispatch_createudp(mgr, sockmgr, taskmgr, localaddr,
                                    attributes, &disp);
-
        if (result == ISC_R_SUCCESS) {
                *dispp = disp;
        }
-
        UNLOCK(&mgr->lock);
-       return (ISC_R_SUCCESS);
+
+       return (result);
 }
 
 static isc_result_t
 dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
                   isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr,
                   unsigned int attributes, dns_dispatch_t **dispp) {
-       isc_result_t result;
+       isc_result_t result = ISC_R_SUCCESS;
        dns_dispatch_t *disp = NULL;
        isc_socket_t *sock = NULL;
        isc_sockaddr_t sa_any;
        int pf, i = 0;
 
-       /*
-        * dispatch_allocate() checks mgr for us.
-        */
-       disp = NULL;
-       result = dispatch_allocate(mgr, &disp);
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
-
        pf = isc_sockaddr_pf(localaddr);
-
-       disp->socktype = isc_sockettype_udp;
+       dispatch_allocate(mgr, isc_sockettype_udp, pf, attributes, &disp);
 
        /*
         * For dispatches with a specified source address, we open a
@@ -2014,25 +2021,6 @@ dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
        isc_mem_create(&disp->sepool);
        isc_mem_setname(disp->sepool, "disp_sepool");
 
-       attributes &= ~DNS_DISPATCHATTR_TCP;
-       attributes |= DNS_DISPATCHATTR_UDP;
-
-       switch (pf) {
-       case PF_INET:
-               attributes |= DNS_DISPATCHATTR_IPV4;
-               break;
-
-       case PF_INET6:
-               attributes |= DNS_DISPATCHATTR_IPV6;
-               break;
-
-       default:
-               result = ISC_R_NOTIMPLEMENTED;
-               goto kill_socket;
-       }
-
-       disp->attributes = attributes;
-
        /*
         * Append it to the dispatcher list.
         */
index c9bdc429544aeb655be22d9e666c3e77c409e5e4..69e0d81c62c69a34fb32da311cbee56cee63af4b 100644 (file)
@@ -223,10 +223,10 @@ dns_dispatch_createudp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
  */
 
 isc_result_t
-dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socket_t *sock,
+dns_dispatch_createtcp(dns_dispatchmgr_t *mgr, isc_socketmgr_t *sockmgr,
                       isc_taskmgr_t *taskmgr, const isc_sockaddr_t *localaddr,
                       const isc_sockaddr_t *destaddr, unsigned int attributes,
-                      dns_dispatch_t **dispp);
+                      isc_dscp_t dscp, dns_dispatch_t **dispp);
 /*%<
  * Create a new dns_dispatch and attach it to the provided isc_socket_t.
  *
index fed16798eef2bb7495fdbd75f8c4b12729aa27c7..960ff04d95fbaceddd9f0951c994a07f9e98a72a 100644 (file)
@@ -498,9 +498,6 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr,
             const isc_sockaddr_t *srcaddr, const isc_sockaddr_t *destaddr,
             isc_dscp_t dscp, bool *connected, dns_dispatch_t **dispatchp) {
        isc_result_t result;
-       isc_socket_t *sock = NULL;
-       isc_sockaddr_t src;
-       isc_sockaddr_t bind_any;
 
        if (!newtcp) {
                result = dns_dispatch_gettcp(requestmgr->dispatchmgr, destaddr,
@@ -517,31 +514,9 @@ tcp_dispatch(bool newtcp, dns_requestmgr_t *requestmgr,
                }
        }
 
-       result = isc_socket_create(requestmgr->socketmgr,
-                                  isc_sockaddr_pf(destaddr),
-                                  isc_sockettype_tcp, &sock);
-       if (result != ISC_R_SUCCESS) {
-               return (result);
-       }
-       if (srcaddr == NULL) {
-               isc_sockaddr_anyofpf(&bind_any, isc_sockaddr_pf(destaddr));
-               result = isc_socket_bind(sock, &bind_any, 0);
-       } else {
-               src = *srcaddr;
-               isc_sockaddr_setport(&src, 0);
-               result = isc_socket_bind(sock, &src, 0);
-       }
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup;
-       }
-
-       isc_socket_dscp(sock, dscp);
-       result = dns_dispatch_createtcp(requestmgr->dispatchmgr, sock,
-                                       requestmgr->taskmgr, srcaddr, destaddr,
-                                       0, dispatchp);
-
-cleanup:
-       isc_socket_detach(&sock);
+       result = dns_dispatch_createtcp(
+               requestmgr->dispatchmgr, requestmgr->socketmgr,
+               requestmgr->taskmgr, srcaddr, destaddr, 0, dscp, dispatchp);
        return (result);
 }
 
index 9a39175319ee8beb9405f655b325e7339b0b0716..5d156e5edc243417aaf5eabaae2e38398a434c37 100644 (file)
@@ -228,7 +228,6 @@ typedef struct query {
        dns_dispatchmgr_t *dispatchmgr;
        dns_dispatch_t *dispatch;
        dns_adbaddrinfo_t *addrinfo;
-       isc_socket_t *tcpsocket;
        isc_time_t start;
        dns_messageid_t id;
        dns_dispentry_t *dispentry;
@@ -1200,7 +1199,7 @@ resquery_destroy(resquery_t **queryp) {
        *queryp = NULL;
        REQUIRE(!ISC_LINK_LINKED(query, link));
 
-       INSIST(query->tcpsocket == NULL);
+       INSIST(query->dispatch == NULL);
 
        fctx = query->fctx;
        res = fctx->res;
@@ -1245,13 +1244,13 @@ update_edns_stats(resquery_t *query) {
 static void
 fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp,
                 isc_time_t *finish, bool no_response, bool age_untried) {
-       fetchctx_t *fctx;
-       resquery_t *query;
+       fetchctx_t *fctx = NULL;
+       resquery_t *query = NULL;
        unsigned int rtt, rttms;
        unsigned int factor;
-       dns_adbfind_t *find;
+       dns_adbfind_t *find = NULL;
+       isc_socket_t *sock = NULL;
        dns_adbaddrinfo_t *addrinfo;
-       isc_socket_t *sock;
        isc_stdtime_t now;
 
        query = *queryp;
@@ -1423,33 +1422,20 @@ fctx_cancelquery(resquery_t **queryp, dns_dispatchevent_t **deventp,
         * only needs to worry about managing the connect and send events;
         * the dispatcher manages the recv events.
         */
-       if (RESQUERY_CONNECTING(query)) {
-               /*
-                * Cancel the connect.
-                */
-               if (query->tcpsocket != NULL) {
-                       isc_socket_cancel(query->tcpsocket, NULL,
-                                         ISC_SOCKCANCEL_CONNECT);
-               } else if (query->dispentry != NULL) {
-                       sock = dns_dispatch_getentrysocket(query->dispentry);
-                       if (sock != NULL) {
-                               isc_socket_cancel(sock, NULL,
-                                                 ISC_SOCKCANCEL_CONNECT);
-                       }
-               }
+       if (query->dispentry != NULL) {
+               sock = dns_dispatch_getentrysocket(query->dispentry);
+       } else {
+               sock = dns_dispatch_getsocket(query->dispatch);
        }
-       if (RESQUERY_SENDING(query)) {
-               /*
-                * Cancel the pending send.
-                */
-               if (query->dispentry != NULL) {
-                       sock = dns_dispatch_getentrysocket(query->dispentry);
-               } else {
-                       sock = dns_dispatch_getsocket(query->dispatch);
-               }
-               if (sock != NULL) {
-                       isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND);
-               }
+
+       /* Cancel the connect. */
+       if (sock != NULL && RESQUERY_CONNECTING(query)) {
+               isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_CONNECT);
+       }
+
+       /* Cancel the pending send. */
+       if (sock != NULL && RESQUERY_SENDING(query)) {
+               isc_socket_cancel(sock, NULL, ISC_SOCKCANCEL_SEND);
        }
 
        if (query->dispentry != NULL) {
@@ -1829,19 +1815,12 @@ process_sendevent(resquery_t *query, isc_event_t *event) {
        bool destroy_query = false;
        bool retry = false;
        isc_result_t result;
-       fetchctx_t *fctx;
+       fetchctx_t *fctx = NULL;
 
        fctx = query->fctx;
 
        if (RESQUERY_CANCELED(query)) {
                if (query->sends == 0 && query->connects == 0) {
-                       /*
-                        * This query was canceled while the
-                        * isc_socket_sendto/connect() was in progress.
-                        */
-                       if (query->tcpsocket != NULL) {
-                               isc_socket_detach(&query->tcpsocket);
-                       }
                        destroy_query = true;
                }
        } else {
@@ -2007,10 +1986,10 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) {
 static isc_result_t
 fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
           unsigned int options) {
-       dns_resolver_t *res;
-       isc_task_t *task;
+       dns_resolver_t *res = NULL;
+       isc_task_t *task = NULL;
        isc_result_t result;
-       resquery_t *query;
+       resquery_t *query = NULL;
        isc_sockaddr_t addr;
        bool have_addr = false;
        unsigned int srtt;
@@ -2123,21 +2102,12 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
                        query->dscp = dscp;
                }
 
-               result = isc_socket_create(res->socketmgr, pf,
-                                          isc_sockettype_tcp,
-                                          &query->tcpsocket);
+               result = dns_dispatch_createtcp(
+                       res->dispatchmgr, res->socketmgr, res->taskmgr, &addr,
+                       &addrinfo->sockaddr, 0, query->dscp, &query->dispatch);
                if (result != ISC_R_SUCCESS) {
                        goto cleanup_query;
                }
-
-               result = isc_socket_bind(query->tcpsocket, &addr, 0);
-               if (result != ISC_R_SUCCESS) {
-                       goto cleanup_socket;
-               }
-
-               /*
-                * A dispatch will be created once the connect succeeds.
-                */
        } else {
                if (have_addr) {
                        switch (isc_sockaddr_pf(&addr)) {
@@ -2195,19 +2165,16 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
        query->magic = QUERY_MAGIC;
 
        if ((query->options & DNS_FETCHOPT_TCP) != 0) {
+               isc_socket_t *sock = NULL;
+
                /*
                 * Connect to the remote server.
-                *
-                * XXXRTH  Should we attach to the socket?
                 */
-               if (query->dscp != -1) {
-                       isc_socket_dscp(query->tcpsocket, query->dscp);
-               }
-               result = isc_socket_connect(query->tcpsocket,
-                                           &addrinfo->sockaddr, task,
+               sock = dns_dispatch_getsocket(query->dispatch);
+               result = isc_socket_connect(sock, &addrinfo->sockaddr, task,
                                            resquery_connected, query);
                if (result != ISC_R_SUCCESS) {
-                       goto cleanup_socket;
+                       goto cleanup_dispatch;
                }
                query->connects++;
                QTRACE("connecting via TCP");
@@ -2244,9 +2211,6 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
 
        return (ISC_R_SUCCESS);
 
-cleanup_socket:
-       isc_socket_detach(&query->tcpsocket);
-
 cleanup_dispatch:
        if (query->dispatch != NULL) {
                dns_dispatch_detach(&query->dispatch);
@@ -2961,11 +2925,8 @@ resquery_connected(isc_task_t *task, isc_event_t *event) {
                 * This query was canceled while the connect() was in
                 * progress.
                 */
-               isc_socket_detach(&query->tcpsocket);
                resquery_destroy(&query);
        } else {
-               int attrs = 0;
-
                switch (sevent->result) {
                case ISC_R_SUCCESS:
 
@@ -2989,27 +2950,16 @@ resquery_connected(isc_task_t *task, isc_event_t *event) {
                                fctx_done(fctx, result, __LINE__);
                                break;
                        }
-                       /*
-                        * We are connected.  Create a dispatcher and
-                        * send the query.
-                        */
-                       attrs = DNS_DISPATCHATTR_CONNECTED;
-                       result = dns_dispatch_createtcp(
-                               query->dispatchmgr, query->tcpsocket,
-                               query->fctx->res->taskmgr, NULL, NULL, attrs,
-                               &query->dispatch);
 
                        /*
-                        * Regardless of whether dns_dispatch_create()
-                        * succeeded or not, we don't need our reference
-                        * to the socket anymore.
+                        * We are connected.  Update the dispatcher and
+                        * send the query.
                         */
-                       isc_socket_detach(&query->tcpsocket);
-
-                       if (result == ISC_R_SUCCESS) {
-                               result = resquery_send(query);
-                       }
+                       dns_dispatch_changeattributes(
+                               query->dispatch, DNS_DISPATCHATTR_CONNECTED,
+                               DNS_DISPATCHATTR_CONNECTED);
 
+                       result = resquery_send(query);
                        if (result != ISC_R_SUCCESS) {
                                FCTXTRACE("query canceled: "
                                          "resquery_send() failed; responding");
@@ -3030,10 +2980,6 @@ resquery_connected(isc_task_t *task, isc_event_t *event) {
                                   "no route to host; no response",
                                   sevent->result);
 
-                       /*
-                        * No route to remote.
-                        */
-                       isc_socket_detach(&query->tcpsocket);
                        /*
                         * Do not query this server again in this fetch context
                         * if the server is unavailable over TCP.
@@ -3049,7 +2995,7 @@ resquery_connected(isc_task_t *task, isc_event_t *event) {
                                   "unexpected event result; responding",
                                   sevent->result);
 
-                       isc_socket_detach(&query->tcpsocket);
+                       dns_dispatch_detach(&query->dispatch);
                        fctx_cancelquery(&query, NULL, NULL, false, false);
                        break;
                }