From b08dcc66791292f8448a6a99ddaed40221fb2949 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 28 Nov 2017 11:02:09 +0100 Subject: [PATCH] dnsdist: Keep the TCP connection open on cache hit, generated answers We used to close the TCP connection right away on cases where that did not make sense: - on a cache hit - on a self-generated answer - on a servfail answer caused by the lack of usable downstream servers We still close the TCP connections on drops, dynamic blocks, lack of usable downstream servers without `setServFailWhenNoServer()` set, invalid queries, network errors.. (cherry picked from commit dcacff179cc046fa40982bfe22eca36e1b39b665) --- pdns/dnsdist-tcp.cc | 8 +- regression-tests.dnsdist/test_TCPKeepAlive.py | 250 ++++++++++++++++++ 2 files changed, 255 insertions(+), 3 deletions(-) create mode 100644 regression-tests.dnsdist/test_TCPKeepAlive.py diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc index b5fe2fd5b..275e0e704 100644 --- a/pdns/dnsdist-tcp.cc +++ b/pdns/dnsdist-tcp.cc @@ -278,8 +278,9 @@ void* tcpClientThread(int pipefd) ds = nullptr; outstanding = false; - if(!getNonBlockingMsgLen(ci.fd, &qlen, g_tcpRecvTimeout)) + if(!getNonBlockingMsgLen(ci.fd, &qlen, g_tcpRecvTimeout)) { break; + } ci.cs->queries++; g_stats.queries++; @@ -376,7 +377,7 @@ void* tcpClientThread(int pipefd) #endif sendResponseToClient(ci.fd, query, dq.len); g_stats.selfAnswered++; - goto drop; + continue; } std::shared_ptr serverPool = getPool(*localPools, poolname); @@ -424,7 +425,7 @@ void* tcpClientThread(int pipefd) #endif sendResponseToClient(ci.fd, cachedResponse, cachedResponseSize); g_stats.cacheHits++; - goto drop; + continue; } g_stats.cacheMisses++; } @@ -443,6 +444,7 @@ void* tcpClientThread(int pipefd) } #endif sendResponseToClient(ci.fd, query, dq.len); + continue; } break; diff --git a/regression-tests.dnsdist/test_TCPKeepAlive.py b/regression-tests.dnsdist/test_TCPKeepAlive.py new file mode 100644 index 000000000..9f0f3fc54 --- /dev/null +++ b/regression-tests.dnsdist/test_TCPKeepAlive.py @@ -0,0 +1,250 @@ +#!/usr/bin/env python +import struct +import time +import dns +from dnsdisttests import DNSDistTest + +class TestTCPKeepAlive(DNSDistTest): + """ + These tests make sure that dnsdist keeps the TCP connection alive + in various cases, like cache hits, self-generated answer, and + that it doesn't in error cases (Drop, invalid queries...) + """ + + _tcpIdleTimeout = 20 + _maxTCPQueriesPerConn = 99 + _maxTCPConnsPerClient = 3 + _maxTCPConnDuration = 99 + _config_template = """ + newServer{address="127.0.0.1:%s"} + setTCPRecvTimeout(%s) + setMaxTCPQueriesPerConnection(%s) + setMaxTCPConnectionsPerClient(%s) + setMaxTCPConnectionDuration(%s) + pc = newPacketCache(100, 86400, 1) + getPool(""):setCache(pc) + addAction("refused.tcpka.tests.powerdns.com.", RCodeAction(dnsdist.REFUSED)) + addAction("dropped.tcpka.tests.powerdns.com.", DropAction()) + addResponseAction("dropped-response.tcpka.tests.powerdns.com.", DropResponseAction()) + -- create the pool named "nosuchpool" + getPool("nosuchpool") + addAction("nodownstream-servfail.tcpka.tests.powerdns.com.", PoolAction("nosuchpool")) + setServFailWhenNoServer(true) + """ + _config_params = ['_testServerPort', '_tcpIdleTimeout', '_maxTCPQueriesPerConn', '_maxTCPConnsPerClient', '_maxTCPConnDuration'] + + def testTCPKaSelfGenerated(self): + """ + TCP KeepAlive: Self-generated answer + """ + name = 'refused.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + expectedResponse = dns.message.make_response(query) + expectedResponse.set_rcode(dns.rcode.REFUSED) + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + self.assertEquals(expectedResponse, response) + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 5) + + def testTCPKaCacheHit(self): + """ + TCP KeepAlive: Cache Hit + """ + name = 'cachehit.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + expectedResponse = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 3600, + dns.rdataclass.IN, + dns.rdatatype.A, + '192.0.2.1') + expectedResponse.answer.append(rrset) + + # first query to fill the cache + (receivedQuery, receivedResponse) = self.sendTCPQuery(query, expectedResponse) + self.assertTrue(receivedQuery) + self.assertTrue(receivedResponse) + receivedQuery.id = query.id + self.assertEquals(query, receivedQuery) + self.assertEquals(receivedResponse, expectedResponse) + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + self.assertEquals(expectedResponse, response) + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 5) + + def testTCPKaNoDownstreamServFail(self): + """ + TCP KeepAlive: No downstream ServFail + + The query is routed to a pool that has no server, + and dnsdist is configured to send a ServFail when + that happens. We should keep the TCP connection open. + """ + name = 'nodownstream-servfail.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + expectedResponse = dns.message.make_response(query) + expectedResponse.set_rcode(dns.rcode.SERVFAIL) + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + self.assertEquals(expectedResponse, response) + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 5) + + def testTCPKaQRBitSet(self): + """ + TCP KeepAlive: QR bit set in question + """ + name = 'qrset.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + query.flags |= dns.flags.QR + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 0) + + def testTCPKaDrop(self): + """ + TCP KeepAlive: Drop + """ + name = 'dropped.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + query.flags |= dns.flags.QR + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 0) + + def testTCPKaDropResponse(self): + """ + TCP KeepAlive: Drop Response + """ + name = 'dropped-response.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 0) + +class TestTCPKeepAliveNoDownstreamDrop(DNSDistTest): + """ + This test makes sure that dnsdist drops the TCP connection + if no downstream server is available and setServFailWhenNoServer() + is not set. + """ + + _tcpIdleTimeout = 20 + _maxTCPQueriesPerConn = 99 + _maxTCPConnsPerClient = 3 + _maxTCPConnDuration = 99 + _config_template = """ + newServer{address="127.0.0.1:%s"} + setTCPRecvTimeout(%s) + setMaxTCPQueriesPerConnection(%s) + setMaxTCPConnectionsPerClient(%s) + setMaxTCPConnectionDuration(%s) + -- create the pool named "nosuchpool" + getPool("nosuchpool") + addAction("nodownstream-drop.tcpka.tests.powerdns.com.", PoolAction("nosuchpool")) + """ + _config_params = ['_testServerPort', '_tcpIdleTimeout', '_maxTCPQueriesPerConn', '_maxTCPConnsPerClient', '_maxTCPConnDuration'] + + def testTCPKaNoDownstreamDrop(self): + """ + TCP KeepAlive: No downstream Drop + + The query is routed to a pool that has no server, + and dnsdist is configured to drop the query when + that happens. We should close the TCP connection right away. + """ + name = 'nodownstream-drop.tcpka.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + + conn = self.openTCPConnection() + + count = 0 + for idx in xrange(5): + try: + self.sendTCPQueryOverConnection(conn, query) + response = self.recvTCPResponseOverConnection(conn) + if response is None: + break + count = count + 1 + except: + pass + + conn.close() + self.assertEqual(count, 0) -- 2.40.0