[3794] Initial statistics support added in DHCPv4.

src/bin/dhcp4/Makefile.am

@@ -82,6 +82,7 @@ kea_dhcp4_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la
 kea_dhcp4_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
+kea_dhcp4_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 
 kea_dhcp4dir = $(pkgdatadir)
 kea_dhcp4_DATA = dhcp4.spec

src/bin/dhcp4/dhcp4_srv.cc

@@ -41,6 +41,7 @@
 #include <hooks/hooks_log.h>
 #include <hooks/hooks_manager.h>
 #include <util/strutil.h>
+#include <stats/stats_mgr.h>
 
 #include <asio.hpp>
 #include <boost/bind.hpp>
@@ -400,6 +401,12 @@ Dhcpv4Srv::run() {
             continue;
         }
 
+        // Log reception of the packet. We need to increase it early, as any
+        // failures in unpacking will cause the packet to be dropped. We
+        // will increase type specific packets further down the road.
+        // See processStatsReceived().
+        isc::stats::StatsMgr::instance().addValue("pkt4-received", 1ul);
+
         // In order to parse the DHCP options, the server needs to use some
         // configuration information such as: existing option spaces, option
         // definitions etc. This is the kind of information which is not
@@ -462,6 +469,9 @@ Dhcpv4Srv::run() {
             }
         }
 
+        // Update statistics accordingly for received packet.
+        processStatsReceived(query);
+
         // Assign this packet to one or more classes if needed. We need to do
         // this before calling accept(), because getSubnet4() may need client
         // class information.
@@ -653,6 +663,10 @@ Dhcpv4Srv::run() {
                 .arg(static_cast<int>(rsp->getType()))
                 .arg(rsp->toText());
             sendPacket(rsp);
+
+            // Update statistics accordingly for sent packet.
+            processStatsSent(rsp);
+
         } catch (const std::exception& e) {
             LOG_ERROR(packet_logger, DHCP4_PACKET_SEND_FAIL)
                 .arg(rsp->getLabel())
@@ -2223,5 +2237,76 @@ Daemon::getVersion(bool extended) {
     return (tmp.str());
 }
 
+void Dhcpv4Srv::processStatsReceived(const Pkt4Ptr& query) {
+    // Note that we're not bumping pkt4-received statistic as it was
+    // increased early in the packet reception code.
+
+    string stat_name = "pkt4-unknown-received";
+    try {
+        switch (query->getType()) {
+        case DHCPDISCOVER:
+            stat_name = "pkt4-discover-received";
+            break;
+        case DHCPOFFER:
+            // should not happen, but we'll keep a counter for it anyway
+            stat_name = "pkt4-offer-received";
+            break;
+        case DHCPREQUEST:
+            stat_name = "pkt4-request-received";
+            break;
+        case DHCPACK:
+            stat_name = "pkt4-ack-received";
+            break;
+        case DHCPNAK:
+            stat_name = "pkt4-nak-received";
+            break;
+        case DHCPRELEASE:
+            stat_name = "pkt4-release-received";
+        break;
+        case DHCPDECLINE:
+            stat_name = "pkt4-decline-received";
+            break;
+        case DHCPINFORM:
+            stat_name = "pkt4-inform-received";
+            break;
+        default:
+            ; // do nothing
+        }
+    }
+    catch (...) {
+        // If the incoming packet doesn't have option 53 (message type)
+        // or a hook set pkt4_receive_skip, then Pkt4::getType() may
+        // throw an exception. That's ok, we'll then use the default
+        // name of pkt4-unknown-received.
+    }
+
+    isc::stats::StatsMgr::instance().addValue(stat_name, 1ul);
+}
+
+void Dhcpv4Srv::processStatsSent(const Pkt4Ptr& response) {
+    // Increase generic counter for sent packets.
+    isc::stats::StatsMgr::instance().addValue("pkt4-sent", 1ul);
+
+    // Increase packet type specific counter for packets sent.
+    string stat_name;
+    switch (response->getType()) {
+    case DHCPOFFER:
+        // should not happen, but we'll keep a counter for it anyway
+        stat_name = "pkt4-offer-sent";
+        break;
+    case DHCPACK:
+        stat_name = "pkt4-ack-sent";
+        break;
+    case DHCPNAK:
+        stat_name = "pkt4-nak-sent";
+        break;
+    default:
+        // That should never happen
+        stat_name = "pkt4-unknown-received";
+    }
+
+    isc::stats::StatsMgr::instance().addValue(stat_name, 1ul);
+}
+
 }   // namespace dhcp
 }   // namespace isc

src/bin/dhcp4/dhcp4_srv.h

@@ -736,6 +736,13 @@ private:
     /// @return Option that contains netmask information
     static OptionPtr getNetmaskOption(const Subnet4Ptr& subnet);
 
+    /// @brief Updates statistics for received packets
+    /// @param query packet received
+    static void processStatsReceived(const Pkt4Ptr& query);
+
+    /// @brief Updates statistics for transmitted packets
+    /// @param query packet transmitted
+    static void processStatsSent(const Pkt4Ptr& response);
 
     uint16_t port_;  ///< UDP port number on which server listens.
     bool use_bcast_; ///< Should broadcast be enabled on sockets (if true).

src/bin/dhcp4/tests/Makefile.am

@@ -115,6 +115,7 @@ dhcp4_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/util/libkea-util.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
+dhcp4_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/testutils/libdhcpsrvtest.la
 dhcp4_unittests_LDADD += $(top_builddir)/src/lib/util/io/libkea-util-io.la
 endif

src/bin/dhcp4/tests/dhcp4_client.cc

@@ -142,6 +142,8 @@ Dhcp4Client::applyConfiguration() {
         return;
     }
 
+    // Let's keep the old lease in case this is a response to Inform.
+    Lease4 old_lease = config_.lease_;
     config_.reset();
 
     // Routers
@@ -175,11 +177,17 @@ Dhcp4Client::applyConfiguration() {
         config_.serverid_ = opt_serverid->readAddress();
     }
 
-    /// @todo Set the valid lifetime, t1, t2 etc.
-    config_.lease_ = Lease4(IOAddress(context_.response_->getYiaddr()),
-                            context_.response_->getHWAddr(),
-                            0, 0, 0, 0, 0, time(NULL), 0, false, false,
-                            "");
+    // If the message sent was Inform, we don't want to throw
+    // away the old lease info, just the bits about options.
+    if (context_.query_->getType() == DHCPINFORM) {
+        config_.lease_ = old_lease;
+    } else {
+        /// @todo Set the valid lifetime, t1, t2 etc.
+        config_.lease_ = Lease4(IOAddress(context_.response_->getYiaddr()),
+                                context_.response_->getHWAddr(),
+                                0, 0, 0, 0, 0, time(NULL), 0, false, false,
+                                "");
+    }
 }
 
 void

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -43,6 +43,7 @@
 #include <hooks/server_hooks.h>
 #include <hooks/hooks_manager.h>
 #include <config/ccsession.h>
+#include <stats/stats_mgr.h>
 
 #include <boost/scoped_ptr.hpp>
 
@@ -61,6 +62,26 @@ using namespace isc::test;
 
 namespace {
 
+const char* CONFIGS[] = {
+    // Configuration 0:
+    // - 1 subnet: 10.254.226.0/25
+    // - used for recorded traffic (see PktCaptures::captureRelayedDiscover)
+    "{ \"interfaces-config\": {"
+        "    \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"rebind-timer\": 2000, "
+        "\"renew-timer\": 1000, "
+        "\"subnet4\": [ { "
+        "    \"pools\": [ { \"pool\": \"10.254.226.0/25\" } ],"
+        "    \"subnet\": \"10.254.226.0/24\", "
+        "    \"rebind-timer\": 2000, "
+        "    \"renew-timer\": 1000, "
+        "    \"valid-lifetime\": 4000,"
+        "    \"interface\": \"eth0\" "
+        " } ],"
+        "\"valid-lifetime\": 4000 }"
+};
+
 // This test verifies that the destination address of the response
 // message is set to giaddr, when giaddr is set to non-zero address
 // in the received message.
@@ -1085,22 +1106,7 @@ TEST_F(Dhcpv4SrvTest, relayAgentInfoEcho) {
     // subnet 10.254.226.0/24 is in use, because this packet
     // contains the giaddr which belongs to this subnet and
     // this giaddr is used to select the subnet
-    std::string config = "{ \"interfaces-config\": {"
-        "    \"interfaces\": [ \"*\" ]"
-        "},"
-        "\"rebind-timer\": 2000, "
-        "\"renew-timer\": 1000, "
-        "\"subnet4\": [ { "
-        "    \"pools\": [ { \"pool\": \"10.254.226.0/25\" } ],"
-        "    \"subnet\": \"10.254.226.0/24\", "
-        "    \"rebind-timer\": 2000, "
-        "    \"renew-timer\": 1000, "
-        "    \"valid-lifetime\": 4000,"
-        "    \"interface\": \"eth0\" "
-        " } ],"
-        "\"valid-lifetime\": 4000 }";
-
-    configure(config);
+    configure(CONFIGS[0]);
 
     // Let's create a relayed DISCOVER. This particular relayed DISCOVER has
     // added option 82 (relay agent info) with 3 suboptions. The server
@@ -3504,4 +3510,37 @@ TEST_F(Dhcpv4SrvTest, acceptMessageType) {
     }
 }
 
+TEST_F(Dhcpv4SrvTest, statisticsDecline) {
+    IfaceMgrTestConfig test_config(true);
+    IfaceMgr::instance().openSockets4();
+
+    NakedDhcpv4Srv srv(0);
+
+    // Use of the captured DHCPDISCOVER packet requires that
+    // subnet 10.254.226.0/24 is in use, because this packet
+    // contains the giaddr which belongs to this subnet and
+    // this giaddr is used to select the subnet
+    configure(CONFIGS[0]);
+
+    Pkt4Ptr decline = PktCaptures::captureRelayedDiscover();
+    decline->setType(DHCPDECLINE);
+
+    // Simulate that we have received that traffic
+    srv.fakeReceive(decline);
+    srv.run();
+
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_rcvd = mgr.getObservation("pkt4-received");
+    ObservationPtr pkt4_decline_rcvd = mgr.getObservation("pkt4-decline-received");
+
+    // All expected statstics must be present.
+    ASSERT_TRUE(pkt4_rcvd);
+    ASSERT_TRUE(pkt4_decline_rcvd);
+
+    // They also must have expected values.
+    EXPECT_EQ(1, pkt4_rcvd->getInteger().first);
+    EXPECT_EQ(1, pkt4_decline_rcvd->getInteger().first);
+}
+
 }; // end of anonymous namespace

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -29,6 +29,7 @@
 #include <dhcpsrv/lease.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <stats/stats_mgr.h>
 
 using namespace std;
 using namespace isc::asiolink;
@@ -53,6 +54,9 @@ Dhcpv4SrvTest::Dhcpv4SrvTest()
     CfgMgr::instance().clear();
     CfgMgr::instance().getStagingCfg()->getCfgSubnets4()->add(subnet_);
     CfgMgr::instance().commit();
+
+    // Let's wipe all existing statistics.
+    isc::stats::StatsMgr::instance().removeAll();
 }
 
 Dhcpv4SrvTest::~Dhcpv4SrvTest() {
@@ -60,6 +64,9 @@ Dhcpv4SrvTest::~Dhcpv4SrvTest() {
     // Make sure that we revert to default value
     CfgMgr::instance().clear();
     CfgMgr::instance().echoClientId(true);
+
+    // Let's wipe all existing statistics.
+    isc::stats::StatsMgr::instance().removeAll();
 }
 
 void Dhcpv4SrvTest::addPrlOption(Pkt4Ptr& pkt) {

src/bin/dhcp4/tests/dora_unittest.cc

@@ -24,6 +24,7 @@
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcp4/tests/dhcp4_client.h>
 #include <boost/shared_ptr.hpp>
+#include <stats/stats_mgr.h>
 
 using namespace isc;
 using namespace isc::asiolink;
@@ -171,6 +172,17 @@ public:
         : Dhcpv4SrvTest(),
           iface_mgr_test_config_(true) {
         IfaceMgr::instance().openSockets4();
+
+        // Let's wipe all existing statistics.
+        isc::stats::StatsMgr::instance().removeAll();
+    }
+
+    /// @brief Desctructor.
+    ///
+    /// Cleans up statistics after the test.
+    ~DORATest() {
+        // Let's wipe all existing statistics.
+        isc::stats::StatsMgr::instance().removeAll();
     }
 
     /// @brief Test that server doesn't allocate the lease for a client
@@ -959,4 +971,107 @@ TEST_F(DORATest, reservationsWithConflicts) {
     ASSERT_EQ(in_pool_addr, clientB.config_.lease_.addr_);
 }
 
+/// This test verifies that after a client completes its DORA exchange,
+/// appropriate statistics are updated.
+TEST_F(DORATest, statisticsDORA) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+
+    // Perform 4-way exchange with the server but to not request any
+    // specific address in the DHCPDISCOVER message.
+    ASSERT_NO_THROW(client.doDORA());
+
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+
+    // Ok, let's check the statistics.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
+    ObservationPtr pkt4_discover_received = mgr.getObservation("pkt4-discover-received");
+    ObservationPtr pkt4_offer_sent = mgr.getObservation("pkt4-offer-sent");
+    ObservationPtr pkt4_request_received = mgr.getObservation("pkt4-request-received");
+    ObservationPtr pkt4_ack_sent = mgr.getObservation("pkt4-ack-sent");
+    ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
+
+    // All expected statstics must be present.
+    ASSERT_TRUE(pkt4_received);
+    ASSERT_TRUE(pkt4_discover_received);
+    ASSERT_TRUE(pkt4_offer_sent);
+    ASSERT_TRUE(pkt4_request_received);
+    ASSERT_TRUE(pkt4_ack_sent);
+    ASSERT_TRUE(pkt4_sent);
+
+    // They also must have expected values.
+    EXPECT_EQ(2, pkt4_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_discover_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_offer_sent->getInteger().first);
+    EXPECT_EQ(1, pkt4_request_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_ack_sent->getInteger().first);
+    EXPECT_EQ(2, pkt4_sent->getInteger().first);
+
+    // Let the client send request 3 times, which should make the server
+    // to send 3 acks.
+    client.setState(Dhcp4Client::RENEWING);
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_NO_THROW(client.doRequest());
+    ASSERT_NO_THROW(client.doRequest());
+
+    // Let's see if the stats are properly updated.
+    EXPECT_EQ(5, pkt4_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_discover_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_offer_sent->getInteger().first);
+    EXPECT_EQ(4, pkt4_request_received->getInteger().first);
+    EXPECT_EQ(4, pkt4_ack_sent->getInteger().first);
+    EXPECT_EQ(5, pkt4_sent->getInteger().first);
+}
+
+// This test verifies that after a client completes an exchange that result
+// in NAK, appropriate statistics are updated.
+TEST_F(DORATest, statisticsNAK) {
+    Dhcp4Client client(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    configure(DORA_CONFIGS[0], *client.getServer());
+    // Obtain a lease from the server using the 4-way exchange.
+
+    // Get a lease.
+    client.doDORA();
+
+    // Wipe all stats.
+    isc::stats::StatsMgr::instance().removeAll();
+
+    client.setState(Dhcp4Client::INIT_REBOOT);
+    client.config_.lease_.addr_ = IOAddress("10.0.0.30");
+    ASSERT_NO_THROW(client.doRequest());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    EXPECT_EQ(DHCPNAK, static_cast<int>(resp->getType()));
+
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
+    ObservationPtr pkt4_request_received = mgr.getObservation("pkt4-request-received");
+    ObservationPtr pkt4_ack_sent = mgr.getObservation("pkt4-ack-sent");
+    ObservationPtr pkt4_nak_sent = mgr.getObservation("pkt4-nak-sent");
+    ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
+
+    // All expected statstics must be present.
+    ASSERT_TRUE(pkt4_received);
+    ASSERT_TRUE(pkt4_request_received);
+    ASSERT_FALSE(pkt4_ack_sent); // No acks were sent, no such statistic expected.
+    ASSERT_TRUE(pkt4_nak_sent);
+    ASSERT_TRUE(pkt4_sent);
+
+    // They also must have expected values.
+    EXPECT_EQ(1, pkt4_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_request_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_nak_sent->getInteger().first);
+    EXPECT_EQ(1, pkt4_sent->getInteger().first);
+}
+
 } // end of anonymous namespace

src/bin/dhcp4/tests/inform_unittest.cc

@@ -19,6 +19,7 @@
 #include <dhcp/tests/iface_mgr_test_config.h>
 #include <dhcp4/tests/dhcp4_test_utils.h>
 #include <dhcp4/tests/dhcp4_client.h>
+#include <stats/stats_mgr.h>
 
 using namespace isc;
 using namespace isc::asiolink;
@@ -136,6 +137,17 @@ public:
         : Dhcpv4SrvTest(),
           iface_mgr_test_config_(true) {
         IfaceMgr::instance().openSockets4();
+
+        // Let's wipe all existing statistics.
+        isc::stats::StatsMgr::instance().removeAll();
+    }
+
+    /// @brief Desctructor.
+    ///
+    /// Cleans up statistics after the test.
+    ~InformTest() {
+        // Let's wipe all existing statistics.
+        isc::stats::StatsMgr::instance().removeAll();
     }
 
     /// @brief Interface Manager's fake configuration control.
@@ -378,5 +390,58 @@ TEST_F(InformTest, relayedClientNoCiaddr) {
     EXPECT_EQ("192.0.2.203", client.config_.dns_servers_[1].toText());
 }
 
+/// This test verifies that after a client completes its INFORM exchange,
+/// appropriate statistics are updated.
+TEST_F(InformTest, statisticsInform) {
+    Dhcp4Client client;
+    // Configure DHCP server.
+    configure(INFORM_CONFIGS[0], *client.getServer());
+    // Request some configuration when DHCPINFORM is sent.
+    client.requestOptions(DHO_LOG_SERVERS, DHO_COOKIE_SERVERS);
+    // Preconfigure the client with the IP address.
+    client.createLease(IOAddress("10.0.0.56"), 600);
+
+    // Send DHCPINFORM message to the server.
+    ASSERT_NO_THROW(client.doInform());
+
+    // Make sure that the server responded.
+    ASSERT_TRUE(client.getContext().response_);
+    Pkt4Ptr resp = client.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+
+    // Ok, let's check the statistics.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt4_received = mgr.getObservation("pkt4-received");
+    ObservationPtr pkt4_inform_received = mgr.getObservation("pkt4-inform-received");
+    ObservationPtr pkt4_ack_sent = mgr.getObservation("pkt4-ack-sent");
+    ObservationPtr pkt4_sent = mgr.getObservation("pkt4-sent");
+
+    // All expected statistics must be present.
+    ASSERT_TRUE(pkt4_received);
+    ASSERT_TRUE(pkt4_inform_received);
+    ASSERT_TRUE(pkt4_ack_sent);
+    ASSERT_TRUE(pkt4_sent);
+
+    // And they must have expected values.
+    EXPECT_EQ(1, pkt4_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_inform_received->getInteger().first);
+    EXPECT_EQ(1, pkt4_ack_sent->getInteger().first);
+    EXPECT_EQ(1, pkt4_sent->getInteger().first);
+
+    // Let the client send iform 4 times, which should make the server
+    // to send 4 acks.
+    ASSERT_NO_THROW(client.doInform());
+    ASSERT_NO_THROW(client.doInform());
+    ASSERT_NO_THROW(client.doInform());
+    ASSERT_NO_THROW(client.doInform());
+
+    // Let's see if the stats are properly updated.
+    EXPECT_EQ(5, pkt4_received->getInteger().first);
+    EXPECT_EQ(5, pkt4_inform_received->getInteger().first);
+    EXPECT_EQ(5, pkt4_ack_sent->getInteger().first);
+    EXPECT_EQ(5, pkt4_sent->getInteger().first);
+}
 
 } // end of anonymous namespace