From: Otto Moerbeek Date: Tue, 1 Feb 2022 14:45:54 +0000 (+0100) Subject: If we have to resolve a nameserver name, submit a AAAA query for the X-Git-Tag: rec-4.7.0-alpha1~10^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d205e5d8d1315b09b1de633749c5907604f0d98a;p=thirdparty%2Fpdns.git If we have to resolve a nameserver name, submit a AAAA query for the same name asynchronously if it's not in the negcache. --- diff --git a/pdns/recursordist/rec-taskqueue.cc b/pdns/recursordist/rec-taskqueue.cc index cc1e71f265..b9b8a61b68 100644 --- a/pdns/recursordist/rec-taskqueue.cc +++ b/pdns/recursordist/rec-taskqueue.cc @@ -26,47 +26,71 @@ #include "stat_t.hh" #include "syncres.hh" -static LockGuarded s_taskQueue; -static pdns::stat_t s_almost_expired_tasks_pushed; -static pdns::stat_t s_almost_expired_tasks_run; -static pdns::stat_t s_almost_expired_tasks_exceptions; +struct Queues +{ + pdns::TaskQueue queue; + std::set running; +}; +static LockGuarded s_taskQueue; + +struct taskstats +{ + pdns::stat_t pushed; + pdns::stat_t run; + pdns::stat_t exceptions; +}; -static void resolve(const struct timeval& now, bool logErrors, const pdns::ResolveTask& task) +static struct taskstats s_almost_expired_tasks; +static struct taskstats s_resolve_tasks; + +static void resolve(const struct timeval& now, bool logErrors, const pdns::ResolveTask& task) noexcept { const string msg = "Exception while running a background ResolveTask"; SyncRes sr(now); vector ret; sr.setRefreshAlmostExpired(task.d_refreshMode); + bool ex = true; try { g_log << Logger::Debug << "TaskQueue: resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << endl; int res = sr.beginResolve(task.d_qname, QType(task.d_qtype), QClass::IN, ret); - ++s_almost_expired_tasks_run; + ex = false; g_log << Logger::Debug << "TaskQueue: DONE resolving " << task.d_qname.toString() << '|' << QType(task.d_qtype).toString() << ": " << res << endl; } catch (const std::exception& e) { - ++s_almost_expired_tasks_exceptions; g_log << Logger::Error << msg << ": " << e.what() << endl; } catch (const PDNSException& e) { - ++s_almost_expired_tasks_exceptions; g_log << Logger::Notice << msg << ": " << e.reason << endl; } catch (const ImmediateServFailException& e) { - ++s_almost_expired_tasks_exceptions; if (logErrors) { g_log << Logger::Notice << msg << ": " << e.reason << endl; } } catch (const PolicyHitException& e) { - ++s_almost_expired_tasks_exceptions; if (logErrors) { g_log << Logger::Notice << msg << ": PolicyHit" << endl; } } catch (...) { - ++s_almost_expired_tasks_exceptions; g_log << Logger::Error << msg << endl; } + if (ex) { + if (task.d_refreshMode) { + ++s_almost_expired_tasks.exceptions; + } + else { + ++s_resolve_tasks.exceptions; + } + } + else { + if (task.d_refreshMode) { + ++s_almost_expired_tasks.run; + } + else { + ++s_resolve_tasks.run; + } + } } void runTaskOnce(bool logErrors) @@ -74,50 +98,82 @@ void runTaskOnce(bool logErrors) pdns::ResolveTask task; { auto lock = s_taskQueue.lock(); - if (lock->empty()) { + if (lock->queue.empty()) { return; } - task = lock->pop(); + task = lock->queue.pop(); + lock->running.insert(task); } bool expired = task.run(logErrors); + s_taskQueue.lock()->running.erase(task); if (expired) { - s_taskQueue.lock()->incExpired(); + s_taskQueue.lock()->queue.incExpired(); } } void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline) { - ++s_almost_expired_tasks_pushed; pdns::ResolveTask task{qname, qtype, deadline, true, resolve}; - s_taskQueue.lock()->push(std::move(task)); + auto lock = s_taskQueue.lock(); + bool running = lock->running.count(task) > 0; + if (!running) { + ++s_almost_expired_tasks.pushed; + lock->queue.push(std::move(task)); + } +} + +void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline) +{ + pdns::ResolveTask task{qname, qtype, deadline, false, resolve}; + auto lock = s_taskQueue.lock(); + bool running = lock->running.count(task) > 0; + if (!running) { + ++s_resolve_tasks.pushed; + lock->queue.push(std::move(task)); + } } uint64_t getTaskPushes() { - return s_taskQueue.lock()->getPushes(); + return s_taskQueue.lock()->queue.getPushes(); } uint64_t getTaskExpired() { - return s_taskQueue.lock()->getExpired(); + return s_taskQueue.lock()->queue.getExpired(); } uint64_t getTaskSize() { - return s_taskQueue.lock()->size(); + return s_taskQueue.lock()->queue.size(); } uint64_t getAlmostExpiredTasksPushed() { - return s_almost_expired_tasks_pushed; + return s_almost_expired_tasks.pushed; } uint64_t getAlmostExpiredTasksRun() { - return s_almost_expired_tasks_run; + return s_almost_expired_tasks.run; } uint64_t getAlmostExpiredTaskExceptions() { - return s_almost_expired_tasks_exceptions; + return s_almost_expired_tasks.exceptions; +} + +uint64_t getResolveTasksPushed() +{ + return s_almost_expired_tasks.pushed; +} + +uint64_t getResolveTasksRun() +{ + return s_almost_expired_tasks.run; +} + +uint64_t getResolveTaskExceptions() +{ + return s_almost_expired_tasks.exceptions; } diff --git a/pdns/recursordist/rec-taskqueue.hh b/pdns/recursordist/rec-taskqueue.hh index 95e734b68d..fe1fea6bfc 100644 --- a/pdns/recursordist/rec-taskqueue.hh +++ b/pdns/recursordist/rec-taskqueue.hh @@ -25,12 +25,18 @@ void runTaskOnce(bool logErrors); void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline); +void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline); // General task stats uint64_t getTaskPushes(); uint64_t getTaskExpired(); uint64_t getTaskSize(); +// Resolve specific stats +uint64_t getResolveTasksPushed(); +uint64_t getResolveTasksRun(); +uint64_t getResolveTaskExceptions(); + // Almost expired specific stats uint64_t getAlmostExpiredTasksPushed(); uint64_t getAlmostExpiredTasksRun(); diff --git a/pdns/recursordist/taskqueue.hh b/pdns/recursordist/taskqueue.hh index 9db1d42218..624d87d1d5 100644 --- a/pdns/recursordist/taskqueue.hh +++ b/pdns/recursordist/taskqueue.hh @@ -34,10 +34,9 @@ #include "dnsname.hh" #include "qtype.hh" -using namespace ::boost::multi_index; - namespace pdns { +using namespace ::boost::multi_index; // ATM we have one task type, if we get more, the unique key in the index needs to be adapted struct ResolveTask @@ -48,6 +47,10 @@ struct ResolveTask bool d_refreshMode; // Whether to run this task in regular mode (false) or in the mode that refreshes almost expired tasks std::function d_func; + bool operator<(const ResolveTask& a) const + { + return tie(d_qname, d_qtype, d_refreshMode) < tie(d_qname, d_qtype, d_refreshMode); + } bool run(bool logErrors); }; @@ -86,9 +89,11 @@ private: struct HashTag { }; + struct SequencedTag { }; + typedef multi_index_container< ResolveTask, indexed_by< diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index fef6a3bac4..6cc8900372 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -558,3 +558,7 @@ void pushAlmostExpiredTask(const DNSName& qname, uint16_t qtype, time_t deadline { g_test_tasks.push({qname, qtype, deadline, true, nullptr}); } + +void pushResolveTask(const DNSName& qname, uint16_t qtype, time_t deadline) +{ +} diff --git a/pdns/recursordist/test-syncres_cc2.cc b/pdns/recursordist/test-syncres_cc2.cc index 8e46deb05f..15d53bf7fe 100644 --- a/pdns/recursordist/test-syncres_cc2.cc +++ b/pdns/recursordist/test-syncres_cc2.cc @@ -311,6 +311,10 @@ BOOST_AUTO_TEST_CASE(test_glueless_referral_with_non_resolving) BOOST_REQUIRE_EQUAL(ret.size(), 0U); BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 2U); + // Originally empty NoData results where not cached, now they are + BOOST_CHECK_EQUAL(g_negCache->size(), 4U); + g_negCache->clear(); + // Again, should not change anything res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); BOOST_CHECK_EQUAL(res, RCode::ServFail); @@ -320,6 +324,10 @@ BOOST_AUTO_TEST_CASE(test_glueless_referral_with_non_resolving) BOOST_CHECK_EQUAL(SyncRes::getNonResolvingNSSize(), 2U); + // Originally empty NoData results where not cached, now they are + BOOST_CHECK_EQUAL(g_negCache->size(), 4U); + g_negCache->clear(); + // Again, but now things should start working because of the queryCounter getting high enough // and one entry remains in the non-resolving cache res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret); diff --git a/pdns/recursordist/test-syncres_cc3.cc b/pdns/recursordist/test-syncres_cc3.cc index bcd6ddbb23..29f881d809 100644 --- a/pdns/recursordist/test-syncres_cc3.cc +++ b/pdns/recursordist/test-syncres_cc3.cc @@ -1210,8 +1210,8 @@ BOOST_AUTO_TEST_CASE(test_forward_zone_recurse_rd_dnssec_nodata_bogus) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusMissingNegativeIndication); BOOST_REQUIRE_EQUAL(ret.size(), 0U); - /* we don't store empty results */ - BOOST_CHECK_EQUAL(queriesCount, 4U); + /* we don store empty results */ + BOOST_CHECK_EQUAL(queriesCount, 3U); } BOOST_AUTO_TEST_CASE(test_auth_zone_oob) diff --git a/pdns/recursordist/test-syncres_cc7.cc b/pdns/recursordist/test-syncres_cc7.cc index 7d4484175f..16cb4e32ad 100644 --- a/pdns/recursordist/test-syncres_cc7.cc +++ b/pdns/recursordist/test-syncres_cc7.cc @@ -1416,8 +1416,8 @@ BOOST_AUTO_TEST_CASE(test_dnssec_bogus_nodata) BOOST_CHECK_EQUAL(res, RCode::NoError); BOOST_CHECK_EQUAL(sr->getValidationState(), vState::BogusMissingNegativeIndication); BOOST_REQUIRE_EQUAL(ret.size(), 0U); - /* we don't store empty results */ - BOOST_CHECK_EQUAL(queriesCount, 4U); + /* we do store empty results */ + BOOST_CHECK_EQUAL(queriesCount, 3U); } BOOST_AUTO_TEST_CASE(test_dnssec_bogus_nxdomain) diff --git a/pdns/syncres.cc b/pdns/syncres.cc index b61f183029..bb39a50822 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -36,6 +36,7 @@ #include "syncres.hh" #include "dnsseckeeper.hh" #include "validate-recursor.hh" +#include "rec-taskqueue.hh" thread_local SyncRes::ThreadLocalStorage SyncRes::t_sstorage; thread_local std::unique_ptr t_timeouts; @@ -1136,13 +1137,14 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, bool oldRequireAuthData = d_requireAuthData; bool oldValidationRequested = d_DNSSECValidationRequested; bool oldFollowCNAME = d_followCNAME; + bool seenV6 = false; const unsigned int startqueries = d_outqueries; d_requireAuthData = false; d_DNSSECValidationRequested = false; d_followCNAME = true; try { - // First look for both A and AAAA in the cache and be satisfied if we find anything + // First look for both A and AAAA in the cache res_t cset; if (s_doIPv4 && g_recCache->get(d_now.tv_sec, qname, QType::A, false, &cset, d_cacheRemote, d_refresh, d_routingTag) > 0) { for (const auto &i : cset) { @@ -1154,14 +1156,16 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, if (s_doIPv6 && g_recCache->get(d_now.tv_sec, qname, QType::AAAA, false, &cset, d_cacheRemote, d_refresh, d_routingTag) > 0) { for (const auto &i : cset) { if (auto rec = getRR(i)) { + seenV6 = true; ret.push_back(rec->getCA(53)); } } } if (ret.empty()) { - // Nothing in the cache... + // Neither A nor AAAA in the cache... vState newState = vState::Indeterminate; cset.clear(); + // Go out to get A's if (s_doIPv4 && doResolve(qname, QType::A, cset, depth+1, beenthere, newState) == 0) { // this consults cache, OR goes out for (auto const &i : cset) { if (i.d_type == QType::A) { @@ -1173,25 +1177,26 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, } if (s_doIPv6) { // s_doIPv6 **IMPLIES** pdns::isQueryLocalAddressFamilyEnabled(AF_INET6) returned true if (ret.empty()) { - // We only go out to find IPv6 records if we did not find any IPv4 ones. - // Once IPv6 adoption matters, this needs to be revisited + // We only go out imediately to find IPv6 records if we did not find any IPv4 ones. newState = vState::Indeterminate; cset.clear(); if (doResolve(qname, QType::AAAA, cset, depth+1, beenthere, newState) == 0) { // this consults cache, OR goes out for (const auto &i : cset) { if (i.d_type == QType::AAAA) { - if (auto rec = getRR(i)) + if (auto rec = getRR(i)) { + seenV6 = true; ret.push_back(rec->getCA(53)); + } } } } } else { - // We have some IPv4 records, don't bother with going out to get IPv6, but do consult the cache, we might have - // encountered some IPv6 glue + // We have some IPv4 records, consult the cache, we might have encountered some IPv6 glue cset.clear(); if (g_recCache->get(d_now.tv_sec, qname, QType::AAAA, false, &cset, d_cacheRemote, d_refresh, d_routingTag) > 0) { for (const auto &i : cset) { if (auto rec = getRR(i)) { + seenV6 = true; ret.push_back(rec->getCA(53)); } } @@ -1199,6 +1204,15 @@ vector SyncRes::getAddrs(const DNSName &qname, unsigned int depth, } } } + if (s_doIPv6 && !seenV6 && !cacheOnly) { + // No IPv6 NS records in cache, chek negcache and submit async task if negache does not have the data + // so that the next time the cache or the negcache will have data + NegCache::NegCacheEntry ne; + bool inNegCache = g_negCache->get(qname, QType::AAAA, d_now, ne, true); + if (!inNegCache) { + pushResolveTask(qname, QType::AAAA, d_now.tv_sec + 60); + } + } } catch (const PolicyHitException& e) { /* we ignore a policy hit while trying to retrieve the addresses @@ -3628,6 +3642,18 @@ bool SyncRes::processRecords(const std::string& prefix, const DNSName& qname, co uint32_t dnameTTL = 0; bool referralOnDS = false; + if (lwr.d_rcode == 0 && qtype.getCode() != 0 && lwr.d_records.size() == 0) { + // NODATA and no SOA, put that into the negcache for a while + NegCache::NegCacheEntry ne; + ne.d_auth = auth; + ne.d_name = qname; + ne.d_qtype = qtype; + ne.d_ttd = d_now.tv_sec + std::max(s_minimumTTL, 60U); + ne.d_validationState = vState::BogusMissingNegativeIndication; + LOG(prefix<add(ne); + } + for (auto& rec : lwr.d_records) { if (rec.d_type != QType::OPT && rec.d_class != QClass::IN) { continue;