dnsdist: Support "no server available" result from Lua FFI LB policies

diff --git a/pdns/dnsdistdist/dnsdist-lbpolicies.cc b/pdns/dnsdistdist/dnsdist-lbpolicies.cc
index 17f2f5238e4e15698cbbeab341bb754c62f5d014..b4e3532619acba185df68dec0242143f25803425 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lbpolicies.cc
+++ b/pdns/dnsdistdist/dnsdist-lbpolicies.cc
@@ -379,6 +379,11 @@ std::shared_ptr<DownstreamState> ServerPolicy::getSelectedBackend(const ServerPo
         selected = policy(&serversList, &dnsq);
       }
 
+      if (selected >= servers.size()) {
+        /* invalid offset, meaning that there is no server available */
+        return {};
+      }
+
       selectedBackend = servers.at(selected).second;
     }
   }

diff --git a/pdns/dnsdistdist/docs/guides/serverselection.rst b/pdns/dnsdistdist/docs/guides/serverselection.rst
index 90dd2face0fb48f0b541e0bc2ccd0bb792c6c6a9..1d9458621d4a2ba8dc6efaa513bc0606f44e80e0 100644 (file)
--- a/pdns/dnsdistdist/docs/guides/serverselection.rst
+++ b/pdns/dnsdistdist/docs/guides/serverselection.rst
@@ -110,6 +110,25 @@ Or::
 
   setServerPolicyLua("splitsetup", splitSetup)
 
+A faster, FFI version is also available since 1.5.0:
+
+.. code-block:: lua
+
+  local ffi = require("ffi")
+  local C = ffi.C
+
+  local counter = 0
+  function luaffiroundrobin(servers_list, dq)
+    counter = counter + 1
+    return (counter % tonumber(C.dnsdist_ffi_servers_list_get_count(servers_list)))
+  end
+  setServerPolicyLuaFFI("luaffiroundrobin", luaffiroundrobin)
+
+Note that this version returns the index (starting at 0) of the server to select,
+instead of returning the server itself. It was initially not possible to indicate
+that all servers were unavailable from these policies, but since 1.9.2 returning
+a value equal or greater than the number of servers will be interpreted as such.
+
 For performance reasons, 1.6.0 introduced per-thread Lua FFI policies that are run in a lock-free per-thread Lua context instead of the global one.
 This reduces contention between threads at the cost of preventing sharing data between threads for these policies. Since the policy needs to be recompiled
 in the context of each thread instead of the global one, Lua code that returns a function should be passed to the function as a string instead of directly
@@ -214,6 +233,9 @@ Functions
 
   .. versionadded:: 1.5.0
 
+  .. versionchanged:: 1.9.2
+    Returning a value equal or greater than the number of servers will be interpreted as all servers being unavailable.
+
   Set server selection policy to one named ``name`` and provided by the FFI function ``function``.
 
   :param string name: name for this policy
@@ -223,6 +245,9 @@ Functions
 
   .. versionadded:: 1.6.0
 
+  .. versionchanged:: 1.9.2
+    Returning a value equal or greater than the number of servers will be interpreted as all servers being unavailable.
+
   Set server selection policy to one named ``name`` and the Lua FFI function returned by the Lua code passed in ``code``.
   The resulting policy will be executed in a lock-free per-thread context, instead of running in the global Lua context.
 

diff --git a/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc b/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc
index fb74a16264e9d60cdcd60d7349af2974d78ae885..8b6209f2bbe73325de7cb245874d040d113e00a7 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc
+++ b/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc
@@ -654,6 +654,43 @@ BOOST_AUTO_TEST_CASE(test_lua_ffi_rr) {
   resetLuaContext();
 }
 
+BOOST_AUTO_TEST_CASE(test_lua_ffi_no_server_available) {
+  DNSName name("powerdns.com.");
+  static const std::string policySetupStr = R"foo(
+    local ffi = require("ffi")
+    local C = ffi.C
+    local counter = 0
+    function ffipolicy(servers_list, dq)
+      local serversCount = tonumber(C.dnsdist_ffi_servers_list_get_count(servers_list))
+      -- return clearly out of bounds value to indicate that no server can be used
+      return serversCount + 100
+    end
+
+    setServerPolicyLuaFFI("FFI policy", ffipolicy)
+  )foo";
+  resetLuaContext();
+  g_lua.lock()->executeCode(getLuaFFIWrappers());
+  g_lua.lock()->writeFunction("setServerPolicyLuaFFI", [](string name, ServerPolicy::ffipolicyfunc_t policy) {
+      g_policy.setState(ServerPolicy(name, policy));
+    });
+  g_lua.lock()->executeCode(policySetupStr);
+
+  {
+    ServerPolicy pol = g_policy.getCopy();
+    ServerPolicy::NumberedServerVector servers;
+    for (size_t idx = 1; idx <= 10; idx++) {
+      servers.push_back({ idx, std::make_shared<DownstreamState>(ComboAddress("192.0.2." + std::to_string(idx) + ":53")) });
+      servers.at(idx - 1).second->setUp();
+    }
+    BOOST_REQUIRE_EQUAL(servers.size(), 10U);
+
+    auto dq = getDQ(&name);
+    auto server = pol.getSelectedBackend(servers, dq);
+    BOOST_REQUIRE(server == nullptr);
+  }
+  resetLuaContext();
+}
+
 BOOST_AUTO_TEST_CASE(test_lua_ffi_hashed) {
   std::vector<DNSName> names;
   names.reserve(1000);

diff --git a/regression-tests.dnsdist/test_Routing.py b/regression-tests.dnsdist/test_Routing.py
index 1dabca1a746270c1977c711dffce86b36718a460..186cba069217e39ff790e4c0d3ccf754ac0fba0b 100644 (file)
--- a/regression-tests.dnsdist/test_Routing.py
+++ b/regression-tests.dnsdist/test_Routing.py
@@ -799,3 +799,37 @@ class TestRoutingHighValueWRandom(DNSDistTest):
             self.assertEqual(self._responsesCounter['UDP Responder 2'], numberOfQueries - self._responsesCounter['UDP Responder'])
         if 'TCP Responder 2' in self._responsesCounter:
             self.assertEqual(self._responsesCounter['TCP Responder 2'], numberOfQueries - self._responsesCounter['TCP Responder'])
+
+class TestRoutingLuaFFILBNoServer(DNSDistTest):
+
+    _config_template = """
+    -- we want a ServFail answer when all servers are down
+    setServFailWhenNoServer(true)
+
+    local ffi = require("ffi")
+    local C = ffi.C
+    function luaffipolicy(servers_list, dq)
+      -- return a large value, outside of the number of servers, to indicate that
+      -- no server is available
+      return tonumber(C.dnsdist_ffi_servers_list_get_count(servers_list)) + 100
+    end
+    setServerPolicyLuaFFI("luaffipolicy", luaffipolicy)
+
+    s1 = newServer{address="127.0.0.1:%s"}
+    s1:setDown()
+    """
+    _verboseMode = True
+
+    def testOurPolicy(self):
+        """
+        Routing: LuaFFI policy, all servers are down
+        """
+        name = 'lua-ffi-no-servers.routing.tests.powerdns.com.'
+        query = dns.message.make_query(name, 'A', 'IN')
+        expectedResponse = dns.message.make_response(query)
+        expectedResponse.set_rcode(dns.rcode.SERVFAIL)
+
+        for method in ("sendUDPQuery", "sendTCPQuery"):
+            sender = getattr(self, method)
+            (_, receivedResponse) = sender(query, response=None, useQueue=False)
+            self.assertEqual(expectedResponse, receivedResponse)