[5705] Changes after review

- doc updated
 - mostly comments

doc/guide/dhcp4-srv.xml

@@ -3541,7 +3541,7 @@ It is merely echoed by the server
       to skip reservation checks when dealing with existing leases. Therefore,
       system administrators are encouraged to use out-of-pool reservations if
       possible.</para>
-      <para>Beginning, with Kea 1.5.0, there is now support for global
+      <para>Beginning with Kea 1.5.0, there is now support for global
       host reservations.  These are reservations that are specified at the
       global level within the configuration and that do not belong to any
       specific subnet.  Kea will still match inbound client packets to a
@@ -3553,7 +3553,8 @@ It is merely echoed by the server
       </para>
       <note>You can reserve any ip-address in a global reservation. Just keep
       in mind that Kea will not do any sanity checking on the address and for
-      Kea 1.5.0, support for this should be considered experimental.
+      Kea 1.5.0, support for global reservation mechanism should be
+      considered experimental.
       </note>
     </section>
 
@@ -3632,6 +3633,15 @@ It is merely echoed by the server
       out-of-pool reservations. If the reserved address does not belong to a
       pool, there is no way that other clients could get this address.
       </para>
+
+    <note>
+     <para>The conflict resolution mechanism does not work for global
+     reservations. As of Kea 1.5.0, it is generally recommended to not use
+     global reservations for addresses. If you want to use it anyway,
+     you have to manually ensure that the reserved addressed are non
+     in the dynamic pools.</para>
+    </note>
+
     </section>
 
     <section xml:id="reservation4-hostname">
@@ -3901,7 +3911,7 @@ It is merely echoed by the server
       <listitem><simpara> <command>all</command> - enables both in-pool
       and out-of-pool host reservation types. This is the default value. This
       setting is the safest and the most flexible. As all checks are conducted,
-      it is also the slowest.
+      it is also the slowest. This does not check against global reservations.
       </simpara></listitem>
 
       <listitem><simpara> <command>out-of-pool</command> - allows only out of
@@ -3981,7 +3991,7 @@ It is merely echoed by the server
     ]
 }
 </screen>
-
+  For more details regarding global erservations, see <xref linkend="global-reservations4"/>.
 
       </para>
 
@@ -4029,7 +4039,84 @@ If not specified, the default value is:
 src/lib/dhcpsrv/cfg_host_operations.cc -->
 
 </para>
-   </section>
+</section>
+
+
+  <section id="global-reservations4">
+    <title>Global reservations in DHCPv4</title>
+
+    <para>In some deployments, such as mobile, clients can roam within the
+    network and there is a desire to specify certain parameters regardless of
+    the client's current location. To facilitate such a need, a global
+    reservation mechanism has been implemented. The idea behind it is that
+    regular host reservations are tied to specific subnets, by using specific
+    subnet-id. Kea 1.5.0 introduced a new capability to specify global
+    reservation that can be used in every subnet that has global reservations
+    enabled.</para>
+
+    <para>This feature can be used to assign certain parameters, such as
+    hostname or some dedicated, host specific options. It can also be used to
+    assign addresses. However, global reservations that assign addresses bypass
+    the whole topology determination provided by DHCP logic implemented in
+    Kea. It is very easy to misuse this feature and get configuration that is
+    inconsistent. To give specific example, imagine a case of global reservation
+    for address 192.0.2.100 and two subnets 192.0.2.0/24 and 192.0.5.0/24. If
+    global reservations are used in both subnets and a device matching global
+    host reservations visits part of the network that is serviced by
+    192.0.5.0/24, it will get an IP address 192.0.2.100, a subnet 192.0.5.0 and
+    a default router 192.0.5.1. Obviously such a configuration is unusable, as
+    the client won't be able to reach its default gateway.</para>
+
+    <para>
+      To use global host reservations a configuration similar to the following
+      can be used:
+
+<screen>
+"Dhcp4:" {
+    // This specify global reservations. They will apply to all subnets that
+    // have global reservations enabled.
+    <userinput>
+    "reservations": [
+    {
+       "hw-address": "aa:bb:cc:dd:ee:ff",
+       "hostname": "hw-host-dynamic"
+    },
+    {
+       "hw-address": "01:02:03:04:05:06",
+       "hostname": "hw-host-fixed",
+
+       // Use of IP address is global reservation is risky. If used outside of
+       // matching subnet, such as 192.0.1.0/24, it will result in a broken
+       // configuration being handled to the client.
+       "ip-address": "192.0.1.77"
+    },
+    {
+       "duid": "01:02:03:04:05",
+       "hostname": "duid-host"
+    },
+    {
+       "circuit-id": "'charter950'",
+       "hostname": "circuit-id-host"
+    },
+    {
+       "client-id": "01:11:22:33:44:55:66",
+       "hostname": "client-id-host"
+    }
+    ]</userinput>,
+    "valid-lifetime": 600,
+    "subnet4": [ {
+        "subnet": "10.0.0.0/24",
+        <userinput>"reservation-mode": "global",</userinput>
+        "pools": [ { "pool": "10.0.0.10-10.0.0.100" } ]
+    } ]
+}
+</screen>
+    </para>
+
+    <para>When using database backends, the global host reservations are
+    distinguished from regular reservations by using subnet-id value of
+    zero.</para>
+  </section> <!-- end of global reservations -->
 
   </section>
   <!-- end of host reservations section -->

src/bin/dhcp4/tests/dhcp4_test_utils.cc

@@ -611,6 +611,9 @@ Dhcpv4SrvTest::configure(const std::string& config, NakedDhcpv4Srv& srv,
         FAIL() << "parsing failure:"
                 << "config:" << config << std::endl
                 << "error: " << ex.what();
+
+        // No point in going deeper into the swamp...
+        return;
     }
 
     ConstElementPtr status;

src/bin/dhcp4/tests/host_unittest.cc

@@ -32,12 +32,12 @@ namespace {
 /// @brief Set of JSON configuration(s) used throughout the Host tests.
 ///
 /// - Configuration 0:
-///   - Used for testing global host reservations 
+///   - Used for testing global host reservations
-///   - 5 global reservations 
+///   - 5 global reservations
 ///   - 1 subnet: 10.0.0.0/24
 const char* CONFIGS[] = {
     // Configuration 0
-    // 1 subnet, mode HR_GLOBAL, 
+    // 1 subnet, mode HR_GLOBAL,
     // global reservations for different identifier types
     "{ \"interfaces-config\": {\n"
         "      \"interfaces\": [ \"*\" ]\n"
@@ -75,7 +75,7 @@ const char* CONFIGS[] = {
         "} ]\n"
     "}\n"
     ,
-    // Configuration 1 global vs in-pool 
+    // Configuration 1 global vs in-pool
     // 2 subnets, one mode default (aka HR_ALL), one mode HR_GLOBAL
     // Host reservations for the same client, one global, one in each subnet
     "{ \"interfaces-config\": {\n"
@@ -143,7 +143,7 @@ const char* CONFIGS[] = {
         "]\n"
         "}\n"
     ,
-    // Configuration 3 global and all 
+    // Configuration 3 global and all
     "{ \"interfaces-config\": {\n"
         "      \"interfaces\": [ \"*\" ]\n"
         "},\n"
@@ -171,7 +171,7 @@ const char* CONFIGS[] = {
         "}\n"
 };
 
-/// @brief Test fixture class for testing v4 exchanges.
+/// @brief Test fixture class for testing global v4 reservations.
 class HostTest : public Dhcpv4SrvTest {
 public:
 
@@ -198,8 +198,17 @@ public:
     /// @brief Interface Manager's fake configuration control.
     IfaceMgrTestConfig iface_mgr_test_config_;
 
-    void runDoraTest(const std::string& config, Dhcp4Client& client, 
+    /// @brief Conducts DORA exchange and checks assigned address and hostname
-                     const std::string& expected_host, 
+    ///
+    /// If expected_host is empty, the test expects the hostname option to not
+    /// be assigned.
+    ///
+    /// @param config configuration to be used
+    /// @param client reference to a client instance
+    /// @param expected_host expected hostname to be assigned (may be empty)
+    /// @param expected_addr expected address to be assigned
+    void runDoraTest(const std::string& config, Dhcp4Client& client,
+                     const std::string& expected_host,
                      const std::string& expected_addr) {
 
         // Configure DHCP server.
@@ -231,7 +240,7 @@ public:
         EXPECT_EQ(client.config_.lease_.addr_.toText(), expected_addr);
     }
 
-    
+
 };
 
 // Verifies that a client, which fails to match to a global
@@ -243,8 +252,7 @@ TEST_F(HostTest, globalHardwareNoMatch) {
     runDoraTest(CONFIGS[0], client, "", "10.0.0.10");
 }
 
-
+// Verifies that a client, that matches to a global hostname
-// Verifies that a client, that matches to a global hostname 
 // reservation, gets both the hostname and a dynamic address,
 // when the subnet mode is HR_GLOBAL
 TEST_F(HostTest, globalHardwareDynamicAddress) {
@@ -254,9 +262,8 @@ TEST_F(HostTest, globalHardwareDynamicAddress) {
     runDoraTest(CONFIGS[0], client, "hw-host-dynamic", "10.0.0.10");
 }
 
-
 // Verifies that a client matched to a global address reservation
-// reservation, gets both the hostname and the reserved address
+// gets both the hostname and the reserved address
 // when the subnet mode is HR_GLOBAL
 TEST_F(HostTest, globalHardwareFixedAddress) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
@@ -266,7 +273,7 @@ TEST_F(HostTest, globalHardwareFixedAddress) {
     runDoraTest(CONFIGS[0], client, "hw-host-fixed", "192.0.1.77");
 }
 
-// Verifies that a client, can be matched to a global reservation by DUID
+// Verifies that a client can be matched to a global reservation by DUID
 TEST_F(HostTest, globalDuid) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
 
@@ -281,7 +288,7 @@ TEST_F(HostTest, globalDuid) {
     runDoraTest(CONFIGS[0], client, "duid-host", "10.0.0.10");
 }
 
-// Verifies that a client, can be matched to a global reservation by ciruit-id 
+// Verifies that a client can be matched to a global reservation by circuit-id
 TEST_F(HostTest, globalCircuitId) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
 
@@ -297,7 +304,7 @@ TEST_F(HostTest, globalCircuitId) {
     runDoraTest(CONFIGS[0], client, "circuit-id-host", "10.0.0.10");
 }
 
-// Verifies that a client, can be matched to a global reservation by client-id 
+// Verifies that a client can be matched to a global reservation by client-id
 TEST_F(HostTest, globalClientID) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
 
@@ -312,15 +319,15 @@ TEST_F(HostTest, globalClientID) {
 }
 
 // Verifies that even when a matching global reservation exists,
-// client will get a subnet scoped reservation, when subnet 
+// client will get a subnet scoped reservation, when subnet
 // reservation mode is default
 TEST_F(HostTest, defaultOverGlobal) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
-   
+
-    // Hardware address matches all reservations 
+    // Hardware address matches all reservations
     client.setHWAddress("aa:bb:cc:dd:ee:ff");
 
-    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its 
+    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its
     // reservation should be used, rather than global.
     runDoraTest(CONFIGS[1], client, "subnet-10-host", "10.0.0.10");
 }
@@ -328,11 +335,11 @@ TEST_F(HostTest, defaultOverGlobal) {
 // Verifies that when there are matching reservations at
 // both the global and subnet levels, client will be matched
 // to the global reservation, when subnet reservation mode
-// is HR_GLOBAL. 
+// is HR_GLOBAL.
 TEST_F(HostTest, globalOverSubnet) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
-   
+
-    // Hardware address matches all reservations 
+    // Hardware address matches all reservations
     client.setHWAddress("aa:bb:cc:dd:ee:ff");
 
     // Change to subnet 20
@@ -349,11 +356,11 @@ TEST_F(HostTest, globalOverSubnet) {
 // is HR_OUT_OF_POOL
 TEST_F(HostTest, outOfPoolOverGlobal) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
-   
+
-    // Hardware address matches all reservations 
+    // Hardware address matches all reservations
     client.setHWAddress("aa:bb:cc:dd:ee:ff");
 
-    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its 
+    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its
     // reservation should be used, rather than global.
     runDoraTest(CONFIGS[2], client, "subnet-10-host", "192.0.5.10");
 }
@@ -364,11 +371,11 @@ TEST_F(HostTest, outOfPoolOverGlobal) {
 // is HR_ALL
 TEST_F(HostTest, allOverGlobal) {
     Dhcp4Client client(Dhcp4Client::SELECTING);
-   
+
-    // Hardware address matches all reservations 
+    // Hardware address matches all reservations
     client.setHWAddress("aa:bb:cc:dd:ee:ff");
 
-    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its 
+    // Subnet 10 usses default HR mode(i.e. "in-pool"), so its
     // reservation should be used, rather than global.
     runDoraTest(CONFIGS[3], client, "subnet-10-host", "192.0.5.10");
 }