2
0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-31 14:05:33 +00:00

[#1147] Checkpoint: finished v6 same client

This commit is contained in:
Francis Dupont
2020-05-05 23:24:48 +02:00
parent 3e05a2fa4c
commit f99f8da70c
5 changed files with 347 additions and 45 deletions

View File

@@ -7,7 +7,9 @@
#include <config.h>
#include <dhcp6/client_handler.h>
#include <dhcp6/dhcp6_log.h>
#include <exceptions/exceptions.h>
#include <stats/stats_mgr.h>
using namespace std;
@@ -29,35 +31,36 @@ ClientHandler::~ClientHandler() {
locked_.reset();
}
ClientHandler::Client::Client(Pkt6Ptr query)
ClientHandler::Client::Client(Pkt6Ptr query, DuidPtr client_id)
: query_(query), thread_(this_thread::get_id()) {
if (!query) {
isc_throw(InvalidParameter, "null query in ClientHandler");
}
if (!query->getClientId()) {
isc_throw(InvalidParameter, "query has no client Id in ClientHandler");
if (!client_id) {
isc_throw(InvalidParameter, "null client-id in ClientHandler");
}
duid_ = client_id->getDuid();
}
Pkt6Ptr
ClientHandler::ClientPtr
ClientHandler::lookup(const DuidPtr& duid) {
if (!duid) {
isc_throw(InvalidParameter, "duid is null in ClientHandler::lookup");
}
auto it = clients_.find(duid->getDuid());
if (it == clients_.end()) {
return (Pkt6Ptr());
return (0);
}
return (it->query_);
return (ClientPtr(new Client(*it)));
}
void
ClientHandler::lock() {
ClientHandler::lock(Client client) {
if (!locked_) {
isc_throw(Unexpected, "nothing to lock in ClientHandler::lock");
}
Client client(locked_);
clients_.insert(Client(locked_));
// Assume insert will never fail so not checking its result.
clients_.insert(client);
}
void
@@ -65,16 +68,8 @@ ClientHandler::unLock() {
if (!locked_) {
isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock");
}
const DuidPtr& duid = locked_->getClientId();
if (!duid) {
isc_throw(Unexpected, "no duid unlock in ClientHandler::unLock");
}
auto it = clients_.find(duid->getDuid());
if (it == clients_.end()) {
// Should not happen
return;
}
clients_.erase(it);
// Assume erase will never fail so not checking its result.
clients_.erase(locked_->getDuid());
}
bool
@@ -94,15 +89,28 @@ ClientHandler::tryLock(Pkt6Ptr query) {
// A lot of code assumes this will never happen...
isc_throw(Unexpected, "empty DUID in ClientHandler::tryLock");
}
lock_guard<mutex> lock_(mutex_);
const Pkt6Ptr& duplicate = lookup(duid);
if (duplicate) {
// Should log.
return (true);
ClientPtr holder = 0;
{
// Try to acquire the lock and return the holder when it failed.
lock_guard<mutex> lock_(mutex_);
holder = lookup(duid);
if (!holder) {
locked_ = duid;
lock(Client(query, duid));
return (false);
}
}
locked_ = query;
lock();
return (false);
// This query is a duplicate: currently it is simply dropped.
// Logging a warning as it is supposed to be a rare event
// with well behaving clients...
LOG_WARN(bad_packet6_logger, DHCP6_PACKET_DROP_DUPLICATE)
.arg(query->toText())
.arg(this_thread::get_id())
.arg(holder->query_->toText())
.arg(holder->thread_);
stats::StatsMgr::instance().addValue("pkt6-receive-drop",
static_cast<int64_t>(1));
return (true);
}
} // namespace dhcp

View File

@@ -10,7 +10,6 @@
#include <dhcp/pkt6.h>
#include <boost/noncopyable.hpp>
#include <boost/multi_index_container.hpp>
#include <boost/multi_index/mem_fun.hpp>
#include <boost/multi_index/member.hpp>
#include <boost/multi_index/hashed_index.hpp>
#include <mutex>
@@ -46,25 +45,25 @@ private:
/// @brief Constructor.
///
/// @param query The query.
/// @param client_id The client ID.
/// @throw if the query is null or has empty client ID.
Client(Pkt6Ptr query);
Client(Pkt6Ptr query, DuidPtr client_id);
/// @brief The query being processed.
Pkt6Ptr query_;
/// @brief Cached binary client ID.
std::vector<uint8_t> duid_;
/// @brief The ID of the thread processing the query.
std::thread::id thread_;
/// @brief Key extractor.
///
/// Returns the content of the Duid aka client ID.
const std::vector<uint8_t>& getClientId() const {
return (query_->getClientId()->getDuid());
}
};
/// @brief Query locked by this handler.
Pkt6Ptr locked_;
/// @brief The type of unique pointers to clients.
typedef std::unique_ptr<Client> ClientPtr;
/// @brief Client ID locked by this handler.
DuidPtr locked_;
/// @brief Mutex to protect the client container.
static std::mutex mutex_;
@@ -74,13 +73,15 @@ private:
/// The mutex must be held by the caller.
///
/// @param duid The duid of the query from the client.
/// @return The query holding the client or null.
static Pkt6Ptr lookup(const DuidPtr& duid);
/// @return The held client or null.
static ClientPtr lookup(const DuidPtr& duid);
/// @brief Acquire a client.
///
/// The mutex must be held by the caller.
void lock();
///
/// @param client The filled client object.
void lock(Client client);
/// @brief Release a client.
///
@@ -90,7 +91,7 @@ private:
/// @brief The type of the client container.
typedef boost::multi_index_container<
// This container stores clients.
// This container stores client objects.
Client,
// Start specification of indexes here.
@@ -99,9 +100,9 @@ private:
// First index is used to search by Duid.
boost::multi_index::hashed_unique<
// Duid content is extracted by calling Client::getClientId()
boost::multi_index::const_mem_fun<
Client, const std::vector<uint8_t>&, &Client::getClientId
// Client ID binary content as a member of the Client object.
boost::multi_index::member<
Client, std::vector<uint8_t>, &Client::duid_
>
>
>

View File

@@ -526,6 +526,12 @@ after a specified amount of time since receiving "dhcp-disable" command.
This debug message is emitted when an incoming packet was classified
into the special class 'DROP' and dropped. The packet details are displayed.
% DHCP6_PACKET_DROP_DUPLICATE dropped as sent by the same client than a packet being processed by another thread: dropped %1 by thread %2 as duplicate of %3 processed by %4
Currently multi-threading processing avoids races between packets sent by
the same client by dropping new packets until processing is finished.
Packet details and thread identifiers are included for both packets in
this warning message.
% DHCP6_PACKET_DROP_PARSE_FAIL failed to parse packet from %1 to %2, received over interface %3, reason: %4
The DHCPv6 server has received a packet that it is unable to
interpret. The reason why the packet is invalid is included in the message.

View File

@@ -82,6 +82,7 @@ TESTS += dhcp6_unittests
# This list is ordered alphabetically. When adding new files, please maintain
# this order.
dhcp6_unittests_SOURCES = classify_unittests.cc
dhcp6_unittests_SOURCES += client_handler_unittest.cc
dhcp6_unittests_SOURCES += config_parser_unittest.cc
dhcp6_unittests_SOURCES += config_backend_unittest.cc
dhcp6_unittests_SOURCES += confirm_unittest.cc

View File

@@ -0,0 +1,286 @@
// Copyright (C) 2020 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
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
#include <config.h>
#include <dhcp/option.h>
#include <dhcp6/client_handler.h>
#include <dhcp6/tests/dhcp6_test_utils.h>
#include <stats/stats_mgr.h>
using namespace isc;
using namespace isc::dhcp;
using namespace isc::dhcp::test;
using namespace isc::stats;
using namespace isc::util;
namespace {
/// @brief Test fixture class for testing client handler.
class ClientHandleTest : public ::testing::Test {
public:
/// @brief Constructor.
///
/// Creates the pkt6-receive-drop statistic.
ClientHandleTest() {
StatsMgr::instance().setValue("pkt6-receive-drop", static_cast<int64_t>(0));
}
/// @brief Destructor.
///
/// Removes statistics.
~ClientHandleTest() {
StatsMgr::instance().removeAll();
}
/// @brief Generates a client-id option.
///
/// (from dhcp6_test_utils.h)
///
/// @return A random client-id option.
OptionPtr generateClientId(uint8_t base = 100) {
const size_t len = 32;
OptionBuffer duid(len);
for (size_t i = 0; i < len; ++i) {
duid[i] = base + i;
}
return (OptionPtr(new Option(Option::V6, D6O_CLIENTID, duid)));
}
/// @brief Check statistics.
///
/// @param bumped True if pkt6-receive-drop shoud have been bumped by one,
/// false otherwise.
void checkStat(bool bumped) {
ObservationPtr obs = StatsMgr::instance().getObservation("pkt6-receive-drop");
ASSERT_TRUE(obs);
if (bumped) {
EXPECT_EQ(1, obs->getInteger().first);
} else {
EXPECT_EQ(0, obs->getInteger().first);
}
}
};
// Verifies behavior with empty block.
TEST_F(ClientHandleTest, empty) {
try {
// Get a client handler.
ClientHandler client_handler;
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
}
// Verifies behavior with one query.
TEST_F(ClientHandleTest, oneQuery) {
// Get a query.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
sol->addOption(generateClientId());
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
checkStat(false);
}
// Verifies behavior with two queries for the same client.
TEST_F(ClientHandleTest, sharedQueries) {
// Get two queries.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
Pkt6Ptr req(new Pkt6(DHCPV6_REQUEST, 2345));
OptionPtr client_id = generateClientId();
// Same client ID: same client.
sol->addOption(client_id);
req->addOption(client_id);
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it with the solicit.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
// Get a second client handler.
ClientHandler client_handler2;
// Try to lock it with a request.
EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req));
// Should return true (race with the duplicate).
EXPECT_TRUE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
checkStat(true);
}
// Verifies behavior with a sequence of two queries.
TEST_F(ClientHandleTest, sequence) {
// Get two queries.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
Pkt6Ptr req(new Pkt6(DHCPV6_REQUEST, 2345));
OptionPtr client_id = generateClientId();
// Same client ID: same client.
sol->addOption(client_id);
req->addOption(client_id);
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it with the solicit.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
// As it is a different block the lock was released.
try {
// Get a second client handler.
ClientHandler client_handler2;
// Try to lock it with a request.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
checkStat(false);
}
// Verifies behavior with different clients.
TEST_F(ClientHandleTest, notSharedQueries) {
// Get two queries.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
Pkt6Ptr req(new Pkt6(DHCPV6_REQUEST, 2345));
OptionPtr client_id = generateClientId();
OptionPtr client_id2 = generateClientId(111);
// Different client ID: different client.
sol->addOption(client_id);
req->addOption(client_id2);
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it with the solicit.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
// Get a second client handler.
ClientHandler client_handler2;
// Try to lock it with a request.
EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
checkStat(false);
}
// Verifies behavior without client ID.
TEST_F(ClientHandleTest, noClientId) {
// Get two queries.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
Pkt6Ptr req(new Pkt6(DHCPV6_REQUEST, 2345));
// No client id: nothing to recognize the client.
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it with the solicit.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
// Get a second client handler.
ClientHandler client_handler2;
// Try to lock it with a request.
EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
checkStat(false);
}
// Verifies the query is required.
TEST_F(ClientHandleTest, noQuery) {
Pkt6Ptr no_pkt;
try {
// Get a client handler.
ClientHandler client_handler;
EXPECT_THROW(client_handler.tryLock(no_pkt), InvalidParameter);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
}
// Verifies that double tryLock call fails.
TEST_F(ClientHandleTest, doubleTryLock) {
// Get a query.
Pkt6Ptr sol(new Pkt6(DHCPV6_SOLICIT, 1234));
sol->addOption(generateClientId());
try {
// Get a client handler.
ClientHandler client_handler;
// Try to lock it.
bool duplicate = false;
EXPECT_NO_THROW(duplicate = client_handler.tryLock(sol));
// Should return false (no duplicate).
EXPECT_FALSE(duplicate);
// Try to lock a second time.
EXPECT_THROW(client_handler.tryLock(sol), Unexpected);
} catch (const std::exception& ex) {
ADD_FAILURE() << "unexpected exception: " << ex.what();
}
}
// Cannot verifies that empty client ID fails because getClientId() handles
// this condition and replaces it by no client ID.
} // end of anonymous namespace