From: Grigorii Demidov Date: Wed, 18 Apr 2018 15:12:36 +0000 (+0200) Subject: daemon/worker: adjust tcp timeouts X-Git-Tag: v2.3.0^2~2^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2e4d4be41ddc4da786caaa7495fa07e712c8bba9;p=thirdparty%2Fknot-resolver.git daemon/worker: adjust tcp timeouts This is an attempt to fix two problems: 1. kresd tries to close incoming TCP connection too early. This may lead to multiple client reconnections. This problem primarily affects TCP/TLS clients who send several queries over single TCP connection. 2. In certain circumstances outbound TCP connection doesn't timeout despite that fact that upstream doesn't send back any answers. This may lead to timeouts on non-problematic queries. --- diff --git a/daemon/io.c b/daemon/io.c index fa1ac7f4f..63bca9f71 100644 --- a/daemon/io.c +++ b/daemon/io.c @@ -252,7 +252,8 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf) if (s->tasks.len == 0) { worker_session_close(s); } else { /* If there are tasks running, defer until they finish. */ - uv_timer_start(&s->timeout, tcp_timeout_trigger, 1, KR_CONN_RTT_MAX/2); + uv_timer_start(&s->timeout, tcp_timeout_trigger, + MAX_TCP_INACTIVITY, MAX_TCP_INACTIVITY); } } /* Connection spawned at least one request, reset its deadline for next query. @@ -294,14 +295,18 @@ static void _tcp_accept(uv_stream_t *master, int status, bool tls) return; } + uint64_t timeout = KR_CONN_RTT_MAX / 2; session->has_tls = tls; - if (tls && !session->tls_ctx) { - session->tls_ctx = tls_new(master->loop->data); - session->tls_ctx->c.session = session; - session->tls_ctx->c.handshake_state = TLS_HS_IN_PROGRESS; + if (tls) { + timeout += KR_CONN_RTT_MAX * 3; + if (!session->tls_ctx) { + session->tls_ctx = tls_new(master->loop->data); + session->tls_ctx->c.session = session; + session->tls_ctx->c.handshake_state = TLS_HS_IN_PROGRESS; + } } uv_timer_t *timer = &session->timeout; - uv_timer_start(timer, tcp_timeout_trigger, KR_CONN_RTT_MAX/2, KR_CONN_RTT_MAX/2); + uv_timer_start(timer, tcp_timeout_trigger, timeout, timeout); io_start_read((uv_handle_t *)client); } diff --git a/daemon/tls.c b/daemon/tls.c index 77ab64644..4e7b457b6 100644 --- a/daemon/tls.c +++ b/daemon/tls.c @@ -890,7 +890,7 @@ int tls_client_connect_start(struct tls_client_ctx_t *client_ctx, struct tls_common_ctx *ctx = &client_ctx->c; gnutls_session_set_ptr(ctx->tls_session, client_ctx); - gnutls_handshake_set_timeout(ctx->tls_session, 5000); + gnutls_handshake_set_timeout(ctx->tls_session, KR_CONN_RTT_MAX * 3); session->tls_client_ctx = client_ctx; ctx->handshake_cb = handshake_cb; ctx->handshake_state = TLS_HS_IN_PROGRESS; diff --git a/daemon/worker.c b/daemon/worker.c index 2469d4ed4..ff398bbbc 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -826,12 +826,7 @@ static int qr_task_on_send(struct qr_task *task, uv_handle_t *handle, int status if (session->waiting.len > 0) { struct qr_task *t = session->waiting.at[0]; int ret = qr_task_send(t, handle, &session->peer.ip, t->pktbuf); - uv_timer_t *timer = &session->timeout; - uv_timer_stop(timer); - if (ret == kr_ok()) { - session->timeout.data = session; - timer_start(session, on_tcp_watchdog_timeout, MAX_TCP_INACTIVITY, 0); - } else { + if (ret != kr_ok()) { while (session->waiting.len > 0) { struct qr_task *t = session->waiting.at[0]; if (session->outgoing) { @@ -1079,9 +1074,6 @@ static int session_next_waiting_send(struct session *session) struct qr_task *task = session->waiting.at[0]; ret = qr_task_send(task, session->handle, &peer->ip, task->pktbuf); } - uv_timer_stop(&session->timeout); - session->timeout.data = session; - timer_start(session, on_tcp_watchdog_timeout, MAX_TCP_INACTIVITY, 0); return ret; } @@ -1120,6 +1112,10 @@ static int session_tls_hs_cb(struct session *session, int status) worker_del_tcp_connected(worker, &peer->ip); assert(session->tasks.len == 0); session_close(session); + } else { + uv_timer_stop(&session->timeout); + session->timeout.data = session; + timer_start(session, on_tcp_watchdog_timeout, MAX_TCP_INACTIVITY, 0); } return kr_ok(); } @@ -1223,6 +1219,7 @@ static void on_connect(uv_connect_t *req, int status) if (ret == kr_ok()) { ret = session_next_waiting_send(session); if (ret == kr_ok()) { + timer_start(session, on_tcp_watchdog_timeout, MAX_TCP_INACTIVITY, 0); worker_add_tcp_connected(worker, &session->peer.ip, session); iorequest_release(worker, req); return; @@ -1290,7 +1287,6 @@ static void on_tcp_watchdog_timeout(uv_timer_t *timer) assert(session->outgoing); uv_timer_stop(timer); struct worker_ctx *worker = get_worker(); - if (session->outgoing) { if (session->has_tls) { worker_del_tcp_waiting(worker, &session->peer.ip); @@ -1692,9 +1688,11 @@ static int qr_task_step(struct qr_task *task, session_close(session); return qr_task_finalize(task, KR_STATE_FAIL); } - uv_timer_stop(&session->timeout); - ret = timer_start(session, on_tcp_watchdog_timeout, - MAX_TCP_INACTIVITY, 0); + if (session->tasks.len == 1) { + uv_timer_stop(&session->timeout); + ret = timer_start(session, on_tcp_watchdog_timeout, + MAX_TCP_INACTIVITY, 0); + } if (ret < 0) { session_del_waiting(session, task); session_del_tasks(session, task); @@ -2280,17 +2278,18 @@ int worker_process_tcp(struct worker_ctx *worker, uv_stream_t *handle, /* Message was assembled, clear temporary. */ session->buffering = NULL; session->msg_hdr_idx = 0; + if (session->outgoing) { + session_del_tasks(session, task); + } /* Parse the packet and start resolving complete query */ int ret = parse_packet(pkt_buf); if (ret == 0) { if (session->outgoing) { - session_del_tasks(session, task); /* To prevent slow lorris attack restart watchdog only after * the whole message was successfully assembled and parsed */ uv_timer_stop(&session->timeout); - if (session->tasks.len > 0) { - timer_start(session, on_tcp_watchdog_timeout, - MAX_TCP_INACTIVITY, 0); + if (session->tasks.len > 0 || session->waiting.len > 0) { + timer_start(session, on_tcp_watchdog_timeout, MAX_TCP_INACTIVITY, 0); } } else { /* Start only new queries, @@ -2307,6 +2306,8 @@ int worker_process_tcp(struct worker_ctx *worker, uv_stream_t *handle, assert(false); } } + } + if (ret == 0) { const struct sockaddr *addr = session->outgoing ? &session->peer.ip : NULL; /* since there can be next dns message, we must to proceed * even if qr_task_step() returns error */