[4109a] Addressed comments

diff --git a/doc/guide/dhcp6-srv.xml b/doc/guide/dhcp6-srv.xml
index 5eec3e79da059089673c782147795f5821145d17..0c9766f656d23925ea607e8b498952bfe01eaab2 100644 (file)
--- a/doc/guide/dhcp6-srv.xml
+++ b/doc/guide/dhcp6-srv.xml
@@ -3279,6 +3279,33 @@ should include options from the isc option space:
               </entry>
             </row>
 
+            <row>
+              <entry>pkt6-dhcpv4-query-received</entry>
+              <entry>integer</entry>
+              <entry>
+                Number of DHCPv4-QUERY packets received. This
+                statistic is expected to grow if there are devices
+                that are using DHCPv4-over-DHCPv6. DHCPv4-QUERY
+                messages are used by DHCPv4 clients on an IPv6 only
+                line so that encapsulate requests over DHCPv6.
+              </entry>
+            </row>
+
+            <row>
+              <entry>pkt6-dhcpv4-response-received</entry>
+              <entry>integer</entry>
+              <entry>
+                Number of DHCPv4-RESPONSE packets received. This
+                statistic is expected to remain zero at all times, as
+                DHCPv4-RESPONSE packets are sent by the server and the
+                server is never expected to receive them. A non-zero
+                value indicates an error.  One likely cause would be a
+                misbehaving relay agent that incorrectly forwards
+                DHCPv4-RESPONSE message towards the server, rather
+                back to the clients.
+              </entry>
+            </row>
+
             <row>
               <entry>pkt6-unknown-received</entry>
               <entry>integer</entry>
@@ -3319,6 +3346,16 @@ should include options from the isc option space:
               </entry>
             </row>
 
+            <row>
+              <entry>pkt6-dhcpv4-response-sent</entry>
+              <entry>integer</entry>
+              <entry>Number of DHCPv4-RESPONSE packets sent. This
+              statistic is expected to grow in most cases after a
+              DHCPv4-QUERY is processed. There are certain cases where
+              there is no response.
+              </entry>
+            </row>
+
             <row>
             <entry>subnet[id].total-nas</entry>
             <entry>integer</entry>

diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index aa65cd13bc96a447d071b93a9bc454215cb628ad..136b8432b3c9220f6d5f2936f8f2f49c330011a0 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -165,8 +165,6 @@ createStatusCode(const Pkt6& pkt, const Option6IA& ia, const uint16_t status_cod
 namespace isc {
 namespace dhcp {
 
-int Dhcpv6Srv::hook_index_buffer6_send = Hooks.hook_index_buffer6_send_;
-
 const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
 Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
@@ -3092,6 +3090,13 @@ void Dhcpv6Srv::processStatsReceived(const Pkt6Ptr& query) {
     case DHCPV6_INFORMATION_REQUEST:
         stat_name = "pkt6-infrequest-received";
         break;
+    case DHCPV6_DHCPV4_QUERY:
+        stat_name = "pkt6-dhcpv4-query-received";
+        break;
+    case DHCPV6_DHCPV4_RESPONSE:
+        // Should not happen, but let's keep a counter for it
+        stat_name = "pkt6-dhcpv4-response-received";
+        break;
     default:
             ; // do nothing
     }
@@ -3112,6 +3117,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
     case DHCPV6_REPLY:
         stat_name = "pkt6-reply-sent";
         break;
+    case DHCPV6_DHCPV4_RESPONSE:
+        stat_name = "pkt6-dhcpv4-response-sent";
+        break;
     default:
         // That should never happen
         return;
@@ -3120,5 +3128,9 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
     StatsMgr::instance().addValue(stat_name, static_cast<int64_t>(1));
 }
 
+int Dhcpv6Srv::getHookIndexBuffer6Send() {
+    return (Hooks.hook_index_buffer6_send_);
+}
+
 };
 };

diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index 63847e85b8e4ae13f118b51590a479c7ddb1e96a..6521e9173305d65fde4b72d130073da196a100b9 100644 (file)
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -292,7 +292,9 @@ protected:
     /// @brief Processes incoming DHCPv4-query message.
     ///
     /// @param dhcp4_query message received from client
-    /// @return Reply (empty) message to (not) be sent to the client.
+    /// @return Reply message to be sent to the client (always NULL
+    /// as the response will be returned after by
+    /// @ref isc::dhcp::Dhcp6to4Ipc::handler()).
     Pkt6Ptr processDhcp4Query(const Pkt6Ptr& dhcp4_query);
 
     /// @brief Selects a subnet for a given client's packet.
@@ -791,14 +793,15 @@ private:
     uint16_t port_;
 
 public:
-    /// @note used by DHCPv4-over-DHCPv6 so must be public
+    /// @note used by DHCPv4-over-DHCPv6 so must be public and static
 
     /// @brief Updates statistics for transmitted packets
     /// @param response packet transmitted
     static void processStatsSent(const Pkt6Ptr& response);
 
-    /// the index of the buffer6_send hook
-    static int hook_index_buffer6_send;
+    /// @brief Returns the index of the buffer6_send hook
+    /// @return the index of the buffer6_send hook
+    static int getHookIndexBuffer6Send();
 
 protected:
 

diff --git a/src/bin/dhcp6/dhcp6to4_ipc.cc b/src/bin/dhcp6/dhcp6to4_ipc.cc
index 95516e965dbe67f82bbc9bd163a14e78c38e2228..fbc691da8fc60c07c8bbd9587b5070381d4806fc 100644 (file)
--- a/src/bin/dhcp6/dhcp6to4_ipc.cc
+++ b/src/bin/dhcp6/dhcp6to4_ipc.cc
@@ -85,7 +85,8 @@ void Dhcp6to4Ipc::handler() {
     buf.clear();
     pkt->pack();
 
-    // Don't use getType(): get the message type from the buffer
+    // Don't use getType(): get the message type from the buffer as we
+    // want to know if it is a relayed message (vs. internal message type).
     uint8_t msg_type = buf[0];
     if ((msg_type == DHCPV6_RELAY_FORW) || (msg_type == DHCPV6_RELAY_REPL)) {
         pkt->setRemotePort(DHCP6_SERVER_PORT);
@@ -99,7 +100,7 @@ void Dhcp6to4Ipc::handler() {
 
     try {
         // Let's execute all callouts registered for buffer6_send
-        if (HooksManager::calloutsPresent(Dhcpv6Srv::hook_index_buffer6_send)) {
+      if (HooksManager::calloutsPresent(Dhcpv6Srv::getHookIndexBuffer6Send())) {
             CalloutHandlePtr callout_handle = getCalloutHandle(pkt);
 
             // Delete previously set arguments
@@ -109,7 +110,7 @@ void Dhcp6to4Ipc::handler() {
             callout_handle->setArgument("response6", pkt);
 
             // Call callouts
-            HooksManager::callCallouts(Dhcpv6Srv::hook_index_buffer6_send,
+            HooksManager::callCallouts(Dhcpv6Srv::getHookIndexBuffer6Send(),
                                        *callout_handle);
 
             // Callouts decided to skip the next processing step. The next

diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index cf81f9569eaccb729c396403c5a509c51f5a5fa7..9d9342814032f34fe709eaf22f985ad0eff04dca 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -2585,6 +2585,13 @@ TEST_F(Dhcpv6SrvTest, receiveReplyStat) {
     testReceiveStats(DHCPV6_REPLY, "pkt6-reply-received");
 }
 
+// Test checks if pkt6-dhcpv4-response-received is bumped up correctly.
+// Note that in properly configured network the server never receives
+// Dhcpv4-Response messages.
+TEST_F(Dhcpv6SrvTest, receiveDhcpv4ResponseStat) {
+    testReceiveStats(DHCPV6_DHCPV4_RESPONSE, "pkt6-dhcpv4-response-received");
+}
+
 // Test checks if pkt6-unknown-received is bumped up correctly.
 TEST_F(Dhcpv6SrvTest, receiveUnknownStat) {
     testReceiveStats(123, "pkt6-unknown-received");
@@ -2610,6 +2617,11 @@ TEST_F(Dhcpv6SrvTest, receiveDeclineStat) {
     testReceiveStats(DHCPV6_DECLINE, "pkt6-decline-received");
 }
 
+// Test checks if pkt6-dhcpv4-query-received is bumped up correctly.
+TEST_F(Dhcpv6SrvTest, receiveDhcpv4QueryStat) {
+    testReceiveStats(DHCPV6_DHCPV4_QUERY, "pkt6-dhcpv4-query-received");
+}
+
 // Test checks if reception of a malformed packet increases pkt-parse-failed
 // and pkt6-receive-drop
 TEST_F(Dhcpv6SrvTest, receiveParseFailedStat) {

diff --git a/src/bin/dhcp6/tests/dhcp6to4_ipc_unittest.cc b/src/bin/dhcp6/tests/dhcp6to4_ipc_unittest.cc
index 905a9d8714a43489d29ef7fe459b01174e26f9c5..8fada87cc5c8442a4efecc977789a54752a89a2b 100644 (file)
--- a/src/bin/dhcp6/tests/dhcp6to4_ipc_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6to4_ipc_unittest.cc
@@ -14,6 +14,7 @@
 #include <dhcp6/dhcp6to4_ipc.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/testutils/dhcp4o6_test_ipc.h>
+#include <stats/stats_mgr.h>
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_manager.h>
 #include <hooks/library_handle.h>
@@ -25,6 +26,7 @@ using namespace isc;
 using namespace isc::asiolink;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
+using namespace isc::stats;
 using namespace isc::hooks;
 using namespace isc::util;
 
@@ -51,6 +53,8 @@ public:
         // Install buffer6_send_callout
         EXPECT_NO_THROW(HooksManager::preCalloutsLibraryHandle().
                         registerCallout("buffer6_send", buffer6_send_callout));
+        // Let's wipe all existing statistics.
+        StatsMgr::instance().removeAll();
     }
 
     /// @brief Configure DHCP4o6 port.
@@ -128,8 +132,15 @@ TEST_F(Dhcp6to4IpcTest, receive) {
     ASSERT_NO_THROW(ipc.open());
     ASSERT_NO_THROW(src_ipc.open());
 
+    // Get statistics
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt6_snd = mgr.getObservation("pkt6-sent");
+    ObservationPtr d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
+    EXPECT_FALSE(pkt6_snd);
+    EXPECT_FALSE(d4_resp);
+
     // Create message to be sent over IPC.
-    Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_QUERY, 1234));
+    Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_RESPONSE, 1234));
     pkt->addOption(createDHCPv4MsgOption());
     pkt->setIface("eth0");
     pkt->setRemoteAddr(IOAddress("2001:db8:1::123"));
@@ -152,13 +163,20 @@ TEST_F(Dhcp6to4IpcTest, receive) {
     EXPECT_TRUE(forwarded->getOption(D6O_DHCPV4_MSG));
     EXPECT_EQ("eth0", forwarded->getIface());
     EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText());
+
+    // Verify statistics
+    pkt6_snd = mgr.getObservation("pkt6-sent");
+    d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
+    ASSERT_TRUE(pkt6_snd);
+    ASSERT_TRUE(d4_resp);
+    EXPECT_EQ(1, pkt6_snd->getInteger().first);
+    EXPECT_EQ(1, d4_resp->getInteger().first);
 }
 
-// #4296 addresses this
-#if 0
 // This test verifies that the DHCPv4 endpoint of the DHCPv4o6 IPC can
 // receive relayed messages.
-TEST_F(Dhcp6to4IpcTest, receiveRelayed) {
+// This is currently not supported: it is a known defect addressed by #4296.
+TEST_F(Dhcp6to4IpcTest, DISABLED_receiveRelayed) {
     // Create instance of the IPC endpoint under test.
     Dhcp6to4Ipc& ipc = Dhcp6to4Ipc::instance();
     // Create instance of the IPC endpoint being used as a source of messages.
@@ -168,8 +186,15 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) {
     ASSERT_NO_THROW(ipc.open());
     ASSERT_NO_THROW(src_ipc.open());
 
+    // Get statistics
+    StatsMgr& mgr = StatsMgr::instance();
+    ObservationPtr pkt6_snd = mgr.getObservation("pkt6-sent");
+    ObservationPtr d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
+    EXPECT_FALSE(pkt6_snd);
+    EXPECT_FALSE(d4_resp);
+
     // Create relayed message to be sent over IPC.
-    Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_QUERY, 1234));
+    Pkt6Ptr pkt(new Pkt6(DHCPV6_DHCPV4_RESPONSE, 1234));
     pkt->addOption(createDHCPv4MsgOption());
     Pkt6::RelayInfo relay;
     relay.linkaddr_ = IOAddress("3000:1::1");
@@ -199,7 +224,14 @@ TEST_F(Dhcp6to4IpcTest, receiveRelayed) {
     EXPECT_EQ("eth0", forwarded->getIface());
     EXPECT_EQ("2001:db8:1::123", forwarded->getRemoteAddr().toText());
     EXPECT_EQ(DHCP6_CLIENT_PORT, forwarded->getRemotePort());
+
+    // Verify statistics
+    pkt6_snd = mgr.getObservation("pkt6-sent");
+    d4_resp = mgr.getObservation("pkt6-dhcpv4-response-sent");
+    ASSERT_TRUE(pkt6_snd);
+    ASSERT_TRUE(d4_resp);
+    EXPECT_EQ(1, pkt6_snd->getInteger().first);
+    EXPECT_EQ(1, d4_resp->getInteger().first);
 }
-#endif
 
 } // end of anonymous namespace