From: Grigorii Demidov Date: Fri, 30 Nov 2018 15:43:32 +0000 (+0100) Subject: bugfixes in tcp connection error handling X-Git-Tag: v3.2.0~15^2~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d51e9287ded0f10e7d0fbf4a41ab000cad976f46;p=thirdparty%2Fknot-resolver.git bugfixes in tcp connection error handling --- diff --git a/daemon/session.c b/daemon/session.c index c96a78397..1975258a9 100644 --- a/daemon/session.c +++ b/daemon/session.c @@ -398,7 +398,7 @@ void session_waitinglist_retry(struct session *session, bool increase_timeout_cn if (increase_timeout_cnt) { worker_task_timeout_inc(task); } - worker_task_step(task, NULL, NULL); + worker_task_step(task, &session->peer.ip, NULL); worker_task_unref(task); } } diff --git a/daemon/worker.c b/daemon/worker.c index 5c153811e..81856fa93 100644 --- a/daemon/worker.c +++ b/daemon/worker.c @@ -794,6 +794,10 @@ static void on_connect(uv_connect_t *req, int status) assert(session_flags(session)->outgoing); if (status == UV_ECANCELED) { + if (kr_verbose_status) { + const char *peer_str = kr_straddr(peer); + kr_log_verbose( "[wrkr]=> connect to '%s' cancelled\n", peer_str ? peer_str : ""); + } worker_del_tcp_waiting(worker, peer); assert(session_is_empty(session) && session_flags(session)->closing); return; @@ -806,6 +810,11 @@ static void on_connect(uv_connect_t *req, int status) } if (status != 0) { + if (kr_verbose_status) { + const char *peer_str = kr_straddr(peer); + kr_log_verbose( "[wrkr]=> connect to '%s' failed (%s)\n", + peer_str ? peer_str : "", uv_strerror(status)); + } worker_del_tcp_waiting(worker, peer); assert(session_tasklist_is_empty(session)); session_waitinglist_retry(session, false); @@ -826,13 +835,9 @@ static void on_connect(uv_connect_t *req, int status) } } - struct qr_task *task = session_waitinglist_get(session); - struct kr_query *qry = task_get_last_pending_query(task); - WITH_VERBOSE (qry) { - struct sockaddr *peer = session_get_peer(session); - char peer_str[INET6_ADDRSTRLEN]; - inet_ntop(peer->sa_family, kr_inaddr(peer), peer_str, sizeof(peer_str)); - VERBOSE_MSG(qry, "=> connected to '%s'\n", peer_str); + if (kr_verbose_status) { + const char *peer_str = kr_straddr(peer); + kr_log_verbose( "[wrkr]=> connected to '%s'\n", peer_str ? peer_str : ""); } session_flags(session)->connected = true; @@ -1300,8 +1305,8 @@ static int tcp_task_step(struct qr_task *task, { assert(task->pending_count == 0); - const struct sockaddr *addr = - packet_source ? packet_source : task->addrlist; + /* target */ + const struct sockaddr *addr = task->addrlist; if (addr->sa_family == AF_UNSPEC) { /* Target isn't defined. Finalize task with SERVFAIL. * Although task->pending_count is zero, there are can be followers, diff --git a/lib/nsrep.c b/lib/nsrep.c index 57747dd32..47f6ac9ae 100644 --- a/lib/nsrep.c +++ b/lib/nsrep.c @@ -474,13 +474,18 @@ int kr_nsrep_copy_set(struct kr_nsrep *dst, const struct kr_nsrep *src) return kr_ok(); } -int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache) +int kr_nsrep_sort(struct kr_nsrep *ns, struct kr_context *ctx) { - if (!ns || !rtt_cache) { + if (!ns || !ctx) { assert(false); return kr_error(EINVAL); } + kr_nsrep_rtt_lru_t *rtt_cache = ctx->cache_rtt; + + ns->reputation = 0; + ns->score = KR_NS_MAX_SCORE + 1; + if (ns->addr[0].ip.sa_family == AF_UNSPEC) { return kr_error(EINVAL); } @@ -489,6 +494,7 @@ int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache) * along the addresses. */ unsigned scores[KR_NSREP_MAXADDR]; int i; + bool timeouted_address_is_already_selected = false; for (i = 0; i < KR_NSREP_MAXADDR; ++i) { const struct sockaddr *sa = &ns->addr[i].ip; if (sa->sa_family == AF_UNSPEC) { @@ -499,10 +505,23 @@ int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache) kr_family_len(sa->sa_family)); if (!rtt_cache_entry) { scores[i] = 1; /* prefer unknown to probe RTT */ - } else if ((kr_rand_uint(100) < 10) && - (kr_rand_uint(KR_NS_MAX_SCORE) >= rtt_cache_entry->score)) { - /* some probability to bump bad ones up for re-probe */ - scores[i] = 1; + if (sa->sa_family == AF_INET) { + scores[i] += FAVOUR_IPV6; + } + } else if (rtt_cache_entry->score >= KR_NS_TIMEOUT) { + uint64_t now = kr_now(); + uint64_t elapsed = now - rtt_cache_entry->tout_timestamp; + scores[i] = KR_NS_MAX_SCORE + 1; + elapsed = elapsed > UINT_MAX ? UINT_MAX : elapsed; + if (elapsed > ctx->cache_rtt_tout_retry_interval && + !timeouted_address_is_already_selected) { + scores[i] = 1; + if (sa->sa_family == AF_INET) { + scores[i] += FAVOUR_IPV6; + } + rtt_cache_entry->tout_timestamp = now; + timeouted_address_is_already_selected = true; + } } else { scores[i] = rtt_cache_entry->score; if (sa->sa_family == AF_INET) { @@ -510,10 +529,8 @@ int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache) } } if (VERBOSE_STATUS) { - char sa_str[INET6_ADDRSTRLEN]; - inet_ntop(sa->sa_family, kr_inaddr(sa), sa_str, sizeof(sa_str)); kr_log_verbose("[ ][nsre] score %d for %s;\t cached RTT: %d\n", - scores[i], sa_str, + scores[i], kr_straddr(sa), rtt_cache_entry ? rtt_cache_entry->score : -1); } } @@ -535,9 +552,10 @@ int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache) } } - /* At least two addresses must be in the address list */ - assert(count > 0); - ns->score = scores[0]; - ns->reputation = 0; + if (count > 0) { + ns->score = scores[0]; + ns->reputation = 0; + } + return kr_ok(); } diff --git a/lib/nsrep.h b/lib/nsrep.h index 15af9fa08..0318de844 100644 --- a/lib/nsrep.h +++ b/lib/nsrep.h @@ -174,12 +174,14 @@ int kr_nsrep_update_rep(struct kr_nsrep *ns, unsigned reputation, kr_nsrep_lru_t int kr_nsrep_copy_set(struct kr_nsrep *dst, const struct kr_nsrep *src); /** - * Sort addresses in the query nsrep list + * Sort addresses in the query nsrep list by cached RTT. + * if RTT is greater then KR_NS_TIMEOUT, address will placed at the beginning of the + * nsrep list once in cache.ns_tout() milliseconds. Otherwise it will be sorted + * as if it has cached RTT equal to KR_NS_MAX_SCORE + 1. * @param ns updated kr_nsrep - * @param rtt_cache RTT LRU cache + * @param ctx name resolution context. * @return 0 or an error code - * @note ns reputation is zeroed, as KR_NS_NOIP{4,6} flags are useless - * in STUB/FORWARD mode. + * @note ns reputation is zeroed and score is set to KR_NS_MAX_SCORE + 1. */ KR_EXPORT -int kr_nsrep_sort(struct kr_nsrep *ns, kr_nsrep_rtt_lru_t *rtt_cache); +int kr_nsrep_sort(struct kr_nsrep *ns, struct kr_context *ctx); diff --git a/lib/resolve.c b/lib/resolve.c index 400993677..22d64fdee 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -1423,12 +1423,14 @@ ns_election: const struct kr_qflags qflg = qry->flags; const bool retry = qflg.TCP || qflg.BADCOOKIE_AGAIN; - bool check_timeouted = false; if (qflg.AWAIT_IPV4 || qflg.AWAIT_IPV6) { kr_nsrep_elect_addr(qry, request->ctx); } else if (qflg.FORWARD || qflg.STUB) { - kr_nsrep_sort(&qry->ns, request->ctx->cache_rtt); - check_timeouted = true; + kr_nsrep_sort(&qry->ns, request->ctx); + if (qry->ns.score > KR_NS_MAX_SCORE) { + VERBOSE_MSG(qry, "=> no valid NS left\n"); + return KR_STATE_FAIL; + } } else if (!qry->ns.name || !retry) { /* Keep NS when requerying/stub/badcookie. */ /* Root DNSKEY must be fetched from the hints to avoid chicken and egg problem. */ if (qry->sname[0] == '\0' && qry->stype == KNOT_RRTYPE_DNSKEY) { @@ -1436,22 +1438,20 @@ ns_election: qry->flags.NO_THROTTLE = true; /* Pick even bad SBELT servers */ } kr_nsrep_elect(qry, request->ctx); - check_timeouted = true; - } - - if (check_timeouted && qry->ns.score > KR_NS_TIMEOUT) { - if (kr_zonecut_is_empty(&qry->zone_cut)) { - VERBOSE_MSG(qry, "=> no NS with an address\n"); - } else { - VERBOSE_MSG(qry, "=> no valid NS left\n"); - } - if (!qry->flags.NO_NS_FOUND) { - qry->flags.NO_NS_FOUND = true; - } else { - ITERATE_LAYERS(request, qry, reset); - kr_rplan_pop(rplan, qry); + if (qry->ns.score > KR_NS_MAX_SCORE) { + if (kr_zonecut_is_empty(&qry->zone_cut)) { + VERBOSE_MSG(qry, "=> no NS with an address\n"); + } else { + VERBOSE_MSG(qry, "=> no valid NS left\n"); + } + if (!qry->flags.NO_NS_FOUND) { + qry->flags.NO_NS_FOUND = true; + } else { + ITERATE_LAYERS(request, qry, reset); + kr_rplan_pop(rplan, qry); + } + return KR_STATE_PRODUCE; } - return KR_STATE_PRODUCE; } /* Resolve address records */