[#2826] Addressed review comments

2 files changed, 13 insertions, 0 deletions

diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 4671ce77bd..97a93a035a 100644
--- 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 df03d26398..a7462ac602 100644
--- 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();