[2157] corrected condition for "request.badednsver"

src/bin/auth/auth_srv.cc

@@ -570,8 +570,7 @@ AuthSrv::processMessage(const IOMessage& io_message, Message& message,
         // note: This can only be reliable after TSIG check succeeds.
         ConstEDNSPtr edns = message.getEDNS();
         if (edns) {
-            impl_->stats_attrs_.setRequestEDNS(edns->getVersion() == 0,
-                                               edns->getVersion() != 0);
+            impl_->stats_attrs_.setRequestEDNS0(true);
             impl_->stats_attrs_.setRequestDO(edns->getDNSSECAwareness());
         }
 

src/bin/auth/statistics.cc.pre

@@ -157,9 +157,6 @@ Counters::incRequest(const MessageAttributes& msgattrs) {
     if (msgattrs.getRequestEDNS0()) {
         server_msg_counter_.inc(MSG_REQUEST_EDNS0);
     }
-    if (msgattrs.getRequestEDNSBadVer()) {
-        server_msg_counter_.inc(MSG_REQUEST_BADEDNSVER);
-    }
 
     // request DNSSEC
     if (msgattrs.getRequestDO()) {
@@ -202,6 +199,10 @@ Counters::incResponse(const MessageAttributes& msgattrs,
         rcode < num_rcode_to_msgcounter ?
         rcode_to_msgcounter[rcode] : MSG_RCODE_OTHER;
     server_msg_counter_.inc(rcode_type);
+    // Unsupported EDNS version
+    if (rcode == Rcode::BADVERS().getCode()) {
+        server_msg_counter_.inc(MSG_REQUEST_BADEDNSVER);
+    }
 
     if (msgattrs.getRequestOpCode() == Opcode::QUERY_CODE) {
         // compound attributes

src/bin/auth/statistics.h

@@ -45,7 +45,6 @@ private:
     uint8_t req_opcode_;            // OpCode
     enum BitAttributes {
         REQ_IS_EDNS_0,              // EDNS ver.0
-        REQ_IS_EDNS_BADVER,         // EDNS version other than 0
         REQ_IS_DNSSEC_OK,           // DNSSEC OK (DO) bit is set
         REQ_IS_TSIG,                // signed with valid TSIG
         REQ_IS_SIG0,                // signed with valid SIG(0)
@@ -113,21 +112,12 @@ public:
         return (bit_attributes_[REQ_IS_EDNS_0]);
     };
 
-    /// \brief Get request is EDNS version other than 0.
-    /// \return true if EDNS version other than 0
-    /// \throw None
-    bool getRequestEDNSBadVer() const {
-        return (bit_attributes_[REQ_IS_EDNS_BADVER]);
-    };
-
     /// \brief Set request EDNS attributes.
     /// \param is_edns_0 true if request is EDNS version 0
     /// \param is_edns_badver true if request is EDNS version other than 0
     /// \throw None
-    void setRequestEDNS(const bool is_edns_0, const bool is_edns_badver) {
-        assert(!(is_edns_0 && is_edns_badver));
+    void setRequestEDNS0(const bool is_edns_0) {
         bit_attributes_[REQ_IS_EDNS_0] = is_edns_0;
-        bit_attributes_[REQ_IS_EDNS_BADVER] = is_edns_badver;
     };
 
     /// \brief Get request DNSSEC OK (DO) bit.

src/bin/auth/tests/statistics_unittest.cc.pre

@@ -114,7 +114,7 @@ TEST_F(CountersTest, incrementResponse) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
 
         response.setRcode(Rcode::REFUSED());
@@ -159,7 +159,7 @@ TEST_F(CountersTest, incrementProtocolType) {
         msgattrs.setRequestIPVersion(af);
         msgattrs.setRequestTransportProtocol(proto);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
 
         response.setRcode(Rcode::REFUSED());
@@ -212,7 +212,7 @@ TEST_F(CountersTest, incrementDO) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(is_dnssec_ok);
 
         response.setRcode(Rcode::REFUSED());
@@ -243,21 +243,19 @@ TEST_F(CountersTest, incrementEDNS) {
     std::map<std::string, int> expect;
 
     // Test these patterns:
-    //     request edns0   badednsver  response edns0
-    //    --------------------------------------------
-    //     false           false       true
-    //     false           true        true
-    //     true            false       false
+    //     request edns0   response edns0
+    //    --------------------------------
+    //     false           true
+    //     true            false
     //
     // They can't be both true since edns0 and badednsver are exclusive.
-    int count_req_edns0 = 0, count_res_edns0 = 0, count_badver = 0;
-    for (int i = 0; i < 3; ++i) {
+    int count_req_edns0 = 0, count_res_edns0 = 0;
+    for (int i = 0; i < 2; ++i) {
         const bool is_edns0 = i & 1;
-        const bool is_badver = i & 2;
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(is_edns0, is_badver);
+        msgattrs.setRequestEDNS0(is_edns0);
         msgattrs.setRequestDO(true);
 
         if (!is_edns0) {
@@ -278,9 +276,6 @@ TEST_F(CountersTest, incrementEDNS) {
         } else {
             ++count_res_edns0;
         }
-        if (is_badver) {
-            ++count_badver;
-        }
 
         expect.clear();
         expect["opcode.query"] = i+1;
@@ -288,7 +283,7 @@ TEST_F(CountersTest, incrementEDNS) {
         expect["request.udp"] = i+1;
         expect["request.edns0"] = count_req_edns0;
         expect["response.edns0"] = count_res_edns0;
-        expect["request.badednsver"] = count_badver;
+        expect["request.badednsver"] = 0;
         expect["request.dnssec_ok"] = i+1;
         expect["responses"] = i+1;
         expect["qrynoauthans"] = i+1;
@@ -322,7 +317,7 @@ TEST_F(CountersTest, incrementTSIG) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(is_tsig, is_sig0, is_badsig);
 
@@ -381,7 +376,7 @@ TEST_F(CountersTest, incrementOpcode) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(i);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(false, false, false);
 
@@ -430,7 +425,7 @@ TEST_F(CountersTest, incrementRcode) {
     std::map<std::string, int> expect;
 
     // Test all rcodes (NOERROR..BADVERS)
-    int count_all = 0, count_rcode_other = 0;
+    int count_all = 0, count_rcode_other = 0, count_ednsbadver = 0;
     for (uint16_t i = Rcode::NOERROR().getCode(),
                   e = Rcode::BADVERS().getCode();
          i <= e;
@@ -439,7 +434,7 @@ TEST_F(CountersTest, incrementRcode) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::IQUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(false, false, false);
 
@@ -459,7 +454,6 @@ TEST_F(CountersTest, incrementRcode) {
         expect["request.v4"] = count_all;
         expect["request.udp"] = count_all;
         expect["request.edns0"] = count_all;
-        expect["request.badednsver"] = 0;
         expect["request.dnssec_ok"] = count_all;
         expect["request.tsig"] = 0;
         expect["request.sig0"] = 0;
@@ -468,6 +462,11 @@ TEST_F(CountersTest, incrementRcode) {
         if (rcode_to_msgcounter[i] == MSG_RCODE_OTHER) {
             count_rcode_other += i;
         }
+        // "request.badednsver" counts for Rcode == BADVERS
+        if (rcode_to_msgcounter[i] == MSG_RCODE_BADVERS) {
+            count_ednsbadver += i;
+        }
+        expect["request.badednsver"] = count_ednsbadver;
         for (uint16_t j = 0; j <= i; ++j) {
             if (rcode_to_msgcounter[j] == MSG_RCODE_OTHER) {
                 expect["rcode.other"] = count_rcode_other;
@@ -498,7 +497,7 @@ TEST_F(CountersTest, incrementTruncated) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::IQUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(false, false, false);
         msgattrs.setResponseTruncated(is_truncated);
@@ -544,7 +543,7 @@ TEST_F(CountersTest, incrementQryAuthAnsAndNoAuthAns) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(false, false, false);
 
@@ -584,7 +583,7 @@ TEST_F(CountersTest, incrementQrySuccess) {
     msgattrs.setRequestIPVersion(AF_INET);
     msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
     msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-    msgattrs.setRequestEDNS(true, false);
+    msgattrs.setRequestEDNS0(true);
     msgattrs.setRequestDO(true);
     msgattrs.setRequestSig(false, false, false);
 
@@ -633,7 +632,7 @@ TEST_F(CountersTest, incrementQryReferralAndNxrrset) {
         msgattrs.setRequestIPVersion(AF_INET);
         msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
         msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-        msgattrs.setRequestEDNS(true, false);
+        msgattrs.setRequestEDNS0(true);
         msgattrs.setRequestDO(true);
         msgattrs.setRequestSig(false, false, false);
 
@@ -676,7 +675,7 @@ TEST_F(CountersTest, incrementAuthQryRej) {
     msgattrs.setRequestIPVersion(AF_INET);
     msgattrs.setRequestTransportProtocol(IPPROTO_UDP);
     msgattrs.setRequestOpCode(Opcode::QUERY_CODE);
-    msgattrs.setRequestEDNS(true, false);
+    msgattrs.setRequestEDNS0(true);
     msgattrs.setRequestDO(true);
     msgattrs.setRequestSig(false, false, false);