]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#553] Implemented a bound address unordered set
authorFrancis Dupont <fdupont@isc.org>
Mon, 25 May 2020 17:32:28 +0000 (19:32 +0200)
committerFrancis Dupont <fdupont@isc.org>
Tue, 7 Jul 2020 10:47:26 +0000 (12:47 +0200)
src/lib/dhcp/iface_mgr.cc
src/lib/dhcp/iface_mgr.h
src/lib/dhcp/tests/iface_mgr_unittest.cc

index fd26a1dd76135b6996a177f4385e9af0b1468387..c21c685f8dc23fd624e2a6eca7e2b40069cde227 100644 (file)
@@ -66,6 +66,10 @@ Iface::Iface(const std::string& name, int ifindex)
      flag_multicast_(false), flag_broadcast_(false), flags_(0),
      inactive4_(false), inactive6_(false)
 {
+    // Sanity checks.
+    if (name.empty()) {
+        isc_throw(BadValue, "Interface name must not be empty");
+    }
     if ((ifindex_ < 0) || (ifindex_ > std::numeric_limits<int32_t>::max())) {
         isc_throw(OutOfRange, "Interface index must be in 0.." <<
                   std::numeric_limits<int32_t>::max());
@@ -184,8 +188,7 @@ bool Iface::delSocket(const uint16_t sockfd) {
 }
 
 IfaceMgr::IfaceMgr()
-    : fail_on_index_not_found_(false),
-      packet_filter_(new PktFilterInet()),
+    : packet_filter_(new PktFilterInet()),
       packet_filter6_(new PktFilterInet6()),
       test_mode_(false),
       allow_loopback_(false) {
@@ -285,6 +288,9 @@ Iface::countActive4() const {
 }
 
 void IfaceMgr::closeSockets() {
+    // Clears bound addresses.
+    clearBoundAddresses();
+
     // Stops the receiver thread if there is one.
     stopDHCPReceiver();
 
@@ -442,6 +448,10 @@ IfaceMgr::hasOpenSocket(const uint16_t family) const {
 
 bool
 IfaceMgr::hasOpenSocket(const IOAddress& addr) const {
+    // Fast track for IPv4 using bound addresses.
+    if (addr.isV4() && !bound_address_.empty()) {
+        return (bound_address_.count(addr.toUint32()) != 0);
+    }
     // Iterate over all interfaces and search for open sockets.
     for (IfacePtr iface : ifaces_) {
         for (SocketInfo sock : iface->getSockets()) {
@@ -630,7 +640,10 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast,
 
     // If we have open sockets, start the receiver.
     if (count > 0) {
-        // starts the receiver thread (if queueing is enabled).
+        // Collects bound addresses.
+        collectBoundAddresses();
+
+        // Starts the receiver thread (if queueing is enabled).
         startDHCPReceiver(AF_INET);
     }
 
@@ -880,17 +893,11 @@ IfaceMgr::getIface(const std::string& ifname) {
 
 IfacePtr
 IfaceMgr::getIface(const PktPtr& pkt) {
-    IfacePtr iface;
     if (pkt->indexSet()) {
-            iface = getIface(pkt->getIndex());
-            if (fail_on_index_not_found_) {
-                return (iface);
-            }
-    }
-    if (!iface) {
-        iface = getIface(pkt->getIface());
+        return (getIface(pkt->getIndex()));
+    } else {
+        return (getIface(pkt->getIface()));
     }
-    return (iface);
 }
 
 void
@@ -898,6 +905,26 @@ IfaceMgr::clearIfaces() {
     ifaces_.clear();
 }
 
+void
+IfaceMgr::clearBoundAddresses() {
+    bound_address_.clear();
+}
+
+void
+IfaceMgr::collectBoundAddresses() {
+    for (IfacePtr iface : ifaces_) {
+        for (SocketInfo sock : iface->getSockets()) {
+            const IOAddress& addr = sock.addr_;
+            if (!addr.isV4()) {
+                continue;
+            }
+            if (bound_address_.count(addr.toUint32()) == 0) {
+                bound_address_.insert(addr);
+            }
+        }
+    }
+}
+
 void
 IfaceMgr::clearUnicasts() {
     for (IfacePtr iface : ifaces_) {
index 8fd8d5e64082eeac572513e9439ad94d0314892b..8b248b8b15d0891004495b89af99db9ea80fd8aa 100644 (file)
@@ -144,6 +144,8 @@ public:
     ///
     /// @param name name of the interface
     /// @param ifindex interface index (unique integer identifier)
+    /// @param BadValue when name is empty.
+    /// @throw OutOfRange when ifindex is not in 0..2147483647 range.
     Iface(const std::string& name, int ifindex);
 
     /// @brief Destructor.
@@ -588,6 +590,25 @@ private:
     IfaceContainer ifaces_container_;
 };
 
+/// @brief Type definition for the unordered set of IPv4 bound addresses.
+///
+/// In order to make @c hasOpenSocket with an IPv4 address faster bound
+/// addresses should be collected after calling @c CfgIface::openSockets.
+typedef boost::multi_index_container<
+    /// Container comprises elements of asiolink::IOAddress type.
+    asiolink::IOAddress,
+    // Here we start enumerating the only index.
+    boost::multi_index::indexed_by<
+        // Start definition of index #0.
+        boost::multi_index::hashed_unique<
+            // Use the address in its network order integer form as the key.
+            boost::multi_index::const_mem_fun<
+                asiolink::IOAddress, uint32_t, &asiolink::IOAddress::toUint32
+            >
+        >
+    >
+> BoundAddresses;
+
 /// @brief Forward declaration to the @c IfaceMgr.
 class IfaceMgr;
 
@@ -634,10 +655,6 @@ public:
     /// we don't support packets larger than 1500.
     static const uint32_t RCVBUFSIZE = 1500;
 
-    // TODO performance improvement: we may change this into
-    //      2 maps (ifindex-indexed and name-indexed) and
-    //      also hide it (make it public make tests easier for now)
-
     /// IfaceMgr is a singleton class. This method returns reference
     /// to its sole instance.
     ///
@@ -719,6 +736,10 @@ public:
 
     /// @brief Returns interface with specified packet
     ///
+    /// @note When the interface index is set (@c Pkt::indexSet
+    ///       returns true) it is searched for, if it is not set
+    ///       the name instead is searched for.
+    ///
     /// @param pkt packet with interface index and name
     ///
     /// @return interface with packet interface index or name
@@ -752,6 +773,12 @@ public:
     /// @brief Clears unicast addresses on all interfaces.
     void clearUnicasts();
 
+    /// @brief Clears bound addresses.
+    void clearBoundAddresses();
+
+    /// @brief Collect bound addresses.
+    void collectBoundAddresses();
+
     /// @brief Return most suitable socket for transmitting specified IPv6 packet.
     ///
     /// This method takes Pkt6Ptr (see overloaded implementation that takes
@@ -1389,14 +1416,11 @@ protected:
     /// interfaces.txt file.
     void stubDetectIfaces();
 
-    // TODO: having 2 maps (ifindex->iface and ifname->iface would)
-    //      probably be better for performance reasons
-
     /// @brief List of available interfaces
     IfaceCollection ifaces_;
 
-    /// @brief Return an error when index search fails.
-    bool fail_on_index_not_found_;
+    /// @brief Unordered set of IPv4 bound addresses.
+    BoundAddresses bound_address_;
 
     // TODO: Also keep this interface on Iface once interface detection
     // is implemented. We may need it e.g. to close all sockets on
index 4f29c52fcac9c1fd5eedb975ab2628e36e4ad64b..effe3d8129c6b6f718ca8ddb7c5dfc907115f534 100644 (file)
@@ -927,6 +927,9 @@ TEST_F(IfaceMgrTest, ifaceClass) {
     Iface iface("eth5", 7);
     EXPECT_STREQ("eth5/7", iface.getFullName().c_str());
 
+    EXPECT_THROW_MSG(Iface("", 10), BadValue,
+                     "Interface name must not be empty");
+
     EXPECT_THROW_MSG(Iface("foo", -1), OutOfRange,
                      "Interface index must be in 0..2147483647");
 }
@@ -968,13 +971,10 @@ TEST_F(IfaceMgrTest, getIfaceByPkt) {
     iface = ifacemgr.getIface(pkt4);
     EXPECT_FALSE(iface);
 
-    // Not existing index depends on fail_on_index_not_found_.
-    // Currently fail_on_index_not_found_ is false.
+    // Not existing index fails.
     pkt6->setIndex(3);
     iface = ifacemgr.getIface(pkt6);
-    ASSERT_TRUE(iface);
-    EXPECT_EQ("eth0", iface->getName());
-    EXPECT_EQ(1, iface->getIndex());
+    ASSERT_FALSE(iface);
 }
 
 // Test that the IPv4 address can be retrieved for the interface.
@@ -2590,9 +2590,14 @@ TEST_F(IfaceMgrTest, socketInfo) {
         IfaceNotFound
     );
 
-    // This will work
+    // Index is now checked first
     pkt6->setIface(LOOPBACK_NAME);
-    EXPECT_EQ(9, ifacemgr->getSocket(pkt6));
+    EXPECT_THROW(
+        ifacemgr->getSocket(pkt6),
+        IfaceNotFound
+    );
+
+    // This will work
     pkt6->setIndex(LOOPBACK_INDEX);
     EXPECT_EQ(9, ifacemgr->getSocket(pkt6));
 
@@ -2625,9 +2630,14 @@ TEST_F(IfaceMgrTest, socketInfo) {
         IfaceNotFound
     );
 
-    // Socket info is set, packet has well defined interface. It should work.
+    // Index is now checked first.
     pkt4->setIface(LOOPBACK_NAME);
-    EXPECT_EQ(7, ifacemgr->getSocket(pkt4).sockfd_);
+    EXPECT_THROW(
+        ifacemgr->getSocket(pkt4),
+        IfaceNotFound
+    );
+
+    // Socket info is set, packet has well defined interface. It should work.
     pkt4->setIndex(LOOPBACK_INDEX);
     EXPECT_EQ(7, ifacemgr->getSocket(pkt4).sockfd_);