From b43797bd4c5daa8134910d4eda5f013f0fa4eb2d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 19:42:52 +0000 Subject: [PATCH 01/27] [dhcpmon]: Add packet event quiescing Allow packet handlers to be suspended and resumed without terminating socket event loops, and bound each callback batch so quiescing completes under sustained traffic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 21 ++++++++++++++++++++- src/event_mgr.h | 2 ++ src/packet_handler.cpp | 10 ++++++++-- src/sock_mgr.cpp | 35 +++++++++++++++++++++++++++++++++++ src/sock_mgr.h | 6 ++++++ 5 files changed, 71 insertions(+), 3 deletions(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index 6e1e0e76c..dfd593757 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -70,10 +70,11 @@ void event_mgr::del_all_events(const std::string &tag) { int count = 0; for (const auto &event : this->event_map[tag]) { + int fd = event_get_fd(event); event_del(event); event_free(event); count++; - syslog(LOG_INFO, "event_mgr: Deleted event (fd=%d) of tag %s from %s", event_get_fd(event), tag.c_str(), this->name.c_str()); + syslog(LOG_INFO, "event_mgr: Deleted event (fd=%d) of tag %s from %s", fd, tag.c_str(), this->name.c_str()); } if (tag != "") { std::unordered_set &tagless_set = this->event_map[""]; @@ -88,6 +89,24 @@ void event_mgr::del_all_events(const std::string &tag) syslog(LOG_INFO, "event_mgr: Deleted %d events of tag %s for %s", count, tag.c_str(), this->name.c_str()); } +void event_mgr::suspend_all_events(const std::string &tag) +{ + for (const auto &event : this->event_map[tag]) { + event_del(event); + } +} + +int event_mgr::resume_all_events(const std::string &tag) +{ + for (const auto &event : this->event_map[tag]) { + if (event_add(event, NULL) < 0) { + this->suspend_all_events(tag); + return -1; + } + } + return 0; +} + /** * @code activate_all_events(tag, res); * diff --git a/src/event_mgr.h b/src/event_mgr.h index 90ff4a146..26a1b5135 100644 --- a/src/event_mgr.h +++ b/src/event_mgr.h @@ -12,6 +12,8 @@ class event_mgr { int init_base(); int add_event(struct event* event, const struct timeval *timeout, const std::string &tag=""); void del_all_events(const std::string &tag=""); + void suspend_all_events(const std::string &tag); + int resume_all_events(const std::string &tag); void activate_all_events(const std::string &tag="", int res=0); void free(); struct event_base* get_base(); diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 7ec5d07a3..4353633dc 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -16,6 +16,8 @@ #include "dhcp_check_profile.h" /** to get dhcp/v6 check profile */ #include "util.h" +static constexpr int MAX_PACKETS_PER_CALLBACK = 64; + /** * @code _increase_cache_counter(ifname, sock, type); * @brief helper function to increase cache counter. Simple increase of counter, no complications. In the event of @@ -864,8 +866,12 @@ void callback_common(int fd, short event, void *arg) socklen_t slen = sizeof(sll); sock_info_t &sock_info = sock_mgr_get_sock_info(fd); - while ((buffer_sz = recvfrom(fd, sock_info.buffer, sock_info.snaplen, MSG_DONTWAIT, (struct sockaddr *)&sll, &slen)) > 0) - { + for (int packet_count = 0; packet_count < MAX_PACKETS_PER_CALLBACK; packet_count++) { + buffer_sz = recvfrom(fd, sock_info.buffer, sock_info.snaplen, MSG_DONTWAIT, + (struct sockaddr *)&sll, &slen); + if (buffer_sz <= 0) { + break; + } char ifname_buf[IF_NAMESIZE]; if (if_indextoname(sll.sll_ifindex, ifname_buf) == NULL) { syslog_debug(LOG_WARNING, "if_indextoname: invalid input interface index %d %s", sll.sll_ifindex, strerror(errno)); diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 8d3e48d81..ef934fc44 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -36,6 +36,11 @@ static const char dhcpv6_outbound_filter[] = "outbound and ip6 and udp and (port /** Tags for different events, so we can triiger only one type */ static const char packet_handler_tag[] = "PacketHandler"; static const char cache_counter_updater_tag[] = "CacheCounterUpdater"; +static const char keepalive_tag[] = "Keepalive"; + +static void keepalive_callback(evutil_socket_t, short, void *) +{ +} /* sock fd to sock_info mapping */ std::unordered_map sock_map; @@ -385,6 +390,18 @@ int sock_mgr_init_event_mgr() sock_mgr_free_event_mgr(); return -1; } + struct event *keepalive_event = event_new(info.event_mgr_ptr->get_base(), -1, EV_PERSIST, + keepalive_callback, NULL); + struct timeval keepalive_interval = {.tv_sec = 3600, .tv_usec = 0}; + if (keepalive_event == NULL || + info.event_mgr_ptr->add_event(keepalive_event, &keepalive_interval, keepalive_tag) < 0) { + if (keepalive_event != NULL) { + event_free(keepalive_event); + } + syslog(LOG_ALERT, "Failed to initialize event manager keepalive %s", info.name); + sock_mgr_free_event_mgr(); + return -1; + } } return 0; @@ -432,6 +449,24 @@ void sock_mgr_unregister_packet_handler() } } +void sock_mgr_suspend_packet_handler() +{ + for (const auto &[sock, info] : sock_map) { + info.event_mgr_ptr->suspend_all_events(packet_handler_tag); + } +} + +int sock_mgr_resume_packet_handler() +{ + for (const auto &[sock, info] : sock_map) { + if (info.event_mgr_ptr->resume_all_events(packet_handler_tag) < 0) { + sock_mgr_suspend_packet_handler(); + return -1; + } + } + return 0; +} + int sock_mgr_register_cache_counter_updater(event_callback_fn callback) { syslog(LOG_INFO, "Registering cache counter updater for all sockets"); diff --git a/src/sock_mgr.h b/src/sock_mgr.h index 9619a9526..736ce9b8d 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -59,6 +59,12 @@ int sock_mgr_register_packet_handler(); /** Unregister packet handler for socket manager */ void sock_mgr_unregister_packet_handler(); +/** Temporarily suspend registered packet handlers */ +void sock_mgr_suspend_packet_handler(); + +/** Resume registered packet handlers */ +int sock_mgr_resume_packet_handler(); + /** Register cache counter updater callback for socket manager */ int sock_mgr_register_cache_counter_updater(event_callback_fn callback); From 27f0f10f71762746844c1865cbde8526344242f8 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 20:03:49 +0000 Subject: [PATCH 02/27] [dhcpmon]: Reconcile runtime membership state Rebuild membership maps transactionally and reconcile cache and COUNTERS_DB state while preserving unchanged interface counters and rolling back failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_devman.cpp | 42 ++++++++++++---- src/dhcp_devman.h | 9 ++++ src/dhcp_mon.cpp | 120 ++++++++++++++++++++++++++++++++++++-------- src/dhcp_mon.h | 10 ++++ src/sock_mgr.cpp | 14 ++++++ src/sock_mgr.h | 4 ++ 6 files changed, 168 insertions(+), 31 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 9fa951e3c..28cbbdcbf 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -205,11 +205,13 @@ bool dhcp_devman_is_tracked_interface(const std::string &ifname) * @param none * @return none */ -static void update_vlan_mapping() +static void update_vlan_mapping(const std::shared_ptr &config_db, + std::unordered_map &vlan_mapping, + std::unordered_map> &reverse_vlan_mapping) { syslog(LOG_INFO, "Updating vlan mapping from VLAN_MEMBER"); auto match_pattern = std::string("VLAN_MEMBER|*"); - auto keys = mConfigDbPtr->keys(match_pattern); + auto keys = config_db->keys(match_pattern); std::string all_ifname; std::string all_skipped_ifname; for (const auto &key : keys) { @@ -221,8 +223,8 @@ static void update_vlan_mapping() all_skipped_ifname += "<" + ifname + ", " + vlan + ">, "; continue; } - vlan_map[ifname] = vlan; - rev_vlan_map[vlan].insert(ifname); + vlan_mapping[ifname] = vlan; + reverse_vlan_mapping[vlan].insert(ifname); all_ifname += "<" + ifname + ", " + vlan + ">, "; } syslog(LOG_INFO, "Added vlan member interface mappings: %s", all_ifname.c_str()); @@ -236,11 +238,13 @@ static void update_vlan_mapping() * @param none * @return none */ -static void update_portchannel_mapping() +static void update_portchannel_mapping(const std::shared_ptr &config_db, + std::unordered_map &portchannel_mapping, + std::unordered_map> &reverse_portchannel_mapping) { syslog(LOG_INFO, "Updating port-channel mapping from PORTCHANNEL_MEMBER"); auto match_pattern = std::string("PORTCHANNEL_MEMBER|*"); - auto keys = mConfigDbPtr->keys(match_pattern); + auto keys = config_db->keys(match_pattern); std::string all_ifname; std::string all_skipped_ifname; for (const auto &key : keys) { @@ -252,14 +256,31 @@ static void update_portchannel_mapping() all_skipped_ifname += "<" + ifname + ", " + portchannel + ">, "; continue; } - portchan_map[ifname] = portchannel; - rev_portchan_map[portchannel].insert(ifname); + portchannel_mapping[ifname] = portchannel; + reverse_portchannel_mapping[portchannel].insert(ifname); all_ifname += "<" + ifname + ", " + portchannel + ">, "; } syslog(LOG_INFO, "Added port-channel member interface mappings: %s", all_ifname.c_str()); syslog(LOG_INFO, "Skipped port-channel member interface mappings: %s", all_skipped_ifname.c_str()); } +void dhcp_devman_refresh_mappings() +{ + std::unordered_map new_vlan_map; + std::unordered_map new_portchan_map; + std::unordered_map> new_rev_vlan_map; + std::unordered_map> new_rev_portchan_map; + auto config_db = std::make_shared("CONFIG_DB", 0); + + update_vlan_mapping(config_db, new_vlan_map, new_rev_vlan_map); + update_portchannel_mapping(config_db, new_portchan_map, new_rev_portchan_map); + + vlan_map.swap(new_vlan_map); + portchan_map.swap(new_portchan_map); + rev_vlan_map.swap(new_rev_vlan_map); + rev_portchan_map.swap(new_rev_portchan_map); +} + int dhcp_devman_init() { syslog(LOG_INFO, "Initializing dhcp device manager"); @@ -297,8 +318,7 @@ int dhcp_devman_init() agg_dev_prefix = agg_dev_all + "-"; // vlan and its members, portchannel and its members are initialized regardless of whether they are in cmdline - update_vlan_mapping(); - update_portchannel_mapping(); + dhcp_devman_refresh_mappings(); syslog(LOG_INFO, "Dhcp device manager initialized successfully"); @@ -309,6 +329,8 @@ void dhcp_devman_free() { vlan_map.clear(); portchan_map.clear(); + rev_vlan_map.clear(); + rev_portchan_map.clear(); for (const auto &[ifname, context] : intfs) { dhcp_device_free(context); } diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index 73c1cd6f2..bc2885559 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -117,6 +117,15 @@ bool dhcp_devman_is_tracked_interface(const std::string &ifname); */ int dhcp_devman_init(); +/** + * @code dhcp_devman_refresh_mappings(); + * + * @brief rebuild VLAN and PortChannel membership mappings transactionally from CONFIG_DB. + * + * @return none + */ +void dhcp_devman_refresh_mappings(); + /** * @code dhcp_devman_free(); * diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index ef4f6623d..c3de72965 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -6,7 +6,12 @@ #include #include +#include #include +#include +#include +#include +#include #include #include #include @@ -47,6 +52,7 @@ static const char db_update_tag[] = "DB_UPDATE"; static std::chrono::steady_clock::time_point last_update_time{}; /** Default time point to check whether a time_point has been initialized or updated yet. */ static const std::chrono::steady_clock::time_point default_time_point{}; +static std::thread::id main_thread_id; std::shared_ptr mConfigDbPtr = std::make_shared ("CONFIG_DB", 0); std::shared_ptr mCountersDbPtr = std::make_shared ("COUNTERS_DB", 0); @@ -459,45 +465,117 @@ static void free_event_mgr(struct event_mgr *mgr) * @param none * @return 0 upon success, negative upon failure */ -static void initialize_all_intf_counters() +static void reconcile_all_intf_counters(bool initialize_db) { - for (const auto &[vlan, intfs] : rev_vlan_map) { - for (const auto &ifname : intfs) { + std::unordered_set valid_ifnames; + auto ensure_interface = [&valid_ifnames, initialize_db](const std::string &ifname) { + valid_ifnames.insert(ifname); + if (initialize_db && !all_counters_initialized(ifname)) { initialize_all_counters(ifname); + } else if (!initialize_db && !sock_mgr_all_cache_counters_initialized(ifname)) { + sock_mgr_init_cache_counters(ifname, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } - initialize_all_counters(vlan); - sock_mgr_init_cache_counters(agg_dev_prefix + vlan, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); - } + }; + auto ensure_aggregate = [&valid_ifnames](const std::string &ifname) { + valid_ifnames.insert(ifname); + if (!sock_mgr_all_cache_counters_initialized(ifname)) { + sock_mgr_init_cache_counters(ifname, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); + } + }; - for (const auto &[portchan, intfs] : rev_portchan_map) { - for (const auto &ifname : intfs) { - initialize_all_counters(ifname); + for (const auto &[vlan, members] : rev_vlan_map) { + for (const auto &ifname : members) { + ensure_interface(ifname); } - initialize_all_counters(portchan); - sock_mgr_init_cache_counters(agg_dev_prefix + portchan, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); + ensure_interface(vlan); + ensure_aggregate(agg_dev_prefix + vlan); } - // Now all vlan and portchannel related interfaces have entries in counters, now do the rest (uplink) - for (const auto &itr : intfs) { - if (!all_counters_initialized(itr.first)) { - initialize_all_counters(itr.first); + for (const auto &[portchan, members] : rev_portchan_map) { + for (const auto &ifname : members) { + ensure_interface(ifname); } + ensure_interface(portchan); + ensure_aggregate(agg_dev_prefix + portchan); + } + + for (const auto &entry : intfs) { + ensure_interface(entry.first); } - // also initialize mgmt and agg device counters if (mgmt_ifname.size() > 0) { - initialize_all_counters(mgmt_ifname); + ensure_interface(mgmt_ifname); } + ensure_aggregate(agg_dev_all); - sock_mgr_init_cache_counters(agg_dev_all, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); + sock_mgr_remove_cache_counters_except(valid_ifnames); + for (int sock : {rx_sock, tx_sock, rx_sock_v6, tx_sock_v6}) { + sock_info_t &sock_info = sock_mgr_get_sock_info(sock); + recalculate_agg_counter(sock_info.all_counters); + recalculate_agg_counter(sock_info.all_counters_snapshot); + } - // counter db (the interfaces) might be outdated, clean up stale entries to be in sync with current tracked interfaces - cleanup_stale_db_counters(); + if (initialize_db) { + cleanup_stale_db_counters(); + } +} + +int dhcp_mon_reconcile_topology() +{ + if (std::this_thread::get_id() != main_thread_id) { + syslog(LOG_ALERT, "Topology reconciliation must run on the main event-loop thread"); + return -1; + } + + std::lock_guard lock(db_sync_mutex); + if (!sock_mgr_pause_write_cache_to_db_all_cleared()) { + return 1; + } + + auto old_vlan_map = vlan_map; + auto old_portchan_map = portchan_map; + auto old_rev_vlan_map = rev_vlan_map; + auto old_rev_portchan_map = rev_portchan_map; + std::unordered_map> old_counters; + for (int sock : {rx_sock, tx_sock, rx_sock_v6, tx_sock_v6}) { + sock_info_t &sock_info = sock_mgr_get_sock_info(sock); + old_counters[sock] = {sock_info.all_counters, sock_info.all_counters_snapshot}; + } + + try { + dhcp_devman_refresh_mappings(); + reconcile_all_intf_counters(false); + mCountersDbPtr = std::make_shared("COUNTERS_DB", 0); + sock_mgr_update_db_counters(); + cleanup_stale_db_counters(); + sock_mgr_update_snapshot(); + } catch (const std::exception &e) { + syslog(LOG_ALERT, "Failed to reconcile DHCP interface membership: %s", e.what()); + vlan_map = std::move(old_vlan_map); + portchan_map = std::move(old_portchan_map); + rev_vlan_map = std::move(old_rev_vlan_map); + rev_portchan_map = std::move(old_rev_portchan_map); + for (auto &[sock, counters] : old_counters) { + sock_info_t &sock_info = sock_mgr_get_sock_info(sock); + sock_info.all_counters = std::move(counters.first); + sock_info.all_counters_snapshot = std::move(counters.second); + } + try { + mCountersDbPtr = std::make_shared("COUNTERS_DB", 0); + sock_mgr_update_db_counters(); + cleanup_stale_db_counters(); + } catch (const std::exception &rollback_error) { + syslog(LOG_ALERT, "Failed to restore COUNTERS_DB after topology rollback: %s", rollback_error.what()); + } + return -1; + } + return 0; } int dhcp_mon_init(size_t snaplen, int window_sec, int max_count, int db_update_interval) { int rv = -1; + main_thread_id = std::this_thread::get_id(); syslog(LOG_INFO, "Initializing dhcp monitor with snaplen %zu, window_sec %d, max_count %d, db_update_interval %d", snaplen, window_sec, max_count, db_update_interval); @@ -519,7 +597,7 @@ int dhcp_mon_init(size_t snaplen, int window_sec, int max_count, int db_update_i // deinitialization of counters is not our responsibility // cache counter will be cleanup by sock_mgr_free and the initialized db we intend to keep - initialize_all_intf_counters(); + reconcile_all_intf_counters(true); syslog(LOG_INFO, "Initialized all counters for tracked interfaces"); window_interval_sec = window_sec; diff --git a/src/dhcp_mon.h b/src/dhcp_mon.h index 5a8671b8f..17bf93693 100644 --- a/src/dhcp_mon.h +++ b/src/dhcp_mon.h @@ -26,6 +26,16 @@ extern bool debug_on; */ int dhcp_mon_init(size_t snaplen, int window_sec, int max_count, int db_update_interval); +/** + * @code dhcp_mon_reconcile_topology(); + * + * @brief rebuild interface membership and reconcile counters after packet processing is quiesced. + * Must be called from the main event-loop thread. + * + * @return 0 on success, 1 when counter clear is active, otherwise negative + */ +int dhcp_mon_reconcile_topology(); + /** * @code dhcp_mon_free(); * diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 8d3e48d81..a53a2edbc 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -614,6 +614,20 @@ bool sock_mgr_all_cache_counters_initialized(const std::string &ifname) return true; } +void sock_mgr_remove_cache_counters_except(const std::unordered_set &valid_ifnames) +{ + for (auto &[sock, info] : sock_map) { + for (auto itr = info.all_counters.begin(); itr != info.all_counters.end();) { + if (valid_ifnames.find(itr->first) == valid_ifnames.end()) { + info.all_counters_snapshot.erase(itr->first); + itr = info.all_counters.erase(itr); + } else { + itr++; + } + } + } +} + void sock_mgr_update_db_counters() { syslog_debug(LOG_INFO, "Updating all cache counters to DB counters"); diff --git a/src/sock_mgr.h b/src/sock_mgr.h index 9619a9526..630025b64 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -104,6 +105,9 @@ void sock_mgr_init_cache_counters(const std::string &ifname, uint8_t dhcp_messag /** Check if cache counters are initialized for given ifname for all sockets */ bool sock_mgr_all_cache_counters_initialized(const std::string &ifname); +/** Remove cache counters that are not present in the valid interface set */ +void sock_mgr_remove_cache_counters_except(const std::unordered_set &valid_ifnames); + /** Update database counters from cache counters for all sockets */ void sock_mgr_update_db_counters(); From 049ef7ca70dc3e1b5813e76256a501caa2a92fb2 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 21:07:10 +0000 Subject: [PATCH 03/27] [dhcpmon]: Listen for membership updates Register VLAN_MEMBER and PORTCHANNEL_MEMBER subscriber file descriptors on the main event loop and apply quiesced transactional refreshes without restarting dhcpmon. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index c3de72965..d467c476e 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -53,6 +54,11 @@ static std::chrono::steady_clock::time_point last_update_time{}; /** Default time point to check whether a time_point has been initialized or updated yet. */ static const std::chrono::steady_clock::time_point default_time_point{}; static std::thread::id main_thread_id; +static bool topology_refresh_pending = false; +static bool config_subscribers_failed = false; +static std::shared_ptr vlan_member_subscriber; +static std::shared_ptr portchannel_member_subscriber; +static const char config_event_tag[] = "CONFIG_UPDATE"; std::shared_ptr mConfigDbPtr = std::make_shared ("CONFIG_DB", 0); std::shared_ptr mCountersDbPtr = std::make_shared ("COUNTERS_DB", 0); @@ -61,6 +67,62 @@ std::shared_ptr mStateDbMuxTablePtr = std::make_shared mStateDbPtr.get(), "HW_MUX_CABLE_TABLE" ); +static void config_update_callback(evutil_socket_t fd, short event, void *arg) +{ + auto *subscriber = static_cast(arg); + try { + subscriber->readData(); + std::deque entries; + subscriber->pops(entries); + if (!entries.empty()) { + topology_refresh_pending = true; + } + } catch (const std::exception &e) { + syslog(LOG_ALERT, "Failed to read DHCP membership update: %s", e.what()); + config_subscribers_failed = true; + topology_refresh_pending = true; + main_event_mgr->suspend_all_events(config_event_tag); + } +} + +static void clear_config_events() +{ + main_event_mgr->del_all_events(config_event_tag); + vlan_member_subscriber.reset(); + portchannel_member_subscriber.reset(); +} + +static int register_config_events() +{ + clear_config_events(); + try { + vlan_member_subscriber = std::make_shared( + mConfigDbPtr.get(), "VLAN_MEMBER"); + portchannel_member_subscriber = std::make_shared( + mConfigDbPtr.get(), "PORTCHANNEL_MEMBER"); + } catch (const std::exception &e) { + syslog(LOG_ALERT, "Failed to initialize DHCP membership subscribers: %s", e.what()); + return -1; + } + + for (const auto &subscriber : {vlan_member_subscriber, portchannel_member_subscriber}) { + struct event *config_event = event_new(main_event_mgr->get_base(), subscriber->getFd(), + EV_READ | EV_PERSIST, config_update_callback, + subscriber.get()); + if (config_event == NULL || + main_event_mgr->add_event(config_event, NULL, config_event_tag) < 0) { + if (config_event != NULL) { + event_free(config_event); + } + syslog(LOG_ALERT, "Failed to register DHCP membership event"); + clear_config_events(); + return -1; + } + } + config_subscribers_failed = false; + return 0; +} + /** * @code recalculate_agg_counter(all_counters); * @@ -399,6 +461,31 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received timeout signal for DHCP relay health check"); + if (config_subscribers_failed) { + if (register_config_events() < 0) { + return; + } + topology_refresh_pending = true; + } + + if (topology_refresh_pending) { + sock_mgr_suspend_packet_handler(); + int result = dhcp_mon_reconcile_topology(); + if (sock_mgr_resume_packet_handler() < 0) { + syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); + return; + } + if (result == 1) { + return; + } + topology_refresh_pending = result < 0; + if (result != 0) { + return; + } + syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); + return; + } + dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_SNAPSHOT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT_V6); @@ -745,6 +832,10 @@ static int register_main_events() break; } + if (register_config_events() < 0) { + break; + } + rv = 0; syslog(LOG_INFO, "Main events registered successfully"); @@ -789,6 +880,13 @@ int dhcp_mon_start() goto unregister_cache_counter_updater; } + topology_refresh_pending = true; + sock_mgr_suspend_packet_handler(); + if (dhcp_mon_reconcile_topology() != 0 || sock_mgr_resume_packet_handler() < 0) { + goto unregister_main_events; + } + topology_refresh_pending = false; + sock_mgr_drain_sock_buffer(); // it could fail and we wouldnt know it because its in another thread From aa103b13668ab3665874749c03ccf29f6dfa489c Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:15:06 +0000 Subject: [PATCH 04/27] [dhcpmon]: Preserve health checks during refresh deferral Continue checking the last topology while counter clear or subscriber recovery delays a pending refresh. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index d467c476e..7079137f4 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -461,14 +461,17 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received timeout signal for DHCP relay health check"); + bool subscribers_available = true; if (config_subscribers_failed) { if (register_config_events() < 0) { - return; + topology_refresh_pending = true; + subscribers_available = false; + } else { + topology_refresh_pending = true; } - topology_refresh_pending = true; } - if (topology_refresh_pending) { + if (topology_refresh_pending && subscribers_available) { sock_mgr_suspend_packet_handler(); int result = dhcp_mon_reconcile_topology(); if (sock_mgr_resume_packet_handler() < 0) { @@ -476,14 +479,15 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) return; } if (result == 1) { + topology_refresh_pending = true; + } else { + topology_refresh_pending = result < 0; + if (result != 0) { + return; + } + syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); return; } - topology_refresh_pending = result < 0; - if (result != 0) { - return; - } - syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); - return; } dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT); From efb32828bb1bfe9418f85cb6bc7dda9f4d4f9117 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:06:44 +0000 Subject: [PATCH 05/27] [dhcpmon]: Harden packet event resume Reset sockaddr length for each batch receive and reject timeout events from the fd-only resume path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 5 +++++ src/packet_handler.cpp | 1 + 2 files changed, 6 insertions(+) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index dfd593757..5d6e43f2f 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -99,6 +99,11 @@ void event_mgr::suspend_all_events(const std::string &tag) int event_mgr::resume_all_events(const std::string &tag) { for (const auto &event : this->event_map[tag]) { + if (event_get_fd(event) < 0) { + syslog(LOG_ALERT, "event_mgr: Cannot resume non-fd event with tag %s", tag.c_str()); + this->suspend_all_events(tag); + return -1; + } if (event_add(event, NULL) < 0) { this->suspend_all_events(tag); return -1; diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 4353633dc..9f38ed5b2 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -867,6 +867,7 @@ void callback_common(int fd, short event, void *arg) sock_info_t &sock_info = sock_mgr_get_sock_info(fd); for (int packet_count = 0; packet_count < MAX_PACKETS_PER_CALLBACK; packet_count++) { + slen = sizeof(sll); buffer_sz = recvfrom(fd, sock_info.buffer, sock_info.snaplen, MSG_DONTWAIT, (struct sockaddr *)&sll, &slen); if (buffer_sz <= 0) { From 21ae089929ee20f0b5a43e81658ca493442421db Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 06/27] [dhcpmon]: Guard packet event suspension Reject untagged suspend/resume requests and log the event manager and fd when a packet event cannot be restored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index 5d6e43f2f..b92cd919a 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -91,6 +91,11 @@ void event_mgr::del_all_events(const std::string &tag) void event_mgr::suspend_all_events(const std::string &tag) { + if (tag.empty()) { + syslog(LOG_ALERT, "event_mgr: Refusing to suspend untagged events for %s", + this->name.c_str()); + return; + } for (const auto &event : this->event_map[tag]) { event_del(event); } @@ -98,13 +103,21 @@ void event_mgr::suspend_all_events(const std::string &tag) int event_mgr::resume_all_events(const std::string &tag) { + if (tag.empty()) { + syslog(LOG_ALERT, "event_mgr: Refusing to resume untagged events for %s", + this->name.c_str()); + return -1; + } for (const auto &event : this->event_map[tag]) { if (event_get_fd(event) < 0) { - syslog(LOG_ALERT, "event_mgr: Cannot resume non-fd event with tag %s", tag.c_str()); + syslog(LOG_ALERT, "event_mgr: Cannot resume non-fd event with tag %s for %s", + tag.c_str(), this->name.c_str()); this->suspend_all_events(tag); return -1; } if (event_add(event, NULL) < 0) { + syslog(LOG_ALERT, "event_mgr: Failed to resume event (fd=%d) with tag %s for %s", + event_get_fd(event), tag.c_str(), this->name.c_str()); this->suspend_all_events(tag); return -1; } From 1bef9c3b476937620d6411081407ed63608a7ceb Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:04:09 +1000 Subject: [PATCH 07/27] [dhcpmon]: Fail safely on topology refresh errors Return a startup error when CONFIG_DB mapping initialization throws, and stop the monitor so supervisor recovery can restore packet handling after a resume failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_devman.cpp | 9 ++++++++- src/dhcp_mon.cpp | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 28cbbdcbf..88fea920a 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include "dhcp_devman.h" @@ -318,7 +320,12 @@ int dhcp_devman_init() agg_dev_prefix = agg_dev_all + "-"; // vlan and its members, portchannel and its members are initialized regardless of whether they are in cmdline - dhcp_devman_refresh_mappings(); + try { + dhcp_devman_refresh_mappings(); + } catch (const std::exception &e) { + syslog(LOG_ALERT, "Failed to initialize DHCP interface mappings: %s", e.what()); + return -1; + } syslog(LOG_INFO, "Dhcp device manager initialized successfully"); diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 7079137f4..d37188fda 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -476,6 +476,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) int result = dhcp_mon_reconcile_topology(); if (sock_mgr_resume_packet_handler() < 0) { syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); + dhcp_mon_stop(); return; } if (result == 1) { From 282c578a9ec9f02300c4d3fef6412b5e826cceb9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:33:38 +1000 Subject: [PATCH 08/27] [dhcpmon]: Reject unknown packet event tags Look up tagged event sets without default insertion, logging unknown suspend tags and returning an error for unknown resume tags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index b92cd919a..76f4b8b24 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -96,7 +96,13 @@ void event_mgr::suspend_all_events(const std::string &tag) this->name.c_str()); return; } - for (const auto &event : this->event_map[tag]) { + const auto tagged_events = this->event_map.find(tag); + if (tagged_events == this->event_map.end()) { + syslog(LOG_ALERT, "event_mgr: Cannot suspend unknown tag %s for %s", + tag.c_str(), this->name.c_str()); + return; + } + for (const auto &event : tagged_events->second) { event_del(event); } } @@ -108,7 +114,13 @@ int event_mgr::resume_all_events(const std::string &tag) this->name.c_str()); return -1; } - for (const auto &event : this->event_map[tag]) { + const auto tagged_events = this->event_map.find(tag); + if (tagged_events == this->event_map.end()) { + syslog(LOG_ALERT, "event_mgr: Cannot resume unknown tag %s for %s", + tag.c_str(), this->name.c_str()); + return -1; + } + for (const auto &event : tagged_events->second) { if (event_get_fd(event) < 0) { syslog(LOG_ALERT, "event_mgr: Cannot resume non-fd event with tag %s for %s", tag.c_str(), this->name.c_str()); From ab9d9d7d8d00bafff37cde5c72ee9d24806382d6 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:43:56 +1000 Subject: [PATCH 09/27] [dhcpmon]: Wait for in-flight packet callbacks Gate packet callbacks before deleting their events and take an exclusive quiesce lock so topology reconciliation cannot race a callback already executing on a socket event thread. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/packet_handler.cpp | 7 +++++++ src/sock_mgr.cpp | 37 ++++++++++++++++++++++++++++++------- src/sock_mgr.h | 6 ++++++ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 9f38ed5b2..d52f2d048 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -861,6 +861,13 @@ void packet_handler_v6(int sock, const std::string &ifname, const dhcp_device_co void callback_common(int fd, short event, void *arg) { + if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + return; + } + std::shared_lock packet_handler_lock(packet_handler_quiesce_mutex); + if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + return; + } ssize_t buffer_sz; struct sockaddr_ll sll; socklen_t slen = sizeof(sll); diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index bd3fa073f..1d4d7a7ae 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "sock_mgr.h" @@ -45,6 +46,10 @@ static void keepalive_callback(evutil_socket_t, short, void *) /* sock fd to sock_info mapping */ std::unordered_map sock_map; +std::shared_mutex packet_handler_quiesce_mutex; +std::atomic packet_handlers_enabled{true}; +static std::unique_lock packet_handler_quiesce_lock; + extern std::shared_ptr mCountersDbPtr; extern std::string downstream_ifname; @@ -451,20 +456,38 @@ void sock_mgr_unregister_packet_handler() void sock_mgr_suspend_packet_handler() { - for (const auto &[sock, info] : sock_map) { - info.event_mgr_ptr->suspend_all_events(packet_handler_tag); + if (packet_handler_quiesce_lock.owns_lock()) { + syslog(LOG_ALERT, "Packet handlers are already suspended"); + return; + } + packet_handlers_enabled.store(false, std::memory_order_release); + for (const auto &entry : sock_map) { + entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); } + packet_handler_quiesce_lock = std::unique_lock(packet_handler_quiesce_mutex); } int sock_mgr_resume_packet_handler() { - for (const auto &[sock, info] : sock_map) { - if (info.event_mgr_ptr->resume_all_events(packet_handler_tag) < 0) { - sock_mgr_suspend_packet_handler(); - return -1; + if (!packet_handler_quiesce_lock.owns_lock()) { + syslog(LOG_ALERT, "Packet handlers are not suspended"); + return -1; + } + int result = 0; + for (const auto &entry : sock_map) { + if (entry.second.event_mgr_ptr->resume_all_events(packet_handler_tag) < 0) { + for (const auto &suspended_entry : sock_map) { + suspended_entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); + } + result = -1; + break; } } - return 0; + if (result == 0) { + packet_handlers_enabled.store(true, std::memory_order_release); + } + packet_handler_quiesce_lock.unlock(); + return result; } int sock_mgr_register_cache_counter_updater(event_callback_fn callback) diff --git a/src/sock_mgr.h b/src/sock_mgr.h index a5ca52fd8..541535d87 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -9,7 +9,9 @@ #ifndef SOCKET_MANAGER_H_ #define SOCKET_MANAGER_H_ +#include #include +#include #include #include #include @@ -42,6 +44,10 @@ typedef struct { /** sock file descriptors, serve as the identifier of all related information described in sock_info_t */ extern int rx_sock, tx_sock, rx_sock_v6, tx_sock_v6; +/** Guards in-flight packet callbacks while topology and counters are reconciled */ +extern std::shared_mutex packet_handler_quiesce_mutex; +extern std::atomic packet_handlers_enabled; + /** Initialize socket manager with given snaplen */ int sock_mgr_init(uint32_t snaplen); From 6ce460da43ea308ec0b0438054b92b4b328a5038 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:45:09 +1000 Subject: [PATCH 10/27] [dhcpmon]: Release packet quiesce on startup failure Always resume and release the packet-handler quiesce lock after initial topology reconciliation, including the error path before event-loop cleanup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index d37188fda..e79dc8515 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -867,6 +867,8 @@ static int register_main_events() int dhcp_mon_start() { int rv = -1; + int reconcile_result = -1; + int resume_result = -1; syslog(LOG_INFO, "Starting dhcp monitor in %s", debug_on ? "debug mode" : "normal mode"); @@ -887,7 +889,9 @@ int dhcp_mon_start() topology_refresh_pending = true; sock_mgr_suspend_packet_handler(); - if (dhcp_mon_reconcile_topology() != 0 || sock_mgr_resume_packet_handler() < 0) { + reconcile_result = dhcp_mon_reconcile_topology(); + resume_result = sock_mgr_resume_packet_handler(); + if (reconcile_result != 0 || resume_result < 0) { goto unregister_main_events; } topology_refresh_pending = false; From 14919daa5d1a90f3e9bc1c87bfc0ca44923383d1 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:02:47 +1000 Subject: [PATCH 11/27] [dhcpmon]: Handle topology snapshot failures Catch exceptions while copying the rollback snapshot before any live topology or counter state is changed, and return a controlled reconciliation failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index e79dc8515..ca50a50fb 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -624,14 +624,23 @@ int dhcp_mon_reconcile_topology() return 1; } - auto old_vlan_map = vlan_map; - auto old_portchan_map = portchan_map; - auto old_rev_vlan_map = rev_vlan_map; - auto old_rev_portchan_map = rev_portchan_map; + decltype(vlan_map) old_vlan_map; + decltype(portchan_map) old_portchan_map; + decltype(rev_vlan_map) old_rev_vlan_map; + decltype(rev_portchan_map) old_rev_portchan_map; std::unordered_map> old_counters; - for (int sock : {rx_sock, tx_sock, rx_sock_v6, tx_sock_v6}) { - sock_info_t &sock_info = sock_mgr_get_sock_info(sock); - old_counters[sock] = {sock_info.all_counters, sock_info.all_counters_snapshot}; + try { + old_vlan_map = vlan_map; + old_portchan_map = portchan_map; + old_rev_vlan_map = rev_vlan_map; + old_rev_portchan_map = rev_portchan_map; + for (int sock : {rx_sock, tx_sock, rx_sock_v6, tx_sock_v6}) { + sock_info_t &sock_info = sock_mgr_get_sock_info(sock); + old_counters[sock] = {sock_info.all_counters, sock_info.all_counters_snapshot}; + } + } catch (const std::exception &e) { + syslog(LOG_ALERT, "Failed to snapshot DHCP topology before reconciliation: %s", e.what()); + return -1; } try { From 8b4b0590f240b3edddefdf7ff08987a4673cc1a1 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:03:35 +1000 Subject: [PATCH 12/27] [dhcpmon]: Avoid implicit event tag creation Use explicit tag lookups for event deletion and activation so unknown tags are logged without mutating the event map or reporting a silent no-op. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index 76f4b8b24..84da07fd5 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -69,19 +69,26 @@ int event_mgr::add_event(struct event* event, const struct timeval *timeout, con void event_mgr::del_all_events(const std::string &tag) { int count = 0; - for (const auto &event : this->event_map[tag]) { + const auto tagged_events = this->event_map.find(tag); + if (tagged_events == this->event_map.end()) { + if (!tag.empty()) { + syslog(LOG_WARNING, "event_mgr: Cannot delete unknown tag %s for %s", + tag.c_str(), this->name.c_str()); + } + return; + } + auto all_events = this->event_map.find(""); + for (const auto &event : tagged_events->second) { int fd = event_get_fd(event); + if (!tag.empty() && all_events != this->event_map.end()) { + all_events->second.erase(event); + } event_del(event); event_free(event); count++; syslog(LOG_INFO, "event_mgr: Deleted event (fd=%d) of tag %s from %s", fd, tag.c_str(), this->name.c_str()); } - if (tag != "") { - std::unordered_set &tagless_set = this->event_map[""]; - std::unordered_set &tagged_set = this->event_map[tag]; - for (const auto &event : tagged_set) { - tagless_set.erase(event); - } + if (!tag.empty()) { this->event_map.erase(tag); } else { this->event_map.clear(); @@ -146,7 +153,15 @@ int event_mgr::resume_all_events(const std::string &tag) */ void event_mgr::activate_all_events(const std::string &tag, int res) { - for (const auto &event : this->event_map[tag]) { + const auto tagged_events = this->event_map.find(tag); + if (tagged_events == this->event_map.end()) { + if (!tag.empty()) { + syslog(LOG_WARNING, "event_mgr: Cannot activate unknown tag %s for %s", + tag.c_str(), this->name.c_str()); + } + return; + } + for (const auto &event : tagged_events->second) { event_active(event, res, 0); syslog(LOG_INFO, "event_mgr: Activated event (fd=%d) of tag %s from %s", event_get_fd(event), tag.c_str(), this->name.c_str()); } From c7ba5ce2a0916532e7e63fae44bd22e30248b651 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:17:01 +1000 Subject: [PATCH 13/27] [dhcpmon]: Avoid blocking callbacks during quiesce Use a non-blocking shared-lock attempt so an event-loop callback exits immediately when topology reconciliation owns the exclusive quiesce lock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/packet_handler.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index d52f2d048..742b388fd 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include "packet_handler.h" @@ -864,8 +865,10 @@ void callback_common(int fd, short event, void *arg) if (!packet_handlers_enabled.load(std::memory_order_acquire)) { return; } - std::shared_lock packet_handler_lock(packet_handler_quiesce_mutex); - if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + std::shared_lock packet_handler_lock(packet_handler_quiesce_mutex, + std::try_to_lock); + if (!packet_handler_lock.owns_lock() || + !packet_handlers_enabled.load(std::memory_order_acquire)) { return; } ssize_t buffer_sz; From 8764b407800d7f43617dd87e58df6f183b286e2b Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:17:01 +1000 Subject: [PATCH 14/27] [dhcpmon]: Keep event cleanup idempotent Treat deletion of an absent event tag as the expected cleanup no-op while retaining explicit lookup and avoiding map mutation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/event_mgr.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index 84da07fd5..a794cee1a 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -71,10 +71,6 @@ void event_mgr::del_all_events(const std::string &tag) int count = 0; const auto tagged_events = this->event_map.find(tag); if (tagged_events == this->event_map.end()) { - if (!tag.empty()) { - syslog(LOG_WARNING, "event_mgr: Cannot delete unknown tag %s for %s", - tag.c_str(), this->name.c_str()); - } return; } auto all_events = this->event_map.find(""); From 7756d7fcddade467920198bc23f17dc4ebe11dfc Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:18:14 +1000 Subject: [PATCH 15/27] [dhcpmon]: Correct counter reconcile documentation Document the renamed helper, its initialize_db argument, and its void return contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index ca50a50fb..7d6bf316f 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -552,10 +552,10 @@ static void free_event_mgr(struct event_mgr *mgr) } /** - * @code initialize_all_intf_counters(); - * @brief Initialize all db counters and cache counters for all tracked interfaces - * @param none - * @return 0 upon success, negative upon failure + * @code reconcile_all_intf_counters(initialize_db); + * @brief Reconcile cache counters for all tracked interfaces + * @param initialize_db initialize missing database counters when true + * @return none */ static void reconcile_all_intf_counters(bool initialize_db) { From 43dbc8835dd9ac69503bee529580818fbfb76dd1 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:29:00 +1000 Subject: [PATCH 16/27] [dhcpmon]: Synchronize counter sampling and updates Take the exclusive counter-state lock for health snapshots, DB synchronization, clear-counter cache updates, and signal status reads so packet callbacks cannot race those operations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 7d6bf316f..3c044d01a 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -280,9 +280,12 @@ static void cleanup_stale_db_counters() static void signal_callback(evutil_socket_t fd, short event, void *arg) { syslog(LOG_INFO, "Received signal: %s", strsignal(fd)); - - dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT); - dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT_V6); + + { + std::unique_lock counter_lock(packet_handler_quiesce_mutex); + dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT); + dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT_V6); + } if ((fd == SIGTERM) || (fd == SIGINT)) { syslog(LOG_INFO, "Received signal to stop dhcpmon"); @@ -291,6 +294,7 @@ static void signal_callback(evutil_socket_t fd, short event, void *arg) if (fd == SIGUSR1) { // we need to sync cache counter from COUNTERS_DB syslog(LOG_INFO, "Received signal to stop writing to DB counter"); + std::unique_lock counter_lock(packet_handler_quiesce_mutex); std::lock_guard lock(db_sync_mutex); sock_mgr_pause_write_cache_to_db(); syslog(LOG_INFO, "Stopped writing to DB counter"); @@ -328,6 +332,7 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void syslog(LOG_INFO, "Start updating %s cache counter from DB counter", sock_info.name); + std::unique_lock counter_lock(packet_handler_quiesce_mutex); std::lock_guard lock(db_sync_mutex); // can only sync db to cache counter and db updater is paused, otherwise its unexpected @@ -460,6 +465,7 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void static void timeout_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received timeout signal for DHCP relay health check"); + std::unique_lock counter_lock(packet_handler_quiesce_mutex); bool subscribers_available = true; if (config_subscribers_failed) { @@ -516,6 +522,7 @@ static void db_update_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received db update signal"); syslog_debug(LOG_INFO, "Sync cache counter to DB counter"); + std::unique_lock counter_lock(packet_handler_quiesce_mutex); std::lock_guard lock(db_sync_mutex); // If there is clear counter going on and its been longer than expected // consider the clear counter operation failed so we don't block db update forever From 1123988b4e1e580a2f3d7a79d692394e98bdadfa Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:30:26 +1000 Subject: [PATCH 17/27] [dhcpmon]: Lock health sampling after topology refresh Acquire the standalone counter-sampling lock only after any topology refresh has completed and released the packet-handler quiesce lock. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 3c044d01a..716633519 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -465,7 +465,6 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void static void timeout_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received timeout signal for DHCP relay health check"); - std::unique_lock counter_lock(packet_handler_quiesce_mutex); bool subscribers_available = true; if (config_subscribers_failed) { @@ -497,6 +496,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) } } + std::unique_lock counter_lock(packet_handler_quiesce_mutex); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_SNAPSHOT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT_V6); From cdd77fc2f3a43d3ed72a0137de34773137d0853d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:37:21 +1000 Subject: [PATCH 18/27] [dhcpmon]: Guarantee counter writer progress Announce pending exclusive counter-state operations before locking so packet callbacks stop admitting shared readers and cannot starve health, DB, clear, or topology writers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 10 +++++----- src/packet_handler.cpp | 6 ++++-- src/sock_mgr.cpp | 18 ++++++++++++++++++ src/sock_mgr.h | 14 ++++++++++++++ 4 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 716633519..804cadcaf 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -282,7 +282,7 @@ static void signal_callback(evutil_socket_t fd, short event, void *arg) syslog(LOG_INFO, "Received signal: %s", strsignal(fd)); { - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT); dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT_V6); } @@ -294,7 +294,7 @@ static void signal_callback(evutil_socket_t fd, short event, void *arg) if (fd == SIGUSR1) { // we need to sync cache counter from COUNTERS_DB syslog(LOG_INFO, "Received signal to stop writing to DB counter"); - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; std::lock_guard lock(db_sync_mutex); sock_mgr_pause_write_cache_to_db(); syslog(LOG_INFO, "Stopped writing to DB counter"); @@ -332,7 +332,7 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void syslog(LOG_INFO, "Start updating %s cache counter from DB counter", sock_info.name); - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; std::lock_guard lock(db_sync_mutex); // can only sync db to cache counter and db updater is paused, otherwise its unexpected @@ -496,7 +496,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) } } - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_SNAPSHOT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT_V6); @@ -522,7 +522,7 @@ static void db_update_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received db update signal"); syslog_debug(LOG_INFO, "Sync cache counter to DB counter"); - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; std::lock_guard lock(db_sync_mutex); // If there is clear counter going on and its been longer than expected // consider the clear counter operation failed so we don't block db update forever diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 742b388fd..5cbd12363 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -862,13 +862,15 @@ void packet_handler_v6(int sock, const std::string &ifname, const dhcp_device_co void callback_common(int fd, short event, void *arg) { - if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + if (!packet_handlers_enabled.load(std::memory_order_acquire) || + counter_state_writers_pending.load(std::memory_order_acquire) > 0) { return; } std::shared_lock packet_handler_lock(packet_handler_quiesce_mutex, std::try_to_lock); if (!packet_handler_lock.owns_lock() || - !packet_handlers_enabled.load(std::memory_order_acquire)) { + !packet_handlers_enabled.load(std::memory_order_acquire) || + counter_state_writers_pending.load(std::memory_order_acquire) > 0) { return; } ssize_t buffer_sz; diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 1d4d7a7ae..ae04474ac 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -48,12 +48,30 @@ std::unordered_map sock_map; std::shared_mutex packet_handler_quiesce_mutex; std::atomic packet_handlers_enabled{true}; +std::atomic counter_state_writers_pending{0}; static std::unique_lock packet_handler_quiesce_lock; extern std::shared_ptr mCountersDbPtr; extern std::string downstream_ifname; +counter_state_write_lock::counter_state_write_lock() +{ + counter_state_writers_pending.fetch_add(1, std::memory_order_acq_rel); + try { + lock = std::unique_lock(packet_handler_quiesce_mutex); + } catch (...) { + counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel); + throw; + } +} + +counter_state_write_lock::~counter_state_write_lock() +{ + lock.unlock(); + counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel); +} + /** * @code opensocket(); * diff --git a/src/sock_mgr.h b/src/sock_mgr.h index 541535d87..1e3b22b06 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -10,6 +10,7 @@ #define SOCKET_MANAGER_H_ #include +#include #include #include #include @@ -47,6 +48,19 @@ extern int rx_sock, tx_sock, rx_sock_v6, tx_sock_v6; /** Guards in-flight packet callbacks while topology and counters are reconciled */ extern std::shared_mutex packet_handler_quiesce_mutex; extern std::atomic packet_handlers_enabled; +extern std::atomic counter_state_writers_pending; + +class counter_state_write_lock +{ + public: + counter_state_write_lock(); + ~counter_state_write_lock(); + counter_state_write_lock(const counter_state_write_lock &) = delete; + counter_state_write_lock &operator=(const counter_state_write_lock &) = delete; + + private: + std::unique_lock lock; +}; /** Initialize socket manager with given snaplen */ int sock_mgr_init(uint32_t snaplen); From f90475fcd0d3b5685e7b9485db286795a2cfacea Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:55:31 +1000 Subject: [PATCH 19/27] [dhcpmon]: Block packet callbacks without spinning Use a condition-backed reader gate for pending counter writers, make topology quiesce transitions explicit and recoverable, and surface counter-lock failures without unwinding through libevent callbacks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 28 +++++++-- src/packet_handler.cpp | 12 +--- src/sock_mgr.cpp | 130 +++++++++++++++++++++++++++++++++++------ src/sock_mgr.h | 13 ++++- 4 files changed, 151 insertions(+), 32 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 804cadcaf..2cbbb43d1 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -283,8 +283,10 @@ static void signal_callback(evutil_socket_t fd, short event, void *arg) { counter_state_write_lock counter_lock; - dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT); - dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT_V6); + if (counter_lock.owns_lock()) { + dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT); + dhcp_devman_print_all_status(DHCP_COUNTERS_CURRENT_V6); + } } if ((fd == SIGTERM) || (fd == SIGINT)) { @@ -295,6 +297,9 @@ static void signal_callback(evutil_socket_t fd, short event, void *arg) // we need to sync cache counter from COUNTERS_DB syslog(LOG_INFO, "Received signal to stop writing to DB counter"); counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { + return; + } std::lock_guard lock(db_sync_mutex); sock_mgr_pause_write_cache_to_db(); syslog(LOG_INFO, "Stopped writing to DB counter"); @@ -333,6 +338,9 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void syslog(LOG_INFO, "Start updating %s cache counter from DB counter", sock_info.name); counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { + return; + } std::lock_guard lock(db_sync_mutex); // can only sync db to cache counter and db updater is paused, otherwise its unexpected @@ -477,7 +485,11 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) } if (topology_refresh_pending && subscribers_available) { - sock_mgr_suspend_packet_handler(); + if (sock_mgr_suspend_packet_handler() < 0) { + syslog(LOG_ALERT, "Failed to suspend packet handlers for topology refresh"); + dhcp_mon_stop(); + return; + } int result = dhcp_mon_reconcile_topology(); if (sock_mgr_resume_packet_handler() < 0) { syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); @@ -497,6 +509,9 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) } counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { + return; + } dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_SNAPSHOT); dhcp_devman_print_all_status_debug(DHCP_COUNTERS_CURRENT_V6); @@ -523,6 +538,9 @@ static void db_update_callback(evutil_socket_t fd, short event, void *arg) syslog_debug(LOG_INFO, "Received db update signal"); syslog_debug(LOG_INFO, "Sync cache counter to DB counter"); counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { + return; + } std::lock_guard lock(db_sync_mutex); // If there is clear counter going on and its been longer than expected // consider the clear counter operation failed so we don't block db update forever @@ -904,7 +922,9 @@ int dhcp_mon_start() } topology_refresh_pending = true; - sock_mgr_suspend_packet_handler(); + if (sock_mgr_suspend_packet_handler() < 0) { + goto unregister_main_events; + } reconcile_result = dhcp_mon_reconcile_topology(); resume_result = sock_mgr_resume_packet_handler(); if (reconcile_result != 0 || resume_result < 0) { diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 5cbd12363..aa564c1a0 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include "packet_handler.h" @@ -862,15 +861,8 @@ void packet_handler_v6(int sock, const std::string &ifname, const dhcp_device_co void callback_common(int fd, short event, void *arg) { - if (!packet_handlers_enabled.load(std::memory_order_acquire) || - counter_state_writers_pending.load(std::memory_order_acquire) > 0) { - return; - } - std::shared_lock packet_handler_lock(packet_handler_quiesce_mutex, - std::try_to_lock); - if (!packet_handler_lock.owns_lock() || - !packet_handlers_enabled.load(std::memory_order_acquire) || - counter_state_writers_pending.load(std::memory_order_acquire) > 0) { + counter_state_read_lock counter_lock; + if (!counter_lock.owns_lock()) { return; } ssize_t buffer_sz; diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index ae04474ac..e5ea3f69c 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -11,7 +11,9 @@ #include #include #include +#include #include +#include #include #include "sock_mgr.h" @@ -50,26 +52,97 @@ std::shared_mutex packet_handler_quiesce_mutex; std::atomic packet_handlers_enabled{true}; std::atomic counter_state_writers_pending{0}; static std::unique_lock packet_handler_quiesce_lock; +static std::mutex counter_state_wait_mutex; +static std::condition_variable counter_state_wait_cv; extern std::shared_ptr mCountersDbPtr; extern std::string downstream_ifname; +static void set_packet_handlers_enabled(bool enabled) +{ + { + std::lock_guard wait_lock(counter_state_wait_mutex); + packet_handlers_enabled.store(enabled, std::memory_order_release); + } + counter_state_wait_cv.notify_all(); +} + counter_state_write_lock::counter_state_write_lock() { - counter_state_writers_pending.fetch_add(1, std::memory_order_acq_rel); + { + std::lock_guard wait_lock(counter_state_wait_mutex); + counter_state_writers_pending.fetch_add(1, std::memory_order_acq_rel); + } try { lock = std::unique_lock(packet_handler_quiesce_mutex); - } catch (...) { - counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel); - throw; + } catch (const std::system_error &e) { + bool notify = false; + { + std::lock_guard wait_lock(counter_state_wait_mutex); + notify = counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel) == 1; + } + if (notify) { + counter_state_wait_cv.notify_all(); + } + syslog(LOG_ALERT, "Failed to lock DHCP counter state: %s", e.what()); } } counter_state_write_lock::~counter_state_write_lock() { + if (!lock.owns_lock()) { + return; + } lock.unlock(); - counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel); + bool notify = false; + { + std::lock_guard wait_lock(counter_state_wait_mutex); + notify = counter_state_writers_pending.fetch_sub(1, std::memory_order_acq_rel) == 1; + } + if (notify) { + counter_state_wait_cv.notify_all(); + } +} + +bool counter_state_write_lock::owns_lock() const +{ + return lock.owns_lock(); +} + +counter_state_read_lock::counter_state_read_lock() +{ + while (packet_handlers_enabled.load(std::memory_order_acquire)) { + { + std::unique_lock wait_lock(counter_state_wait_mutex); + counter_state_wait_cv.wait(wait_lock, [] { + return !packet_handlers_enabled.load(std::memory_order_acquire) || + counter_state_writers_pending.load(std::memory_order_acquire) == 0; + }); + } + if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + return; + } + try { + lock = std::shared_lock(packet_handler_quiesce_mutex); + } catch (const std::system_error &e) { + syslog(LOG_ALERT, "Failed to lock DHCP counter state for packet handling: %s", e.what()); + return; + } + if (!packet_handlers_enabled.load(std::memory_order_acquire)) { + lock.unlock(); + return; + } + if (counter_state_writers_pending.load(std::memory_order_acquire) == 0) { + return; + } + lock.unlock(); + } +} + +bool counter_state_read_lock::owns_lock() const +{ + return lock.owns_lock(); } /** @@ -472,17 +545,37 @@ void sock_mgr_unregister_packet_handler() } } -void sock_mgr_suspend_packet_handler() +int sock_mgr_suspend_packet_handler() { if (packet_handler_quiesce_lock.owns_lock()) { syslog(LOG_ALERT, "Packet handlers are already suspended"); - return; + return -1; } - packet_handlers_enabled.store(false, std::memory_order_release); for (const auto &entry : sock_map) { entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); } - packet_handler_quiesce_lock = std::unique_lock(packet_handler_quiesce_mutex); + set_packet_handlers_enabled(false); + try { + packet_handler_quiesce_lock = std::unique_lock(packet_handler_quiesce_mutex); + } catch (const std::system_error &e) { + syslog(LOG_ALERT, "Failed to quiesce packet handlers: %s", e.what()); + set_packet_handlers_enabled(true); + int restore_result = 0; + for (const auto &entry : sock_map) { + if (entry.second.event_mgr_ptr->resume_all_events(packet_handler_tag) < 0) { + restore_result = -1; + } + } + if (restore_result < 0) { + set_packet_handlers_enabled(false); + for (const auto &entry : sock_map) { + entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); + } + syslog(LOG_ALERT, "Failed to restore packet handlers after quiesce failure"); + } + return -1; + } + return 0; } int sock_mgr_resume_packet_handler() @@ -491,21 +584,24 @@ int sock_mgr_resume_packet_handler() syslog(LOG_ALERT, "Packet handlers are not suspended"); return -1; } - int result = 0; + set_packet_handlers_enabled(true); + packet_handler_quiesce_lock.unlock(); + for (const auto &entry : sock_map) { if (entry.second.event_mgr_ptr->resume_all_events(packet_handler_tag) < 0) { + set_packet_handlers_enabled(false); for (const auto &suspended_entry : sock_map) { suspended_entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); } - result = -1; - break; + try { + packet_handler_quiesce_lock = std::unique_lock(packet_handler_quiesce_mutex); + } catch (const std::system_error &e) { + syslog(LOG_ALERT, "Failed to restore packet quiesce lock after resume failure: %s", e.what()); + } + return -1; } } - if (result == 0) { - packet_handlers_enabled.store(true, std::memory_order_release); - } - packet_handler_quiesce_lock.unlock(); - return result; + return 0; } int sock_mgr_register_cache_counter_updater(event_callback_fn callback) diff --git a/src/sock_mgr.h b/src/sock_mgr.h index 1e3b22b06..ab294bacd 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -55,6 +55,7 @@ class counter_state_write_lock public: counter_state_write_lock(); ~counter_state_write_lock(); + bool owns_lock() const; counter_state_write_lock(const counter_state_write_lock &) = delete; counter_state_write_lock &operator=(const counter_state_write_lock &) = delete; @@ -62,6 +63,16 @@ class counter_state_write_lock std::unique_lock lock; }; +class counter_state_read_lock +{ + public: + counter_state_read_lock(); + bool owns_lock() const; + + private: + std::shared_lock lock; +}; + /** Initialize socket manager with given snaplen */ int sock_mgr_init(uint32_t snaplen); @@ -81,7 +92,7 @@ int sock_mgr_register_packet_handler(); void sock_mgr_unregister_packet_handler(); /** Temporarily suspend registered packet handlers */ -void sock_mgr_suspend_packet_handler(); +int sock_mgr_suspend_packet_handler(); /** Resume registered packet handlers */ int sock_mgr_resume_packet_handler(); From 851ec793b3182f9402eb1e0d230c01fa9bf6d8d6 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:26:04 +1000 Subject: [PATCH 20/27] [dhcpmon]: Harden topology refresh retries Reject unquiesced reconciliation, keep health and snapshot processing active after a rolled-back refresh failure, and keep the CONFIG_DB callback warning-free. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 2cbbb43d1..189b67eb6 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -67,7 +67,7 @@ std::shared_ptr mStateDbMuxTablePtr = std::make_shared mStateDbPtr.get(), "HW_MUX_CABLE_TABLE" ); -static void config_update_callback(evutil_socket_t fd, short event, void *arg) +static void config_update_callback(evutil_socket_t, short, void *arg) { auto *subscriber = static_cast(arg); try { @@ -496,13 +496,8 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) dhcp_mon_stop(); return; } - if (result == 1) { - topology_refresh_pending = true; - } else { - topology_refresh_pending = result < 0; - if (result != 0) { - return; - } + topology_refresh_pending = result != 0; + if (result == 0) { syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); return; } @@ -643,6 +638,10 @@ int dhcp_mon_reconcile_topology() syslog(LOG_ALERT, "Topology reconciliation must run on the main event-loop thread"); return -1; } + if (packet_handlers_enabled.load(std::memory_order_acquire)) { + syslog(LOG_ALERT, "Topology reconciliation requires suspended packet handlers"); + return -1; + } std::lock_guard lock(db_sync_mutex); if (!sock_mgr_pause_write_cache_to_db_all_cleared()) { From 70c0f3934744744bc49be793fcdf7b4c715c1916 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:21:22 +1000 Subject: [PATCH 21/27] [dhcpmon]: Keep packet callback fast path lock-free Try the shared counter lock directly when no writer is pending, using the condition-variable wait path only when quiescing or writer contention requires it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/sock_mgr.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index e5ea3f69c..5f38ee73d 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -112,6 +112,25 @@ bool counter_state_write_lock::owns_lock() const counter_state_read_lock::counter_state_read_lock() { + if (packet_handlers_enabled.load(std::memory_order_acquire) && + counter_state_writers_pending.load(std::memory_order_acquire) == 0) { + try { + lock = std::shared_lock(packet_handler_quiesce_mutex, + std::try_to_lock); + } catch (const std::system_error &e) { + syslog(LOG_ALERT, "Failed to lock DHCP counter state for packet handling: %s", e.what()); + return; + } + if (lock.owns_lock() && + packet_handlers_enabled.load(std::memory_order_acquire) && + counter_state_writers_pending.load(std::memory_order_acquire) == 0) { + return; + } + if (lock.owns_lock()) { + lock.unlock(); + } + } + while (packet_handlers_enabled.load(std::memory_order_acquire)) { { std::unique_lock wait_lock(counter_state_wait_mutex); From 1213defffd6143baa2613db92780f9dfe3950906 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:22:26 +1000 Subject: [PATCH 22/27] [dhcpmon]: Keep topology reconciliation internal Limit the quiesced main-thread reconciliation helper to dhcp_mon.cpp so external callers cannot bypass its synchronization contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 3 ++- src/dhcp_mon.h | 10 ---------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 189b67eb6..1b21492c8 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -59,6 +59,7 @@ static bool config_subscribers_failed = false; static std::shared_ptr vlan_member_subscriber; static std::shared_ptr portchannel_member_subscriber; static const char config_event_tag[] = "CONFIG_UPDATE"; +static int dhcp_mon_reconcile_topology(); std::shared_ptr mConfigDbPtr = std::make_shared ("CONFIG_DB", 0); std::shared_ptr mCountersDbPtr = std::make_shared ("COUNTERS_DB", 0); @@ -632,7 +633,7 @@ static void reconcile_all_intf_counters(bool initialize_db) } } -int dhcp_mon_reconcile_topology() +static int dhcp_mon_reconcile_topology() { if (std::this_thread::get_id() != main_thread_id) { syslog(LOG_ALERT, "Topology reconciliation must run on the main event-loop thread"); diff --git a/src/dhcp_mon.h b/src/dhcp_mon.h index 17bf93693..5a8671b8f 100644 --- a/src/dhcp_mon.h +++ b/src/dhcp_mon.h @@ -26,16 +26,6 @@ extern bool debug_on; */ int dhcp_mon_init(size_t snaplen, int window_sec, int max_count, int db_update_interval); -/** - * @code dhcp_mon_reconcile_topology(); - * - * @brief rebuild interface membership and reconcile counters after packet processing is quiesced. - * Must be called from the main event-loop thread. - * - * @return 0 on success, 1 when counter clear is active, otherwise negative - */ -int dhcp_mon_reconcile_topology(); - /** * @code dhcp_mon_free(); * From b8520a295ad1762e948502cd6b8524ee62ae1e59 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:25:11 +1000 Subject: [PATCH 23/27] [dhcpmon]: Avoid unused socket bindings Iterate through counter-map entries directly where only sock_info is needed, keeping the reconciliation build warning-free. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/sock_mgr.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 5f38ee73d..1c22a0ff0 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -796,7 +796,8 @@ void sock_mgr_init_cache_counters(const std::string &ifname, uint8_t dhcp_messag bool sock_mgr_all_cache_counters_initialized(const std::string &ifname) { - for (const auto &[sock, info] : sock_map) { + for (const auto &entry : sock_map) { + const auto &info = entry.second; auto itr = info.all_counters.find(ifname); if (itr == info.all_counters.end()) { return false; @@ -807,7 +808,8 @@ bool sock_mgr_all_cache_counters_initialized(const std::string &ifname) void sock_mgr_remove_cache_counters_except(const std::unordered_set &valid_ifnames) { - for (auto &[sock, info] : sock_map) { + for (auto &entry : sock_map) { + auto &info = entry.second; for (auto itr = info.all_counters.begin(); itr != info.all_counters.end();) { if (valid_ifnames.find(itr->first) == valid_ifnames.end()) { info.all_counters_snapshot.erase(itr->first); From c2422007508f9850e8480a30e8ab515c2af81e3c Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:52:11 +1000 Subject: [PATCH 24/27] [dhcpmon]: Write DB counters from a stable snapshot Copy counter maps while holding counter-state and DB synchronization locks, then release packet callbacks before Redis I/O while retaining DB serialization through writeback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 37 +++++++++++++++++++++---------------- src/sock_mgr.cpp | 21 ++++++++++++++++++--- src/sock_mgr.h | 5 +++++ 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 1b21492c8..fbbf1427f 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -533,26 +533,31 @@ static void db_update_callback(evutil_socket_t fd, short event, void *arg) { syslog_debug(LOG_INFO, "Received db update signal"); syslog_debug(LOG_INFO, "Sync cache counter to DB counter"); - counter_state_write_lock counter_lock; - if (!counter_lock.owns_lock()) { - return; - } - std::lock_guard lock(db_sync_mutex); - // If there is clear counter going on and its been longer than expected - // consider the clear counter operation failed so we don't block db update forever - if (!sock_mgr_pause_write_cache_to_db_all_cleared() && last_update_time != default_time_point) { - auto now = std::chrono::steady_clock::now(); - auto elapsed = std::chrono::duration_cast(now - last_update_time); - if (elapsed.count() >= clear_counter_timeout) { - syslog(LOG_WARNING, "Clear counter going on for too long, abort clear counter"); - sock_mgr_clear_pause_write_cache_to_db(); - } else { - syslog(LOG_INFO, "Clear counter is ongoing, skip syncing write cache counter to DB counter"); + socket_counters_t counters_by_socket; + std::unique_lock lock; + { + counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { return; } + lock = std::unique_lock(db_sync_mutex); + // If there is clear counter going on and its been longer than expected + // consider the clear counter operation failed so we don't block db update forever + if (!sock_mgr_pause_write_cache_to_db_all_cleared() && last_update_time != default_time_point) { + auto now = std::chrono::steady_clock::now(); + auto elapsed = std::chrono::duration_cast(now - last_update_time); + if (elapsed.count() >= clear_counter_timeout) { + syslog(LOG_WARNING, "Clear counter going on for too long, abort clear counter"); + sock_mgr_clear_pause_write_cache_to_db(); + } else { + syslog(LOG_INFO, "Clear counter is ongoing, skip syncing write cache counter to DB counter"); + return; + } + } + counters_by_socket = sock_mgr_copy_cache_counters(); } last_update_time = std::chrono::steady_clock::now(); - sock_mgr_update_db_counters(); + sock_mgr_update_db_counters(counters_by_socket); cleanup_stale_db_counters(); syslog_debug(LOG_INFO, "Successfully synced cache counter to DB counter"); } diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 1c22a0ff0..cee428df4 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -821,17 +821,27 @@ void sock_mgr_remove_cache_counters_except(const std::unordered_set } } -void sock_mgr_update_db_counters() +socket_counters_t sock_mgr_copy_cache_counters() +{ + socket_counters_t counters_by_socket; + for (const auto &[sock, info] : sock_map) { + counters_by_socket.emplace(sock, info.all_counters); + } + return counters_by_socket; +} + +void sock_mgr_update_db_counters(const socket_counters_t &counters_by_socket) { syslog_debug(LOG_INFO, "Updating all cache counters to DB counters"); - for (const auto &[sock, info] : sock_map) { + for (const auto &[sock, all_counters] : counters_by_socket) { + const sock_info_t &info = sock_mgr_get_sock_info(sock); syslog_debug(LOG_INFO, "Start updating socket %d %s DB counter from cache counter", sock, info.name); int msg_type_count = info.is_v6 ? DHCPV6_MESSAGE_TYPE_COUNT : DHCP_MESSAGE_TYPE_COUNT; const std::string *msg_type_name = info.is_v6 ? db_counter_name_v6 : db_counter_name; std::string all_ifname; std::string all_skipped_ifname; - for (const auto &[ifname, counter] : info.all_counters) { + for (const auto &[ifname, counter] : all_counters) { if (is_agg_counter(ifname) == true) { all_skipped_ifname += ifname + ", "; continue; @@ -846,4 +856,9 @@ void sock_mgr_update_db_counters() syslog_debug(LOG_INFO, "Skipped aggregated device counter entry of %sfor downstream vlan %s", all_skipped_ifname.c_str(), downstream_ifname.c_str()); } +} + +void sock_mgr_update_db_counters() +{ + sock_mgr_update_db_counters(sock_mgr_copy_cache_counters()); } \ No newline at end of file diff --git a/src/sock_mgr.h b/src/sock_mgr.h index ab294bacd..61908c415 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -23,6 +23,7 @@ typedef std::unordered_map counter_t; typedef std::unordered_map all_counters_t; +typedef std::unordered_map socket_counters_t; /** struct for socket information */ typedef struct { @@ -147,5 +148,9 @@ void sock_mgr_remove_cache_counters_except(const std::unordered_set /** Update database counters from cache counters for all sockets */ void sock_mgr_update_db_counters(); +void sock_mgr_update_db_counters(const socket_counters_t &counters_by_socket); + +/** Copy cache counters for all sockets */ +socket_counters_t sock_mgr_copy_cache_counters(); #endif /* SOCKET_MANAGER_H_ */ From 0ce04819841fdd0490ce6f723cc300aefc89b03c Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:16:04 +1000 Subject: [PATCH 25/27] [dhcpmon]: Drop stale monitor packets after refresh Drain raw socket receive buffers while packet handlers remain suspended after a successful topology commit, preventing pre-change packets from being attributed with new mappings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_mon.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index fbbf1427f..40a0139b7 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -492,6 +492,9 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) return; } int result = dhcp_mon_reconcile_topology(); + if (result == 0) { + sock_mgr_drain_sock_buffer(); + } if (sock_mgr_resume_packet_handler() < 0) { syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); dhcp_mon_stop(); From 23fff73de4c0a01d8d8279b88070f0d6f647ebc5 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:14:15 +1000 Subject: [PATCH 26/27] [dhcpmon]: Skip unknown counter snapshot sockets Validate snapshot socket keys before looking up metadata so malformed or stale caller data cannot throw from sock_map.at(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/sock_mgr.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index cee428df4..e6acd7624 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -835,7 +835,12 @@ void sock_mgr_update_db_counters(const socket_counters_t &counters_by_socket) syslog_debug(LOG_INFO, "Updating all cache counters to DB counters"); for (const auto &[sock, all_counters] : counters_by_socket) { - const sock_info_t &info = sock_mgr_get_sock_info(sock); + const auto sock_info = sock_map.find(sock); + if (sock_info == sock_map.end()) { + syslog(LOG_WARNING, "Skip DB counter snapshot for unknown socket %d", sock); + continue; + } + const sock_info_t &info = sock_info->second; syslog_debug(LOG_INFO, "Start updating socket %d %s DB counter from cache counter", sock, info.name); int msg_type_count = info.is_v6 ? DHCPV6_MESSAGE_TYPE_COUNT : DHCP_MESSAGE_TYPE_COUNT; const std::string *msg_type_name = info.is_v6 ? db_counter_name_v6 : db_counter_name; From a4d4e77bbd7af8afc0a9756aeb56d4d0488150b0 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:15:15 +1000 Subject: [PATCH 27/27] [dhcpmon]: Prune stale snapshot-only counters Reconcile current and snapshot counter maps independently so operator[]-created snapshot entries cannot survive after an interface leaves the tracked topology. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/sock_mgr.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index e6acd7624..53f54b019 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -812,12 +812,18 @@ void sock_mgr_remove_cache_counters_except(const std::unordered_set auto &info = entry.second; for (auto itr = info.all_counters.begin(); itr != info.all_counters.end();) { if (valid_ifnames.find(itr->first) == valid_ifnames.end()) { - info.all_counters_snapshot.erase(itr->first); itr = info.all_counters.erase(itr); } else { itr++; } } + for (auto itr = info.all_counters_snapshot.begin(); itr != info.all_counters_snapshot.end();) { + if (valid_ifnames.find(itr->first) == valid_ifnames.end()) { + itr = info.all_counters_snapshot.erase(itr); + } else { + itr++; + } + } } }