1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
|
From: =?utf-8?b?VmxhZGltw61yIMSMdW7DoXQ=?= <vladimir.cunat@nic.cz>
Date: Sat, 29 Jul 2023 17:53:34 +0200
Subject: 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.
---
daemon/io.c | 38 ++++++++++----------------------------
daemon/session.c | 26 +++++++++++++++++++++-----
daemon/session.h | 5 +++--
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/daemon/io.c b/daemon/io.c
index 48bfed3..ac0d969 100644
--- a/daemon/io.c
+++ b/daemon/io.c
@@ -337,33 +337,6 @@ 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;
@@ -381,7 +354,16 @@ 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 = 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));
+ }
+
+ session_tcp_penalize(s);
+ worker_end_tcp(s);
return;
}
diff --git a/daemon/session.c b/daemon/session.c
index a1f2207..ed0ff68 100644
--- a/daemon/session.c
+++ b/daemon/session.c
@@ -123,11 +123,6 @@ 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);
@@ -582,6 +577,24 @@ ssize_t session_wirebuf_trim(struct session *session, ssize_t len)
return len;
}
+void session_tcp_penalize(struct session *s)
+{
+ if (s->was_useful || !s->sflags.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 = 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);
+}
+
knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
{
session->sflags.wirebuf_error = false;
@@ -617,6 +630,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
msg_size = knot_wire_read_u16(msg_start);
if (msg_size >= session->wire_buf_size) {
session->sflags.wirebuf_error = true;
+ session_tcp_penalize(session);
return NULL;
}
if (msg_size + 2 > wirebuf_msg_data_size) {
@@ -624,6 +638,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
}
if (msg_size == 0) {
session->sflags.wirebuf_error = true;
+ session_tcp_penalize(session);
return NULL;
}
msg_start += 2;
@@ -631,6 +646,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
msg_size = wirebuf_msg_data_size;
} else {
session->sflags.wirebuf_error = true;
+ session_tcp_penalize(session);
return NULL;
}
diff --git a/daemon/session.h b/daemon/session.h
index 603d7cb..1f95ac5 100644
--- a/daemon/session.h
+++ b/daemon/session.h
@@ -91,8 +91,9 @@ 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);
+/** Penalize this server if the session hasn't been useful (and is outgoing). */
+void session_tcp_penalize(struct session *session);
+
/** Get pointer to session flags */
struct session_flags *session_flags(struct session *session);
/** Get pointer to peer address. */
|
