BIND 10 trac1613, updated. fe5b8226c1d3237ed8feb924ad789cbdbee0287e [1613] fix rcode counting, and add additional tests for that
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Feb 16 10:46:29 UTC 2012
The branch, trac1613 has been updated
via fe5b8226c1d3237ed8feb924ad789cbdbee0287e (commit)
from 427039d6ab89abc66c53479f27e5c15d677b1431 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit fe5b8226c1d3237ed8feb924ad789cbdbee0287e
Author: Jelte Jansen <jelte at isc.org>
Date: Thu Feb 16 11:41:57 2012 +0100
[1613] fix rcode counting, and add additional tests for that
-----------------------------------------------------------------------
Summary of changes:
src/bin/auth/auth.spec.pre.in | 2 +-
src/bin/auth/auth_srv.cc | 28 ++++++++++-----
src/bin/auth/auth_srv.h | 30 ++++++++++++++++
src/bin/auth/tests/auth_srv_unittest.cc | 56 +++++++++++++++++++++++++++++++
4 files changed, 106 insertions(+), 10 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/auth/auth.spec.pre.in b/src/bin/auth/auth.spec.pre.in
index 5049fc1..1f7b566 100644
--- a/src/bin/auth/auth.spec.pre.in
+++ b/src/bin/auth/auth.spec.pre.in
@@ -344,7 +344,7 @@
"item_optional": true,
"item_default": 0,
"item_title": "Sent 'no such rrset' response",
- "item_description": "The number of total responses with rcode 8 ()"
+ "item_description": "The number of total responses with rcode 8 (NXRRSET)"
},
{
"item_name": "rcode.notauth",
diff --git a/src/bin/auth/auth_srv.cc b/src/bin/auth/auth_srv.cc
index e27e365..d948c67 100644
--- a/src/bin/auth/auth_srv.cc
+++ b/src/bin/auth/auth_srv.cc
@@ -409,13 +409,13 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
// Ignore all responses.
if (message->getHeaderFlag(Message::HEADERFLAG_QR)) {
LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_RESPONSE_RECEIVED);
- server->resume(false);
+ resumeServer(server, message, false);
return;
}
} catch (const Exception& ex) {
LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_HEADER_PARSE_FAIL)
.arg(ex.what());
- server->resume(false);
+ resumeServer(server, message, false);
return;
}
@@ -426,13 +426,13 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PROTOCOL_ERROR)
.arg(error.getRcode().toText()).arg(error.what());
makeErrorMessage(message, buffer, error.getRcode());
- server->resume(true);
+ resumeServer(server, message, true);
return;
} catch (const Exception& ex) {
LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_PACKET_PARSE_ERROR)
.arg(ex.what());
makeErrorMessage(message, buffer, Rcode::SERVFAIL());
- server->resume(true);
+ resumeServer(server, message, true);
return;
} // other exceptions will be handled at a higher layer.
@@ -459,7 +459,7 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
if (tsig_error != TSIGError::NOERROR()) {
makeErrorMessage(message, buffer, tsig_error.toRcode(), tsig_context);
- server->resume(true);
+ resumeServer(server, message, true);
return;
}
@@ -492,10 +492,7 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
}
}
- // update per rcode statistics counter.
- impl_->counters_.inc(message->getRcode());
-
- server->resume(send_answer);
+ resumeServer(server, message, send_answer);
}
bool
@@ -786,6 +783,11 @@ AuthSrv::getCounter(const Opcode opcode) const {
return (impl_->counters_.getCounter(opcode));
}
+uint64_t
+AuthSrv::getCounter(const Rcode rcode) const {
+ return (impl_->counters_.getCounter(rcode));
+}
+
const AddressList&
AuthSrv::getListenAddresses() const {
return (impl_->listen_addresses_);
@@ -805,3 +807,11 @@ void
AuthSrv::setTSIGKeyRing(const boost::shared_ptr<TSIGKeyRing>* keyring) {
impl_->keyring_ = keyring;
}
+
+void
+AuthSrv::resumeServer(DNSServer* server, MessagePtr message, bool done) {
+ if (done) {
+ impl_->counters_.inc(message->getRcode());
+ }
+ server->resume(done);
+}
diff --git a/src/bin/auth/auth_srv.h b/src/bin/auth/auth_srv.h
index 5383e5a..676582a 100644
--- a/src/bin/auth/auth_srv.h
+++ b/src/bin/auth/auth_srv.h
@@ -361,6 +361,20 @@ public:
/// \return the value of the counter.
uint64_t getCounter(const isc::dns::Opcode opcode) const;
+ /// \brief Get the value of per Rcode counter in the Auth Counters.
+ ///
+ /// This function calls AuthCounters::getCounter(isc::dns::Rcode) and
+ /// returns its return value.
+ ///
+ /// \note This is a tentative interface as an attempt of experimentally
+ /// supporting more statistics counters. This should eventually be more
+ /// generalized. In any case, this method is mainly for testing.
+ ///
+ /// \throw None
+ /// \param rcode The rcode of the counter to get the value of
+ /// \return the value of the counter.
+ uint64_t getCounter(const isc::dns::Rcode rcode) const;
+
/**
* \brief Set and get the addresses we listen on.
*/
@@ -382,6 +396,22 @@ public:
keyring);
private:
+ /// \brief Resume the server
+ ///
+ /// This is a wrapper call for DNSServer::resume(done), if 'done' is true,
+ /// the Rcode set in the given Message is counted in the statistics
+ /// counter.
+ ///
+ /// This method is expected to be called by processMessage()
+ ///
+ /// \param server The DNSServer as passed to processMessage()
+ /// \param message The response as constructed by processMessage()
+ /// \param done If true, the Rcode from the given message is counted,
+ /// this value is then passed to server->resume(bool)
+ void resumeServer(isc::asiodns::DNSServer* server,
+ isc::dns::MessagePtr message,
+ bool done);
+
AuthSrvImpl* impl_;
isc::asiolink::SimpleCallback* checkin_;
isc::asiodns::DNSLookup* dns_lookup_;
diff --git a/src/bin/auth/tests/auth_srv_unittest.cc b/src/bin/auth/tests/auth_srv_unittest.cc
index d9c3de6..6bbfe97 100644
--- a/src/bin/auth/tests/auth_srv_unittest.cc
+++ b/src/bin/auth/tests/auth_srv_unittest.cc
@@ -84,6 +84,32 @@ protected:
server.processMessage(*io_message, parse_message, response_obuffer,
&dnsserv);
}
+
+ // Helper for checking Rcode statistic counters
+ void checkRcodeCounter(const Rcode& rcode, int expected_value) {
+ EXPECT_EQ(expected_value, server.getCounter(rcode)) <<
+ "Expected Rcode count for " << rcode.toText() <<
+ " " << expected_value << ", was: " <<
+ server.getCounter(rcode);
+ }
+ // Checks whether all Rcode counters are set to zero
+ void checkAllRcodeCountersZero() {
+ for (int i = 0; i < 17; i++) {
+ checkRcodeCounter(Rcode(i), 0);
+ }
+ }
+ // Checks whether all Rcode counters are set to zero except the given
+ // rcode (it is checked to be set to 'value')
+ void checkAllRcodeCountersZeroExcept(const Rcode& rcode, int value) {
+ for (int i = 0; i < 17; i++) {
+ const Rcode rc(i);
+ if (rc == rcode) {
+ checkRcodeCounter(Rcode(i), value);
+ } else {
+ checkRcodeCounter(Rcode(i), 0);
+ }
+ }
+ }
IOService ios_;
DNSService dnss_;
MockSession statistics_session;
@@ -145,6 +171,7 @@ TEST_F(AuthSrvTest, builtInQuery) {
response_obuffer->getData(),
response_obuffer->getLength(),
&response_data[0], response_data.size());
+ checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
}
// Same test emulating the UDPServer class behavior (defined in libasiolink).
@@ -195,38 +222,46 @@ TEST_F(AuthSrvTest, iqueryViaDNSServer) {
// Unsupported requests. Should result in NOTIMP.
TEST_F(AuthSrvTest, unsupportedRequest) {
unsupportedRequest();
+ // unsupportedRequest tries 14 different opcodes
+ checkAllRcodeCountersZeroExcept(Rcode::NOTIMP(), 14);
}
// Multiple questions. Should result in FORMERR.
TEST_F(AuthSrvTest, multiQuestion) {
multiQuestion();
+ checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
}
// Incoming data doesn't even contain the complete header. Must be silently
// dropped.
TEST_F(AuthSrvTest, shortMessage) {
shortMessage();
+ checkAllRcodeCountersZero();
}
// Response messages. Must be silently dropped, whether it's a valid response
// or malformed or could otherwise cause a protocol error.
TEST_F(AuthSrvTest, response) {
response();
+ checkAllRcodeCountersZero();
}
// Query with a broken question
TEST_F(AuthSrvTest, shortQuestion) {
shortQuestion();
+ checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
}
// Query with a broken answer section
TEST_F(AuthSrvTest, shortAnswer) {
shortAnswer();
+ checkAllRcodeCountersZeroExcept(Rcode::FORMERR(), 1);
}
// Query with unsupported version of EDNS.
TEST_F(AuthSrvTest, ednsBadVers) {
ednsBadVers();
+ checkAllRcodeCountersZeroExcept(Rcode::BADVERS(), 1);
}
TEST_F(AuthSrvTest, AXFROverUDP) {
@@ -244,6 +279,7 @@ TEST_F(AuthSrvTest, AXFRSuccess) {
server.processMessage(*io_message, parse_message, response_obuffer, &dnsserv);
EXPECT_FALSE(dnsserv.hasAnswer());
EXPECT_TRUE(xfrout.isConnected());
+ checkAllRcodeCountersZero();
}
// Try giving the server a TSIG signed request and see it can anwer signed as
@@ -279,6 +315,8 @@ TEST_F(AuthSrvTest, TSIGSigned) {
response_obuffer->getLength()));
EXPECT_EQ(TSIGError::NOERROR(), error) <<
"The server signed the response, but it doesn't seem to be valid";
+
+ checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
}
// Give the server a signed request, but don't give it the key. It will
@@ -311,6 +349,8 @@ TEST_F(AuthSrvTest, TSIGSignedBadKey) {
EXPECT_EQ(TSIGError::BAD_KEY_CODE, tsig->getRdata().getError());
EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
"It should be unsigned with this error";
+
+ checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
}
// Give the server a signed request, but signed by a different key
@@ -344,6 +384,8 @@ TEST_F(AuthSrvTest, TSIGBadSig) {
EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError());
EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
"It should be unsigned with this error";
+
+ checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
}
// Give the server a signed unsupported request with a bad signature.
@@ -383,6 +425,8 @@ TEST_F(AuthSrvTest, TSIGCheckFirst) {
// TSIG should have failed, and so the per opcode counter shouldn't be
// incremented.
EXPECT_EQ(0, server.getCounter(Opcode::RESERVED14()));
+
+ checkAllRcodeCountersZeroExcept(Rcode::NOTAUTH(), 1);
}
TEST_F(AuthSrvTest, AXFRConnectFail) {
@@ -531,6 +575,8 @@ TEST_F(AuthSrvTest, notify) {
EXPECT_EQ(Name("example.com"), question->getName());
EXPECT_EQ(RRClass::IN(), question->getClass());
EXPECT_EQ(RRType::SOA(), question->getType());
+
+ checkAllRcodeCountersZeroExcept(Rcode::NOERROR(), 1);
}
TEST_F(AuthSrvTest, notifyForCHClass) {
@@ -799,6 +845,10 @@ TEST_F(AuthSrvTest, queryCounterUDPNormal) {
&dnsserv);
// After processing UDP query, the counter should be 1.
EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_UDP_QUERY));
+ // The counter for SUCCESS responses should also be one
+ EXPECT_EQ(1, server.getCounter(Opcode::QUERY()));
+ // The counter for REFUSED responses should also be one, the rest zero
+ checkAllRcodeCountersZeroExcept(Rcode::REFUSED(), 1);
}
// Submit TCP normal query and check query counter
@@ -814,6 +864,10 @@ TEST_F(AuthSrvTest, queryCounterTCPNormal) {
&dnsserv);
// After processing TCP query, the counter should be 1.
EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY));
+ // The counter for SUCCESS responses should also be one
+ EXPECT_EQ(1, server.getCounter(Opcode::QUERY()));
+ // The counter for REFUSED responses should also be one, the rest zero
+ checkAllRcodeCountersZeroExcept(Rcode::REFUSED(), 1);
}
// Submit TCP AXFR query and check query counter
@@ -829,6 +883,8 @@ TEST_F(AuthSrvTest, queryCounterTCPAXFR) {
EXPECT_FALSE(dnsserv.hasAnswer());
// After processing TCP AXFR query, the counter should be 1.
EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY));
+ // No rcodes should be incremented
+ checkAllRcodeCountersZero();
}
// Submit TCP IXFR query and check query counter
More information about the bind10-changes
mailing list