Process another set of review comments from pieterlexis

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

diff --git a/pdns/recursordist/RECURSOR-MIB.in b/pdns/recursordist/RECURSOR-MIB.in
index 206faadd5bbf663572a3a88a7bd0dc4eb57a01c5..d3c4989f7c92dcba6821fa96177431c6b6efde68 100644 (file)
--- a/pdns/recursordist/RECURSOR-MIB.in
+++ b/pdns/recursordist/RECURSOR-MIB.in
@@ -15,23 +15,18 @@ IMPORTS
         FROM SNMPv2-CONF;
 
 rec MODULE-IDENTITY
-<<<<<<< HEAD
-    LAST-UPDATED "202505270000Z"
-=======
-    LAST-UPDATED "202504290000Z"
->>>>>>> 86b0c8c41 (Typos in comments and docs from Miod)
+
+    LAST-UPDATED "202509100000Z"
     ORGANIZATION "PowerDNS BV"
     CONTACT-INFO "support@powerdns.com"
     DESCRIPTION
        "This MIB module describes information gathered through PowerDNS Recursor."
 
-<<<<<<< HEAD
+    REVISION "202509100000Z"
+    DESCRIPTION "Added metrics related to cookies"
+
     REVISION "202505270000Z"
     DESCRIPTION "Added metric for missing ECS in reply"
-=======
-    REVISION "202504290000Z"
-    DESCRIPTION "Added metrics related to cookies"
->>>>>>> 86b0c8c41 (Typos in comments and docs from Miod)
 
     REVISION "202408280000Z"
     DESCRIPTION "Added metric for too many incoming TCP connections"

diff --git a/pdns/recursordist/RECURSOR-MIB.txt b/pdns/recursordist/RECURSOR-MIB.txt
index c51f6ab5956072fbc95d6a0715a3c5e4bb5da50b..e3d6cd31f9cfb0aab0cc1e3d731ffddcd1d3b7b6 100644 (file)
--- a/pdns/recursordist/RECURSOR-MIB.txt
+++ b/pdns/recursordist/RECURSOR-MIB.txt
@@ -15,23 +15,18 @@ IMPORTS
         FROM SNMPv2-CONF;
 
 rec MODULE-IDENTITY
-<<<<<<< HEAD
-    LAST-UPDATED "202505270000Z"
-=======
-    LAST-UPDATED "202504290000Z"
->>>>>>> 86b0c8c41 (Typos in comments and docs from Miod)
+
+    LAST-UPDATED "202509100000Z"
     ORGANIZATION "PowerDNS BV"
     CONTACT-INFO "support@powerdns.com"
     DESCRIPTION
        "This MIB module describes information gathered through PowerDNS Recursor."
 
-<<<<<<< HEAD
+    REVISION "202509100000Z"
+    DESCRIPTION "Added metrics related to cookies"
+
     REVISION "202505270000Z"
     DESCRIPTION "Added metric for missing ECS in reply"
-=======
-    REVISION "202504290000Z"
-    DESCRIPTION "Added metrics related to cookies"
->>>>>>> 86b0c8c41 (Typos in comments and docs from Miod)
 
     REVISION "202408280000Z"
     DESCRIPTION "Added metric for too many incoming TCP connections"

diff --git a/pdns/recursordist/lwres.cc b/pdns/recursordist/lwres.cc
index c9d03f772f2cd9518bcca327f0cf872a6fb43ab2..68d5032ff92074c7d51d9501ac299668f925f089 100644 (file)
--- a/pdns/recursordist/lwres.cc
+++ b/pdns/recursordist/lwres.cc
@@ -486,8 +486,7 @@ static void addPadding(const DNSPacketWriter& pw, size_t bufsize, DNSPacketWrite
 static void outgoingCookie(const OptLog& log, const ComboAddress& address, const timeval& now, DNSPacketWriter::optvect_t& opts, std::optional<EDNSCookiesOpt>& cookieSentOut, std::optional<ComboAddress>& addressToBindTo)
 {
   auto lock = s_cookiestore.lock();
-  auto found = lock->find(address);
-  if (found != lock->end()) {
+  if (auto found = lock->find(address); found != lock->end()) {
     switch (found->getSupport()) {
     case CookieEntry::Support::Supported:
     case CookieEntry::Support::Probing:
@@ -501,18 +500,17 @@ static void outgoingCookie(const OptLog& log, const ComboAddress& address, const
       VLOG(log, "Server " << address.toString() << " does not support cookies" << endl);
       break;
     }
+    return;
   }
-  else {
-    // Server not in table, it's either new or was purged
-    CookieEntry entry;
-    entry.d_address = address;
-    entry.d_cookie.makeClientCookie();
-    cookieSentOut = entry.d_cookie;
-    entry.setSupport(CookieEntry::Support::Probing, now.tv_sec);
-    lock->emplace(entry);
-    opts.emplace_back(EDNSOptionCode::COOKIE, cookieSentOut->makeOptString());
-    VLOG(log, "Sending new client cookie info to " << address.toString() << ": " << entry.d_cookie.toDisplayString() << endl);
-  }
+  // Server not in table, it's either new or was purged
+  CookieEntry entry;
+  entry.d_address = address;
+  entry.d_cookie.makeClientCookie();
+  cookieSentOut = entry.d_cookie;
+  entry.setSupport(CookieEntry::Support::Probing, now.tv_sec);
+  lock->emplace(entry);
+  opts.emplace_back(EDNSOptionCode::COOKIE, cookieSentOut->makeOptString());
+  VLOG(log, "Sending new client cookie info to " << address.toString() << ": " << entry.d_cookie.toDisplayString() << endl);
 }
 
 static std::pair<bool, LWResult::Result> incomingCookie(const OptLog& log, const ComboAddress& address, const ComboAddress& localip, const timeval& now, const std::optional<EDNSCookiesOpt>& cookieSentOut, const EDNSOpts& edo, bool doTCP, LWResult& lwr, bool& cookieFoundInReply)
@@ -674,7 +672,7 @@ static LWResult::Result asyncresolve(const OptLog& log, const ComboAddress& addr
     }
     catch (const PDNSException& e) {
       if (addressToBindTo) {
-        // Cookie info already has been added to packet, so we must retry from a higher level
+        // Cookie info already has been added to packet, so we must retry from a higher level: SyncRes::asyncresolveWrapper
         auto lock = s_cookiestore.lock();
         lock->erase(address);
         return LWResult::Result::BindError;

diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
index f638e0f25f54d0d748e20712677b16c3b17b7777..611ca825013e8e127754008ce7f5694f02be7939 100644 (file)
--- a/pdns/recursordist/rec-main.cc
+++ b/pdns/recursordist/rec-main.cc
@@ -2558,7 +2558,7 @@ static void houseKeepingWork(Logr::log_t log)
 
     static PeriodicTask pruneCookiesTask{"pruneCookiesTask", 30};
     pruneCookiesTask.runIfDue(now, [now]() {
-      pruneCookies(now.tv_sec - 1800);
+      pruneCookies(now.tv_sec - 3000);
     });
 
     // By default, refresh at 80% of max-cache-ttl with a minimum period of 10s

diff --git a/pdns/recursordist/rec-tcpout.cc b/pdns/recursordist/rec-tcpout.cc
index 2abfb78c04bdfc2d5b3b8a809b222ad6a38fbea9..d1edba0bec0d3751b35a43bc50e8a87040c6c644 100644 (file)
--- a/pdns/recursordist/rec-tcpout.cc
+++ b/pdns/recursordist/rec-tcpout.cc
@@ -51,26 +51,26 @@ void TCPOutConnectionManager::cleanup(const struct timeval& now)
   }
 }
 
-void TCPOutConnectionManager::store(const struct timeval& now, const endpoints_t& pair, Connection&& connection)
+void TCPOutConnectionManager::store(const struct timeval& now, const endpoints_t& endpoints, Connection&& connection)
 {
   ++connection.d_numqueries;
   if (s_maxQueries > 0 && connection.d_numqueries >= s_maxQueries) {
     return;
   }
 
-  if (d_idle_connections.size() >= s_maxIdlePerThread || d_idle_connections.count(pair) >= s_maxIdlePerAuth) {
+  if (d_idle_connections.size() >= s_maxIdlePerThread || d_idle_connections.count(endpoints) >= s_maxIdlePerAuth) {
     cleanup(now);
   }
 
   if (d_idle_connections.size() >= s_maxIdlePerThread) {
     return;
   }
-  if (d_idle_connections.count(pair) >= s_maxIdlePerAuth) {
+  if (d_idle_connections.count(endpoints) >= s_maxIdlePerAuth) {
     return;
   }
 
   gettimeofday(&connection.d_last_used, nullptr);
-  d_idle_connections.emplace(pair, std::move(connection));
+  d_idle_connections.emplace(endpoints, std::move(connection));
 }
 
 TCPOutConnectionManager::Connection TCPOutConnectionManager::get(const endpoints_t& pair)

diff --git a/pdns/recursordist/rec-tcpout.hh b/pdns/recursordist/rec-tcpout.hh
index 26d973b3713a1324981b5454cb5b46e4e2557110..e52a20b94387e8ede62a3c6d74c6a5feb6d1075b 100644 (file)
--- a/pdns/recursordist/rec-tcpout.hh
+++ b/pdns/recursordist/rec-tcpout.hh
@@ -55,7 +55,7 @@ public:
 
   using endpoints_t = std::pair<ComboAddress, std::optional<ComboAddress>>;
 
-  void store(const struct timeval& now, const endpoints_t& pair, Connection&& connection);
+  void store(const struct timeval& now, const endpoints_t& endpoints, Connection&& connection);
   Connection get(const endpoints_t& pair);
   void cleanup(const struct timeval& now);