[master] Merge branch 'trac1399'

doc/guide/bind10-guide.xml

@@ -2286,6 +2286,10 @@ eth0 fe80::21e:8cff:fe9b:7349
 &gt; <userinput>Stats show</userinput>
 {
     "Auth": {
+        "opcode.iquery": 0,
+        "opcode.notify": 10,
+        "opcode.query": 869617,
+        ...
         "queries.tcp": 1749,
         "queries.udp": 867868
     },

src/bin/auth/auth.spec.pre.in

@@ -139,6 +139,134 @@
         "item_default": 0,
         "item_title": "Queries UDP",
         "item_description": "A number of total query counts which all auth servers receive over UDP since they started initially"
+      },
+      {
+        "item_name": "opcode.query",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received query requests",
+        "item_description": "The number of total request counts whose opcode is query"
+      },
+      {
+        "item_name": "opcode.iquery",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received inverse query requests",
+        "item_description": "The number of total request counts whose opcode is inverse query"
+      },
+      {
+        "item_name": "opcode.status",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received status requests",
+        "item_description": "The number of total request counts whose opcode is status"
+      },
+      {
+        "item_name": "opcode.reserved3",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 3",
+        "item_description": "The number of total request counts whose opcode is 3 (reserved)"
+      },
+      {
+        "item_name": "opcode.notify",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received notify requests",
+        "item_description": "The number of total request counts whose opcode is notify"
+      },
+      {
+        "item_name": "opcode.update",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received update requests",
+        "item_description": "The number of total request counts whose opcode is update"
+      },
+      {
+        "item_name": "opcode.reserved6",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 6",
+        "item_description": "The number of total request counts whose opcode is 6 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved7",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 7",
+        "item_description": "The number of total request counts whose opcode is 7 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved8",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 8",
+        "item_description": "The number of total request counts whose opcode is8 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved9",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 9",
+        "item_description": "The number of total request counts whose opcode is9 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved10",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 10",
+        "item_description": "The number of total request counts whose opcode is10 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved11",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 11",
+        "item_description": "The number of total request counts whose opcode is11 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved12",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 12",
+        "item_description": "The number of total request counts whose opcode is12 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved13",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 13",
+        "item_description": "The number of total request counts whose opcode is13 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved14",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 14",
+        "item_description": "The number of total request counts whose opcode is14 (reserved)"
+      },
+      {
+        "item_name": "opcode.reserved15",
+        "item_type": "integer",
+        "item_optional": true,
+        "item_default": 0,
+        "item_title": "Received requests opcode 15",
+        "item_description": "The number of total request counts whose opcode is15 (reserved)"
       }
     ]
   }

src/bin/auth/auth_srv.cc

@@ -457,12 +457,20 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
                                           io_message.getDataSize());
     }
 
-    bool sendAnswer = true;
     if (tsig_error != TSIGError::NOERROR()) {
         makeErrorMessage(message, buffer, tsig_error.toRcode(), tsig_context);
-    } else if (message->getOpcode() == Opcode::NOTIFY()) {
-        sendAnswer = impl_->processNotify(io_message, message, buffer,
-                                          tsig_context);
+        server->resume(true);
+        return;
+    }
+
+    // update per opcode statistics counter.  This can only be reliable after
+    // TSIG check succeeds.
+    impl_->counters_.inc(message->getOpcode());
+
+    bool send_answer = true;
+    if (message->getOpcode() == Opcode::NOTIFY()) {
+        send_answer = impl_->processNotify(io_message, message, buffer,
+                                           tsig_context);
     } else if (message->getOpcode() != Opcode::QUERY()) {
         LOG_DEBUG(auth_logger, DBG_AUTH_DETAIL, AUTH_UNSUPPORTED_OPCODE)
                   .arg(message->getOpcode().toText());
@@ -473,18 +481,18 @@ AuthSrv::processMessage(const IOMessage& io_message, MessagePtr message,
         ConstQuestionPtr question = *message->beginQuestion();
         const RRType &qtype = question->getType();
         if (qtype == RRType::AXFR()) {
-            sendAnswer = impl_->processXfrQuery(io_message, message, buffer,
-                                                tsig_context);
+            send_answer = impl_->processXfrQuery(io_message, message, buffer,
+                                                 tsig_context);
         } else if (qtype == RRType::IXFR()) {
-            sendAnswer = impl_->processXfrQuery(io_message, message, buffer,
-                                                tsig_context);
+            send_answer = impl_->processXfrQuery(io_message, message, buffer,
+                                                 tsig_context);
         } else {
-            sendAnswer = impl_->processNormalQuery(io_message, message, buffer,
-                                                   tsig_context);
+            send_answer = impl_->processNormalQuery(io_message, message,
+                                                    buffer, tsig_context);
         }
     }
 
-    server->resume(sendAnswer);
+    server->resume(send_answer);
 }
 
 bool
@@ -770,6 +778,11 @@ AuthSrv::getCounter(const AuthCounters::ServerCounterType type) const {
     return (impl_->counters_.getCounter(type));
 }
 
+uint64_t
+AuthSrv::getCounter(const Opcode opcode) const {
+    return (impl_->counters_.getCounter(opcode));
+}
+
 const AddressList&
 AuthSrv::getListenAddresses() const {
     return (impl_->listen_addresses_);

src/bin/auth/auth_srv.h

@@ -24,6 +24,7 @@
 #include <cc/data.h>
 #include <config/ccsession.h>
 #include <dns/message.h>
+#include <dns/opcode.h>
 #include <util/buffer.h>
 
 #include <asiodns/dns_server.h>
@@ -345,6 +346,20 @@ public:
     /// \return the value of the counter.
     uint64_t getCounter(const AuthCounters::ServerCounterType type) const;
 
+    /// \brief Get the value of per Opcode counter in the Auth Counters.
+    ///
+    /// This function calls AuthCounters::getCounter(isc::dns::Opcode) 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 opcode The opcode of the counter to get the value of
+    /// \return the value of the counter.
+    uint64_t getCounter(const isc::dns::Opcode opcode) const;
+
     /**
      * \brief Set and get the addresses we listen on.
      */

src/bin/auth/statistics.cc

@@ -15,17 +15,24 @@
 #include <auth/statistics.h>
 #include <auth/auth_log.h>
 
+#include <dns/opcode.h>
+
 #include <cc/data.h>
 #include <cc/session.h>
 
 #include <statistics/counter.h>
 #include <statistics/counter_dict.h>
 
+#include <algorithm>
+#include <cctype>
+#include <cassert>
+#include <string>
 #include <sstream>
 #include <iostream>
 
 #include <boost/noncopyable.hpp>
 
+using namespace isc::dns;
 using namespace isc::auth;
 using namespace isc::statistics;
 
@@ -40,6 +47,9 @@ public:
     AuthCountersImpl();
     ~AuthCountersImpl();
     void inc(const AuthCounters::ServerCounterType type);
+    void inc(const Opcode opcode) {
+        opcode_counter_.inc(opcode.getCode());
+    }
     void inc(const std::string& zone,
              const AuthCounters::PerZoneCounterType type);
     bool submitStatistics() const;
@@ -48,8 +58,13 @@ public:
     (AuthCounters::validator_type validator);
     // Currently for testing purpose only
     uint64_t getCounter(const AuthCounters::ServerCounterType type) const;
+    uint64_t getCounter(const Opcode opcode) const {
+        return (opcode_counter_.get(opcode.getCode()));
+    }
 private:
     Counter server_counter_;
+    Counter opcode_counter_;
+    static const size_t NUM_OPCODES = 16;
     CounterDictionary per_zone_counter_;
     isc::cc::AbstractSession* statistics_session_;
     AuthCounters::validator_type validator_;
@@ -60,6 +75,7 @@ AuthCountersImpl::AuthCountersImpl() :
     // size of server_counter_: AuthCounters::SERVER_COUNTER_TYPES
     // size of per_zone_counter_: AuthCounters::PER_ZONE_COUNTER_TYPES
     server_counter_(AuthCounters::SERVER_COUNTER_TYPES),
+    opcode_counter_(NUM_OPCODES),
     per_zone_counter_(AuthCounters::PER_ZONE_COUNTER_TYPES),
     statistics_session_(NULL)
 {
@@ -94,8 +110,22 @@ AuthCountersImpl::submitStatistics() const {
                       <<     "{ \"queries.udp\": "
                       <<     server_counter_.get(AuthCounters::SERVER_UDP_QUERY)
                       <<     ", \"queries.tcp\": "
-                      <<     server_counter_.get(AuthCounters::SERVER_TCP_QUERY)
-                      <<   " }"
+                      <<     server_counter_.get(
+                          AuthCounters::SERVER_TCP_QUERY);
+    // Insert non 0 Opcode counters.
+    for (int i = 0; i < NUM_OPCODES; ++i) {
+        const Counter::Type counter = opcode_counter_.get(i);
+        if (counter != 0) {
+            // The counter item name should be derived lower-cased textual
+            // representation of the code.
+            std::string opcode_txt = Opcode(i).toText();
+            std::transform(opcode_txt.begin(), opcode_txt.end(),
+                           opcode_txt.begin(), ::tolower);
+            statistics_string << ", \"opcode." << opcode_txt << "\": "
+                              << counter;
+        }
+    }
+    statistics_string <<   " }"
                       <<   "}"
                       << "]}";
     isc::data::ConstElementPtr statistics_element =
@@ -159,6 +189,11 @@ AuthCounters::inc(const AuthCounters::ServerCounterType type) {
     impl_->inc(type);
 }
 
+void
+AuthCounters::inc(const Opcode opcode) {
+    impl_->inc(opcode);
+}
+
 bool
 AuthCounters::submitStatistics() const {
     return (impl_->submitStatistics());
@@ -176,6 +211,11 @@ AuthCounters::getCounter(const AuthCounters::ServerCounterType type) const {
     return (impl_->getCounter(type));
 }
 
+uint64_t
+AuthCounters::getCounter(const Opcode opcode) const {
+    return (impl_->getCounter(opcode));
+}
+
 void
 AuthCounters::registerStatisticsValidator
     (AuthCounters::validator_type validator) const

src/bin/auth/statistics.h

@@ -15,6 +15,8 @@
 #ifndef __STATISTICS_H
 #define __STATISTICS_H 1
 
+#include <dns/opcode.h>
+
 #include <cc/session.h>
 #include <stdint.h>
 #include <boost/scoped_ptr.hpp>
@@ -87,6 +89,15 @@ public:
     /// 
     void inc(const ServerCounterType type);
 
+    /// \brief Increment the counter of a per opcode counter.
+    ///
+    /// \note This is a tentative interface.  See \c getCounter().
+    ///
+    /// \param opcode The opcode of the counter to increment.
+    ///
+    /// \throw None
+    void inc(const isc::dns::Opcode opcode);
+
     /// \brief Submit statistics counters to statistics module.
     ///
     /// This method is desinged to be called periodically
@@ -125,7 +136,7 @@ public:
     ///
     void setStatisticsSession(isc::cc::AbstractSession* statistics_session);
 
-    /// \brief Get a value of a counter in the AuthCounters.
+    /// \brief Get the value of a counter in the AuthCounters.
     ///
     /// This function returns a value of the counter specified by \a type.
     /// This method never throws an exception.
@@ -135,9 +146,22 @@ public:
     /// \param type Type of a counter to get the value of
     ///
     /// \return the value of the counter specified by \a type.
-    ///
     uint64_t getCounter(const AuthCounters::ServerCounterType type) const;
 
+    /// \brief Get the value of a per opcode counter.
+    ///
+    /// This method returns the value of the per opcode counter for the
+    /// specified \c opcode.
+    ///
+    /// \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 opcode The opcode of the counter to get the value of
+    /// \return the value of the counter.
+    uint64_t getCounter(const isc::dns::Opcode opcode) const;
+
     /// \brief A type of validation function for the specification in
     /// isc::config::ModuleSpec.
     ///

src/bin/auth/tests/auth_srv_unittest.cc

@@ -23,6 +23,7 @@
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
 #include <dns/name.h>
+#include <dns/opcode.h>
 #include <dns/rrclass.h>
 #include <dns/rrtype.h>
 #include <dns/rrttl.h>
@@ -376,6 +377,9 @@ TEST_F(AuthSrvTest, TSIGCheckFirst) {
     EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig->getRdata().getError());
     EXPECT_EQ(0, tsig->getRdata().getMACSize()) <<
         "It should be unsigned with this error";
+    // TSIG should have failed, and so the per opcode counter shouldn't be
+    // incremented.
+    EXPECT_EQ(0, server.getCounter(Opcode::RESERVED14()));
 }
 
 TEST_F(AuthSrvTest, AXFRConnectFail) {
@@ -839,6 +843,30 @@ TEST_F(AuthSrvTest, queryCounterTCPIXFR) {
     EXPECT_EQ(1, server.getCounter(AuthCounters::SERVER_TCP_QUERY));
 }
 
+TEST_F(AuthSrvTest, queryCounterOpcodes) {
+    for (int i = 0; i < 16; ++i) {
+        // The counter should be initialized to 0.
+        EXPECT_EQ(0, server.getCounter(Opcode(i)));
+
+        // For each possible opcode, create a request message and send it
+        UnitTestUtil::createRequestMessage(request_message, Opcode(i),
+                                           default_qid, Name("example.com"),
+                                           RRClass::IN(), RRType::NS());
+        createRequestPacket(request_message, IPPROTO_UDP);
+
+        // "send" the request N-th times where N is i + 1 for i-th code.
+        // we intentionally use different values for each code
+        for (int j = 0; j <= i; ++j) {
+            parse_message->clear(Message::PARSE);
+            server.processMessage(*io_message, parse_message, response_obuffer,
+                                  &dnsserv);
+        }
+
+        // Confirm the counter.
+        EXPECT_EQ(i + 1, server.getCounter(Opcode(i)));
+    }
+}
+
 // class for queryCounterUnexpected test
 // getProtocol() returns IPPROTO_IP
 class DummyUnknownSocket : public IOSocket {

src/bin/auth/tests/statistics_unittest.cc

@@ -14,10 +14,14 @@
 
 #include <config.h>
 
+#include <string>
+
 #include <gtest/gtest.h>
 
 #include <boost/bind.hpp>
 
+#include <dns/opcode.h>
+
 #include <cc/data.h>
 #include <cc/session.h>
 
@@ -170,6 +174,16 @@ TEST_F(AuthCountersTest, incrementInvalidCounter) {
                  isc::OutOfRange);
 }
 
+TEST_F(AuthCountersTest, incrementOpcodeCounter) {
+    // The counter should be initialized to 0.  If we increment it by 1
+    // the counter should be 1.
+    for (int i = 0; i < 16; ++i) {
+        EXPECT_EQ(0, counters.getCounter(Opcode(i)));
+        counters.inc(Opcode(i));
+        EXPECT_EQ(1, counters.getCounter(Opcode(i)));
+    }
+}
+
 TEST_F(AuthCountersTest, submitStatisticsWithoutSession) {
     // Set statistics_session to NULL and call submitStatistics().
     // Expect to return false.
@@ -189,6 +203,28 @@ TEST_F(AuthCountersTest, submitStatisticsWithException) {
     statistics_session_.setThrowSessionTimeout(false);
 }
 
+void
+opcodeDataCheck(ConstElementPtr data, const int expected[16]) {
+    const char* item_names[] = {
+        "query", "iquery", "status", "reserved3", "notify", "update",
+        "reserved6", "reserved7", "reserved8", "reserved9", "reserved10",
+        "reserved11", "reserved12", "reserved13", "reserved14", "reserved15",
+        NULL
+    };
+    int i;
+    for (i = 0; i < 16; ++i) {
+        ASSERT_NE(static_cast<const char*>(NULL), item_names[i]);
+        const string item_name = "opcode." + string(item_names[i]);
+        if (expected[i] == 0) {
+            EXPECT_FALSE(data->get(item_name));
+        } else {
+            EXPECT_EQ(expected[i], data->get(item_name)->intValue());
+        }
+    }
+    // We should have examined all names
+    ASSERT_EQ(static_cast<const char*>(NULL), item_names[i]);
+}
+
 TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
     // Submit statistics data.
     // Validate if it submits correct data.
@@ -217,6 +253,44 @@ TEST_F(AuthCountersTest, submitStatisticsWithoutValidator) {
     // UDP query counter is 2 and TCP query counter is 1.
     EXPECT_EQ(2, statistics_data->get("queries.udp")->intValue());
     EXPECT_EQ(1, statistics_data->get("queries.tcp")->intValue());
+
+    // By default opcode counters are all 0 and omitted
+    const int opcode_results[16] = { 0, 0, 0, 0, 0, 0, 0, 0,
+                                     0, 0, 0, 0, 0, 0, 0, 0 };
+    opcodeDataCheck(statistics_data, opcode_results);
+}
+
+void
+updateOpcodeCounters(AuthCounters &counters, const int expected[16]) {
+    for (int i = 0; i < 16; ++i) {
+        for (int j = 0; j < expected[i]; ++j) {
+            counters.inc(Opcode(i));
+        }
+    }
+}
+
+TEST_F(AuthCountersTest, submitStatisticsWithOpcodeCounters) {
+    // Increment some of the opcode counters.  Then they should appear in the
+    // submitted data; others shouldn't
+    const int opcode_results[16] = { 1, 2, 3, 0, 4, 5, 0, 0,
+                                     0, 0, 0, 0, 0, 0, 0, 0 };
+    updateOpcodeCounters(counters, opcode_results);
+    counters.submitStatistics();
+    ConstElementPtr statistics_data = statistics_session_.sent_msg
+        ->get("command")->get(1)->get("data");
+    opcodeDataCheck(statistics_data, opcode_results);
+}
+
+TEST_F(AuthCountersTest, submitStatisticsWithAllOpcodeCounters) {
+    // Increment all opcode counters.  Then they should appear in the
+    // submitted data.
+    const int opcode_results[16] = { 1, 1, 1, 1, 1, 1, 1, 1,
+                                     1, 1, 1, 1, 1, 1, 1, 1 };
+    updateOpcodeCounters(counters, opcode_results);
+    counters.submitStatistics();
+    ConstElementPtr statistics_data = statistics_session_.sent_msg
+        ->get("command")->get(1)->get("data");
+    opcodeDataCheck(statistics_data, opcode_results);
 }
 
 TEST_F(AuthCountersTest, submitStatisticsWithValidator) {

tests/system/bindctl/tests.sh

@@ -27,6 +27,7 @@ n=0
 # TODO: consider consistency with statistics definition in auth.spec
 auth_queries_tcp="\<queries\.tcp\>"
 auth_queries_udp="\<queries\.udp\>"
+auth_opcode_queries="\<opcode\.query\>"
 
 echo "I:Checking b10-auth is working by default ($n)"
 $DIG +norec @10.53.0.1 -p 53210 ns.example.com. A >dig.out.$n || status=1
@@ -46,6 +47,7 @@ echo 'Stats show
 # sent from the server startup script)
 grep $auth_queries_tcp".*\<1\>" bindctl.out.$n > /dev/null || status=1
 grep $auth_queries_udp".*\<1\>" bindctl.out.$n > /dev/null || status=1
+grep $auth_opcode_queries".*\<2\>" bindctl.out.$n > /dev/null || status=1
 if [ $status != 0 ]; then echo "I:failed"; fi
 n=`expr $n + 1`
 
@@ -80,6 +82,7 @@ echo 'Stats show
 # The statistics counters should have been reset while stop/start.
 grep $auth_queries_tcp".*\<0\>" bindctl.out.$n > /dev/null || status=1
 grep $auth_queries_udp".*\<1\>" bindctl.out.$n > /dev/null || status=1
+grep $auth_opcode_queries".*\<1\>" bindctl.out.$n > /dev/null || status=1
 if [ $status != 0 ]; then echo "I:failed"; fi
 n=`expr $n + 1`
 
@@ -104,6 +107,7 @@ echo 'Stats show
 # The statistics counters shouldn't be reset due to hot-swapping datasource.
 grep $auth_queries_tcp".*\<0\>" bindctl.out.$n > /dev/null || status=1
 grep $auth_queries_udp".*\<2\>" bindctl.out.$n > /dev/null || status=1
+grep $auth_opcode_queries".*\<2\>" bindctl.out.$n > /dev/null || status=1
 if [ $status != 0 ]; then echo "I:failed"; fi
 n=`expr $n + 1`