diff --git a/CHANGES b/CHANGES index f5a5a86b96..03a8be49d3 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6141. [bug] Fix several issues in nsupdate timeout handling and + update the -t option's documentation. [GL #3674] + 6140. [func] Implement automatic parental-agents ('checkds yes'). [GL #3901] diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index a52dfe76e8..c4f16ae03a 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -94,12 +94,11 @@ #include "../dig/readline.h" -#define MAXCMD (128 * 1024) -#define MAXWIRE (64 * 1024) -#define INITTEXT (2 * 1024) -#define MAXTEXT (128 * 1024) -#define FIND_TIMEOUT 5 -#define TTL_MAX 2147483647U /* Maximum signed 32 bit integer. */ +#define MAXCMD (128 * 1024) +#define MAXWIRE (64 * 1024) +#define INITTEXT (2 * 1024) +#define MAXTEXT (128 * 1024) +#define TTL_MAX 2147483647U /* Maximum signed 32 bit integer. */ #define DNSDEFAULTPORT 53 @@ -1243,9 +1242,6 @@ parse_args(int argc, char **argv) { isc_commandline_argument); exit(1); } - if (udp_timeout == 0) { - udp_timeout = UINT_MAX; - } break; case 'r': result = isc_parse_uint32(&udp_retries, @@ -2721,11 +2717,11 @@ recvsoa(void *arg) { srcaddr = localaddr4; } - result = dns_request_create( - requestmgr, soaquery, srcaddr, addr, req_transport, - req_tls_ctx_cache, options, NULL, FIND_TIMEOUT * 20, - FIND_TIMEOUT, 3, isc_loop_main(loopmgr), recvsoa, - reqinfo, &request); + result = dns_request_create(requestmgr, soaquery, srcaddr, addr, + req_transport, req_tls_ctx_cache, + options, NULL, timeout, udp_timeout, + udp_retries, isc_loop_main(loopmgr), + recvsoa, reqinfo, &request); check_result(result, "dns_request_create"); requests++; return; @@ -2961,7 +2957,7 @@ sendrequest(isc_sockaddr_t *destaddr, dns_message_t *msg, result = dns_request_create( requestmgr, msg, srcaddr, destaddr, req_transport, req_tls_ctx_cache, options, default_servers ? NULL : tsigkey, - FIND_TIMEOUT * 20, FIND_TIMEOUT, 3, isc_loop_main(loopmgr), + timeout, udp_timeout, udp_retries, isc_loop_main(loopmgr), recvsoa, reqinfo, request); check_result(result, "dns_request_create"); requests++; @@ -3168,11 +3164,10 @@ send_gssrequest(isc_sockaddr_t *destaddr, dns_message_t *msg, srcaddr = localaddr4; } - result = dns_request_create(requestmgr, msg, srcaddr, destaddr, - req_transport, req_tls_ctx_cache, options, - tsigkey, FIND_TIMEOUT * 20, FIND_TIMEOUT, 3, - isc_loop_main(loopmgr), recvgss, reqinfo, - request); + result = dns_request_create( + requestmgr, msg, srcaddr, destaddr, req_transport, + req_tls_ctx_cache, options, tsigkey, timeout, udp_timeout, + udp_retries, isc_loop_main(loopmgr), recvgss, reqinfo, request); check_result(result, "dns_request_create"); if (debugging) { show_message(stdout, msg, "Outgoing update query:"); @@ -3506,6 +3501,8 @@ getinput(void *arg) { int main(int argc, char **argv) { + uint32_t timeoutms; + style = &dns_master_style_debug; input = stdin; @@ -3528,6 +3525,10 @@ main(int argc, char **argv) { parse_args(argc, argv); + /* Set the network manager timeouts in milliseconds. */ + timeoutms = timeout * 1000; + isc_nm_settimeouts(netmgr, timeoutms, timeoutms, timeoutms, timeoutms); + setup_system(); isc_loopmgr_setup(loopmgr, getinput, NULL); diff --git a/bin/nsupdate/nsupdate.rst b/bin/nsupdate/nsupdate.rst index cc1aa39faf..481fbbb6ef 100644 --- a/bin/nsupdate/nsupdate.rst +++ b/bin/nsupdate/nsupdate.rst @@ -187,7 +187,11 @@ Options .. option:: -t timeout This option sets the maximum time an update request can take before it is aborted. The - default is 300 seconds. If zero, the timeout is disabled. + default is 300 seconds. If zero, the timeout is disabled for TCP mode. For UDP mode, + the option :option:`-u` takes precedence over this option, unless the option :option:`-u` + is set to zero, in which case the interval is computed from the :option:`-t` timeout interval + and the number of UDP retries. For UDP mode, the timeout can not be disabled, and will + be rounded up to 1 second in case if both :option:`-t` and :option:`-u` are set to zero. .. option:: -T diff --git a/bin/tests/system/nsupdate/ans4/ans.pl b/bin/tests/system/nsupdate/ans4/ans.pl index d4299c492b..30c792f1cf 100644 --- a/bin/tests/system/nsupdate/ans4/ans.pl +++ b/bin/tests/system/nsupdate/ans4/ans.pl @@ -31,6 +31,8 @@ if (!$localport) { $localport = 5300; } my $udpsock = IO::Socket::INET->new(LocalAddr => "$server_addr", LocalPort => $localport, Proto => "udp", Reuse => 1) or die "$!"; +my $tcpsock = IO::Socket::INET->new(LocalAddr => "$server_addr", + LocalPort => $localport, Proto => "tcp", Listen => 5, Reuse => 1) or die "$!"; print "listening on $server_addr:$localport.\n"; @@ -49,6 +51,7 @@ for (;;) { $rin = ''; vec($rin, fileno($udpsock), 1) = 1; + vec($rin, fileno($tcpsock), 1) = 1; select($rout = $rin, undef, undef, undef); @@ -56,5 +59,7 @@ for (;;) { printf "UDP request\n"; my $buf; $udpsock->recv($buf, 512); + } elsif (vec($rout, fileno($tcpsock), 1)) { + printf "TCP request\n"; } } diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 16b6535bdb..d57c357195 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -1276,15 +1276,97 @@ grep "records in zone (4) exceeds max-records (3)" ns3/named.run > /dev/null || n=$((n + 1)) ret=0 -echo_i "check whether valid addresses are used for primary failover ($n)" -$NSUPDATE -t 1 < nsupdate.out.test$n 2>&1 && ret=1 +echo_i "check whether valid addresses are used for primary failover (UDP with defaults) ($n)" +t1=$($PERL -e 'print time()') +$NSUPDATE < nsupdate.out.test$n 2>&1 && ret=1 server 10.53.0.4 ${PORT} zone unreachable. update add unreachable. 600 A 192.0.2.1 send END +t2=`$PERL -e 'print time()'` grep "; Communication with 10.53.0.4#${PORT} failed: timed out" nsupdate.out.test$n > /dev/null 2>&1 || ret=1 grep "not implemented" nsupdate.out.test$n > /dev/null 2>&1 && ret=1 +elapsed=$((t2 - t1)) +# Check that default timeout value is respected, there should be 4 tries with 3 seconds each. +test $elapsed -lt 12 && ret=1 +test $elapsed -gt 15 && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check whether valid addresses are used for primary failover (UDP with -u udptimeout) ($n)" +t1=$($PERL -e 'print time()') +$NSUPDATE -u 4 -r 1 < nsupdate.out.test$n 2>&1 && ret=1 +server 10.53.0.4 ${PORT} +zone unreachable. +update add unreachable. 600 A 192.0.2.1 +send +END +t2=`$PERL -e 'print time()'` +grep "; Communication with 10.53.0.4#${PORT} failed: timed out" nsupdate.out.test$n > /dev/null 2>&1 || ret=1 +grep "not implemented" nsupdate.out.test$n > /dev/null 2>&1 && ret=1 +elapsed=$((t2 - t1)) +# Check that given timeout value is respected, there should be 2 tries with 4 seconds each. +test $elapsed -lt 8 && ret=1 +test $elapsed -gt 12 && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check whether valid addresses are used for primary failover (UDP with -t timeout) ($n)" +t1=$($PERL -e 'print time()') +$NSUPDATE -u 0 -t 8 -r 1 < nsupdate.out.test$n 2>&1 && ret=1 +server 10.53.0.4 ${PORT} +zone unreachable. +update add unreachable. 600 A 192.0.2.1 +send +END +t2=`$PERL -e 'print time()'` +grep "; Communication with 10.53.0.4#${PORT} failed: timed out" nsupdate.out.test$n > /dev/null 2>&1 || ret=1 +grep "not implemented" nsupdate.out.test$n > /dev/null 2>&1 && ret=1 +elapsed=$((t2 - t1)) +# Check that given timeout value is respected, there should be 2 tries with 4 seconds each. +test $elapsed -lt 8 && ret=1 +test $elapsed -gt 12 && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check whether valid addresses are used for primary failover (UDP with -u udptimeout -t timeout) ($n)" +t1=$($PERL -e 'print time()') +$NSUPDATE -u 4 -t 30 -r 1 < nsupdate.out.test$n 2>&1 && ret=1 +server 10.53.0.4 ${PORT} +zone unreachable. +update add unreachable. 600 A 192.0.2.1 +send +END +t2=`$PERL -e 'print time()'` +grep "; Communication with 10.53.0.4#${PORT} failed: timed out" nsupdate.out.test$n > /dev/null 2>&1 || ret=1 +grep "not implemented" nsupdate.out.test$n > /dev/null 2>&1 && ret=1 +elapsed=$((t2 - t1)) +# Check that given timeout value is respected, there should be 2 tries with 4 seconds each, as -u takes precedence over -t. +test $elapsed -lt 8 && ret=1 +test $elapsed -gt 12 && ret=1 +[ $ret = 0 ] || { echo_i "failed"; status=1; } + +n=$((n + 1)) +ret=0 +echo_i "check whether valid addresses are used for primary failover (TCP with -t timeout) ($n)" +t1=$($PERL -e 'print time()') +$NSUPDATE -t 8 -v < nsupdate.out.test$n 2>&1 && ret=1 +server 10.53.0.4 ${PORT} +zone unreachable. +update add unreachable. 600 A 192.0.2.1 +send +END +t2=`$PERL -e 'print time()'` +grep "; Communication with 10.53.0.4#${PORT} failed: timed out" nsupdate.out.test$n > /dev/null 2>&1 || ret=1 +grep "not implemented" nsupdate.out.test$n > /dev/null 2>&1 && ret=1 +elapsed=$((t2 - t1)) +# Check that given timeout value is respected, there should be 1 try with 8 seconds. +test $elapsed -lt 8 && ret=1 +test $elapsed -gt 12 && ret=1 [ $ret = 0 ] || { echo_i "failed"; status=1; } n=$((n + 1)) diff --git a/lib/dns/request.c b/lib/dns/request.c index f78a57d7a3..3c1bda28db 100644 --- a/lib/dns/request.c +++ b/lib/dns/request.c @@ -421,6 +421,8 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, REQUIRE(cb != NULL); REQUIRE(requestp != NULL && *requestp == NULL); REQUIRE(timeout > 0); + REQUIRE(udpretries != UINT_MAX); + if (srcaddr != NULL) { REQUIRE(isc_sockaddr_pf(srcaddr) == isc_sockaddr_pf(destaddr)); } @@ -447,7 +449,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, request->cb = cb; request->arg = arg; request->result = ISC_R_FAILURE; - request->udpcount = udpretries; + request->udpcount = udpretries + 1; isc_buffer_usedregion(msgbuf, &r); if (r.length < DNS_MESSAGE_HEADERLEN || r.length > 65535) { @@ -460,7 +462,7 @@ dns_request_createraw(dns_requestmgr_t *requestmgr, isc_buffer_t *msgbuf, request->timeout = timeout * 1000; } else { if (udptimeout == 0) { - udptimeout = timeout / (udpretries + 1); + udptimeout = timeout / request->udpcount; } if (udptimeout == 0) { udptimeout = 1; @@ -569,6 +571,7 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, REQUIRE(cb != NULL); REQUIRE(requestp != NULL && *requestp == NULL); REQUIRE(timeout > 0); + REQUIRE(udpretries != UINT_MAX); mctx = requestmgr->mctx; @@ -598,9 +601,7 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, request->cb = cb; request->arg = arg; request->result = ISC_R_FAILURE; - request->udpcount = udpretries; - - request->udpcount = udpretries; + request->udpcount = udpretries + 1; if (key != NULL) { dns_tsigkey_attach(key, &request->tsigkey); @@ -615,8 +616,8 @@ dns_request_create(dns_requestmgr_t *requestmgr, dns_message_t *message, tcp = true; request->timeout = timeout * 1000; } else { - if (udptimeout == 0 && udpretries != 0) { - udptimeout = timeout / (udpretries + 1); + if (udptimeout == 0) { + udptimeout = timeout / request->udpcount; } if (udptimeout == 0) { udptimeout = 1; @@ -971,9 +972,13 @@ req_response(isc_result_t result, isc_region_t *region, void *arg) { req_log(ISC_LOG_DEBUG(3), "req_response: request %p: %s", request, isc_result_totext(result)); + REQUIRE(VALID_REQUEST(request)); + if (result == ISC_R_TIMEDOUT) { LOCK(&request->requestmgr->locks[request->hash]); - if (request->udpcount != 0) { + if (request->udpcount > 1 && + (request->flags & DNS_REQUEST_F_TCP) == 0) + { request->udpcount -= 1; dns_dispatch_resume(request->dispentry, request->timeout); @@ -988,8 +993,6 @@ req_response(isc_result_t result, isc_region_t *region, void *arg) { goto done; } - REQUIRE(VALID_REQUEST(request)); - LOCK(&request->requestmgr->locks[request->hash]); if (result != ISC_R_SUCCESS) {