From 1ebf851bf0906dab0739fed9426c8184e2f59746 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 2 Dec 2020 09:51:26 +0100 Subject: [PATCH 1/2] - Fix #360: for the additionally reported TCP Fast Open makes TCP connections fail, in that case we print a hint that this is happening with the error in the logs. --- doc/Changelog | 5 +++++ util/netevent.c | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/doc/Changelog b/doc/Changelog index 30b8d34a1..1e895f9f2 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +2 December 2020: Wouter + - Fix #360: for the additionally reported TCP Fast Open makes TCP + connections fail, in that case we print a hint that this is + happening with the error in the logs. + 1 December 2020: Wouter - Fix #358: Squelch udp connect 'no route to host' errors on low verbosity. diff --git a/util/netevent.c b/util/netevent.c index 311a114ee..1a5d22892 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -1594,6 +1594,13 @@ comm_point_tcp_handle_read(int fd, struct comm_point* c, int short_ok) if(errno == ECONNRESET && verbosity < 2) return 0; /* silence reset by peer */ #endif +#ifdef ENOTCONN + if(errno == ENOTCONN) { + log_err_addr("read (in tcp s) failed and this could be because TCP Fast Open is enabled [--disable-tfo-client --disable-tfo-server] but does not work", sock_strerror(errno), + &c->repinfo.addr, c->repinfo.addrlen); + return 0; + } +#endif #else /* USE_WINSOCK */ if(WSAGetLastError() == WSAECONNRESET) return 0; From 16c496bff66faecef693ecadbd386f861b553752 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Wed, 2 Dec 2020 10:10:27 +0100 Subject: [PATCH 2/2] - Fix #356: deadlock when listening tcp. --- doc/Changelog | 1 + services/listen_dnsport.c | 10 +++++----- util/netevent.c | 28 +++++++++++++++++----------- util/netevent.h | 10 ++++++++++ 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index 1e895f9f2..405060fc4 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -2,6 +2,7 @@ - Fix #360: for the additionally reported TCP Fast Open makes TCP connections fail, in that case we print a hint that this is happening with the error in the logs. + - Fix #356: deadlock when listening tcp. 1 December 2020: Wouter - Fix #358: Squelch udp connect 'no route to host' errors on low diff --git a/services/listen_dnsport.c b/services/listen_dnsport.c index d63c0e0aa..709c9e6ce 100644 --- a/services/listen_dnsport.c +++ b/services/listen_dnsport.c @@ -1821,12 +1821,12 @@ tcp_req_info_setup_listen(struct tcp_req_info* req) req->cp->tcp_is_reading = 0; comm_point_stop_listening(req->cp); comm_point_start_listening(req->cp, -1, - req->cp->tcp_timeout_msec); + adjusted_tcp_timeout(req->cp)); } else if(rd) { req->cp->tcp_is_reading = 1; comm_point_stop_listening(req->cp); comm_point_start_listening(req->cp, -1, - req->cp->tcp_timeout_msec); + adjusted_tcp_timeout(req->cp)); /* and also read it (from SSL stack buffers), so * no event read event is expected since the remainder of * the TLS frame is sitting in the buffers. */ @@ -1834,7 +1834,7 @@ tcp_req_info_setup_listen(struct tcp_req_info* req) } else { comm_point_stop_listening(req->cp); comm_point_start_listening(req->cp, -1, - req->cp->tcp_timeout_msec); + adjusted_tcp_timeout(req->cp)); comm_point_listen_for_rw(req->cp, 0, 0); } } @@ -1947,7 +1947,7 @@ tcp_req_info_handle_readdone(struct tcp_req_info* req) send_it: c->tcp_is_reading = 0; comm_point_stop_listening(c); - comm_point_start_listening(c, -1, c->tcp_timeout_msec); + comm_point_start_listening(c, -1, adjusted_tcp_timeout(c)); return; } req->in_worker_handle = 0; @@ -2065,7 +2065,7 @@ tcp_req_info_send_reply(struct tcp_req_info* req) /* switch to listen to write events */ comm_point_stop_listening(req->cp); comm_point_start_listening(req->cp, -1, - req->cp->tcp_timeout_msec); + adjusted_tcp_timeout(req->cp)); return; } /* queue up the answer behind the others already pending */ diff --git a/util/netevent.c b/util/netevent.c index 1a5d22892..98796a788 100644 --- a/util/netevent.c +++ b/util/netevent.c @@ -762,6 +762,13 @@ comm_point_udp_callback(int fd, short event, void* arg) } } +int adjusted_tcp_timeout(struct comm_point* c) +{ + if(c->tcp_timeout_msec < TCP_QUERY_TIMEOUT_MINIMUM) + return TCP_QUERY_TIMEOUT_MINIMUM; + return c->tcp_timeout_msec; +} + /** Use a new tcp handler for new query fd, set to read query */ static void setup_tcp_handler(struct comm_point* c, int fd, int cur, int max) @@ -795,10 +802,7 @@ setup_tcp_handler(struct comm_point* c, int fd, int cur, int max) c->tcp_timeout_msec /= 500; else if (handler_usage > 80) c->tcp_timeout_msec = 0; - comm_point_start_listening(c, fd, - c->tcp_timeout_msec < TCP_QUERY_TIMEOUT_MINIMUM - ? TCP_QUERY_TIMEOUT_MINIMUM - : c->tcp_timeout_msec); + comm_point_start_listening(c, fd, adjusted_tcp_timeout(c)); } void comm_base_handle_slow_accept(int ATTR_UNUSED(fd), @@ -1108,10 +1112,11 @@ tcp_callback_writer(struct comm_point* c) if( (*c->callback)(c, c->cb_arg, NETEVENT_PKT_WRITTEN, &c->repinfo) ) { comm_point_start_listening(c, -1, - c->tcp_timeout_msec); + adjusted_tcp_timeout(c)); } } else { - comm_point_start_listening(c, -1, c->tcp_timeout_msec); + comm_point_start_listening(c, -1, + adjusted_tcp_timeout(c)); } } } @@ -1132,7 +1137,8 @@ tcp_callback_reader(struct comm_point* c) comm_point_stop_listening(c); fptr_ok(fptr_whitelist_comm_point(c->callback)); if( (*c->callback)(c, c->cb_arg, NETEVENT_NOERROR, &c->repinfo) ) { - comm_point_start_listening(c, -1, c->tcp_timeout_msec); + comm_point_start_listening(c, -1, + adjusted_tcp_timeout(c)); } } } @@ -2656,7 +2662,7 @@ comm_point_http2_handle_read(int ATTR_UNUSED(fd), struct comm_point* c) if(nghttp2_session_want_write(c->h2_session->session)) { c->tcp_is_reading = 0; comm_point_stop_listening(c); - comm_point_start_listening(c, -1, c->tcp_timeout_msec); + comm_point_start_listening(c, -1, adjusted_tcp_timeout(c)); } else if(!nghttp2_session_want_read(c->h2_session->session)) return 0; /* connection can be closed */ return 1; @@ -2974,7 +2980,7 @@ comm_point_http2_handle_write(int ATTR_UNUSED(fd), struct comm_point* c) if(nghttp2_session_want_read(c->h2_session->session)) { c->tcp_is_reading = 1; comm_point_stop_listening(c); - comm_point_start_listening(c, -1, c->tcp_timeout_msec); + comm_point_start_listening(c, -1, adjusted_tcp_timeout(c)); } else if(!nghttp2_session_want_write(c->h2_session->session)) return 0; /* connection can be closed */ return 1; @@ -3932,11 +3938,11 @@ comm_point_send_reply(struct comm_reply *repinfo) repinfo->c->tcp_is_reading = 0; comm_point_stop_listening(repinfo->c); comm_point_start_listening(repinfo->c, -1, - repinfo->c->tcp_timeout_msec); + adjusted_tcp_timeout(repinfo->c)); return; } else { comm_point_start_listening(repinfo->c, -1, - repinfo->c->tcp_timeout_msec); + adjusted_tcp_timeout(repinfo->c)); } } } diff --git a/util/netevent.h b/util/netevent.h index daa954b64..266a74ff3 100644 --- a/util/netevent.h +++ b/util/netevent.h @@ -661,6 +661,16 @@ void comm_point_start_listening(struct comm_point* c, int newfd, int msec); */ void comm_point_listen_for_rw(struct comm_point* c, int rd, int wr); +/** + * For TCP handlers that use c->tcp_timeout_msec, this routine adjusts + * it with the minimum. Otherwise, a 0 value advertised without the + * minimum applied moves to a 0 in comm_point_start_listening and that + * routine treats it as no timeout, listen forever, which is not wanted. + * @param c: comm point to use the tcp_timeout_msec of. + * @return adjusted tcp_timeout_msec value with the minimum if smaller. + */ +int adjusted_tcp_timeout(struct comm_point* c); + /** * Get size of memory used by comm point. * For TCP handlers this includes subhandlers.