Replace netmgr per-protocol sequential function with a common one

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.
This commit is contained in:
Ondřej Surý
2021-06-22 12:24:44 +02:00
committed by Artem Boldariev
parent 48256df57c
commit ec86759401
5 changed files with 36 additions and 68 deletions

View 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);
/*%<

View 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

View 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;

View 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;

View 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,