From: Francis Dupont Date: Tue, 21 Jul 2026 06:25:20 +0000 (+0200) Subject: [#4496] Added cleanup/decapsulation X-Git-Tag: Kea-3.3.0~28 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=1f58e7e393dad04b3a00a67b1cf8debec8b829f0;p=thirdparty%2Fkea.git [#4496] Added cleanup/decapsulation --- diff --git a/changelog_unreleased/4496-dhcp6-and-ntp-option-56-only-one-ipv6-ntp-server-is-possible b/changelog_unreleased/4496-dhcp6-and-ntp-option-56-only-one-ipv6-ntp-server-is-possible new file mode 100644 index 0000000000..0301990588 --- /dev/null +++ b/changelog_unreleased/4496-dhcp6-and-ntp-option-56-only-one-ipv6-ntp-server-is-possible @@ -0,0 +1,8 @@ +[func] fdupont + Changed the way sub-options are encapsulated for + special option spaces to allow multiple suboptions + with the same code point. Currently only the + "v6-ntp-server-suboptions" space of the DHCPv6 + "ntp-server" is special but if this feature is + useful for other spaces it can be made configurable. + (Gitlab #4496) diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index f6f07c44cd..bef6d9e539 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -3841,6 +3841,79 @@ TEST_F(Dhcp6ParserTest, optionDataEncapsulate) { EXPECT_EQ(111U, option_foo2->getType()); } +// The v6-ntp-server-suboptions space is special because it allows +// multiple sub-options of the same type. +TEST_F(Dhcp6ParserTest, optionDataNtpServer) { + string config = "{ " + genIfaceConfig() + "," + "\"preferred-lifetime\": 3000," + "\"valid-lifetime\": 4000," + "\"rebind-timer\": 2000," + "\"renew-timer\": 1000," + "\"option-data\": [ {" + " \"name\": \"ntp-server\"" + " }," + " {" + " \"name\": \"ntp-server-address\"," + " \"space\": \"v6-ntp-server-suboptions\"," + " \"data\": \"2001:db8::77\"" + " }," + " {" + " \"name\": \"ntp-server-address\"," + " \"space\": \"v6-ntp-server-suboptions\"," + " \"data\": \"2001:db8::88\"" + " } ]" + "}"; + + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP6(config)); + extractConfig(config); + + ConstElementPtr status; + EXPECT_NO_THROW(status = Dhcpv6SrvTest::configure(srv_, json)); + ASSERT_TRUE(status); + checkResult(status, 0); + + // We should have one option available. + OptionContainerPtr options = + CfgMgr::instance().getStagingCfg()->getCfgOption()->getAll(DHCP6_OPTION_SPACE); + ASSERT_TRUE(options); + ASSERT_EQ(1U, options->size()); + + // Get the option. + OptionDescriptor desc = + CfgMgr::instance().getStagingCfg()->getCfgOption()->get(DHCP6_OPTION_SPACE, D6O_NTP_SERVER); + EXPECT_TRUE(desc.option_); + EXPECT_EQ(D6O_NTP_SERVER, desc.option_->getType()); + + // This option should comprise two sub-options. + auto const& subs = desc.option_->getOptions(); + ASSERT_EQ(2U, subs.size()); + + // Check sub-options. + bool got_first = false; + bool got_second = false; + IOAddress addr1("2001:db8::77"); + IOAddress addr2("2001:db8::88"); + auto const buf1 = addr1.toBytes(); + auto const buf2 = addr2.toBytes(); + for (auto const& sub : subs) { + ASSERT_EQ(NTP_SUBOPTION_SRV_ADDR, sub.first); + ASSERT_TRUE(sub.second); + ASSERT_EQ(NTP_SUBOPTION_SRV_ADDR, sub.second->getType()); + if (sub.second->getData() == buf1) { + EXPECT_FALSE(got_first); + got_first = true; + } else if (sub.second->getData() == buf2) { + EXPECT_FALSE(got_second); + got_second = true; + } else { + FAIL() << "unexpected content of " << sub.second->toText(); + } + } + EXPECT_TRUE(got_first); + EXPECT_TRUE(got_second); +} + // Goal of this test is to verify options configuration // for a single subnet. In particular this test checks // that local options configuration overrides global diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc index 3a07578b1c..7df192bd11 100644 --- a/src/lib/dhcpsrv/cfg_option.cc +++ b/src/lib/dhcpsrv/cfg_option.cc @@ -335,6 +335,14 @@ CfgOption::encapsulateInternal(const OptionPtr& option) { if (encap_space == DHCP4_OPTION_SPACE || encap_space == DHCP6_OPTION_SPACE) { return; } + // Handle multiple encapsulating spaces. + bool multiple_encapsulating = false; + if (multiple_encapsulating_spaces_.count(encap_space) > 0) { + multiple_encapsulating = true; + // Cleanup the option so encapsulate() can be called again. + decapsulateInternal(option); + } + // Retrieve all options from the encapsulated option space. OptionContainerPtr encap_options = getAll(encap_space); for (auto const& encap_opt : *encap_options) { @@ -346,14 +354,23 @@ CfgOption::encapsulateInternal(const OptionPtr& option) { // Add sub-option if there isn't one added already, or // if encapsulating space is a multiple exception. OptionPtr existing = option->getOption(encap_opt.option_->getType()); - if (!existing || - (multiple_encapsulating_spaces_.count(encap_space) > 0)) { + if (multiple_encapsulating | + !option->getOption(encap_opt.option_->getType())) { option->addOption(encap_opt.option_); } encapsulateInternal(encap_opt.option_); } } +void +CfgOption::decapsulateInternal(const OptionPtr& option) { + // Dereference any existing sub options. + auto sub_options = option->getOptions(); + for (auto const& sub : sub_options) { + option->delOption(sub.second->getType()); + } +} + template void CfgOption::mergeInternal(const OptionSpaceContainergetOptions(); - for (auto const& sub : sub_options) { - // Dereference sub option. - option_it.option_->delOption(sub.second->getType()); + if (option_it.option_) { + decapsulateInternal(option_it.option_); } } } diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h index 9727a70aee..48779ce27b 100644 --- a/src/lib/dhcpsrv/cfg_option.h +++ b/src/lib/dhcpsrv/cfg_option.h @@ -936,6 +936,11 @@ private: /// @param option which encapsulated options. void encapsulateInternal(const OptionPtr& option); + /// @brief Undo encapsulation for an option. + /// + /// @param option which decapsulated options. + void decapsulateInternal(const OptionPtr& option); + /// @brief Merges data from two option containers. /// /// This method merges options from one option container to another diff --git a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc index 6625174123..7dfbb84fc4 100644 --- a/src/lib/dhcpsrv/tests/cfg_option_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_option_unittest.cc @@ -839,6 +839,15 @@ TEST_F(CfgOptionTest, multipleEncapsulatingSpaces) { for (auto const& sub : subs) { EXPECT_EQ(NTP_SUBOPTION_SRV_ADDR, sub.first); } + + // Encapsulate again. + ASSERT_NO_THROW(cfg.encapsulate()); + + // Check we have ntp-server with still 2 (not 4) sub-options. + desc = cfg.get(DHCP6_OPTION_SPACE, D6O_NTP_SERVER); + opt = desc.option_; + ASSERT_TRUE(opt); + ASSERT_EQ(2U, opt->getOptions().size()); } // This test verifies that an option can be deleted from the configuration.