From b6d3b710f52a158084e1e2e2dd5346a874b3f9de Mon Sep 17 00:00:00 2001 From: Shivan Sahib Date: Sun, 11 Oct 2020 19:22:02 +0000 Subject: [PATCH 01/10] WIP: Persistent TCP connections Add logging --- xfrd-tcp.c | 47 ++++++++++++++++++++++++++++++----------------- xfrd.c | 2 +- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/xfrd-tcp.c b/xfrd-tcp.c index d00c13b75..4fef6d20c 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -360,6 +360,7 @@ xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) } if((event & EV_TIMEOUT) && tp->handler_added) { /* tcp connection timed out */ + VERBOSITY(1, (LOG_INFO, "xyzzy TCP conn timed out")); DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: event tcp timeout")); xfrd_tcp_pipe_stop(tp); } @@ -407,7 +408,27 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) assert(zone->tcp_conn == -1); assert(zone->tcp_waiting == 0); + VERBOSITY(1, (LOG_INFO, "xyzzy in xfrd_tcp_obtain, total number of TCP conns is %d", set->tcp_count)); + + /* check for a pipeline to the same master with unused ID */ + if((tp = pipeline_find(set, zone))!= NULL) { + VERBOSITY(1, (LOG_INFO, "xyzzy pipeline found for %s!", zone->apex_str)); + int i; + if(zone->zone_handler.ev_fd != -1) + xfrd_udp_release(zone); + for(i=0; itcp_state[i] == tp) + zone->tcp_conn = i; + } + xfrd_deactivate_zone(zone); + xfrd_unset_timer(zone); + pipeline_setup_new_zone(set, tp, zone); + return; + } + + if(set->tcp_count < XFRD_MAX_TCP) { + VERBOSITY(1, (LOG_INFO, "xyzzy tcp count for %s < max tcp!", zone->apex_str)); int i; assert(!set->tcp_waiting_first); set->tcp_count ++; @@ -454,20 +475,10 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) pipeline_setup_new_zone(set, tp, zone); return; } - /* check for a pipeline to the same master with unused ID */ - if((tp = pipeline_find(set, zone))!= NULL) { - int i; - if(zone->zone_handler.ev_fd != -1) - xfrd_udp_release(zone); - for(i=0; itcp_state[i] == tp) - zone->tcp_conn = i; - } - xfrd_deactivate_zone(zone); - xfrd_unset_timer(zone); - pipeline_setup_new_zone(set, tp, zone); - return; - } + + + + /* wait, at end of line */ DEBUG(DEBUG_XFRD,2, (LOG_INFO, "xfrd: max number of tcp " @@ -797,7 +808,7 @@ conn_read(struct xfrd_tcp* tcp) } } else if(received == 0) { /* EOF */ - return -1; + return 0; } tcp->total_bytes += received; if(tcp->total_bytes < sizeof(tcp->msglen)) { @@ -908,7 +919,9 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) xfrd_make_request(zone); break; } - xfrd_tcp_release(xfrd->tcp_set, zone); + // xyzzy we don't need this for xot + // why do we need to release after transfer is done? + //xfrd_tcp_release(xfrd->tcp_set, zone); assert(zone->round_num == -1); break; case xfrd_packet_notimpl: @@ -983,7 +996,7 @@ void xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, int conn) { - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipe released")); + VERBOSITY(1, (LOG_INFO, "xyzzy tcp pipe released")); /* one handler per tcp pipe */ if(tp->handler_added) event_del(&tp->handler); diff --git a/xfrd.c b/xfrd.c index 65d6d955a..c36c0caa7 100644 --- a/xfrd.c +++ b/xfrd.c @@ -1185,7 +1185,7 @@ xfrd_handle_incoming_soa(xfrd_zone_type* zone, if(zone->soa_disk_acquired && soa->serial == zone->soa_disk.serial) { /* soa in disk has been loaded in memory */ - log_msg(LOG_INFO, "zone %s serial %u is updated to %u", + log_msg(LOG_INFO, "xyzzy zone %s serial %u is updated to %u", zone->apex_str, (unsigned)ntohl(zone->soa_nsd.serial), (unsigned)ntohl(soa->serial)); zone->soa_nsd = zone->soa_disk; From 9c8ec8bd46472dafad401dbc9ccd166cd41eeb09 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Wed, 14 Oct 2020 16:38:25 +0100 Subject: [PATCH 02/10] pipelining... --- xfrd-tcp.c | 51 +++++++++++++++++++++++++++++++++++---------------- xfrd.c | 4 ++-- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/xfrd-tcp.c b/xfrd-tcp.c index 4fef6d20c..c7521a310 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -344,6 +344,9 @@ tcp_pipe_reset_timeout(struct xfrd_tcp_pipeline* tp) void xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) { + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** xyzzy xfrd_handle_tcp_pipe event %d", + event)); + struct xfrd_tcp_pipeline* tp = (struct xfrd_tcp_pipeline*)arg; if((event & EV_WRITE)) { tcp_pipe_reset_timeout(tp); @@ -360,9 +363,16 @@ xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) } if((event & EV_TIMEOUT) && tp->handler_added) { /* tcp connection timed out */ - VERBOSITY(1, (LOG_INFO, "xyzzy TCP conn timed out")); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: event tcp timeout")); - xfrd_tcp_pipe_stop(tp); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** event tcp timeout")); + /* TODO: We probably want to handle timeouts related to errors (where there + are outstanding queries) and timeouts due to a keepalive timer expiring + with more finesse and different time limits?*/ + /* pipe is unused (timeout triggered after inactivity), just release */ + if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) + xfrd_tcp_pipe_release(xfrd->tcp_set, tp, -1); + else + /* do cleanup before releasing */ + xfrd_tcp_pipe_stop(tp); } } @@ -408,11 +418,11 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) assert(zone->tcp_conn == -1); assert(zone->tcp_waiting == 0); - VERBOSITY(1, (LOG_INFO, "xyzzy in xfrd_tcp_obtain, total number of TCP conns is %d", set->tcp_count)); + VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy in xfrd_tcp_obtain, total number of TCP conns is %d", set->tcp_count)); /* check for a pipeline to the same master with unused ID */ if((tp = pipeline_find(set, zone))!= NULL) { - VERBOSITY(1, (LOG_INFO, "xyzzy pipeline found for %s!", zone->apex_str)); + VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy pipeline found for %s!", zone->apex_str)); int i; if(zone->zone_handler.ev_fd != -1) xfrd_udp_release(zone); @@ -428,7 +438,7 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) if(set->tcp_count < XFRD_MAX_TCP) { - VERBOSITY(1, (LOG_INFO, "xyzzy tcp count for %s < max tcp!", zone->apex_str)); + VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy opening new tcp conn, tcp count for %s < max tcp", zone->apex_str)); int i; assert(!set->tcp_waiting_first); set->tcp_count ++; @@ -808,7 +818,7 @@ conn_read(struct xfrd_tcp* tcp) } } else if(received == 0) { /* EOF */ - return 0; + return -1; } tcp->total_bytes += received; if(tcp->total_bytes < sizeof(tcp->msglen)) { @@ -873,6 +883,8 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) ret = conn_read(tcp); if(ret == -1) { + DEBUG(DEBUG_XFRD,1, (LOG_INFO, + "xfrd: *** xyzzy conn_read returned -1, stopping pipe")); xfrd_tcp_pipe_stop(tp); return; } @@ -919,9 +931,7 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) xfrd_make_request(zone); break; } - // xyzzy we don't need this for xot - // why do we need to release after transfer is done? - //xfrd_tcp_release(xfrd->tcp_set, zone); + xfrd_tcp_release(xfrd->tcp_set, zone); assert(zone->round_num == -1); break; case xfrd_packet_notimpl: @@ -948,7 +958,7 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { int conn = zone->tcp_conn; struct xfrd_tcp_pipeline* tp = set->tcp_state[conn]; - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s released tcp conn to %s", + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** zone %s mapping to tcp conn of %s released", zone->apex_str, zone->master->ip_address_spec)); assert(zone->tcp_conn != -1); assert(zone->tcp_waiting == 0); @@ -960,8 +970,8 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) /* remove it from the ID list */ if(tp->id[zone->query_id] != TCP_NULL_SKIP) tcp_pipe_id_remove(tp, zone); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: released tcp pipe now %d unused", - tp->num_unused)); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** zone %s removed from pipeline send list and ids, %d unused", + zone->apex_str, tp->num_unused)); /* if pipe was full, but no more, then see if waiting element is * for the same master, and can fill the unused ID */ if(tp->num_unused == 1 && set->tcp_waiting_first) { @@ -988,15 +998,15 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) } /* if all unused, or only skipped leftover, close the pipeline */ - if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) - xfrd_tcp_pipe_release(set, tp, conn); + // if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) + // xfrd_tcp_pipe_release(set, tp, conn); } void xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, int conn) { - VERBOSITY(1, (LOG_INFO, "xyzzy tcp pipe released")); + VERBOSITY(1, (LOG_INFO, "xfrd: *** tcp pipe released")); /* one handler per tcp pipe */ if(tp->handler_added) event_del(&tp->handler); @@ -1011,6 +1021,15 @@ xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, /* remove from pipetree */ (void)rbtree_delete(xfrd->tcp_set->pipetree, &tp->node); + /* find the connection for this pipeline so it can be re-used*/ + if (conn == -1) { + for(int i=0; itcp_state[i] == tp) + conn = i; + } + } + assert(conn != -1); + /* a waiting zone can use the free tcp slot (to another server) */ /* if that zone fails to set-up or connect, we try to start the next * waiting zone in the list */ diff --git a/xfrd.c b/xfrd.c index c36c0caa7..6ea4a559c 100644 --- a/xfrd.c +++ b/xfrd.c @@ -197,7 +197,7 @@ xfrd_init(int socket, struct nsd* nsd, int shortsoa, int reload_active, #endif xfrd->tcp_set = xfrd_tcp_set_create(xfrd->region); - xfrd->tcp_set->tcp_timeout = nsd->tcp_timeout; + xfrd->tcp_set->tcp_timeout = 30; #if !defined(HAVE_ARC4RANDOM) && !defined(HAVE_GETRANDOM) srandom((unsigned long) getpid() * (unsigned long) time(NULL)); #endif @@ -1185,7 +1185,7 @@ xfrd_handle_incoming_soa(xfrd_zone_type* zone, if(zone->soa_disk_acquired && soa->serial == zone->soa_disk.serial) { /* soa in disk has been loaded in memory */ - log_msg(LOG_INFO, "xyzzy zone %s serial %u is updated to %u", + log_msg(LOG_INFO, "zone %s serial %u is updated to %u", zone->apex_str, (unsigned)ntohl(zone->soa_nsd.serial), (unsigned)ntohl(soa->serial)); zone->soa_nsd = zone->soa_disk; From b931e882968ecc74294af38c1d75185c33af592f Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Fri, 6 Nov 2020 14:10:09 +0000 Subject: [PATCH 03/10] Handle case where the far end closes an idle connection --- xfrd-tcp.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/xfrd-tcp.c b/xfrd-tcp.c index c7521a310..0e0a95fd7 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -301,22 +301,26 @@ static void xfrd_tcp_pipe_stop(struct xfrd_tcp_pipeline* tp) { int i, conn = -1; - assert(tp->num_unused < ID_PIPE_NUM); /* at least one 'in-use' */ - assert(ID_PIPE_NUM - tp->num_unused > tp->num_skip); /* at least one 'nonskip' */ - /* need to retry for all the zones connected to it */ - /* these could use different lists and go to a different nextmaster*/ - for(i=0; iid[i] && tp->id[i] != TCP_NULL_SKIP) { - xfrd_zone_type* zone = tp->id[i]; - conn = zone->tcp_conn; - zone->tcp_conn = -1; - zone->tcp_waiting = 0; - tcp_pipe_sendlist_remove(tp, zone); - tcp_pipe_id_remove(tp, zone); - xfrd_set_refresh_now(zone); - } - } - assert(conn != -1); + /* With connections left open when idle, it is possible to arrive here because the far end + shuts the idle connection (causing a read event with no data). + So just warn if we get here while the pipe is still in use */ + if (!(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused)) { + log_msg(LOG_WARNING, "xfrd: an in use TCP connection was closed by the far end, retrying all zones"); + /* need to retry for all the zones connected to it */ + /* these could use different lists and go to a different nextmaster*/ + for(i=0; iid[i] && tp->id[i] != TCP_NULL_SKIP) { + xfrd_zone_type* zone = tp->id[i]; + conn = zone->tcp_conn; + zone->tcp_conn = -1; + zone->tcp_waiting = 0; + tcp_pipe_sendlist_remove(tp, zone); + tcp_pipe_id_remove(tp, zone); + xfrd_set_refresh_now(zone); + } + } + assert(conn != -1); + } /* now release the entire tcp pipe */ xfrd_tcp_pipe_release(xfrd->tcp_set, tp, conn); } @@ -371,7 +375,7 @@ xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) xfrd_tcp_pipe_release(xfrd->tcp_set, tp, -1); else - /* do cleanup before releasing */ + /* timed out while in use, do cleanup before releasing */ xfrd_tcp_pipe_stop(tp); } } From 68320bef8a9a5372edaf96c457f2ed780335403b Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Fri, 6 Nov 2020 15:33:58 +0000 Subject: [PATCH 04/10] Clean up of code --- .gitignore | 3 +++ xfrd-tcp.c | 47 +++++++++++++++++++++-------------------------- xfrd.c | 2 +- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index bb6d0346e..ffecc6771 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,6 @@ .cproject .project .settings/ + +# Separate build directory +build/* diff --git a/xfrd-tcp.c b/xfrd-tcp.c index 0e0a95fd7..55250008a 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -301,8 +301,8 @@ static void xfrd_tcp_pipe_stop(struct xfrd_tcp_pipeline* tp) { int i, conn = -1; - /* With connections left open when idle, it is possible to arrive here because the far end - shuts the idle connection (causing a read event with no data). + /* With connections now left open when (effectively) idle, it is possible to arrive here + because the far end shuts the idle connection (causing a read event with no data). So just warn if we get here while the pipe is still in use */ if (!(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused)) { log_msg(LOG_WARNING, "xfrd: an in use TCP connection was closed by the far end, retrying all zones"); @@ -330,7 +330,12 @@ tcp_pipe_reset_timeout(struct xfrd_tcp_pipeline* tp) { int fd = tp->handler.ev_fd; struct timeval tv; - tv.tv_sec = xfrd->tcp_set->tcp_timeout; + /* pipe is effectively unused - for now set a default 10s idle timeout until EDNS0 + Keepalive is implemented */ + if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) + tv.tv_sec = 10; + else + tv.tv_sec = xfrd->tcp_set->tcp_timeout; tv.tv_usec = 0; if(tp->handler_added) event_del(&tp->handler); @@ -348,9 +353,6 @@ tcp_pipe_reset_timeout(struct xfrd_tcp_pipeline* tp) void xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) { - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** xyzzy xfrd_handle_tcp_pipe event %d", - event)); - struct xfrd_tcp_pipeline* tp = (struct xfrd_tcp_pipeline*)arg; if((event & EV_WRITE)) { tcp_pipe_reset_timeout(tp); @@ -367,13 +369,12 @@ xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) } if((event & EV_TIMEOUT) && tp->handler_added) { /* tcp connection timed out */ - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** event tcp timeout")); - /* TODO: We probably want to handle timeouts related to errors (where there - are outstanding queries) and timeouts due to a keepalive timer expiring - with more finesse and different time limits?*/ - /* pipe is unused (timeout triggered after inactivity), just release */ - if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) + /* pipe is unused (timeout triggered while idle), just release indicating + we don't have access to the conn for the pipe from here */ + if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) { + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: idle pipeline had tcp timeout event")); xfrd_tcp_pipe_release(xfrd->tcp_set, tp, -1); + } else /* timed out while in use, do cleanup before releasing */ xfrd_tcp_pipe_stop(tp); @@ -422,11 +423,8 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) assert(zone->tcp_conn == -1); assert(zone->tcp_waiting == 0); - VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy in xfrd_tcp_obtain, total number of TCP conns is %d", set->tcp_count)); - /* check for a pipeline to the same master with unused ID */ if((tp = pipeline_find(set, zone))!= NULL) { - VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy pipeline found for %s!", zone->apex_str)); int i; if(zone->zone_handler.ev_fd != -1) xfrd_udp_release(zone); @@ -437,12 +435,12 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) xfrd_deactivate_zone(zone); xfrd_unset_timer(zone); pipeline_setup_new_zone(set, tp, zone); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s is re-using pipeline %d", + zone->apex_str, i)); return; } - if(set->tcp_count < XFRD_MAX_TCP) { - VERBOSITY(1, (LOG_INFO, "xfrd: *** xyzzy opening new tcp conn, tcp count for %s < max tcp", zone->apex_str)); int i; assert(!set->tcp_waiting_first); set->tcp_count ++; @@ -487,12 +485,10 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) xfrd_deactivate_zone(zone); xfrd_unset_timer(zone); pipeline_setup_new_zone(set, tp, zone); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s opened a new tcp conn %d", + zone->apex_str, i)); return; } - - - - /* wait, at end of line */ DEBUG(DEBUG_XFRD,2, (LOG_INFO, "xfrd: max number of tcp " @@ -887,8 +883,6 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) ret = conn_read(tcp); if(ret == -1) { - DEBUG(DEBUG_XFRD,1, (LOG_INFO, - "xfrd: *** xyzzy conn_read returned -1, stopping pipe")); xfrd_tcp_pipe_stop(tp); return; } @@ -955,6 +949,7 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) xfrd_make_request(zone); break; } + tcp_pipe_reset_timeout(tp); } void @@ -962,7 +957,7 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { int conn = zone->tcp_conn; struct xfrd_tcp_pipeline* tp = set->tcp_state[conn]; - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** zone %s mapping to tcp conn of %s released", + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s mapping to tcp conn %s is released", zone->apex_str, zone->master->ip_address_spec)); assert(zone->tcp_conn != -1); assert(zone->tcp_waiting == 0); @@ -974,7 +969,7 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) /* remove it from the ID list */ if(tp->id[zone->query_id] != TCP_NULL_SKIP) tcp_pipe_id_remove(tp, zone); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: *** zone %s removed from pipeline send list and ids, %d unused", + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s removed from pipeline mappings, %d unused", zone->apex_str, tp->num_unused)); /* if pipe was full, but no more, then see if waiting element is * for the same master, and can fill the unused ID */ @@ -1010,7 +1005,7 @@ void xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, int conn) { - VERBOSITY(1, (LOG_INFO, "xfrd: *** tcp pipe released")); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipe released")); /* one handler per tcp pipe */ if(tp->handler_added) event_del(&tp->handler); diff --git a/xfrd.c b/xfrd.c index 6ea4a559c..65d6d955a 100644 --- a/xfrd.c +++ b/xfrd.c @@ -197,7 +197,7 @@ xfrd_init(int socket, struct nsd* nsd, int shortsoa, int reload_active, #endif xfrd->tcp_set = xfrd_tcp_set_create(xfrd->region); - xfrd->tcp_set->tcp_timeout = 30; + xfrd->tcp_set->tcp_timeout = nsd->tcp_timeout; #if !defined(HAVE_ARC4RANDOM) && !defined(HAVE_GETRANDOM) srandom((unsigned long) getpid() * (unsigned long) time(NULL)); #endif From fa00a045083ed6ad64323347affa29bfd02a49a9 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Fri, 6 Nov 2020 16:51:30 +0000 Subject: [PATCH 05/10] Add new parameter for idle timeout --- configlexer.lex | 1 + configparser.y | 3 +++ configure.ac | 10 ++++++++++ nsd-checkconf.c | 2 ++ nsd.c | 1 + nsd.conf.5.in | 4 ++++ nsd.h | 1 + options.c | 1 + options.h | 1 + xfrd-tcp.c | 4 ++-- xfrd-tcp.h | 2 ++ xfrd.c | 1 + 12 files changed, 29 insertions(+), 2 deletions(-) diff --git a/configlexer.lex b/configlexer.lex index 1b892fe3b..63e3b6503 100644 --- a/configlexer.lex +++ b/configlexer.lex @@ -225,6 +225,7 @@ tcp-count{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_COUNT;} tcp-reject-overflow{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_REJECT_OVERFLOW;} tcp-query-count{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_QUERY_COUNT;} tcp-timeout{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_TIMEOUT;} +tcp-idle-timeout{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_IDLE_TIMEOUT;} tcp-mss{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_TCP_MSS;} outgoing-tcp-mss{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_OUTGOING_TCP_MSS;} ipv4-edns-size{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_IPV4_EDNS_SIZE;} diff --git a/configparser.y b/configparser.y index f72d4a3f6..cdf34d88d 100644 --- a/configparser.y +++ b/configparser.y @@ -91,6 +91,7 @@ static int parse_range(const char *str, long long *low, long long *high); %token VAR_TCP_REJECT_OVERFLOW %token VAR_TCP_QUERY_COUNT %token VAR_TCP_TIMEOUT +%token VAR_TCP_IDLE_TIMEOUT %token VAR_TCP_MSS %token VAR_OUTGOING_TCP_MSS %token VAR_IPV4_EDNS_SIZE @@ -310,6 +311,8 @@ server_option: { cfg_parser->opt->tcp_query_count = (int)$2; } | VAR_TCP_TIMEOUT number { cfg_parser->opt->tcp_timeout = (int)$2; } + | VAR_TCP_IDLE_TIMEOUT number + { cfg_parser->opt->tcp_idle_timeout = (int)$2; } | VAR_TCP_MSS number { cfg_parser->opt->tcp_mss = (int)$2; } | VAR_OUTGOING_TCP_MSS number diff --git a/configure.ac b/configure.ac index 6c6bb2249..953794bc8 100644 --- a/configure.ac +++ b/configure.ac @@ -899,6 +899,16 @@ AC_ARG_WITH([tcp_timeout], [tcp_timeout=$withval]) AC_DEFINE_UNQUOTED([TCP_TIMEOUT], $tcp_timeout, [Define to the default tcp timeout.]) +dnl +dnl Determine the default tcp idle timeout (used when closing outgoing XFR TCP connections) +dnl +tcp_idle_timeout=10 +AC_ARG_WITH([tcp_idle_timeout], + AC_HELP_STRING([--with-tcp-idle-timeout=number], [Limit the default tcp idle timeout, used when closing outgoing XFR TCP connections]), + [tcp_idle_timeout=$withval]) +AC_DEFINE_UNQUOTED([TCP_IDLE_TIMEOUT], $tcp_idle_timeout, [Define to the default tcp idle timeout.]) + + dnl dnl Features dnl diff --git a/nsd-checkconf.c b/nsd-checkconf.c index b392c7111..99a0a843e 100644 --- a/nsd-checkconf.c +++ b/nsd-checkconf.c @@ -401,6 +401,7 @@ config_print_zone(nsd_options_type* opt, const char* k, int s, const char *o, SERV_GET_INT(tcp_count, o); SERV_GET_INT(tcp_query_count, o); SERV_GET_INT(tcp_timeout, o); + SERV_GET_INT(tcp_idle_timeout, o); SERV_GET_INT(tcp_mss, o); SERV_GET_INT(outgoing_tcp_mss, o); SERV_GET_INT(ipv4_edns_size, o); @@ -547,6 +548,7 @@ config_test_print_server(nsd_options_type* opt) printf("\ttcp-count: %d\n", opt->tcp_count); printf("\ttcp-query-count: %d\n", opt->tcp_query_count); printf("\ttcp-timeout: %d\n", opt->tcp_timeout); + printf("\ttcp-idle-timeout: %d\n", opt->tcp_idle_timeout); printf("\ttcp-mss: %d\n", opt->tcp_mss); printf("\toutgoing-tcp-mss: %d\n", opt->outgoing_tcp_mss); printf("\tipv4-edns-size: %d\n", (int) opt->ipv4_edns_size); diff --git a/nsd.c b/nsd.c index be43cae57..833c8b13a 100644 --- a/nsd.c +++ b/nsd.c @@ -1069,6 +1069,7 @@ main(int argc, char *argv[]) nsd.maximum_tcp_count = nsd.options->tcp_count; } nsd.tcp_timeout = nsd.options->tcp_timeout; + nsd.tcp_idle_timeout = nsd.options->tcp_idle_timeout; nsd.tcp_query_count = nsd.options->tcp_query_count; nsd.tcp_mss = nsd.options->tcp_mss; nsd.outgoing_tcp_mss = nsd.options->outgoing_tcp_mss; diff --git a/nsd.conf.5.in b/nsd.conf.5.in index 851c6de83..0805f2b97 100644 --- a/nsd.conf.5.in +++ b/nsd.conf.5.in @@ -280,6 +280,10 @@ Default is 0, meaning there is no maximum. Overrides the default TCP timeout. This also affects zone transfers over TCP. The default is 120 seconds. .TP +.B tcp\-idle\-timeout:\fR +Overrides the default TCP idle timeout. This is used when closing outgoing +TCP connections used for zone transfers. The default is 10 seconds. +.TP .B tcp-mss:\fR Maximum segment size (MSS) of TCP socket on which the server responds to queries. Value lower than common MSS on Ethernet diff --git a/nsd.h b/nsd.h index 86b8ad66e..cf9fd7805 100644 --- a/nsd.h +++ b/nsd.h @@ -267,6 +267,7 @@ struct nsd int current_tcp_count; int tcp_query_count; int tcp_timeout; + int tcp_idle_timeout; int tcp_mss; int outgoing_tcp_mss; size_t ipv4_edns_size; diff --git a/options.c b/options.c index 7e7d823c1..62cfb9320 100644 --- a/options.c +++ b/options.c @@ -79,6 +79,7 @@ nsd_options_create(region_type* region) opt->tcp_reject_overflow = 0; opt->tcp_query_count = 0; opt->tcp_timeout = TCP_TIMEOUT; + opt->tcp_idle_timeout = TCP_IDLE_TIMEOUT; opt->tcp_mss = 0; opt->outgoing_tcp_mss = 0; opt->ipv4_edns_size = EDNS_MAX_MESSAGE_LEN; diff --git a/options.h b/options.h index 943620f3b..50fee0dd2 100644 --- a/options.h +++ b/options.h @@ -87,6 +87,7 @@ struct nsd_options { int confine_to_zone; int tcp_query_count; int tcp_timeout; + int tcp_idle_timeout; int tcp_mss; int outgoing_tcp_mss; size_t ipv4_edns_size; diff --git a/xfrd-tcp.c b/xfrd-tcp.c index 55250008a..0bedb6567 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -330,10 +330,10 @@ tcp_pipe_reset_timeout(struct xfrd_tcp_pipeline* tp) { int fd = tp->handler.ev_fd; struct timeval tv; - /* pipe is effectively unused - for now set a default 10s idle timeout until EDNS0 + /* pipe is effectively unused - for now set a fixed idle timeout until EDNS0 Keepalive is implemented */ if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) - tv.tv_sec = 10; + tv.tv_sec = xfrd->tcp_set->tcp_idle_timeout; else tv.tv_sec = xfrd->tcp_set->tcp_timeout; tv.tv_usec = 0; diff --git a/xfrd-tcp.h b/xfrd-tcp.h index fd6918768..20d8960e1 100644 --- a/xfrd-tcp.h +++ b/xfrd-tcp.h @@ -33,6 +33,8 @@ struct xfrd_tcp_set { int tcp_count; /* TCP timeout. */ int tcp_timeout; + /* TCP idle timeout. */ + int tcp_idle_timeout; /* rbtree with pipelines sorted by master */ rbtree_type* pipetree; /* double linked list of zones waiting for a TCP connection */ diff --git a/xfrd.c b/xfrd.c index 65d6d955a..92313dba4 100644 --- a/xfrd.c +++ b/xfrd.c @@ -198,6 +198,7 @@ xfrd_init(int socket, struct nsd* nsd, int shortsoa, int reload_active, xfrd->tcp_set = xfrd_tcp_set_create(xfrd->region); xfrd->tcp_set->tcp_timeout = nsd->tcp_timeout; + xfrd->tcp_set->tcp_idle_timeout = nsd->tcp_idle_timeout; #if !defined(HAVE_ARC4RANDOM) && !defined(HAVE_GETRANDOM) srandom((unsigned long) getpid() * (unsigned long) time(NULL)); #endif From a403e8b62c48f44e03af9e0aaacb77e7b7165746 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Fri, 6 Nov 2020 17:45:42 +0000 Subject: [PATCH 06/10] Add option to enable connection reuse by default, but set to no for now. Option not yet used. --- configlexer.lex | 1 + configparser.y | 3 +++ nsd-checkconf.c | 2 ++ nsd.c | 1 + nsd.conf.5.in | 5 +++++ nsd.conf.sample.in | 8 ++++++++ nsd.h | 2 ++ options.c | 1 + options.h | 1 + xfrd-tcp.c | 3 +++ xfrd-tcp.h | 2 ++ xfrd.c | 1 + 12 files changed, 30 insertions(+) diff --git a/configlexer.lex b/configlexer.lex index 63e3b6503..a1d34ed3d 100644 --- a/configlexer.lex +++ b/configlexer.lex @@ -242,6 +242,7 @@ difffile{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_DIFFFILE;} xfrdfile{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_XFRDFILE;} xfrdir{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_XFRDIR;} xfrd-reload-timeout{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_XFRD_RELOAD_TIMEOUT;} +xfrd-conn-reuse{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_XFRD_CONN_REUSE;} verbosity{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_VERBOSITY;} zone{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_ZONE;} zonefile{COLON} { LEXOUT(("v(%s) ", yytext)); return VAR_ZONEFILE;} diff --git a/configparser.y b/configparser.y index cdf34d88d..8a46324a8 100644 --- a/configparser.y +++ b/configparser.y @@ -98,6 +98,7 @@ static int parse_range(const char *str, long long *low, long long *high); %token VAR_IPV6_EDNS_SIZE %token VAR_STATISTICS %token VAR_XFRD_RELOAD_TIMEOUT +%token VAR_XFRD_CONN_REUSE %token VAR_LOG_TIME_ASCII %token VAR_ROUND_ROBIN %token VAR_MINIMAL_RESPONSES @@ -350,6 +351,8 @@ server_option: { cfg_parser->opt->xfrdir = region_strdup(cfg_parser->opt->region, $2); } | VAR_XFRD_RELOAD_TIMEOUT number { cfg_parser->opt->xfrd_reload_timeout = (int)$2; } + | VAR_XFRD_CONN_REUSE boolean + { cfg_parser->opt->xfrd_conn_reuse = $2; } | VAR_VERBOSITY number { cfg_parser->opt->verbosity = (int)$2; } | VAR_RRL_SIZE number diff --git a/nsd-checkconf.c b/nsd-checkconf.c index 99a0a843e..d8795508e 100644 --- a/nsd-checkconf.c +++ b/nsd-checkconf.c @@ -408,6 +408,7 @@ config_print_zone(nsd_options_type* opt, const char* k, int s, const char *o, SERV_GET_INT(ipv6_edns_size, o); SERV_GET_INT(statistics, o); SERV_GET_INT(xfrd_reload_timeout, o); + SERV_GET_BIN(xfrd_conn_reuse, o); SERV_GET_INT(verbosity, o); SERV_GET_INT(send_buffer_size, o); SERV_GET_INT(receive_buffer_size, o); @@ -563,6 +564,7 @@ config_test_print_server(nsd_options_type* opt) print_string_var("zonelistfile:", opt->zonelistfile); print_string_var("xfrdir:", opt->xfrdir); printf("\txfrd-reload-timeout: %d\n", opt->xfrd_reload_timeout); + printf("\txfrd-conn-reuse: %s\n", opt->xfrd_conn_reuse?"yes":"no"); printf("\tlog-time-ascii: %s\n", opt->log_time_ascii?"yes":"no"); printf("\tround-robin: %s\n", opt->round_robin?"yes":"no"); printf("\tminimal-responses: %s\n", opt->minimal_responses?"yes":"no"); diff --git a/nsd.c b/nsd.c index 833c8b13a..183458e99 100644 --- a/nsd.c +++ b/nsd.c @@ -1070,6 +1070,7 @@ main(int argc, char *argv[]) } nsd.tcp_timeout = nsd.options->tcp_timeout; nsd.tcp_idle_timeout = nsd.options->tcp_idle_timeout; + nsd.xfrd_conn_reuse = nsd.options->xfrd_conn_reuse; nsd.tcp_query_count = nsd.options->tcp_query_count; nsd.tcp_mss = nsd.options->tcp_mss; nsd.outgoing_tcp_mss = nsd.options->outgoing_tcp_mss; diff --git a/nsd.conf.5.in b/nsd.conf.5.in index 0805f2b97..143828eeb 100644 --- a/nsd.conf.5.in +++ b/nsd.conf.5.in @@ -374,6 +374,11 @@ transfer, then it will wait for the number of seconds before it will trigger a new reload. Setting this value throttles the reloads to once per the number of seconds. The default is 1 second. .TP +.B xfrd\-conn\-reuse:\fR +When making outgoing XFR requests to the same master an open TCP connection +will be used in preference to opening a new connection for each request. +Default is no. +.TP .B verbosity:\fR This value specifies the verbosity level for (non\-debug) logging. Default is 0. 1 gives more information about incoming notifies and diff --git a/nsd.conf.sample.in b/nsd.conf.sample.in index 8fb0ad88c..17b014bc1 100644 --- a/nsd.conf.sample.in +++ b/nsd.conf.sample.in @@ -166,6 +166,10 @@ server: # Override the default (120 seconds) TCP timeout. # tcp-timeout: 120 + # Override the default (10 seconds) TCP idle timeout, used when + # closing outgoing XFR TCP connections. + # tcp-idle-timeout: 120 + # Maximum segment size (MSS) of TCP socket on which the server # responds to queries. Default is 0, system default MSS. # tcp-mss: 0 @@ -187,6 +191,10 @@ server: # Number of seconds between reloads triggered by xfrd. # xfrd-reload-timeout: 1 + # Prefer to reuse open connections to a master instead of opening + # a new connection for each transfer request to that master + # xfrd-conn-reuse: no + # log timestamp in ascii (y-m-d h:m:s.msec), yes is default. # log-time-ascii: yes diff --git a/nsd.h b/nsd.h index cf9fd7805..ef07e783d 100644 --- a/nsd.h +++ b/nsd.h @@ -268,6 +268,8 @@ struct nsd int tcp_query_count; int tcp_timeout; int tcp_idle_timeout; + int xfrd_conn_reuse; + int tcp_mss; int outgoing_tcp_mss; size_t ipv4_edns_size; diff --git a/options.c b/options.c index 62cfb9320..c8dca8153 100644 --- a/options.c +++ b/options.c @@ -123,6 +123,7 @@ nsd_options_create(region_type* region) opt->zonefiles_write = ZONEFILES_WRITE_INTERVAL; else opt->zonefiles_write = 0; opt->xfrd_reload_timeout = 1; + opt->xfrd_conn_reuse = 0; opt->tls_service_key = NULL; opt->tls_service_ocsp = NULL; opt->tls_service_pem = NULL; diff --git a/options.h b/options.h index 50fee0dd2..b983590eb 100644 --- a/options.h +++ b/options.h @@ -103,6 +103,7 @@ struct nsd_options { const char* zonelistfile; const char* nsid; int xfrd_reload_timeout; + int xfrd_conn_reuse; int zonefiles_check; int zonefiles_write; int log_time_ascii; diff --git a/xfrd-tcp.c b/xfrd-tcp.c index 0bedb6567..f0fc56def 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -423,6 +423,9 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) assert(zone->tcp_conn == -1); assert(zone->tcp_waiting == 0); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: connection reuse is set to %s", + set->xfrd_conn_reuse?"yes":"no")); + /* check for a pipeline to the same master with unused ID */ if((tp = pipeline_find(set, zone))!= NULL) { int i; diff --git a/xfrd-tcp.h b/xfrd-tcp.h index 20d8960e1..870e7e306 100644 --- a/xfrd-tcp.h +++ b/xfrd-tcp.h @@ -35,6 +35,8 @@ struct xfrd_tcp_set { int tcp_timeout; /* TCP idle timeout. */ int tcp_idle_timeout; + /* TCP connection reuse. */ + int xfrd_conn_reuse; /* rbtree with pipelines sorted by master */ rbtree_type* pipetree; /* double linked list of zones waiting for a TCP connection */ diff --git a/xfrd.c b/xfrd.c index 92313dba4..a1943b30a 100644 --- a/xfrd.c +++ b/xfrd.c @@ -199,6 +199,7 @@ xfrd_init(int socket, struct nsd* nsd, int shortsoa, int reload_active, xfrd->tcp_set = xfrd_tcp_set_create(xfrd->region); xfrd->tcp_set->tcp_timeout = nsd->tcp_timeout; xfrd->tcp_set->tcp_idle_timeout = nsd->tcp_idle_timeout; + xfrd->tcp_set->xfrd_conn_reuse = nsd->xfrd_conn_reuse; #if !defined(HAVE_ARC4RANDOM) && !defined(HAVE_GETRANDOM) srandom((unsigned long) getpid() * (unsigned long) time(NULL)); #endif From e24a7c25342e56051ebdaa981ce0a44f772fed00 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Mon, 9 Nov 2020 18:55:47 +0000 Subject: [PATCH 07/10] Use the new option to control pipelining, code clean up and polish documentation --- configure.ac | 2 +- nsd.conf.5.in | 6 +- nsd.conf.sample.in | 6 +- xfrd-tcp.c | 196 ++++++++++++++++++++++++++++----------------- 4 files changed, 130 insertions(+), 80 deletions(-) diff --git a/configure.ac b/configure.ac index 953794bc8..442c8a341 100644 --- a/configure.ac +++ b/configure.ac @@ -904,7 +904,7 @@ dnl Determine the default tcp idle timeout (used when closing outgoing XFR TCP c dnl tcp_idle_timeout=10 AC_ARG_WITH([tcp_idle_timeout], - AC_HELP_STRING([--with-tcp-idle-timeout=number], [Limit the default tcp idle timeout, used when closing outgoing XFR TCP connections]), + AC_HELP_STRING([--with-tcp-idle-timeout=number], [Limit the default tcp idle timeout, used when closing outgoing XFR TCP connections when xfrd-conn-resuse option is enabled]), [tcp_idle_timeout=$withval]) AC_DEFINE_UNQUOTED([TCP_IDLE_TIMEOUT], $tcp_idle_timeout, [Define to the default tcp idle timeout.]) diff --git a/nsd.conf.5.in b/nsd.conf.5.in index 143828eeb..41712961f 100644 --- a/nsd.conf.5.in +++ b/nsd.conf.5.in @@ -282,7 +282,8 @@ The default is 120 seconds. .TP .B tcp\-idle\-timeout:\fR Overrides the default TCP idle timeout. This is used when closing outgoing -TCP connections used for zone transfers. The default is 10 seconds. +TCP connections used for zone transfers when the xfrd\-conn\-reuse option is enabled. +The default is 10 seconds. .TP .B tcp-mss:\fR Maximum segment size (MSS) of TCP socket on which the server responds @@ -377,7 +378,8 @@ once per the number of seconds. The default is 1 second. .B xfrd\-conn\-reuse:\fR When making outgoing XFR requests to the same master an open TCP connection will be used in preference to opening a new connection for each request. -Default is no. +After all transfers complete, connections will be left open for +tcp\-idle\-timeout seconds to increase the chance of reuse. Default is no. .TP .B verbosity:\fR This value specifies the verbosity level for (non\-debug) logging. diff --git a/nsd.conf.sample.in b/nsd.conf.sample.in index 17b014bc1..c315f8402 100644 --- a/nsd.conf.sample.in +++ b/nsd.conf.sample.in @@ -167,8 +167,8 @@ server: # tcp-timeout: 120 # Override the default (10 seconds) TCP idle timeout, used when - # closing outgoing XFR TCP connections. - # tcp-idle-timeout: 120 + # closing outgoing XFR TCP connections when xfrd-conn-reuse is enabled. + # tcp-idle-timeout: 10 # Maximum segment size (MSS) of TCP socket on which the server # responds to queries. Default is 0, system default MSS. @@ -192,7 +192,7 @@ server: # xfrd-reload-timeout: 1 # Prefer to reuse open connections to a master instead of opening - # a new connection for each transfer request to that master + # a new connection for each transfer request to that master. # xfrd-conn-reuse: no # log timestamp in ascii (y-m-d h:m:s.msec), yes is default. diff --git a/xfrd-tcp.c b/xfrd-tcp.c index f0fc56def..bdfcecb86 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -296,16 +296,30 @@ tcp_pipe_id_remove(struct xfrd_tcp_pipeline* tp, xfrd_zone_type* zone) (void)rbtree_insert(xfrd->tcp_set->pipetree, &tp->node); } +/* determine if pipeline is actually in use*/ +static int +tcp_pipe_in_use(struct xfrd_tcp_pipeline* tp) { + /* Check there are no active transfers (outstanding query ids) but we + need to ignore any outstanding responses for skipped transactions + - in other check we do not have at least one 'nonskip' ID */ + /* NOTE: There is a minor question if a tcp connection still with skipped + transactions is truely 'idle', but for our purposes we are going to treat + it as if it is...*/ + return (ID_PIPE_NUM - tp->num_unused > tp->num_skip)?1:0; +} + /* stop the tcp pipe (and all its zones need to retry) */ static void xfrd_tcp_pipe_stop(struct xfrd_tcp_pipeline* tp) { int i, conn = -1; - /* With connections now left open when (effectively) idle, it is possible to arrive here - because the far end shuts the idle connection (causing a read event with no data). - So just warn if we get here while the pipe is still in use */ - if (!(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused)) { - log_msg(LOG_WARNING, "xfrd: an in use TCP connection was closed by the far end, retrying all zones"); + /* With connections now left open when (effectively) idle, it is possible to + arrive here because the far end shuts the idle connection (causing a read + event with no data). So just warn if we get here while the pipe is still + in use */ + if (tcp_pipe_in_use(tp)) { + log_msg(LOG_WARNING, "xfrd: an in use TCP connection was closed by the" + "far end, retrying all zones"); /* need to retry for all the zones connected to it */ /* these could use different lists and go to a different nextmaster*/ for(i=0; ihandler.ev_fd; struct timeval tv; - /* pipe is effectively unused - for now set a fixed idle timeout until EDNS0 + /* pipe is unused - for now set a fixed idle timeout until EDNS0 Keepalive is implemented */ - if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) + if(!tcp_pipe_in_use(tp)) tv.tv_sec = xfrd->tcp_set->tcp_idle_timeout; else tv.tv_sec = xfrd->tcp_set->tcp_timeout; @@ -369,10 +383,11 @@ xfrd_handle_tcp_pipe(int ATTR_UNUSED(fd), short event, void* arg) } if((event & EV_TIMEOUT) && tp->handler_added) { /* tcp connection timed out */ - /* pipe is unused (timeout triggered while idle), just release indicating - we don't have access to the conn for the pipe from here */ - if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) { - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: idle pipeline had tcp timeout event")); + /* pipe is unused (timeout triggered while idle), just release it but + note that we don't have access to the conn for the pipe from here */ + if(!tcp_pipe_in_use(tp)){ + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: idle pipeline had tcp timeout" + "event")); xfrd_tcp_pipe_release(xfrd->tcp_set, tp, -1); } else @@ -416,16 +431,11 @@ pipeline_setup_new_zone(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, } } -void -xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) -{ - struct xfrd_tcp_pipeline* tp; - assert(zone->tcp_conn == -1); - assert(zone->tcp_waiting == 0); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: connection reuse is set to %s", - set->xfrd_conn_reuse?"yes":"no")); +static struct xfrd_tcp_pipeline* +xfrd_tcp_find_pipeline(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { + struct xfrd_tcp_pipeline* tp; /* check for a pipeline to the same master with unused ID */ if((tp = pipeline_find(set, zone))!= NULL) { int i; @@ -438,61 +448,92 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) xfrd_deactivate_zone(zone); xfrd_unset_timer(zone); pipeline_setup_new_zone(set, tp, zone); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s is re-using pipeline %d", - zone->apex_str, i)); - return; + DEBUG(DEBUG_XFRD,1, (LOG_INFO, + "xfrd: zone %s is re-using tcp conn pipeline %d", + zone->apex_str, zone->tcp_conn)); + return tp; } + return NULL; +} - if(set->tcp_count < XFRD_MAX_TCP) { - int i; - assert(!set->tcp_waiting_first); - set->tcp_count ++; - /* find a free tcp_buffer */ - for(i=0; itcp_state[i]->tcp_r->fd == -1) { - zone->tcp_conn = i; - break; - } - } - /** What if there is no free tcp_buffer? return; */ - if (zone->tcp_conn < 0) { - return; - } +static struct xfrd_tcp_pipeline* +xfrd_tcp_new_pipeline(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { - tp = set->tcp_state[zone->tcp_conn]; - zone->tcp_waiting = 0; + struct xfrd_tcp_pipeline* tp; + /* Open a new pipeline if one is available*/ + if(set->tcp_count < XFRD_MAX_TCP) { + int i; + assert(!set->tcp_waiting_first); + set->tcp_count ++; + /* find a free tcp_buffer */ + for(i=0; itcp_state[i]->tcp_r->fd == -1) { + zone->tcp_conn = i; + break; + } + } + /** What if there is no free tcp_buffer? return; */ + if (zone->tcp_conn < 0) { + zone->tcp_conn = -1; + set->tcp_count --; + xfrd_set_refresh_now(zone); + return NULL; + } - /* stop udp use (if any) */ - if(zone->zone_handler.ev_fd != -1) - xfrd_udp_release(zone); + tp = set->tcp_state[zone->tcp_conn]; + zone->tcp_waiting = 0; - if(!xfrd_tcp_open(set, tp, zone)) { - zone->tcp_conn = -1; - set->tcp_count --; - xfrd_set_refresh_now(zone); - return; - } - /* ip and ip_len set by tcp_open */ - tp->node.key = tp; - tp->num_unused = ID_PIPE_NUM; - tp->num_skip = 0; - tp->tcp_send_first = NULL; - tp->tcp_send_last = NULL; - memset(tp->id, 0, sizeof(tp->id)); - for(i=0; iunused[i] = i; - } + /* stop udp use (if any) */ + if(zone->zone_handler.ev_fd != -1) + xfrd_udp_release(zone); - /* insert into tree */ - (void)rbtree_insert(set->pipetree, &tp->node); - xfrd_deactivate_zone(zone); - xfrd_unset_timer(zone); - pipeline_setup_new_zone(set, tp, zone); + if(!xfrd_tcp_open(set, tp, zone)) { + zone->tcp_conn = -1; + set->tcp_count --; + xfrd_set_refresh_now(zone); + return NULL; + } + /* ip and ip_len set by tcp_open */ + tp->node.key = tp; + tp->num_unused = ID_PIPE_NUM; + tp->num_skip = 0; + tp->tcp_send_first = NULL; + tp->tcp_send_last = NULL; + memset(tp->id, 0, sizeof(tp->id)); + for(i=0; iunused[i] = i; + } + + /* insert into tree */ + (void)rbtree_insert(set->pipetree, &tp->node); + xfrd_deactivate_zone(zone); + xfrd_unset_timer(zone); + pipeline_setup_new_zone(set, tp, zone); DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s opened a new tcp conn %d", - zone->apex_str, i)); - return; - } + zone->apex_str, zone->tcp_conn)); + return tp; + } + return NULL; +} + +void +xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) +{ + struct xfrd_tcp_pipeline* tp; + assert(zone->tcp_conn == -1); + assert(zone->tcp_waiting == 0); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: connection reuse is set to %s", + set->xfrd_conn_reuse?"yes":"no")); + + /* Select an existing or new pipeline first depending on the configuration*/ + if ((tp = set->xfrd_conn_reuse?xfrd_tcp_find_pipeline(set, zone): + xfrd_tcp_new_pipeline(set, zone))!=NULL) + return; + if ((tp = set->xfrd_conn_reuse?xfrd_tcp_new_pipeline(set, zone): + xfrd_tcp_find_pipeline(set, zone))!=NULL) + return; + /* wait, at end of line */ DEBUG(DEBUG_XFRD,2, (LOG_INFO, "xfrd: max number of tcp " "connections (%d) reached.", XFRD_MAX_TCP)); @@ -952,7 +993,6 @@ xfrd_tcp_read(struct xfrd_tcp_pipeline* tp) xfrd_make_request(zone); break; } - tcp_pipe_reset_timeout(tp); } void @@ -960,8 +1000,9 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { int conn = zone->tcp_conn; struct xfrd_tcp_pipeline* tp = set->tcp_state[conn]; - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s mapping to tcp conn %s is released", - zone->apex_str, zone->master->ip_address_spec)); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, + "xfrd: zone %s mapping to tcp conn %d (%s) is released", + zone->apex_str, zone->tcp_conn, zone->master->ip_address_spec)); assert(zone->tcp_conn != -1); assert(zone->tcp_waiting == 0); zone->tcp_conn = -1; @@ -972,7 +1013,8 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) /* remove it from the ID list */ if(tp->id[zone->query_id] != TCP_NULL_SKIP) tcp_pipe_id_remove(tp, zone); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s removed from pipeline mappings, %d unused", + DEBUG(DEBUG_XFRD,1, (LOG_INFO, + "xfrd: zone %s query id removed from pipeline, %d unused ids", zone->apex_str, tp->num_unused)); /* if pipe was full, but no more, then see if waiting element is * for the same master, and can fill the unused ID */ @@ -999,16 +1041,19 @@ xfrd_tcp_release(struct xfrd_tcp_set* set, xfrd_zone_type* zone) /* waiting zone did not go to same server */ } - /* if all unused, or only skipped leftover, close the pipeline */ - // if(tp->num_unused >= ID_PIPE_NUM || tp->num_skip >= ID_PIPE_NUM - tp->num_unused) - // xfrd_tcp_pipe_release(set, tp, conn); + if (!set->xfrd_conn_reuse) { + /* if all unused, or only skipped leftover, close the pipeline */ + if(!tcp_pipe_in_use(tp)) + xfrd_tcp_pipe_release(set, tp, conn); + } else { + tcp_pipe_reset_timeout(tp); + } } void xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, int conn) { - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipe released")); /* one handler per tcp pipe */ if(tp->handler_added) event_del(&tp->handler); @@ -1030,6 +1075,7 @@ xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, conn = i; } } + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipeline %d released", conn)); assert(conn != -1); /* a waiting zone can use the free tcp slot (to another server) */ @@ -1077,6 +1123,8 @@ xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, /* no task to start, cleanup */ assert(!set->tcp_waiting_first); set->tcp_count --; + DEBUG(DEBUG_XFRD,1, (LOG_INFO, + "xfrd: %d tcp pipelines in use", set->tcp_count)); assert(set->tcp_count >= 0); } From 00566012e1620753e2226f25d50d39974fc6be17 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Mon, 30 Nov 2020 14:29:59 +0000 Subject: [PATCH 08/10] Fix warning from clang-analysis unit test about unused pointer, update checkconf unit test to support new options --- tpkg/checkconf.tdir/checkconf.check | 10 ++++++++++ tpkg/checkconf.tdir/checkconf.check2 | 10 ++++++++++ xfrd-tcp.c | 12 ++++++------ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/tpkg/checkconf.tdir/checkconf.check b/tpkg/checkconf.tdir/checkconf.check index 6565eaf79..cbcf32aa7 100644 --- a/tpkg/checkconf.tdir/checkconf.check +++ b/tpkg/checkconf.tdir/checkconf.check @@ -24,6 +24,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -38,6 +39,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -135,6 +137,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -149,6 +152,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -199,6 +203,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -213,6 +218,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: no round-robin: no minimal-responses: no @@ -272,6 +278,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -286,6 +293,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -389,6 +397,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -403,6 +412,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no diff --git a/tpkg/checkconf.tdir/checkconf.check2 b/tpkg/checkconf.tdir/checkconf.check2 index 6565eaf79..cbcf32aa7 100644 --- a/tpkg/checkconf.tdir/checkconf.check2 +++ b/tpkg/checkconf.tdir/checkconf.check2 @@ -24,6 +24,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -38,6 +39,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -135,6 +137,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -149,6 +152,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -199,6 +203,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -213,6 +218,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: no round-robin: no minimal-responses: no @@ -272,6 +278,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -286,6 +293,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no @@ -389,6 +397,7 @@ server: tcp-count: 100 tcp-query-count: 0 tcp-timeout: 120 + tcp-idle-timeout: 10 tcp-mss: 0 outgoing-tcp-mss: 0 ipv4-edns-size: 4096 @@ -403,6 +412,7 @@ server: zonelistfile: "/var/db/nsd/zone.list" xfrdir: "/tmp" xfrd-reload-timeout: 1 + xfrd-conn-reuse: no log-time-ascii: yes round-robin: no minimal-responses: no diff --git a/xfrd-tcp.c b/xfrd-tcp.c index bdfcecb86..426dd852c 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -527,12 +527,12 @@ xfrd_tcp_obtain(struct xfrd_tcp_set* set, xfrd_zone_type* zone) set->xfrd_conn_reuse?"yes":"no")); /* Select an existing or new pipeline first depending on the configuration*/ - if ((tp = set->xfrd_conn_reuse?xfrd_tcp_find_pipeline(set, zone): - xfrd_tcp_new_pipeline(set, zone))!=NULL) - return; - if ((tp = set->xfrd_conn_reuse?xfrd_tcp_new_pipeline(set, zone): - xfrd_tcp_find_pipeline(set, zone))!=NULL) - return; + tp = set->xfrd_conn_reuse?xfrd_tcp_find_pipeline(set, zone): + xfrd_tcp_new_pipeline(set, zone); + if (tp != NULL) return; + tp = set->xfrd_conn_reuse?xfrd_tcp_new_pipeline(set, zone): + xfrd_tcp_find_pipeline(set, zone); + if (tp != NULL) return; /* wait, at end of line */ DEBUG(DEBUG_XFRD,2, (LOG_INFO, "xfrd: max number of tcp " From 0ea7b599bd0be2b6d485ebaf765c617b7a361836 Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Tue, 1 Dec 2020 16:49:09 +0000 Subject: [PATCH 09/10] make sure open tcp connections are closed when shutting down --- xfrd-tcp.c | 17 ++++++++++++----- xfrd.c | 7 +++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/xfrd-tcp.c b/xfrd-tcp.c index 426dd852c..262f8aa04 100644 --- a/xfrd-tcp.c +++ b/xfrd-tcp.c @@ -509,8 +509,8 @@ xfrd_tcp_new_pipeline(struct xfrd_tcp_set* set, xfrd_zone_type* zone) { xfrd_deactivate_zone(zone); xfrd_unset_timer(zone); pipeline_setup_new_zone(set, tp, zone); - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s opened a new tcp conn %d", - zone->apex_str, zone->tcp_conn)); + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: zone %s opened a new tcp conn num %d with fd %d", + zone->apex_str, zone->tcp_conn, set->tcp_state[zone->tcp_conn]->tcp_r->fd)); return tp; } return NULL; @@ -1054,6 +1054,9 @@ void xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, int conn) { + + DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipeline %d being released - has fd %d", + conn, tp->tcp_r->fd)); /* one handler per tcp pipe */ if(tp->handler_added) event_del(&tp->handler); @@ -1068,15 +1071,19 @@ xfrd_tcp_pipe_release(struct xfrd_tcp_set* set, struct xfrd_tcp_pipeline* tp, /* remove from pipetree */ (void)rbtree_delete(xfrd->tcp_set->pipetree, &tp->node); - /* find the connection for this pipeline so it can be re-used*/ + /* this signals the pipe should not be reused e.g., if we are shutting down */ + if (conn == -2) + return; + + /* the calling function didn't know the conn (e.g. timeout) + find the connection for this pipeline so it can be re-used*/ if (conn == -1) { for(int i=0; itcp_state[i] == tp) conn = i; } } - DEBUG(DEBUG_XFRD,1, (LOG_INFO, "xfrd: tcp pipeline %d released", conn)); - assert(conn != -1); + assert(conn >= 0); /* a waiting zone can use the free tcp slot (to another server) */ /* if that zone fails to set-up or connect, we try to start the next diff --git a/xfrd.c b/xfrd.c index a1943b30a..56969ef68 100644 --- a/xfrd.c +++ b/xfrd.c @@ -371,6 +371,13 @@ xfrd_shutdown() } } close_notify_fds(xfrd->notify_zones); + /* and any open TCP connections so the far end knows we are gone */ + struct xfrd_tcp_pipeline* tp; + for(int i=0; itcp_set->tcp_state[i]; + if (tp->tcp_r->fd != -1) + xfrd_tcp_pipe_release(xfrd->tcp_set, tp, -2); + } /* wait for server parent (if necessary) */ if(xfrd->reload_pid != -1) { From 2e1d6f80c82f58ded6ffe454d40845afb2fca17e Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Wed, 2 Dec 2020 13:22:27 +0000 Subject: [PATCH 10/10] tidy tabs vs spaces --- nsd.conf.sample.in | 4 ++-- nsd.h | 4 ++-- options.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nsd.conf.sample.in b/nsd.conf.sample.in index c315f8402..62b1872e8 100644 --- a/nsd.conf.sample.in +++ b/nsd.conf.sample.in @@ -167,7 +167,7 @@ server: # tcp-timeout: 120 # Override the default (10 seconds) TCP idle timeout, used when - # closing outgoing XFR TCP connections when xfrd-conn-reuse is enabled. + # closing outgoing XFR TCP connections when xfrd-conn-reuse is enabled. # tcp-idle-timeout: 10 # Maximum segment size (MSS) of TCP socket on which the server @@ -192,7 +192,7 @@ server: # xfrd-reload-timeout: 1 # Prefer to reuse open connections to a master instead of opening - # a new connection for each transfer request to that master. + # a new connection for each transfer request to that master. # xfrd-conn-reuse: no # log timestamp in ascii (y-m-d h:m:s.msec), yes is default. diff --git a/nsd.h b/nsd.h index ef07e783d..1b6b9350d 100644 --- a/nsd.h +++ b/nsd.h @@ -268,8 +268,8 @@ struct nsd int tcp_query_count; int tcp_timeout; int tcp_idle_timeout; - int xfrd_conn_reuse; - + int xfrd_conn_reuse; + int tcp_mss; int outgoing_tcp_mss; size_t ipv4_edns_size; diff --git a/options.h b/options.h index b983590eb..09b11f951 100644 --- a/options.h +++ b/options.h @@ -87,7 +87,7 @@ struct nsd_options { int confine_to_zone; int tcp_query_count; int tcp_timeout; - int tcp_idle_timeout; + int tcp_idle_timeout; int tcp_mss; int outgoing_tcp_mss; size_t ipv4_edns_size;