From: Francis Dupont Date: Wed, 6 May 2020 00:33:45 +0000 (+0200) Subject: [#1147] Checkpoint: finished same client X-Git-Tag: Kea-1.7.9~142 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cebeb2eaa05994e82d80c30388e057ae7a77db5d;p=thirdparty%2Fkea.git [#1147] Checkpoint: finished same client --- diff --git a/src/bin/dhcp4/Makefile.am b/src/bin/dhcp4/Makefile.am index b50dc9124a..82d8e8a40f 100644 --- a/src/bin/dhcp4/Makefile.am +++ b/src/bin/dhcp4/Makefile.am @@ -36,6 +36,7 @@ libdhcp4_la_SOURCES += json_config_parser.cc json_config_parser.h libdhcp4_la_SOURCES += dhcp4_log.cc dhcp4_log.h libdhcp4_la_SOURCES += dhcp4_srv.cc dhcp4_srv.h libdhcp4_la_SOURCES += dhcp4to6_ipc.cc dhcp4to6_ipc.h +libdhcp4_la_SOURCES += client_handler.cc client_handler.h libdhcp4_la_SOURCES += dhcp4_lexer.ll location.hh position.hh stack.hh libdhcp4_la_SOURCES += dhcp4_parser.cc dhcp4_parser.h libdhcp4_la_SOURCES += parser_context.cc parser_context.h parser_context_decl.h diff --git a/src/bin/dhcp4/client_handler.cc b/src/bin/dhcp4/client_handler.cc new file mode 100644 index 0000000000..1275391b1b --- /dev/null +++ b/src/bin/dhcp4/client_handler.cc @@ -0,0 +1,218 @@ +// 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 + +#include +#include +#include +#include + +using namespace std; + +namespace isc { +namespace dhcp { + +mutex ClientHandler::mutex_client_id_; + +mutex ClientHandler::mutex_hwaddr_; + +ClientHandler::ClientByIdContainer ClientHandler::clients_client_id_; + +ClientHandler::ClientByHWAddrContainer ClientHandler::clients_hwaddr_; + +ClientHandler::ClientHandler() : locked_client_id_(), locked_hwaddr_() { +} + +ClientHandler::~ClientHandler() { + if (locked_client_id_) { + lock_guard lock_(mutex_client_id_); + unLockById(); + } + locked_client_id_.reset(); + if (locked_hwaddr_) { + lock_guard lock_(mutex_hwaddr_); + unLockByHWAddr(); + } + locked_hwaddr_.reset(); +} + +ClientHandler::Client::Client(Pkt4Ptr query, DuidPtr client_id, + HWAddrPtr hwaddr) + : query_(query), thread_(this_thread::get_id()) { + if (!query) { + isc_throw(InvalidParameter, "null query in ClientHandler (id)"); + } + if (!client_id && !hwaddr) { + isc_throw(InvalidParameter, "null client-id and hwaddr in ClientHandler"); + } + if (client_id) { + duid_ = client_id->getDuid(); + } + if (hwaddr) { + htype_ = hwaddr->htype_; + hwaddr_ = hwaddr->hwaddr_; + } +} + +ClientHandler::ClientPtr +ClientHandler::lookup(const DuidPtr& duid) { + if (!duid) { + isc_throw(InvalidParameter, "duid is null in ClientHandler::lookup"); + } + auto it = clients_client_id_.find(duid->getDuid()); + if (it == clients_client_id_.end()) { + return (0); + } + return (ClientPtr(new Client(*it))); +} + +ClientHandler::ClientPtr +ClientHandler::lookup(const HWAddrPtr& hwaddr) { + if (!hwaddr) { + isc_throw(InvalidParameter, "hwaddr is null in ClientHandler::lookup"); + } + if (hwaddr->hwaddr_.empty()) { + isc_throw(InvalidParameter, "hwaddr is empty in ClientHandler::lookup"); + } + auto key = boost::make_tuple(hwaddr->htype_, hwaddr->hwaddr_); + auto it = clients_hwaddr_.find(key); + if (it == clients_hwaddr_.end()) { + return (0); + } + return (ClientPtr(new Client(*it))); +} + +void +ClientHandler::lockById(Client client) { + if (!locked_client_id_) { + isc_throw(Unexpected, "nothing to lock in ClientHandler::lock (id)"); + } + // Assume insert will never fail so not checking its result. + clients_client_id_.insert(client); +} + +void +ClientHandler::lockByHWAddr(Client client) { + if (!locked_hwaddr_) { + isc_throw(Unexpected, "nothing to lock in ClientHandler::lock (hw)"); + } + // Assume insert will never fail so not checking its result. + clients_hwaddr_.insert(client); +} + +void +ClientHandler::unLockById() { + if (!locked_client_id_) { + isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock (id)"); + } + // Assume erase will never fail so not checking its result. + clients_client_id_.erase(locked_client_id_->getDuid()); +} + +void +ClientHandler::unLockByHWAddr() { + if (!locked_hwaddr_) { + isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock (hw)"); + } + auto key = boost::make_tuple(locked_hwaddr_->htype_, locked_hwaddr_->hwaddr_); + // Assume erase will never fail so not checking its result. + auto it = clients_hwaddr_.find(key); + if (it == clients_hwaddr_.end()) { + // Should not happen. + return; + } + clients_hwaddr_.erase(it); +} + +bool +ClientHandler::tryLock(Pkt4Ptr query) { + // Sanity checks. + if (!query) { + isc_throw(InvalidParameter, "null query in ClientHandler::tryLock"); + } + if (locked_client_id_) { + isc_throw(Unexpected, + "already handling client-id in ClientHandler::tryLock"); + } + if (locked_hwaddr_) { + isc_throw(Unexpected, + "already handling hwaddr in ClientHandler::tryLock"); + } + + // Get identifiers. + OptionPtr opt_client_id = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER); + DuidPtr duid; + if (opt_client_id) { + duid.reset(new ClientId(opt_client_id->getData())); + } + HWAddrPtr hwaddr = query->getHWAddr(); + if (hwaddr && hwaddr->hwaddr_.empty()) { + hwaddr.reset(); + } + if (!duid && !hwaddr) { + // Can't do something useful: cross fingers. + return (false); + } + + ClientPtr holder_id = 0; + ClientPtr holder_hw = 0; + Client client(query, duid, hwaddr); + + // Try first duid. + if (duid) { + // Try to acquire the by-client-id lock and return the holder + // when it failed. + lock_guard lock_(mutex_client_id_); + holder_id = lookup(duid); + if (!holder_id) { + locked_client_id_ = duid; + lockById(client); + } + } + if (!holder_id) { + if (!hwaddr) { + return (false); + } + // Try to acquire the by-hw-addr lock and return the holder + // when it failed. + lock_guard lock_(mutex_hwaddr_); + holder_hw = lookup(hwaddr); + if (!holder_hw) { + locked_hwaddr_ = hwaddr; + lockByHWAddr(client); + return (false); + } + } + + if (holder_id) { + // This query is a by-client-id-option 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_packet4_logger, DHCP4_PACKET_DROP_0011) + .arg(query->toText()) + .arg(this_thread::get_id()) + .arg(holder_id->query_->toText()) + .arg(holder_id->thread_); + } else { + // This query is a by-hardware address 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_packet4_logger, DHCP4_PACKET_DROP_0012) + .arg(query->toText()) + .arg(this_thread::get_id()) + .arg(holder_hw->query_->toText()) + .arg(holder_hw->thread_); + } + stats::StatsMgr::instance().addValue("pkt4-receive-drop", + static_cast(1)); + return (true); +} + +} // namespace dhcp +} // namespace isc diff --git a/src/bin/dhcp4/client_handler.h b/src/bin/dhcp4/client_handler.h new file mode 100644 index 0000000000..4170478b39 --- /dev/null +++ b/src/bin/dhcp4/client_handler.h @@ -0,0 +1,182 @@ +// 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/. + +#ifndef CLIENT_HANDLER_H +#define CLIENT_HANDLER_H + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace isc { +namespace dhcp { + +/// @brief Client race avoidance RAII handler. +class ClientHandler : public boost::noncopyable { +public: + + /// @brief Constructor. + ClientHandler(); + + /// @brief Destructor. + /// + /// Releases the client if it was acquired. + virtual ~ClientHandler(); + + /// @brief Tries to acquires a client. + /// + /// @param query The query from the client. + /// @return true if the client was acquired, false if there is already + /// a query from the same client. + bool tryLock(Pkt4Ptr query); + +private: + + /// @brief Structure representing a client. + struct Client { + + /// @brief Constructor. + /// + /// @param query The query. + /// @param client_id The client ID. + /// @param hwaddr The hardware address. + /// @throw if the query is null or client_id and hwaddr are null. + Client(Pkt4Ptr query, DuidPtr client_id, HWAddrPtr hwaddr); + + /// @brief The query being processed. + Pkt4Ptr query_; + + /// @brief Cached binary client ID. + std::vector duid_; + + /// @brief Cached hardware type. + uint16_t htype_; + + /// @brief Cached binary hardware address. + std::vector hwaddr_; + + /// @brief The ID of the thread processing the query. + std::thread::id thread_; + }; + + /// @brief The type of unique pointers to clients by ID. + typedef std::unique_ptr ClientPtr; + + /// @brief Client ID locked by this handler. + DuidPtr locked_client_id_; + + /// @brief Hardware address locked by this handler. + HWAddrPtr locked_hwaddr_; + + /// @brief Mutex to protect the client-by-id container. + static std::mutex mutex_client_id_; + + /// @brief Mutex to protect the client-by-hwaddr container. + static std::mutex mutex_hwaddr_; + + /// @brief Lookup a client-by-id. + /// + /// The by-id mutex must be held by the caller. + /// + /// @param duid The duid of the query from the client. + /// @return The held client or null. + static ClientPtr lookup(const DuidPtr& duid); + + /// @brief Lookup a client-by-hwaddr. + /// + /// The by-hwaddr mutex must be held by the caller. + /// + /// @param duid The duid of the query from the client. + /// @return The held client or null. + static ClientPtr lookup(const HWAddrPtr& hwaddr); + + /// @brief Acquire a client by client ID option. + /// + /// The by-id mutex must be held by the caller. + /// + /// @param client The filled client object. + void lockById(Client client); + + /// @brief Acquire a client by hardware address. + /// + /// The by-hwaddr mutex must be held by the caller. + /// + /// @param client The filled client object. + void lockByHWAddr(Client client); + + /// @brief Release a client by client ID option. + /// + /// The by-idmutex must be held by the caller. + void unLockById(); + + /// @brief Release a client by hardware address. + /// + /// The by-hwaddr mutex must be held by the caller. + void unLockByHWAddr(); + + /// @brief The type of the client-by-id container. + typedef boost::multi_index_container< + + // This container stores client-by-id objects. + Client, + + // Start specification of indexes here. + boost::multi_index::indexed_by< + + // First index is used to search by Duid. + boost::multi_index::hashed_unique< + + // Client ID binary content as a member of the Client object. + boost::multi_index::member< + Client, std::vector, &Client::duid_ + > + > + > + > ClientByIdContainer; + + /// @brief The client-by-id container. + static ClientByIdContainer clients_client_id_; + + /// @brief The type of the client-by-hwaddr container. + typedef boost::multi_index_container< + + // This container stores client-by-hwaddr objects. + Client, + + // Start specification of indexes here. + boost::multi_index::indexed_by< + + // First index is used to search by HWAddr. + boost::multi_index::hashed_unique< + + // This is a composite index for type and binary components. + boost::multi_index::composite_key< + Client, + boost::multi_index::member< + Client, uint16_t, &Client::htype_ + >, + boost::multi_index::member< + Client, std::vector, &Client::hwaddr_ + > + > + > + > + > ClientByHWAddrContainer; + + /// @brief The client-by-hwaddr container. + static ClientByHWAddrContainer clients_hwaddr_; + +}; + +} // namespace isc +} // namespace dhcp + +#endif // CLIENT_HANDLER_H diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index 958ecc2d92..7dce137455 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -554,6 +554,20 @@ that it was BOOTP packet. This debug message is emitted when an incoming packet was classified into the special class 'DROP' and dropped. The packet details are displayed. +% DHCP4_PACKET_DROP_0011 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 +a client using the same client id option by dropping new packets until +processing is finished. +Packet details and thread identifiers are included for both packets in +this warning message. + +% DHCP4_PACKET_DROP_0012 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 +a client using the same hardware address by dropping new packets until +processing is finished. +Packet details and thread identifiers are included for both packets in +this warning message. + % DHCP4_PACKET_NAK_0001 %1: failed to select a subnet for incoming packet, src %2, type %3 This error message is output when a packet was received from a subnet for which the DHCPv4 server has not been configured. The most probable diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index f86ed6ba6e..cac51e8bf9 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -1223,6 +1224,20 @@ Dhcpv4Srv::processPacket(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) { return; } + // Create a client race avoidance RAII handler. + ClientHandler client_handler; + + // Check for lease modifier queries from the same client being processed. + if (MultiThreadingMgr::instance().getMode() && + ((query->getType() == DHCPDISCOVER) || + (query->getType() == DHCPREQUEST) || + (query->getType() == DHCPRELEASE) || + (query->getType() == DHCPDECLINE))) { + if (client_handler.tryLock(query)) { + return; + } + } + AllocEngine::ClientContext4Ptr ctx; try { diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 69d92c8f80..aec34b0636 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -105,6 +105,7 @@ dhcp4_unittests_SOURCES += get_config_unittest.cc get_config_unittest.h dhcp4_unittests_SOURCES += shared_network_unittest.cc dhcp4_unittests_SOURCES += host_unittest.cc dhcp4_unittests_SOURCES += vendor_opts_unittest.cc +dhcp4_unittests_SOURCES += client_handler_unittest.cc nodist_dhcp4_unittests_SOURCES = marker_file.h test_libraries.h diff --git a/src/bin/dhcp4/tests/client_handler_unittest.cc b/src/bin/dhcp4/tests/client_handler_unittest.cc new file mode 100644 index 0000000000..79275c3720 --- /dev/null +++ b/src/bin/dhcp4/tests/client_handler_unittest.cc @@ -0,0 +1,409 @@ +// 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 +#include +#include +#include +#include + +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 pkt4-receive-drop statistic. + ClientHandleTest() { + StatsMgr::instance().setValue("pkt4-receive-drop", static_cast(0)); + } + + /// @brief Destructor. + /// + /// Removes statistics. + ~ClientHandleTest() { + StatsMgr::instance().removeAll(); + } + + /// @brief Generates a client-id option. + /// + /// (from dhcp4_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::V4, DHO_DHCP_CLIENT_IDENTIFIER, duid))); + } + + /// @brief Generates a hardware address. + /// + /// (from dhcp4_test_utils.h) + /// + /// @return A random hardware address. + HWAddrPtr generateHWAddr(uint8_t base = 50) { + const uint16_t hw_type = 6; + const size_t len = 6; + OptionBuffer mac(len); + for (size_t i = 0; i < len; ++i) { + mac[i] = base + i; + } + return (HWAddrPtr(new HWAddr(mac, hw_type))); + } + + /// @brief Check statistics. + /// + /// @param bumped True if pkt4-receive-drop shoud have been bumped by one, + /// false otherwise. + void checkStat(bool bumped) { + ObservationPtr obs = StatsMgr::instance().getObservation("pkt4-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. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + sol->addOption(generateClientId()); + sol->setHWAddr(generateHWAddr()); + + 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 (id). +TEST_F(ClientHandleTest, sharedQueriesById) { + // Get two queries. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + OptionPtr client_id = generateClientId(); + // Same client ID: same client. + sol->addOption(client_id); + req->addOption(client_id); + sol->setHWAddr(generateHWAddr()); + req->setHWAddr(generateHWAddr(55)); + + 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 two queries for the same client (hw). +TEST_F(ClientHandleTest, sharedQueriesByHWAddr) { + // Get two queries. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + sol->addOption(generateClientId()); + req->addOption(generateClientId(111)); + HWAddrPtr hwaddr = generateHWAddr(); + // Same hardware address: same client. + sol->setHWAddr(hwaddr); + req->setHWAddr(hwaddr); + + 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 two queries for the same client (hw only). +TEST_F(ClientHandleTest, sharedQueriesByHWAddrOnly) { + // Get two queries. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + // No client ID. + HWAddrPtr hwaddr = generateHWAddr(); + // Same hardware address: same client. + sol->setHWAddr(hwaddr); + req->setHWAddr(hwaddr); + + 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. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + OptionPtr client_id = generateClientId(); + // Same client ID: same client. + sol->addOption(client_id); + req->addOption(client_id); + HWAddrPtr hwaddr = generateHWAddr(); + // Same hardware address: same client. + sol->setHWAddr(hwaddr); + req->setHWAddr(hwaddr); + + 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. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + OptionPtr client_id = generateClientId(); + OptionPtr client_id2 = generateClientId(111); + HWAddrPtr hwaddr = generateHWAddr(); + HWAddrPtr hwaddr1 = generateHWAddr(55); + // Different client ID and hardware address: different client. + sol->addOption(client_id); + req->addOption(client_id2); + sol->setHWAddr(hwaddr); + req->setHWAddr(hwaddr1); + + 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 nor hardware address. +TEST_F(ClientHandleTest, noClientIdHWAddr) { + // Get two queries. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + Pkt4Ptr req(new Pkt4(DHCPREQUEST, 2345)); + // No client id nor hardware address: 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) { + Pkt4Ptr 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. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 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(); + } +} + +// Verifies that double tryLock call fails (hw only). +TEST_F(ClientHandleTest, doubleTryLockByHWAddr) { + // Get a query without a client ID. + Pkt4Ptr sol(new Pkt4(DHCPDISCOVER, 1234)); + sol->setHWAddr(generateHWAddr()); + + 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 diff --git a/src/bin/dhcp6/client_handler.h b/src/bin/dhcp6/client_handler.h index e43f589cc1..09e33d251a 100644 --- a/src/bin/dhcp6/client_handler.h +++ b/src/bin/dhcp6/client_handler.h @@ -10,8 +10,8 @@ #include #include #include -#include #include +#include #include #include