From: Vladimír Čunát Date: Thu, 24 Apr 2025 07:44:22 +0000 (+0200) Subject: daemon: fix DoH with multiple "parallel" queries in one connection X-Git-Tag: v6.0.12~3^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fenvironments%2Fdocs-develop-doh-w2qmpf%2Fdeployments%2F6652;p=thirdparty%2Fknot-resolver.git daemon: fix DoH with multiple "parallel" queries in one connection --- diff --git a/NEWS b/NEWS index 1107b88db..fd589b89d 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ Knot Resolver 6.0.12 (2025-0m-dd) Bugfixes -------- +- daemon: fix DoH with multiple "parallel" queries in one connection (#931, !1677) - /management/unix-socket: revert to absolute path (#926, !1664) - fix `tags` when used in /local-data/rules/*/records (!1670) - stats: request latency was very incorrect in some cases (!1676) diff --git a/daemon/http.c b/daemon/http.c index 89b5e4c42..fbfb76518 100644 --- a/daemon/http.c +++ b/daemon/http.c @@ -72,7 +72,9 @@ struct pl_http_sess_data { struct protolayer_data h; struct nghttp2_session *h2; - queue_http_stream streams; /* Streams present in the wire buffer. */ + /** Info about streams pending for kr_request creation. */ + queue_http_stream streams; + trie_t *stream_write_queues; /* Dictionary of stream data that needs to be freed after write. */ int32_t incomplete_stream; int32_t last_stream; /* The last used stream - mostly the same as incomplete_stream, but can be used after @@ -912,12 +914,12 @@ static int pl_http_sess_deinit(struct session2 *session, void *data) http_free_headers(stream->headers); queue_pop(http->streams); } + queue_deinit(http->streams); trie_apply(http->stream_write_queues, stream_write_data_break_err, NULL); trie_free(http->stream_write_queues); http_cleanup_stream(http); - queue_deinit(http->streams); wire_buf_deinit(&http->wire_buf); nghttp2_session_del(http->h2); @@ -989,8 +991,8 @@ static enum protolayer_iter_cb_result pl_http_wrap( prov.read_callback = read_callback; struct pl_http_sess_data *http = sess_data; - int32_t stream_id = http->last_stream; - int ret = http_send_response(sess_data, stream_id, &prov, HTTP_STATUS_OK); + int32_t stream_id = ctx->req->qsource.stream_id; + int ret = http_send_response(http, stream_id, &prov, HTTP_STATUS_OK); if (ret) return protolayer_break(ctx, ret); @@ -1022,10 +1024,11 @@ static void pl_http_request_init(struct session2 *session, struct http_stream *stream = &queue_head(http->streams); req->qsource.stream_id = stream->id; if (stream->headers) { + // the request takes ownership of the referred-to memory req->qsource.headers = *stream->headers; free(stream->headers); - stream->headers = NULL; } + queue_pop(http->streams); } __attribute__((constructor)) diff --git a/daemon/session2.c b/daemon/session2.c index faca57bfe..22c6674a9 100644 --- a/daemon/session2.c +++ b/daemon/session2.c @@ -612,7 +612,7 @@ static int session2_submit( struct session2 *session, enum protolayer_direction direction, size_t layer_ix, struct protolayer_payload payload, const struct comm_info *comm, - protolayer_finished_cb cb, void *baton) + struct kr_request *req, protolayer_finished_cb cb, void *baton) { if (session->closing) return kr_error(ECANCELED); @@ -647,6 +647,7 @@ static int session2_submit( .direction = direction, .layer_ix = layer_ix, .session = session, + .req = req, .finished_cb = cb, .finished_cb_baton = baton }; @@ -1259,7 +1260,7 @@ int session2_unwrap(struct session2 *s, struct protolayer_payload payload, void *baton) { return session2_submit(s, PROTOLAYER_UNWRAP, - 0, payload, comm, cb, baton); + 0, payload, comm, NULL/*req*/, cb, baton); } int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol, @@ -1272,16 +1273,16 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol, if (kr_fails_assert(ok)) // not found or "last layer" return kr_error(EINVAL); return session2_submit(s, PROTOLAYER_UNWRAP, - layer_ix + 1, payload, comm, cb, baton); + layer_ix + 1, payload, comm, NULL/*req*/, cb, baton); } int session2_wrap(struct session2 *s, struct protolayer_payload payload, - const struct comm_info *comm, protolayer_finished_cb cb, - void *baton) + const struct comm_info *comm, struct kr_request *req, + protolayer_finished_cb cb, void *baton) { return session2_submit(s, PROTOLAYER_WRAP, protolayer_grps[s->proto].num_layers - 1, - payload, comm, cb, baton); + payload, comm, req, cb, baton); } int session2_wrap_after(struct session2 *s, enum protolayer_type protocol, @@ -1293,7 +1294,7 @@ int session2_wrap_after(struct session2 *s, enum protolayer_type protocol, if (kr_fails_assert(layer_ix > 0)) // not found or "last layer" return kr_error(EINVAL); return session2_submit(s, PROTOLAYER_WRAP, layer_ix - 1, - payload, comm, cb, baton); + payload, comm, NULL/*req*/, cb, baton); } static void session2_event_wrap(struct session2 *s, enum protolayer_event_type event, void *baton) @@ -1644,8 +1645,8 @@ static int session2_transport_pushv(struct session2 *s, } int ret = session2_wrap(parent, protolayer_payload_iovec(iov, iovcnt, iov_short_lived), - comm, session2_transport_parent_pushv_finished, - ctx); + comm, NULL/*req*/, + session2_transport_parent_pushv_finished, ctx); return (ret < 0) ? ret : kr_ok(); default: diff --git a/daemon/session2.h b/daemon/session2.h index 5228ad864..f94a82fa0 100644 --- a/daemon/session2.h +++ b/daemon/session2.h @@ -441,6 +441,10 @@ struct protolayer_iter_ctx { * automatically - layers may use it to allocate memory. */ knot_mm_t pool; + /** Careful! It's only present sometimes and read-only really. + * TODO: maybe a better mechanism should be designed instead of this. */ + struct kr_request *req; + /* callback for when the layer iteration has ended - read-only for layers: */ protolayer_finished_cb finished_cb; void *finished_cb_baton; @@ -1087,8 +1091,8 @@ int session2_unwrap_after(struct session2 *s, enum protolayer_type protocol, * Returns one of `enum protolayer_ret` or a negative number * indicating an error. */ int session2_wrap(struct session2 *s, struct protolayer_payload payload, - const struct comm_info *comm, protolayer_finished_cb cb, - void *baton); + const struct comm_info *comm, struct kr_request *req, + protolayer_finished_cb cb, void *baton); /** Same as `session2_wrap`, but looks up the specified `protocol` in the * session's assigned protocol group and sends the `payload` to the layer that diff --git a/daemon/worker.c b/daemon/worker.c index 07480b24c..70963afdd 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -667,7 +667,7 @@ static int qr_task_send(struct qr_task *task, struct session2 *session, struct protolayer_payload payload = protolayer_payload_buffer( (char *)pkt->wire, pkt->size, false); payload.ttl = packet_ttl(pkt); - ret = session2_wrap(session, payload, comm, qr_task_wrap_finished, task); + ret = session2_wrap(session, payload, comm, &task->ctx->req, qr_task_wrap_finished, task); if (ret >= 0) { session2_touch(session); diff --git a/lib/resolve.h b/lib/resolve.h index ef4f3f467..51d47925a 100644 --- a/lib/resolve.h +++ b/lib/resolve.h @@ -251,7 +251,11 @@ struct kr_request { uint32_t price_factor16; size_t size; /**< query packet size */ int32_t stream_id; /**< HTTP/2 stream ID for DoH requests */ - kr_http_header_array_t headers; /**< HTTP/2 headers for DoH requests */ + /** HTTP/2 headers for DoH requests + * + * Note that this owns malloc-ed memory inside (outside ->pool). + */ + kr_http_header_array_t headers; } qsource; struct { unsigned rtt; /**< Current upstream RTT */