[#3140] Updated DHCPv6 server

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 4ba05feca5743342d0bdc2974c7bfea655bea4a8..9077efe51c5f668b97dfe235988c17389bf5a5b8 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -393,6 +393,8 @@ Dhcpv6Srv::testServerID(const Pkt6Ptr& pkt) {
                 .arg(pkt->getLabel())
                 .arg(duidToString(server_id))
                 .arg(duidToString(getServerID()));
+            StatsMgr::instance().addValue("pkt6-not-for-us",
+                                          static_cast<int64_t>(1));
             return (false);
         }
     }
@@ -411,6 +413,8 @@ Dhcpv6Srv::testUnicast(const Pkt6Ptr& pkt) const {
             LOG_DEBUG(bad_packet6_logger, DBGLVL_PKT_HANDLING, DHCP6_PACKET_DROP_UNICAST)
                 .arg(pkt->getLabel())
                 .arg(pkt->getName());
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             return (false);
         }
         break;
@@ -815,6 +819,9 @@ Dhcpv6Srv::processPacketAndSendResponse(Pkt6Ptr query) {
     Pkt6Ptr rsp;
     try {
         rsp = processPacket(query);
+        if (!rsp) {
+            return;
+        }
     } catch (...) {
         StatsMgr::instance().addValue("pkt6-processing-failed",
                                       static_cast<int64_t>(1));
@@ -822,9 +829,6 @@ Dhcpv6Srv::processPacketAndSendResponse(Pkt6Ptr query) {
                                       static_cast<int64_t>(1));
         throw;
     }
-    if (!rsp) {
-        return;
-    }
 
     CalloutHandlePtr callout_handle = getCalloutHandle(query);
     processPacketBufferSend(callout_handle, rsp);
@@ -939,7 +943,9 @@ Dhcpv6Srv::processPacket(Pkt6Ptr query) {
     if (!testServerID(query)) {
 
         // Increase the statistic of dropped packets.
-        StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
+        // The drop cause statistic was increased by testServerID.
+        StatsMgr::instance().addValue("pkt6-receive-drop",
+                                      static_cast<int64_t>(1));
         return (Pkt6Ptr());
     }
 
@@ -949,7 +955,8 @@ Dhcpv6Srv::processPacket(Pkt6Ptr query) {
     if (!testUnicast(query)) {
 
         // Increase the statistic of dropped packets.
-        StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
+        StatsMgr::instance().addValue("pkt6-receive-drop",
+                                      static_cast<int64_t>(1));
         return (Pkt6Ptr());
     }
 
@@ -1008,6 +1015,7 @@ Dhcpv6Srv::processPacket(Pkt6Ptr query) {
     }
 
     // Reject the message if it doesn't pass the sanity check.
+    // Statistics were updated by sanityCheck.
     if (!sanityCheck(query)) {
         return (Pkt6Ptr());
     }
@@ -1160,6 +1168,7 @@ Dhcpv6Srv::processLocalizedQuery6(AllocEngine::ClientContext6& ctx) {
     }
 
     Pkt6Ptr rsp;
+    // The only expected exception is RFCViolation.
     bool rfc_violation = false;
     try {
         try {
@@ -1223,12 +1232,14 @@ Dhcpv6Srv::processLocalizedQuery6(AllocEngine::ClientContext6& ctx) {
             .arg(e.what());
 
         // Increase the statistic of dropped packets.
+        // The RFCViolation thrower updated the drop cause statistic.
         if (!rfc_violation) {
             StatsMgr::instance().addValue("pkt6-processing-failed",
                                           static_cast<int64_t>(1));
         }
         StatsMgr::instance().addValue("pkt6-receive-drop",
                                       static_cast<int64_t>(1));
+        return (Pkt6Ptr());
     }
 
     if (!rsp) {
@@ -2069,6 +2080,8 @@ Dhcpv6Srv::sanityCheck(const Pkt6Ptr& pkt) {
             .arg(pkt->getName())
             .arg(pkt->getRemoteAddr().toText())
             .arg(e.what());
+        StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                      static_cast<int64_t>(1));
     }
 
     // Increase the statistic of dropped packets.
@@ -4567,19 +4580,29 @@ Dhcpv6Srv::processAddrRegInform(AllocEngine::ClientContext6& ctx) {
     const uint32_t no_iaid = 0; /* there's no IAID in the ADDR-REG-INFORM */
 
     // Check if the message is bad and shout be dropped.
+    bool invalid = false;
     try {
         // Check there is no IA_NA option.
         if (addr_reg_inf->getOption(D6O_IA_NA)) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "unexpected IA_NA option");
         }
 
         // Check there is no IA_TA option.
         if (addr_reg_inf->getOption(D6O_IA_TA)) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "unexpected IA_TA option");
         }
 
         // Check there is no IA_PD option.
         if (addr_reg_inf->getOption(D6O_IA_PD)) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "unexpected IA_PD option");
         }
 
@@ -4587,22 +4610,33 @@ Dhcpv6Srv::processAddrRegInform(AllocEngine::ClientContext6& ctx) {
         // There must be one.
         OptionCollection addrs = addr_reg_inf->getOptions(D6O_IAADDR);
         if (addrs.size() != 1) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "Exactly 1 IAADDR option expected, but "
                       << addrs.size() << " received");
         }
         iaaddr = boost::dynamic_pointer_cast<Option6IAAddr>(addrs.begin()->second);
         if (!iaaddr) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(Unexpected, "can't convert the IAADDR option");
         }
 
         // Client and IADDR addresses must match.
         if (addr != iaaddr->getAddress()) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "Address mismatch: client at " << addr
                       << " wants to register " << iaaddr->getAddress());
         }
 
         // Should be in the subnet.
         if (!subnet->inRange(addr)) {
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "Address " << addr << " is not in subnet "
                       << subnet->toText() << " (id " << subnet->getID() << ")");
         }
@@ -4613,6 +4647,9 @@ Dhcpv6Srv::processAddrRegInform(AllocEngine::ClientContext6& ctx) {
 
         // Address registration can't be mixed with standard allocation.
         if (old_lease && (old_lease->state_ != Lease6::STATE_REGISTERED)) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "Address " << addr << " already in use "
                       << *old_lease);
         }
@@ -4620,6 +4657,9 @@ Dhcpv6Srv::processAddrRegInform(AllocEngine::ClientContext6& ctx) {
         // Address must not be reserved.
         auto hosts = HostMgr::instance().getAll6(addr);
         if (!hosts.empty()) {
+            invalid = true;
+            StatsMgr::instance().addValue("pkt6-rfc-violation",
+                                          static_cast<int64_t>(1));
             isc_throw(RFCViolation, "Address " << addr << " is reserved");
         }
     } catch (const std::exception &ex) {
@@ -4627,6 +4667,12 @@ Dhcpv6Srv::processAddrRegInform(AllocEngine::ClientContext6& ctx) {
         LOG_INFO(packet6_logger, DHCP6_ADDR_REG_INFORM_FAIL)
             .arg(addr)
             .arg(ex.what());
+        if (!invalid) {
+            StatsMgr::instance().addValue("pkt6-processing-failed",
+                                          static_cast<int64_t>(1));
+        }
+        StatsMgr::instance().addValue("pkt6-receive-drop",
+                                      static_cast<int64_t>(1));
         return (Pkt6Ptr());
     }
 

diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index 92322b5ae23f52e44a48cdd745b5e12a7a48b981..894f452c03ed897b182f60800ea391d7f44a2441 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -2475,6 +2475,14 @@ TEST_F(Dhcpv6SrvTest, testServerID) {
     // Message should be dropped
     EXPECT_FALSE(srv_->testServerID(req));
 
+
+    // Check the pkt6-not-for-us stat was bumped by one.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr stat = mgr.getObservation("pkt6-not-for-us");
+    ASSERT_TRUE(stat);
+    EXPECT_EQ(1, stat->getInteger().first);
+
     // Delete server identifier option and add new one, with same value as
     // server's server identifier.
     req->delOption(D6O_SERVERID);
@@ -2511,6 +2519,14 @@ TEST_F(Dhcpv6SrvTest, testUnicast) {
             << "being sent to unicast address; this message should"
             " be discarded according to section 18.4 of RFC 8415";
     }
+
+    // The pkt6-rfc-violation stat should be bumped by one each time.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr stat = mgr.getObservation("pkt6-rfc-violation");
+    ASSERT_TRUE(stat);
+    EXPECT_EQ(sizeof(not_allowed_unicast), stat->getInteger().first);
+
     // Explicitly list client/relay message types which are allowed to
     // be sent to unicast.
     const uint8_t allowed_unicast[] = {