From 8ab7f581de542dfcd8b8631bf6392f9264c5a387 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Wed, 16 Sep 2026 17:53:23 -0700 Subject: [PATCH 01/12] client: sync DNS re-resolution for hostname-based endpoints Replace AsyncResolver (async DNS on separate thread) with synchronous setAddress() calls, matching the startup code path. Hostnames are now preserved in Config and Channel so periodic re-resolution and reconnect can re-resolve using the same sync mechanism used at initialization. - Add isHostname() utility to detect hostname vs IP addresses - Preserve original hostnames in Config for addressList and nameServers - Store forcedServerHostname on Channel for reconnect re-resolution - Add periodic DNS recheck timer for search destinations - Remove AsyncResolver, clientresolver.cpp/h, and dedicated DNS thread --- src/client.cpp | 112 +++++++++++++++++++++++++++++++++++----- src/clientimpl.h | 17 ++++-- src/config.cpp | 16 ++++-- src/pvxs/client.h | 10 ++++ src/util.cpp | 34 +++++++++++- src/utilpvt.h | 3 ++ test/Makefile | 4 ++ test/testdnsresolve.cpp | 87 +++++++++++++++++++++++++++++++ 8 files changed, 261 insertions(+), 22 deletions(-) create mode 100644 test/testdnsresolve.cpp diff --git a/src/client.cpp b/src/client.cpp index 948097736..751dd523a 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -68,6 +68,8 @@ constexpr timeval beaconCleanInterval{180, 0}; // special interval to attempt to reconnect to disconnected name servers constexpr timeval tcpNSCheckInterval{10, 0}; +constexpr timeval dnsRecheckInterval{30, 0}; + // searchSequenceID in CMD_SEARCH is redundant. // So we use a static value and instead rely on IDs for individual PVs constexpr uint32_t search_seq{0x66696e64}; // "find" @@ -217,13 +219,18 @@ void Channel::disconnect(const std::shared_ptr& self) name.c_str()); } else if(context->state==ContextImpl::Running) { // reconnect to specific server + if(!forcedServerHostname.empty()) { + forcedServer.setAddress(forcedServerHostname.c_str(), forcedServer.port()); + log_info_printf(io, "Forced server re-resolved for '%s': %s\n", + name.c_str(), forcedServer.tostring().c_str()); + } + conn = Connection::build(context, forcedServer, true); conn->pending[cid] = self; state = Connecting; conn->createChannels(); - } } @@ -382,6 +389,9 @@ std::shared_ptr Channel::build(const std::shared_ptr& cont } else { // bypass search and connect so a specific server chan->forcedServer = forceServer; + if(isHostname(server)) { + chan->forcedServerHostname = server; + } chan->conn = Connection::build(context, forceServer); chan->conn->pending[chan->cid] = chan; @@ -564,6 +574,8 @@ ContextImpl::ContextImpl(const Config& conf, const evbase& tcp_loop) event_new(tcp_loop.base, -1, EV_TIMEOUT|EV_PERSIST, &ContextImpl::cacheCleanS, this)) ,nsChecker(__FILE__, __LINE__, event_new(tcp_loop.base, -1, EV_TIMEOUT|EV_PERSIST, &ContextImpl::onNSCheckS, this)) + ,dnsRecheckTimer(__FILE__, __LINE__, + event_new(tcp_loop.base, -1, EV_TIMEOUT|EV_PERSIST, &ContextImpl::onDNSRecheckS, this)) { searchBuckets.resize(nBuckets); @@ -614,8 +626,15 @@ ContextImpl::ContextImpl(const Config& conf, const evbase& tcp_loop) if(isucast && ep.addr.family()==AF_INET && bcasts.find(ep.addr)!=bcasts.end()) isucast = false; - log_info_printf(io, "Searching to %s%s\n", std::string(SB()<second; + + log_info_printf(io, "Searching to %s%s%s\n", std::string(SB()<second; + + log_info_printf(io, "Searching to TCP %s%s\n", saddr.tostring().c_str(), + (nsHostname.empty()?"":(std::string(" hostname=")+nsHostname).c_str())); + nameServers.push_back({saddr, nullptr, std::move(nsHostname)}); } if(searchDest.empty() && nameServers.empty()) @@ -679,14 +705,16 @@ void ContextImpl::startNS() tcp_loop.call([this]() { // start connections to name servers for(auto& ns : nameServers) { - const auto& serv = ns.first; - ns.second = Connection::build(shared_from_this(), serv); - ns.second->nameserver = true; - log_debug_printf(io, "Connecting to nameserver %s\n", ns.second->peerName.c_str()); + ns.conn = Connection::build(shared_from_this(), ns.addr); + ns.conn->nameserver = true; + log_debug_printf(io, "Connecting to nameserver %s\n", ns.conn->peerName.c_str()); } if(event_add(nsChecker.get(), &tcpNSCheckInterval)) log_err_printf(setup, "Error enabling TCP search reconnect timer\n%s", ""); + + if(event_add(dnsRecheckTimer.get(), &dnsRecheckInterval)) + log_err_printf(setup, "Error enabling DNS recheck timer\n%s", ""); }); } @@ -705,6 +733,7 @@ void ContextImpl::close() (void)event_del(searchRx6.get()); (void)event_del(beaconCleaner.get()); (void)event_del(cacheCleaner.get()); + (void)event_del(dnsRecheckTimer.get()); auto conns(std::move(connByAddr)); // explicitly break ref. loop of channel cache @@ -1224,7 +1253,7 @@ void ContextImpl::tickSearch(SearchKind kind, bool poked) pport[0] = pport[1] = 0; for(auto& pair : nameServers) { - auto& serv = pair.second; + auto& serv = pair.conn; if(!serv->ready || !serv->connection()) continue; @@ -1323,12 +1352,33 @@ void ContextImpl::tickBeaconCleanS(evutil_socket_t fd, short evt, void *raw) void ContextImpl::onNSCheck() { for(auto& ns : nameServers) { - if(ns.second && ns.second->state != ConnBase::Disconnected) // hold-off, connecting, or connected + if(ns.conn && ns.conn->state != ConnBase::Disconnected) continue; - ns.second = Connection::build(shared_from_this(), ns.first); - ns.second->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s\n", ns.second->peerName.c_str()); + if(ns.hostname.empty()) { + ns.conn = Connection::build(shared_from_this(), ns.addr); + ns.conn->nameserver = true; + log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); + } else { + SockAddr resolved; + try { + resolved.setAddress(ns.hostname.c_str(), ns.addr.port()); + } catch(std::exception& e) { + log_warn_printf(io, "DNS resolution failed for nameserver '%s': %s\n", + ns.hostname.c_str(), e.what()); + continue; + } + if(resolved != ns.addr) { + log_info_printf(io, "Nameserver %s re-resolved: %s -> %s\n", + ns.hostname.c_str(), ns.addr.tostring().c_str(), + resolved.tostring().c_str()); + ns.addr = resolved; + } + ns.conn = Connection::build(shared_from_this(), ns.addr); + ns.conn->nameserver = true; + log_debug_printf(io, "Reconnecting nameserver %s (%s)\n", + ns.conn->peerName.c_str(), ns.hostname.c_str()); + } } } @@ -1341,6 +1391,40 @@ void ContextImpl::onNSCheckS(evutil_socket_t fd, short evt, void *raw) } } +void ContextImpl::onDNSRecheck() +{ + for(auto& sd : searchDest) { + if(sd.hostname.empty()) + continue; + + SockAddr resolved; + try { + resolved.setAddress(sd.hostname.c_str(), sd.dest.addr.port()); + } catch(std::exception& e) { + log_warn_printf(io, "DNS resolution failed for search dest '%s': %s\n", + sd.hostname.c_str(), e.what()); + continue; + } + + if(resolved != sd.dest.addr) { + log_info_printf(io, "Search dest %s re-resolved: %s -> %s\n", + sd.hostname.c_str(), + sd.dest.addr.tostring().c_str(), + resolved.tostring().c_str()); + sd.dest.addr = resolved; + } + } +} + +void ContextImpl::onDNSRecheckS(evutil_socket_t fd, short evt, void *raw) +{ + try { + static_cast(raw)->onDNSRecheck(); + }catch(std::exception& e){ + log_exc_printf(io, "Unhandled error in DNS recheck timer callback: %s\n", e.what()); + } +} + void ContextImpl::cacheClean(const std::string& name, Context::cacheAction action) { auto next(chanByName.begin()), diff --git a/src/clientimpl.h b/src/clientimpl.h index 8745c5cdd..35deeb16d 100644 --- a/src/clientimpl.h +++ b/src/clientimpl.h @@ -198,6 +198,7 @@ struct Channel { // channel created with .server() to bypass normal search process SockAddr forcedServer; + std::string forcedServerHostname; // when state==Searching, number of repetitions size_t nSearch = 0u; @@ -275,10 +276,12 @@ struct ContextImpl : public std::enable_shared_from_this // search destination address and whether to set the unicast flag struct SearchDest { - const SockEndpoint dest; + SockEndpoint dest; const bool isucast; bool lastSuccess = true; - SearchDest(SockEndpoint dest, bool isu) :dest(dest), isucast(isu) {} + std::string hostname; + SearchDest(SockEndpoint dest, bool isu, std::string hostname = {}) + :dest(dest), isucast(isu), hostname(std::move(hostname)) {} }; std::vector searchDest; @@ -299,7 +302,12 @@ struct ContextImpl : public std::enable_shared_from_this std::map> connByAddr; - std::vector>> nameServers; + struct NameServerEntry { + SockAddr addr; + std::shared_ptr conn; + std::string hostname; + }; + std::vector nameServers; evbase tcp_loop; const evevent searchRx4, searchRx6; @@ -315,6 +323,7 @@ struct ContextImpl : public std::enable_shared_from_this const evevent beaconCleaner; const evevent cacheCleaner; const evevent nsChecker; + const evevent dnsRecheckTimer; INST_COUNTER(ClientContextImpl); @@ -345,6 +354,8 @@ struct ContextImpl : public std::enable_shared_from_this static void cacheCleanS(evutil_socket_t fd, short evt, void *raw); void onNSCheck(); static void onNSCheckS(evutil_socket_t fd, short evt, void *raw); + void onDNSRecheck(); + static void onDNSRecheckS(evutil_socket_t fd, short evt, void *raw); }; struct Context::Pvt { diff --git a/src/config.cpp b/src/config.cpp index fc292ae7b..2a95c4ac9 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -149,7 +149,8 @@ namespace { constexpr double tmoScale = 4.0/3.0; // 40 second idle timeout / 30 configured void split_addr_into(const char* name, std::vector& out, const std::string& inp, - uint16_t defaultPort, bool required=false) + uint16_t defaultPort, bool required=false, + std::map* hostnameMap=nullptr) { size_t pos=0u; @@ -166,7 +167,12 @@ void split_addr_into(const char* name, std::vector& out, const std: SockEndpoint ep(temp); if(ep.addr.port()==0) ep.addr.setPort(defaultPort); - out.push_back(SB()<& defs, boo } if(pickone({"EPICS_PVA_ADDR_LIST"})) { - split_addr_into(pickone.name.c_str(), self.addressList, pickone.val, self.udp_port); + split_addr_into(pickone.name.c_str(), self.addressList, pickone.val, self.udp_port, + false, &self.addressHostnames); } if(pickone({"EPICS_PVA_NAME_SERVERS"})) { - split_addr_into(pickone.name.c_str(), self.nameServers, pickone.val, self.tcp_port); + split_addr_into(pickone.name.c_str(), self.nameServers, pickone.val, self.tcp_port, + false, &self.nameServerHostnames); } if(pickone({"EPICS_PVA_AUTO_ADDR_LIST"})) { diff --git a/src/pvxs/client.h b/src/pvxs/client.h index 91cde741a..227b8d41d 100644 --- a/src/pvxs/client.h +++ b/src/pvxs/client.h @@ -1026,6 +1026,16 @@ struct PVXS_API Config { //! @since 0.2.0 std::vector nameServers; + //! Maps resolved IP string -> original hostname for entries in addressList. + //! Populated automatically when addressList entries are hostnames. + //! @since NEXT + std::map addressHostnames; + + //! Maps resolved IP string -> original hostname for entries in nameServers. + //! Populated automatically when nameServers entries are hostnames. + //! @since NEXT + std::map nameServerHostnames; + //! UDP port to bind. Default is 5076. May be zero, cf. Server::config() to find allocated port. unsigned short udp_port = 5076; //! Default TCP port for name servers diff --git a/src/util.cpp b/src/util.cpp index 76f473098..15cf539cd 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -892,4 +892,36 @@ void strDiff(std::ostream& out, } } -}} +} + +bool isHostname(const std::string& s) +{ + // strip port suffix and brackets to test the host part only + std::string host(s); + + if(!host.empty() && host.front() == '[') { + // bracketed IPv6: [::1]:port or [::1] + auto bracket = host.find(']'); + if(bracket != std::string::npos) + host = host.substr(1, bracket - 1); + } else { + // for non-bracketed: only strip port if there's exactly one colon (host:port) + auto first_colon = host.find(':'); + auto last_colon = host.rfind(':'); + if(first_colon != std::string::npos && first_colon == last_colon) + host = host.substr(0, first_colon); + } + + if(host.empty()) + return false; + + in_addr dummy4; + in6_addr dummy6; + if(evutil_inet_pton(AF_INET, host.c_str(), &dummy4) == 1) + return false; + if(evutil_inet_pton(AF_INET6, host.c_str(), &dummy6) == 1) + return false; + return true; +} + +} diff --git a/src/utilpvt.h b/src/utilpvt.h index 8d95bada6..7133baaf9 100644 --- a/src/utilpvt.h +++ b/src/utilpvt.h @@ -318,6 +318,9 @@ struct InstCounter { #define DEFINE_INST_COUNTER2(KLASS, NAME) std::atomic KLASS::cnt_ ## NAME {0u} #define DEFINE_INST_COUNTER(KLASS) DEFINE_INST_COUNTER2(KLASS, KLASS) +PVXS_API +bool isHostname(const std::string& s); + } // namespace pvxs #endif // UTILPVT_H diff --git a/test/Makefile b/test/Makefile index bfe42c33d..9dc733c23 100644 --- a/test/Makefile +++ b/test/Makefile @@ -187,6 +187,10 @@ TESTPROD_HOST += eatspam eatspam_SRCS += eatspam.cpp # not a unittest +TESTPROD_HOST += testdnsresolve +testdnsresolve_SRCS += testdnsresolve.cpp +TESTS += testdnsresolve + TESTSCRIPTS_HOST += $(TESTS:%=%.t) ifdef BASE_3_15 ifneq ($(filter $(T_A),$(CROSS_COMPILER_RUNTEST_ARCHS)),) diff --git a/test/testdnsresolve.cpp b/test/testdnsresolve.cpp new file mode 100644 index 000000000..4839f5297 --- /dev/null +++ b/test/testdnsresolve.cpp @@ -0,0 +1,87 @@ +/** + * Copyright - See the COPYRIGHT that is included with this distribution. + * pvxs is distributed subject to a Software License Agreement found + * in file LICENSE that is included with this distribution. + */ + +#include +#include +#include + +#include +#include +#include + +#include + +using namespace pvxs; + +namespace { + +void test_isHostname() +{ + testDiag("%s", __func__); + + testTrue(isHostname("localhost")); + testTrue(isHostname("myhost.example.com")); + testTrue(isHostname("myhost:5075")); + testTrue(!isHostname("127.0.0.1")); + testTrue(!isHostname("127.0.0.1:5075")); + testTrue(!isHostname("::1")); + testTrue(!isHostname("[::1]:5075")); +} + +void test_config_hostname_preservation() +{ + testDiag("%s", __func__); + + client::Config conf; + conf.udp_port = 5076; + conf.tcp_port = 5075; + + epicsEnvSet("EPICS_PVA_NAME_SERVERS", "localhost:5075"); + epicsEnvSet("EPICS_PVA_ADDR_LIST", ""); + epicsEnvSet("EPICS_PVA_AUTO_ADDR_LIST", "NO"); + + conf.applyEnv(); + + testOk(conf.nameServers.size() == 1, "nameServers has one entry"); + testOk(conf.nameServerHostnames.size() == 1, "nameServerHostnames has one entry"); + + if(!conf.nameServerHostnames.empty()) { + auto it = conf.nameServerHostnames.begin(); + testOk(it->second == "localhost:5075", + "hostname preserved: '%s'", it->second.c_str()); + } else { + testSkip(1, "no hostname entries"); + } +} + +void test_config_ip_no_hostname() +{ + testDiag("%s", __func__); + + client::Config conf; + epicsEnvSet("EPICS_PVA_NAME_SERVERS", "127.0.0.1:5075"); + epicsEnvSet("EPICS_PVA_ADDR_LIST", ""); + epicsEnvSet("EPICS_PVA_AUTO_ADDR_LIST", "NO"); + + conf.applyEnv(); + + testOk(conf.nameServerHostnames.empty(), + "no hostname stored for bare IP (size=%zu)", conf.nameServerHostnames.size()); +} + +} // namespace + +MAIN(testdnsresolve) +{ + SockAttach attach; + testPlan(11); + testSetup(); + test_isHostname(); + test_config_hostname_preservation(); + test_config_ip_no_hostname(); + cleanup_for_valgrind(); + return testDone(); +} From a6b4deb3b0ea00a0a43d3ab699348ab6fb717a11 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Wed, 16 Sep 2026 19:07:00 -0700 Subject: [PATCH 02/12] client: proactively re-resolve nameserver hostnames in DNS recheck Extend onDNSRecheck() to also re-resolve nameserver hostnames, not just search destinations. If the IP changes, the old connection is cleaned up and a new one is established immediately. --- src/client.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/client.cpp b/src/client.cpp index 751dd523a..278e07dc2 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1414,6 +1414,34 @@ void ContextImpl::onDNSRecheck() sd.dest.addr = resolved; } } + + for(auto& ns : nameServers) { + if(ns.hostname.empty()) + continue; + + SockAddr resolved; + try { + resolved.setAddress(ns.hostname.c_str(), ns.addr.port()); + } catch(std::exception& e) { + log_warn_printf(io, "DNS resolution failed for nameserver '%s': %s\n", + ns.hostname.c_str(), e.what()); + continue; + } + + if(resolved != ns.addr) { + log_info_printf(io, "Nameserver %s re-resolved: %s -> %s\n", + ns.hostname.c_str(), ns.addr.tostring().c_str(), + resolved.tostring().c_str()); + ns.addr = resolved; + + if(ns.conn) + ns.conn->cleanup(); + ns.conn = Connection::build(shared_from_this(), ns.addr); + ns.conn->nameserver = true; + log_debug_printf(io, "Reconnecting nameserver %s after DNS change\n", + ns.conn->peerName.c_str()); + } + } } void ContextImpl::onDNSRecheckS(evutil_socket_t fd, short evt, void *raw) From 83079d67a690cbbfd9fbb08be17307893219d871 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Thu, 17 Sep 2026 12:40:05 -0700 Subject: [PATCH 03/12] client: start DNS recheck timer for ADDR_LIST hostname entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit startNS() bailed early when nameServers was empty, leaving dnsRecheckTimer dormant. This made onDNSRecheck()'s searchDest re-resolution path dead code — hostnames from EPICS_PVA_ADDR_LIST were resolved once at startup but never re-checked. Now startNS() also checks whether any searchDest entry carries a hostname and starts the DNS recheck timer in that case. The TCP nameserver reconnect timer (nsChecker) remains gated on nameServers being non-empty. --- src/client.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 278e07dc2..9b46236bf 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -699,19 +699,23 @@ ContextImpl::~ContextImpl() {} void ContextImpl::startNS() { - if(nameServers.empty()) // vector size const after ctor, contents remain mutable + bool hasHostnames = std::any_of(searchDest.begin(), searchDest.end(), + [](const SearchDest& sd){ return !sd.hostname.empty(); }); + + if(nameServers.empty() && !hasHostnames) return; - tcp_loop.call([this]() { - // start connections to name servers + tcp_loop.call([this, hasHostnames]() { for(auto& ns : nameServers) { ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; log_debug_printf(io, "Connecting to nameserver %s\n", ns.conn->peerName.c_str()); } - if(event_add(nsChecker.get(), &tcpNSCheckInterval)) - log_err_printf(setup, "Error enabling TCP search reconnect timer\n%s", ""); + if(!nameServers.empty()) { + if(event_add(nsChecker.get(), &tcpNSCheckInterval)) + log_err_printf(setup, "Error enabling TCP search reconnect timer\n%s", ""); + } if(event_add(dnsRecheckTimer.get(), &dnsRecheckInterval)) log_err_printf(setup, "Error enabling DNS recheck timer\n%s", ""); From ffc7c3a87c498c369ccd083d8dc9bdcdc12e3d72 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Thu, 17 Sep 2026 14:13:48 -0700 Subject: [PATCH 04/12] client: fix nameserver reconnect erasing fresh connByAddr entry Connection::build() inserts the new connection into connByAddr keyed by address. Assigning the result to ns.conn then dropped the last reference to the old connection, whose destructor runs cleanup() and erases connByAddr[peerAddr]. For a stable address (e.g. a Kubernetes ClusterIP that is unchanged across pod restarts), this erased the freshly inserted entry, leaving search replies unable to find the nameserver connection. Reset ns.conn before build() so the old destructor's erase runs while the dead entry is still mapped, letting the fresh insert survive. Applied to both onNSCheck() and onDNSRecheck(). --- src/client.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 9b46236bf..713860aea 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -1360,6 +1360,9 @@ void ContextImpl::onNSCheck() continue; if(ns.hostname.empty()) { + // drop old conn first so its dtor's connByAddr.erase(peerAddr) runs + // before build() inserts the fresh entry (same addr -> would erase it) + ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); @@ -1378,6 +1381,7 @@ void ContextImpl::onNSCheck() resolved.tostring().c_str()); ns.addr = resolved; } + ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; log_debug_printf(io, "Reconnecting nameserver %s (%s)\n", @@ -1438,8 +1442,9 @@ void ContextImpl::onDNSRecheck() resolved.tostring().c_str()); ns.addr = resolved; - if(ns.conn) - ns.conn->cleanup(); + // drop old conn first so its dtor's connByAddr.erase(peerAddr) runs + // before build() inserts the fresh entry + ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; log_debug_printf(io, "Reconnecting nameserver %s after DNS change\n", From 62ee566e1bfb7b004ba5d818d3038cb5b3443dfe Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 09:38:00 -0700 Subject: [PATCH 05/12] client: fix EPICS_PVA_SERVER_PORT default ordering for bare-hostname name servers self.tcp_port==0 fallback checked self.nameServers before it was populated by EPICS_PVA_NAME_SERVERS parsing, so it never fired; a bare hostname without an explicit port got resolved and cached with port 0, breaking later DNS re-resolution keying. --- src/config.cpp | 13 +++++++------ test/testdnsresolve.cpp | 30 +++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/config.cpp b/src/config.cpp index 2a95c4ac9..3634d6a25 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -578,18 +578,19 @@ void _fromDefs(Config& self, const std::map& defs, boo log_warn_printf(clientsetup, "%s invalid integer : %s", pickone.name.c_str(), e.what()); } } - if(self.tcp_port==0u && !self.nameServers.empty()) { - log_warn_printf(clientsetup, "ignoring EPICS_PVA_SERVER_PORT=%d\n", 0); - self.tcp_port = 5075; - } - if(pickone({"EPICS_PVA_ADDR_LIST"})) { split_addr_into(pickone.name.c_str(), self.addressList, pickone.val, self.udp_port, false, &self.addressHostnames); } if(pickone({"EPICS_PVA_NAME_SERVERS"})) { - split_addr_into(pickone.name.c_str(), self.nameServers, pickone.val, self.tcp_port, + auto nameServersName(pickone.name); + auto nameServersVal(pickone.val); + if(self.tcp_port==0u) { + log_warn_printf(clientsetup, "ignoring EPICS_PVA_SERVER_PORT=%d\n", 0); + self.tcp_port = 5075; + } + split_addr_into(nameServersName.c_str(), self.nameServers, nameServersVal, self.tcp_port, false, &self.nameServerHostnames); } diff --git a/test/testdnsresolve.cpp b/test/testdnsresolve.cpp index 4839f5297..f9c2dc872 100644 --- a/test/testdnsresolve.cpp +++ b/test/testdnsresolve.cpp @@ -57,6 +57,33 @@ void test_config_hostname_preservation() } } +void test_config_hostname_no_port_defaults_tcp_port() +{ + testDiag("%s", __func__); + + client::Config conf; + conf.udp_port = 5076; + conf.tcp_port = 0; + + epicsEnvUnset("EPICS_PVA_SERVER_PORT"); + epicsEnvSet("EPICS_PVA_NAME_SERVERS", "localhost"); + epicsEnvSet("EPICS_PVA_ADDR_LIST", ""); + epicsEnvSet("EPICS_PVA_AUTO_ADDR_LIST", "NO"); + + conf.applyEnv(); + + testOk(conf.tcp_port == 5075, "tcp_port defaulted to 5075 (got %u)", conf.tcp_port); + + testOk(conf.nameServers.size() == 1, "nameServers has one entry"); + if(!conf.nameServers.empty()) { + auto& ep = conf.nameServers[0]; + testOk(ep.find(":0") == std::string::npos, + "resolved nameserver endpoint doesn't carry port 0 ('%s')", ep.c_str()); + } else { + testSkip(1, "no nameServers entries"); + } +} + void test_config_ip_no_hostname() { testDiag("%s", __func__); @@ -77,10 +104,11 @@ void test_config_ip_no_hostname() MAIN(testdnsresolve) { SockAttach attach; - testPlan(11); + testPlan(14); testSetup(); test_isHostname(); test_config_hostname_preservation(); + test_config_hostname_no_port_defaults_tcp_port(); test_config_ip_no_hostname(); cleanup_for_valgrind(); return testDone(); From dabfdad60825bcf48248715db46657da430c71f0 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 12:13:30 -0700 Subject: [PATCH 06/12] client: fix hostname nameserver permanent reconnect failure after IP change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onNSCheck was reconnecting hostname-based nameservers to potentially stale IPs (OS DNS cache) every 10s, while onDNSRecheck only rebuilt the connection when the resolved IP changed. When a k8s service is deleted+recreated with a new IP, DNS may still return the old IP during propagation — so onDNSRecheck never triggered a reconnect, and onNSCheck kept hammering the dead old IP. Fix by: - onNSCheck skips hostname entries entirely; onDNSRecheck owns them - onDNSRecheck reconnects when IP changed OR conn is Disconnected, so a downed connection always retries even if DNS hasn't propagated yet - Reduce dnsRecheckInterval 30s -> 10s to match tcpNSCheckInterval and recover within typical k8s DNS propagation time --- src/client.cpp | 51 +++++++++++++++++++------------------------------- 1 file changed, 19 insertions(+), 32 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 713860aea..b4c60ae69 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -68,7 +68,7 @@ constexpr timeval beaconCleanInterval{180, 0}; // special interval to attempt to reconnect to disconnected name servers constexpr timeval tcpNSCheckInterval{10, 0}; -constexpr timeval dnsRecheckInterval{30, 0}; +constexpr timeval dnsRecheckInterval{10, 0}; // searchSequenceID in CMD_SEARCH is redundant. // So we use a static value and instead rely on IDs for individual PVs @@ -1356,37 +1356,18 @@ void ContextImpl::tickBeaconCleanS(evutil_socket_t fd, short evt, void *raw) void ContextImpl::onNSCheck() { for(auto& ns : nameServers) { + if(!ns.hostname.empty()) + continue; // hostname entries owned by onDNSRecheck + if(ns.conn && ns.conn->state != ConnBase::Disconnected) continue; - if(ns.hostname.empty()) { - // drop old conn first so its dtor's connByAddr.erase(peerAddr) runs - // before build() inserts the fresh entry (same addr -> would erase it) - ns.conn.reset(); - ns.conn = Connection::build(shared_from_this(), ns.addr); - ns.conn->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); - } else { - SockAddr resolved; - try { - resolved.setAddress(ns.hostname.c_str(), ns.addr.port()); - } catch(std::exception& e) { - log_warn_printf(io, "DNS resolution failed for nameserver '%s': %s\n", - ns.hostname.c_str(), e.what()); - continue; - } - if(resolved != ns.addr) { - log_info_printf(io, "Nameserver %s re-resolved: %s -> %s\n", - ns.hostname.c_str(), ns.addr.tostring().c_str(), - resolved.tostring().c_str()); - ns.addr = resolved; - } - ns.conn.reset(); - ns.conn = Connection::build(shared_from_this(), ns.addr); - ns.conn->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s (%s)\n", - ns.conn->peerName.c_str(), ns.hostname.c_str()); - } + // drop old conn first so its dtor's connByAddr.erase(peerAddr) runs + // before build() inserts the fresh entry (same addr -> would erase it) + ns.conn.reset(); + ns.conn = Connection::build(shared_from_this(), ns.addr); + ns.conn->nameserver = true; + log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); } } @@ -1436,19 +1417,25 @@ void ContextImpl::onDNSRecheck() continue; } - if(resolved != ns.addr) { + bool ipChanged = (resolved != ns.addr); + bool connDown = (!ns.conn || ns.conn->state == ConnBase::Disconnected); + + if(ipChanged) { log_info_printf(io, "Nameserver %s re-resolved: %s -> %s\n", ns.hostname.c_str(), ns.addr.tostring().c_str(), resolved.tostring().c_str()); ns.addr = resolved; + } + if(ipChanged || connDown) { // drop old conn first so its dtor's connByAddr.erase(peerAddr) runs // before build() inserts the fresh entry ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s after DNS change\n", - ns.conn->peerName.c_str()); + log_debug_printf(io, "Reconnecting nameserver %s (%s)%s\n", + ns.conn->peerName.c_str(), ns.hostname.c_str(), + ipChanged ? " after DNS change" : ""); } } } From 6ae8bb5093eb6b4d13d20997fb8922a957bd1c46 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 15:56:32 -0700 Subject: [PATCH 07/12] client: log nameserver connect/reconnect attempts at info level Raises the connect/reconnect log lines in startNS/onNSCheck/onDNSRecheck from debug to info, and includes the hostname on connect, so nameserver contact attempts are visible without enabling debug logging. --- src/client.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index b4c60ae69..0b6fa6d31 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -709,7 +709,8 @@ void ContextImpl::startNS() for(auto& ns : nameServers) { ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_debug_printf(io, "Connecting to nameserver %s\n", ns.conn->peerName.c_str()); + log_info_printf(io, "Connecting to nameserver %s%s%s\n", ns.conn->peerName.c_str(), + (ns.hostname.empty()?"":" hostname="), ns.hostname.c_str()); } if(!nameServers.empty()) { @@ -1367,7 +1368,7 @@ void ContextImpl::onNSCheck() ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); + log_info_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); } } @@ -1433,7 +1434,7 @@ void ContextImpl::onDNSRecheck() ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_debug_printf(io, "Reconnecting nameserver %s (%s)%s\n", + log_info_printf(io, "Reconnecting nameserver %s (%s)%s\n", ns.conn->peerName.c_str(), ns.hostname.c_str(), ipChanged ? " after DNS change" : ""); } From c7e73e21eefd5dd8b32ed52e3c40ba97bd266110 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 15:57:43 -0700 Subject: [PATCH 08/12] client: revert nameserver connect logs back to debug level Keeps the added hostname text in the log messages but reverts the level back to debug, undoing the info bump from the prior commit. --- src/client.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 0b6fa6d31..ac824233c 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -709,7 +709,7 @@ void ContextImpl::startNS() for(auto& ns : nameServers) { ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_info_printf(io, "Connecting to nameserver %s%s%s\n", ns.conn->peerName.c_str(), + log_debug_printf(io, "Connecting to nameserver %s%s%s\n", ns.conn->peerName.c_str(), (ns.hostname.empty()?"":" hostname="), ns.hostname.c_str()); } @@ -1368,7 +1368,7 @@ void ContextImpl::onNSCheck() ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_info_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); + log_debug_printf(io, "Reconnecting nameserver %s\n", ns.conn->peerName.c_str()); } } @@ -1434,7 +1434,7 @@ void ContextImpl::onDNSRecheck() ns.conn.reset(); ns.conn = Connection::build(shared_from_this(), ns.addr); ns.conn->nameserver = true; - log_info_printf(io, "Reconnecting nameserver %s (%s)%s\n", + log_debug_printf(io, "Reconnecting nameserver %s (%s)%s\n", ns.conn->peerName.c_str(), ns.hostname.c_str(), ipChanged ? " after DNS change" : ""); } From 256026670f91c20e1c281f8801703fb8094eccd9 Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 19:03:16 -0700 Subject: [PATCH 09/12] client: add temporary trace logs for nameserver hostname mapping Debugging why EPICS_PVA_NAME_SERVERS hostname association is lost at runtime (reconnects go through the bare-IP onNSCheck path instead of onDNSRecheck). Traces hostnameMap population in split_addr_into and the lookup in ContextImpl's nameServers build loop. --- src/client.cpp | 3 +++ src/config.cpp | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/src/client.cpp b/src/client.cpp index ac824233c..b12d58290 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -651,6 +651,9 @@ ContextImpl::ContextImpl(const Config& conf, const evbase& tcp_loop) if(hit != effective.nameServerHostnames.end()) nsHostname = hit->second; + log_warn_printf(io, "TRACE nameServers loop: addr='%s' hostnameMapSize=%zu found=%d nsHostname='%s'\n", + addr.c_str(), effective.nameServerHostnames.size(), (int)(hit != effective.nameServerHostnames.end()), nsHostname.c_str()); + log_info_printf(io, "Searching to TCP %s%s\n", saddr.tostring().c_str(), (nsHostname.empty()?"":(std::string(" hostname=")+nsHostname).c_str())); nameServers.push_back({saddr, nullptr, std::move(nsHostname)}); diff --git a/src/config.cpp b/src/config.cpp index 3634d6a25..493e1bb17 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -170,8 +170,12 @@ void split_addr_into(const char* name, std::vector& out, const std: auto resolved = (SB()<size()); } } catch(std::exception& e){ From 57dd56bc4205e2c23a21f19fd689e8e0e58b96eb Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Fri, 18 Sep 2026 19:21:34 -0700 Subject: [PATCH 10/12] client: fix SockAddr::setAddress truncating long hostnames before DNS resolution setAddress() copied the pre-colon substring into a fixed scratch[INET6_ADDRSTRLEN+1] (47-byte) buffer and threw "IPv4 address too long" whenever it overflowed, before ever reaching the evutil_inet_pton/GetAddrInfo fallback used for hostname resolution. Kubernetes Service FQDNs (e.g. foo-service.namespace.svc.cluster.local) routinely exceed 46 characters, so any such hostname given via EPICS_PVA_NAME_SERVERS was rejected outright instead of being resolved, silently defeating the DNS-recheck/hostname-tracking reconnect logic. Use a std::string instead of a fixed buffer to remove the length limit. --- src/util.cpp | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 15cf539cd..ca7ffd7b1 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -464,14 +464,14 @@ void SockAddr::setAddress(const char *name, unsigned short defport) throw std::runtime_error(SB()<<"IPv6 with mismatched brackets \""<sa.sa_family = AF_INET; @@ -479,14 +479,9 @@ void SockAddr::setAddress(const char *name, unsigned short defport) } else if(firstc && firstc==lastc && !openb) { // no bracket and only one ':' - // ipv4 w/ port - size_t addrlen = firstc-name; - if(addrlen >= sizeof(scratch)) - throw std::runtime_error(SB()<<"IPv4 address too long \""<sa.sa_family = AF_INET; sockaddr = (void*)&temp->in.sin_addr.s_addr; @@ -502,13 +497,8 @@ void SockAddr::setAddress(const char *name, unsigned short defport) } else if(openb) { // brackets // ipv6, maybe with port - size_t addrlen = closeb-openb-1u; - if(addrlen >= sizeof(scratch)) - throw std::runtime_error(SB()<<"IPv6 address too long \""< closeb) port = lastc+1; else From 489a38babca257ce21a6e49027c2847929b2906d Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Mon, 21 Sep 2026 14:40:34 -0700 Subject: [PATCH 11/12] client: update SockAddr::setAddress documentation to clarify hostname handling --- src/util.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util.cpp b/src/util.cpp index ca7ffd7b1..569c9aed3 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -452,6 +452,14 @@ void SockAddr::setAddress(const char *name, unsigned short defport) * [ipv6] * ipv4:port * ipv4 + * hostname:port + * hostname + * + * "addr" below is first tried as a literal IP (old behavior, no DNS + * involved). Only when that parse fails is it treated as a hostname + * and resolved via DNS (see evutil_inet_pton()/GetAddrInfo fallback + * below) -- so any of the ipv4/ipv6 forms above may have its address + * portion replaced with a hostname. */ // TODO: could optimize to find all of these with a single loop const char *firstc = strchr(name, ':'), From c4d3acbad1b0180de8f804823ad5894859bb986e Mon Sep 17 00:00:00 2001 From: Claudio Bisegni Date: Wed, 23 Sep 2026 14:04:38 -0700 Subject: [PATCH 12/12] test: replace epicsEnvUnset with portable helper for base 3.14 compat epicsEnvUnset() is only available on newer EPICS base (7.0+) and broke the build on the "Native Linux with 3.14" CI job. Add a small unsetEnv() helper using unsetenv()/_putenv() instead. --- test/testdnsresolve.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/testdnsresolve.cpp b/test/testdnsresolve.cpp index f9c2dc872..a18373a2c 100644 --- a/test/testdnsresolve.cpp +++ b/test/testdnsresolve.cpp @@ -4,6 +4,8 @@ * in file LICENSE that is included with this distribution. */ +#include + #include #include #include @@ -18,6 +20,16 @@ using namespace pvxs; namespace { +// epicsEnvUnset() is not available on older EPICS base (e.g. 3.14) +void unsetEnv(const char *name) +{ +#ifdef _WIN32 + _putenv((std::string(name)+"=").c_str()); +#else + unsetenv(name); +#endif +} + void test_isHostname() { testDiag("%s", __func__); @@ -65,7 +77,7 @@ void test_config_hostname_no_port_defaults_tcp_port() conf.udp_port = 5076; conf.tcp_port = 0; - epicsEnvUnset("EPICS_PVA_SERVER_PORT"); + unsetEnv("EPICS_PVA_SERVER_PORT"); epicsEnvSet("EPICS_PVA_NAME_SERVERS", "localhost"); epicsEnvSet("EPICS_PVA_ADDR_LIST", ""); epicsEnvSet("EPICS_PVA_AUTO_ADDR_LIST", "NO");