From: Francis Dupont Date: Sun, 24 May 2020 23:38:03 +0000 (+0200) Subject: [#553] Checkpoint: added IfaceCollection X-Git-Tag: Kea-1.7.10~100 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=42ba6176826166da3f095e00a8437e16ce10f98f;p=thirdparty%2Fkea.git [#553] Checkpoint: added IfaceCollection --- diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index 52edb700d8..47e2cad81c 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -368,8 +368,7 @@ public: // deal with sockets here, just check if configuration handling // is sane. - const IfaceMgr::IfaceCollection& ifaces = - IfaceMgr::instance().getIfaces(); + const IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); // There must be some interface detected if (ifaces.empty()) { diff --git a/src/bin/dhcp6/tests/dhcp6_test_utils.cc b/src/bin/dhcp6/tests/dhcp6_test_utils.cc index 3c297d97ac..8e7b84ad79 100644 --- a/src/bin/dhcp6/tests/dhcp6_test_utils.cc +++ b/src/bin/dhcp6/tests/dhcp6_test_utils.cc @@ -837,8 +837,7 @@ NakedDhcpv6SrvTest::NakedDhcpv6SrvTest() // it's ok if that fails. There should not be such a file anyway static_cast(remove(DUID_FILE)); - const isc::dhcp::IfaceMgr::IfaceCollection& ifaces = - isc::dhcp::IfaceMgr::instance().getIfaces(); + const IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); // There must be some interface detected if (ifaces.empty()) { @@ -850,7 +849,7 @@ NakedDhcpv6SrvTest::NakedDhcpv6SrvTest() valid_ifindex_ = (*ifaces.begin())->getIndex(); // Let's wipe all existing statistics. - isc::stats::StatsMgr::instance().removeAll(); + StatsMgr::instance().removeAll(); } NakedDhcpv6SrvTest::~NakedDhcpv6SrvTest() { diff --git a/src/bin/perfdhcp/perf_socket.cc b/src/bin/perfdhcp/perf_socket.cc index 11e5e3acf6..dc654b94c7 100644 --- a/src/bin/perfdhcp/perf_socket.cc +++ b/src/bin/perfdhcp/perf_socket.cc @@ -12,9 +12,6 @@ #include #include -#include - - using namespace isc::dhcp; using namespace isc::asiolink; @@ -135,8 +132,8 @@ PerfSocket::~PerfSocket() { void PerfSocket::initSocketData() { - BOOST_FOREACH(IfacePtr iface, IfaceMgr::instance().getIfaces()) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : IfaceMgr::instance().getIfaces()) { + for (SocketInfo s : iface->getSockets()) { if (s.sockfd_ == sockfd_) { ifindex_ = iface->getIndex(); addr_ = s.addr_; @@ -151,13 +148,13 @@ Pkt4Ptr PerfSocket::receive4(uint32_t timeout_sec, uint32_t timeout_usec) { Pkt4Ptr pkt = IfaceMgr::instance().receive4(timeout_sec, timeout_usec); if (pkt) { - try { + try { pkt->unpack(); - } catch (const std::exception &e) { - ExchangeStats::malformed_pkts_++; - std::cout << "Incorrect DHCP packet received" - << e.what() << std::endl; - } + } catch (const std::exception &e) { + ExchangeStats::malformed_pkts_++; + std::cout << "Incorrect DHCP packet received" + << e.what() << std::endl; + } } return (pkt); } diff --git a/src/bin/perfdhcp/tests/command_options_unittest.cc b/src/bin/perfdhcp/tests/command_options_unittest.cc index 7e7923318c..411ede6e7a 100644 --- a/src/bin/perfdhcp/tests/command_options_unittest.cc +++ b/src/bin/perfdhcp/tests/command_options_unittest.cc @@ -774,7 +774,7 @@ TEST_F(CommandOptionsTest, Interface) { // here it is called by CommandOptions object internally // so this function is covered by the test. dhcp::IfaceMgr& iface_mgr = dhcp::IfaceMgr::instance(); - const dhcp::IfaceMgr::IfaceCollection& ifaces = iface_mgr.getIfaces(); + const dhcp::IfaceCollection& ifaces = iface_mgr.getIfaces(); std::string iface_name; CommandOptions opt; // The local loopback interface should be available. diff --git a/src/lib/dhcp/duid_factory.cc b/src/lib/dhcp/duid_factory.cc index 84719cd889..1d25ce2930 100644 --- a/src/lib/dhcp/duid_factory.cc +++ b/src/lib/dhcp/duid_factory.cc @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -244,10 +243,8 @@ DUIDFactory::createLL(const uint16_t htype, void DUIDFactory::createLinkLayerId(std::vector& identifier, uint16_t& htype) const { - const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); - // Let's find suitable interface. - BOOST_FOREACH(IfacePtr iface, ifaces) { + for (IfacePtr iface : IfaceMgr::instance().getIfaces()) { // All the following checks could be merged into one multi-condition // statement, but let's keep them separated as perhaps one day // we will grow knobs to selectively turn them on or off. Also, diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index ab8c18e3a0..b243167b69 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -16,14 +16,15 @@ #include #include #include +#include -#include #include #include #include #include #include +#include #include #include @@ -65,6 +66,10 @@ Iface::Iface(const std::string& name, int ifindex) flag_multicast_(false), flag_broadcast_(false), flags_(0), inactive4_(false), inactive6_(false) { + if ((ifindex_ < 0) || (ifindex_ > std::numeric_limits::max())) { + isc_throw(OutOfRange, "Interface index must be in 0.." << + std::numeric_limits::max()); + } memset(mac_, 0, sizeof(mac_)); } @@ -179,10 +184,11 @@ bool Iface::delSocket(const uint16_t sockfd) { } IfaceMgr::IfaceMgr() - :packet_filter_(new PktFilterInet()), - packet_filter6_(new PktFilterInet6()), - test_mode_(false), - allow_loopback_(false) { + : fail_on_index_not_found_(false), + packet_filter_(new PktFilterInet()), + packet_filter6_(new PktFilterInet6()), + test_mode_(false), + allow_loopback_(false) { // Ensure that PQMs have been created to guarantee we have // default packet queues in place. @@ -206,7 +212,7 @@ IfaceMgr::IfaceMgr() } void Iface::addUnicast(const isc::asiolink::IOAddress& addr) { - BOOST_FOREACH(Address a, unicasts_) { + for (Address a : unicasts_) { if (a.get() == addr) { isc_throw(BadValue, "Address " << addr << " already defined on the " << name_ << " interface."); @@ -219,7 +225,7 @@ bool Iface::getAddress4(isc::asiolink::IOAddress& address) const { // Iterate over existing addresses assigned to the interface. // Try to find the one that is IPv4. - BOOST_FOREACH(Address addr, getAddresses()) { + for (Address addr : getAddresses()) { // If address is IPv4, we assign it to the function argument // and return true. if (addr.get().isV4()) { @@ -233,7 +239,7 @@ Iface::getAddress4(isc::asiolink::IOAddress& address) const { bool Iface::hasAddress(const isc::asiolink::IOAddress& address) const { - BOOST_FOREACH(Address addr, getAddresses()) { + for (Address addr : getAddresses()) { if (address == addr.get()) { return (true); } @@ -270,7 +276,7 @@ Iface::setActive(const bool active) { unsigned int Iface::countActive4() const { uint16_t count = 0; - BOOST_FOREACH(Address addr, addrs_) { + for (Address addr : addrs_) { if (!addr.unspecified() && addr.get().isV4()) { ++count; } @@ -282,7 +288,7 @@ void IfaceMgr::closeSockets() { // Stops the receiver thread if there is one. stopDHCPReceiver(); - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { iface->closeSockets(); } } @@ -319,7 +325,7 @@ IfaceMgr::addExternalSocket(int socketfd, SocketCallback callback) { << socketfd); } std::lock_guard lock(callbacks_mutex_); - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { // There's such a socket description there already. // Update the callback and we're done if (s.socket_ == socketfd) { @@ -421,8 +427,8 @@ IfaceMgr::setPacketFilter(const PktFilter6Ptr& packet_filter) { bool IfaceMgr::hasOpenSocket(const uint16_t family) const { // Iterate over all interfaces and search for open sockets. - BOOST_FOREACH(IfacePtr iface, ifaces_) { - BOOST_FOREACH(SocketInfo sock, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo sock : iface->getSockets()) { // Check if the socket matches specified family. if (sock.family_ == family) { // There is at least one socket open, so return. @@ -437,8 +443,8 @@ IfaceMgr::hasOpenSocket(const uint16_t family) const { bool IfaceMgr::hasOpenSocket(const IOAddress& addr) const { // Iterate over all interfaces and search for open sockets. - BOOST_FOREACH(IfacePtr iface, ifaces_) { - BOOST_FOREACH(SocketInfo sock, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo sock : iface->getSockets()) { // Check if the socket address matches the specified address or // if address is unspecified (in6addr_any). if (sock.addr_ == addr) { @@ -448,7 +454,7 @@ IfaceMgr::hasOpenSocket(const IOAddress& addr) const { // Handle the case that the address is unspecified (any). // In this case, we should check if the specified address // belongs to any of the interfaces. - BOOST_FOREACH(Iface::Address a, iface->getAddresses()) { + for (Iface::Address a : iface->getAddresses()) { if (addr == a.get()) { return (true); } @@ -504,7 +510,7 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast, int count = 0; int bcast_num = 0; - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { // If the interface is inactive, there is nothing to do. Simply // proceed to the next detected interface. if (iface->inactive4_) { @@ -549,7 +555,7 @@ IfaceMgr::openSockets4(const uint16_t port, const bool use_bcast, } } - BOOST_FOREACH(Iface::Address addr, iface->getAddresses()) { + for (Iface::Address addr : iface->getAddresses()) { // Skip non-IPv4 addresses and those that weren't selected.. if (addr.unspecified() || !addr.get().isV4()) { continue; @@ -632,7 +638,7 @@ IfaceMgr::openSockets6(const uint16_t port, IfaceMgrErrorMsgCallback error_handler) { int count = 0; - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { if (iface->inactive6_) { continue; @@ -665,7 +671,7 @@ IfaceMgr::openSockets6(const uint16_t port, } // Open unicast sockets if there are any unicast addresses defined - BOOST_FOREACH(Iface::Address addr, iface->getUnicasts()) { + for (Iface::Address addr : iface->getUnicasts()) { try { openSocket(iface->getName(), addr, port); @@ -681,7 +687,7 @@ IfaceMgr::openSockets6(const uint16_t port, } - BOOST_FOREACH(Iface::Address addr, iface->getAddresses()) { + for (Iface::Address addr : iface->getAddresses()) { // Skip all but V6 addresses. if (!addr.get().isV6()) { @@ -763,7 +769,7 @@ IfaceMgr::addInterface(const IfacePtr& iface) { void IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) { - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { const Iface::AddressCollection& addrs = iface->getAddresses(); out << "Detected interface " << iface->getFullName() @@ -778,7 +784,7 @@ IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) { << ")" << endl; out << " " << addrs.size() << " addr(s):"; - BOOST_FOREACH(Iface::Address addr, addrs) { + for (Iface::Address addr : addrs) { out << " " << addr.get().toText(); } out << endl; @@ -786,29 +792,97 @@ IfaceMgr::printIfaces(std::ostream& out /*= std::cout*/) { } IfacePtr -IfaceMgr::getIface(int ifindex) { - BOOST_FOREACH(IfacePtr iface, ifaces_) { - if (iface->getIndex() == ifindex) - return (iface); +IfaceCollection::getIface(uint32_t ifindex) { + return (getIfaceInternal(ifindex, MultiThreadingMgr::instance().getMode())); +} + + +IfacePtr +IfaceCollection::getIface(const std::string& ifname) { + return (getIfaceInternal(ifname, MultiThreadingMgr::instance().getMode())); +} + +IfacePtr +IfaceCollection::getIfaceInternal(uint32_t ifindex, bool need_lock) { + if (need_lock) { + lock_guard lock(mutex_); + if (cache_ && (cache_->getIndex() == ifindex)) { + return (cache_); + } + } else { + if (cache_ && (cache_->getIndex() == ifindex)) { + return (cache_); + } + } + const auto& idx = ifaces_container_.get<1>(); + auto it = idx.find(ifindex); + if (it == idx.end()) { + return (IfacePtr()); // not found + } + if (need_lock) { + lock_guard lock(mutex_); + cache_ = *it; + return (cache_); + } else { + lock_guard lock(mutex_); + cache_ = *it; + return (cache_); } +} - return (IfacePtr()); // not found +IfacePtr +IfaceCollection::getIfaceInternal(const std::string& ifname, bool need_lock) { + if (need_lock) { + lock_guard lock(mutex_); + if (cache_ && (cache_->getName() == ifname)) { + return (cache_); + } + } else { + if (cache_ && (cache_->getName() == ifname)) { + return (cache_); + } + } + const auto& idx = ifaces_container_.get<2>(); + auto it = idx.find(ifname); + if (it == idx.end()) { + return (IfacePtr()); // not found + } + if (need_lock) { + lock_guard lock(mutex_); + cache_ = *it; + return (cache_); + } else { + lock_guard lock(mutex_); + cache_ = *it; + return (cache_); + } } IfacePtr -IfaceMgr::getIface(const std::string& ifname) { - std::cerr << "getIface(name) unefficient\n"; - BOOST_FOREACH(IfacePtr iface, ifaces_) { - if (iface->getName() == ifname) - return (iface); +IfaceMgr::getIface(int ifindex) { + if ((ifindex < 0) || (ifindex > std::numeric_limits::max())) { + return (IfacePtr()); // out of range } + return (ifaces_.getIface(ifindex)); +} - return (IfacePtr()); // not found +IfacePtr +IfaceMgr::getIface(const std::string& ifname) { + if (ifname.empty()) { + return (IfacePtr()); // empty + } + return (ifaces_.getIface(ifname)); } IfacePtr IfaceMgr::getIface(const PktPtr& pkt) { - IfacePtr iface = getIface(pkt->getIndex()); + IfacePtr iface; + if (pkt->indexSet()) { + iface = getIface(pkt->getIndex()); + if (fail_on_index_not_found_) { + return (iface); + } + } if (!iface) { iface = getIface(pkt->getIface()); } @@ -822,7 +896,7 @@ IfaceMgr::clearIfaces() { void IfaceMgr::clearUnicasts() { - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { iface->clearUnicasts(); } } @@ -850,7 +924,7 @@ int IfaceMgr::openSocketFromIface(const std::string& ifname, const uint16_t port, const uint8_t family) { // Search for specified interface among detected interfaces. - BOOST_FOREACH(IfacePtr iface, ifaces_) { + for (IfacePtr iface : ifaces_) { if ((iface->getFullName() != ifname) && (iface->getName() != ifname)) { continue; @@ -891,8 +965,8 @@ int IfaceMgr::openSocketFromAddress(const IOAddress& addr, const uint16_t port) { // Search through detected interfaces and addresses to match // local address we got. - BOOST_FOREACH(IfacePtr iface, ifaces_) { - BOOST_FOREACH(Iface::Address a, iface->getAddresses()) { + for (IfacePtr iface : ifaces_) { + for (Iface::Address a : iface->getAddresses()) { // Local address must match one of the addresses // on detected interfaces. If it does, we have @@ -1046,7 +1120,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / { std::lock_guard lock(callbacks_mutex_); if (!callbacks_.empty()) { - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { // Add this socket to listening set addFDtoSet(s.socket_, maxfd, &sockets); } @@ -1116,7 +1190,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / bool found = false; { std::lock_guard lock(callbacks_mutex_); - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { if (!FD_ISSET(s.socket_, &sockets)) { continue; } @@ -1159,7 +1233,6 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* " one million microseconds"); } boost::scoped_ptr candidate; - IfacePtr iface; fd_set sockets; int maxfd = 0; @@ -1168,8 +1241,8 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies /// provided set to indicated which sockets have something to read. - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { // Only deal with IPv4 addresses. if (s.addr_.isV4()) { // Add this socket to listening set @@ -1182,7 +1255,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* { std::lock_guard lock(callbacks_mutex_); if (!callbacks_.empty()) { - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { // Add this socket to listening set addFDtoSet(s.socket_, maxfd, &sockets); } @@ -1227,7 +1300,7 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* bool found = false; { std::lock_guard lock(callbacks_mutex_); - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { if (!FD_ISSET(s.socket_, &sockets)) { continue; } @@ -1254,25 +1327,27 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /* } // Let's find out which interface/socket has the data - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + IfacePtr recv_if; + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { if (FD_ISSET(s.sockfd_, &sockets)) { candidate.reset(new SocketInfo(s)); break; } } if (candidate) { + recv_if = iface; break; } } - if (!candidate) { + if (!candidate || !recv_if) { isc_throw(SocketReadError, "received data over unknown socket"); } // Now we have a socket, let's get some data from it! // Assuming that packet filter is not null, because its modifier checks it. - return (packet_filter_->receive(*iface, *candidate)); + return (packet_filter_->receive(*recv_if, *candidate)); } Pkt6Ptr @@ -1313,8 +1388,8 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) /// @todo: marginal performance optimization. We could create the set once /// and then use its copy for select(). Please note that select() modifies /// provided set to indicated which sockets have something to read. - BOOST_FOREACH(IfacePtr iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { // Only deal with IPv6 addresses. if (s.addr_.isV6()) { // Add this socket to listening set @@ -1327,7 +1402,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) { std::lock_guard lock(callbacks_mutex_); if (!callbacks_.empty()) { - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { // Add this socket to listening set addFDtoSet(s.socket_, maxfd, &sockets); } @@ -1372,7 +1447,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) bool found = false; { std::lock_guard lock(callbacks_mutex_); - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { if (!FD_ISSET(s.socket_, &sockets)) { continue; } @@ -1399,8 +1474,8 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ ) } // Let's find out which interface/socket has the data - BOOST_FOREACH(IfacePtr iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { if (FD_ISSET(s.sockfd_, &sockets)) { candidate.reset(new SocketInfo(s)); break; @@ -1435,7 +1510,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ { std::lock_guard lock(callbacks_mutex_); if (!callbacks_.empty()) { - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { // Add this socket to listening set addFDtoSet(s.socket_, maxfd, &sockets); } @@ -1505,7 +1580,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ bool found = false; { std::lock_guard lock(callbacks_mutex_); - BOOST_FOREACH(SocketCallbackInfo s, callbacks_) { + for (SocketCallbackInfo s : callbacks_) { if (!FD_ISSET(s.socket_, &sockets)) { continue; } @@ -1543,7 +1618,6 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ void IfaceMgr::receiveDHCP4Packets() { - IfacePtr iface; fd_set sockets; int maxfd = 0; @@ -1553,8 +1627,8 @@ IfaceMgr::receiveDHCP4Packets() { addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE), maxfd, &sockets); // Add Interface sockets. - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { // Only deal with IPv4 addresses. if (s.addr_.isV4()) { // Add this socket to listening set. @@ -1601,8 +1675,8 @@ IfaceMgr::receiveDHCP4Packets() { } // Let's find out which interface/socket has data. - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { if (FD_ISSET(s.sockfd_, &sockets)) { receiveDHCP4Packet(*iface, s); // Can take time so check one more time the watch socket. @@ -1618,7 +1692,6 @@ IfaceMgr::receiveDHCP4Packets() { void IfaceMgr::receiveDHCP6Packets() { - IfacePtr iface; fd_set sockets; int maxfd = 0; @@ -1628,8 +1701,8 @@ IfaceMgr::receiveDHCP6Packets() { addFDtoSet(dhcp_receiver_->getWatchFd(WatchedThread::TERMINATE), maxfd, &sockets); // Add Interface sockets. - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { // Only deal with IPv6 addresses. if (s.addr_.isV6()) { // Add this socket to listening set. @@ -1675,8 +1748,8 @@ IfaceMgr::receiveDHCP6Packets() { } // Let's find out which interface/socket has data. - BOOST_FOREACH(iface, ifaces_) { - BOOST_FOREACH(SocketInfo s, iface->getSockets()) { + for (IfacePtr iface : ifaces_) { + for (SocketInfo s : iface->getSockets()) { if (FD_ISSET(s.sockfd_, &sockets)) { receiveDHCP6Packet(s); // Can take time so check one more time the watch socket. @@ -1877,6 +1950,5 @@ IfaceMgr::configureDHCPPacketQueue(uint16_t family, data::ConstElementPtr queue_ return(enable_queue); } - } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index 5e1b934732..8fd8d5e640 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -21,6 +21,10 @@ #include #include +#include +#include +#include +#include #include #include #include @@ -33,7 +37,6 @@ namespace isc { namespace dhcp { - /// @brief IfaceMgr exception thrown thrown when interface detection fails. class IfaceDetectError : public Exception { public: @@ -209,7 +212,7 @@ public: /// @brief Returns interface index. /// /// @return interface index - uint16_t getIndex() const { return ifindex_; } + uint32_t getIndex() const { return ifindex_; } /// @brief Returns interface name. /// @@ -455,8 +458,136 @@ private: std::vector read_buffer_; }; +/// @brief Type definition for the pointer to an @c Iface object. typedef boost::shared_ptr IfacePtr; +/// @brief Collection of pointers to network interfaces. +class IfaceCollection { +public: + + /// @brief Multi index container for network interfaces. + /// + /// This container allows to search for a network interfaces using + /// three indexes: + /// - sequenced: used to access elements in the order they have + /// been added to the container. + /// - interface index: used to access an interface using its index. + /// - interface name: used to access an interface using its name. + /// Note that indexes and names are unique. + typedef boost::multi_index_container< + // Container comprises elements of IfacePtr type. + IfacePtr, + // Here we start enumerating various indexes. + boost::multi_index::indexed_by< + // Sequenced index allows accessing elements in the same way + // as elements in std::list. Sequenced is the index #0. + boost::multi_index::sequenced<>, + // Start definition of index #1. + boost::multi_index::hashed_unique< + // Use the interface index as the key. + boost::multi_index::const_mem_fun< + Iface, uint32_t, &Iface::getIndex + > + >, + // Start definition of index #2. + boost::multi_index::hashed_unique< + // Use the interface name as the key. + boost::multi_index::const_mem_fun< + Iface, std::string, &Iface::getName + > + > + > + > IfaceContainer; + + /// @brief Constructor. + IfaceCollection() { } + + /// @brief Destructor. + ~IfaceCollection() { } + + /// @brief Begin iterator. + /// + /// @return The container sequence begin iterator. + IfaceContainer::const_iterator begin() const { + return (ifaces_container_.begin()); + } + + /// @brief End iterator. + /// + /// @return The container sequence end iterator. + IfaceContainer::const_iterator end() const { + return (ifaces_container_.end()); + } + + /// @brief Empty predicate. + /// + /// @return If the container is empty true else false. + bool empty() const { + return (ifaces_container_.empty()); + } + + /// @brief Return the number of interfaces. + /// + /// @return The container size. + size_t size() const { + return (ifaces_container_.size()); + } + + /// @brief Clear the collection. + void clear() { + cache_.reset(); + ifaces_container_.clear(); + } + + /// @brief Adds an interface to the collection. + /// + /// The interface is added at the end of sequence. + /// + /// @param iface reference to Iface object. + void push_back(const IfacePtr& iface) { + ifaces_container_.push_back(iface); + } + + /// @brief Lookup by interface index. + /// + /// @param ifindex The index of the interface to find. + /// @return The interface with the index or null. + IfacePtr getIface(uint32_t ifindex); + + /// @brief Lookup by interface name. + /// + /// @param ifname The name of the interface to find. + /// @return The interface with the name or null. + IfacePtr getIface(const std::string& ifname); + +private: + /// @brief Lookup by interface index. + /// + /// @param ifindex The index of the interface to find. + /// @param need_lock True when the cache operation needs to hold the mutex. + /// @return The interface with the index or null. + IfacePtr getIfaceInternal(uint32_t ifindex, bool need_lock); + + /// @brief Lookup by interface name. + /// + /// The mutex must be held when called from a packet processing thread. + /// + /// @param ifname The name of the interface to find. + /// @param need_lock True when the cache operation needs to hold the mutex. + /// @return The interface with the name or null. + IfacePtr getIfaceInternal(const std::string& ifname, bool need_lock); + + /// @brief The mutex for protecting the cache from concurrent + /// access from packet processing threads. + std::mutex mutex_; + + /// @brief The last interface returned by a lookup method. + IfacePtr cache_; + + /// @brief The container. + IfaceContainer ifaces_container_; +}; + /// @brief Forward declaration to the @c IfaceMgr. class IfaceMgr; @@ -507,9 +638,6 @@ public: // 2 maps (ifindex-indexed and name-indexed) and // also hide it (make it public make tests easier for now) - /// Type that holds a list of pointers to interfaces. - typedef std::list IfaceCollection; - /// IfaceMgr is a singleton class. This method returns reference /// to its sole instance. /// @@ -1264,9 +1392,12 @@ protected: // TODO: having 2 maps (ifindex->iface and ifname->iface would) // probably be better for performance reasons - /// List of available interfaces + /// @brief List of available interfaces IfaceCollection ifaces_; + /// @brief Return an error when index search fails. + bool fail_on_index_not_found_; + // TODO: Also keep this interface on Iface once interface detection // is implemented. We may need it e.g. to close all sockets on // specific interface diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index 3d153ad233..4f29c52fca 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -328,14 +329,13 @@ public: const bool up, const bool running, const bool inactive4, const bool inactive6) { - for (IfaceMgr::IfaceCollection::iterator iface = ifaces_.begin(); - iface != ifaces_.end(); ++iface) { - if ((*iface)->getName() == name) { - (*iface)->flag_loopback_ = loopback; - (*iface)->flag_up_ = up; - (*iface)->flag_running_ = running; - (*iface)->inactive4_ = inactive4; - (*iface)->inactive6_ = inactive6; + for (IfacePtr iface : ifaces_) { + if (iface->getName() == name) { + iface->flag_loopback_ = loopback; + iface->flag_up_ = up; + iface->flag_running_ = running; + iface->inactive4_ = inactive4; + iface->inactive6_ = inactive6; } } } @@ -921,11 +921,60 @@ TEST_F(IfaceMgrTest, instance) { EXPECT_NO_THROW(IfaceMgr::instance()); } +// Basic tests for Iface inner class. TEST_F(IfaceMgrTest, ifaceClass) { - // Basic tests for Iface inner class Iface iface("eth5", 7); EXPECT_STREQ("eth5/7", iface.getFullName().c_str()); + + EXPECT_THROW_MSG(Iface("foo", -1), OutOfRange, + "Interface index must be in 0..2147483647"); +} + +// This test checks the getIface by packet method. +TEST_F(IfaceMgrTest, getIfaceByPkt) { + NakedIfaceMgr ifacemgr; + // Create a set of fake interfaces. At the same time, remove the actual + // interfaces that have been detected by the IfaceMgr. + ifacemgr.createIfaces(); + + // Try IPv4 packet by name. + Pkt4Ptr pkt4(new Pkt4(DHCPDISCOVER, 1234)); + IfacePtr iface = ifacemgr.getIface(pkt4); + EXPECT_FALSE(iface); + pkt4->setIface("eth0"); + iface = ifacemgr.getIface(pkt4); + EXPECT_TRUE(iface); + EXPECT_FALSE(pkt4->indexSet()); + + // Try IPv6 packet by index. + Pkt6Ptr pkt6(new Pkt6(DHCPV6_REPLY, 123456)); + iface = ifacemgr.getIface(pkt6); + EXPECT_FALSE(iface); + ASSERT_TRUE(ifacemgr.getIface("eth0")); + pkt6->setIndex(ifacemgr.getIface("eth0")->getIndex() + 1); + iface = ifacemgr.getIface(pkt6); + ASSERT_TRUE(iface); + + // Index has precedence when both name and index are available. + EXPECT_EQ("eth1", iface->getName()); + pkt6->setIface("eth0"); + iface = ifacemgr.getIface(pkt6); + ASSERT_TRUE(iface); + EXPECT_EQ("eth1", iface->getName()); + + // Not existing name fails. + pkt4->setIface("eth2"); + 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. + pkt6->setIndex(3); + iface = ifacemgr.getIface(pkt6); + ASSERT_TRUE(iface); + EXPECT_EQ("eth0", iface->getName()); + EXPECT_EQ(1, iface->getIndex()); } // Test that the IPv4 address can be retrieved for the interface. @@ -967,6 +1016,21 @@ TEST_F(IfaceMgrTest, ifaceHasAddress) { EXPECT_FALSE(iface->hasAddress(IOAddress("2001:db8:1::2"))); } +// This test checks it is not allowed to add duplicate interfaces. +TEST_F(IfaceMgrTest, addInterface) { + IfaceMgrTestConfig config(true); + + IfacePtr dup_name(new Iface("eth1", 123)); + EXPECT_THROW_MSG(IfaceMgr::instance().addInterface(dup_name), Unexpected, + "Can't add eth1/123 when eth1/2 already exists."); + IfacePtr dup_index(new Iface("eth2", 2)); + EXPECT_THROW_MSG(IfaceMgr::instance().addInterface(dup_index), Unexpected, + "Can't add eth2/2 when eth1/2 already exists."); + + IfacePtr eth2(new Iface("eth2", 3)); + EXPECT_NO_THROW(IfaceMgr::instance().addInterface(eth2)); +} + // TODO: Implement getPlainMac() test as soon as interface detection // is implemented. TEST_F(IfaceMgrTest, getIface) { @@ -991,10 +1055,8 @@ TEST_F(IfaceMgrTest, getIface) { cout << "There are " << ifacemgr->getIfacesLst().size() << " interfaces." << endl; - for (IfaceMgr::IfaceCollection::iterator iface=ifacemgr->getIfacesLst().begin(); - iface != ifacemgr->getIfacesLst().end(); - ++iface) { - cout << " " << (*iface)->getFullName() << endl; + for (IfacePtr iface : ifacemgr->getIfacesLst()) { + cout << " " << iface->getFullName() << endl; } // Check that interface can be retrieved by ifindex diff --git a/src/lib/dhcpsrv/cfg_iface.cc b/src/lib/dhcpsrv/cfg_iface.cc index 951d880ee6..ea137ca041 100644 --- a/src/lib/dhcpsrv/cfg_iface.cc +++ b/src/lib/dhcpsrv/cfg_iface.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2014-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2014-2020 Internet Systems Consortium, Inc. ("ISC") // // This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this @@ -10,7 +10,6 @@ #include #include #include -#include #include using namespace isc::asiolink; @@ -41,8 +40,7 @@ CfgIface::equals(const CfgIface& other) const { bool CfgIface::multipleAddressesPerInterfaceActive() const { - const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); - BOOST_FOREACH(IfacePtr iface, ifaces) { + for (IfacePtr iface : IfaceMgr::instance().getIfaces()) { if (iface->countActive4() > 1) { return (true); } @@ -172,7 +170,7 @@ CfgIface::openSockets(const uint16_t family, const uint16_t port, // use_bcast is ignored for V6. sopen = IfaceMgr::instance().openSockets6(port, error_callback); } - + if (!sopen) { // If no socket were opened, log a warning because the server will // not respond to any queries. @@ -191,8 +189,7 @@ CfgIface::reset() { void CfgIface::setState(const uint16_t family, const bool inactive, const bool loopback_inactive) const { - const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces(); - BOOST_FOREACH(IfacePtr iface, ifaces) { + for (IfacePtr iface : IfaceMgr::instance().getIfaces()) { bool iface_inactive = iface->flag_loopback_ ? loopback_inactive : inactive; if (family == AF_INET) { iface->inactive4_ = iface_inactive; @@ -209,7 +206,7 @@ void CfgIface::setIfaceAddrsState(const uint16_t family, const bool active, Iface& iface) const { // Activate/deactivate all addresses. - BOOST_FOREACH(Iface::Address addr, iface.getAddresses()) { + for (Iface::Address addr : iface.getAddresses()) { if (addr.get().getFamily() == family) { iface.setActive(addr.get(), active); }