]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Replace netmgr per-protocol sequential function with a common one
authorOndřej Surý <ondrej@isc.org>
Tue, 22 Jun 2021 10:24:44 +0000 (12:24 +0200)
committerArtem Boldariev <artem@boldariev.com>
Tue, 22 Jun 2021 14:21:44 +0000 (17:21 +0300)
Previously, each protocol (TCPDNS, TLSDNS) has specified own function to
disable pipelining on the connection.  An oversight would lead to
assertion failure when opcode is not query over non-TCPDNS protocol
because the isc_nm_tcpdns_sequential() function would be called over
non-TCPDNS socket.  This commit removes the per-protocol functions and
refactors the code to have and use common isc_nm_sequential() function
that would either disable the pipelining on the socket or would handle
the request in per specific manner.  Currently it ignores the call for
HTTP sockets and causes assertion failure for protocols where it doesn't
make sense to call the function at all.

lib/isc/include/isc/netmgr.h
lib/isc/netmgr/netmgr.c
lib/isc/netmgr/tcpdns.c
lib/isc/netmgr/tlsdns.c
lib/ns/client.c

index e2ce614df469a62158c3c9bf1d6b9714bd28c541..6f654424b0baa32f04097bdc11f60e0f3ac59261 100644 (file)
@@ -364,7 +364,7 @@ isc_nm_listentlsdns(isc_nm_t *mgr, isc_sockaddr_t *iface,
  */
 
 void
-isc_nm_tcpdns_sequential(isc_nmhandle_t *handle);
+isc_nm_sequential(isc_nmhandle_t *handle);
 /*%<
  * Disable pipelining on this connection. Each DNS packet will be only
  * processed after the previous completes.
@@ -390,24 +390,6 @@ isc_nm_tcpdns_keepalive(isc_nmhandle_t *handle, bool value);
  * to determine when to close a connection, rather than the idle timeout.
  */
 
-void
-isc_nm_tlsdns_sequential(isc_nmhandle_t *handle);
-/*%<
- * Disable pipelining on this connection. Each DNS packet will be only
- * processed after the previous completes.
- *
- * The socket must be unpaused after the query is processed.  This is done
- * the response is sent, or if we're dropping the query, it will be done
- * when a handle is fully dereferenced by calling the socket's
- * closehandle_cb callback.
- *
- * Note: This can only be run while a message is being processed; if it is
- * run before any messages are read, no messages will be read.
- *
- * Also note: once this has been set, it cannot be reversed for a given
- * connection.
- */
-
 void
 isc_nm_tlsdns_keepalive(isc_nmhandle_t *handle, bool value);
 /*%<
index 50ff5a27221772e648c85c0a9559e5e5a8936e35..69c520e7bc4c5a2540190bedb005845fb6166e0e 100644 (file)
@@ -3255,6 +3255,40 @@ isc_nm_work_offload(isc_nm_t *netmgr, isc_nm_workcb_t work_cb,
        RUNTIME_CHECK(r == 0);
 }
 
+void
+isc_nm_sequential(isc_nmhandle_t *handle) {
+       isc_nmsocket_t *sock = NULL;
+
+       REQUIRE(VALID_NMHANDLE(handle));
+       REQUIRE(VALID_NMSOCK(handle->sock));
+
+       sock = handle->sock;
+
+       switch (sock->type) {
+       case isc_nm_tcpdnssocket:
+       case isc_nm_tlsdnssocket:
+               break;
+       case isc_nm_httpsocket:
+               return;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
+       }
+
+       /*
+        * We don't want pipelining on this connection. That means
+        * that we need to pause after reading each request, and
+        * resume only after the request has been processed. This
+        * is done in resume_processing(), which is the socket's
+        * closehandle_cb callback, called whenever a handle
+        * is released.
+        */
+
+       isc__nmsocket_timer_stop(sock);
+       isc__nm_stop_reading(sock);
+       atomic_store(&sock->sequential, true);
+}
+
 #ifdef NETMGR_TRACE
 /*
  * Dump all active sockets in netmgr. We output to stderr
index 223c8d63037c531cbcc8128cec9a799df318b48a..da225cae16b1e9424e77414c3922085807a8a9e4 100644 (file)
@@ -1436,30 +1436,6 @@ isc__nm_async_tcpdnscancel(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 }
 
-void
-isc_nm_tcpdns_sequential(isc_nmhandle_t *handle) {
-       isc_nmsocket_t *sock = NULL;
-
-       REQUIRE(VALID_NMHANDLE(handle));
-       REQUIRE(VALID_NMSOCK(handle->sock));
-       REQUIRE(handle->sock->type == isc_nm_tcpdnssocket);
-
-       sock = handle->sock;
-
-       /*
-        * We don't want pipelining on this connection. That means
-        * that we need to pause after reading each request, and
-        * resume only after the request has been processed. This
-        * is done in resume_processing(), which is the socket's
-        * closehandle_cb callback, called whenever a handle
-        * is released.
-        */
-
-       isc__nmsocket_timer_stop(sock);
-       isc__nm_stop_reading(sock);
-       atomic_store(&sock->sequential, true);
-}
-
 void
 isc_nm_tcpdns_keepalive(isc_nmhandle_t *handle, bool value) {
        isc_nmsocket_t *sock = NULL;
index 41bf6caef8d12341f264742d93e4c9752a4f53c9..db62ac8343e7242333bddd6b4c5048d6ff8fc371 100644 (file)
@@ -2006,30 +2006,6 @@ isc__nm_async_tlsdnscancel(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc__nm_failed_read_cb(sock, ISC_R_EOF, false);
 }
 
-void
-isc_nm_tlsdns_sequential(isc_nmhandle_t *handle) {
-       isc_nmsocket_t *sock = NULL;
-
-       REQUIRE(VALID_NMHANDLE(handle));
-       REQUIRE(VALID_NMSOCK(handle->sock));
-       REQUIRE(handle->sock->type == isc_nm_tlsdnssocket);
-
-       sock = handle->sock;
-
-       /*
-        * We don't want pipelining on this connection. That means
-        * that we need to pause after reading each request, and
-        * resume only after the request has been processed. This
-        * is done in resume_processing(), which is the socket's
-        * closehandle_cb callback, called whenever a handle
-        * is released.
-        */
-
-       isc__nmsocket_timer_stop(sock);
-       isc__nm_stop_reading(sock);
-       atomic_store(&sock->sequential, true);
-}
-
 void
 isc_nm_tlsdns_keepalive(isc_nmhandle_t *handle, bool value) {
        isc_nmsocket_t *sock = NULL;
index ff098cc6468d450a3b9e2beea6ba43931611d9c6..814581b8fad9995a25abc175640eb1b9542f1814 100644 (file)
@@ -1818,7 +1818,7 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult,
              dns_acl_allowed(&netaddr, NULL, client->sctx->keepresporder,
                              env))))
        {
-               isc_nm_tcpdns_sequential(handle);
+               isc_nm_sequential(handle);
        }
 
        dns_opcodestats_increment(client->sctx->opcodestats,