[5376] Assign configured server identifier in outbound messages.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 874d85449eef456bf1f7f25cce52f74020154668..6e05c28bc04d9df4943bae58af38fd88ab852ff1 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1142,6 +1142,12 @@ Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
 void
 Dhcpv4Srv::appendServerID(Dhcpv4Exchange& ex) {
 
+    // Do not append generated server identifier if there is one appended already.
+    // This is when explicitly configured server identifier option is present.
+    if (ex.getResponse()->getOption(DHO_DHCP_SERVER_IDENTIFIER)) {
+        return;
+    }
+
     // Use local address on which the packet has been received as a
     // server identifier. In some cases it may be a different address,
     // e.g. broadcast packet or DHCPv4o6 packet.
@@ -1386,7 +1392,8 @@ Dhcpv4Srv::appendBasicOptions(Dhcpv4Exchange& ex) {
     static const uint16_t required_options[] = {
         DHO_ROUTERS,
         DHO_DOMAIN_NAME_SERVERS,
-        DHO_DOMAIN_NAME };
+        DHO_DOMAIN_NAME,
+        DHO_DHCP_SERVER_IDENTIFIER };
 
     static size_t required_options_size =
         sizeof(required_options) / sizeof(required_options[0]);

diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h
index 0f07121a850d90e422e20e9e3c9a039ca5a021fb..2fa90f127e3da6782f16cd6a578ab68d80d1fced 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.h
+++ b/src/bin/dhcp4/dhcp4_srv.h
@@ -525,7 +525,8 @@ protected:
     /// - Subnet Mask,
     /// - Router,
     /// - Name Server,
-    /// - Domain Name.
+    /// - Domain Name,
+    /// - Server Identifier.
     ///
     /// @param ex DHCPv4 exchange holding the client's message to be checked.
     void appendBasicOptions(Dhcpv4Exchange& ex);
@@ -681,10 +682,11 @@ protected:
 
     /// @brief Adds server identifier option to the server's response.
     ///
-    /// This method adds a server identifier to the DHCPv4 message. This is set
-    /// to the local address on which the client's query has been received with
-    /// the exception of broadcast traffic and DHCPv4o6 query for which a socket
-    /// on the particular interface is found and its address is used as server id.
+    /// This method adds a server identifier to the DHCPv4 message if it doesn't
+    /// exist yet. This is set to the local address on which the client's query has
+    /// been received with the exception of broadcast traffic and DHCPv4o6 query for
+    /// which a socket on the particular interface is found and its address is used
+    /// as server id.
     ///
     /// @note This method doesn't throw exceptions by itself but the underlying
     /// classes being used my throw. The reason for this method to not sanity

diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc
index f6a23c4f735466213617fb53eacfdad147fdcbf5..691e6e3eb7b0146d5cd258fb8a281b1f5d942c71 100644 (file)
--- a/src/bin/dhcp4/tests/dora_unittest.cc
+++ b/src/bin/dhcp4/tests/dora_unittest.cc
@@ -87,14 +87,19 @@ namespace {
 ///     - boot-file-name = "bootfile.efi"
 ///
 /// - Configuration 7:
+///   - Used for testing custom value of dhcp-server-identifier option.
+///   - 1 subnets: 10.0.0.0/24 and 192.0.2.0/24
+///   - Custom server identifier specified for each subnet.
+///
+/// - Configuration 8:
 ///   - Simple configuration with a single subnet and single pool
 ///   - Using MySQL lease database backend to store leases
 ///
-/// - Configuration 8:
+/// - Configuration 9:
 ///   - Simple configuration with a single subnet and single pool
 ///   - Using PostgreSQL lease database backend to store leases
 ///
-/// - Configuration 9:
+/// - Configuration 10:
 ///   - Simple configuration with a single subnet and single pool
 ///   - Using Cassandra lease database backend to store leases
 const char* DORA_CONFIGS[] = {
@@ -282,6 +287,38 @@ const char* DORA_CONFIGS[] = {
     "}",
 
 // Configuration 7
+    "{ \"interfaces-config\": {"
+        "      \"interfaces\": [ \"*\" ]"
+        "},"
+        "\"valid-lifetime\": 600,"
+        "\"subnet4\": ["
+        "    {"
+        "        \"subnet\": \"10.0.0.0/24\", "
+        "        \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
+        "        \"interface\": \"eth0\","
+        "        \"option-data\": ["
+        "            {"
+        "                \"name\": \"dhcp-server-identifier\","
+        "                \"data\": \"1.2.3.4\""
+        "            }"
+        "        ]"
+        "    },"
+        "    {"
+        "        \"subnet\": \"192.0.2.0/24\", "
+        "        \"pools\": [ { \"pool\": \"192.0.2.10-192.0.2.100\" } ],"
+        "        \"interface\": \"eth1\","
+        "        \"option-data\": ["
+        "            {"
+        "                \"name\": \"dhcp-server-identifier\","
+        "                \"data\": \"2.3.4.5\""
+        "            }"
+        "        ]"
+
+        "    }"
+        "]"
+    "}",
+
+// Configuration 8
     "{ \"interfaces-config\": {"
         "   \"interfaces\": [ \"*\" ]"
         "},"
@@ -299,7 +336,7 @@ const char* DORA_CONFIGS[] = {
         " } ]"
     "}",
 
-// Configuration 8
+// Configuration 9
     "{ \"interfaces-config\": {"
         "   \"interfaces\": [ \"*\" ]"
         "},"
@@ -317,7 +354,7 @@ const char* DORA_CONFIGS[] = {
         " } ]"
     "}",
 
-// Configuration 9
+// Configuration 10
     "{ \"interfaces-config\": {"
         "   \"interfaces\": [ \"*\" ]"
         "},"
@@ -1568,6 +1605,33 @@ TEST_F(DORATest, multiStageBoot) {
     testMultiStageBoot(0);
 }
 
+// This test verifies that custom server identifier can be specified for
+// a subnet.
+TEST_F(DORATest, customServerIdentifier) {
+    Dhcp4Client client1(Dhcp4Client::SELECTING);
+    // Configure DHCP server.
+    ASSERT_NO_THROW(configure(DORA_CONFIGS[7], *client1.getServer()));
+
+    ASSERT_NO_THROW(client1.doDORA());
+    // Make sure that the server responded.
+    ASSERT_TRUE(client1.getContext().response_);
+    Pkt4Ptr resp = client1.getContext().response_;
+    // Make sure that the server has responded with DHCPACK.
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    // The explicitly configured server identifier should take precedence
+    // over generated server identifier.
+    EXPECT_EQ("1.2.3.4", client1.config_.serverid_.toText());
+
+    // Repeat the test for different subnet.
+    Dhcp4Client client2(client1.getServer(), Dhcp4Client::SELECTING);
+    client2.setIfaceName("eth1");
+    ASSERT_NO_THROW(client2.doDORA());
+    ASSERT_TRUE(client2.getContext().response_);
+    resp = client2.getContext().response_;
+    ASSERT_EQ(DHCPACK, static_cast<int>(resp->getType()));
+    EXPECT_EQ("2.3.4.5", client2.config_.serverid_.toText());
+}
+
 // Starting tests which require MySQL backend availability. Those tests
 // will not be executed if Kea has been compiled without the
 // --with-dhcp-mysql.
@@ -1595,8 +1659,8 @@ public:
 // Test that the client using the same hardware address but multiple
 // client identifiers will obtain multiple leases (MySQL lease database).
 TEST_F(DORAMySQLTest, multiStageBoot) {
-    // DORA_CONFIGS[7] to be used for server configuration.
-    testMultiStageBoot(7);
+    // DORA_CONFIGS[9] to be used for server configuration.
+    testMultiStageBoot(8);
 }
 
 #endif
@@ -1628,8 +1692,8 @@ public:
 // Test that the client using the same hardware address but multiple
 // client identifiers will obtain multiple leases (PostgreSQL lease database).
 TEST_F(DORAPgSQLTest, multiStageBoot) {
-    // DORA_CONFIGS[8] to be used for server configuration.
-    testMultiStageBoot(8);
+    // DORA_CONFIGS[9] to be used for server configuration.
+    testMultiStageBoot(9);
 }
 
 #endif
@@ -1658,8 +1722,8 @@ public:
 // Test that the client using the same hardware address but multiple
 // client identifiers will obtain multiple leases (CQL lease database).
 TEST_F(DORACQLTest, multiStageBoot) {
-    // DORA_CONFIGS[9] to be used for server configuration.
-    testMultiStageBoot(9);
+    // DORA_CONFIGS[10] to be used for server configuration.
+    testMultiStageBoot(10);
 }
 
 #endif