Use OpenBSD's reliable local UDP port randomization and retry on

sendmsg() EAGAIN. The latter can happen on non-blocking UDP sockets
on OpenBSD.

diff --git a/pdns/iputils.cc b/pdns/iputils.cc
index 64f2138b6c673c5ec8ed043d5aefa105f5bf3756..17aa12c74e9ace4fa1bd75713a71f9efc0824616 100644 (file)
--- a/pdns/iputils.cc
+++ b/pdns/iputils.cc
@@ -238,6 +238,27 @@ bool IsAnyAddress(const ComboAddress& addr)
   
   return false;
 }
+int sendOnNBSocket(int fd, const struct msghdr *msgh)
+{
+  int sendErr = 0;
+#ifdef __OpenBSD__
+  for (int i = 0; i < 10; i++) {
+    if (sendmsg(fd, msgh, 0) != -1) {
+      sendErr = 0;
+      break;
+    }
+    sendErr = errno;
+    if (sendErr != EAGAIN) {
+      break;
+    }
+  }
+#else
+  if (sendmsg(fd, msgh, 0) == -1) {
+    sendErr = errno;
+  }
+#endif
+  return sendErr;
+}
 
 ssize_t sendfromto(int sock, const char* data, size_t len, int flags, const ComboAddress& from, const ComboAddress& to)
 {

diff --git a/pdns/iputils.hh b/pdns/iputils.hh
index 355f61b672b4b2ac56313ab2f342fa7740bdf606..3552a8e26fc1806f54c7aab9b01ed8810a388bd7 100644 (file)
--- a/pdns/iputils.hh
+++ b/pdns/iputils.hh
@@ -1433,6 +1433,7 @@ bool IsAnyAddress(const ComboAddress& addr);
 bool HarvestDestinationAddress(const struct msghdr* msgh, ComboAddress* destination);
 bool HarvestTimestamp(struct msghdr* msgh, struct timeval* tv);
 void fillMSGHdr(struct msghdr* msgh, struct iovec* iov, cmsgbuf_aligned* cbuf, size_t cbufsize, char* data, size_t datalen, ComboAddress* addr);
+int sendOnNBSocket(int fd, const struct msghdr *msgh);
 ssize_t sendfromto(int sock, const char* data, size_t len, int flags, const ComboAddress& from, const ComboAddress& to);
 size_t sendMsgWithOptions(int fd, const char* buffer, size_t len, const ComboAddress* dest, const ComboAddress* local, unsigned int localItf, int flags);
 

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 684db957f314e3d73d66aa1504f4fbb1234e14ed..1ed48cb1f7f4ebaa99ecd9c2f9f4c5be281eafcd 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -194,7 +194,7 @@ typedef vector<int> tcpListenSockets_t;
 typedef map<int, ComboAddress> listenSocketsAddresses_t; // is shared across all threads right now
 
 static listenSocketsAddresses_t g_listenSocketsAddresses; // is shared across all threads right now
-static set<int> g_fromtosockets; // listen sockets that use 'sendfromto()' mechanism
+static set<int> g_fromtosockets; // listen sockets that use 'sendfromto()' mechanism (without actually using sendfromto())
 static AtomicCounter counter;
 static std::shared_ptr<SyncRes::domainmap_t> g_initialDomainMap; // new threads needs this to be setup
 static std::shared_ptr<NetmaskGroup> g_initialAllowFrom; // new thread needs to be setup with this
@@ -578,50 +578,50 @@ private:
   // returns -1 for errors which might go away, throws for ones that won't
   static int makeClientSocket(int family)
   {
-    int ret=socket(family, SOCK_DGRAM, 0 ); // turns out that setting CLO_EXEC and NONBLOCK from here is not a performance win on Linux (oddly enough)
+    int ret = socket(family, SOCK_DGRAM, 0); // turns out that setting CLO_EXEC and NONBLOCK from here is not a performance win on Linux (oddly enough)
 
-    if(ret < 0 && errno==EMFILE) // this is not a catastrophic error
+    if (ret < 0 && errno ==  EMFILE) { // this is not a catastrophic error
       return ret;
+    }
+    if (ret < 0) {
+      throw PDNSException("Making a socket for resolver (family = " + std::to_string(family) + "): " + stringerror());
+    }
 
-    if(ret<0)
-      throw PDNSException("Making a socket for resolver (family = "+std::to_string(family)+"): "+stringerror());
-
-    //    setCloseOnExec(ret); // we're not going to exec
-
-    int tries=10;
+#ifndef __OpenBSD__
+    int tries = 10;
+#else
+    int tries = 2; // hit the reliable kernel random case for OpenBSD, using sysctl net.inet.udp.baddynamic to exclude ports
+#endif
     ComboAddress sin;
-    while(--tries) {
-      uint16_t port;
+    while (--tries) {
+      in_port_t port;
 
-      if(tries==1)  // fall back to kernel 'random'
+      if (tries == 1) {  // fall back to kernel 'random'
         port = 0;
-      else {
+      } else {
         do {
           port = s_minUdpSourcePort + dns_random(s_maxUdpSourcePort - s_minUdpSourcePort + 1);
-        }
-        while (s_avoidUdpSourcePorts.count(port));
+        } while (s_avoidUdpSourcePorts.count(port));
       }
 
-      sin=pdns::getQueryLocalAddress(family, port); // does htons for us
-
-      if (::bind(ret, (struct sockaddr *)&sin, sin.getSocklen()) >= 0)
+      sin = pdns::getQueryLocalAddress(family, port); // does htons for us
+      if (::bind(ret, reinterpret_cast<struct sockaddr*>(&sin), sin.getSocklen()) >= 0)
         break;
     }
 
-    if(!tries) {
+    if (!tries) {
       closesocket(ret);
-      throw PDNSException("Resolver binding to local query client socket on "+sin.toString()+": "+stringerror());
+      throw PDNSException("Resolver binding to local query client socket on " + sin.toString() + ": " + stringerror());
     }
 
     try {
       setReceiveSocketErrors(ret, family);
       setNonBlocking(ret);
     }
-    catch(...) {
+    catch (...) {
       closesocket(ret);
       throw;
     }
-
     return ret;
   }
 };
@@ -1900,10 +1900,10 @@ static void startDoResolve(void *p)
       if(g_fromtosockets.count(dc->d_socket)) {
         addCMsgSrcAddr(&msgh, &cbuf, &dc->d_local, 0);
       }
-      if(sendmsg(dc->d_socket, &msgh, 0) < 0 && g_logCommonErrors) {
-        int err = errno;
+      int sendErr = sendOnNBSocket(dc->d_socket, &msgh);
+      if (sendErr && g_logCommonErrors) {
         g_log << Logger::Warning << "Sending UDP reply to client " << dc->getRemote() << " failed with: "
-              << strerror(err) << endl;
+              << strerror(sendErr) << endl;
       }
 
       if(variableAnswer || sr.wasVariable()) {
@@ -2717,11 +2717,11 @@ static string* doProcessUDPQuestion(const std::string& question, const ComboAddr
       if(g_fromtosockets.count(fd)) {
         addCMsgSrcAddr(&msgh, &cbuf, &destaddr, 0);
       }
-      if(sendmsg(fd, &msgh, 0) < 0 && g_logCommonErrors) {
-        int err = errno;
+      int sendErr = sendOnNBSocket(fd, &msgh);
+      if (sendErr && g_logCommonErrors) {
         g_log << Logger::Warning << "Sending UDP reply to client " << source.toStringWithPort()
               << (source != fromaddr ? " (via " + fromaddr.toStringWithPort() + ")" : "") << " failed with: "
-              << strerror(err) << endl;
+              << strerror(sendErr) << endl;
       }
       if(response.length() >= sizeof(struct dnsheader)) {
         struct dnsheader tmpdh;