From 53e319b51c6a6165144e1a2e5e4ffbbc554003cd Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 29 Jun 2023 19:29:59 +0200 Subject: [PATCH] [#2826] Addressed review comments --- src/lib/dhcpsrv/parsers/dhcp_parsers.cc | 12 ++++++++++++ .../testutils/generic_host_data_source_unittest.cc | 1 + 2 files changed, 13 insertions(+) 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(); -- 2.47.2