2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-30 05:27:55 +00:00

[#2408] Improved handling rejected leases

The new communication state functions are now MT safe. The rejected leases
have also expiration time attached.
This commit is contained in:
Marcin Siodelski 2022-09-22 10:34:31 +02:00
parent 3a72bd31c7
commit e15dcdb37c
4 changed files with 273 additions and 46 deletions

View File

@ -22,6 +22,7 @@
#include <boost/pointer_cast.hpp>
#include <ctime>
#include <functional>
#include <limits>
#include <sstream>
@ -324,6 +325,47 @@ CommunicationState::getAnalyzedMessagesCount() const {
return (analyzed_messages_count_);
}
size_t
CommunicationState::getRejectedLeaseUpdatesCount() {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
return (getRejectedLeaseUpdatesCountInternal());
} else {
return (getRejectedLeaseUpdatesCountInternal());
}
}
bool
CommunicationState::reportRejectedLeaseUpdate(const PktPtr& message,
const uint32_t lifetime) {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
return (reportRejectedLeaseUpdateInternal(message, lifetime));
} else {
return (reportRejectedLeaseUpdateInternal(message, lifetime));
}
}
bool
CommunicationState::reportSuccessfulLeaseUpdate(const PktPtr& message) {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
return (reportSuccessfulLeaseUpdateInternal(message));
} else {
return (reportSuccessfulLeaseUpdateInternal(message));
}
}
void
CommunicationState::clearRejectedLeaseUpdates() {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
return (clearRejectedLeaseUpdatesInternal());
} else {
return (clearRejectedLeaseUpdatesInternal());
}
}
bool
CommunicationState::clockSkewShouldWarn() {
if (MultiThreadingMgr::instance().getMode()) {
@ -364,7 +406,7 @@ CommunicationState::clockSkewShouldWarnInternal() {
}
bool
CommunicationState::clockSkewShouldTerminate() const {
CommunicationState::clockSkewShouldTerminate() {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
// Issue a warning if the clock skew is greater than 60s.
@ -375,7 +417,7 @@ CommunicationState::clockSkewShouldTerminate() const {
}
bool
CommunicationState::clockSkewShouldTerminateInternal() const {
CommunicationState::clockSkewShouldTerminateInternal() {
if (isClockSkewGreater(TERM_CLOCK_SKEW)) {
LOG_ERROR(ha_logger, HA_HIGH_CLOCK_SKEW_CAUSES_TERMINATION)
.arg(logFormatClockSkewInternal());
@ -385,7 +427,7 @@ CommunicationState::clockSkewShouldTerminateInternal() const {
}
bool
CommunicationState::rejectedLeaseUpdatesShouldTerminate() const {
CommunicationState::rejectedLeaseUpdatesShouldTerminate() {
if (MultiThreadingMgr::instance().getMode()) {
std::lock_guard<std::mutex> lk(*mutex_);
return (rejectedLeaseUpdatesShouldTerminateInternal());
@ -395,9 +437,9 @@ CommunicationState::rejectedLeaseUpdatesShouldTerminate() const {
}
bool
CommunicationState::rejectedLeaseUpdatesShouldTerminateInternal() const {
CommunicationState::rejectedLeaseUpdatesShouldTerminateInternal() {
if (config_->getMaxRejectedLeaseUpdates() &&
(config_->getMaxRejectedLeaseUpdates() <= getRejectedLeaseUpdatesCount())) {
(config_->getMaxRejectedLeaseUpdates() <= getRejectedLeaseUpdatesCountInternal())) {
LOG_ERROR(ha_logger, HA_REJECTED_LEASE_UPDATES_CAUSE_TERMINATION);
return (true);
}
@ -701,28 +743,33 @@ CommunicationState4::clearConnectingClients() {
}
size_t
CommunicationState4::getRejectedLeaseUpdatesCount() const {
return (rejected_clients_.size());
CommunicationState4::getRejectedLeaseUpdatesCountInternal() {
return (getRejectedLeaseUpdatesCountFromContainer(rejected_clients_));
}
bool
CommunicationState4::reportRejectedLeaseUpdate(const PktPtr& message) {
CommunicationState4::reportRejectedLeaseUpdateInternal(const PktPtr& message, const uint32_t lifetime) {
Pkt4Ptr msg = boost::dynamic_pointer_cast<Pkt4>(message);
if (!msg) {
isc_throw(BadValue, "DHCP message for which the lease update was rejected is not a DHCPv4 message");
}
auto client_id = getClientId(message, DHO_DHCP_CLIENT_IDENTIFIER);
RejectedClient4 client{ msg->getHWAddr()->hwaddr_, client_id, time(NULL) + lifetime };
auto existing_client = rejected_clients_.find(boost::make_tuple(msg->getHWAddr()->hwaddr_, client_id));
if (existing_client == rejected_clients_.end()) {
RejectedClient4 new_client{ msg->getHWAddr()->hwaddr_, client_id };
rejected_clients_.insert(new_client);
rejected_clients_.insert(client);
return (true);
}
rejected_clients_.replace(existing_client, client);
return (false);
}
bool
CommunicationState4::reportSuccessfulLeaseUpdate(const PktPtr& message) {
CommunicationState4::reportSuccessfulLeaseUpdateInternal(const PktPtr& message) {
// Early check if there is anything to do.
if (getRejectedLeaseUpdatesCountInternal() == 0) {
return (false);
}
Pkt4Ptr msg = boost::dynamic_pointer_cast<Pkt4>(message);
if (!msg) {
isc_throw(BadValue, "DHCP message for which the lease update was successful is not a DHCPv4 message");
@ -737,7 +784,7 @@ CommunicationState4::reportSuccessfulLeaseUpdate(const PktPtr& message) {
}
void
CommunicationState4::clearRejectedLeaseUpdates() {
CommunicationState4::clearRejectedLeaseUpdatesInternal() {
rejected_clients_.clear();
}
@ -869,12 +916,12 @@ CommunicationState6::clearConnectingClients() {
}
size_t
CommunicationState6::getRejectedLeaseUpdatesCount() const {
return (rejected_clients_.size());
CommunicationState6::getRejectedLeaseUpdatesCountInternal() {
return (getRejectedLeaseUpdatesCountFromContainer(rejected_clients_));
}
bool
CommunicationState6::reportRejectedLeaseUpdate(const PktPtr& message) {
CommunicationState6::reportRejectedLeaseUpdateInternal(const PktPtr& message, const uint32_t lifetime) {
Pkt6Ptr msg = boost::dynamic_pointer_cast<Pkt6>(message);
if (!msg) {
isc_throw(BadValue, "DHCP message for which the lease update was rejected is not a DHCPv6 message");
@ -883,17 +930,22 @@ CommunicationState6::reportRejectedLeaseUpdate(const PktPtr& message) {
if (duid.empty()) {
return (false);
}
RejectedClient6 client{ duid, time(NULL) + lifetime };
auto existing_client = rejected_clients_.find(duid);
if (existing_client == rejected_clients_.end()) {
RejectedClient6 new_client{ duid };
rejected_clients_.insert(new_client);
rejected_clients_.insert(client);
return (true);
}
rejected_clients_.replace(existing_client, client);
return (false);
}
bool
CommunicationState6::reportSuccessfulLeaseUpdate(const PktPtr& message) {
CommunicationState6::reportSuccessfulLeaseUpdateInternal(const PktPtr& message) {
// Early check if there is anything to do.
if (getRejectedLeaseUpdatesCountInternal() == 0) {
return (false);
}
Pkt6Ptr msg = boost::dynamic_pointer_cast<Pkt6>(message);
if (!msg) {
isc_throw(BadValue, "DHCP message for which the lease update was successful is not a DHCPv6 message");
@ -911,10 +963,9 @@ CommunicationState6::reportSuccessfulLeaseUpdate(const PktPtr& message) {
}
void
CommunicationState6::clearRejectedLeaseUpdates() {
CommunicationState6::clearRejectedLeaseUpdatesInternal() {
rejected_clients_.clear();
}
} // end of namespace isc::ha
} // end of namespace isc

View File

@ -299,13 +299,65 @@ protected:
public:
/// @brief Returns the number of lease updates rejected by the partner (MT safe).
///
/// Each rejected lease update is counted only once if it failed
/// multiple times. Before returning the counter, it discards expired
/// rejected lease updates.
///
/// @return Current rejected lease update number count.
size_t getRejectedLeaseUpdatesCount();
protected:
/// @brief Returns the number of lease updates rejected by the partner.
///
/// Each rejected lease update is counted only once if it failed
/// multiple times
/// multiple times. Before returning the counter, it discards expired
/// rejected lease updates.
///
/// @return Current rejected lease update number count.
virtual size_t getRejectedLeaseUpdatesCount() const = 0;
virtual size_t getRejectedLeaseUpdatesCountInternal() = 0;
/// @brief Extracts the number of lease updates rejected by the partner
/// from the specified container.
///
/// @param rejected_clients container holding rejected clients (v4 or v6).
/// @tparam RejectedClientsType type of the container holding rejected
/// clients.
/// @return Current rejected lease update number count.
template<typename RejectedClientsType>
static size_t getRejectedLeaseUpdatesCountFromContainer(RejectedClientsType& rejected_clients) {
if (rejected_clients.empty()) {
return (0);
}
auto& idx = rejected_clients.template get<1>();
auto upper_limit = idx.upper_bound(time(NULL));
if (upper_limit != idx.end()) {
auto lower_limit = idx.cbegin();
idx.erase(lower_limit, upper_limit);
}
return (rejected_clients.size());
}
public:
/// @brief Marks that the lease update failed due to a conflict for the
/// specified DHCP message (MT safe).
///
/// If the conflict has been already reported for the given client, the
/// rejected lease count remains unchanged.
///
/// @param message DHCP message for which a lease update failed due to
/// a conflict.
/// @param lifetime a time in seconds after which the rejected lease
/// update entry should be discarded.
/// @return true if the update was rejected for the first time, false
/// otherwise.
bool reportRejectedLeaseUpdate(const dhcp::PktPtr& message,
const uint32_t lifetime = 86400);
protected:
/// @brief Marks that the lease update failed due to a conflict for the
/// specified DHCP message.
@ -315,9 +367,26 @@ public:
///
/// @param message DHCP message for which a lease update failed due to
/// a conflict.
/// @param lifetime a time in seconds after which the rejected lease
/// update entry should be discarded.
/// @return true if the update was rejected for the first time, false
/// otherwise.
virtual bool reportRejectedLeaseUpdate(const dhcp::PktPtr& message) = 0;
virtual bool reportRejectedLeaseUpdateInternal(const dhcp::PktPtr& message,
const uint32_t lifetime) = 0;
public:
/// @brief Marks the lease update successful (MT safe).
///
/// If the lease update was previously marked "in conflict", it is
/// now cleared, effectively lowering the number of conflicted leases.
///
/// @param message DHCP message for which the lease update was
/// successful.
/// @return true when the lease was marked "in conflict" and it is
/// now cleared.
bool reportSuccessfulLeaseUpdate(const dhcp::PktPtr& message);
protected:
/// @brief Marks the lease update successful.
///
@ -328,10 +397,19 @@ public:
/// successful.
/// @return true when the lease was marked "in conflict" and it is
/// now cleared.
virtual bool reportSuccessfulLeaseUpdate(const dhcp::PktPtr& message) = 0;
virtual bool reportSuccessfulLeaseUpdateInternal(const dhcp::PktPtr& message) = 0;
public:
/// @brief Clears rejected client leases (MT safe).
void clearRejectedLeaseUpdates();
protected:
/// @brief Clears rejected client leases.
virtual void clearRejectedLeaseUpdates() = 0;
virtual void clearRejectedLeaseUpdatesInternal() = 0;
public:
/// @brief Issues a warning about high clock skew between the active
/// servers if one is warranted.
@ -399,7 +477,7 @@ public:
/// 60 seconds. In the future it may become configurable.
///
/// @return true if the HA service should enter "terminated" state.
bool clockSkewShouldTerminate() const;
bool clockSkewShouldTerminate();
private:
/// @brief Indicates whether the HA service should enter "terminated"
@ -418,7 +496,7 @@ private:
/// 60 seconds. In the future it may become configurable.
///
/// @return true if the HA service should enter "terminated" state.
bool clockSkewShouldTerminateInternal() const;
bool clockSkewShouldTerminateInternal();
/// @brief Checks if the clock skew is greater than the specified number
/// of seconds.
@ -437,7 +515,7 @@ public:
/// exceeds the value of max-rejected-lease-updates, false when the
/// max-rejected-lease-updates is 0 or is greater than the current
/// number of rejected lease updates.
bool rejectedLeaseUpdatesShouldTerminate() const;
bool rejectedLeaseUpdatesShouldTerminate();
private:
@ -448,7 +526,7 @@ private:
/// exceeds the value of max-rejected-lease-updates, false when the
/// max-rejected-lease-updates is 0 or is greater than the current
/// number of rejected lease updates.
bool rejectedLeaseUpdatesShouldTerminateInternal() const;
bool rejectedLeaseUpdatesShouldTerminateInternal();
public:
@ -716,13 +794,16 @@ public:
/// @return Number of unacked clients.
virtual size_t getUnackedClientsCount() const;
protected:
/// @brief Returns the number of lease updates rejected by the partner.
///
/// Each rejected lease update is counted only once if it failed
/// multiple times
/// multiple times. Before returning the counter, it discards expired
/// rejected lease updates.
///
/// @return Current rejected lease update number count.
virtual size_t getRejectedLeaseUpdatesCount() const;
virtual size_t getRejectedLeaseUpdatesCountInternal();
/// @brief Marks that the lease update failed due to a conflict for the
/// specified DHCP message.
@ -732,9 +813,12 @@ public:
///
/// @param message DHCP message for which a lease update failed due to
/// a conflict.
/// @param lifetime a time in seconds after which the rejected lease
/// update entry should be discarded.
/// @return true if the update was rejected for the first time, false
/// otherwise.
virtual bool reportRejectedLeaseUpdate(const dhcp::PktPtr& message);
virtual bool reportRejectedLeaseUpdateInternal(const dhcp::PktPtr& message,
const uint32_t lifetime);
/// @brief Marks the lease update successful.
///
@ -745,12 +829,10 @@ public:
/// successful.
/// @return true when the lease was marked "in conflict" and it is
/// now cleared.
virtual bool reportSuccessfulLeaseUpdate(const dhcp::PktPtr& message);
virtual bool reportSuccessfulLeaseUpdateInternal(const dhcp::PktPtr& message);
/// @brief Clears rejected client leases.
virtual void clearRejectedLeaseUpdates();
protected:
virtual void clearRejectedLeaseUpdatesInternal();
/// @brief Checks if the DHCPv4 message appears to be unanswered.
///
@ -835,6 +917,7 @@ protected:
struct RejectedClient4 {
std::vector<uint8_t> hwaddr_;
std::vector<uint8_t> clientid_;
int64_t expire_;
};
/// @brief Multi index container holding information about the clients
@ -842,7 +925,11 @@ protected:
typedef boost::multi_index_container<
RejectedClient4,
boost::multi_index::indexed_by<
ClientIdent4<RejectedClient4>
ClientIdent4<RejectedClient4>,
boost::multi_index::ordered_non_unique<
boost::multi_index::member<RejectedClient4, int64_t,
&RejectedClient4::expire_>
>
>
> RejectedClients4;
@ -906,13 +993,16 @@ public:
/// @return Number of unacked clients.
virtual size_t getUnackedClientsCount() const;
protected:
/// @brief Returns the number of lease updates rejected by the partner.
///
/// Each rejected lease update is counted only once if it failed
/// multiple times
/// multiple times. Before returning the counter, it discards expired
/// rejected lease updates.
///
/// @return Current rejected lease update number count.
virtual size_t getRejectedLeaseUpdatesCount() const;
virtual size_t getRejectedLeaseUpdatesCountInternal();
/// @brief Marks that the lease update failed due to a conflict for the
/// specified DHCP message.
@ -922,9 +1012,12 @@ public:
///
/// @param message DHCP message for which a lease update failed due to
/// a conflict.
/// @param lifetime a time in seconds after which the rejected lease
/// update entry should be discarded.
/// @return true if the update was rejected for the first time, false
/// otherwise.
virtual bool reportRejectedLeaseUpdate(const dhcp::PktPtr& message);
virtual bool reportRejectedLeaseUpdateInternal(const dhcp::PktPtr& message,
const uint32_t lifetime = 86400);
/// @brief Marks the lease update successful.
///
@ -935,10 +1028,10 @@ public:
/// successful.
/// @return true when the lease was marked "in conflict" and it is
/// now cleared.
virtual bool reportSuccessfulLeaseUpdate(const dhcp::PktPtr& message);
virtual bool reportSuccessfulLeaseUpdateInternal(const dhcp::PktPtr& message);
/// @brief Clears rejected client leases.
virtual void clearRejectedLeaseUpdates();
virtual void clearRejectedLeaseUpdatesInternal();
protected:
@ -1011,6 +1104,7 @@ protected:
/// rejected lease update.
struct RejectedClient6 {
std::vector<uint8_t> duid_;
int64_t expire_;
};
/// @brief Multi index container holding information about the clients
@ -1018,7 +1112,11 @@ protected:
typedef boost::multi_index_container<
RejectedClient6,
boost::multi_index::indexed_by<
ClientIdent6<RejectedClient6>
ClientIdent6<RejectedClient6>,
boost::multi_index::ordered_non_unique<
boost::multi_index::member<RejectedClient6, int64_t,
&RejectedClient6::expire_>
>
>
> RejectedClients6;

View File

@ -137,6 +137,10 @@ public:
/// rejected leases.
void reportRejectedLeasesV6InvalidValuesTest();
/// @brief Test that old rejected lease updates are discarded while
/// getting the rejected lease updates count.
void getRejectedLeaseUpdatesCountFromContainerTest();
/// @brief Returns test heartbeat implementation.
///
/// @return Pointer to heartbeat implementation function under test.
@ -729,15 +733,24 @@ void
CommunicationStateTest::reportRejectedLeasesV4Test() {
// Initially, there should be no rejected leases.
EXPECT_EQ(0, state_.getRejectedLeaseUpdatesCount());
// Reject lease update.
auto msg = createMessage4(DHCPREQUEST, 1, 0, 0);
state_.reportRejectedLeaseUpdate(msg);
EXPECT_EQ(1, state_.getRejectedLeaseUpdatesCount());
// Reject another lease update.
msg = createMessage4(DHCPREQUEST, 2, 0, 0);
state_.reportRejectedLeaseUpdate(msg);
EXPECT_EQ(2, state_.getRejectedLeaseUpdatesCount());
// Reject a lease with a short (zero) lease lifetime.
// This lease should be discarded when we call the
// getRejectedLeaseUpdatesCount().
msg = createMessage4(DHCPREQUEST, 3, 0, 0);
state_.reportRejectedLeaseUpdate(msg, 0);
EXPECT_EQ(2, state_.getRejectedLeaseUpdatesCount());
// Reject lease update for a client using the same MAC
// address but different client identifier. It should
// be treated as a different lease.
@ -779,6 +792,9 @@ CommunicationStateTest::reportSuccessfulLeasesV4Test() {
void
CommunicationStateTest::reportRejectedLeasesV4InvalidValuesTest() {
// Populate one valid update. Without it our functions under test
// would return early.
state_.reportRejectedLeaseUpdate(createMessage4(DHCPREQUEST, 1, 0, 0));
// Using DHCPv6 message in the DHCPv4 context is a programming
// error and deserves an exception.
auto msg = createMessage6(DHCPV6_REQUEST, 1, 0);
@ -790,18 +806,29 @@ void
CommunicationStateTest::reportRejectedLeasesV6Test() {
// Initially, there should be no rejected leases.
EXPECT_EQ(0, state6_.getRejectedLeaseUpdatesCount());
// Reject lease update.
auto msg = createMessage6(DHCPV6_SOLICIT, 1, 0);
auto msg = createMessage6(DHCPV6_REQUEST, 1, 0);
state6_.reportRejectedLeaseUpdate(msg);
EXPECT_EQ(1, state6_.getRejectedLeaseUpdatesCount());
// Reject another lease update.
msg = createMessage6(DHCPV6_SOLICIT, 2, 0);
msg = createMessage6(DHCPV6_REQUEST, 2, 0);
state6_.reportRejectedLeaseUpdate(msg);
EXPECT_EQ(2, state6_.getRejectedLeaseUpdatesCount());
// Reject a lease with a short (zero) lease lifetime.
// This lease should be discarded when we call the
// getRejectedLeaseUpdatesCount().
msg = createMessage6(DHCPV6_REQUEST, 3, 0);
state6_.reportRejectedLeaseUpdate(msg, 0);
EXPECT_EQ(2, state6_.getRejectedLeaseUpdatesCount());
// Reject it again. It should not affect the counter.
msg = createMessage6(DHCPV6_SOLICIT, 2, 0);
msg = createMessage6(DHCPV6_REQUEST, 2, 0);
state6_.reportRejectedLeaseUpdate(msg);
EXPECT_EQ(2, state6_.getRejectedLeaseUpdatesCount());
// Clear rejected lease updates and make sure they
// are now 0.
state6_.clearRejectedLeaseUpdates();
@ -838,6 +865,9 @@ CommunicationStateTest::reportSuccessfulLeasesV6Test() {
void
CommunicationStateTest::reportRejectedLeasesV6InvalidValuesTest() {
// Populate one valid update. Without it our functions under test
// would return early.
state6_.reportRejectedLeaseUpdate(createMessage6(DHCPV6_REQUEST, 1, 0));
// Using DHCPv4 message in the DHCPv6 context is a programming
// error and deserves an exception.
auto msg0 = createMessage4(DHCPREQUEST, 1, 1, 0);
@ -850,6 +880,49 @@ CommunicationStateTest::reportRejectedLeasesV6InvalidValuesTest() {
EXPECT_FALSE(state6_.reportSuccessfulLeaseUpdate(msg1));
}
void
CommunicationStateTest::getRejectedLeaseUpdatesCountFromContainerTest() {
// Create a simple multi index container with two indexes. The
// first index is on the ordinal number to distinguish between
// different entries. The second index is on the expire_ field
// that is identical to the expire_ field in the RejectedClients4
// and RejectedClients6 containers.
struct Entry {
int64_t ordinal_;
int64_t expire_;
};
typedef boost::multi_index_container<
Entry,
boost::multi_index::indexed_by<
boost::multi_index::ordered_unique<
boost::multi_index::member<Entry, int64_t,
&Entry::ordinal_>
>,
boost::multi_index::ordered_non_unique<
boost::multi_index::member<Entry, int64_t,
&Entry::expire_>
>
>
> Entries;
// Add many entries to the container. Odd entries have lifetime
// expiring in the future. Even entries have lifetimes expiring in
// the past.
Entries entries;
for (auto i = 0; i < 1000; i++) {
entries.insert({i, time(NULL) + (i % 2 ? 100 + i : -1 - i)});
}
// Get the count of valid entries. It should remove the expiring
// entries.
auto valid_entries_count = state_.getRejectedLeaseUpdatesCountFromContainer(entries);
EXPECT_EQ(500, valid_entries_count);
EXPECT_EQ(500, entries.size());
// Validate that we removed expired entries, not the valid ones.
for (auto entry : entries) {
EXPECT_EQ(1, entry.ordinal_ % 2);
}
}
TEST_F(CommunicationStateTest, partnerStateTest) {
partnerStateTest();
}
@ -1057,4 +1130,8 @@ TEST_F(CommunicationStateTest, reportRejectedLeasesV6InvalidValuesTestMultiThrea
reportRejectedLeasesV6InvalidValuesTest();
}
TEST_F(CommunicationStateTest, getRejectedLeaseUpdatesCountFromContainerTest) {
getRejectedLeaseUpdatesCountFromContainerTest();
}
}

View File

@ -60,6 +60,7 @@ public:
using StateType::my_time_at_skew_;
using StateType::partner_time_at_skew_;
using StateType::unsent_update_count_;
using StateType::getRejectedLeaseUpdatesCountFromContainer;
};
/// @brief Type of the NakedCommunicationState for DHCPv4.