From: Oto Šťáva Date: Tue, 1 Aug 2023 14:36:53 +0000 (+0200) Subject: daemon: more avoidance of excessive TCP reconnections X-Git-Tag: v6.0.2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7aec8ebdf1428afcb7f5bc62764149ffeaf3d3fe;p=thirdparty%2Fknot-resolver.git daemon: more avoidance of excessive TCP reconnections Previously this penalization was only triggered if the remote server closed TCP. Now it's extended to us closing it when the server (only) sends back some nonsense. At least for the cases which I could see immediately. That's just three trivial one-line additions; the rest is refactoring. Adapted to 6.0 from commit 6468ab22 by Oto Šťáva Co-Authored-By: Vladimír Čunat --- diff --git a/daemon/io.c b/daemon/io.c index 598d6ada4..c077d91bb 100644 --- a/daemon/io.c +++ b/daemon/io.c @@ -466,33 +466,6 @@ int io_listen_udp(uv_loop_t *loop, uv_udp_t *handle, int fd) } -static void tcp_disconnect(struct session2 *s, int errcode) -{ - if (kr_log_is_debug(IO, NULL)) { - struct sockaddr *peer = session2_get_peer(s); - char *peer_str = kr_straddr(peer); - kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n", - peer_str ? peer_str : "", - uv_strerror(errcode)); - } - - if (!s->was_useful && s->outgoing) { - /* We want to penalize the IP address, if a task is asking a query. - * It might not be the right task, but that doesn't matter so much - * for attributing the useless session to the IP address. */ - struct qr_task *t = session2_tasklist_get_first(s); - struct kr_query *qry = NULL; - if (t) { - struct kr_request *req = worker_task_request(t); - qry = array_tail(req->rplan.pending); - } - if (qry) /* We reuse the error for connection, as it's quite similar. */ - qry->server_selection.error(qry, worker_task_get_transport(t), - KR_SELECTION_TCP_CONNECT_FAILED); - } - worker_end_tcp(s); -} - static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) { struct session2 *s = handle->data; @@ -510,7 +483,15 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) } if (nread < 0 || !buf->base) { - tcp_disconnect(s, nread); + if (kr_log_is_debug(IO, NULL)) { + struct sockaddr *peer = session2_get_peer(s); + char *peer_str = kr_straddr(peer); + kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n", + peer_str ? peer_str : "", + uv_strerror(nread)); + } + session2_penalize(s); + worker_end_tcp(s); return; } diff --git a/daemon/session2.c b/daemon/session2.c index 491f0e6a0..c3d5765a9 100644 --- a/daemon/session2.c +++ b/daemon/session2.c @@ -1154,6 +1154,25 @@ void session2_waitinglist_finalize(struct session2 *session, int status) } } +void session2_penalize(struct session2 *session) +{ + if (session->was_useful || !session->outgoing) + return; + + /* We want to penalize the IP address, if a task is asking a query. + * It might not be the right task, but that doesn't matter so much + * for attributing the useless session to the IP address. */ + struct qr_task *t = session2_tasklist_get_first(session); + struct kr_query *qry = NULL; + if (t) { + struct kr_request *req = worker_task_request(t); + qry = array_tail(req->rplan.pending); + } + if (qry) /* We reuse the error for connection, as it's quite similar. */ + qry->server_selection.error(qry, worker_task_get_transport(t), + KR_SELECTION_TCP_CONNECT_FAILED); +} + int session2_unwrap(struct session2 *s, struct protolayer_payload payload, const struct comm_info *comm, protolayer_finished_cb cb, void *baton) diff --git a/daemon/session2.h b/daemon/session2.h index 133bb10ff..2a25e9d11 100644 --- a/daemon/session2.h +++ b/daemon/session2.h @@ -993,6 +993,11 @@ static inline bool session2_is_empty(const struct session2 *session) session2_waitinglist_is_empty(session); } +/** Penalizes the server the specified `session` is connected to, if the session + * has not been useful (see `struct session2::was_useful`). Only applies to + * `outgoing` sessions, and the session should not be connection-less. */ +void session2_penalize(struct session2 *session); + /** Sends the specified `payload` to be processed in the `_UNWRAP` direction by * the session's protocol layers. * diff --git a/daemon/worker.c b/daemon/worker.c index 8d0a6b28d..2d293ba9f 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -41,6 +41,8 @@ #define MAX_PIPELINED 100 #endif +#define MAX_DGRAM_LEN UINT16_MAX + #define VERBOSE_MSG(qry, ...) kr_log_q(qry, WORKER, __VA_ARGS__) /** Client request state. */ @@ -1757,6 +1759,12 @@ static enum protolayer_iter_cb_result pl_dns_dgram_unwrap( int ret = kr_ok(); for (int i = 0; i < ctx->payload.iovec.cnt; i++) { const struct iovec *iov = &ctx->payload.iovec.iov[i]; + if (iov->iov_len > MAX_DGRAM_LEN) { + session2_penalize(session); + ret = kr_error(EFBIG); + break; + } + knot_pkt_t *pkt = produce_packet( iov->iov_base, iov->iov_len); if (!pkt) { @@ -1772,6 +1780,10 @@ static enum protolayer_iter_cb_result pl_dns_dgram_unwrap( mp_flush(the_worker->pkt_pool.ctx); return protolayer_break(ctx, ret); } else if (ctx->payload.type == PROTOLAYER_PAYLOAD_BUFFER) { + if (ctx->payload.buffer.len > MAX_DGRAM_LEN) { + session2_penalize(session); + return protolayer_break(ctx, kr_error(EFBIG)); + } knot_pkt_t *pkt = produce_packet( ctx->payload.buffer.buf, ctx->payload.buffer.len); @@ -1782,9 +1794,15 @@ static enum protolayer_iter_cb_result pl_dns_dgram_unwrap( mp_flush(the_worker->pkt_pool.ctx); return protolayer_break(ctx, ret); } else if (ctx->payload.type == PROTOLAYER_PAYLOAD_WIRE_BUF) { + const size_t msg_len = wire_buf_data_length(ctx->payload.wire_buf); + if (msg_len > MAX_DGRAM_LEN) { + session2_penalize(session); + return protolayer_break(ctx, kr_error(EFBIG)); + } + knot_pkt_t *pkt = produce_packet( wire_buf_data(ctx->payload.wire_buf), - wire_buf_data_length(ctx->payload.wire_buf)); + msg_len); if (!pkt) return protolayer_break(ctx, KNOT_EMALF); @@ -2079,10 +2097,12 @@ static knot_pkt_t *stream_produce_packet(struct session2 *session, uint16_t msg_len = knot_wire_read_u16(wire_buf_data(wb)); if (msg_len == 0) { *out_err = true; + session2_penalize(session); return NULL; } if (msg_len >= wb->size) { *out_err = true; + session2_penalize(session); return NULL; } if (wire_buf_data_length(wb) < msg_len + sizeof(uint16_t)) {