From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Thu, 19 Jan 2023 17:54:11 +0000 (+0100)
Subject: daemon/io: penalize servers that close without reply
X-Git-Tag: v5.6.0~1^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3e28a8a64458a326b8091af2dabbe67be9203f6b;p=thirdparty%2Fknot-resolver.git

daemon/io: penalize servers that close without reply
---

diff --git a/daemon/io.c b/daemon/io.c
index 47aecccb1..48bfed301 100644
--- a/daemon/io.c
+++ b/daemon/io.c
@@ -337,6 +337,33 @@ void tcp_timeout_trigger(uv_timer_t *timer)
 	}
 }
 
+static void tcp_disconnect(struct session *s, int errcode)
+{
+	if (kr_log_is_debug(IO, NULL)) {
+		struct sockaddr *peer = session_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 (!session_was_useful(s) && session_flags(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 = session_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 session *s = handle->data;
@@ -354,14 +381,7 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
 	}
 
 	if (nread < 0 || !buf->base) {
-		if (kr_log_is_debug(IO, NULL)) {
-			struct sockaddr *peer = session_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));
-		}
-		worker_end_tcp(s);
+		tcp_disconnect(s, nread);
 		return;
 	}
 
diff --git a/daemon/session.c b/daemon/session.c
index 97256be24..a1f22072b 100644
--- a/daemon/session.c
+++ b/daemon/session.c
@@ -51,6 +51,7 @@ struct session {
 	ssize_t wire_buf_end_idx;     /**< Data end offset in wire_buf. */
 	uint64_t last_activity;       /**< Time of last IO activity (if any occurs).
 				       *   Otherwise session creation time. */
+	bool was_useful;              /**< I.e. produced a DNS message at some point. */
 };
 
 static void on_session_close(uv_handle_t *handle)
@@ -122,6 +123,11 @@ void session_close(struct session *session)
 	}
 }
 
+bool session_was_useful(const struct session *session)
+{
+	return session->was_useful;
+}
+
 int session_start_read(struct session *session)
 {
 	return io_start_read(session->handle);
@@ -628,7 +634,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
 		return NULL;
 	}
 
-
+	session->was_useful = true;
 	knot_pkt_t *pkt = knot_pkt_new(msg_start, msg_size, mm);
 	session->sflags.wirebuf_error = (pkt == NULL);
 	return pkt;
diff --git a/daemon/session.h b/daemon/session.h
index eccf45b5f..603d7cb45 100644
--- a/daemon/session.h
+++ b/daemon/session.h
@@ -91,6 +91,8 @@ int session_tasklist_finalize_expired(struct session *session);
 /** Both of task lists (associated & waiting). */
 /** Check if empty. */
 bool session_is_empty(const struct session *session);
+/** Return whether session seems to have done something useful. */
+bool session_was_useful(const struct session *session);
 /** Get pointer to session flags */
 struct session_flags *session_flags(struct session *session);
 /** Get pointer to peer address. */
diff --git a/daemon/worker.c b/daemon/worker.c
index c8feb16ac..8b6b49e67 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -2111,6 +2111,11 @@ struct request_ctx *worker_task_get_request(struct qr_task *task)
 	return task->ctx;
 }
 
+struct kr_transport *worker_task_get_transport(struct qr_task *task)
+{
+	return task->transport;
+}
+
 struct session *worker_request_get_source_session(const struct kr_request *req)
 {
 	static_assert(offsetof(struct request_ctx, req) == 0,
diff --git a/daemon/worker.h b/daemon/worker.h
index 8885aebb5..fd9b1f3a0 100644
--- a/daemon/worker.h
+++ b/daemon/worker.h
@@ -108,6 +108,8 @@ knot_pkt_t *worker_task_get_pktbuf(const struct qr_task *task);
 
 struct request_ctx *worker_task_get_request(struct qr_task *task);
 
+struct kr_transport *worker_task_get_transport(struct qr_task *task);
+
 /** Note: source session is NULL in case the request hasn't come over network. */
 KR_EXPORT struct session *worker_request_get_source_session(const struct kr_request *req);