From: Francis Dupont Date: Sun, 10 May 2020 19:34:49 +0000 (+0200) Subject: [#1147] Checkpoint: client queue of one X-Git-Tag: Kea-1.7.9~136 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e6cd1043e52ff71e9a3182e73a39deb7061dd76;p=thirdparty%2Fkea.git [#1147] Checkpoint: client queue of one --- diff --git a/src/bin/dhcp4/client_handler.h b/src/bin/dhcp4/client_handler.h index 6dc289beef..dd9ef9ea31 100644 --- a/src/bin/dhcp4/client_handler.h +++ b/src/bin/dhcp4/client_handler.h @@ -21,22 +21,22 @@ namespace isc { namespace dhcp { -/// @brief Client race avoidance RAII handler. -class ClientHandler : public boost::noncopyable { -public: +/// @brief Define the type of packet processing continuation. +typedef std::function Continuation; - /// @brief Define the type of packet processing continuation. - typedef std::function Continuation; +/// @brief Define the type of shared pointers to continuations. +typedef boost::shared_ptr ContinuationPtr; - /// @brief Define the type of shared pointers to continuations. - typedef boost::shared_ptr ContinuationPtr; +/// @brief Continuation factory. +/// +/// @param cont Continuation rvalue. +inline ContinuationPtr makeContinuation(Continuation&& cont) { + return (boost::make_shared(cont)); +} - /// @brief Continuation factory. - /// - /// @param cont Continuation rvalue. - static ContinuationPtr makeContinuation(Continuation&& cont) { - return (boost::make_shared(cont)); - } +/// @brief Client race avoidance RAII handler. +class ClientHandler : public boost::noncopyable { +public: /// @brief Constructor. ClientHandler(); diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e33e7de9cb..bdd6a2c46f 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1224,20 +1224,6 @@ 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; - } - } - processDhcp4Query(query, rsp, allow_packet_park); } @@ -1263,6 +1249,23 @@ Dhcpv4Srv::processDhcp4QueryAndSendResponse(Pkt4Ptr& query, Pkt4Ptr& rsp, void Dhcpv4Srv::processDhcp4Query(Pkt4Ptr& query, Pkt4Ptr& rsp, bool allow_packet_park) { + // 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))) { + ContinuationPtr cont = + makeContinuation(std::bind(&Dhcpv4Srv::processDhcp4QueryAndSendResponse, + this, query, rsp, allow_packet_park)); + if (client_handler.tryLock(query, cont)) { + return; + } + } + AllocEngine::ClientContext4Ptr ctx; try { diff --git a/src/bin/dhcp4/tests/client_handler_unittest.cc b/src/bin/dhcp4/tests/client_handler_unittest.cc index aa2e8d0460..c1c39b3172 100644 --- a/src/bin/dhcp4/tests/client_handler_unittest.cc +++ b/src/bin/dhcp4/tests/client_handler_unittest.cc @@ -463,8 +463,8 @@ TEST_F(ClientHandleTest, serializeTwoQueriesById) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the discover. bool duplicate = false; @@ -477,8 +477,8 @@ TEST_F(ClientHandleTest, serializeTwoQueriesById) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -521,8 +521,8 @@ TEST_F(ClientHandleTest, serializeTwoQueriesByHWAddr) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the discover. bool duplicate = false; @@ -535,8 +535,8 @@ TEST_F(ClientHandleTest, serializeTwoQueriesByHWAddr) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -681,8 +681,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesById) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the discover. bool duplicate = false; @@ -695,8 +695,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesById) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -708,8 +708,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesById) { ClientHandler client_handler3; // Create a continuation. - ClientHandler::ContinuationPtr cont3 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); + ContinuationPtr cont3 = + makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); // Try to lock it with a release. EXPECT_NO_THROW(duplicate = client_handler3.tryLock(rel, cont3)); @@ -759,8 +759,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesHWAddr) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the discover. bool duplicate = false; @@ -773,8 +773,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesHWAddr) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -786,8 +786,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesHWAddr) { ClientHandler client_handler3; // Create a continuation. - ClientHandler::ContinuationPtr cont3 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); + ContinuationPtr cont3 = + makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); // Try to lock it with a release. EXPECT_NO_THROW(duplicate = client_handler3.tryLock(rel, cont3)); @@ -841,8 +841,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesMixed) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the discover. bool duplicate = false; @@ -855,8 +855,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesMixed) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -868,8 +868,8 @@ TEST_F(ClientHandleTest, serializeThreeQueriesMixed) { ClientHandler client_handler3; // Create a continuation. - ClientHandler::ContinuationPtr cont3 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); + ContinuationPtr cont3 = + makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); // Try to lock it with a release. EXPECT_NO_THROW(duplicate = client_handler3.tryLock(rel, cont3)); diff --git a/src/bin/dhcp6/client_handler.h b/src/bin/dhcp6/client_handler.h index 01f0dd1130..aae2d19c53 100644 --- a/src/bin/dhcp6/client_handler.h +++ b/src/bin/dhcp6/client_handler.h @@ -20,22 +20,22 @@ namespace isc { namespace dhcp { -/// @brief Client race avoidance RAII handler. -class ClientHandler : public boost::noncopyable { -public: +/// @brief Define the type of packet processing continuation. +typedef std::function Continuation; - /// @brief Define the type of packet processing continuation. - typedef std::function Continuation; +/// @brief Define the type of shared pointers to continuations. +typedef boost::shared_ptr ContinuationPtr; - /// @brief Define the type of shared pointers to continuations. - typedef boost::shared_ptr ContinuationPtr; +/// @brief Continuation factory. +/// +/// @param cont Continuation rvalue. +inline ContinuationPtr makeContinuation(Continuation&& cont) { + return (boost::make_shared(cont)); +} - /// @brief Continuation factory. - /// - /// @param cont Continuation rvalue. - static ContinuationPtr makeContinuation(Continuation&& cont) { - return (boost::make_shared(cont)); - } +/// @brief Client race avoidance RAII handler. +class ClientHandler : public boost::noncopyable { +public: /// @brief Constructor. ClientHandler(); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index cf673b0e8e..8a272cd335 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -851,7 +851,10 @@ Dhcpv6Srv::processDhcp6Query(Pkt6Ptr& query, Pkt6Ptr& rsp) { (query->getType() == DHCPV6_REBIND) || (query->getType() == DHCPV6_RELEASE) || (query->getType() == DHCPV6_DECLINE))) { - drop = client_handler.tryLock(query); + ContinuationPtr cont = + makeContinuation(std::bind(&Dhcpv6Srv::processDhcp6QueryAndSendResponse, + this, query, rsp)); + drop = client_handler.tryLock(query, cont); } // Stop here if ClientHandler tryLock decided the packet is a duplicate. diff --git a/src/bin/dhcp6/tests/client_handler_unittest.cc b/src/bin/dhcp6/tests/client_handler_unittest.cc index cd4ee0f66a..dc3b330443 100644 --- a/src/bin/dhcp6/tests/client_handler_unittest.cc +++ b/src/bin/dhcp6/tests/client_handler_unittest.cc @@ -337,8 +337,8 @@ TEST_F(ClientHandleTest, serializeTwoQueries) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the solicit. bool duplicate = false; @@ -351,8 +351,8 @@ TEST_F(ClientHandleTest, serializeTwoQueries) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -443,8 +443,8 @@ TEST_F(ClientHandleTest, serializeThreeQueries) { ClientHandler client_handler; // Create a continuation. - ClientHandler::ContinuationPtr cont1 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); + ContinuationPtr cont1 = + makeContinuation(std::bind(&ClientHandleTest::setCalled1, this)); // Try to lock it with the solicit. bool duplicate = false; @@ -457,8 +457,8 @@ TEST_F(ClientHandleTest, serializeThreeQueries) { ClientHandler client_handler2; // Create a continuation. - ClientHandler::ContinuationPtr cont2 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); + ContinuationPtr cont2 = + makeContinuation(std::bind(&ClientHandleTest::setCalled2, this)); // Try to lock it with a request. EXPECT_NO_THROW(duplicate = client_handler2.tryLock(req, cont2)); @@ -470,8 +470,8 @@ TEST_F(ClientHandleTest, serializeThreeQueries) { ClientHandler client_handler3; // Create a continuation. - ClientHandler::ContinuationPtr cont3 = - ClientHandler::makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); + ContinuationPtr cont3 = + makeContinuation(std::bind(&ClientHandleTest::setCalled3, this)); // Try to lock it with a renew. EXPECT_NO_THROW(duplicate = client_handler3.tryLock(ren, cont3));