From: Francis Dupont Date: Fri, 31 May 2024 10:30:07 +0000 (+0200) Subject: [#3375] Made update atomic and added UT X-Git-Tag: Kea-2.7.0~56 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=91378b3ae1604244185122898f3e777b3615e34a;p=thirdparty%2Fkea.git [#3375] Made update atomic and added UT --- diff --git a/src/lib/dhcpsrv/cfg_hosts.cc b/src/lib/dhcpsrv/cfg_hosts.cc index b715abf8de..e8f5887589 100644 --- a/src/lib/dhcpsrv/cfg_hosts.cc +++ b/src/lib/dhcpsrv/cfg_hosts.cc @@ -1289,6 +1289,61 @@ CfgHosts::del6(const SubnetID& subnet_id, return (erased_hosts != 0); } +void +CfgHosts::update(HostPtr const& host) { + bool deleted(false); + HostContainerIndex0& idx = hosts_.get<0>(); + std::vector const& identifier(host->getIdentifier()); + auto const t = boost::make_tuple(identifier, host->getIdentifierType()); + MultiThreadingLock lock(*mutex_); + auto const& range = idx.equal_range(t); + if (host->getIPv4SubnetID() != SUBNET_ID_UNUSED) { + // inline del4. + for (auto key = range.first; key != range.second;) { + if ((*key)->getIPv4SubnetID() != host->getIPv4SubnetID()) { + ++key; + // Skip hosts from other subnets. + continue; + } + + key = idx.erase(key); + deleted = true; + LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_CFG_UPDATE_DEL4) + .arg(1) + .arg(host->getIPv4SubnetID()) + .arg(host->getIdentifierAsText()); + } + } else if (host->getIPv6SubnetID() != SUBNET_ID_UNUSED) { + // inline del6. + HostContainer6Index3& idx6 = hosts6_.get<3>(); + for (auto key = range.first; key != range.second;) { + if ((*key)->getIPv6SubnetID() != host->getIPv6SubnetID()) { + ++key; + // Skip hosts from other subnets. + } + + auto host_id = (*key)->getHostId(); + key = idx.erase(key); + deleted = true; + size_t erased_reservations = idx6.erase(host_id); + LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_CFG_UPDATE_DEL6) + .arg(1) + .arg(erased_reservations) + .arg(host->getIPv6SubnetID()) + .arg(host->getIdentifierAsText()); + } + } else { + isc_throw(HostNotFound, "Mandatory 'subnet-id' parameter missing."); + } + if (!deleted) { + isc_throw(HostNotFound, "Host not updated (not found)."); + } + LOG_DEBUG(hosts_logger, HOSTS_DBG_TRACE, HOSTS_CFG_UPDATE_ADD) + .arg(host->toText()); + add4(host); + add6(host); +} + bool CfgHosts::setIPReservationsUnique(const bool unique) { MultiThreadingLock lock(*mutex_); diff --git a/src/lib/dhcpsrv/cfg_hosts.h b/src/lib/dhcpsrv/cfg_hosts.h index 7b675a93c9..91a969bc34 100644 --- a/src/lib/dhcpsrv/cfg_hosts.h +++ b/src/lib/dhcpsrv/cfg_hosts.h @@ -593,6 +593,15 @@ public: const Host::IdentifierType& identifier_type, const uint8_t* identifier_begin, const size_t identifier_len); + /// @brief Attempts to update an existing host entry. + /// + /// The implementation is common to multiple host data sources, so let's + /// provide it in the base host data source. In some instances, it may + /// require synchronization e.g. with transactions in case of databases. + /// + /// @param host the host up to date with the requested changes + virtual void update(HostPtr const& host); + /// @brief Return backend type /// /// Returns the type of the backend (e.g. "mysql", "memfile" etc.) diff --git a/src/lib/dhcpsrv/hosts_messages.cc b/src/lib/dhcpsrv/hosts_messages.cc index 4b5b75f46d..edcd1f62cc 100644 --- a/src/lib/dhcpsrv/hosts_messages.cc +++ b/src/lib/dhcpsrv/hosts_messages.cc @@ -63,6 +63,9 @@ extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_ADDRESS6_NULL = "HO extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER = "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER"; extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_HOST = "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_HOST"; extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_NULL = "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_NULL"; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_ADD = "HOSTS_CFG_UPDATE_ADD"; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_DEL4 = "HOSTS_CFG_UPDATE_DEL4"; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_DEL6 = "HOSTS_CFG_UPDATE_DEL6"; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4 = "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4"; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER = "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER"; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_HOST = "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_HOST"; @@ -138,6 +141,9 @@ const char* values[] = { "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER", "get one host with %1 reservation for subnet id %2, identified by %3", "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_HOST", "using subnet id %1 and identifier %2, found host: %3", "HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_NULL", "host not found using subnet id %1 and identifier %2", + "HOSTS_CFG_UPDATE_ADD", "add the host for reservations: %1", + "HOSTS_CFG_UPDATE_DEL4", "deleted %1 host(s) for subnet id %2 and identifier %3", + "HOSTS_CFG_UPDATE_DEL6", "deleted %1 host(s) having %2 IPv6 reservation(s) for subnet id %3 and identifier %4", "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4", "trying alternate sources for host using subnet id %1 and address %2", "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER", "get one host with IPv4 reservation for subnet id %1, identified by %2", "HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_HOST", "using subnet id %1 and identifier %2, found in %3 host: %4", diff --git a/src/lib/dhcpsrv/hosts_messages.h b/src/lib/dhcpsrv/hosts_messages.h index f40666e6c5..63725bae67 100644 --- a/src/lib/dhcpsrv/hosts_messages.h +++ b/src/lib/dhcpsrv/hosts_messages.h @@ -64,6 +64,9 @@ extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_ADDRESS6_NULL; extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER; extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_HOST; extern const isc::log::MessageID HOSTS_CFG_GET_ONE_SUBNET_ID_IDENTIFIER_NULL; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_ADD; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_DEL4; +extern const isc::log::MessageID HOSTS_CFG_UPDATE_DEL6; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER; extern const isc::log::MessageID HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_IDENTIFIER_HOST; diff --git a/src/lib/dhcpsrv/hosts_messages.mes b/src/lib/dhcpsrv/hosts_messages.mes index fb18cded95..93e89b4d77 100644 --- a/src/lib/dhcpsrv/hosts_messages.mes +++ b/src/lib/dhcpsrv/hosts_messages.mes @@ -1,4 +1,4 @@ -# Copyright (C) 2015-2023 Internet Systems Consortium, Inc. ("ISC") +# Copyright (C) 2015-2024 Internet Systems Consortium, Inc. ("ISC") # # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this @@ -277,6 +277,26 @@ subnet id and specific host identifier. This debug message is issued when no host was found using the specified subnet id and host identifier. +% HOSTS_CFG_UPDATE_ADD add the host for reservations: %1 +This debug message is issued when new host (with reservations) is +added to the server's configuration during an update. The argument +describes the host and its reservations in detail. + +% HOSTS_CFG_UPDATE_DEL4 deleted %1 host(s) for subnet id %2 and identifier %3 +This debug message is issued when IPv4 reservations are deleted for +the specified subnet and identifier during an update. The first +argument specifies how many hosts have been deleted. The second +argument is the subnet identifier. The third argument is the +identifier. + +% HOSTS_CFG_UPDATE_DEL6 deleted %1 host(s) having %2 IPv6 reservation(s) for subnet id %3 and identifier %4 +This debug message is issued when IPv6 reservations are deleted for +the specified subnet and identifier during an update. The first +argument specifies how many hosts have been deleted. The second +argument specifies how many reservations have been deleted. The third +argument is the subnet identifier. The fourth argument is the +identifier. + % HOSTS_MGR_ALTERNATE_GET4_SUBNET_ID_ADDRESS4 trying alternate sources for host using subnet id %1 and address %2 This debug message is issued when the Host Manager doesn't find the host connected to the specific subnet and having the reservation for diff --git a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc index 0811c01363..66ccd54e4d 100644 --- a/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfg_hosts_unittest.cc @@ -71,6 +71,7 @@ public: void testGetAll4ByAddress(); void testDeleteForIPv4(); void testDeleteForIPv6(); + void testDelete2ForIPv6(); void testDel4(); void testDel6(); void testDeleteAll4(); @@ -713,6 +714,41 @@ TEST_F(CfgHostsTest, deleteForIPv6MultiThreading) { testDeleteForIPv6(); } +// This test checks that two IPv6 reservations for the specified subnet ID and +// IPv6 address can be deleted. +void +CfgHostsTest::testDelete2ForIPv6() { + CfgHosts cfg; + // Add host with two addresses. + IOAddress address1("2001:db8:1::1"); + IOAddress address2("2001:db8:2::2"); + size_t host_count = 10; + SubnetID subnet_id(42); + + HostPtr host = HostPtr(new Host(duids_[0]->toText(), "duid", + SUBNET_ID_UNUSED, subnet_id, + IOAddress::IPV4_ZERO_ADDRESS())); + host->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, address1)); + host->addReservation(IPv6Resrv(IPv6Resrv::TYPE_NA, address2)); + cfg.add(host); + + // Delete one host using first address. + EXPECT_TRUE(cfg.del(subnet_id, address1)); + + // Check if all addresses were removed. + EXPECT_FALSE(cfg.get6(subnet_id, address2)); + EXPECT_FALSE(cfg.del(subnet_id, address2)); +} + +TEST_F(CfgHostsTest, delete2ForIPv6) { + testDelete2ForIPv6(); +} + +TEST_F(CfgHostsTest, delete2ForIPv6MultiThreading) { + MultiThreadingTest mt(true); + testDelete2ForIPv6(); +} + // This test checks that false is returned for deleting the IPv4 reservation // that doesn't exist. TEST_F(CfgHostsTest, deleteForMissingIPv4) { @@ -1808,7 +1844,7 @@ TEST_F(CfgHostsTest, duplicatesSubnet6DUIDMultiThreading) { testDuplicatesSubnet6DUID(); } -// Checks that updates work correctly. Note it is not really MT safe. +// Checks that updates work correctly. void CfgHostsTest::testUpdate() { CfgHosts cfg;