From a464e9a6eb366c62423a28bc7fbfa98f6b0d21e5 Mon Sep 17 00:00:00 2001 From: Michael Davidsaver Date: Thu, 20 Feb 2025 12:07:12 -0800 Subject: [PATCH] redesign IfaceMap Switch to periodic poll on dedicated worker thread instead of opportunistic poll on use. --- src/config.cpp | 23 +-- src/evhelper.cpp | 275 +++++++++++++++------------------- src/evhelper.h | 54 +++---- src/os/WIN32/osdSockExt.cpp | 4 +- src/os/default/osdSockExt.cpp | 4 +- src/udp_collector.cpp | 2 +- test/testsock.cpp | 12 +- test/testudpfwd.cpp | 6 +- tools/mshim.cpp | 2 +- 9 files changed, 170 insertions(+), 212 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 96bca3e..fc292ae 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -60,7 +60,7 @@ SockEndpoint::SockEndpoint(const char* ep, uint16_t defport) iface = at+1; } - auto& ifmap = IfaceMap::instance(); + auto ifmap(IfaceMap::instance()); if(addr.family()==AF_INET6) { if(iface.empty() && addr->in6.sin6_scope_id) { @@ -90,7 +90,7 @@ MCastMembership SockEndpoint::resolve() const if(!addr.isMCast()) throw std::logic_error("not mcast"); - auto& ifmap = IfaceMap::instance(); + auto ifmap(IfaceMap::instance()); MCastMembership m; m.af = addr.family(); @@ -282,7 +282,8 @@ void printAddresses(std::vector& out, const std::vector& ifaces, - std::vector& addrs) + std::vector& addrs, + const IfaceMap& ifmap) { SockAttach attach; evsocket dummy(AF_INET, SOCK_DGRAM, 0); @@ -307,9 +308,9 @@ void expandAddrList(const std::vector& ifaces, } void addGroups(std::vector& ifaces, - const std::vector& addrs) + const std::vector& addrs, + const IfaceMap& ifmap) { - auto& ifmap = IfaceMap::instance(); std::set allifaces; for(const auto& addr : addrs) { @@ -492,7 +493,7 @@ void Config::expand() ifaces.emplace_back(SockAddr::any(AF_INET)); } - auto& ifmap = IfaceMap::instance(); + auto ifmap(IfaceMap::instance()); for(size_t i=0; i adds all bcasts // otherwise add bcast for each iface address - expandAddrList(ifaces, bdest); - addGroups(ifaces, bdest); + expandAddrList(ifaces, bdest, ifmap); + addGroups(ifaces, bdest, ifmap); auto_beacon = false; } @@ -622,6 +623,8 @@ void Config::updateDefs(defs_t& defs) const void Config::expand() { + auto ifmap(IfaceMap::instance()); + if(udp_port==0) throw std::runtime_error("Client can't use UDP random port"); @@ -635,8 +638,8 @@ void Config::expand() ifaces.emplace_back(SockAddr::any(AF_INET)); if(autoAddrList) { - expandAddrList(ifaces, addrs); - addGroups(ifaces, addrs); + expandAddrList(ifaces, addrs, ifmap); + addGroups(ifaces, addrs, ifmap); autoAddrList = false; } diff --git a/src/evhelper.cpp b/src/evhelper.cpp index 3a42477..2a7cdc7 100644 --- a/src/evhelper.cpp +++ b/src/evhelper.cpp @@ -545,7 +545,7 @@ void evsocket::mcast_prep_sendto(const SockEndpoint& ep) const else if(!ep.addr.isMCast()) return; - auto& ifmap = IfaceMap::instance(); + auto ifmap = IfaceMap::instance(); if(af==AF_INET) { SockAddr iface(AF_INET); @@ -697,215 +697,174 @@ bool evsocket::init_canIPv6() noexcept # define getMonotonic getCurrent #endif -static IfaceMap* theinstance; +struct IfMapDaemon : private epicsThreadRunable { + epicsMutex lock; + epicsEvent wake; + std::shared_ptr latest; + bool stop = false; + epicsThread worker; + IfMapDaemon() + :latest(IfaceMap::refresh()) + ,worker(*this, "IfMapDaemon", + epicsThreadGetStackSize(epicsThreadStackBig), + epicsThreadPriorityMin) + { + worker.start(); + } + ~IfMapDaemon() + { + { + Guard G(lock); + stop = true; + } + wake.signal(); + worker.exitWait(); + } + + virtual void run() override final + { + Guard G(lock); + while(!stop) { + try { + std::shared_ptr next; + { + epicsGuardRelease U(G); + wake.wait(15.0); // arbitrary period... + next = IfaceMap::refresh(); + } + latest.swap(next); + + } catch(std::exception& e){ + log_crit_printf(logiface, "While updating IfaceMap : %s\n", e.what()); + } + } + } +} *ifmapper; static void mapInit() { - theinstance = new IfaceMap(); + ifmapper = new IfMapDaemon(); } -IfaceMap& IfaceMap::instance() +IfaceMap IfaceMap::instance() { threadOnce<&mapInit>(); - assert(theinstance); - return *theinstance; + assert(ifmapper); + Guard G(ifmapper->lock); + auto ret(ifmapper->latest); + if(!ret) + throw std::logic_error("No IfaceMap"); + return IfaceMap(std::move(ret)); } void IfaceMap::cleanup() { - delete theinstance; - theinstance = nullptr; + delete ifmapper; + ifmapper = nullptr; } -IfaceMap::IfaceMap() +std::shared_ptr IfaceMap::refresh() { - refresh(); -} - -void IfaceMap::refresh(bool force) -{ - auto now(epicsTime::getMonotonic()); - auto age = now-updated; - double threshold = force ? 10.0 : 600.0; // TODO: configurable? - if(age()); + next->byIndex = _refresh(); // cross-index - decltype (byName) tempN; - decltype (byAddr) tempA; - for(auto& pair : temp) { + for(auto& pair : next->byIndex) { auto& iface = pair.second; - tempN[iface.name] = &iface; + next->byName[iface.name] = &iface; for(auto& pair : iface.addrs) { - tempA.emplace(pair.first, std::make_pair(&iface, false)); - if(pair.second.family()==AF_INET) - tempA.emplace(pair.second, std::make_pair(&iface, true)); + next->byAddr.emplace(pair.first, std::make_pair(&iface, false)); + if(pair.first.family()==AF_INET && pair.second.family()==AF_INET) { + next->byAddr.emplace(pair.second, std::make_pair(&iface, true)); + iface.bcast[pair.second] = pair.first; + } } } - byIndex.swap(temp); - byName.swap(tempN); - byAddr.swap(tempA); - updated = now; + return next; } -namespace { - -template -bool try_cache(IfaceMap& self, FN&& fn) +bool IfaceMap::has_address(uint64_t ifindex, const SockAddr &addr) const { - bool force = false; -retry: - self.refresh(force); - bool found = fn(); - if(!found && !force) { - force = true; - goto retry; - } - return found; -} - -} // namespace - -bool IfaceMap::has_address(uint64_t ifindex, const SockAddr &addr) -{ - Guard G(lock); - if(addr.isAny()) return true; - bool found = try_cache(*this, [this, ifindex, &addr]() { - auto ifit(byIndex.find(ifindex)); - if(ifit!=byIndex.end()) { - const auto& addrs = ifit->second.addrs; - return addrs.find(addr)!=addrs.end(); - } - return false; - }); - return found; -} - -std::string IfaceMap::name_of(uint64_t index) -{ - Guard G(lock); - - std::string name; - bool found = try_cache(*this, [this, index, &name](){ - auto it(byIndex.find(index)); - if(it!=byIndex.end()) { - name = it->second.name; - return true; - } - return false; - }); - if(!found) { - // fallback to numeric index - name = SB()<byIndex.find(ifindex)); + if(ifit!=current->byIndex.end()) { + const auto& addrs = ifit->second.addrs; + return addrs.find(addr)!=addrs.end(); } - return name; + return false; } -std::string IfaceMap::name_of(const SockAddr& addr) +std::string IfaceMap::name_of(uint64_t index) const { - Guard G(lock); - - std::string name; - try_cache(*this, [this, addr, &name](){ - auto it(byAddr.find(addr)); - if(it!=byAddr.end()) { - name = it->second.first->name; - return true; - } - return false; - }); - return name; + auto it(current->byIndex.find(index)); + if(it!=current->byIndex.end()) { + return it->second.name; + } + // fallback to numeric index + return SB()<second->index; - return hit; - }); - return ret; + auto it(current->byAddr.find(addr)); + if(it!=current->byAddr.end()) + return it->second.first->name;; + return std::string(); } -uint64_t IfaceMap::index_of(const SockAddr &addr) +uint64_t IfaceMap::index_of(const std::string& name) const { - Guard G(lock); - - uint64_t ret = 0u; - try_cache(*this, [&ret, this, addr]() { - auto it = byAddr.find(addr); - bool hit = it!=byAddr.end() && !it->second.second; - if(hit) - ret = it->second.first->index; - return hit; - }); - return ret; + auto it = current->byName.find(name); + if(it!=current->byName.end()) + return it->second->index; + return 0; } -bool IfaceMap::is_iface(const SockAddr& addr) +uint64_t IfaceMap::index_of(const SockAddr &addr) const { - Guard G(lock); - - return try_cache(*this, [this, addr]() { - auto it(byAddr.find(addr)); - return it!=byAddr.end() && !it->second.second; - }); + auto it = current->byAddr.find(addr); + if(it!=current->byAddr.end() && !it->second.second) + return it->second.first->index; + return 0; } -bool IfaceMap::is_lo(uint64_t index) +bool IfaceMap::is_iface(const SockAddr& addr) const { - bool is_lo = false; - (void)try_cache(*this, [this, &is_lo, index]() { - auto ifit(byIndex.find(index)); - if(ifit!=byIndex.end()) { // hit - is_lo = ifit->second.isLO; - } - return false; - }); - return is_lo; + auto it(current->byAddr.find(addr)); + return it!=current->byAddr.end() && !it->second.second; } -bool IfaceMap::is_broadcast(const SockAddr& addr) +bool IfaceMap::is_lo(uint64_t index) const { - Guard G(lock); - - return try_cache(*this, [this, addr]() { - auto it(byAddr.find(addr)); - return it!=byAddr.end() && it->second.second; - }); + auto ifit(current->byIndex.find(index)); + if(ifit!=current->byIndex.end()) { // hit + return ifit->second.isLO; + } + return false; } -SockAddr IfaceMap::address_of(const std::string& name) +bool IfaceMap::is_broadcast(const SockAddr& addr) const { - Guard G(lock); - - SockAddr ret; - try_cache(*this, [this, name, &ret]() { - auto it(byName.find(name)); - if(it!=byName.end() && !it->second->addrs.empty()) { - ret = it->second->addrs.begin()->first; - } - return false; - }); - return ret; + auto it(current->byAddr.find(addr)); + return it!=current->byAddr.end() && it->second.second; } -std::set IfaceMap::all_external() +SockAddr IfaceMap::address_of(const std::string& name) const +{ + auto it(current->byName.find(name)); + if(it!=current->byName.end() && !it->second->addrs.empty()) { + return it->second->addrs.begin()->first; + } + return SockAddr(); +} + +std::set IfaceMap::all_external() const { std::set ret; - Guard G(lock); - refresh(); - for(auto& pair : byIndex) { + for(auto& pair : current->byIndex) { ret.emplace(pair.second.name); } return ret; diff --git a/src/evhelper.h b/src/evhelper.h index 6a9904a..d90762b 100644 --- a/src/evhelper.h +++ b/src/evhelper.h @@ -302,55 +302,57 @@ struct PVXS_API evsocket struct PVXS_API IfaceMap { static - IfaceMap& instance(); + IfaceMap instance(); static void cleanup(); - IfaceMap(); - // return true if ifindex is valid, and addr an interface address assigned to it. - bool has_address(uint64_t ifindex, const SockAddr& addr); + bool has_address(uint64_t ifindex, const SockAddr& addr) const; // lookup interface name by index - std::string name_of(uint64_t index); + std::string name_of(uint64_t index) const; // find (an) interface name with this address. useful for IPv4. returns empty string if not found. - std::string name_of(const SockAddr& addr); + std::string name_of(const SockAddr& addr) const; // returns 0 if not found - uint64_t index_of(const std::string& name); + uint64_t index_of(const std::string& name) const; // lookup interface index by interface address (not broadcast addr) - uint64_t index_of(const SockAddr& addr); + uint64_t index_of(const SockAddr& addr) const; // is this a valid interface or broadcast address? - bool is_iface(const SockAddr& addr); + bool is_iface(const SockAddr& addr) const; // is this index the/a loopback interface? - bool is_lo(uint64_t index); + bool is_lo(uint64_t index) const; // is this a valid interface or broadcast address? - bool is_broadcast(const SockAddr& addr); + bool is_broadcast(const SockAddr& addr) const; // look up interface address. useful for IPV4. returns AF_UNSPEC if not found - SockAddr address_of(const std::string& name); + SockAddr address_of(const std::string& name) const; // all interface names except LO - std::set all_external(); - - // caller must hold lock - void refresh(bool force=false); + std::set all_external() const; struct Iface { std::string name; uint64_t index; bool isLO; - // interface address(s) -> (maybe) broadcast addr - std::map addrs; + // addrs - interface address -> (maybe) broadcast addr + // bcast - broadcast -> interface address + std::map addrs, bcast; Iface(const std::string& name, uint64_t index, bool isLO) :name(name), index(index), isLO(isLO) {} }; - SockAttach attach; - epicsMutex lock; - std::map byIndex; - std::map byName; - // map address to tuple of interface and broadcast? - std::multimap, SockAddrOnlyLess> byAddr; - epicsTime updated; + struct Current { + std::map byIndex; + std::map byName; + // map address to tuple of interface and broadcast? + std::multimap, SockAddrOnlyLess> byAddr; + }; + std::shared_ptr current; + + IfaceMap() = default; + IfaceMap(const IfaceMap&) = default; + IfaceMap(std::shared_ptr&& cur) : current(std::move(cur)) {} + static + std::shared_ptr refresh(); private: static - decltype (byIndex) _refresh(); + decltype (Current::byIndex) _refresh(); }; } // namespace impl diff --git a/src/os/WIN32/osdSockExt.cpp b/src/os/WIN32/osdSockExt.cpp index 29e6195..ccaf7d1 100644 --- a/src/os/WIN32/osdSockExt.cpp +++ b/src/os/WIN32/osdSockExt.cpp @@ -205,9 +205,9 @@ namespace impl { # define GAA_FLAG_INCLUDE_ALL_INTERFACES 0 #endif -decltype (IfaceMap::byIndex) IfaceMap::_refresh() { +decltype (IfaceMap::Current::byIndex) IfaceMap::_refresh() { std::vector ifaces(1024u); - decltype (byIndex) temp; + decltype (Current::byIndex) temp; { constexpr ULONG flags = GAA_FLAG_SKIP_ANYCAST diff --git a/src/os/default/osdSockExt.cpp b/src/os/default/osdSockExt.cpp index c9a2683..840dd78 100644 --- a/src/os/default/osdSockExt.cpp +++ b/src/os/default/osdSockExt.cpp @@ -253,10 +253,10 @@ int sendtox::call() namespace impl { -decltype (IfaceMap::byIndex) IfaceMap::_refresh() { +decltype (IfaceMap::Current::byIndex) IfaceMap::_refresh() { ifaddrs* addrs = nullptr; - decltype (byIndex) temp; + decltype (Current::byIndex) temp; if(getifaddrs(&addrs)) { log_warn_printf(logiface, "Unable to getifaddrs() errno=%d\n", errno); diff --git a/src/udp_collector.cpp b/src/udp_collector.cpp index 1c4d463..70350e7 100644 --- a/src/udp_collector.cpp +++ b/src/udp_collector.cpp @@ -96,7 +96,7 @@ struct UDPManager::Pvt { SockAttach attach; evbase loop; - IfaceMap& ifmap; + IfaceMap ifmap; // only manipulate from loop worker thread // key'd by address family and port# diff --git a/test/testsock.cpp b/test/testsock.cpp index aee29ed..dde60b3 100644 --- a/test/testsock.cpp +++ b/test/testsock.cpp @@ -180,18 +180,14 @@ void test_ifacemap() { testDiag("Enter %s", __func__); - auto& ifs = IfaceMap::instance(); + auto ifs(IfaceMap::instance()); - epicsGuard G(ifs.lock); // since we are playing around with the internals... - - ifs.refresh(true); - - testFalse(ifs.byIndex.empty())<<" found "<byIndex.empty())<<" found "<byIndex.size()<<" interfaces"; bool foundlo = false; const auto lo(SockAddr::loopback(AF_INET)); - for(const auto& pair : ifs.byIndex) { + for(const auto& pair : ifs.current->byIndex) { auto& iface = pair.second; testDiag("Interface %u \"%s\"", unsigned(iface.index), iface.name.c_str()); for(const auto& pair : iface.addrs) { @@ -297,7 +293,7 @@ void test_local_mcast() { testDiag("Enter %s", __func__); - IfaceMap ifinfo; + auto ifinfo(IfaceMap::instance()); evsocket A(AF_INET, SOCK_DGRAM, 0, true), B(AF_INET, SOCK_DGRAM, 0, true); diff --git a/test/testudpfwd.cpp b/test/testudpfwd.cpp index 7f49425..2ebb184 100644 --- a/test/testudpfwd.cpp +++ b/test/testudpfwd.cpp @@ -101,11 +101,9 @@ void testFwdIface() std::vector ifaddrs; { - auto& ifs(IfaceMap::instance()); + auto ifs(IfaceMap::instance()); - epicsGuard G(ifs.lock); - - for(auto it : ifs.byIndex) { + for(auto it : ifs.current->byIndex) { auto& iface = it.second; if(iface.isLO) continue; diff --git a/tools/mshim.cpp b/tools/mshim.cpp index 18257c5..62b2984 100644 --- a/tools/mshim.cpp +++ b/tools/mshim.cpp @@ -76,7 +76,7 @@ SockEndpoint parseEP(const char* optarg, const server::Config& conf) struct App { const SockAttach attach; - IfaceMap& ifmap; + IfaceMap ifmap; const evsocket sockTx{AF_INET, SOCK_DGRAM, 0}; std::vector destinations;