[3795] Changes after review:

 - Added several more unit-tests for pkt6-receive-drop
   (not all cases are really testable)
 - User's Guide cleanup
 - using namespace isc::stats added
 - Dhcp6Client::useServerId() implemented

doc/guide/dhcp4-srv.xml

@@ -2834,7 +2834,7 @@ temporarily override a list of interface names and listen on all interfaces.
               reset during reconfiguration event.</entry>
             </row>
             <row>
-              <entry><emphasis>subnet[id].assigned-addresses</emphasis></entry>
+              <entry>subnet[id].assigned-addresses</entry>
               <entry>integer</entry>
               <entry>This statistic shows the number of assigned addresses in a
               given subnet.  This statistic increases every time a new lease is

doc/guide/dhcp6-srv.xml

@@ -2613,6 +2613,26 @@ should include options from the isc option space:
             </row>
 
             <row>
+              <entry>pkt6-receive-drop</entry>
+              <entry>integer</entry>
+              <entry>Number of incoming packets that were dropped. Exact reason
+              for dropping packets is logged, but the most common reasons may
+              be: an unacceptable or not supported packet type, direct responses
+              are forbidden, the server-id sent by the client does not match the
+              server's server-id or the packet is malformed.</entry>
+            </row>
+
+            <row>
+              <entry>pkt6-parse-failed</entry>
+              <entry>integer</entry>
+              <entry>Number of incoming packets that could not be parsed.
+              A non-zero value of this statistic indicates that the server
+              received a malformed or truncated packet. This may indicate problems
+              in your network, faulty clients, faulty relay agents or server
+              code bug.</entry>
+            </row>
+
+            <row>
               <entry>pkt6-solicit-received</entry>
               <entry>integer</entry>
               <entry>
@@ -2682,10 +2702,10 @@ should include options from the isc option space:
               <entry>pkt6-release-received</entry>
               <entry>integer</entry>
               <entry>Number of RELEASE packets received. This statistic is expected
-              to grow every time a device is being shut down in the network. It
+              to grow when a device is being shut down in the network. It
               indicates that the address or prefix assigned is reported as no longer
-              needed. Note that in wireless networks, the number of RELEASE messages
+              needed. Note that many devices, especially wireless, do not send RELEASE,
-              is significantly lower than the number of REQUEST messages.
+              because of design choice or due to moving out of range.
               </entry>
             </row>
 
@@ -2701,7 +2721,7 @@ should include options from the isc option space:
               assigned conflicting addresses.
             </entry>
             </row>
-            
+
             <row>
               <entry>pkt6-infrequest-received</entry>
               <entry>integer</entry>
@@ -2754,26 +2774,6 @@ should include options from the isc option space:
               </entry>
             </row>
 
-            <row>
-              <entry>pkt6-parse-failed</entry>
-              <entry>integer</entry>
-              <entry>Number of incoming packets that could not be parsed.
-              A non-zero value of this statistic indicates that the server
-              received a malformed or truncated packet. This may indicate problems
-              in your network, faulty clients, faulty relay agents or server
-              code bug.</entry>
-            </row>
-            
-            <row>
-              <entry>pkt6-receive-drop</entry>
-              <entry>integer</entry>
-              <entry>Number of incoming packets that were dropped. Exact reason
-              for dropping packets is logged, but the most common reasons may
-              be: an unacceptable or not supported packet type, direct responses
-              are forbidden, the server-id sent by the client does not match the
-              server's server-id or the packet is malformed.</entry>
-            </row>
-
           </tbody>
           </tgroup>
         </table>

src/bin/dhcp6/dhcp6_srv.cc

@@ -74,6 +74,7 @@ using namespace isc::dhcp;
 using namespace isc::dhcp_ddns;
 using namespace isc::hooks;
 using namespace isc::log;
+using namespace isc::stats;
 using namespace isc::util;
 using namespace std;
 
@@ -335,8 +336,7 @@ bool Dhcpv6Srv::run() {
                 // 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("pkt6-received",
+                StatsMgr::instance().addValue("pkt6-received", static_cast<int64_t>(1));
-                                                          static_cast<int64_t>(1));
 
             } else {
                 LOG_DEBUG(packet_logger, DBG_DHCP6_DETAIL, DHCP6_BUFFER_WAIT_INTERRUPTED)
@@ -440,11 +440,11 @@ bool Dhcpv6Srv::run() {
                     .arg(query->getIface())
                     .arg(e.what());
 
-               // Increase the statistics of parse failures and dropped packets.
+                // Increase the statistics of parse failures and dropped packets.
-                isc::stats::StatsMgr::instance().addValue("pkt6-parse-failed",
+                StatsMgr::instance().addValue("pkt6-parse-failed",
-                                                          static_cast<int64_t>(1));
+                                              static_cast<int64_t>(1));
-                isc::stats::StatsMgr::instance().addValue("pkt6-receive-drop",
+                StatsMgr::instance().addValue("pkt6-receive-drop",
-                                                          static_cast<int64_t>(1));
+                                              static_cast<int64_t>(1));
                 continue;
             }
         }
@@ -457,8 +457,7 @@ bool Dhcpv6Srv::run() {
         if (!testServerID(query)) {
 
             // Increase the statistic of dropped packets.
-            isc::stats::StatsMgr::instance().addValue("pkt6-receive-drop",
+            StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
-                                                      static_cast<int64_t>(1));
             continue;
         }
 
@@ -468,8 +467,7 @@ bool Dhcpv6Srv::run() {
         if (!testUnicast(query)) {
 
             // Increase the statistic of dropped packets.
-            isc::stats::StatsMgr::instance().addValue("pkt6-receive-drop",
+            StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
-                                                      static_cast<int64_t>(1));
             continue;
         }
 
@@ -568,8 +566,7 @@ bool Dhcpv6Srv::run() {
                 .arg(e.what());
 
             // Increase the statistic of dropped packets.
-            isc::stats::StatsMgr::instance().addValue("pkt6-receive-drop",
+            StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
-                                                      static_cast<int64_t>(1));
 
         } catch (const isc::Exception& e) {
 
@@ -585,8 +582,7 @@ bool Dhcpv6Srv::run() {
                 .arg(e.what());
 
             // Increase the statistic of dropped packets.
-            isc::stats::StatsMgr::instance().addValue("pkt6-receive-drop",
+            StatsMgr::instance().addValue("pkt6-receive-drop", static_cast<int64_t>(1));
-                                                      static_cast<int64_t>(1));
         }
 
         if (rsp) {
@@ -2981,14 +2977,12 @@ void Dhcpv6Srv::processStatsReceived(const Pkt6Ptr& query) {
             ; // do nothing
     }
 
-    isc::stats::StatsMgr::instance().addValue(stat_name,
+    StatsMgr::instance().addValue(stat_name, static_cast<int64_t>(1));
-                                              static_cast<int64_t>(1));
 }
 
 void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
     // Increase generic counter for sent packets.
-    isc::stats::StatsMgr::instance().addValue("pkt6-sent",
+    StatsMgr::instance().addValue("pkt6-sent", static_cast<int64_t>(1));
-                                              static_cast<int64_t>(1));
 
     // Increase packet type specific counter for packets sent.
     string stat_name;
@@ -3004,8 +2998,7 @@ void Dhcpv6Srv::processStatsSent(const Pkt6Ptr& response) {
         return;
     }
 
-    isc::stats::StatsMgr::instance().addValue(stat_name,
+    StatsMgr::instance().addValue(stat_name, static_cast<int64_t>(1));
-                                              static_cast<int64_t>(1));
 }
 
 };

src/bin/dhcp6/tests/dhcp6_client.cc

@@ -327,7 +327,11 @@ Dhcp6Client::doSolicit() {
 void
 Dhcp6Client::doRequest() {
     Pkt6Ptr query = createMsg(DHCPV6_REQUEST);
-    query->addOption(context_.response_->getOption(D6O_SERVERID));
+    if (!forced_server_id_) {
+        query->addOption(context_.response_->getOption(D6O_SERVERID));
+    } else {
+        query->addOption(forced_server_id_);
+    }
     copyIAs(context_.response_, query);
 
     // Add Client FQDN if configured.

src/bin/dhcp6/tests/dhcp6_client.h

@@ -448,6 +448,17 @@ public:
         use_rapid_commit_ = rapid_commit;
     }
 
+    /// @brief Specifies server-id to be used in send messages
+    ///
+    /// Overrides the server-id to be sent when server-id is expected to be
+    /// sent. May be NULL, which means use proper server-id sent in Advertise
+    /// (which is a normal client behavior).
+    ///
+    /// @param server_id server-id to be sent
+    void useServerId(const OptionPtr& server_id) {
+        forced_server_id_ = server_id;
+    }
+
     /// @brief Creates an instance of the Client FQDN option to be included
     /// in the client's message.
     ///
@@ -614,6 +625,9 @@ private:
     /// to true. See @ref sendORO for details.
     std::vector<uint16_t> oro_;
 
+    /// @brief forced (Overridden) value of the server-id option (may be NULL)
+    OptionPtr forced_server_id_;
+
     /// @brief FQDN requested by the client.
     Option6ClientFqdnPtr fqdn_;
 };

src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -2336,7 +2336,7 @@ TEST_F(Dhcpv6SrvTest, receiveAdvertiseStat) {
 // Note that in properly configured network the server never receives Reply
 // messages.
 TEST_F(Dhcpv6SrvTest, receiveReplyStat) {
-    testReceiveStats(DHCPV6_ADVERTISE, "pkt6-advertise-received");
+    testReceiveStats(DHCPV6_REPLY, "pkt6-reply-received");
 }
 
 // Test checks if pkt6-unknown-received is bumped up correctly.
@@ -2407,6 +2407,7 @@ TEST_F(Dhcpv6SrvTest, receiveParseFailedStat) {
     EXPECT_EQ(1, recv_drop->getInteger().first);
 }
 
+
 /// @todo: Add more negative tests for processX(), e.g. extend sanityCheck() test
 /// to call processX() methods.
 

src/bin/dhcp6/tests/sarr_unittest.cc

@@ -19,6 +19,7 @@
 #include <dhcp6/tests/dhcp6_client.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/d2_client_mgr.h>
+#include <asiolink/io_address.h>
 #include <stats/stats_mgr.h>
 
 using namespace isc;
@@ -342,4 +343,58 @@ TEST_F(SARRTest, sarrStats) {
     EXPECT_EQ(2, pkt6_sent->getInteger().first);
 }
 
+// This test verifies that pkt6-receive-drop is increased properly when the
+// client's packet is rejected due to mismatched server-id value.
+TEST_F(SARRTest, pkt6ReceiveDropStat1) {
+
+    // Dummy server-id (0xff repeated 10 times)
+    std::vector<uint8_t> data(10, 0xff);
+    OptionPtr bogus_srv_id(new Option(Option::V6, D6O_SERVERID, data));
+
+    // Let's use one of the existing configurations and tell the client to
+    // ask for an address.
+    Dhcp6Client client;
+    configure(CONFIGS[1], *client.getServer());
+    client.setInterface("eth1");
+    client.useNA();
+
+    client.doSolicit();
+    client.useServerId(bogus_srv_id);
+    client.doRequest();
+
+    // Ok, let's check the statistics. None should be present.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+
+    ObservationPtr pkt6_recv_drop = mgr.getObservation("pkt6-receive-drop");
+    ASSERT_TRUE(pkt6_recv_drop);
+
+    EXPECT_EQ(1, pkt6_recv_drop->getInteger().first);
+}
+
+// This test verifies that pkt6-receive-drop is increased properly when the
+// client's packet is rejected due to sending to unicast.
+TEST_F(SARRTest, pkt6ReceiveDropStat2) {
+
+    // Let's use one of the existing configurations and tell the client to
+    // ask for an address.
+    Dhcp6Client client;
+    configure(CONFIGS[1], *client.getServer());
+    client.setInterface("eth1");
+    client.useNA();
+
+    client.setDestAddress(asiolink::IOAddress("2001:db8::1")); // Pretend it's unicast
+    client.doSolicit();
+
+    // Ok, let's check the statistics. None should be present.
+    using namespace isc::stats;
+    StatsMgr& mgr = StatsMgr::instance();
+
+    ObservationPtr pkt6_recv_drop = mgr.getObservation("pkt6-receive-drop");
+    ASSERT_TRUE(pkt6_recv_drop);
+
+    EXPECT_EQ(1, pkt6_recv_drop->getInteger().first);
+}
+
+
 } // end of anonymous namespace