From f83c8e217e38ff3052f9b966dc12b41593697dd8 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 17 Oct 2017 13:47:29 +0200 Subject: [PATCH 1/9] Decrease assumptions based on network_by_query_id --- src/request-internal.c | 17 ++++++ src/stub.c | 119 +++++++++++++++++------------------------ src/types-internal.h | 4 +- 3 files changed, 68 insertions(+), 72 deletions(-) diff --git a/src/request-internal.c b/src/request-internal.c index 26bd3df2..c9f27db0 100644 --- a/src/request-internal.c +++ b/src/request-internal.c @@ -107,6 +107,12 @@ network_req_cleanup(getdns_network_req *net_req) { assert(net_req); + if (net_req->query_id_registered) { + (void) _getdns_rbtree_delete( + net_req->query_id_registered, net_req->node.key); + net_req->query_id_registered = NULL; + net_req->node.key = NULL; + } if (net_req->response && (net_req->response < net_req->wire_data || net_req->response > net_req->wire_data+ net_req->wire_data_sz)) GETDNS_FREE(net_req->owner->my_mf, net_req->response); @@ -123,6 +129,12 @@ netreq_reset(getdns_network_req *net_req) */ net_req->unbound_id = -1; _getdns_netreq_change_state(net_req, NET_REQ_NOT_SENT); + if (net_req->query_id_registered) { + (void) _getdns_rbtree_delete(net_req->query_id_registered, + (void *)(intptr_t)GLDNS_ID_WIRE(net_req->query)); + net_req->query_id_registered = NULL; + net_req->node.key = NULL; + } net_req->dnssec_status = GETDNS_DNSSEC_INDETERMINATE; net_req->tsig_status = GETDNS_DNSSEC_INDETERMINATE; net_req->response_len = 0; @@ -196,6 +208,11 @@ network_req_init(getdns_network_req *net_req, getdns_dns_req *owner, /* Scheduling, touch only via _getdns_netreq_change_state! */ net_req->state = NET_REQ_NOT_SENT; + /* A registered netreq (on a statefull transport) + * Deregister on reset and cleanup. + */ + net_req->query_id_registered = NULL; + net_req->node.key = NULL; if (max_query_sz == 0) { net_req->query = NULL; diff --git a/src/stub.c b/src/stub.c index 46f0056a..31bfb786 100644 --- a/src/stub.c +++ b/src/stub.c @@ -83,8 +83,7 @@ static void upstream_write_cb(void *userarg); static void upstream_idle_timeout_cb(void *userarg); static void upstream_schedule_netreq(getdns_upstream *upstream, getdns_network_req *netreq); -static void upstream_reschedule_events(getdns_upstream *upstream, - uint64_t idle_timeout); +static void upstream_reschedule_events(getdns_upstream *upstream); static int upstream_working_ok(getdns_upstream *upstream); static int upstream_auth_status_ok(getdns_upstream *upstream, getdns_network_req *netreq); @@ -498,20 +497,20 @@ stub_cleanup(getdns_network_req *netreq) DEBUG_STUB("%s %-35s: MSG: %p\n", STUB_DEBUG_CLEANUP, __FUNC__, (void*)netreq); getdns_dns_req *dnsreq = netreq->owner; - getdns_upstream *upstream; GETDNS_CLEAR_EVENT(dnsreq->loop, &netreq->event); - /* Nothing globally scheduled? Then nothing queued */ - if (!netreq->upstream || !(upstream = netreq->upstream)->event.ev) - return; - - /* Delete from upstream->netreq_by_query_id (if present) */ - (void) _getdns_rbtree_delete(&upstream->netreq_by_query_id, - (void *)(intptr_t)GLDNS_ID_WIRE(netreq->query)); - - remove_from_write_queue(upstream, netreq); - upstream_reschedule_events(upstream, upstream->keepalive_timeout); + if (netreq->query_id_registered) { + (void) _getdns_rbtree_delete( + netreq->query_id_registered, netreq->node.key); + netreq->query_id_registered = NULL; + netreq->node.key = NULL; + } + if (netreq->upstream) { + remove_from_write_queue(netreq->upstream, netreq); + if (netreq->upstream->event.ev) + upstream_reschedule_events(netreq->upstream); + } } static void @@ -528,14 +527,6 @@ upstream_failed(getdns_upstream *upstream, int during_setup) if (during_setup) { /* Reset timeout on setup failure to trigger fallback handling.*/ GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); - /* Need this check because if the setup failed because the interface is - not up we get -1 and then a seg fault. Found when using IPv6 address - but IPv6 interface not enabled.*/ - if (upstream->fd != -1) { - GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, TIMEOUT_FOREVER, - getdns_eventloop_event_init(&upstream->event, upstream, - NULL, upstream_write_cb, NULL)); - } /* Special case if failure was due to authentication issues since this upstream could be used oppotunistically with no problem.*/ if (!(upstream->transport == GETDNS_TRANSPORT_TLS && @@ -545,6 +536,14 @@ upstream_failed(getdns_upstream *upstream, int during_setup) upstream->conn_shutdowns++; /* [TLS1]TODO: Re-try these queries if possible.*/ } + upstream->conn_state = GETDNS_CONN_TEARDOWN; + + if (upstream->write_queue) { + if (!during_setup) + during_setup = -1; + while (upstream->write_queue) + upstream_write_cb(upstream); + } while (upstream->netreq_by_query_id.count) { netreq = (getdns_network_req *) _getdns_rbtree_first(&upstream->netreq_by_query_id); @@ -552,7 +551,13 @@ upstream_failed(getdns_upstream *upstream, int during_setup) _getdns_netreq_change_state(netreq, NET_REQ_ERRORED); _getdns_check_dns_req_complete(netreq->owner); } - upstream->conn_state = GETDNS_CONN_TEARDOWN; + if (during_setup == 0) + return; + + else if (during_setup > 0) + _getdns_upstream_reset(upstream); + else + _getdns_upstream_shutdown(upstream); } void @@ -613,42 +618,12 @@ upstream_idle_timeout_cb(void *userarg) static void upstream_setup_timeout_cb(void *userarg) { - int ret; getdns_upstream *upstream = (getdns_upstream *)userarg; -#ifdef USE_POLL_DEFAULT_EVENTLOOP - struct pollfd fds; -#else - fd_set fds; - struct timeval tval; -#endif DEBUG_STUB("%s %-35s: FD: %d\n", STUB_DEBUG_CLEANUP, __FUNC__, upstream->fd); - /* Clean up and trigger a write to let the fallback code to its job */ - upstream_failed(upstream, 1); - /* Need to handle the case where the far end doesn't respond to a - * TCP SYN and doesn't do a reset (as is the case with e.g. 8.8.8.8@853). - * For that case the socket never becomes writable so doesn't trigger any - * callbacks. If so then clear out the queue in one go.*/ -#ifdef USE_POLL_DEFAULT_EVENTLOOP - fds.fd = upstream->fd; - fds.events = POLLOUT; - ret = _getdns_poll(&fds, 1, 0); -#else - FD_ZERO(&fds); - FD_SET((int)(upstream->fd), &fds); - tval.tv_sec = 0; - tval.tv_usec = 0; - ret = select(upstream->fd+1, NULL, &fds, NULL, &tval); -#endif - if (ret == 0) { - DEBUG_STUB("%s %-35s: FD: %d Cleaning up dangling queue\n", - STUB_DEBUG_CLEANUP, __FUNC__, upstream->fd); - while (upstream->write_queue) - upstream_write_cb(upstream); - } - _getdns_upstream_reset(upstream); + upstream_failed(upstream, 1); } @@ -748,6 +723,7 @@ stub_tcp_write(int fd, getdns_tcp_state *tcp, getdns_network_req *netreq) } while (!_getdns_rbtree_insert( &netreq->upstream->netreq_by_query_id, &netreq->node)); + netreq->query_id_registered = &netreq->upstream->netreq_by_query_id; GLDNS_ID_SET(netreq->query, query_id); @@ -1241,6 +1217,7 @@ stub_tls_write(getdns_upstream *upstream, getdns_tcp_state *tcp, } while (!_getdns_rbtree_insert( &netreq->upstream->netreq_by_query_id, &netreq->node)); + netreq->query_id_registered = &netreq->upstream->netreq_by_query_id; GLDNS_ID_SET(netreq->query, query_id); @@ -1555,12 +1532,9 @@ upstream_read_cb(void *userarg) case STUB_SETUP_ERROR: /* Can happen for TLS HS*/ case STUB_TCP_ERROR: upstream_failed(upstream, (q == STUB_TCP_ERROR ? 0:1) ); - if (!upstream->write_queue) - _getdns_upstream_shutdown(upstream); return; default: - /* Lookup netreq */ query_id = (uint16_t) q; query_id_intptr = (intptr_t) query_id; @@ -1572,7 +1546,16 @@ upstream_read_cb(void *userarg) upstream->tcp.to_read = 2; return; } + if (netreq->query_id_registered == &upstream->netreq_by_query_id) { + netreq->query_id_registered = NULL; + netreq->node.key = NULL; + } else if (netreq->query_id_registered) { + (void) _getdns_rbtree_delete( + netreq->query_id_registered, netreq->node.key); + netreq->query_id_registered = NULL; + netreq->node.key = NULL; + } DEBUG_STUB("%s %-35s: MSG: %p (read)\n", STUB_DEBUG_READ, __FUNC__, (void*)netreq); _getdns_netreq_change_state(netreq, NET_REQ_FINISHED); @@ -1685,10 +1668,8 @@ upstream_write_cb(void *userarg) /* Could not complete the set up. Need to fallback.*/ DEBUG_STUB("%s %-35s: Upstream: %p ERROR = %d\n", STUB_DEBUG_WRITE, __FUNC__, (void*)userarg, q); - (void) _getdns_rbtree_delete(&upstream->netreq_by_query_id, - (void *)(intptr_t)GLDNS_ID_WIRE(netreq->query)); upstream_failed(upstream, (q == STUB_TCP_ERROR ? 0:1)); - /* Fall through */ + return; case STUB_CONN_GONE: case STUB_NO_AUTH: /* Cleaning up after connection or auth check failure. Need to fallback. */ @@ -1702,11 +1683,12 @@ upstream_write_cb(void *userarg) _getdns_netreq_change_state(netreq, NET_REQ_ERRORED); _getdns_check_dns_req_complete(netreq->owner); } - if (!upstream->write_queue) - _getdns_upstream_shutdown(upstream); return; default: + /* Unqueue the netreq from the write_queue */ + remove_from_write_queue(upstream, netreq); + if (netreq->owner->return_call_reporting && netreq->upstream->tls_obj && netreq->debug_tls_peer_cert.data == NULL && @@ -1719,9 +1701,6 @@ upstream_write_cb(void *userarg) netreq->debug_tls_auth_status = netreq->upstream->tls_auth_state; upstream->queries_sent++; - /* Unqueue the netreq from the write_queue */ - remove_from_write_queue(upstream, netreq); - /* Empty write_queue?, then deschedule upstream write_cb */ if (upstream->write_queue == NULL) { assert(upstream->write_queue_last == NULL); @@ -2015,7 +1994,6 @@ upstream_connect(getdns_upstream *upstream, getdns_transport_list_t transport, fd = tcp_connect(upstream, transport); if (fd == -1) { upstream_failed(upstream, 1); - _getdns_upstream_reset(upstream); return -1; } upstream->loop = dnsreq->loop; @@ -2025,7 +2003,6 @@ upstream_connect(getdns_upstream *upstream, getdns_transport_list_t transport, upstream->tls_obj = tls_create_object(dnsreq, fd, upstream); if (upstream->tls_obj == NULL) { upstream_failed(upstream, 1); - _getdns_upstream_reset(upstream); _getdns_closesocket(fd); return -1; } @@ -2137,7 +2114,7 @@ fallback_on_write(getdns_network_req *netreq) } static void -upstream_reschedule_events(getdns_upstream *upstream, uint64_t idle_timeout) { +upstream_reschedule_events(getdns_upstream *upstream) { DEBUG_STUB("%s %-35s: FD: %d \n", STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd); @@ -2159,7 +2136,8 @@ upstream_reschedule_events(getdns_upstream *upstream, uint64_t idle_timeout) { upstream->fd, TIMEOUT_FOREVER, &upstream->event); else { DEBUG_STUB("%s %-35s: FD: %d Connection idle - timeout is %d\n", - STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd, (int)idle_timeout); + STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd, + (int)upstream->keepalive_timeout); /* TODO: Schedule a read also anyway, * to digest timed out answers. * Dont forget to schedule with upstream->fd then! @@ -2167,10 +2145,9 @@ upstream_reschedule_events(getdns_upstream *upstream, uint64_t idle_timeout) { * upstream->event.read_cb = upstream_read_cb; */ upstream->event.timeout_cb = upstream_idle_timeout_cb; - if (upstream->conn_state != GETDNS_CONN_OPEN) - idle_timeout = 0; GETDNS_SCHEDULE_EVENT(upstream->loop, -1, - idle_timeout, &upstream->event); + ( upstream->conn_state == GETDNS_CONN_OPEN + ? upstream->keepalive_timeout : 0), &upstream->event); } } diff --git a/src/types-internal.h b/src/types-internal.h index 56b7a2f0..05589f4a 100644 --- a/src/types-internal.h +++ b/src/types-internal.h @@ -188,7 +188,9 @@ typedef struct getdns_tcp_state { typedef struct getdns_network_req { /* For storage in upstream->netreq_by_query_id */ - _getdns_rbnode_t node; + _getdns_rbnode_t node; + /* The netreq_by_query_id tree in which this netreq was registered */ + _getdns_rbtree_t *query_id_registered; #ifdef HAVE_MDNS_SUPPORT /* * for storage of continuous query context in hash table of cached results. From dc5a78b154183d42148277d8d7fd55f84d4c111c Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 17 Oct 2017 14:19:59 +0200 Subject: [PATCH 2/9] Printing something which is not on stack (causing segfault in some cases) --- src/stub.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stub.c b/src/stub.c index 31bfb786..5eb1705a 100644 --- a/src/stub.c +++ b/src/stub.c @@ -1907,8 +1907,8 @@ upstream_select_stateful(getdns_network_req *netreq, getdns_transport_list_t tra upstream->conn_state = GETDNS_CONN_CLOSED; upstream->conn_backoff_interval = 1; _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_NOTICE, - "%-40s : No valid upstreams... promoting backed-off upstream %s for re-try...\n", - upstreams->upstreams[i].addr_str); + "%-40s : No valid upstreams... promoting this backed-off upstream for re-try...\n", + upstream->addr_str); return upstream; } From fa597399e2d214685163d025809c67245769dd37 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 17 Oct 2017 15:14:09 +0200 Subject: [PATCH 3/9] Update stubby --- stubby | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stubby b/stubby index d541e605..c9e89293 160000 --- a/stubby +++ b/stubby @@ -1 +1 @@ -Subproject commit d541e60530717f590d4a4a9d458f987cbe90fc59 +Subproject commit c9e89293beac4fe599024bfb021cb2ae832cc93f From eedd1a14482393505e2dfc1c0f6b80f76d851b5d Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Tue, 17 Oct 2017 16:58:01 +0200 Subject: [PATCH 4/9] Eat incoming garbage on statefull transports Can deal with timed out queries that are answered anyway. + reset the upstream on failure always (since requests are rescheduled for fallback by upstream_failed now anyway) --- src/context.c | 15 +++++++++++---- src/stub.c | 26 ++++++++++++-------------- src/sync.c | 34 ++++++++++++++-------------------- src/tools/getdns_query.c | 17 ++++++++++++----- 4 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/context.c b/src/context.c index 3456bfa0..ccbb52a1 100644 --- a/src/context.c +++ b/src/context.c @@ -800,6 +800,10 @@ _getdns_upstream_reset(getdns_upstream *upstream) /* Now TLS stuff*/ upstream->tls_auth_state = GETDNS_AUTH_NONE; + if (upstream->event.ev && upstream->loop) { + upstream->loop->vmt->clear( + upstream->loop, &upstream->event); + } if (upstream->tls_obj != NULL) { SSL_shutdown(upstream->tls_obj); SSL_free(upstream->tls_obj); @@ -2769,11 +2773,14 @@ getdns_context_set_upstream_recursive_servers(struct getdns_context *context, struct addrinfo hints; RETURN_IF_NULL(context, GETDNS_RETURN_INVALID_PARAMETER); - RETURN_IF_NULL(upstream_list, GETDNS_RETURN_INVALID_PARAMETER); - r = getdns_list_get_length(upstream_list, &count); - if (count == 0 || r != GETDNS_RETURN_GOOD) { - return GETDNS_RETURN_CONTEXT_UPDATE_FAIL; + if ( !upstream_list + || (r = getdns_list_get_length(upstream_list, &count)) + || count == 0) { + _getdns_upstreams_dereference(context->upstreams); + context->upstreams = NULL; + dispatch_updated(context, + GETDNS_CONTEXT_CODE_UPSTREAM_RECURSIVE_SERVERS); } memset(&hints, 0, sizeof(struct addrinfo)); hints.ai_family = AF_UNSPEC; /* Allow IPv4 or IPv6 */ diff --git a/src/stub.c b/src/stub.c index 5eb1705a..fda8df8f 100644 --- a/src/stub.c +++ b/src/stub.c @@ -551,10 +551,7 @@ upstream_failed(getdns_upstream *upstream, int during_setup) _getdns_netreq_change_state(netreq, NET_REQ_ERRORED); _getdns_check_dns_req_complete(netreq->owner); } - if (during_setup == 0) - return; - - else if (during_setup > 0) + if (during_setup > 0) _getdns_upstream_reset(upstream); else _getdns_upstream_shutdown(upstream); @@ -2118,7 +2115,13 @@ upstream_reschedule_events(getdns_upstream *upstream) { DEBUG_STUB("%s %-35s: FD: %d \n", STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd); - GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); + if (upstream->event.ev) + GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); + + if (upstream->fd == -1 || !( upstream->conn_state == GETDNS_CONN_SETUP + || upstream->conn_state == GETDNS_CONN_OPEN )) + return; + if (!upstream->write_queue && upstream->event.write_cb) { upstream->event.write_cb = NULL; } @@ -2138,16 +2141,11 @@ upstream_reschedule_events(getdns_upstream *upstream) { DEBUG_STUB("%s %-35s: FD: %d Connection idle - timeout is %d\n", STUB_DEBUG_SCHEDULE, __FUNC__, upstream->fd, (int)upstream->keepalive_timeout); - /* TODO: Schedule a read also anyway, - * to digest timed out answers. - * Dont forget to schedule with upstream->fd then! - * - * upstream->event.read_cb = upstream_read_cb; - */ + + upstream->event.read_cb = upstream_read_cb; upstream->event.timeout_cb = upstream_idle_timeout_cb; - GETDNS_SCHEDULE_EVENT(upstream->loop, -1, - ( upstream->conn_state == GETDNS_CONN_OPEN - ? upstream->keepalive_timeout : 0), &upstream->event); + GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, + upstream->keepalive_timeout, &upstream->event); } } diff --git a/src/sync.c b/src/sync.c index c618b0e4..debf904b 100644 --- a/src/sync.c +++ b/src/sync.c @@ -116,30 +116,24 @@ getdns_sync_data_cleanup(getdns_sync_data *data) upstream = &ctxt->upstreams->upstreams[i]; if (upstream->loop != &data->context->sync_eventloop.loop) continue; - if (upstream->event.read_cb || upstream->event.write_cb) { - GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); - - } else if (upstream->event.timeout_cb) { - /* Timeout's at upstream are idle-timeouts only. - * They should be fired on completion of the - * synchronous request. - */ - GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); - if (upstream->conn_state != GETDNS_CONN_OPEN || - upstream->keepalive_timeout == 0) - (*upstream->event.timeout_cb)(upstream->event.userarg); + GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); + if (upstream->event.timeout_cb && + ( upstream->conn_state != GETDNS_CONN_OPEN + || upstream->keepalive_timeout == 0)) { + (*upstream->event.timeout_cb)(upstream->event.userarg); + upstream->event.timeout_cb = NULL; } upstream->loop = data->context->extension; upstream->is_sync_loop = 0; - if (upstream->event.read_cb || upstream->event.write_cb) - GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, - TIMEOUT_FOREVER, &upstream->event); - else if (upstream->event.timeout_cb && - upstream->conn_state == GETDNS_CONN_OPEN && - upstream->keepalive_timeout != 0) { - GETDNS_SCHEDULE_EVENT(upstream->loop, upstream->fd, - upstream->keepalive_timeout, &upstream->event); + if ( upstream->event.read_cb || upstream->event.write_cb + || upstream->event.timeout_cb) { + GETDNS_SCHEDULE_EVENT(upstream->loop, + ( upstream->event.read_cb + || upstream->event.write_cb ? upstream->fd : -1), + ( upstream->event.timeout_cb + ? upstream->keepalive_timeout : TIMEOUT_FOREVER ), + &upstream->event); } } } diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index eff84136..b886ae59 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -1279,7 +1279,8 @@ void read_line_cb(void *userarg) if (listen_count) (void) getdns_context_set_listen_addresses( context, NULL, NULL, NULL); - (void) getdns_context_set_idle_timeout(context, 0); + (void) getdns_context_set_upstream_recursive_servers( + context, NULL); return; } if (query_file && verbosity) @@ -1678,16 +1679,22 @@ static void stubby_log(void *userarg, uint64_t system, #ifdef GETDNS_ON_WINDOWS time_t tsec; + if (!verbosity) + return; + gettimeofday(&tv, NULL); tsec = (time_t) tv.tv_sec; gmtime_s(&tm, (const time_t *) &tsec); #else + if (!verbosity) + return; + gettimeofday(&tv, NULL); gmtime_r(&tv.tv_sec, &tm); #endif strftime(buf, 10, "%H:%M:%S", &tm); (void)userarg; (void)system; (void)level; - (void) fprintf(stderr, "[%s.%.6d] STUBBY: ", buf, (int)tv.tv_usec); + (void) fprintf(stderr, "[%s.%.6d] UPSTREAM ", buf, (int)tv.tv_usec); (void) vfprintf(stderr, fmt, ap); } @@ -1741,10 +1748,10 @@ main(int argc, char **argv) (void) parse_config_file(home_stubby_conf_fn, 0); } clear_listen_list_on_arg = 1; - - (void) getdns_context_set_logfunc(context, NULL, - GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_DEBUG, stubby_log); } + (void) getdns_context_set_logfunc(context, NULL, + GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_DEBUG, stubby_log); + if ((r = parse_args(argc, argv))) goto done_destroy_context; clear_listen_list_on_arg = 0; From 87879783ec9d348698903f7ad02109c424ade752 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 18 Oct 2017 14:33:59 +0200 Subject: [PATCH 5/9] Postpone dealing with upstream derenferencing issue --- src/tools/getdns_query.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools/getdns_query.c b/src/tools/getdns_query.c index b886ae59..53e546e2 100644 --- a/src/tools/getdns_query.c +++ b/src/tools/getdns_query.c @@ -1279,8 +1279,9 @@ void read_line_cb(void *userarg) if (listen_count) (void) getdns_context_set_listen_addresses( context, NULL, NULL, NULL); - (void) getdns_context_set_upstream_recursive_servers( - context, NULL); + if (interactive && !query_file) + (void) getdns_context_set_upstream_recursive_servers( + context, NULL); return; } if (query_file && verbosity) From 8886c5317dc18576e9dec8928ccda1b499d73bcb Mon Sep 17 00:00:00 2001 From: Sara Dickinson Date: Thu, 19 Oct 2017 10:36:46 +0100 Subject: [PATCH 6/9] Fix 2 bugs: - backoff time was not incrementing correctly - best authentication information state was not being kept for shutdowns during setup (needed if e.g. hostname authentication failed during handshake). --- src/context.c | 3 ++- src/stub.c | 11 +++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/context.c b/src/context.c index ccbb52a1..01688105 100644 --- a/src/context.c +++ b/src/context.c @@ -754,8 +754,9 @@ upstream_backoff(getdns_upstream *upstream) { upstream->conn_shutdowns = 0; upstream->conn_backoffs++; _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_NOTICE, - "%-40s : !Backing off this upstream - Will retry again at %s", + "%-40s : !Backing off this upstream - Will retry again in %ds at %s", upstream->addr_str, + upstream->conn_backoff_interval, asctime(gmtime(&upstream->conn_retry_time))); } diff --git a/src/stub.c b/src/stub.c index fda8df8f..3a114712 100644 --- a/src/stub.c +++ b/src/stub.c @@ -551,10 +551,7 @@ upstream_failed(getdns_upstream *upstream, int during_setup) _getdns_netreq_change_state(netreq, NET_REQ_ERRORED); _getdns_check_dns_req_complete(netreq->owner); } - if (during_setup > 0) - _getdns_upstream_reset(upstream); - else - _getdns_upstream_shutdown(upstream); + _getdns_upstream_shutdown(upstream); } void @@ -957,8 +954,11 @@ tls_create_object(getdns_dns_req *dnsreq, int fd, getdns_upstream *upstream) X509_VERIFY_PARAM_set1_host(param, upstream->tls_auth_name, 0); #else if (dnsreq->netreqs[0]->tls_auth_min == GETDNS_AUTHENTICATION_REQUIRED) { - DEBUG_STUB("%s %-35s: ERROR: TLS Authentication functionality not available\n", + DEBUG_STUB("%s %-35s: ERROR: Hostname Authentication not available from TLS library (check library version)\n", STUB_DEBUG_SETUP_TLS, __FUNC__); + _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_ERR, + "%-40s : ERROR: Hostname Authentication not available from TLS library (check library version)\n", + upstream->addr_str); upstream->tls_hs_state = GETDNS_HS_FAILED; return NULL; } @@ -1843,7 +1843,6 @@ upstream_select_stateful(getdns_network_req *netreq, getdns_transport_list_t tra if (upstreams->upstreams[i].conn_state == GETDNS_CONN_BACKOFF && upstreams->upstreams[i].conn_retry_time < now) { upstreams->upstreams[i].conn_state = GETDNS_CONN_CLOSED; - upstreams->upstreams[i].conn_backoff_interval = 1; _getdns_upstream_log(upstream, GETDNS_LOG_UPSTREAM_STATS, GETDNS_LOG_NOTICE, "%-40s : Re-instating upstream\n", upstreams->upstreams[i].addr_str); From 272d0cf0efbae3b03bd02e29bdf20bb0e5b2d550 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 19 Oct 2017 12:35:10 +0200 Subject: [PATCH 7/9] Allow clearing of upstreams --- src/test/check_getdns_context_set_upstream_recursive_servers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/check_getdns_context_set_upstream_recursive_servers.h b/src/test/check_getdns_context_set_upstream_recursive_servers.h index e1bb359f..85089e3c 100644 --- a/src/test/check_getdns_context_set_upstream_recursive_servers.h +++ b/src/test/check_getdns_context_set_upstream_recursive_servers.h @@ -65,7 +65,7 @@ CONTEXT_CREATE(TRUE); ASSERT_RC(getdns_context_set_upstream_recursive_servers(context, NULL), - GETDNS_RETURN_INVALID_PARAMETER, "Return code from getdns_context_set_upstream_recursive_servers()"); + GETDNS_RETURN_GOOD, "Return code from getdns_context_set_upstream_recursive_servers()"); CONTEXT_DESTROY; From f8e1ed78b8e249963d0d63becd84bb3a76709b6d Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 19 Oct 2017 12:48:58 +0200 Subject: [PATCH 8/9] Make upstream_reset static (and not shared between .c files) --- src/context.c | 4 ++-- src/context.h | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/context.c b/src/context.c index 01688105..994a9ac4 100644 --- a/src/context.c +++ b/src/context.c @@ -733,7 +733,7 @@ void _getdns_upstream_log(getdns_upstream *upstream, uint64_t system, va_end(args); } -void +static void upstream_backoff(getdns_upstream *upstream) { upstream->conn_state = GETDNS_CONN_BACKOFF; /* Increase the backoff interval incrementally up to the tls_backoff_time*/ @@ -760,7 +760,7 @@ upstream_backoff(getdns_upstream *upstream) { asctime(gmtime(&upstream->conn_retry_time))); } -void +static void _getdns_upstream_reset(getdns_upstream *upstream) { /* Back off connections that never got up service at all (probably no diff --git a/src/context.h b/src/context.h index 46f000c0..1a6d93a4 100644 --- a/src/context.h +++ b/src/context.h @@ -557,6 +557,4 @@ int _getdns_context_write_priv_file(getdns_context *context, int _getdns_context_can_write_appdata(getdns_context *context); -void _getdns_upstream_reset(getdns_upstream *upstream); - #endif /* _GETDNS_CONTEXT_H_ */ From fc073267f1d37f964a6f694eb2b640812ee91153 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Thu, 19 Oct 2017 14:14:37 +0200 Subject: [PATCH 9/9] Dead assignment --- src/stub.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/stub.c b/src/stub.c index 3a114712..49af9d8c 100644 --- a/src/stub.c +++ b/src/stub.c @@ -524,9 +524,8 @@ upstream_failed(getdns_upstream *upstream, int during_setup) when idle.*/ /* [TLS1]TODO: Work out how to re-open the connection and re-try the queries if there is only one upstream.*/ + GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); if (during_setup) { - /* Reset timeout on setup failure to trigger fallback handling.*/ - GETDNS_CLEAR_EVENT(upstream->loop, &upstream->event); /* Special case if failure was due to authentication issues since this upstream could be used oppotunistically with no problem.*/ if (!(upstream->transport == GETDNS_TRANSPORT_TLS && @@ -538,12 +537,9 @@ upstream_failed(getdns_upstream *upstream, int during_setup) } upstream->conn_state = GETDNS_CONN_TEARDOWN; - if (upstream->write_queue) { - if (!during_setup) - during_setup = -1; - while (upstream->write_queue) - upstream_write_cb(upstream); - } + while (upstream->write_queue) + upstream_write_cb(upstream); + while (upstream->netreq_by_query_id.count) { netreq = (getdns_network_req *) _getdns_rbtree_first(&upstream->netreq_by_query_id);