From: Tomek Mrugalski Date: Fri, 15 May 2015 17:31:14 +0000 (+0200) Subject: [3794] Many pkt4-receive-* statistics implemented X-Git-Tag: throw_with_base~3^2~5 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0b7152ce8a2ce7a08fdd50b1673918b737f35bf3;p=thirdparty%2Fkea.git [3794] Many pkt4-receive-* statistics implemented --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 520b6cecdf..459a43aaeb 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -480,6 +480,7 @@ Dhcpv4Srv::run() { // Check whether the message should be further processed or discarded. // There is no need to log anything here. This function logs by itself. if (!accept(query)) { + isc::stats::StatsMgr::instance().addValue("pkt4-receive-drop", 1ul); continue; } diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 47638de033..f35e0f7098 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -3510,37 +3510,61 @@ TEST_F(Dhcpv4SrvTest, acceptMessageType) { } } +// Test checks whether statistic is bumped up appropriately when Decline +// message is received. TEST_F(Dhcpv4SrvTest, statisticsDecline) { - IfaceMgrTestConfig test_config(true); - IfaceMgr::instance().openSockets4(); + NakedDhcpv4Srv srv(0); + pretendReceivingPkt(srv, CONFIGS[0], DHCPDECLINE, "pkt4-decline-received"); +} + +// Test checks whether statistic is bumped up appropriately when Offer +// message is received (this should never happen in a sane metwork). +TEST_F(Dhcpv4SrvTest, statisticsOfferRcvd) { 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]); + pretendReceivingPkt(srv, CONFIGS[0], DHCPOFFER, "pkt4-offer-received"); +} - Pkt4Ptr decline = PktCaptures::captureRelayedDiscover(); - decline->setType(DHCPDECLINE); +// Test checks whether statistic is bumped up appropriately when Ack +// message is received (this should never happen in a sane metwork). +TEST_F(Dhcpv4SrvTest, statisticsAckRcvd) { + NakedDhcpv4Srv srv(0); - // Simulate that we have received that traffic - srv.fakeReceive(decline); - srv.run(); + pretendReceivingPkt(srv, CONFIGS[0], DHCPACK, "pkt4-ack-received"); +} + +// Test checks whether statistic is bumped up appropriately when Nak +// message is received (this should never happen in a sane metwork). +TEST_F(Dhcpv4SrvTest, statisticsNakRcvd) { + NakedDhcpv4Srv srv(0); + + pretendReceivingPkt(srv, CONFIGS[0], DHCPNAK, "pkt4-nak-received"); +} + +// Test checks whether statistic is bumped up appropriately when Release +// message is received. +TEST_F(Dhcpv4SrvTest, statisticsReleaseRcvd) { + NakedDhcpv4Srv srv(0); + pretendReceivingPkt(srv, CONFIGS[0], DHCPRELEASE, "pkt4-release-received"); +} + +// Test checks whether statistic is bumped up appropriately when unknown +// message is received. +TEST_F(Dhcpv4SrvTest, statisticsUnknownRcvd) { + NakedDhcpv4Srv srv(0); + + pretendReceivingPkt(srv, CONFIGS[0], 200, "pkt4-unknown-received"); + + // There should also be pkt4-receive-drop stat bumped up 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); + ObservationPtr drop_stat = mgr.getObservation("pkt4-receive-drop"); - // They also must have expected values. - EXPECT_EQ(1, pkt4_rcvd->getInteger().first); - EXPECT_EQ(1, pkt4_decline_rcvd->getInteger().first); + // This statistic must be present and must be set to 1. + ASSERT_TRUE(drop_stat); + EXPECT_EQ(1, drop_stat->getInteger().first); } }; // end of anonymous namespace diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.cc b/src/bin/dhcp4/tests/dhcp4_test_utils.cc index 7e5c254123..4ca85b494d 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.cc +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -601,6 +602,50 @@ Dhcpv4SrvTest::createExchange(const Pkt4Ptr& query) { return (Dhcpv4Exchange(srv_.alloc_engine_, query, srv_.selectSubnet(query))); } +void +Dhcpv4SrvTest::pretendReceivingPkt(NakedDhcpv4Srv& srv, const std::string& config, + uint8_t pkt_type, const std::string& stat_name) { + + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + // Apply the configuration we just received. + configure(config); + + // Let's just use one of the actual captured packets that we have. + Pkt4Ptr pkt = isc::test::PktCaptures::captureRelayedDiscover(); + + // We just need to tweak it a it, to pretend that it's type is as desired. + // Note that when receiving a packet, its on-wire form is stored in data_ + // field. Most methods (including setType()) operates on option objects + // (objects stored in options_ after unpack() is called). Finally, outgoing + // packets are stored in out_buffer_. So we need to go through the full + // unpack/tweak/pack cycle and do repack, i.e. move the output buffer back + // to incoming buffer. + pkt->unpack(); + pkt->setType(pkt_type); // Set message type. + pkt->pack(); + pkt->data_.resize(pkt->getBuffer().getLength()); + // Copy out_buffer_ to data_ to pretend that it's what was just received. + memcpy(&pkt->data_[0], pkt->getBuffer().getData(), pkt->getBuffer().getLength()); + + // Simulate that we have received that traffic + srv.fakeReceive(pkt); + srv.run(); + + using namespace isc::stats; + StatsMgr& mgr = StatsMgr::instance(); + ObservationPtr pkt4_rcvd = mgr.getObservation("pkt4-received"); + ObservationPtr tested_stat = mgr.getObservation(stat_name); + + // All expected statstics must be present. + ASSERT_TRUE(pkt4_rcvd); + ASSERT_TRUE(tested_stat); + + // They also must have expected values. + EXPECT_EQ(1, pkt4_rcvd->getInteger().first); + EXPECT_EQ(1, tested_stat->getInteger().first); +} }; // end of isc::dhcp::test namespace }; // end of isc::dhcp namespace diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index f3ba99cedd..b147ab7c19 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -391,6 +391,20 @@ public: void configure(const std::string& config, NakedDhcpv4Srv& srv, const bool commit = true); + /// @brief Pretents a packet of specified type was received. + /// + /// Instantiates fake network interfaces, configures passed Dhcpv4Srv, + /// then creates a message of specified type and sends it to the + /// server and then checks whether expected statstics were set + /// appropriately. + /// + /// @param srv the DHCPv4 server to be used + /// @param config JSON configuration to be used + /// @param pkt_type type of the packet to be faked + /// @param stat_name name of the expected statistic + void pretendReceivingPkt(NakedDhcpv4Srv& srv, const std::string& config, + uint8_t pkt_type, const std::string& stat_name); + /// @brief Create @c Dhcpv4Exchange from client's query. Dhcpv4Exchange createExchange(const Pkt4Ptr& query); diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index 5435a2dad8..a9565b788d 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/src/lib/dhcp/pkt4.cc @@ -239,9 +239,9 @@ Pkt4::unpack() { // } (void)offset; - // @todo check will need to be called separately, so hooks can be called - // after the packet is parsed, but before its content is verified - check(); + // No need to call check() here. There are thorough tests for this + // later (see Dhcp4Srv::accept()). We want to drop the packet later, + // so we'll be able to log more detailed drop reason. } void Pkt4::check() { @@ -272,8 +272,15 @@ uint8_t Pkt4::getType() const { void Pkt4::setType(uint8_t dhcp_type) { OptionPtr opt = getOption(DHO_DHCP_MESSAGE_TYPE); if (opt) { + // There is message type option already, update it - opt->setUint8(dhcp_type); + boost::shared_ptr > type_opt = + boost::dynamic_pointer_cast >(opt); + if (type_opt) { + return (type_opt->setValue(dhcp_type)); + } else { + opt->setUint8(dhcp_type); + } } else { // There is no message type option yet, add it std::vector tmp(1, dhcp_type);