ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

fix: dev: Post [CVE-2024-12705] Performance Drop Fixes, Part 2

Browse Source 
This merge request addresses several key performance bottlenecks in the DoH (DNS over HTTPS) implementation by introducing significant optimizations and improvements.

### Key Improvements

1. **Simplification and Optimisation of `http_do_bio()` Function**:
   - The code flow in the `http_do_bio()` function has been significantly simplified.
2. **Flushing HTTP Write Buffer on Outgoing DNS Messages**:
   - The buffer is flushed and a send operation is performed when there is an outgoing DNS message.
3. **Bumping Active Streams Processing Limit**:
   - The total number of active streams has been increased to 60% of the total streams limit.

These changes collectively enhance the performance and reliability of the DoH implementation, making it more efficient and robust for handling high-load scenarios, particularly noticeable in long runs (>= 1h) of `stress:long:rpz:doh+udp:linux:*` tests. It improves perf. for tests for BIND 9.18, but it likely will have a positive but less pronounced effect on newer versions as well.

In essence, the merge request fixes three bottlenecks stacked upon each other.

*It is a logical continuation of the merge requests !10109.* !10109, unfortunately, did not completely [address the performance drop in 9.18](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/221545) for longer runs of the stress test. This merge request [addresses that](https://gitlab.isc.org/isc-projects/bind9/-/pipelines/223661).

**P.S.**

The origin of the fixes is, in fact, the branch in !10193. So this MR is a ... *forward port* of them.

Merge branch 'artem-doh-performance-drop-post-fix' into 'main'

See merge request isc-projects/bind9!10192
This commit is contained in:
Artem Boldariev
2025-03-03 10:10:33 +00:00
parent 239712df16 eaad0aefe6
commit c8104daf8d
1 changed files with 66 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

122
lib/isc/netmgr/http.c
View File

@@ -190,6 +190,8 @@ struct isc_nm_http_session {
size_t data_in_flight;
bool async_queued;
/*
* The statistical values below are for usage on server-side
* only. They are meant to detect clients that are taking too many
@@ -1293,7 +1295,10 @@ http_readcb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
}
isc_buffer_putmem(session->buf, region->base + readlen,
unread_size);
isc_nm_read_stop(session->handle);
if (session->handle != NULL) {
INSIST(VALID_NMHANDLE(session->handle));
isc_nm_read_stop(session->handle);
}
http_do_bio_async(session);
} else {
/* We might have something to receive or send, do IO */
@@ -1462,26 +1467,27 @@ http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle,
* to avoid hitting unnecessary limitations on a TLS record size
* within some tools (e.g. flamethrower).
*/
if (max_total_write_size >= FLUSH_HTTP_WRITE_BUFFER_AFTER) {
if (cb != NULL) {
/*
* Case 1: We have at least FLUSH_HTTP_WRITE_BUFFER_AFTER
* bytes to send. Let's flush it.
* Case 0: The callback is specified, that means that a DNS
* message is ready. Let's flush the the buffer.
*/
total = max_total_write_size;
} else if (max_total_write_size >= FLUSH_HTTP_WRITE_BUFFER_AFTER) {
/*
* Case 1: We have equal or more than
* FLUSH_HTTP_WRITE_BUFFER_AFTER bytes to send. Let's flush it.
*/
total = max_total_write_size;
} else if (session->sending > 0 && total > 0) {
/*
* Case 2: There is one or more write requests in flight and
* we have some new data form nghttp2 to send. Let's put the
* write callback (if any) into the pending write callbacks
* list. Then let's return from the function: as soon as the
* we have some new data form nghttp2 to send.
* Then let's return from the function: as soon as the
* "in-flight" write callback get's called or we have reached
* FLUSH_HTTP_WRITE_BUFFER_AFTER bytes in the write buffer, we
* will flush the buffer.
*/
if (cb != NULL) {
http_append_pending_send_request(session, httphandle,
cb, cbarg);
}
* will flush the buffer. */
INSIST(cb == NULL);
goto nothing_to_send;
} else if (session->sending == 0 && total == 0 &&
session->pending_write_data != NULL)
@@ -1516,8 +1522,6 @@ http_send_outgoing(isc_nm_http_session_t *session, isc_nmhandle_t *httphandle,
(total == 0 && session->sending == 0) ||
(total > 0 && session->sending == 0));
}
#else
INSIST(ISC_LIST_EMPTY(session->pending_write_callbacks));
#endif /* ENABLE_HTTP_WRITE_BUFFERING */
if (total == 0) {
@@ -1579,7 +1583,7 @@ http_too_many_active_streams(isc_nm_http_session_t *session) {
*/
const uint64_t max_active_streams =
ISC_MAX(ISC_NETMGR_MAX_STREAM_CLIENTS_PER_CONN,
session->max_concurrent_streams / 3);
(session->max_concurrent_streams * 6) / 10); /* 60% */
if (session->client) {
return false;
@@ -1599,10 +1603,12 @@ http_too_many_active_streams(isc_nm_http_session_t *session) {
static void
http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
isc_nm_cb_t send_cb, void *send_cbarg) {
isc__nm_uvreq_t *req = NULL;
size_t remaining = 0;
REQUIRE(VALID_HTTP2_SESSION(session));
if (session->closed) {
return;
goto cancel;
} else if (session->closing) {
/*
* There might be leftover callbacks waiting to be received
@@ -1610,23 +1616,24 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
if (session->sending == 0) {
finish_http_session(session);
}
return;
goto cancel;
} else if (nghttp2_session_want_read(session->ngsession) == 0 &&
nghttp2_session_want_write(session->ngsession) == 0 &&
session->pending_write_data == NULL)
{
session->closing = true;
if (session->handle != NULL) {
isc_nm_read_stop(session->handle);
}
if (session->sending == 0) {
finish_http_session(session);
}
goto cancel;
}
if (send_cb != NULL) {
INSIST(VALID_NMHANDLE(send_httphandle));
http_send_outgoing(session, send_httphandle, send_cb,
send_cbarg);
return;
}
INSIST(send_httphandle == NULL);
INSIST(send_cb == NULL);
INSIST(send_cbarg == NULL);
if (session->pending_write_data != NULL && session->sending == 0) {
http_send_outgoing(session, NULL, NULL, NULL);
return;
else if (session->buf != NULL)
{
remaining = isc_buffer_remaininglength(session->buf);
}
if (nghttp2_session_want_read(session->ngsession) != 0) {
@@ -1635,9 +1642,7 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
isc__nmsocket_timer_start(session->handle->sock);
isc_nm_read(session->handle, http_readcb, session);
session->reading = true;
} else if (session->buf != NULL) {
size_t remaining =
isc_buffer_remaininglength(session->buf);
} else if (session->buf != NULL && remaining > 0) {
/* Leftover data in the buffer, use it */
size_t remaining_after = 0;
ssize_t readlen = 0;
@@ -1661,8 +1666,12 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
http_log_flooding_peer(session);
failed_read_cb(ISC_R_RANGE, session);
} else if ((size_t)readlen == remaining) {
isc_buffer_free(&session->buf);
http_do_bio(session, NULL, NULL, NULL);
isc_buffer_clear(session->buf);
isc_buffer_compact(session->buf);
http_do_bio(session, send_httphandle, send_cb,
send_cbarg);
isc__nm_httpsession_detach(&tmpsess);
return;
} else if (remaining_after > 0 &&
remaining_after < remaining)
{
@@ -1679,39 +1688,36 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
* it and that could overwhelm the server.
*/
http_do_bio_async(session);
} else {
http_send_outgoing(session, NULL, NULL, NULL);
}
isc__nm_httpsession_detach(&tmpsess);
return;
} else {
} else if (session->handle != NULL) {
INSIST(VALID_NMHANDLE(session->handle));
/*
* Resume reading, it's idempotent, wait for more
*/
isc__nmsocket_timer_start(session->handle->sock);
isc_nm_read(session->handle, http_readcb, session);
}
} else {
} else if (session->handle != NULL) {
INSIST(VALID_NMHANDLE(session->handle));
/* We don't want more data, stop reading for now */
isc_nm_read_stop(session->handle);
}
/* we might have some data to send after processing */
http_send_outgoing(session, NULL, NULL, NULL);
if (nghttp2_session_want_read(session->ngsession) == 0 &&
nghttp2_session_want_write(session->ngsession) == 0 &&
session->pending_write_data == NULL)
{
session->closing = true;
isc_nm_read_stop(session->handle);
if (session->sending == 0) {
finish_http_session(session);
}
}
http_send_outgoing(session, send_httphandle, send_cb, send_cbarg);
return;
cancel:
if (send_cb == NULL) {
return;
}
req = isc__nm_uvreq_get(send_httphandle->sock);
req->cb.send = send_cb;
req->cbarg = send_cbarg;
isc_nmhandle_attach(send_httphandle, &req->handle);
isc__nm_sendcb(send_httphandle->sock, req, ISC_R_CANCELED, true);
}
static void
@@ -1720,6 +1726,8 @@ http_do_bio_async_cb(void *arg) {
REQUIRE(VALID_HTTP2_SESSION(session));
session->async_queued = false;
if (session->handle != NULL &&
!isc__nmsocket_closing(session->handle->sock))
{
@@ -1736,10 +1744,12 @@ http_do_bio_async(isc_nm_http_session_t *session) {
REQUIRE(VALID_HTTP2_SESSION(session));
if (session->handle == NULL ||
isc__nmsocket_closing(session->handle->sock))
isc__nmsocket_closing(session->handle->sock) ||
session->async_queued)
{
return;
}
session->async_queued = true;
isc__nm_httpsession_attach(session, &tmpsess);
isc_async_run(session->handle->sock->worker->loop, http_do_bio_async_cb,
tmpsess);