When the packet goes out of scope the callout handle is destroyed.
libkea_dhcp___la_LIBADD = $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/dns/libkea-dns++.la
libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/cryptolink/libkea-cryptolink.la
+libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
+libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/log/libkea-log.la
+libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/util/threads/libkea-threads.la
libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/util/libkea-util.la
+libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/cc/libkea-cc.la
libkea_dhcp___la_LIBADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
libkea_dhcp___la_LIBADD += $(BOOST_LIBS)
libkea_dhcp___la_LIBADD += $(CRYPTO_LIBS)
#include <dhcp/option.h>
#include <dhcp/hwaddr.h>
#include <dhcp/classify.h>
+#include <hooks/callout_handle_associate.h>
#include <boost/date_time/posix_time/posix_time.hpp>
#include <boost/shared_ptr.hpp>
///
/// @note This is abstract class. Please instantiate derived classes
/// such as @c Pkt4 or @c Pkt6.
-class Pkt {
+class Pkt : public hooks::CalloutHandleAssociate {
protected:
/// @brief Constructor.
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/asiolink/libkea-asiolink.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/dns/libkea-dns++.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/cryptolink/libkea-cryptolink.la
+libdhcp___unittests_LDADD += $(top_builddir)/src/lib/hooks/libkea-hooks.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/log/libkea-log.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/util/threads/libkea-threads.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/util/libkea-util.la
+libdhcp___unittests_LDADD += $(top_builddir)/src/lib/cc/libkea-cc.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/exceptions/libkea-exceptions.la
libdhcp___unittests_LDADD += $(top_builddir)/src/lib/testutils/libkea-testutils.la
libdhcp___unittests_LDADD += $(LOG4CPLUS_LIBS) $(CRYPTO_LIBS)
///
/// When using the Hooks Framework, there is a need to associate an
/// isc::hooks::CalloutHandle object with each request passing through the
-/// server. For the DHCP servers, the association is provided by this function.
+/// server. For the DHCP servers, the association was provided by this function.
///
-/// The DHCP servers process requests sequentially, but sometimes packets can
-/// be "parked" waiting for the completion of asynchronous operations scheduled
-/// by hook libraries. While the packets are parked, other packets can be
-/// processed. This poses a requirement for the DHCP server to keep
-/// multiple associations between the packets and their handles. At points where
-/// the CalloutHandle is required, the pointer to the current request (packet)
-/// is passed to this function. If the request is a new one, a pointer to the
-/// request stored, a new CalloutHandle is allocated (and stored) and a pointer
-/// to the latter object returned to the caller. If the request matches one of
-/// the stored requests, the CalloutHandle for this request is returned.
+/// After introduction of "packets parking" feature this function was extended
+/// to keep association of packets with the callout handles in a map.
+/// However, it was later found that "garbage collection" of the unused
+/// handles is very hard. Trying to garbage collect handles at each invocation
+/// was highly inefficient and caused server's performance degradation.
///
-/// In order to avoid the endless growth of the collection of stored associations
-/// this function performs housekeeping of the collection every time it is
-/// called with non-null packet pointer. All entries for which the packet
-/// pointer's reference count is less than 2 are removed. These are the packet
-/// objects which only this function holds reference counts to. Thus, they can
-/// be removed because they are not required by the server anymore. They have
-/// been either dropped or processed.
+/// The new approach is using on @c isc::hooks::CalloutHandleAssociate to
+/// associate objects with callout handles. This has a major benefit that
+/// callout handle instances are removed together with the packets associated
+/// with them.
///
-///
-/// A special case is a null pointer being passed to this function. This has
-/// the effect of clearing the pointers to stored handles and packets.
+/// This function uses this new approach and is kept for the compatibility with
+/// existing code.
///
/// @tparam T Pkt4Ptr or Pkt6Ptr object.
/// @param pktptr Pointer to the packet being processed. This is typically a
-/// Pkt4Ptr or Pkt6Ptr object. An empty pointer is passed to clear
-/// the stored pointers.
+/// Pkt4Ptr or Pkt6Ptr object.
///
-/// @return Shared pointer to a CalloutHandle. This is the previously-stored
+/// @return Shared pointer to a CalloutHandle. This is the previously-stored
/// CalloutHandle if pktptr points to a packet that has been seen
-/// before or a new CalloutHandle if it points to a new one. An empty
+/// before or a new CalloutHandle if it points to a new one. An empty
/// pointer is returned if pktptr is itself an empty pointer.
template <typename T>
isc::hooks::CalloutHandlePtr getCalloutHandle(const T& pktptr) {
- isc::hooks::CalloutHandlePtr stored_handle;
- static std::map<T, isc::hooks::CalloutHandlePtr> store;
-
if (pktptr) {
+ return (pktptr->getCalloutHandle());
- std::set<T> to_remove;
-
- // Identify unused handles by checking whether the reference count is
- // less then 2. This indicates that only our local store keeps the
- // pointer to the packet.
- for (auto handle_pair_it = store.begin(); handle_pair_it != store.end();
- ++handle_pair_it) {
-
- if (!handle_pair_it->first || handle_pair_it->first.use_count() < 2) {
- to_remove.insert(handle_pair_it->first);
- }
- }
-
- // Actually remove the unused handles.
- for (const auto& pktptr : to_remove) {
- store.erase(pktptr);
- }
-
- // Pointer given, have we seen it before?
- auto stored_handle_it = store.find(pktptr);
- if (stored_handle_it == store.end()) {
-
- // Not seen before, so store the pointer passed to us and get a new
- // CalloutHandle.
- stored_handle = isc::hooks::HooksManager::createCalloutHandle();
- store[pktptr] = stored_handle;
-
- } else {
- // Return existing pointer.
- stored_handle = stored_handle_it->second;
- }
-
- } else {
- // Empty pointer passed, clear stored data
- store.clear();
}
- return (stored_handle);
+ return (isc::hooks::CalloutHandlePtr());
}
} // namespace shcp
ASSERT_TRUE(pktptr_2);
ASSERT_TRUE(pktptr_1 != pktptr_2);
- EXPECT_EQ(1, pktptr_1.use_count());
- EXPECT_EQ(1, pktptr_2.use_count());
// Get the CalloutHandle for the first packet.
CalloutHandlePtr chptr_1 = getCalloutHandle(pktptr_1);
ASSERT_TRUE(chptr_1);
- // Reference counts on both the callout handle and the packet should have
- // been incremented because of the stored data. The reference count on the
- // other Pkt4 object should not have changed.
- EXPECT_EQ(2, chptr_1.use_count());
- EXPECT_EQ(2, pktptr_1.use_count());
- EXPECT_EQ(1, pktptr_2.use_count());
-
// Try getting another pointer for the same packet. Use a different
// pointer object to check that the function returns a handle based on the
- // pointed-to data and not the pointer. (Clear the temporary pointer after
- // use to avoid complicating reference counts.)
+ // pointed-to data and not the pointer.
Pkt4Ptr pktptr_temp = pktptr_1;
CalloutHandlePtr chptr_2 = getCalloutHandle(pktptr_temp);
pktptr_temp.reset();
ASSERT_TRUE(chptr_2);
EXPECT_TRUE(chptr_1 == chptr_2);
- // Reference count is now 3 on the callout handle - two for pointers here,
- // one for the static pointer in the function. The count is 2 for the
- // object pointed to by pktptr_1 - one for that pointer and one for the
- // pointer in the function.
- EXPECT_EQ(3, chptr_1.use_count());
- EXPECT_EQ(3, chptr_2.use_count());
- EXPECT_EQ(2, pktptr_1.use_count());
- EXPECT_EQ(1, pktptr_2.use_count());
-
// Now ask for a CalloutHandle for a different object. This should return
// a different CalloutHandle.
chptr_2 = getCalloutHandle(pktptr_2);
EXPECT_FALSE(chptr_1 == chptr_2);
-
- // Check reference counts. The getCalloutHandle function should be storing
- // pointers to objects pointed to by chptr_1, pktptr_1, chptr_2 and pktptr_2.
- EXPECT_EQ(2, chptr_1.use_count());
- EXPECT_EQ(2, pktptr_1.use_count());
- EXPECT_EQ(2, chptr_2.use_count());
- EXPECT_EQ(2, pktptr_2.use_count());
-
- // Now clear the pktptr_1 simulating the situation when the DHCP packet
- // has been fully processed.
- pktptr_1.reset();
-
- // Retrieve chptr_2 again to trigger clearing unused pointer chptr_1.
- chptr_2 = getCalloutHandle(pktptr_2);
- ASSERT_TRUE(chptr_2);
-
- // This should merely affect chptr_1 and pktptr_1, but the pointers for the
- // second packet should remain unchanged.
- EXPECT_EQ(1, chptr_1.use_count());
- EXPECT_EQ(2, chptr_2.use_count());
- EXPECT_EQ(2, pktptr_2.use_count());
-
- // Now try clearing the stored pointers.
- Pkt4Ptr pktptr_empty;
- ASSERT_FALSE(pktptr_empty);
-
- CalloutHandlePtr chptr_empty = getCalloutHandle(pktptr_empty);
- EXPECT_FALSE(chptr_empty);
-
- // Reference counts should be back to 1 for the CalloutHandles and the
- // Packet pointers.
- EXPECT_EQ(1, chptr_1.use_count());
- EXPECT_EQ(1, chptr_2.use_count());
- EXPECT_EQ(1, pktptr_2.use_count());
}
// The followings is a trivial test to check that if the template function
lib_LTLIBRARIES = libkea-hooks.la
libkea_hooks_la_SOURCES =
libkea_hooks_la_SOURCES += callout_handle.cc callout_handle.h
+libkea_hooks_la_SOURCES += callout_handle_associate.cc callout_handle_associate.h
libkea_hooks_la_SOURCES += callout_manager.cc callout_manager.h
libkea_hooks_la_SOURCES += hooks.h
libkea_hooks_la_SOURCES += hooks_log.cc hooks_log.h
--- /dev/null
+// Copyright (C) 2018 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 <hooks/callout_handle_associate.h>
+#include <hooks/hooks_manager.h>
+
+namespace isc {
+namespace hooks {
+
+CalloutHandleAssociate::CalloutHandleAssociate()
+ : callout_handle_() {
+}
+
+CalloutHandlePtr
+CalloutHandleAssociate::getCalloutHandle() {
+ if (!callout_handle_) {
+ callout_handle_ = HooksManager::createCalloutHandle();
+ }
+
+ return (callout_handle_);
+}
+
+} // end of namespace isc::hooks
+} // end of namespace isc
--- /dev/null
+// Copyright (C) 2018 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 CALLOUT_HANDLE_ASSOCIATE_H
+#define CALLOUT_HANDLE_ASSOCIATE_H
+
+#include <hooks/callout_handle.h>
+
+namespace isc {
+namespace hooks {
+
+/// @brief Base class for classes which need to be associated with
+/// a @c CalloutHandle object.
+///
+/// The @c CalloutHandle is an object used to pass various parameters
+/// between Kea and the callouts. The Kea servers usually invoke
+/// multiple different callouts for a single packet such as DHCP
+/// packet, control command etc. Therefore, it is required to
+/// associate this packet with an instance of the callout handle, so
+/// this instance can be used for all callouts invoked for this
+/// packet.
+///
+/// Previously this association was made by the @c CalloutHandleStore
+/// class. However, with the introduction of parallel processing
+/// of packets (DHCP packets parking) it became awkward to use.
+/// Attempts to extend this class to hold a map of associations
+/// failed because of no easy way to garbage collect unused handles.
+///
+/// The easiest way to deal with this is to provide ownership of the
+/// @c CalloutHandle to the object with which it is associated. The
+/// class of this object needs to derive from this class. When the
+/// object (e.g. DHCP packet) goes out of scope and is destroyed
+/// this instance is destroyed as well.
+class CalloutHandleAssociate {
+public:
+
+ /// @brief Constructor.
+ CalloutHandleAssociate();
+
+ /// @brief Returns callout handle.
+ ///
+ /// The callout handle is created if it doesn't exist. Subsequent
+ /// calls to this method always return the same handle.
+ ///
+ /// @return Pointer to the callout handle.
+ CalloutHandlePtr getCalloutHandle();
+
+protected:
+
+ /// @brief Callout handle stored.
+ CalloutHandlePtr callout_handle_;
+
+
+};
+
+} // end of isc::hooks
+} // end of isc
+
+#endif
TESTS += run_unittests
run_unittests_SOURCES = run_unittests.cc
run_unittests_SOURCES += callout_handle_unittest.cc
+run_unittests_SOURCES += callout_handle_associate_unittest.cc
run_unittests_SOURCES += callout_manager_unittest.cc
run_unittests_SOURCES += common_test_class.h
run_unittests_SOURCES += handles_unittest.cc
--- /dev/null
+// Copyright (C) 2018 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 <hooks/callout_handle.h>
+#include <hooks/callout_handle_associate.h>
+#include <gtest/gtest.h>
+
+using namespace isc::hooks;
+
+namespace {
+
+// This test verifies that the callout handle can be created and
+// retrieved from the CalloutHandleAssociate.
+TEST(CalloutHandleAssociate, getCalloutHandle) {
+ CalloutHandleAssociate associate;
+ // The handle should be initialized and returned.
+ CalloutHandlePtr callout_handle = associate.getCalloutHandle();
+ ASSERT_TRUE(callout_handle);
+
+ // When calling the second time, the same handle should be returned.
+ CalloutHandlePtr callout_handle2 = associate.getCalloutHandle();
+ EXPECT_TRUE(callout_handle == callout_handle2);
+
+ // A different associate should produce a different handle.
+ CalloutHandleAssociate associate2;
+ callout_handle2 = associate2.getCalloutHandle();
+ EXPECT_FALSE(callout_handle == callout_handle2);
+}
+
+} // end of anonymous namespace
projects / thirdparty / kea.git / commitdiff
