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 <oto.stava@nic.cz>

Co-Authored-By: Vladimír Čunat <vladimir.cunat@nic.cz>

diff --git a/daemon/io.c b/daemon/io.c
index 598d6ada49209571efd0151e40ef6f0601c83e2d..c077d91bb6ae9e0296319b58079d1061c0ec81de 100644 (file)
--- 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 491f0e6a02056f53260ce26c7c990437adf4d657..c3d5765a9229e9e20375baa340dfb5a1fe9852fa 100644 (file)
--- 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 133bb10fffbbba3aaccb7d290741d8583044f25c..2a25e9d11b7d59bd4bbe88a41a762e365bb928a5 100644 (file)
--- 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 8d0a6b28d320e5c7e64fc7799ab4e94035bd7899..2d293ba9f1ba91f5d023f1c754eca68b30496011 100644 (file)
--- 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)) {