From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Tue, 5 Dec 2023 14:42:15 +0000 (+0100)
Subject: rec: lower default max-qperq limit.
X-Git-Tag: auth-4.9.0-alpha1~39^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=70fe557da495020d5a640d9d9d3015cdc9d64f2f;p=thirdparty%2Fpdns.git

rec: lower default max-qperq limit.

The better zone-cut algorithm allows this. In my test of various
cases (all top 1 million names and various reverse resolving cases
mentioned in #8646 (with cold cache) I never saw a qperq higher
than 37 (qname minimization enabled and DNSSEC validation as well).
---

diff --git a/pdns/recursordist/settings/table.py b/pdns/recursordist/settings/table.py
index 75158bc495..37fae870b6 100644
--- a/pdns/recursordist/settings/table.py
+++ b/pdns/recursordist/settings/table.py
@@ -1498,14 +1498,13 @@ Maximum number of Packet Cache entries. Sharded and shared by all threads since
         'name' : 'max_qperq',
         'section' : 'outgoing',
         'type' : LType.Uint64,
-        'default' : '60',
+        'default' : '50',
         'help' : 'Maximum outgoing queries per query',
         'doc' : '''
 The maximum number of outgoing queries that will be sent out during the resolution of a single client query.
-This is used to limit endlessly chasing CNAME redirections.
-If qname-minimization is enabled, the number will be forced to be 100
-at a minimum to allow for the extra queries qname-minimization generates when the cache is empty.
+This is used to avoid cycles resolving names.
  ''',
+        'versionchanged': ('5.0.0', 'The default used to be 60, with an extra allowance if qname minimization was enabled. Having better algorithms allows for a lower default limit.'),
     },
     {
         'name' : 'max_ns_address_qperq',
diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
index 1515067efb..d9e8ee79f6 100644
--- a/pdns/recursordist/syncres.cc
+++ b/pdns/recursordist/syncres.cc
@@ -3470,17 +3470,8 @@ vector<ComboAddress> SyncRes::retrieveAddressesForNS(const std::string& prefix,
 
 void SyncRes::checkMaxQperQ(const DNSName& qname) const
 {
-  auto bound = s_maxqperq;
-  if (d_qNameMinimization) {
-    // With an empty cache, a rev ipv6 query with dnssec enabled takes
-    // almost 100 queries. Default maxqperq is 60
-    // Note: This no longer seems to be true. The examples taken from #8646 take now way less
-    // queries. The main reason seems to be a much better zone cut determination.
-    bound = std::max(100U, bound);
-  }
-
-  if (d_outqueries + d_throttledqueries > bound) {
-    throw ImmediateServFailException("more than " + std::to_string(bound) + " (adjusted max-qperq) queries sent or throttled while resolving " + qname.toLogString());
+  if (d_outqueries + d_throttledqueries > s_maxqperq) {
+    throw ImmediateServFailException("more than " + std::to_string(s_maxqperq) + " (max-qperq) queries sent or throttled while resolving " + qname.toLogString());
   }
 }