diff --git a/lib/ns/client.c b/lib/ns/client.c index 57cc1ab838..24a3fda23d 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -318,6 +318,75 @@ read_settimeout(ns_client_t *client, bool newconn) { } } +/*% + * Allocate a reference counter that will track the number of client structures + * using the TCP connection that 'client' called accept() for. This counter + * will be shared between all client structures associated with this TCP + * connection. + */ +static void +pipeline_init(ns_client_t *client) { + isc_refcount_t *refs; + + REQUIRE(client->pipeline_refs == NULL); + + /* + * A global memory context is used for the allocation as different + * client structures may have different memory contexts assigned and a + * reference counter allocated here might need to be freed by a + * different client. The performance impact caused by memory context + * contention here is expected to be negligible, given that this code + * is only executed for TCP connections. + */ + refs = isc_mem_allocate(client->sctx->mctx, sizeof(*refs)); + isc_refcount_init(refs, 1); + client->pipeline_refs = refs; +} + +/*% + * Increase the count of client structures using the TCP connection that + * 'source' is associated with and put a pointer to that count in 'target', + * thus associating it with the same TCP connection. + */ +static void +pipeline_attach(ns_client_t *source, ns_client_t *target) { + int old_refs; + + REQUIRE(source->pipeline_refs != NULL); + REQUIRE(target->pipeline_refs == NULL); + + old_refs = isc_refcount_increment(source->pipeline_refs); + INSIST(old_refs > 0); + target->pipeline_refs = source->pipeline_refs; +} + +/*% + * Decrease the count of client structures using the TCP connection that + * 'client' is associated with. If this is the last client using this TCP + * connection, free the reference counter and return true; otherwise, return + * false. + */ +static bool +pipeline_detach(ns_client_t *client) { + isc_refcount_t *refs; + int old_refs; + + REQUIRE(client->pipeline_refs != NULL); + + refs = client->pipeline_refs; + client->pipeline_refs = NULL; + + old_refs = isc_refcount_decrement(refs); + INSIST(old_refs > 0); + + if (old_refs == 1) { + isc_mem_free(client->sctx->mctx, refs); + return (true); + } + + return (false); +} + /*% * Check for a deactivation or shutdown request and take appropriate * action. Returns true if either is in progress; in this case @@ -440,6 +509,40 @@ exit_check(ns_client_t *client) { client->tcpmsg_valid = false; } + if (client->tcpquota != NULL) { + if (client->pipeline_refs == NULL || + pipeline_detach(client)) + { + /* + * Only detach from the TCP client quota if + * there are no more client structures using + * this TCP connection. + * + * Note that we check 'pipeline_refs' and not + * 'pipelined' because in some cases (e.g. + * after receiving a request with an opcode + * different than QUERY) 'pipelined' is set to + * false after the reference counter gets + * allocated in pipeline_init() and we must + * still drop our reference as failing to do so + * would prevent the reference counter itself + * from being freed. + */ + isc_quota_detach(&client->tcpquota); + } else { + /* + * There are other client structures using this + * TCP connection, so we cannot detach from the + * TCP client quota to prevent excess TCP + * connections from being accepted. However, + * this client structure might later be reused + * for accepting new connections and thus must + * have its 'tcpquota' field set to NULL. + */ + client->tcpquota = NULL; + } + } + if (client->tcpsocket != NULL) { CTRACE("closetcp"); isc_socket_detach(&client->tcpsocket); @@ -453,44 +556,6 @@ exit_check(ns_client_t *client) { } } - if (client->tcpquota != NULL) { - /* - * If we are not in a pipeline group, or - * we are the last client in the group, detach from - * tcpquota; otherwise, transfer the quota to - * another client in the same group. - */ - if (!ISC_LINK_LINKED(client, glink) || - (client->glink.next == NULL && - client->glink.prev == NULL)) - { - isc_quota_detach(&client->tcpquota); - } else if (client->glink.next != NULL) { - INSIST(client->glink.next->tcpquota == NULL); - client->glink.next->tcpquota = client->tcpquota; - client->tcpquota = NULL; - } else { - INSIST(client->glink.prev->tcpquota == NULL); - client->glink.prev->tcpquota = client->tcpquota; - client->tcpquota = NULL; - } - } - - /* - * Unlink from pipeline group. - */ - if (ISC_LINK_LINKED(client, glink)) { - if (client->glink.next != NULL) { - client->glink.next->glink.prev = - client->glink.prev; - } - if (client->glink.prev != NULL) { - client->glink.prev->glink.next = - client->glink.next; - } - ISC_LINK_INIT(client, glink); - } - if (client->timerset) { (void)isc_timer_reset(client->timer, isc_timertype_inactive, @@ -3108,6 +3173,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { client->mortal = false; client->sendcb = NULL; client->pipelined = false; + client->pipeline_refs = NULL; client->tcpquota = NULL; client->recursionquota = NULL; client->interface = NULL; @@ -3129,7 +3195,6 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) { client->formerrcache.id = 0; ISC_LINK_INIT(client, link); ISC_LINK_INIT(client, rlink); - ISC_LINK_INIT(client, glink); ISC_QLINK_INIT(client, ilink); client->keytag = NULL; client->keytag_len = 0; @@ -3318,6 +3383,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) { !dns_acl_allowed(&netaddr, NULL, client->sctx->keepresporder, env))) { + pipeline_init(client); client->pipelined = true; } @@ -3788,37 +3854,18 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock, client->newstate = client->state = NS_CLIENTSTATE_WORKING; INSIST(client->recursionquota == NULL); client->sctx = manager->sctx; - - /* - * Transfer TCP quota to the new client. - */ - INSIST(client->tcpquota == NULL); - INSIST(oldclient->tcpquota != NULL); - client->tcpquota = oldclient->tcpquota; - oldclient->tcpquota = NULL; - - /* - * Link to a pipeline group, creating it if needed. - */ - if (!ISC_LINK_LINKED(oldclient, glink)) { - oldclient->glink.next = NULL; - oldclient->glink.prev = NULL; - } - client->glink.next = oldclient->glink.next; - client->glink.prev = oldclient; - if (oldclient->glink.next != NULL) { - oldclient->glink.next->glink.prev = client; - } - oldclient->glink.next = client; + client->tcpquota = &client->sctx->tcpquota; client->dscp = ifp->dscp; client->attributes |= NS_CLIENTATTR_TCP; - client->pipelined = true; client->mortal = true; client->sendcb = NULL; client->rcode_override = -1; /* not set */ + pipeline_attach(oldclient, client); + client->pipelined = true; + isc_socket_attach(ifp->tcpsocket, &client->tcplistener); isc_socket_attach(sock, &client->tcpsocket); isc_socket_setname(client->tcpsocket, "worker-tcp", NULL); diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index d0d1662951..a775c6acd2 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -135,6 +135,7 @@ struct ns_client { dns_name_t *signer; /*%< NULL if not valid sig */ bool mortal; /*%< Die after handling request */ bool pipelined; /*%< TCP queries not in sequence */ + isc_refcount_t *pipeline_refs; isc_quota_t *tcpquota; isc_quota_t *recursionquota; ns_interface_t *interface; @@ -166,7 +167,6 @@ struct ns_client { ISC_LINK(ns_client_t) link; ISC_LINK(ns_client_t) rlink; - ISC_LINK(ns_client_t) glink; ISC_QLINK(ns_client_t) ilink; unsigned char cookie[8]; uint32_t expire;