[#2826] Addressed review comments

diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 4671ce77bd1593e1f35063c6bcec828395fe1314..97a93a035ab692dea342d42430a86c0675a1f101 100644 (file)
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -579,6 +579,12 @@ SubnetConfigParser::parse(ConstElementPtr subnet, bool encapsulate_options) {
                   "subnet configuration failed: " << ex.what());
     }
 
+    // We create subnet first and then parse the options straight into the subnet's
+    // CfgOption structure. Previously, we first parsed the options and then copied
+    // them into the CfgOption after creating the subnet but it had two issues. First,
+    // it cost performance. Second, copying options reset the isEncapsulated() flag.
+    // If the options have been encapsulated we want to preserve the flag to ensure
+    // they are not encapsulated several times.
     ConstElementPtr options_params = subnet->get("option-data");
     if (options_params) {
         auto opt_parser = createOptionDataListParser();
@@ -1136,6 +1142,12 @@ PdPoolParser::parse(PoolStoragePtr pools, ConstElementPtr pd_pool_,
                   << " (" << pd_pool_->getPosition() << ")");
     }
 
+    // We create subnet first and then parse the options straight into the subnet's
+    // CfgOption structure. Previously, we first parsed the options and then copied
+    // them into the CfgOption after creating the subnet but it had two issues. First,
+    // it cost performance. Second, copying options reset the isEncapsulated() flag.
+    // If the options have been encapsulated we want to preserve the flag to ensure
+    // they are not encapsulated several times.
     ConstElementPtr option_data = pd_pool_->get("option-data");
     if (option_data) {
         auto opts_parser = createOptionDataListParser();

diff --git a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
index df03d26398a0da9e79fe7354d3ebf6a02e8036b3..a7462ac6027406edcb5cfa591bab7e40ad046b6d 100644 (file)
--- a/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
+++ b/src/lib/dhcpsrv/testutils/generic_host_data_source_unittest.cc
@@ -2054,6 +2054,7 @@ GenericHostDataSourceTest::testOptionsReservations4(const bool formatted,
     ASSERT_NO_FATAL_FAILURE(HostDataSourceUtils::compareHosts(host, *hosts_by_subnet.begin()));
 
     auto returned_host = *hosts_by_subnet.begin();
+    EXPECT_FALSE(returned_host->getCfgOption4()->isEncapsulated());
     ASSERT_NO_THROW(returned_host->encapsulateOptions());
     auto cfg_option = returned_host->getCfgOption4();