[3981] Changes after review:

 - Explanation why are we not updating assigned-addresses
 - DHCP4_DECLINE_LEASE message reworded as suggested
 - processDecline method described properly
 - declineNonMatchingIPAddresses description updated
 - global declined-addresses statistic added and tested
 -

doc/guide/dhcp4-srv.xml

@@ -2718,6 +2718,27 @@ It is merely echoed by the server
 
     </section>
 
+    <section id="dhcp4-decline">
+      <title>Duplicate Addresses (DHCPDECLINE support)</title>
+      <note>
+        <para>@todo: Full text will be added as part of #3990.</para>
+      </note>
+
+      <para>
+        The server does not decrease assigned-addresses statistics
+        when DHCPDECLINE is received and processed successfully. While
+        technically declined address is not assigned anymore, the primary usage
+        of assigned-addresses statistic is to monitor pool utilization. Most
+        people would forget to include declined-addresses in the calculation,
+        and simply do assigned-addresses/total-addresses. This would have a bias
+        towards under-representing pool utilization. As this has a
+        potential for major issues, we decided to decrease assigned
+        addresses not immediately after receiving DHCPDECLINE, but do
+        it later when we recover the address back to the available pool.
+      </para>
+
+    </section>
+
     <section id="dhcp4-stats">
       <title>Statistics in DHCPv4 server</title>
       <note>

src/bin/dhcp4/dhcp4_messages.mes

@@ -177,7 +177,7 @@ This message is printed when DHCPv4 server disables an interface from being
 used to receive DHCPv4 traffic. Sockets on this interface will not be opened
 by the Interface Manager until interface is enabled.
 
-% DHCP4_DECLINE_LEASE Address %1 was declined by client %2. The lease will be unavailable for %3 seconds.
+% DHCP4_DECLINE_LEASE Received DHCPDECLINE for addr %1 from client %2. The lease will be unavailable for %3 seconds.
 This informational message is printed when a client received an address, but
 discovered that it is being used by some other device and notified the server by
 sending a DHCPDECLINE message. The server checked that this address really was

src/bin/dhcp4/dhcp4_srv.cc

@@ -1926,9 +1926,23 @@ Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const std::string& descr) {
     }
 
     // Bump up the statistics.
-    std::stringstream name;
-    name << "subnet[" << lease->subnet_id_ << "].declined-addresses";
-    isc::stats::StatsMgr::instance().addValue(name.str(), static_cast<int64_t>(1));
+
+    // Per subnet declined addresses counter.
+    StatsMgr::instance().addValue(
+        StatsMgr::generateName("subnet", lease->subnet_id_, "declined-addresses"),
+        static_cast<int64_t>(1));
+
+    // Global declined addresses counter.
+    StatsMgr::instance().addValue("declined-addresses", static_cast<int64_t>(1));
+
+    // We do not want to decrease the assigned-addresses at this time. While
+    // technically declined address is not allocated anymore, the primary usage
+    // of assigned-addresses statistic is to monitor pool utilization. Most
+    // people would forget to include declined-addresses in the calculation,
+    // and simply do assigned-addresses/total-addresses. This would have a bias
+    // towards under-representing pool utilization, if we decreased allocated
+    // immediately after receiving DHCPDECLINE, rather than later when we recover
+    // the address.
 
     // @todo: Call hooks.
 

src/bin/dhcp4/dhcp4_srv.h

@@ -402,7 +402,11 @@ protected:
     /// @param release message received from client
     void processRelease(Pkt4Ptr& release);
 
-    /// @brief Stub function that will handle incoming DHCPDECLINE messages.
+    /// @brief Process incoming DHCPDECLINE messages.
+    ///
+    /// This method processes incoming DHCPDECLINE. In particular, it extracts
+    /// Requested IP Address option, check that the address really belongs to
+    /// the client and if it does, calls @ref declineLease.
     ///
     /// @param decline message received from client
     void processDecline(Pkt4Ptr& decline);

src/bin/dhcp4/tests/decline_unittest.cc

@@ -145,9 +145,13 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
     std::stringstream name;
     name << "subnet[" << subnets->at(0)->getID() << "].declined-addresses";
 
-    // Set this statistic explicitly to zero.
+    // Set the subnet specific statistic explicitly to zero.
     isc::stats::StatsMgr::instance().setValue(name.str(), static_cast<int64_t>(0));
 
+    // Set this global statistic explicitly to zero.
+    isc::stats::StatsMgr::instance().setValue("declined-addresses",
+                                              static_cast<int64_t>(0));
+
     // Ok, do the normal lease aquisition.
     CfgMgr::instance().clear();
     Dhcp4Client client(Dhcp4Client::SELECTING);
@@ -160,11 +164,17 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
     // Perform 4-way exchange to obtain a new lease.
     acquireLease(client);
 
-    // Check the delined-addresses statistic before the Decline operation.
+    // Check the declined-addresses (subnet) statistic before the Decline operation.
     ObservationPtr declined_cnt = StatsMgr::instance().getObservation(name.str());
     ASSERT_TRUE(declined_cnt);
     uint64_t before = declined_cnt->getInteger().first;
 
+    // Check the global declined-addresses statistic before the Decline.
+    ObservationPtr declined_global = StatsMgr::instance()
+        .getObservation("declined-addresses");
+    ASSERT_TRUE(declined_global);
+    uint64_t before_global = declined_cnt->getInteger().first;
+
     // Remember the acquired address.
     IOAddress declined_address = client.config_.lease_.addr_;
 
@@ -182,6 +192,10 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
     ASSERT_TRUE(declined_cnt);
     uint64_t after = declined_cnt->getInteger().first;
 
+    declined_global = StatsMgr::instance().getObservation("declined-addresses");
+    ASSERT_TRUE(declined_global);
+    uint64_t after_global = declined_global->getInteger().first;
+
     ASSERT_TRUE(lease);
     // We check if the deline process was successful by checking if the
     // lease is in the database and what is its state.
@@ -191,6 +205,8 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
         // The decline succeded, so the declined-addresses statistic should
         // be increased by one
         EXPECT_EQ(after, before + 1);
+
+        EXPECT_EQ(after_global, before_global + 1);
     } else {
         // the decline was supposed, to be rejected.
         EXPECT_EQ(Lease::STATE_DEFAULT, lease->state_);
@@ -198,6 +214,7 @@ DeclineTest::acquireAndDecline(const std::string& hw_address_1,
         // The decline failed, so the declined-addresses should be the same
         // as before
         EXPECT_EQ(before, after);
+        EXPECT_EQ(before_global, after_global);
     }
 }
 
@@ -266,9 +283,9 @@ TEST_F(DeclineTest, declineNonMatchingHWAddress) {
 }
 
 // This test checks the server's behavior in the following case:
-// - Client acquires new lease.
-// - Client sends DHCPDECLINE with the ciaddr set to a different
-//   address than it has acquired from the server.
+// - Client acquires new lease (address A).
+// - Client sends DHCPDECLINE with the requested IP address set to a different
+//   address B than it has acquired from the server.
 // - Server determines that the client is trying to decline a
 //   wrong address and will refuse to decline.
 TEST_F(DeclineTest, declineNonMatchingIPAddress) {