#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;
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
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
// 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
#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>
/// @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_;
/// 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.
///
/// @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.
// 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_
>
>
>
--- /dev/null
+// 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