From 47602142f870ab60285c4f7cefea6c0c4b93a5df Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Wed, 5 Dec 2018 13:27:02 -0500 Subject: [PATCH 1/7] [#278,!162] Minor simplification of PacketQueue<> interface src/lib/dhcp/packet_queue.h PacketQueue<> enqueuePacket() dequeuePacket() - are now pure virtual, and no longer accept a QueueEnd parameter shouldDropPacket() eatPackets() pushPacket() popPacket() peek() - removed (they are now only in PackeQueueRing<> and its derivations src/lib/dhcp/packet_queue_ring.h New fiel which houses PacketQueueRing<> and its derivations --- src/lib/dhcp/Makefile.am | 2 + src/lib/dhcp/packet_queue.h | 283 +----------------- src/lib/dhcp/packet_queue_mgr4.cc | 1 + src/lib/dhcp/packet_queue_mgr6.cc | 1 + src/lib/dhcp/packet_queue_ring.h | 265 ++++++++++++++++ src/lib/dhcp/tests/packet_queue4_unittest.cc | 217 ++++++-------- src/lib/dhcp/tests/packet_queue6_unittest.cc | 217 ++++++-------- .../dhcp/tests/packet_queue_mgr4_unittest.cc | 1 + .../dhcp/tests/packet_queue_mgr6_unittest.cc | 1 + 9 files changed, 475 insertions(+), 513 deletions(-) create mode 100644 src/lib/dhcp/packet_queue_ring.h diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am index eb1efb5ac2..6409e6c2e8 100644 --- a/src/lib/dhcp/Makefile.am +++ b/src/lib/dhcp/Makefile.am @@ -47,6 +47,7 @@ libkea_dhcp___la_SOURCES += packet_queue.h libkea_dhcp___la_SOURCES += packet_queue_mgr.h libkea_dhcp___la_SOURCES += packet_queue_mgr4.cc packet_queue_mgr4.h libkea_dhcp___la_SOURCES += packet_queue_mgr6.cc packet_queue_mgr6.h +libkea_dhcp___la_SOURCES += packet_queue_ring.h libkea_dhcp___la_SOURCES += pkt.cc pkt.h libkea_dhcp___la_SOURCES += pkt4.cc pkt4.h libkea_dhcp___la_SOURCES += pkt4o6.cc pkt4o6.h @@ -129,6 +130,7 @@ libkea_dhcp___include_HEADERS = \ packet_queue_mgr.h \ packet_queue_mgr4.h \ packet_queue_mgr6.h \ + packet_queue_ring.h \ pkt.h \ pkt4.h \ pkt4o6.h \ diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index 004bd36dff..71f0835ff7 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -42,7 +42,6 @@ enum class QueueEnd { /// implementations which may be used by @c IfaceMgr to store /// inbound packets until they are a dequeued for processing. /// @note Derivations of this class MUST BE thread-safe. -/// @endnote /// template class PacketQueue { @@ -62,114 +61,25 @@ public: /// @brief Adds a packet to the queue /// - /// Calls @c shouldDropPacket to determine if the packet should be queued - /// or dropped. If it should be queued it is added to the end of the - /// queue specified by the "to" parameter. + /// Adds the packet to the queue. Derivations determine + /// which packets to queue and how to queue them. /// /// @param packet packet to enqueue /// @param source socket the packet came from - /// @param to end of the queue from which to remove packet(s). - /// Defaults to BACK. - /// - void enqueuePacket(PacketTypePtr packet, const SocketInfo& source, - const QueueEnd& to=QueueEnd::BACK) { - if (!shouldDropPacket(packet, source)) { - pushPacket(packet, to); - } - } + virtual void enqueuePacket(PacketTypePtr packet, const SocketInfo& source) = 0; /// @brief Dequeues the next packet from the queue /// - /// Calls @eatPackets to discard packets as needed, and then - /// dequeues the next packet (if any) and returns it. Packets - /// are dequeued from the end of the queue specified by the "from" - /// parameter. - /// - /// @param from end of the queue from which to remove packet(s). - /// Defaults to FRONT. - /// - /// @return A pointer to dequeued packet, or an empty pointer - /// if the queue is empty. - PacketTypePtr dequeuePacket(const QueueEnd& from=QueueEnd::FRONT) { - eatPackets(from); - return(popPacket(from)); - } - - /// @brief Determines if a packet should be discarded. - /// - /// This function is called at the beginning of @c enqueuePacket and - /// provides an opportunity to examine the packet and its source - /// and decide whether it should be dropped or added to the queue. - /// Derivations are expected to provide implementations based on - /// their own requirements. Bear in mind that the packet has NOT - /// been unpacked at this point. The default implementation simply - /// returns false. - /// - /// @param packet the packet under consideration - /// @param source the socket the packet came from - /// - /// @return true if the packet should be dropped, false if it should be - /// kept. - virtual bool shouldDropPacket(PacketTypePtr /* packet */, - const SocketInfo& /* source */) { - return (false); - } - - /// @brief Discards packets from one end of the queue. - /// - /// This function is called at the beginning of @c dequeuePacket and - /// provides an opportunity to examine and discard packets from - /// the queue prior to dequeuing the next packet to be - /// processed. Derivations are expected to provide implementations - /// based on their own requirements. The default implemenation is to - /// to simply return without skipping any packets. - /// - /// @param from end of the queue from which packets should discarded - /// This is passed in from @c dequeuePackets. - /// - /// @param from specifies the end of the queue from which packets - /// should be discarded. - /// - /// @return The number of packets discarded. - virtual int eatPackets(const QueueEnd& /* from */) { - return (0); - } - - /// @brief Pushes a packet onto the queue - /// - /// Adds a packet onto the end of queue specified. - /// - /// @param packet packet to add to the queue - /// @param to specifies the end of the queue to which the packet - /// should be added. - virtual void pushPacket(PacketTypePtr& packet, const QueueEnd& to=QueueEnd::BACK) = 0; - - /// @brief Pops a packet from the queue - /// - /// Removes a packet from the end of the queue specified and returns it. - /// - /// @param from specifies the end of the queue from which the packet - /// should be taken. + /// Dequeues the next packet (if any) and returns it. Derivations determine + /// how packets are dequeued. /// /// @return A pointer to dequeued packet, or an empty pointer /// if the queue is empty. - virtual PacketTypePtr popPacket(const QueueEnd& from=QueueEnd::FRONT) = 0; - - /// @brief Gets the packet currently at one end of the queue - /// - /// Returns a pointer the packet at the specified end of the - /// queue without dequeuing it. - /// - /// @param from specifies which end of the queue to examine. - /// - /// @return A pointer to packet, or an empty pointer if the - /// queue is empty. - virtual const PacketTypePtr peek(const QueueEnd& from=QueueEnd::FRONT) const = 0; + virtual PacketTypePtr dequeuePacket() = 0; /// @brief return True if the queue is empty. virtual bool empty() const = 0; - /// @todo size may not apply either... what if there are two internal buffers? /// @brief Returns the current number of packets in the buffer. virtual size_t getSize() const = 0; @@ -225,187 +135,6 @@ typedef boost::shared_ptr> PacketQueue4Ptr; /// DHCPv6 packet queue factories. typedef boost::shared_ptr> PacketQueue6Ptr; - -/// @brief Provides an abstract ring-buffer implementation of the PacketQueue interface. -template -class PacketQueueRing : public PacketQueue { -public: - /// @brief Minimum queue capacity permitted. Below five is pretty much - /// nonsensical. - static const size_t MIN_RING_CAPACITY = 5; - - /// @brief Constructor - /// - /// @param queue_type logical name of the queue implementation - /// @param capacity maximum number of packets the queue can hold - PacketQueueRing(const std::string& queue_type, size_t capacity) - : PacketQueue(queue_type) { - queue_.set_capacity(capacity); - } - - /// @brief virtual Destructor - virtual ~PacketQueueRing(){}; - - /// @brief Pushes a packet onto the queue - /// - /// Adds a packet onto the end of queue specified. Note that this - /// function is protected by a Mutex. - /// - /// @param packet packet to add to the queue - /// @param to specifies the end of the queue to which the packet - /// should be added. - virtual void pushPacket(PacketTypePtr& packet, const QueueEnd& to=QueueEnd::BACK) { - isc::util::thread::Mutex::Locker lock(mutex_); - if (to == QueueEnd::BACK) { - queue_.push_back(packet); - } else { - queue_.push_front(packet); - } - } - - /// @brief Pops a packet from the queue - /// - /// Removes a packet from the end of the queue specified and returns it. Note - /// that this function is protected by a Mutex. - /// - /// @param from specifies the end of the queue from which the packet - /// should be taken. - /// - /// @return A pointer to dequeued packet, or an empty pointer - /// if the queue is empty. - virtual PacketTypePtr popPacket(const QueueEnd& from = QueueEnd::FRONT) { - isc::util::thread::Mutex::Locker lock(mutex_); - PacketTypePtr packet; - if (queue_.empty()) { - return (packet); - } - - if (from == QueueEnd::FRONT) { - packet = queue_.front(); - queue_.pop_front(); - } else { - packet = queue_.back(); - queue_.pop_back(); - } - - return (packet); - } - - - /// @brief Gets the packet currently at one end of the queue - /// - /// Returns a pointer the packet at the specified end of the - /// queue without dequeuing it. - /// - /// @param from specifies which end of the queue to examine. - /// - /// @return A pointer to packet, or an empty pointer if the - /// queue is empty. - virtual const PacketTypePtr peek(const QueueEnd& from=QueueEnd::FRONT) const { - PacketTypePtr packet; - if (!queue_.empty()) { - packet = (from == QueueEnd::FRONT ? queue_.front() : queue_.back()); - } - - return (packet); - } - - /// @brief Returns True if the queue is empty. - virtual bool empty() const { - return(queue_.empty()); - } - - /// @brief Returns the maximum number of packets allowed in the buffer. - virtual size_t getCapacity() const { - return (queue_.capacity()); - } - - /// @brief Sets the maximum number of packets allowed in the buffer. - /// - /// @todo - do we want to change size on the fly? This might need - /// to be private, called only by constructor - /// - /// @throw BadValue if capacity is too low. - virtual void setCapacity(size_t capacity) { - if (capacity < MIN_RING_CAPACITY) { - isc_throw(BadValue, "Queue capacity of " << capacity - << " is invalid. It must be at least " - << MIN_RING_CAPACITY); - } - - /// @todo should probably throw if it's zero - queue_.set_capacity(capacity); - } - - /// @brief Returns the current number of packets in the buffer. - virtual size_t getSize() const { - return (queue_.size()); - } - - /// @brief Discards all packets currently in the buffer. - virtual void clear() { - queue_.clear(); - } - - /// @brief Fetches pertinent information - virtual data::ElementPtr getInfo() const { - data::ElementPtr info = PacketQueue::getInfo(); - info->set("capacity", data::Element::create(static_cast(getCapacity()))); - info->set("size", data::Element::create(static_cast(getSize()))); - return(info); - } - -private: - - /// @brief Packet queue - boost::circular_buffer queue_; - - /// @brief Mutex for protecting queue accesses. - isc::util::thread::Mutex mutex_; -}; - - -/// @brief DHCPv4 packet queue buffer implementation -/// -/// This implementation does not (currently) add any drop -/// or packet skip logic, it operates as a verbatim ring -/// queue for DHCPv4 packets. -/// -class PacketQueueRing4 : public PacketQueueRing { -public: - /// @brief Constructor - /// - /// @param queue_type logical name of the queue implementation - /// @param capacity maximum number of packets the queue can hold - PacketQueueRing4(const std::string& queue_type, size_t capacity) - : PacketQueueRing(queue_type, capacity) { - }; - - /// @brief virtual Destructor - virtual ~PacketQueueRing4(){} -}; - -/// @brief DHCPv6 packet queue buffer implementation -/// -/// This implementation does not (currently) add any drop -/// or packet skip logic, it operates as a verbatim ring -/// queue for DHCPv6 packets. -/// -class PacketQueueRing6 : public PacketQueueRing { -public: - /// @brief Constructor - /// - /// @param queue_type logical name of the queue implementation - /// @param capacity maximum number of packets the queue can hold - PacketQueueRing6(const std::string& queue_type, size_t capacity) - : PacketQueueRing(queue_type, capacity) { - }; - - /// @brief virtual Destructor - virtual ~PacketQueueRing6(){} -}; - - }; // namespace isc::dhcp }; // namespace isc diff --git a/src/lib/dhcp/packet_queue_mgr4.cc b/src/lib/dhcp/packet_queue_mgr4.cc index b7ba4ffeb0..1918a6dfbf 100644 --- a/src/lib/dhcp/packet_queue_mgr4.cc +++ b/src/lib/dhcp/packet_queue_mgr4.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include #include diff --git a/src/lib/dhcp/packet_queue_mgr6.cc b/src/lib/dhcp/packet_queue_mgr6.cc index bcb83ec527..2f08fdf6e4 100644 --- a/src/lib/dhcp/packet_queue_mgr6.cc +++ b/src/lib/dhcp/packet_queue_mgr6.cc @@ -5,6 +5,7 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include #include diff --git a/src/lib/dhcp/packet_queue_ring.h b/src/lib/dhcp/packet_queue_ring.h new file mode 100644 index 0000000000..cea139465d --- /dev/null +++ b/src/lib/dhcp/packet_queue_ring.h @@ -0,0 +1,265 @@ +// 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 PACKET_QUEUE_RING_H +#define PACKET_QUEUE_RING_H + +#include +#include + +#include +#include +#include + +namespace isc { + +namespace dhcp { + +/// @brief Provides an abstract ring-buffer implementation of the PacketQueue interface. +template +class PacketQueueRing : public PacketQueue { +public: + /// @brief Minimum queue capacity permitted. Below five is pretty much + /// nonsensical. + static const size_t MIN_RING_CAPACITY = 5; + + /// @brief Constructor + /// + /// @param queue_type logical name of the queue implementation + /// @param capacity maximum number of packets the queue can hold + PacketQueueRing(const std::string& queue_type, size_t capacity) + : PacketQueue(queue_type) { + queue_.set_capacity(capacity); + } + + /// @brief virtual Destructor + virtual ~PacketQueueRing(){}; + + /// @brief Adds a packet to the queue + /// + /// Calls @c shouldDropPacket to determine if the packet should be queued + /// or dropped. If it should be queued it is added to the end of the + /// queue specified by the "to" parameter. + /// + /// @param packet packet to enqueue + /// @param source socket the packet came from + virtual void enqueuePacket(PacketTypePtr packet, const SocketInfo& source) { + if (!shouldDropPacket(packet, source)) { + pushPacket(packet); + } + } + + /// @brief Dequeues the next packet from the queue + /// + /// Dequeues the next packet (if any) and returns it. + /// + /// @return A pointer to dequeued packet, or an empty pointer + /// if the queue is empty. + virtual PacketTypePtr dequeuePacket() { + eatPackets(QueueEnd::FRONT); + return(popPacket()); + } + + /// @brief Determines if a packet should be discarded. + /// + /// This function is called in @c enqueuePackets for each packet + /// in its packet list. It provides an opportunity to examine the + /// packet and its source and decide whether it should be dropped + /// or added to the queue. Derivations are expected to provide + /// implementations based on their own requirements. Bear in mind + /// that the packet has NOT been unpacked at this point. The default + /// implementation simply returns false (i.e. keep the packet). + /// + /// @param packet the packet under consideration + /// @param source the socket the packet came from + /// + /// @return true if the packet should be dropped, false if it should be + /// kept. + virtual bool shouldDropPacket(PacketTypePtr /* packet */, + const SocketInfo& /* source */) { + return (false); + } + + /// @brief Discards packets from one end of the queue. + /// + /// This function is called at the beginning of @c dequeuePacket and + /// provides an opportunity to examine and discard packets from + /// the queue prior to dequeuing the next packet to be + /// processed. Derivations are expected to provide implementations + /// based on their own requirements. The default implemenation is to + /// to simply return without skipping any packets. + /// + /// @param from end of the queue from which packets should discarded + /// This is passed in from @c dequeuePackets. + /// + /// @param from specifies the end of the queue from which packets + /// should be discarded. + /// + /// @return The number of packets discarded. + virtual int eatPackets(const QueueEnd& /* from */) { + return (0); + } + + /// @brief Pushes a packet onto the queue + /// + /// Adds a packet onto the end of queue specified. + /// + /// @param packet packet to add to the queue + /// @param to specifies the end of the queue to which the packet + /// should be added. + virtual void pushPacket(PacketTypePtr& packet, const QueueEnd& to=QueueEnd::BACK) { + if (to == QueueEnd::BACK) { + queue_.push_back(packet); + } else { + queue_.push_front(packet); + } + } + + /// @brief Pops a packet from the queue + /// + /// Removes a packet from the end of the queue specified and returns it. + /// + /// @param from specifies the end of the queue from which the packet + /// should be taken. It locks the queue's Mutex upon entry. + /// + /// @return A pointer to dequeued packet, or an empty pointer + /// if the queue is empty. + virtual PacketTypePtr popPacket(const QueueEnd& from = QueueEnd::FRONT) { + isc::util::thread::Mutex::Locker lock(mutex_); + PacketTypePtr packet; + if (queue_.empty()) { + return (packet); + } + + if (from == QueueEnd::FRONT) { + packet = queue_.front(); + queue_.pop_front(); + } else { + packet = queue_.back(); + queue_.pop_back(); + } + + return (packet); + } + + + /// @brief Gets the packet currently at one end of the queue + /// + /// Returns a pointer the packet at the specified end of the + /// queue without dequeuing it. + /// + /// @param from specifies which end of the queue to examine. + /// + /// @return A pointer to packet, or an empty pointer if the + /// queue is empty. + virtual const PacketTypePtr peek(const QueueEnd& from=QueueEnd::FRONT) const { + PacketTypePtr packet; + if (!queue_.empty()) { + packet = (from == QueueEnd::FRONT ? queue_.front() : queue_.back()); + } + + return (packet); + } + + /// @brief Returns True if the queue is empty. + virtual bool empty() const { + return(queue_.empty()); + } + + /// @brief Returns the maximum number of packets allowed in the buffer. + virtual size_t getCapacity() const { + return (queue_.capacity()); + } + + /// @brief Sets the maximum number of packets allowed in the buffer. + /// + /// @todo - do we want to change size on the fly? This might need + /// to be private, called only by constructor + /// + /// @throw BadValue if capacity is too low. + virtual void setCapacity(size_t capacity) { + if (capacity < MIN_RING_CAPACITY) { + isc_throw(BadValue, "Queue capacity of " << capacity + << " is invalid. It must be at least " + << MIN_RING_CAPACITY); + } + + /// @todo should probably throw if it's zero + queue_.set_capacity(capacity); + } + + /// @brief Returns the current number of packets in the buffer. + virtual size_t getSize() const { + return (queue_.size()); + } + + /// @brief Discards all packets currently in the buffer. + virtual void clear() { + queue_.clear(); + } + + /// @brief Fetches pertinent information + virtual data::ElementPtr getInfo() const { + data::ElementPtr info = PacketQueue::getInfo(); + info->set("capacity", data::Element::create(static_cast(getCapacity()))); + info->set("size", data::Element::create(static_cast(getSize()))); + return(info); + } + +private: + + /// @brief Packet queue + boost::circular_buffer queue_; + + /// @brief Mutex for protecting queue accesses. + isc::util::thread::Mutex mutex_; +}; + + +/// @brief DHCPv4 packet queue buffer implementation +/// +/// This implementation does not (currently) add any drop +/// or packet skip logic, it operates as a verbatim ring +/// queue for DHCPv4 packets. +/// +class PacketQueueRing4 : public PacketQueueRing { +public: + /// @brief Constructor + /// + /// @param queue_type logical name of the queue implementation + /// @param capacity maximum number of packets the queue can hold + PacketQueueRing4(const std::string& queue_type, size_t capacity) + : PacketQueueRing(queue_type, capacity) { + }; + + /// @brief virtual Destructor + virtual ~PacketQueueRing4(){} +}; + +/// @brief DHCPv6 packet queue buffer implementation +/// +/// This implementation does not (currently) add any drop +/// or packet skip logic, it operates as a verbatim ring +/// queue for DHCPv6 packets. +/// +class PacketQueueRing6 : public PacketQueueRing { +public: + /// @brief Constructor + /// + /// @param queue_type logical name of the queue implementation + /// @param capacity maximum number of packets the queue can hold + PacketQueueRing6(const std::string& queue_type, size_t capacity) + : PacketQueueRing(queue_type, capacity) { + }; + + /// @brief virtual Destructor + virtual ~PacketQueueRing6(){} +}; + +}; // namespace isc::dhcp +}; // namespace isc + +#endif // PACKET_QUEUE_RING_H diff --git a/src/lib/dhcp/tests/packet_queue4_unittest.cc b/src/lib/dhcp/tests/packet_queue4_unittest.cc index b0666b073a..edc75ca49f 100644 --- a/src/lib/dhcp/tests/packet_queue4_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue4_unittest.cc @@ -6,7 +6,7 @@ #include -#include +#include #include #include @@ -83,135 +83,124 @@ public: // Verifies use of the generic PacketQueue interface to // construct a queue implementation. -TEST(TestQueue4, interfaceBasics) { +TEST(PacketQueueRing4, interfaceBasics) { // Verify we can create a queue - PacketQueue4Ptr q4(new TestQueue4(100)); - ASSERT_TRUE(q4); + PacketQueue4Ptr q(new PacketQueueRing4("kea-ring4",100)); + ASSERT_TRUE(q); // It should be empty. - EXPECT_TRUE(q4->empty()); + EXPECT_TRUE(q->empty()); // Type should match. - EXPECT_EQ("kea-ring4", q4->getQueueType()); + EXPECT_EQ("kea-ring4", q->getQueueType()); // Fetch the queue info and verify it has all the expected values. - checkInfo(q4, "{ \"capacity\": 100, \"queue-type\": \"kea-ring4\", \"size\": 0 }"); + checkInfo(q, "{ \"capacity\": 100, \"queue-type\": \"kea-ring4\", \"size\": 0 }"); } -// Verifies the basic mechanics of the adding and -// removing packets to and from the ring buffer. -TEST(TestQueue4, ringTest) { - PacketQueue4Ptr q4(new TestQueue4(3)); +// Verifies the higher level functions of queueing and dequeueing +// from the ring buffer. +TEST(PacketQueueRing4, enqueueDequeueTest) { + PacketQueue4Ptr q(new PacketQueueRing4("kea-ring4", 3)); // Fetch the queue info and verify it has all the expected values. - checkInfo(q4, "{ \"capacity\": 3, \"queue-type\": \"kea-ring4\", \"size\": 0 }"); + checkInfo(q, "{ \"capacity\": 3, \"queue-type\": \"kea-ring4\", \"size\": 0 }"); // Enqueue five packets. The first two should be pushed off. SocketInfo sock1(isc::asiolink::IOAddress("127.0.0.1"), 777, 10); + for (int i = 1; i < 6; ++i) { Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1000+i)); - ASSERT_NO_THROW(q4->enqueuePacket(pkt, sock1)); - checkIntStat(q4, "size", (i > 3 ? 3 : i)); + ASSERT_NO_THROW(q->enqueuePacket(pkt, sock1)); } + // Fetch the queue info and verify it has all the expected values. - checkInfo(q4, "{ \"capacity\": 3, \"queue-type\": \"kea-ring4\", \"size\": 3 }"); + checkInfo(q, "{ \"capacity\": 3, \"queue-type\": \"kea-ring4\", \"size\": 3 }"); + + // We should have transids 1003,1004,1005 + Pkt4Ptr pkt; + for (int i = 3; i < 6; ++i) { + ASSERT_NO_THROW(pkt = q->dequeuePacket()); + ASSERT_TRUE(pkt); + EXPECT_EQ(1000 + i, pkt->getTransid()); + } + + // Queue should be empty. + ASSERT_TRUE(q->empty()); + + // Dequeuing should fail safely, with an empty return. + ASSERT_NO_THROW(pkt = q->dequeuePacket()); + ASSERT_FALSE(pkt); + + // Enqueue three more packets. + for (int i = 0; i < 3; ++i) { + Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1000+i)); + ASSERT_NO_THROW(q->enqueuePacket(pkt, sock1)); + } + + checkIntStat(q, "size", 3); + + // Let's flush the buffer and then verify it is empty. + q->clear(); + EXPECT_TRUE(q->empty()); + checkIntStat(q, "size", 0); +} + +// Verifies peeking, pushing, and popping which +// are unique to PacketQueueRing<> derivations. +TEST(PacketQueueRing4, peekPushPopTest) { + PacketQueueRing4 q("kea-ring4", 3); + + // Push five packets onto the end. The first two should get pushed off. + for (int i = 1; i < 6; ++i) { + Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1000+i)); + ASSERT_NO_THROW(q.pushPacket(pkt)); + } + + // We should have three. + ASSERT_EQ(3, q.getSize()); // We should have transids 1005,1004,1003 (back to front) // Peek front should be transid 1003. Pkt4Ptr pkt; - ASSERT_NO_THROW(pkt = q4->peek(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); // Peek back should be transid 1005. - ASSERT_NO_THROW(pkt = q4->peek(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1005, pkt->getTransid()); // Pop front should return transid 1003. - ASSERT_NO_THROW(pkt = q4->popPacket(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); // Pop back should return transid 1005. - ASSERT_NO_THROW(pkt = q4->popPacket(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1005, pkt->getTransid()); // Peek front should be transid 1004. - ASSERT_NO_THROW(pkt = q4->peek(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Peek back should be transid 1004. - ASSERT_NO_THROW(pkt = q4->peek(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Pop front should return transid 1004. - ASSERT_NO_THROW(pkt = q4->popPacket(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Pop front should return an empty pointer. - ASSERT_NO_THROW(pkt = q4->popPacket(QueueEnd::BACK)); - ASSERT_FALSE(pkt); - - // Enqueue three packets. - for (int i = 1; i < 3; ++i) { - Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1000+i)); - ASSERT_NO_THROW(q4->enqueuePacket(pkt, sock1)); - checkIntStat(q4, "size", i); - } - - // Let's flush the buffer and then verify it is empty. - q4->clear(); - EXPECT_TRUE(q4->empty()); - checkIntStat(q4, "size", 0); -} - -// Verifies the higher level functions of queueing and -// dequeueing with drop and skip logic disabled. -TEST(TestQueue4, enqueueDequeueTest) { - PacketQueue4Ptr q4(new TestQueue4(100)); - EXPECT_TRUE(q4->empty()); - - SocketInfo sock1(isc::asiolink::IOAddress("127.0.0.1"), 777, 10); - - // Enqueue the first packet. - Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1002)); - ASSERT_NO_THROW(q4->enqueuePacket(pkt, sock1)); - checkIntStat(q4, "size", 1); - - // Enqueue a packet onto the front. - pkt.reset(new Pkt4(DHCPDISCOVER, 1003)); - ASSERT_NO_THROW(q4->enqueuePacket(pkt, sock1, QueueEnd::FRONT)); - checkIntStat(q4, "size", 2); - - // Enqueue a packet onto the back. - pkt.reset(new Pkt4(DHCPDISCOVER, 1001)); - ASSERT_NO_THROW(q4->enqueuePacket(pkt, sock1, QueueEnd::BACK)); - checkIntStat(q4, "size", 3); - - // By default we dequeue from the front. We should get transid 1003. - ASSERT_NO_THROW(pkt = q4->dequeuePacket()); - ASSERT_TRUE(pkt); - EXPECT_EQ(1003, pkt->getTransid()); - - // Dequeue from the back, we should get transid 1001. - ASSERT_NO_THROW(pkt = q4->dequeuePacket(QueueEnd::BACK)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1001, pkt->getTransid()); - - // Dequeue from the front, we should get transid 1002. - ASSERT_NO_THROW(pkt = q4->dequeuePacket(QueueEnd::FRONT)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1002, pkt->getTransid()); - - // Queue should be empty. - ASSERT_NO_THROW(pkt = q4->dequeuePacket()); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::BACK)); ASSERT_FALSE(pkt); } @@ -219,58 +208,58 @@ TEST(TestQueue4, enqueueDequeueTest) { // This accesses it's queue instance as a TestQueue4, rather than // a PacketQueue4Ptr, to provide access to TestQueue4 specifics. TEST(TestQueue4, shouldDropPacketTest) { - TestQueue4 q4(100); - EXPECT_TRUE(q4.empty()); - ASSERT_FALSE(q4.drop_enabled_); - ASSERT_EQ(0, q4.eat_count_); + TestQueue4 q(100); + EXPECT_TRUE(q.empty()); + ASSERT_FALSE(q.drop_enabled_); + ASSERT_EQ(0, q.eat_count_); - SocketInfo sockEven(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); - SocketInfo sockOdd(isc::asiolink::IOAddress("127.0.0.1"), 777, 11); + SocketInfo sock_even(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); + SocketInfo sock_odd(isc::asiolink::IOAddress("127.0.0.1"), 777, 11); // Drop is not enabled. // We should be able to enqueue a packet with even numbered values. Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1002)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sockEven)); - EXPECT_EQ(1, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_even)); + ASSERT_EQ(1, q.getSize()); // We should be able to enqueue a packet with odd numbered values. pkt.reset(new Pkt4(DHCPDISCOVER, 1003)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(2, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + ASSERT_EQ(2, q.getSize()); // Enable drop logic. - q4.drop_enabled_ = true; + q.drop_enabled_ = true; // We should not be able to add one with an even-numbered transid. pkt.reset(new Pkt4(DHCPDISCOVER, 1004)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(2, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + ASSERT_EQ(2, q.getSize()); // We should not be able to add one with from even-numbered port. pkt.reset(new Pkt4(DHCPDISCOVER, 1005)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sockEven)); - EXPECT_EQ(2, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_even)); + EXPECT_EQ(2, q.getSize()); // We should be able to add one with an odd-numbered values. pkt.reset(new Pkt4(DHCPDISCOVER, 1007)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(3, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + EXPECT_EQ(3, q.getSize()); // Dequeue them and make sure they are as expected: 1002,1003, and 1007. - ASSERT_NO_THROW(pkt = q4.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1002, pkt->getTransid()); - ASSERT_NO_THROW(pkt = q4.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); - ASSERT_NO_THROW(pkt = q4.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1007, pkt->getTransid()); // Queue should be empty. - ASSERT_NO_THROW(pkt = q4.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_FALSE(pkt); } @@ -278,10 +267,10 @@ TEST(TestQueue4, shouldDropPacketTest) { // This accesses it's queue instance as a TestQueue4, rather than // a PacketQueue4Ptr, to provide access to TestQueue4 specifics. TEST(TestQueue4, eatPacketsTest) { - TestQueue4 q4(100); - EXPECT_TRUE(q4.empty()); - ASSERT_FALSE(q4.drop_enabled_); - ASSERT_EQ(0, q4.eat_count_); + TestQueue4 q(100); + EXPECT_TRUE(q.empty()); + ASSERT_FALSE(q.drop_enabled_); + ASSERT_EQ(0, q.eat_count_); SocketInfo sock(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); @@ -289,25 +278,17 @@ TEST(TestQueue4, eatPacketsTest) { // Let's add five packets. for (int i = 1; i < 6; ++i) { pkt.reset(new Pkt4(DHCPDISCOVER, 1000 + i)); - ASSERT_NO_THROW(q4.enqueuePacket(pkt, sock)); - EXPECT_EQ(i, q4.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock)); + ASSERT_EQ(i, q.getSize()); } - // Setting eat count to two and dequeuing (from the front, by default), - // should discard 1001 and 1002, resulting in a dequeue of 1003. - q4.eat_count_ = 2; - ASSERT_NO_THROW(pkt = q4.dequeuePacket()); + // Setting eat count to two and dequeuing should discard 1001 + // and 1002, resulting in a dequeue of 1003. + q.eat_count_ = 2; + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); - EXPECT_EQ(2, q4.getSize()); - - // Setting eat count to one and dequeing from the back, should discard - // 1005 and dequeue 104. - q4.eat_count_ = 1; - ASSERT_NO_THROW(pkt = q4.dequeuePacket(QueueEnd::BACK)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1004, pkt->getTransid()); - EXPECT_EQ(0, q4.getSize()); + EXPECT_EQ(2, q.getSize()); } } // end of anonymous namespace diff --git a/src/lib/dhcp/tests/packet_queue6_unittest.cc b/src/lib/dhcp/tests/packet_queue6_unittest.cc index 526c1814af..087b0bfc58 100644 --- a/src/lib/dhcp/tests/packet_queue6_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue6_unittest.cc @@ -7,7 +7,7 @@ #include #include -#include +#include #include #include @@ -84,135 +84,124 @@ public: // Verifies use of the generic PacketQueue interface to // construct a queue implementation. -TEST(TestQueue6, interfaceBasics) { +TEST(PacketQueueRing6, interfaceBasics) { // Verify we can create a queue - PacketQueue6Ptr q6(new TestQueue6(100)); - ASSERT_TRUE(q6); + PacketQueue6Ptr q(new PacketQueueRing6("kea-ring6",100)); + ASSERT_TRUE(q); // It should be empty. - EXPECT_TRUE(q6->empty()); + EXPECT_TRUE(q->empty()); // Type should match. - EXPECT_EQ("kea-ring6", q6->getQueueType()); + EXPECT_EQ("kea-ring6", q->getQueueType()); // Fetch the queue info and verify it has all the expected values. - checkInfo(q6, "{ \"capacity\": 100, \"queue-type\": \"kea-ring6\", \"size\": 0 }"); + checkInfo(q, "{ \"capacity\": 100, \"queue-type\": \"kea-ring6\", \"size\": 0 }"); } -// Verifies the basic mechanics of the adding and -// removing packets to and from the ring buffer. -TEST(TestQueue6, ringTest) { - PacketQueue6Ptr q6(new TestQueue6(3)); +// Verifies the higher level functions of queueing and dequeueing +// from the ring buffer. +TEST(PacketQueueRing6, enqueueDequeueTest) { + PacketQueue6Ptr q(new PacketQueueRing6("kea-ring6", 3)); // Fetch the queue info and verify it has all the expected values. - checkInfo(q6, "{ \"capacity\": 3, \"queue-type\": \"kea-ring6\", \"size\": 0 }"); + checkInfo(q, "{ \"capacity\": 3, \"queue-type\": \"kea-ring6\", \"size\": 0 }"); // Enqueue five packets. The first two should be pushed off. SocketInfo sock1(isc::asiolink::IOAddress("127.0.0.1"), 777, 10); + for (int i = 1; i < 6; ++i) { Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1000+i)); - ASSERT_NO_THROW(q6->enqueuePacket(pkt, sock1)); - checkIntStat(q6, "size", (i > 3 ? 3 : i)); + ASSERT_NO_THROW(q->enqueuePacket(pkt, sock1)); } + // Fetch the queue info and verify it has all the expected values. - checkInfo(q6, "{ \"capacity\": 3, \"queue-type\": \"kea-ring6\", \"size\": 3 }"); + checkInfo(q, "{ \"capacity\": 3, \"queue-type\": \"kea-ring6\", \"size\": 3 }"); + + // We should have transids 1003,1004,1005 + Pkt6Ptr pkt; + for (int i = 3; i < 6; ++i) { + ASSERT_NO_THROW(pkt = q->dequeuePacket()); + ASSERT_TRUE(pkt); + EXPECT_EQ(1000 + i, pkt->getTransid()); + } + + // Queue should be empty. + ASSERT_TRUE(q->empty()); + + // Dequeuing should fail safely, with an empty return. + ASSERT_NO_THROW(pkt = q->dequeuePacket()); + ASSERT_FALSE(pkt); + + // Enqueue three more packets. + for (int i = 0; i < 3; ++i) { + Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1000+i)); + ASSERT_NO_THROW(q->enqueuePacket(pkt, sock1)); + } + + checkIntStat(q, "size", 3); + + // Let's flush the buffer and then verify it is empty. + q->clear(); + EXPECT_TRUE(q->empty()); + checkIntStat(q, "size", 0); +} + +// Verifies peeking, pushing, and popping which +// are unique to PacketQueueRing<> derivations. +TEST(PacketQueueRing6, peekPushPopTest) { + PacketQueueRing6 q("kea-ring6", 3); + + // Push five packets onto the end. The first two should get pushed off. + for (int i = 1; i < 6; ++i) { + Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1000+i)); + ASSERT_NO_THROW(q.pushPacket(pkt)); + } + + // We should have three. + ASSERT_EQ(3, q.getSize()); // We should have transids 1005,1004,1003 (back to front) // Peek front should be transid 1003. Pkt6Ptr pkt; - ASSERT_NO_THROW(pkt = q6->peek(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); // Peek back should be transid 1005. - ASSERT_NO_THROW(pkt = q6->peek(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1005, pkt->getTransid()); // Pop front should return transid 1003. - ASSERT_NO_THROW(pkt = q6->popPacket(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); // Pop back should return transid 1005. - ASSERT_NO_THROW(pkt = q6->popPacket(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1005, pkt->getTransid()); // Peek front should be transid 1004. - ASSERT_NO_THROW(pkt = q6->peek(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Peek back should be transid 1004. - ASSERT_NO_THROW(pkt = q6->peek(QueueEnd::BACK)); + ASSERT_NO_THROW(pkt = q.peek(QueueEnd::BACK)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Pop front should return transid 1004. - ASSERT_NO_THROW(pkt = q6->popPacket(QueueEnd::FRONT)); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::FRONT)); ASSERT_TRUE(pkt); EXPECT_EQ(1004, pkt->getTransid()); // Pop front should return an empty pointer. - ASSERT_NO_THROW(pkt = q6->popPacket(QueueEnd::BACK)); - ASSERT_FALSE(pkt); - - // Enqueue three packets. - for (int i = 1; i < 3; ++i) { - Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1000+i)); - ASSERT_NO_THROW(q6->enqueuePacket(pkt, sock1)); - checkIntStat(q6, "size", i); - } - - // Let's flush the buffer and then verify it is empty. - q6->clear(); - EXPECT_TRUE(q6->empty()); - checkIntStat(q6, "size", 0); -} - -// Verifies the higher level functions of queueing and -// dequeueing with drop and skip logic disabled. -TEST(TestQueue6, enqueueDequeueTest) { - PacketQueue6Ptr q6(new TestQueue6(100)); - EXPECT_TRUE(q6->empty()); - - SocketInfo sock1(isc::asiolink::IOAddress("127.0.0.1"), 777, 10); - - // Enqueue the first packet. - Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1002)); - ASSERT_NO_THROW(q6->enqueuePacket(pkt, sock1)); - checkIntStat(q6, "size", 1); - - // Enqueue a packet onto the front. - pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1003)); - ASSERT_NO_THROW(q6->enqueuePacket(pkt, sock1, QueueEnd::FRONT)); - checkIntStat(q6, "size", 2); - - // Enqueue a packet onto the back. - pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1001)); - ASSERT_NO_THROW(q6->enqueuePacket(pkt, sock1, QueueEnd::BACK)); - checkIntStat(q6, "size", 3); - - // By default we dequeue from the front. We should get transid 1003. - ASSERT_NO_THROW(pkt = q6->dequeuePacket()); - ASSERT_TRUE(pkt); - EXPECT_EQ(1003, pkt->getTransid()); - - // Dequeue from the back, we should get transid 1001. - ASSERT_NO_THROW(pkt = q6->dequeuePacket(QueueEnd::BACK)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1001, pkt->getTransid()); - - // Dequeue from the front, we should get transid 1002. - ASSERT_NO_THROW(pkt = q6->dequeuePacket(QueueEnd::FRONT)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1002, pkt->getTransid()); - - // Queue should be empty. - ASSERT_NO_THROW(pkt = q6->dequeuePacket()); + ASSERT_NO_THROW(pkt = q.popPacket(QueueEnd::BACK)); ASSERT_FALSE(pkt); } @@ -220,58 +209,58 @@ TEST(TestQueue6, enqueueDequeueTest) { // This accesses it's queue instance as a TestQueue6, rather than // a PacketQueue6Ptr, to provide access to TestQueue6 specifics. TEST(TestQueue6, shouldDropPacketTest) { - TestQueue6 q6(100); - EXPECT_TRUE(q6.empty()); - ASSERT_FALSE(q6.drop_enabled_); - ASSERT_EQ(0, q6.eat_count_); + TestQueue6 q(100); + EXPECT_TRUE(q.empty()); + ASSERT_FALSE(q.drop_enabled_); + ASSERT_EQ(0, q.eat_count_); - SocketInfo sockEven(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); - SocketInfo sockOdd(isc::asiolink::IOAddress("127.0.0.1"), 777, 11); + SocketInfo sock_even(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); + SocketInfo sock_odd(isc::asiolink::IOAddress("127.0.0.1"), 777, 11); // Drop is not enabled. // We should be able to enqueue a packet with even numbered values. Pkt6Ptr pkt(new Pkt6(DHCPV6_SOLICIT, 1002)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sockEven)); - EXPECT_EQ(1, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_even)); + ASSERT_EQ(1, q.getSize()); // We should be able to enqueue a packet with odd numbered values. pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1003)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(2, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + ASSERT_EQ(2, q.getSize()); // Enable drop logic. - q6.drop_enabled_ = true; + q.drop_enabled_ = true; // We should not be able to add one with an even-numbered transid. pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1004)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(2, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + ASSERT_EQ(2, q.getSize()); // We should not be able to add one with from even-numbered port. pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1005)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sockEven)); - EXPECT_EQ(2, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_even)); + EXPECT_EQ(2, q.getSize()); // We should be able to add one with an odd-numbered values. pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1007)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sockOdd)); - EXPECT_EQ(3, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock_odd)); + EXPECT_EQ(3, q.getSize()); // Dequeue them and make sure they are as expected: 1002,1003, and 1007. - ASSERT_NO_THROW(pkt = q6.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1002, pkt->getTransid()); - ASSERT_NO_THROW(pkt = q6.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); - ASSERT_NO_THROW(pkt = q6.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1007, pkt->getTransid()); // Queue should be empty. - ASSERT_NO_THROW(pkt = q6.dequeuePacket()); + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_FALSE(pkt); } @@ -279,10 +268,10 @@ TEST(TestQueue6, shouldDropPacketTest) { // This accesses it's queue instance as a TestQueue6, rather than // a PacketQueue6Ptr, to provide access to TestQueue6 specifics. TEST(TestQueue6, eatPacketsTest) { - TestQueue6 q6(100); - EXPECT_TRUE(q6.empty()); - ASSERT_FALSE(q6.drop_enabled_); - ASSERT_EQ(0, q6.eat_count_); + TestQueue6 q(100); + EXPECT_TRUE(q.empty()); + ASSERT_FALSE(q.drop_enabled_); + ASSERT_EQ(0, q.eat_count_); SocketInfo sock(isc::asiolink::IOAddress("127.0.0.1"), 888, 10); @@ -290,25 +279,17 @@ TEST(TestQueue6, eatPacketsTest) { // Let's add five packets. for (int i = 1; i < 6; ++i) { pkt.reset(new Pkt6(DHCPV6_SOLICIT, 1000 + i)); - ASSERT_NO_THROW(q6.enqueuePacket(pkt, sock)); - EXPECT_EQ(i, q6.getSize()); + ASSERT_NO_THROW(q.enqueuePacket(pkt, sock)); + ASSERT_EQ(i, q.getSize()); } - // Setting eat count to two and dequeuing (from the front, by default), - // should discard 1001 and 1002, resulting in a dequeue of 1003. - q6.eat_count_ = 2; - ASSERT_NO_THROW(pkt = q6.dequeuePacket()); + // Setting eat count to two and dequeuing should discard 1001 + // and 1002, resulting in a dequeue of 1003. + q.eat_count_ = 2; + ASSERT_NO_THROW(pkt = q.dequeuePacket()); ASSERT_TRUE(pkt); EXPECT_EQ(1003, pkt->getTransid()); - EXPECT_EQ(2, q6.getSize()); - - // Setting eat count to one and dequeing from the back, should discard - // 1005 and dequeue 104. - q6.eat_count_ = 1; - ASSERT_NO_THROW(pkt = q6.dequeuePacket(QueueEnd::BACK)); - ASSERT_TRUE(pkt); - EXPECT_EQ(1004, pkt->getTransid()); - EXPECT_EQ(0, q6.getSize()); + EXPECT_EQ(2, q.getSize()); } } // end of anonymous namespace diff --git a/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc b/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc index d746d68465..ed4b16572e 100644 --- a/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc @@ -6,6 +6,7 @@ #include +#include #include #include diff --git a/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc b/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc index 38e7643766..6003e03363 100644 --- a/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc @@ -6,6 +6,7 @@ #include +#include #include #include -- GitLab From 78bd0adfcc81c96ff7d974137d98900e99333e58 Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 7 Dec 2018 14:22:19 -0500 Subject: [PATCH 2/7] [#278,!162] Added Congestion Handling to Developer's guide doc/devel/congestion-handling.dox New file describes how to develop your own packet queue --- doc/Makefile.am | 1 + doc/devel/congestion-handling.dox | 453 ++++++++++++++++++++++++++++++ doc/devel/mainpage.dox | 2 +- src/lib/dhcp/iface_mgr.h | 2 +- src/lib/dhcp/packet_queue.h | 2 +- src/lib/dhcp/packet_queue_mgr.h | 6 +- 6 files changed, 460 insertions(+), 6 deletions(-) create mode 100644 doc/devel/congestion-handling.dox diff --git a/doc/Makefile.am b/doc/Makefile.am index 41aa95a0e9..ad4192f779 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -8,6 +8,7 @@ EXTRA_DIST += devel/mainpage.dox EXTRA_DIST += devel/terminology.dox EXTRA_DIST += devel/unit-tests.dox EXTRA_DIST += devel/doc.dox +EXTRA_DIST += devel/congestion-handling.dox nobase_dist_doc_DATA = examples/agent/comments.json nobase_dist_doc_DATA += examples/agent/simple.json diff --git a/doc/devel/congestion-handling.dox b/doc/devel/congestion-handling.dox new file mode 100644 index 0000000000..a149661885 --- /dev/null +++ b/doc/devel/congestion-handling.dox @@ -0,0 +1,453 @@ +// 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/. + +/** + +@page congestionHandling Congestion Handling in Kea DHCP4/DHCP6 + +@section background What is Congestion? +Congestion occurs when servers are subjected to client queries +faster than they can be fulfilled. Subsequently, the servers begin +accumulating a backlog of pending queries. The longer the high rate of +traffic continues the farther behind the servers fall. Depending on the +client implementations, those that fail to get leases either give up or simply +continue to retry forever. In the former case, the server may eventually +recover. The latter case is vicious cycle from which the server is unable +to escape. + +In a well-planned deployment, the number and capacity of servers is matched +to the maximum client loads expected. As long as capacity is matched to +load, congestion does not occur. If the load is routinely too heavy, then +the deployment needs to be re-evaluated. Congestion typically occurs when +there is a network event that causes overly large numbers of clients to +simultaneously need leases such as recovery after a network outage. + +@section introduction Congestion Handling Overview + +Kea 1.5 introduces a new feature referred to as Congestion Handling. The +goal of Congestion Handling is to help the servers mitigate the peak +in traffic by fulfilling as many of the most relevant requests as possible +until it subsides. + +Prior to Kea 1.5, kea-dhcp4 and kea-dhcp6 read inbound packets directly +from the interface sockets in the main application thread. This meant that +packets waiting to be processed were held in socket buffers themselves. Once +these buffers fill any new packets are discarded. Under swamped conditions +the servers can end up processing client packets that may no longer be +relevant, or worse are redundant. In other words, the packets waiting in +the FIFO socket buffers become increasingly stale. + +Congestion Handling offers the ability to configure the server to use a +separate thread to read packets from the interface socket buffers. As the +thread reads packets from the buffers they are added to an internal "packet +queue". The server's main application thread process packets from this queue +rather than the socket buffers. By structuring it this way, we've introduced +a configurable layer which can make decisions on which packets to process, +how to store them, and the order in which they are processed by the server. + +The default packet queue implementation for both kea-dhcp4 and kea-dhcp6 +is a simple ring buffer. Once it reaches capacity, new packets get added to +the back of queue by discarding packets from the front of queue. Rather than +always discarding the newest packets, we now always discard the oldest +packets. The capacity of the buffer, (i.e the maximum number of packets the +buffer can contain) is configurable. + +@section custom-implementations Custom Packet Queues + +It is possible to replace the default packet queue implementation with a +custom implementation by registering it with your Kea server via a hook +library. The steps for doing this are listed below: + +-# Develop a derivation of the interface isc::dhcp::PacketQueue +-# Registering and un-registering your implementation via Hook library +-# Configure your Kea server to use your derivation + +(If you are not familiar with writing Kea hook libraries, you may wish to +read @ref hooksdgDevelopersGuide before continuing). + +@subsection packet-queue-derivation Developing isc::dhcp::PacketQueue Derivations + @subsection packet-queue-derivation-basics The Basics + +Your custom packet queue must derive from the class template, +isc::dhcp::PacketQueue. The class is almost entirely abstract and +deliberately brief to provide developers wide latitude in the internals +of their solutions. + +The template argument, PacketTypePtr, is expected to be either +isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr, depending upon which +protocol the implementation will handle. Please note that the +while following text and examples largely focus on DHCPv4 out +of convenience as the concepts are identical for DHCPv6. For +completeness there are code snippets at the end of this +chapter for DHCPv6. + +The two primary functions of interest are: + +-# isc::dhcp::PacketQueue::enqueuePacket() - This function is invoked by +the receiver thread each time a packet has been read from a interface +socket buffer and should be added to the queue. It is passed a pointer to +the unpacked client packet (isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr), and +a reference to the isc::dhcp::SocketInfo describing the interface socket +from which the packet was read. Your derivation is free to use whatever +logic you deem appropriate to decide if a given packet should be added +to the queue or dropped. The socket information is passed along to be used +(or not) in your decision making. The simplest derivation would add every +packet, every time. + +-# isc::dhcp::PacketQueue::dequeuePacket() - This function is invoked by the +server's main thread whenever the receiver thread indicates that packets are +ready. Which packet is dequeued and returned is entirely up to your +derivation. + +The remaining functions that you'll need to implement are self-explanatory. + +How your actual "queue" is implemented is entirely up to you. Kea's default +implementation using a ring buffer based on Boost's boost::circular_buffer +(please refer to isc::dhcp::PacketQueueRing, isc::dhcp::PacketQueueRing4 and +isc::dhcpPacketQueueRing6). The most critical aspects to remember when +developing your implementation are: + +-# It MUST be thread safe since queuing and dequeing packets are done by +separate threads. (You might considering using isc::util::thread::Mutex and +isc::util::thread::Mutex::Locker). + +-# Its efficiency (or lack thereof) will have a direct impact on server +performance. You will have to consider the dynamics of your deployment +to determine where the trade-off between the volume of packets responded +to versus preferring to respond to some subset of those packets lies. + + @subsection packet-queue-derivation-factory Defining a Factory + +isc::dhcp::IfaceMgr using two derivations of isc::dhcp::PacketQueueMgr (one for +DHCPv4 and one for DHCPv6), to register queue implementations and instantiate +the appropriate queue type based the current configuration. In order to +register your queue implementation your hook library must provide a factory +function that will be used to create packet queues. This function will be +invoked by the server during the configuration process to instantiate the +appropriate queue type. For DHCPv4, the factory should be as follows: + +@code + PackQueue4Ptr factory(isc::data::ConstElementPtr parameters) +@endcode + +and for DHCPv6: + +@code + PackQueue6Ptr factory(isc::data::ConstElementPtr parameters) +@endcode + +The factory's only argument is an isc::data::ConstElementPtr. This is will be +an isc::data::MapElement instance containing the contents of the configuration +element "dhcp-queue-control" from the Kea server's configuration. It will +always have the following two values: + +-# "queue-enable" - used by isc::dhcp::IfaceMgr to know whether or not +congestion handling is enabled. Your implementation need not do anything +with this value. + +-# "queue-type" - name of the registered queue implementation to use. +It is used by isc::dhcp::IfaceMgr to invoke the appropriate queue factory. +Your implementation must pass this value through to the isc::dhcp::PacketQueue +constructor. + +Beyond that you may add whatever additional values you may require. In +other words, the content is arbitrary so long as it is valid JSON. It is +up to your factory implementation to examine the contents and use them +to construct a queue instance. + + @subsection packet-queue-derivation-example An Example + +Let's suppose you wish develop a queue for DHCPv4 and your implementation +requires two configurable parameters: capacity and threshold. Your class +declaration might look something like this: + +@code +class YourPacketQueue4 : public isc::dhcp::PacketQueue { +public: + + // Logical name you will register your factory under. + static const std::string QUEUE_TYPE; + + // Factory for instantiating queue instances. + static isc::dhcp::PacketQueue4Ptr factory(isc::data::ConstElementPtr params); + + // Constructor + YourPacketQueue4(const std::string& queue_type, size_t capacity, size_t threshold) + : isc::dhcp::PacketQueue(queue_type) { + + // your constructor steps here + } + + // Adds a packet to your queue using your secret formula based on threshold. + virtual void enqueuePacket(isc::dhcp::Pkt4Ptr packet, const dhcp::SocketInfo& source); + + // Fetches the next packet to process from your queue using your other secret formula. + virtual isc::dhcp::Pkt4Ptr dequeuePacket(); + + : // Imagine you prototyped the rest of the functions + +}; +@endcode + +Your factory implementation would then look something like this: + +@code + +const std::string QUEUE_TYPE = "Your-Q4"; + +isc::dhcp::PacketQueue4Ptr +YourPacketQueue4::factory(isc::data::ConstElementPtr parameters) { + + // You need queue-type to pass into the base class. + // It's guaranteed to be here. + std::string queue_type = isc::data::SimpleParser::getString(parameters, "queue-type"); + + // Now you need to fetch your required parameters. + size_t capacity; + try { + capacity = isc::data::SimpleParser::getInteger(parameters, "capacity"); + } catch (const std::exception& ex) { + isc_throw(isc::dhcp::InvalidQueueParameter, "YourPacketQueue4:factory:" + " 'capacity' parameter is missing/invalid: " << ex.what()); + } + + size_t threshold; + try { + threshold = isc::data::SimpleParser::getInteger(parameters, "threshold"); + } catch (const std::exception& ex) { + isc_throw(isc::dhcp::InvalidQueueParameter, "YourPacketQueue4:factory:" + " 'threshold' parameter is missing/invalid: " << ex.what()); + } + + // You should be all set to create your queue instance! + isc::dhcp::PacketQueue4Ptr queue(new YourPacketQueue4(queue_type, capacity, threshold)); + return (queue); +} + +@endcode + +Kea's configuration parser cannot know your parameter requirements and thus +can only flag JSON syntax errors. Thus it is important for your factory to +validate your parameters according to your requirements and throw meaningful +exceptions when they are not met. This allows users to know what to correct. + +@subsection packet-queue-registration Registering Your Implementation + +All hook libraries must provide a load() and unload() function. Your hook +library should register you queue factory during load() and un-register it +during unload(). Picking up with the our example, those functions might +look something like this: + +@code +// This function is called when the library is loaded. +// +// param - handle library handle (we aren't using it) +// return - 0 when initialization is successful, 1 otherwise +int load(LibraryHandle& /* handle */) { + try { + // Here you register your DHCPv4 queue factory + isc::dhcp::IfaceMgr::instance().getPacketQueueMgr4()-> + registerPacketQueueFactory(YourPacketQueue4::QUEUE_TYPE, + YourPacketQueue::factory); + } catch (const std::exception& ex) { + LOG_ERROR(your_logger, YOUR_LOAD_FAILED) + .arg(ex.what()); + return (1); + } + + LOG_INFO(your_logger, YOUR_LOAD_OK); + return (0); +} + +// This function is called when the library is unloaded. +// +// return - 0 if deregistration was successful, 1 otherwise +int unload() { + + // You need to remove your queue factory. This must be done to make sure + // your queue instance is destroyed before your library is unloaded. + isc::dhcp::IfaceMgr::instance().getPacketQueueMgr4()-> + unregisterPacketQueueFactory(YourPacketQueue4::QUEUE_TYPE); + + LOG_INFO(your_logger, YOUR_UNLOAD_OK); + return (0); +} +@endcode + +@subsection packet-queue-factory Configuring Kea to use Your PacketQueue<> + +You're almost there. You developed your implementation, you've unit tested it +(You did unit test it right?). Now you just have to tell Kea to load it and +use it. Continuing with the example, your kea-dhcp4 configuration would need +to look something like this: + +@code +{ +"Dhcp4": +{ + ... + + "hooks-libraries": [ + { + # Loading your hook library! + "library": "/somepath/lib//libyour_packet_queue.so" + } + + # any other hook libs + ], + + ... + + "dhcp-queue-control": { + "enable-queue": true, + "queue-type": "Your-Q4", + "capacity" : 100, + "threshold" : 75 + }, + + ... +} +@endcode + +@subsection packet-queue-example-dhcpv6 DHCPv6 Example Snippets + +For completeness, this section includes the example from above +implemented for DHCPv6. + +DHCPv6 Class declaration: + +@code +class YourPacketQueue6 : public isc::dhcp::PacketQueue { +public: + + // Logical name you will register your factory under. + static const std::string QUEUE_TYPE; + + // Factory for instantiating queue instances. + static isc::dhcp::PacketQueue6Ptr factory(isc::data::ConstElementPtr params); + + // Constructor + YourPacketQueue6(const std::string& queue_type, size_t capacity, size_t threshold) + : isc::dhcp::PacketQueue(queue_type) { + + // your constructor steps here + } + + // Adds a packet to your queue using your secret formula based on threshold. + virtual void enqueuePacket(isc::dhcp::Pkt6Ptr packet, const dhcp::SocketInfo& source); + + // Fetches the next packet to process from your queue using your other secret formula. + virtual isc::dhcp::Pkt6Ptr dequeuePacket(); + + : // Imagine you prototyped the rest of the functions + +}; +@endcode + +DHCPv6 Factory implemenation: + +@code +const std::string QUEUE_TYPE = "Your-Q6"; + +isc::dhcp::PacketQueue6Ptr +YourPacketQueue6::factory(isc::data::ConstElementPtr parameters) { + + // You need queue-type to pass into the base class. + // It's guaranteed to be here. + std::string queue_type = isc::data::SimpleParser::getString(parameters, "queue-type"); + + // Now you need to fetch your required parameters. + size_t capacity; + try { + capacity = isc::data::SimpleParser::getInteger(parameters, "capacity"); + } catch (const std::exception& ex) { + isc_throw(isc::dhcp::InvalidQueueParameter, "YourPacketQueue6:factory:" + " 'capacity' parameter is missing/invalid: " << ex.what()); + } + + size_t threshold; + try { + threshold = isc::data::SimpleParser::getInteger(parameters, "threshold"); + } catch (const std::exception& ex) { + isc_throw(isc::dhcp::InvalidQueueParameter, "YourPacketQueue6:factory:" + " 'threshold' parameter is missing/invalid: " << ex.what()); + } + + // You should be all set to create your queue instance! + isc::dhcp::PacketQueue6Ptr queue(new YourPacketQueue6(queue_type, capacity, threshold)); + return (queue); +} +@endcode + +DHCPv6 Hook load/unload functions + +@code +// This function is called when the library is loaded. +// +// param - handle library handle (we aren't using it) +// return - 0 when initialization is successful, 1 otherwise +int load(LibraryHandle& /* handle */) { + try { + // Here you register your DHCPv6 queue factory + isc::dhcp::IfaceMgr::instance().getPacketQueueMgr6()-> + registerPacketQueueFactory(YourPacketQueue6::QUEUE_TYPE, + YourPacketQueue::factory); + } catch (const std::exception& ex) { + LOG_ERROR(your_logger, YOUR_LOAD_FAILED) + .arg(ex.what()); + return (1); + } + + LOG_INFO(your_logger, YOUR_LOAD_OK); + return (0); +} + +// This function is called when the library is unloaded. +// +// return - 0 if deregistration was successful, 1 otherwise +int unload() { + + // You need to remove your queue factory. This must be done to make sure + // your queue instance is destroyed before your library is unloaded. + isc::dhcp::IfaceMgr::instance().getPacketQueueMgr6()-> + unregisterPacketQueueFactory(YourPacketQueue6::QUEUE_TYPE); + + LOG_INFO(your_logger, YOUR_UNLOAD_OK); + return (0); +} +@endcode + +Server configuration for kea-dhcp6: + +@code +{ +"Dhcp6": +{ + ... + + "hooks-libraries": [ + { + # Loading your hook library! + "library": "/somepath/lib//libyour_packet_queue.so" + } + + # any other hook libs + ], + + ... + + "dhcp-queue-control": { + "enable-queue": true, + "queue-type": "Your-Q6", + "capacity" : 100, + "threshold" : 75 + }, + + ... +} +@endcode + +*/ diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox index fd115bce18..ee16bfb66a 100644 --- a/doc/devel/mainpage.dox +++ b/doc/devel/mainpage.dox @@ -78,7 +78,7 @@ * - @subpage dhcpv6ConfigBackend * - @subpage dhcpv6SignalBasedReconfiguration * - @subpage dhcpv6Other - * - @subpage dhcpv4o6Dhcp6 + * - @subpage congestionHandling * - @subpage d2 * - @subpage d2ProcessDerivation * - @subpage d2ConfigMgt diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 43fc908815..46f69fab9e 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -1083,7 +1083,7 @@ public: /// /// @param family indicates which receiver to start, /// (AF_INET or AF_INET6) - /// @parm queue_control configuration containing "dhcp-queue-control" + /// @param queue_control configuration containing "dhcp-queue-control" /// content /// @return true if packet queueuing has been enabled, false otherwise /// @throw InvalidOperation if the receiver thread is currently running. diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index 71f0835ff7..e02be0dd74 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -107,7 +107,7 @@ public: /// This method calls @c getInfo() and then converts that into JSON text. /// /// @return string of JSON text - virtual std::string getInfoStr() const { + std::string getInfoStr() const { data::ElementPtr info = getInfo(); std::ostringstream os; info->toJSON(os); diff --git a/src/lib/dhcp/packet_queue_mgr.h b/src/lib/dhcp/packet_queue_mgr.h index 892a1b472b..3e77643362 100644 --- a/src/lib/dhcp/packet_queue_mgr.h +++ b/src/lib/dhcp/packet_queue_mgr.h @@ -38,10 +38,10 @@ public: template class PacketQueueMgr { public: - /// @brief Type of the backend factory function. + /// @brief Defines the type of the packet queue factory function. /// - /// Factory function returns a pointer to the instance of the configuration - /// backend created. + /// Factory function returns a pointer to the instance of the packet + /// queue created. typedef std::function Factory; /// @brief Constructor. -- GitLab From 34239554b8ac2eaedc484720e70d0a6fb681935d Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Fri, 7 Dec 2018 15:33:03 -0500 Subject: [PATCH 3/7] [#278,!162] Minor doxygen clean up --- doc/devel/congestion-handling.dox | 2 +- src/lib/dhcp/packet_queue_ring.h | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/doc/devel/congestion-handling.dox b/doc/devel/congestion-handling.dox index a149661885..3c1e8d06e1 100644 --- a/doc/devel/congestion-handling.dox +++ b/doc/devel/congestion-handling.dox @@ -144,7 +144,7 @@ an isc::data::MapElement instance containing the contents of the configuration element "dhcp-queue-control" from the Kea server's configuration. It will always have the following two values: --# "queue-enable" - used by isc::dhcp::IfaceMgr to know whether or not +-# "enable-queue" - used by isc::dhcp::IfaceMgr to know whether or not congestion handling is enabled. Your implementation need not do anything with this value. diff --git a/src/lib/dhcp/packet_queue_ring.h b/src/lib/dhcp/packet_queue_ring.h index cea139465d..4b66c5c109 100644 --- a/src/lib/dhcp/packet_queue_ring.h +++ b/src/lib/dhcp/packet_queue_ring.h @@ -73,9 +73,6 @@ public: /// that the packet has NOT been unpacked at this point. The default /// implementation simply returns false (i.e. keep the packet). /// - /// @param packet the packet under consideration - /// @param source the socket the packet came from - /// /// @return true if the packet should be dropped, false if it should be /// kept. virtual bool shouldDropPacket(PacketTypePtr /* packet */, @@ -92,12 +89,6 @@ public: /// based on their own requirements. The default implemenation is to /// to simply return without skipping any packets. /// - /// @param from end of the queue from which packets should discarded - /// This is passed in from @c dequeuePackets. - /// - /// @param from specifies the end of the queue from which packets - /// should be discarded. - /// /// @return The number of packets discarded. virtual int eatPackets(const QueueEnd& /* from */) { return (0); -- GitLab From 1f94eaa3d8dfc25cd297a0a8114ce9d6883bd7ab Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 10 Dec 2018 17:17:31 +0100 Subject: [PATCH 4/7] [#278,!162] Minor cleanup in Congestion Handling per review. --- doc/devel/congestion-handling.dox | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/devel/congestion-handling.dox b/doc/devel/congestion-handling.dox index 3c1e8d06e1..5187bdf7e6 100644 --- a/doc/devel/congestion-handling.dox +++ b/doc/devel/congestion-handling.dox @@ -6,7 +6,7 @@ /** -@page congestionHandling Congestion Handling in Kea DHCP4/DHCP6 +@page congestionHandling Congestion Handling in Kea DHCP Servers @section background What is Congestion? Congestion occurs when servers are subjected to client queries @@ -27,12 +27,12 @@ simultaneously need leases such as recovery after a network outage. @section introduction Congestion Handling Overview -Kea 1.5 introduces a new feature referred to as Congestion Handling. The +Kea 1.5.0 introduces a new feature referred to as Congestion Handling. The goal of Congestion Handling is to help the servers mitigate the peak in traffic by fulfilling as many of the most relevant requests as possible until it subsides. -Prior to Kea 1.5, kea-dhcp4 and kea-dhcp6 read inbound packets directly +Prior to Kea 1.5.0, Kea DHCP servers read inbound packets directly from the interface sockets in the main application thread. This meant that packets waiting to be processed were held in socket buffers themselves. Once these buffers fill any new packets are discarded. Under swamped conditions @@ -43,12 +43,12 @@ the FIFO socket buffers become increasingly stale. Congestion Handling offers the ability to configure the server to use a separate thread to read packets from the interface socket buffers. As the thread reads packets from the buffers they are added to an internal "packet -queue". The server's main application thread process packets from this queue +queue". The server's main application thread processes packets from this queue rather than the socket buffers. By structuring it this way, we've introduced a configurable layer which can make decisions on which packets to process, how to store them, and the order in which they are processed by the server. -The default packet queue implementation for both kea-dhcp4 and kea-dhcp6 +The default packet queue implementation for both Kea DHCPv4 and DHCPv6 servers is a simple ring buffer. Once it reaches capacity, new packets get added to the back of queue by discarding packets from the front of queue. Rather than always discarding the newest packets, we now always discard the oldest @@ -87,7 +87,7 @@ chapter for DHCPv6. The two primary functions of interest are: -# isc::dhcp::PacketQueue::enqueuePacket() - This function is invoked by -the receiver thread each time a packet has been read from a interface +the receiver thread each time a packet has been read from an interface socket buffer and should be added to the queue. It is passed a pointer to the unpacked client packet (isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr), and a reference to the isc::dhcp::SocketInfo describing the interface socket @@ -116,8 +116,8 @@ isc::util::thread::Mutex::Locker). -# Its efficiency (or lack thereof) will have a direct impact on server performance. You will have to consider the dynamics of your deployment -to determine where the trade-off between the volume of packets responded -to versus preferring to respond to some subset of those packets lies. +to determine where the trade-off lies between the volume of packets responded +to and preferring to respond to some subset of those packets. @subsection packet-queue-derivation-factory Defining a Factory @@ -160,7 +160,7 @@ to construct a queue instance. @subsection packet-queue-derivation-example An Example -Let's suppose you wish develop a queue for DHCPv4 and your implementation +Let's suppose you wish to develop a queue for DHCPv4 and your implementation requires two configurable parameters: capacity and threshold. Your class declaration might look something like this: @@ -293,7 +293,7 @@ to look something like this: "hooks-libraries": [ { # Loading your hook library! - "library": "/somepath/lib//libyour_packet_queue.so" + "library": "/somepath/lib/libyour_packet_queue.so" } # any other hook libs @@ -431,7 +431,7 @@ Server configuration for kea-dhcp6: "hooks-libraries": [ { # Loading your hook library! - "library": "/somepath/lib//libyour_packet_queue.so" + "library": "/somepath/lib/libyour_packet_queue.so" } # any other hook libs -- GitLab From 414b2d8aa5036deaec4dd5da16313f271bcb736e Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 10 Dec 2018 17:28:50 +0100 Subject: [PATCH 5/7] [#278,!162] Further minor changes in Congestion Handling text. --- doc/devel/congestion-handling.dox | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/devel/congestion-handling.dox b/doc/devel/congestion-handling.dox index 5187bdf7e6..058a3a7f41 100644 --- a/doc/devel/congestion-handling.dox +++ b/doc/devel/congestion-handling.dox @@ -76,7 +76,7 @@ isc::dhcp::PacketQueue. The class is almost entirely abstract and deliberately brief to provide developers wide latitude in the internals of their solutions. -The template argument, PacketTypePtr, is expected to be either +The template argument, @c PacketTypePtr, is expected to be either isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr, depending upon which protocol the implementation will handle. Please note that the while following text and examples largely focus on DHCPv4 out @@ -107,7 +107,7 @@ The remaining functions that you'll need to implement are self-explanatory. How your actual "queue" is implemented is entirely up to you. Kea's default implementation using a ring buffer based on Boost's boost::circular_buffer (please refer to isc::dhcp::PacketQueueRing, isc::dhcp::PacketQueueRing4 and -isc::dhcpPacketQueueRing6). The most critical aspects to remember when +isc::dhcp::PacketQueueRing6). The most critical aspects to remember when developing your implementation are: -# It MUST be thread safe since queuing and dequeing packets are done by @@ -277,7 +277,7 @@ int unload() { } @endcode -@subsection packet-queue-factory Configuring Kea to use Your PacketQueue<> +@subsection packet-queue-factory Configuring Kea to use YourPacketQueue4 You're almost there. You developed your implementation, you've unit tested it (You did unit test it right?). Now you just have to tell Kea to load it and -- GitLab From 21dad40c94ebb91ba02e09f6758a16fbce5c8dbb Mon Sep 17 00:00:00 2001 From: Thomas Markwalder Date: Mon, 10 Dec 2018 12:05:30 -0500 Subject: [PATCH 6/7] [#278,!162] Addressed review comments --- doc/devel/mainpage.dox | 1 + src/lib/dhcp/packet_queue.h | 5 ++++- src/lib/dhcp/packet_queue_ring.h | 13 ++++++++----- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/devel/mainpage.dox b/doc/devel/mainpage.dox index ee16bfb66a..d2bb72bb3f 100644 --- a/doc/devel/mainpage.dox +++ b/doc/devel/mainpage.dox @@ -78,6 +78,7 @@ * - @subpage dhcpv6ConfigBackend * - @subpage dhcpv6SignalBasedReconfiguration * - @subpage dhcpv6Other + * - @subpage dhcpv4o6Dhcp6 * - @subpage congestionHandling * - @subpage d2 * - @subpage d2ProcessDerivation diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index e02be0dd74..64d1302294 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -41,8 +41,11 @@ enum class QueueEnd { /// This class serves as the abstract interface for packet queue /// implementations which may be used by @c IfaceMgr to store /// inbound packets until they are a dequeued for processing. -/// @note Derivations of this class MUST BE thread-safe. /// +/// @tparam PacktTypePtr Type of packet the queue contains. +/// This expected to be either isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr +/// +/// @note Derivations of this class MUST BE thread-safe. template class PacketQueue { public: diff --git a/src/lib/dhcp/packet_queue_ring.h b/src/lib/dhcp/packet_queue_ring.h index 4b66c5c109..4108896c62 100644 --- a/src/lib/dhcp/packet_queue_ring.h +++ b/src/lib/dhcp/packet_queue_ring.h @@ -18,7 +18,10 @@ namespace isc { namespace dhcp { -/// @brief Provides an abstract ring-buffer implementation of the PacketQueue interface. +/// @brief Provides a ring-buffer implementation of the PacketQueue interface. +/// +/// @tparam PacktTypePtr Type of packet the queue contains. +/// This expected to be either isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr template class PacketQueueRing : public PacketQueue { public: @@ -66,11 +69,11 @@ public: /// @brief Determines if a packet should be discarded. /// /// This function is called in @c enqueuePackets for each packet - /// in its packet list. It provides an opportunity to examine the + /// in its packet list. It provides an opportunity to examine the /// packet and its source and decide whether it should be dropped - /// or added to the queue. Derivations are expected to provide - /// implementations based on their own requirements. Bear in mind - /// that the packet has NOT been unpacked at this point. The default + /// or added to the queue. Derivations are expected to provide + /// implementations based on their own requirements. Bear in mind + /// that the packet has NOT been unpacked at this point. The default /// implementation simply returns false (i.e. keep the packet). /// /// @return true if the packet should be dropped, false if it should be -- GitLab From 180e0f89b4a897f0acb7162a86bc1d8b4c46db7c Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Mon, 10 Dec 2018 20:53:53 +0100 Subject: [PATCH 7/7] [#278,!162] Corrected typos in the template argument name. --- src/lib/dhcp/packet_queue.h | 2 +- src/lib/dhcp/packet_queue_ring.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index 64d1302294..07bb7ffb4c 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -42,7 +42,7 @@ enum class QueueEnd { /// implementations which may be used by @c IfaceMgr to store /// inbound packets until they are a dequeued for processing. /// -/// @tparam PacktTypePtr Type of packet the queue contains. +/// @tparam PacketTypePtr Type of packet the queue contains. /// This expected to be either isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr /// /// @note Derivations of this class MUST BE thread-safe. diff --git a/src/lib/dhcp/packet_queue_ring.h b/src/lib/dhcp/packet_queue_ring.h index 4108896c62..3477fff5c0 100644 --- a/src/lib/dhcp/packet_queue_ring.h +++ b/src/lib/dhcp/packet_queue_ring.h @@ -20,7 +20,7 @@ namespace dhcp { /// @brief Provides a ring-buffer implementation of the PacketQueue interface. /// -/// @tparam PacktTypePtr Type of packet the queue contains. +/// @tparam PacketTypePtr Type of packet the queue contains. /// This expected to be either isc::dhcp::Pkt4Ptr or isc::dhcp::Pkt6Ptr template class PacketQueueRing : public PacketQueue { -- GitLab