From 4cd6786e4c598643857b51e0b739ac7f72ce8003 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 16:51:59 +0000 Subject: [PATCH 01/42] [dhcpmon]: Track PortChannels under downstream VLANs Resolve nested Ethernet-to-PortChannel-to-VLAN membership and aggregate counters at each immediate parent without changing existing Dual-ToR attribution. 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 | 32 +++++++++++++++++++++++++++----- src/dhcp_devman.h | 22 ++++++++++++++++++++++ src/dhcp_mon.cpp | 2 +- src/packet_handler.cpp | 4 +--- src/util.h | 12 ------------ 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 9fa951e3c..75308faed 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -248,7 +248,10 @@ static void update_portchannel_mapping() auto second = key.find_last_of('|'); auto portchannel = key.substr(first + 1, second - first - 1); auto ifname = key.substr(second + 1); - if (intfs.find(portchannel) == intfs.end()) { + bool portchannel_is_context = intfs.find(portchannel) != intfs.end(); + bool portchannel_is_vlan_member = vlan_map.find(portchannel) != vlan_map.end(); + // Dual-ToR downlink counters require MUX attribution that is unavailable on a nested PortChannel. + if (!portchannel_is_context && (!portchannel_is_vlan_member || dual_tor_mode)) { all_skipped_ifname += "<" + ifname + ", " + portchannel + ">, "; continue; } @@ -296,7 +299,7 @@ int dhcp_devman_init() agg_dev_all = "Agg-" + downstream_ifname; agg_dev_prefix = agg_dev_all + "-"; - // vlan and its members, portchannel and its members are initialized regardless of whether they are in cmdline + // PortChannel members depend on VLAN mappings to recognize a PortChannel under a monitored VLAN. update_vlan_mapping(); update_portchannel_mapping(); @@ -321,15 +324,34 @@ const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &i if (iter != intfs.end()) { return iter->second; } + const auto port_channel = portchan_map.find(ifname); + if (port_channel != portchan_map.end() && ifname != port_channel->second) { + return dhcp_devman_get_device_context(port_channel->second); + } const auto vlan = vlan_map.find(ifname); if (vlan != vlan_map.end() && ifname != vlan->second) { return dhcp_devman_get_device_context(vlan->second); } + return NULL; +} + +std::string dhcp_devman_get_parent_ifname(const std::string &ifname) +{ const auto port_channel = portchan_map.find(ifname); - if (port_channel != portchan_map.end() && ifname != port_channel->second) { - return dhcp_devman_get_device_context(port_channel->second); + if (port_channel != portchan_map.end()) { + return port_channel->second; } - return NULL; + const auto vlan = vlan_map.find(ifname); + if (vlan != vlan_map.end()) { + return vlan->second; + } + return ""; +} + +std::string dhcp_devman_get_agg_counter_ifname(const std::string &ifname) +{ + const std::string parent_ifname = dhcp_devman_get_parent_ifname(ifname); + return parent_ifname.empty() ? agg_dev_all : agg_dev_prefix + parent_ifname; } void dhcp_devman_print_all_status(dhcp_counters_type_t type) diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index 73c1cd6f2..473e4dbc7 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -138,6 +138,28 @@ void dhcp_devman_free(); */ const dhcp_device_context_t* dhcp_devman_get_device_context(const std::string &ifname); +/** + * @code dhcp_devman_get_parent_ifname(ifname); + * + * @brief find the immediate parent interface of a tracked interface. + * + * @param ifname interface name + * + * @return parent interface name, or an empty string for a root context interface + */ +std::string dhcp_devman_get_parent_ifname(const std::string &ifname); + +/** + * @code dhcp_devman_get_agg_counter_ifname(ifname); + * + * @brief find the aggregate counter for the immediate parent of a tracked interface. + * + * @param ifname interface name + * + * @return aggregate counter name + */ +std::string dhcp_devman_get_agg_counter_ifname(const std::string &ifname); + /** * @code dhcp_devman_print_all_status(type); * diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index ef4f6623d..77ec1406f 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -81,7 +81,7 @@ static void recalculate_agg_counter(all_counters_t &all_counters) if (mgmt_ifname == context->intf) { continue; } - counter_t &agg_counter = all_counters.at(get_agg_counter_ifname(ifname, context->intf)); + counter_t &agg_counter = all_counters.at(dhcp_devman_get_agg_counter_ifname(ifname)); for (const auto &[msg_type, count] : counter) { agg_counter[msg_type] += count; } diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 7ec5d07a3..f4ee536f6 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -52,9 +52,7 @@ static void increase_cache_counter(const std::string &ifname, const dhcp_device_ return; } - // when ifname belongs to another context ifname, increase the aggregate counter for that context, - // else when ifname is the context, we increase agg counter for all. - _increase_cache_counter(get_agg_counter_ifname(ifname, context->intf), sock, type); + _increase_cache_counter(dhcp_devman_get_agg_counter_ifname(ifname), sock, type); // optionally duplicate to context ifname, it will only be true when this is standby physical interface under a vlan on a dual tor if (dup_to_context) { diff --git a/src/util.h b/src/util.h index f4fbd24c3..d5a59763d 100644 --- a/src/util.h +++ b/src/util.h @@ -216,18 +216,6 @@ inline bool is_agg_counter(const std::string &ifname) return ifname.compare(0, agg_dev_prefix.size(), agg_dev_prefix) == 0 || ifname == agg_dev_all; } -/** - * @code get_agg_counter_ifname(ifname, context); - * @brief Get aggregate counter name for given ifname and device context - * @param ifname Interface name - * @param context Pointer to device context - * @return Aggregate counter name - */ -inline std::string get_agg_counter_ifname(const std::string &ifname, const std::string &context_ifname) -{ - return ifname != context_ifname ? agg_dev_prefix + context_ifname : agg_dev_all; -} - /** * @code contains_value(v, value); * @brief Check if a vector contains a specific value From db1cfa78a85d88f4b75f6f6e4999103b76b0866a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 22:49:08 +0000 Subject: [PATCH 02/42] [dhcpmon]: Make relay disparity detection resilient Track DHCPv4 disparity per message type with absolute watermarks, adjacent-window TX credit, main-loop clear reset, and one report per persistent outage; disable the invalid DHCPv6 same-type signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 117 +++++++++++++++++++++++++++++++++++++------ src/dhcp_device.h | 23 +++++++++ src/dhcp_mon.cpp | 8 +++ src/health_check.cpp | 76 +++++++++++++++++++--------- src/health_check.h | 4 ++ 5 files changed, 189 insertions(+), 39 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 8b1ed4f6c..985c31a1d 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -69,6 +70,39 @@ static const char *counter_desc[DHCP_COUNTERS_COUNT] = { [DHCP_COUNTERS_SNAPSHOT_V6] = "Snapshot_V6", }; +typedef struct +{ + uint64_t last_rx; + uint64_t last_tx; + uint32_t pending_windows; + uint8_t tx_credit; + bool initialized; +} relay_flow_state_t; + +static std::unordered_map>> relay_flow_states; +static std::mutex relay_flow_state_mutex; + +static void initialize_relay_flow_states(const std::string &ifname, int rx_sock, int tx_sock, + const int *monitored_msgs, size_t monitored_msg_cnt) +{ + const counter_t &rx_counters = sock_mgr_get_sock_info(rx_sock).all_counters.at(ifname); + const counter_t &tx_counters = sock_mgr_get_sock_info(tx_sock).all_counters.at(ifname); + for (size_t i = 0; i < monitored_msg_cnt; i++) { + int msg_type = monitored_msgs[i]; + relay_flow_states[rx_sock][ifname][msg_type] = { + rx_counters.at(msg_type), tx_counters.at(msg_type), 0, 0, true + }; + } +} + +void dhcp_device_reset_health_state(const std::string &ifname) +{ + std::lock_guard lock(relay_flow_state_mutex); + relay_flow_states.clear(); + initialize_relay_flow_states(ifname, rx_sock, tx_sock, + (const int *)monitored_msgs, monitored_msg_sz); +} + /** * @code check_counter_not_transmitted(ifname, rx_sock, tx_sock, monitored_msgs, monitored_msg_cnt); * @brief Check if there are received DHCP messages that are not transmitted out @@ -78,28 +112,75 @@ static const char *counter_desc[DHCP_COUNTERS_COUNT] = { * @param tx_sock tx socket * @param monitored_msgs array of monitored message types * @param monitored_msg_cnt number of monitored message types - * @return true if there are received messages not transmitted out, false otherwise + * @return DHCP relay health status */ // these helpers use const int * to accept both dhcp_message_type_t and dhcpv6_message_type_t arrays // without duplicating the function for each enum type; safe on GCC/Linux where unscoped enums use int -static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock, int tx_sock, const int *monitored_msgs, size_t monitored_msg_cnt) +static std::unordered_map get_untransmitted_windows(const std::string &ifname, + int rx_sock, int tx_sock, + const int *monitored_msgs, + size_t monitored_msg_cnt) { + std::lock_guard lock(relay_flow_state_mutex); const sock_info_t &rx_sock_info = sock_mgr_get_sock_info(rx_sock); const counter_t &rx_counters = rx_sock_info.all_counters.at(ifname); - const counter_t &rx_counters_snapshot = rx_sock_info.all_counters_snapshot.at(ifname); const sock_info_t &tx_sock_info = sock_mgr_get_sock_info(tx_sock); const counter_t &tx_counters = tx_sock_info.all_counters.at(ifname); - const counter_t &tx_counters_snapshot = tx_sock_info.all_counters_snapshot.at(ifname); - // when there is packet in, no packet out + std::unordered_map result; for (size_t i = 0; i < monitored_msg_cnt; i++) { - if (rx_counters.at(monitored_msgs[i]) > rx_counters_snapshot.at(monitored_msgs[i]) && - tx_counters.at(monitored_msgs[i]) <= tx_counters_snapshot.at(monitored_msgs[i])) { - return true; + int msg_type = monitored_msgs[i]; + uint64_t current_rx = rx_counters.at(msg_type); + uint64_t current_tx = tx_counters.at(msg_type); + relay_flow_state_t &state = relay_flow_states[rx_sock][ifname][msg_type]; + + if (!state.initialized || current_rx < state.last_rx || current_tx < state.last_tx) { + state = {current_rx, current_tx, 0, 0, true}; + result[msg_type] = 0; + continue; + } + + uint64_t rx_delta = current_rx - state.last_rx; + uint64_t tx_delta = current_tx - state.last_tx; + bool had_pending = state.pending_windows > 0; + bool previous_tx_credit = state.tx_credit > 0; + bool current_tx_activity = tx_delta > 0; + state.last_rx = current_rx; + state.last_tx = current_tx; + + if (had_pending) { + if (previous_tx_credit || current_tx_activity) { + state.pending_windows = 0; + state.tx_credit = previous_tx_credit && current_tx_activity ? 1 : 0; + } else { + state.pending_windows++; + state.tx_credit = 0; + } + } else if (rx_delta > 0) { + if (previous_tx_credit) { + state.pending_windows = 0; + state.tx_credit = current_tx_activity ? 1 : 0; + } else if (current_tx_activity) { + state.pending_windows = 0; + state.tx_credit = 0; + } else { + state.pending_windows = 1; + state.tx_credit = 0; + } + } else { + state.pending_windows = 0; + state.tx_credit = current_tx_activity ? 1 : 0; } + result[msg_type] = state.pending_windows; } - return false; + return result; +} + +std::unordered_map dhcp_device_get_untransmitted_windows(const std::string &ifname) +{ + return get_untransmitted_windows(ifname, rx_sock, tx_sock, + (const int *)monitored_msgs, monitored_msg_sz); } /** @@ -112,8 +193,12 @@ static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock */ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &ifname) { - return check_counter_not_transmitted(ifname, rx_sock, tx_sock, (const int *)monitored_msgs, monitored_msg_sz) ? - DHCP_MON_STATUS_UNHEALTHY : DHCP_MON_STATUS_HEALTHY; + for (const auto &[msg_type, windows] : dhcp_device_get_untransmitted_windows(ifname)) { + if (windows > 0) { + return DHCP_MON_STATUS_UNHEALTHY; + } + } + return DHCP_MON_STATUS_HEALTHY; } /** @@ -124,10 +209,10 @@ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &if * @param ifname interface name * @return DHCP_MON_STATUS_HEALTHY, DHCP_MON_STATUS_UNHEALTHY, or DHCP_MON_STATUS_INDETERMINATE */ -static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &ifname) +static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &) { - return check_counter_not_transmitted(ifname, rx_sock_v6, tx_sock_v6, (const int *)monitored_v6_msgs, monitored_v6_msg_sz) ? - DHCP_MON_STATUS_UNHEALTHY : DHCP_MON_STATUS_HEALTHY; + // Client and relay DHCPv6 message types differ across the relay boundary. + return DHCP_MON_STATUS_INDETERMINATE; } /** @@ -382,7 +467,9 @@ void dhcp_device_print_status_debug(const std::string &ifname, dhcp_counters_typ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_check_t check_type) { - if (sock_mgr_counters_unchanged(ifname, (const int *)monitored_msgs, monitored_msg_sz, (const int *)monitored_v6_msgs, monitored_v6_msg_sz)) { + if (check_type != DHCP_DEVICE_CHECK_POSITIVE && check_type != DHCP_DEVICE_CHECK_POSITIVE_V6 && + sock_mgr_counters_unchanged(ifname, (const int *)monitored_msgs, monitored_msg_sz, + (const int *)monitored_v6_msgs, monitored_v6_msg_sz)) { return DHCP_MON_STATUS_INDETERMINATE; } diff --git a/src/dhcp_device.h b/src/dhcp_device.h index 5d4b00721..62d61319a 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -18,6 +18,7 @@ #include #include #include +#include /** DHCP message types */ typedef enum @@ -242,6 +243,28 @@ void dhcp_device_free(dhcp_device_context_t *context); */ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_check_t check_type); +/** + * @code dhcp_device_get_untransmitted_windows(ifname); + * + * @brief update and return unmatched DHCPv4 relay RX age in health windows per message type. + * + * @param ifname interface name + * + * @return message type to unmatched-window count + */ +std::unordered_map dhcp_device_get_untransmitted_windows(const std::string &ifname); + +/** + * @code dhcp_device_reset_health_state(ifname); + * + * @brief reset relay health watermarks to the current counters for an interface. + * + * @param ifname interface name + * + * @return none + */ +void dhcp_device_reset_health_state(const std::string &ifname); + /** * @code dhcp_device_print_status(ifname, type); * diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 77ec1406f..749e44fed 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -41,6 +42,7 @@ static constexpr int MINIMAL_CLEAR_COUNTER_TIMEOUT_SEC = 5; static constexpr int CLEAR_COUNTER_DELAY_AFTER_DB_UPDATE_SEC = 1; /** Mutex lock to modify write_counter_to_db for different threads */ static std::mutex db_sync_mutex; +static std::atomic health_reset_pending{false}; /** tag for db_update event */ static const char db_update_tag[] = "DB_UPDATE"; /** Latest timestamp of writing cache counter to COUNTERS_DB */ @@ -373,6 +375,7 @@ static void update_cache_counter_callback(evutil_socket_t fd, short event, void // for discrepency in interface between cache counter and DB counter, we dont handle it in this function // we leave it to db updater to handle it if (sock_mgr_pause_write_cache_to_db_all_cleared()) { + health_reset_pending = true; syslog(LOG_INFO, "All sockets cleared pause_write_cache_to_db, start write back to DB counter from cache counter"); main_event_mgr->activate_all_events(db_update_tag, EV_TIMEOUT); } @@ -393,6 +396,10 @@ 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 (health_reset_pending.exchange(false)) { + reset_dhcp_relay_health_state(agg_dev_all); + } + 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); @@ -520,6 +527,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(); + reset_dhcp_relay_health_state(agg_dev_all); syslog(LOG_INFO, "Initialized all counters for tracked interfaces"); window_interval_sec = window_sec; diff --git a/src/health_check.cpp b/src/health_check.cpp index e9949d18c..04c6fc79a 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -4,6 +4,8 @@ */ #include +#include +#include #include #include #include @@ -27,10 +29,8 @@ extern std::string agg_dev_prefix; extern std::unordered_map> rev_vlan_map; extern std::unordered_map> rev_portchan_map; -static dhcp_mon_status_t check_agg_health() -{ - return dhcp_device_get_status(agg_dev_all, DHCP_DEVICE_CHECK_POSITIVE); -} +static std::unordered_set reported_disparity_v4; +static std::mutex health_state_mutex; static dhcp_mon_status_t check_mgmt_health() { @@ -59,9 +59,27 @@ static void log_mgmt_error(int duration) duration, mgmt_ifname.c_str()); } -static dhcp_mon_status_t check_agg_health_v6() +static void check_relay_disparity() { - return dhcp_device_get_status(agg_dev_all, DHCP_DEVICE_CHECK_POSITIVE_V6); + auto windows_by_type = dhcp_device_get_untransmitted_windows(agg_dev_all); + uint32_t report_windows = 0; + + for (const auto &[msg_type, windows] : windows_by_type) { + if (windows == 0) { + reported_disparity_v4.erase(msg_type); + continue; + } + if (windows > static_cast(dhcp_unhealthy_max_count) && + reported_disparity_v4.insert(msg_type).second) { + report_windows = std::max(report_windows, windows); + } + } + + if (report_windows > 0) { + int duration = static_cast(report_windows) * window_interval_sec; + alert_dhcp_relay_disparity(duration); + log_agg_error(duration); + } } static dhcp_mon_status_t check_mgmt_health_v6() @@ -148,46 +166,40 @@ static dhcp_mon_status_t check_per_interface_tx_health_v6() /** DHCP monitor state data for aggregate device for mgmt device */ static dhcp_mon_state_t state_data[] = { [0] = { - .check_health = check_agg_health, - .alert = alert_dhcp_relay_disparity, - .log = log_agg_error, - .count = 0, - }, - [1] = { .check_health = check_mgmt_health, .log = log_mgmt_error, .count = 0, + .reported = false, }, - [2] = { - .check_health = check_agg_health_v6, - .alert = alert_dhcp_relay_disparity, - .log = log_agg_error, - .count = 0, - }, - [3] = { + [1] = { .check_health = check_mgmt_health_v6, .log = log_mgmt_error, .count = 0, + .reported = false, }, - [4] = { + [2] = { .check_health = check_per_interface_rx_health, .log = log_agg_per_interface_rx_error, .count = 0, + .reported = false, }, - [5] = { + [3] = { .check_health = check_per_interface_tx_health, .log = log_agg_per_interface_tx_error, .count = 0, + .reported = false, }, - [6] = { + [4] = { .check_health = check_per_interface_rx_health_v6, .log = log_agg_per_interface_rx_error, .count = 0, + .reported = false, }, - [7] = { + [5] = { .check_health = check_per_interface_tx_health_v6, .log = log_agg_per_interface_tx_error, .count = 0, + .reported = false, }, }; @@ -195,13 +207,16 @@ static size_t state_data_sz = sizeof(state_data) / sizeof(*state_data); void check_dhcp_relay_health() { + std::lock_guard lock(health_state_mutex); syslog_debug(LOG_INFO, "Checking DHCP relay health"); + check_relay_disparity(); + for (uint8_t i = 0; i < state_data_sz; i++) { dhcp_mon_status_t dhcp_mon_status = state_data[i].check_health(); switch (dhcp_mon_status) { case DHCP_MON_STATUS_UNHEALTHY: - if (++state_data[i].count > dhcp_unhealthy_max_count) { + if (++state_data[i].count > dhcp_unhealthy_max_count && !state_data[i].reported) { int duration = state_data[i].count * window_interval_sec; if (state_data[i].alert) { @@ -210,10 +225,12 @@ void check_dhcp_relay_health() if (state_data[i].log) { state_data[i].log(duration); } + state_data[i].reported = true; } break; case DHCP_MON_STATUS_HEALTHY: state_data[i].count = 0; + state_data[i].reported = false; break; case DHCP_MON_STATUS_INDETERMINATE: if (state_data[i].count) { @@ -227,4 +244,15 @@ void check_dhcp_relay_health() } syslog_debug(LOG_INFO, "Completed DHCP relay health check"); +} + +void reset_dhcp_relay_health_state(const std::string &ifname) +{ + std::lock_guard lock(health_state_mutex); + reported_disparity_v4.clear(); + for (auto &state : state_data) { + state.count = 0; + state.reported = false; + } + dhcp_device_reset_health_state(ifname); } \ No newline at end of file diff --git a/src/health_check.h b/src/health_check.h index 552dd528c..0879c8d67 100644 --- a/src/health_check.h +++ b/src/health_check.h @@ -17,6 +17,7 @@ typedef struct void (*alert)(int duration); /** alert function when check failed */ void (*log)(int duration); /** log function when check passed */ int count; /** count in the number of unhealthy checks */ + bool reported; /** whether the current unhealthy episode was reported */ } dhcp_mon_state_t; extern event_handle_t g_events_handle; @@ -36,4 +37,7 @@ extern int dhcp_unhealthy_max_count; */ void check_dhcp_relay_health(); +/** Reset all relay health persistence, report state, and flow watermarks */ +void reset_dhcp_relay_health_state(const std::string &ifname); + #endif // HEALTH_CHECK_H \ No newline at end of file From a9de60d9f3b06e4ecde02d5cd47152f174294b42 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 19:42:52 +0000 Subject: [PATCH 03/42] [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 f4ee536f6..ab9efca4c 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 @@ -862,8 +864,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 b7ff2bd92b505578cb4f816ae3a34794fc9a05f1 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 20:03:49 +0000 Subject: [PATCH 04/42] [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 | 45 +++++++++++++---- 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, 170 insertions(+), 32 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 75308faed..e561c8b50 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,14 @@ 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, + const std::unordered_map &vlan_mapping, + 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) { @@ -249,20 +254,37 @@ static void update_portchannel_mapping() auto portchannel = key.substr(first + 1, second - first - 1); auto ifname = key.substr(second + 1); bool portchannel_is_context = intfs.find(portchannel) != intfs.end(); - bool portchannel_is_vlan_member = vlan_map.find(portchannel) != vlan_map.end(); + bool portchannel_is_vlan_member = vlan_mapping.find(portchannel) != vlan_mapping.end(); // Dual-ToR downlink counters require MUX attribution that is unavailable on a nested PortChannel. if (!portchannel_is_context && (!portchannel_is_vlan_member || dual_tor_mode)) { 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_vlan_map, 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"); @@ -300,8 +322,7 @@ int dhcp_devman_init() agg_dev_prefix = agg_dev_all + "-"; // PortChannel members depend on VLAN mappings to recognize a PortChannel under a monitored VLAN. - update_vlan_mapping(); - update_portchannel_mapping(); + dhcp_devman_refresh_mappings(); syslog(LOG_INFO, "Dhcp device manager initialized successfully"); @@ -312,6 +333,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 473e4dbc7..e15307b18 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 749e44fed..b04dfc286 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -7,7 +7,12 @@ #include #include #include +#include #include +#include +#include +#include +#include #include #include #include @@ -49,6 +54,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); @@ -466,45 +472,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); @@ -526,7 +604,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); reset_dhcp_relay_health_state(agg_dev_all); syslog(LOG_INFO, "Initialized all counters for tracked interfaces"); 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 ef934fc44..bd3fa073f 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -649,6 +649,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 736ce9b8d..a5ca52fd8 100644 --- a/src/sock_mgr.h +++ b/src/sock_mgr.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -110,6 +111,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 93a26313ce578a8fb27301e7fc9ddeb1e7696779 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 21:07:10 +0000 Subject: [PATCH 05/42] [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 b04dfc286..6f2f9e3c2 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -55,6 +56,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); @@ -63,6 +69,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); * @@ -406,6 +468,31 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) reset_dhcp_relay_health_state(agg_dev_all); } + 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); @@ -753,6 +840,10 @@ static int register_main_events() break; } + if (register_config_events() < 0) { + break; + } + rv = 0; syslog(LOG_INFO, "Main events registered successfully"); @@ -797,6 +888,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 646cdb2a3f83deb3890195ebb43c476cc1743f8c Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:04:25 +0000 Subject: [PATCH 06/42] [dhcpmon]: Fix aggregate comment typo 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index ab9efca4c..26007187b 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -49,7 +49,7 @@ static void increase_cache_counter(const std::string &ifname, const dhcp_device_ { _increase_cache_counter(ifname, sock, type); - // we seperate mgmt interface from others and do not increase agg counter + // we separate mgmt interface from others and do not increase agg counter if (mgmt_ifname != "" && mgmt_ifname.compare(context->intf) == 0) { return; } From 22e5f0be23f621eff224d6e4743a11f376005fda Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:05:22 +0000 Subject: [PATCH 07/42] [dhcpmon]: Tighten disparity state scope Reset only the requested interface, include direct container dependencies, and label the alert as DHCPv4-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 4 +++- src/health_check.cpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 985c31a1d..bfe4d920b 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include "dhcp_device.h" @@ -98,7 +100,7 @@ static void initialize_relay_flow_states(const std::string &ifname, int rx_sock, void dhcp_device_reset_health_state(const std::string &ifname) { std::lock_guard lock(relay_flow_state_mutex); - relay_flow_states.clear(); + relay_flow_states[rx_sock].erase(ifname); initialize_relay_flow_states(ifname, rx_sock, tx_sock, (const int *)monitored_msgs, monitored_msg_sz); } diff --git a/src/health_check.cpp b/src/health_check.cpp index 04c6fc79a..bf374a01d 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -48,7 +48,7 @@ static void alert_dhcp_relay_disparity(int duration) static void log_agg_error(int duration) { - syslog(LOG_ALERT, "dhcpmon detected DHCPv4/v6 packets received but none transmitted. Duration: %d (sec) for intf: %s", + syslog(LOG_ALERT, "dhcpmon detected DHCPv4 packets received but none transmitted. Duration: %d (sec) for intf: %s", duration, agg_dev_all.c_str()); } From a485c17788a76e92b6352d813acb21edc0c87658 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:06:44 +0000 Subject: [PATCH 08/42] [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 26007187b..40aee2e8c 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -865,6 +865,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 b750228c92af55f71de98b7d3e33df3fd3a0969a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:15:06 +0000 Subject: [PATCH 09/42] [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 6f2f9e3c2..4d4c8b0a9 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -468,14 +468,17 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) reset_dhcp_relay_health_state(agg_dev_all); } + 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) { @@ -483,14 +486,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 a6d754405b647537d0674f4ea3c0e03f8b11566d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 10/42] [dhcpmon]: Ignore self-referential parent mappings Treat a self-mapped VLAN or PortChannel member as a root interface instead of creating a self aggregate. 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index e561c8b50..f6390eb8c 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -361,11 +361,11 @@ const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &i std::string dhcp_devman_get_parent_ifname(const std::string &ifname) { const auto port_channel = portchan_map.find(ifname); - if (port_channel != portchan_map.end()) { + if (port_channel != portchan_map.end() && ifname != port_channel->second) { return port_channel->second; } const auto vlan = vlan_map.find(ifname); - if (vlan != vlan_map.end()) { + if (vlan != vlan_map.end() && ifname != vlan->second) { return vlan->second; } return ""; From 4ce76b163b1b77249e9577f114d8ab465f03e632 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 11/42] [dhcpmon]: Clarify relay health reset scope Include std::string directly and document the global report-state reset separately from the per-interface flow watermark reset. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/health_check.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/health_check.h b/src/health_check.h index 0879c8d67..7d4550ccb 100644 --- a/src/health_check.h +++ b/src/health_check.h @@ -8,6 +8,8 @@ #include "dhcp_device.h" +#include + #include /** DHCP device/interface state */ @@ -37,7 +39,7 @@ extern int dhcp_unhealthy_max_count; */ void check_dhcp_relay_health(); -/** Reset all relay health persistence, report state, and flow watermarks */ +/** Reset relay report state globally and flow watermarks for the given interface */ void reset_dhcp_relay_health_state(const std::string &ifname); #endif // HEALTH_CHECK_H \ No newline at end of file From 72d367a8a5a3317230cdd991ddec226ef816c959 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 12/42] [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 7aa5ef77cb652be3f0c9d6e58f55f9b8880af1cc Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:04:09 +1000 Subject: [PATCH 13/42] [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 f6390eb8c..e803e130f 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -11,6 +11,8 @@ #include #include +#include + #include "dhcp_devman.h" @@ -322,7 +324,12 @@ int dhcp_devman_init() agg_dev_prefix = agg_dev_all + "-"; // PortChannel members depend on VLAN mappings to recognize a PortChannel under a monitored VLAN. - 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 4d4c8b0a9..4d4aefd4e 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -483,6 +483,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 1ca61b37182e2999355caab275eaa019ba5c2c9b Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:05:33 +1000 Subject: [PATCH 14/42] [dhcpmon]: Reset health state after topology refresh Rebase relay-flow watermarks after successful membership reconciliation so recalculated aggregate counters cannot create a false disparity episode. 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 4d4aefd4e..46e58b42b 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -493,6 +493,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) if (result != 0) { return; } + reset_dhcp_relay_health_state(agg_dev_all); syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); return; } From 913420c9851a5dc1a834770c7a5646567ca554c3 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:09:52 +1000 Subject: [PATCH 15/42] [dhcpmon]: Preserve adjacent-window transmit credit Keep one-window TX credit whenever the current health window observes a transmit, even when that activity also clears a pending or current RX. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index bfe4d920b..8397bd981 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -154,7 +154,7 @@ static std::unordered_map get_untransmitted_windows(const std::st if (had_pending) { if (previous_tx_credit || current_tx_activity) { state.pending_windows = 0; - state.tx_credit = previous_tx_credit && current_tx_activity ? 1 : 0; + state.tx_credit = current_tx_activity ? 1 : 0; } else { state.pending_windows++; state.tx_credit = 0; @@ -165,7 +165,7 @@ static std::unordered_map get_untransmitted_windows(const std::st state.tx_credit = current_tx_activity ? 1 : 0; } else if (current_tx_activity) { state.pending_windows = 0; - state.tx_credit = 0; + state.tx_credit = 1; } else { state.pending_windows = 1; state.tx_credit = 0; From e70f72e6e01e552ab171480cd52a8a6a517a33d6 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:33:38 +1000 Subject: [PATCH 16/42] [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 c2ddb144fdf5273a3ee975add54093041b8246fa Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:43:56 +1000 Subject: [PATCH 17/42] [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 40aee2e8c..f02f31d09 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -859,6 +859,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 f78326c7597acf6dfcba4434b9b5c7abfad2248c Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:45:09 +1000 Subject: [PATCH 18/42] [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 46e58b42b..0dbe4d755 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -876,6 +876,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"); @@ -896,7 +898,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 e110b9b0a39645ac3791e9ba2a5e47e3d87e3892 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:02:47 +1000 Subject: [PATCH 19/42] [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 0dbe4d755..faa884982 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -632,14 +632,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 83e3335316c6c31662c243a85fe54200f9f5932d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:03:35 +1000 Subject: [PATCH 20/42] [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 5ee13a2d42b964e6b39f295b63d1d3c03180bbc9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:17:01 +1000 Subject: [PATCH 21/42] [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 f02f31d09..745398187 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include "packet_handler.h" @@ -862,8 +863,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 039981e687323435b65291deefc1c6c58115ea55 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:17:01 +1000 Subject: [PATCH 22/42] [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 a68a49653ae4254c9a1ab4b12479fbda4d2c5937 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:18:14 +1000 Subject: [PATCH 23/42] [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 faa884982..b749ba1cb 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -560,10 +560,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 8a267be40faa4bdfd78b026afa123f10ed0d00c9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:19:25 +1000 Subject: [PATCH 24/42] [dhcpmon]: Keep idle positive health indeterminate Advance pending relay-flow state on every health window, but report positive health only when monitored DHCPv4 counters show current activity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 8397bd981..fcc8d3be7 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -185,6 +185,9 @@ std::unordered_map dhcp_device_get_untransmitted_windows(const st (const int *)monitored_msgs, monitored_msg_sz); } +static bool check_counter_increased(const std::string &ifname, int sock, + const int *monitored_msgs, size_t monitored_msg_cnt); + /** * @code dhcp_device_check_positive_health(ifname); * @brief Check that DHCP relayed messages are being transmitted out of this interface/dev @@ -195,12 +198,16 @@ std::unordered_map dhcp_device_get_untransmitted_windows(const st */ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &ifname) { + bool has_activity = check_counter_increased(ifname, rx_sock, + (const int *)monitored_msgs, monitored_msg_sz) || + check_counter_increased(ifname, tx_sock, + (const int *)monitored_msgs, monitored_msg_sz); for (const auto &[msg_type, windows] : dhcp_device_get_untransmitted_windows(ifname)) { if (windows > 0) { return DHCP_MON_STATUS_UNHEALTHY; } } - return DHCP_MON_STATUS_HEALTHY; + return has_activity ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; } /** From ff79c1a52e6f48c9e08a219e9779333f8f78d078 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:29:00 +1000 Subject: [PATCH 25/42] [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 b749ba1cb..1cc2c53d8 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -282,9 +282,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"); @@ -293,6 +296,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"); @@ -330,6 +334,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 @@ -463,6 +468,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); if (health_reset_pending.exchange(false)) { reset_dhcp_relay_health_state(agg_dev_all); @@ -524,6 +530,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 b5140e81ecfa98563a36303d363b5f57b67eef51 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:30:26 +1000 Subject: [PATCH 26/42] [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 1cc2c53d8..aa3dce406 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -468,7 +468,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); if (health_reset_pending.exchange(false)) { reset_dhcp_relay_health_state(agg_dev_all); @@ -505,6 +504,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 3d8f221016cd9a65e3c11e46f4212e01a2bebf83 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:31:42 +1000 Subject: [PATCH 27/42] [dhcpmon]: Reset health state while counters are quiesced Sample counter-clear and topology-refresh watermarks under the exclusive counter-state lock, before packet handlers resume after reconciliation. 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 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index aa3dce406..33a9d118e 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -470,6 +470,7 @@ 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 (health_reset_pending.exchange(false)) { + std::unique_lock counter_lock(packet_handler_quiesce_mutex); reset_dhcp_relay_health_state(agg_dev_all); } @@ -486,6 +487,9 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) if (topology_refresh_pending && subscribers_available) { sock_mgr_suspend_packet_handler(); int result = dhcp_mon_reconcile_topology(); + if (result == 0) { + reset_dhcp_relay_health_state(agg_dev_all); + } if (sock_mgr_resume_packet_handler() < 0) { syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); dhcp_mon_stop(); @@ -498,7 +502,6 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) if (result != 0) { return; } - reset_dhcp_relay_health_state(agg_dev_all); syslog(LOG_INFO, "Refreshed DHCP interface membership from CONFIG_DB"); return; } From bb213892c1e50c15352f6d144562cc1fcf759f12 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:37:21 +1000 Subject: [PATCH 28/42] [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 | 12 ++++++------ src/packet_handler.cpp | 6 ++++-- src/sock_mgr.cpp | 18 ++++++++++++++++++ src/sock_mgr.h | 14 ++++++++++++++ 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 33a9d118e..1c242d88e 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -284,7 +284,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); } @@ -296,7 +296,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"); @@ -334,7 +334,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 @@ -470,7 +470,7 @@ 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 (health_reset_pending.exchange(false)) { - std::unique_lock counter_lock(packet_handler_quiesce_mutex); + counter_state_write_lock counter_lock; reset_dhcp_relay_health_state(agg_dev_all); } @@ -507,7 +507,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); @@ -533,7 +533,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 745398187..343905483 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -860,13 +860,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 4409d3655b2720a9bc4e9ad3755459b4e51e92a5 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 12:55:31 +1000 Subject: [PATCH 29/42] [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 | 31 ++++++++-- src/packet_handler.cpp | 12 +--- src/sock_mgr.cpp | 130 +++++++++++++++++++++++++++++++++++------ src/sock_mgr.h | 13 ++++- 4 files changed, 154 insertions(+), 32 deletions(-) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 1c242d88e..4f11c1d39 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -285,8 +285,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)) { @@ -297,6 +299,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"); @@ -335,6 +340,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 @@ -471,6 +479,9 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) if (health_reset_pending.exchange(false)) { counter_state_write_lock counter_lock; + if (!counter_lock.owns_lock()) { + return; + } reset_dhcp_relay_health_state(agg_dev_all); } @@ -485,7 +496,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 (result == 0) { reset_dhcp_relay_health_state(agg_dev_all); @@ -508,6 +523,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); @@ -534,6 +552,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 @@ -916,7 +937,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 343905483..e81dd90db 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include "packet_handler.h" @@ -860,15 +859,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 b029d3a32316f0bf291c0d83654e70ba2552ab62 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:03:12 +1000 Subject: [PATCH 30/42] [dhcpmon]: Clarify relay disparity log Describe the per-message missing corresponding transmit without implying that all DHCPv4 transmit activity stopped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/health_check.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/health_check.cpp b/src/health_check.cpp index bf374a01d..71ce5e982 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -48,7 +48,7 @@ static void alert_dhcp_relay_disparity(int duration) static void log_agg_error(int duration) { - syslog(LOG_ALERT, "dhcpmon detected DHCPv4 packets received but none transmitted. Duration: %d (sec) for intf: %s", + syslog(LOG_ALERT, "dhcpmon detected DHCPv4 receive activity without a corresponding transmit. Duration: %d (sec) for intf: %s", duration, agg_dev_all.c_str()); } From 2ddadceb3a93ac7dbab5da5a3b9ea87b28f957e5 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:21:22 +1000 Subject: [PATCH 31/42] [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 7b88a9838f844c2431ff4e6feb580124eb568a77 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:22:26 +1000 Subject: [PATCH 32/42] [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 4f11c1d39..99c862e30 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -61,6 +61,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); @@ -651,7 +652,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 6098f7d289e71825abbe54f505c5be18589b2bb6 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:25:11 +1000 Subject: [PATCH 33/42] [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 f1939a4e92327964c71a67aa6f3b6ef6a240cb66 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:26:04 +1000 Subject: [PATCH 34/42] [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 99c862e30..71f3b6dd0 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -70,7 +70,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 { @@ -511,13 +511,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; } @@ -658,6 +653,10 @@ static 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 3ed73cf7a484133145b3118531eea04c12a9edbd Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:52:11 +1000 Subject: [PATCH 35/42] [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 71f3b6dd0..7cdfb93d9 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -547,26 +547,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 787378896c54d8a8b47cf3c44491d5c0a034fa73 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 13:53:11 +1000 Subject: [PATCH 36/42] [dhcpmon]: Document health counter locking Record the counter-state lock requirement for relay watermark helpers and update the health-state description to match the active checks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.h | 2 ++ src/health_check.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dhcp_device.h b/src/dhcp_device.h index 62d61319a..3927beae3 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -247,6 +247,7 @@ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_ * @code dhcp_device_get_untransmitted_windows(ifname); * * @brief update and return unmatched DHCPv4 relay RX age in health windows per message type. + * Caller must hold the counter-state write lock or otherwise quiesce packet handlers. * * @param ifname interface name * @@ -258,6 +259,7 @@ std::unordered_map dhcp_device_get_untransmitted_windows(const st * @code dhcp_device_reset_health_state(ifname); * * @brief reset relay health watermarks to the current counters for an interface. + * Caller must hold the counter-state write lock or otherwise quiesce packet handlers. * * @param ifname interface name * diff --git a/src/health_check.cpp b/src/health_check.cpp index 71ce5e982..594acf961 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -163,7 +163,7 @@ static dhcp_mon_status_t check_per_interface_tx_health_v6() return DHCP_MON_STATUS_HEALTHY; } -/** DHCP monitor state data for aggregate device for mgmt device */ +/** DHCP monitor state for management traffic and interface hierarchy consistency */ static dhcp_mon_state_t state_data[] = { [0] = { .check_health = check_mgmt_health, From 37a01731ad32315199d23d38e395a788f6a42c26 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:14:15 +1000 Subject: [PATCH 37/42] [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 db17306a4758345c00e018cf0630a09853f7bc20 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:15:15 +1000 Subject: [PATCH 38/42] [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++; + } + } } } From ea77c7e360fe8d641b784d2124d574c520f28cf6 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 14:16:04 +1000 Subject: [PATCH 39/42] [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 | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 7cdfb93d9..4a645cae7 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -505,6 +505,7 @@ static void timeout_callback(evutil_socket_t fd, short event, void *arg) int result = dhcp_mon_reconcile_topology(); if (result == 0) { reset_dhcp_relay_health_state(agg_dev_all); + sock_mgr_drain_sock_buffer(); } if (sock_mgr_resume_packet_handler() < 0) { syslog(LOG_ALERT, "Failed to resume packet handlers after topology refresh"); From a5bd7ead135cdca1f24108ef35ea0f7bf0f7249e Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 15:17:17 +1000 Subject: [PATCH 40/42] [dhcpmon]: Identify the failing interface edge Include family, direction, parent and child-aggregate deltas, expected ratio, and message type in persistent hierarchy mismatch logs for precise loss localization. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 18 ++++++++++++++++++ src/dhcp_device.h | 3 +++ src/health_check.cpp | 4 ++-- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index fcc8d3be7..71450b936 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -105,6 +105,8 @@ void dhcp_device_reset_health_state(const std::string &ifname) (const int *)monitored_msgs, monitored_msg_sz); } +static std::string last_counter_mismatch; + /** * @code check_counter_not_transmitted(ifname, rx_sock, tx_sock, monitored_msgs, monitored_msg_cnt); * @brief Check if there are received DHCP messages that are not transmitted out @@ -311,12 +313,28 @@ static bool check_counters_delta_expected(const std::string &ifname, const std:: uint64_t delta = counters.at(monitored_msgs[i]) - counters_snapshot.at(monitored_msgs[i]); uint64_t other_delta = other_counters.at(monitored_msgs[i]) - other_counters_snapshot.at(monitored_msgs[i]); if (delta * ratio != other_delta) { + const std::string *message_names = sock_info.is_v6 ? db_counter_name_v6 : db_counter_name; + last_counter_mismatch = + std::string(sock_info.is_v6 ? "IPv6 " : "IPv4 ") + + (sock_info.is_rx ? "RX" : "TX") + + " edge parent=" + ifname + + " parent_delta=" + std::to_string(delta) + + " child_aggregate=" + other_ifname + + " child_delta=" + std::to_string(other_delta) + + " expected_ratio=" + std::to_string(ratio) + + " message=" + message_names[monitored_msgs[i]]; return false; } } + return true; } +const std::string &dhcp_device_get_last_counter_mismatch() +{ + return last_counter_mismatch; +} + static dhcp_mon_status_t dhcp_device_check_agg_equal_rx(const std::string &ifname) { std::string agg_ifname = agg_dev_prefix + ifname; diff --git a/src/dhcp_device.h b/src/dhcp_device.h index 3927beae3..6f73d199f 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -243,6 +243,9 @@ void dhcp_device_free(dhcp_device_context_t *context); */ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_check_t check_type); +/** Return details for the most recent interface hierarchy mismatch */ +const std::string &dhcp_device_get_last_counter_mismatch(); + /** * @code dhcp_device_get_untransmitted_windows(ifname); * diff --git a/src/health_check.cpp b/src/health_check.cpp index 594acf961..6cff3e37e 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -108,7 +108,7 @@ static dhcp_mon_status_t check_per_interface_rx_health() static void log_agg_per_interface_rx_error(int duration) { syslog(LOG_ALERT, "sum of rx per interface counter does not equal corresponding vlan/portchan counter." - " Duration: %d (sec)", duration); + " Duration: %d (sec). %s", duration, dhcp_device_get_last_counter_mismatch().c_str()); } static dhcp_mon_status_t check_per_interface_tx_health() @@ -130,7 +130,7 @@ static void log_agg_per_interface_tx_error(int duration) { syslog(LOG_ALERT, "each tx per interface counter does not equal corresponding vlan counter," " or sum of tx per interface counter does not equal corresponding portchan counter." - " Duration: %d (sec)", duration); + " Duration: %d (sec). %s", duration, dhcp_device_get_last_counter_mismatch().c_str()); } static dhcp_mon_status_t check_per_interface_rx_health_v6() From a97b63caa0f120f17e2df0e96a272fc459fd2734 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 15:51:24 +1000 Subject: [PATCH 41/42] [dhcpmon]: Propagate packet event suspend failures Reject non-resumable event tags before deletion, roll back partial event_del failures, and surface suspend errors through the socket manager for coherent fail-stop recovery. 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 | 29 +++++++++++++++++++++++++---- src/event_mgr.h | 2 +- src/sock_mgr.cpp | 34 ++++++++++++++++++++++++++++++---- 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/src/event_mgr.cpp b/src/event_mgr.cpp index a794cee1a..575b3ac15 100644 --- a/src/event_mgr.cpp +++ b/src/event_mgr.cpp @@ -1,4 +1,5 @@ #include +#include #include "event_mgr.h" @@ -92,22 +93,42 @@ 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) +int 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; + return -1; } 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; + return -1; } for (const auto &event : tagged_events->second) { - event_del(event); + if (event_get_fd(event) < 0) { + syslog(LOG_ALERT, "event_mgr: Cannot suspend non-fd event with tag %s for %s", + tag.c_str(), this->name.c_str()); + return -1; + } } + std::vector deleted_events; + for (const auto &event : tagged_events->second) { + if (event_del(event) < 0) { + bool restore_failed = false; + for (struct event *deleted_event : deleted_events) { + if (event_add(deleted_event, NULL) < 0) { + restore_failed = true; + } + } + syslog(LOG_ALERT, "event_mgr: Failed to suspend event (fd=%d) with tag %s for %s", + event_get_fd(event), tag.c_str(), this->name.c_str()); + return restore_failed ? -2 : -1; + } + deleted_events.push_back(event); + } + return 0; } int event_mgr::resume_all_events(const std::string &tag) diff --git a/src/event_mgr.h b/src/event_mgr.h index 26a1b5135..cca15edf8 100644 --- a/src/event_mgr.h +++ b/src/event_mgr.h @@ -12,7 +12,7 @@ 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 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(); diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 53f54b019..0925b55f3 100644 --- a/src/sock_mgr.cpp +++ b/src/sock_mgr.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include "sock_mgr.h" @@ -36,7 +37,7 @@ static const char dhcp_outbound_filter[] = "outbound and ip and udp and (port 67 static const char dhcpv6_inbound_filter[] = "inbound and ip6 and udp and (port 547 or port 546)"; static const char dhcpv6_outbound_filter[] = "outbound and ip6 and udp and (port 547 or port 546)"; -/** Tags for different events, so we can triiger only one type */ +/** Tags for different events, so we can trigger only one type */ static const char packet_handler_tag[] = "PacketHandler"; static const char cache_counter_updater_tag[] = "CacheCounterUpdater"; static const char keepalive_tag[] = "Keepalive"; @@ -570,8 +571,33 @@ int sock_mgr_suspend_packet_handler() syslog(LOG_ALERT, "Packet handlers are already suspended"); return -1; } + std::vector suspended_event_mgrs; for (const auto &entry : sock_map) { - entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); + event_mgr *event_mgr_ptr = entry.second.event_mgr_ptr; + int suspend_result = event_mgr_ptr->suspend_all_events(packet_handler_tag); + if (suspend_result < 0) { + bool rollback_failed = suspend_result < -1; + for (event_mgr *suspended_event_mgr : suspended_event_mgrs) { + if (suspended_event_mgr->resume_all_events(packet_handler_tag) < 0) { + rollback_failed = true; + } + } + if (rollback_failed) { + set_packet_handlers_enabled(false); + for (const auto &rollback_entry : sock_map) { + rollback_entry.second.event_mgr_ptr->suspend_all_events(packet_handler_tag); + } + 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 after suspend rollback failure: %s", + e.what()); + } + } + return -1; + } + suspended_event_mgrs.push_back(event_mgr_ptr); } set_packet_handlers_enabled(false); try { @@ -862,9 +888,9 @@ void sock_mgr_update_db_counters(const socket_counters_t &counters_by_socket) std::string table_name = construct_counter_db_table_key(ifname, info.is_v6); mCountersDbPtr->hset(table_name, info.is_rx ? "RX" : "TX", value); } - syslog_debug(LOG_INFO, "Processing cache counter entry of %sfor downstream vlan %s", + syslog_debug(LOG_INFO, "Processing cache counter entry of %s for downstream vlan %s", all_ifname.c_str(), downstream_ifname.c_str()); - syslog_debug(LOG_INFO, "Skipped aggregated device counter entry of %sfor downstream vlan %s", + syslog_debug(LOG_INFO, "Skipped aggregated device counter entry of %s for downstream vlan %s", all_skipped_ifname.c_str(), downstream_ifname.c_str()); } } From 0d0cf265d81304c79fa1848aabf3348555fd2a9f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 16:07:37 +1000 Subject: [PATCH 42/42] [dhcpmon]: Detect one-sided hierarchy counter changes Evaluate both an interface and its immediate-child aggregate before declaring a quiet window, so child-only packet observations expose the exact forwarding edge that did not advance. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_device.cpp | 65 +++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 71450b936..a34ea8050 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -330,6 +330,19 @@ static bool check_counters_delta_expected(const std::string &ifname, const std:: return true; } +static dhcp_mon_status_t check_aggregate_health(const std::string &ifname, int sock, uint8_t ratio, + const int *monitored_msgs, size_t monitored_msg_cnt) +{ + std::string agg_ifname = agg_dev_prefix + ifname; + if (!check_counter_increased(ifname, sock, monitored_msgs, monitored_msg_cnt) && + !check_counter_increased(agg_ifname, sock, monitored_msgs, monitored_msg_cnt)) { + return DHCP_MON_STATUS_INDETERMINATE; + } + return check_counters_delta_expected(ifname, agg_ifname, sock, ratio, + monitored_msgs, monitored_msg_cnt) ? + DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; +} + const std::string &dhcp_device_get_last_counter_mismatch() { return last_counter_mismatch; @@ -337,62 +350,54 @@ const std::string &dhcp_device_get_last_counter_mismatch() static dhcp_mon_status_t dhcp_device_check_agg_equal_rx(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, rx_sock, 1, (const int *)monitored_msgs, monitored_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, rx_sock, 1, (const int *)monitored_msgs, monitored_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_equal_tx(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, tx_sock, 1, (const int *)monitored_msgs, monitored_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, tx_sock, 1, (const int *)monitored_msgs, monitored_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_equal_rx_v6(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, rx_sock_v6, 1, (const int *)monitored_v6_msgs, monitored_v6_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, rx_sock_v6, 1, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_equal_tx_v6(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, tx_sock_v6, 1, (const int *)monitored_v6_msgs, monitored_v6_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, tx_sock_v6, 1, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_multiple_rx(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, rx_sock, readonly_access(rev_vlan_map, ifname).size() + readonly_access(rev_portchan_map, ifname).size(), - (const int *)monitored_msgs, monitored_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, rx_sock, + readonly_access(rev_vlan_map, ifname).size() + + readonly_access(rev_portchan_map, ifname).size(), + (const int *)monitored_msgs, monitored_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_multiple_tx(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, tx_sock, readonly_access(rev_vlan_map, ifname).size() + readonly_access(rev_portchan_map, ifname).size(), - (const int *)monitored_msgs, monitored_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, tx_sock, + readonly_access(rev_vlan_map, ifname).size() + + readonly_access(rev_portchan_map, ifname).size(), + (const int *)monitored_msgs, monitored_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_multiple_rx_v6(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, rx_sock_v6, readonly_access(rev_vlan_map, ifname).size() + readonly_access(rev_portchan_map, ifname).size(), - (const int *)monitored_v6_msgs, monitored_v6_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, rx_sock_v6, + readonly_access(rev_vlan_map, ifname).size() + + readonly_access(rev_portchan_map, ifname).size(), + (const int *)monitored_v6_msgs, monitored_v6_msg_sz); } static dhcp_mon_status_t dhcp_device_check_agg_multiple_tx_v6(const std::string &ifname) { - std::string agg_ifname = agg_dev_prefix + ifname; - return check_counters_delta_expected(ifname, agg_ifname, tx_sock_v6, readonly_access(rev_vlan_map, ifname).size() + readonly_access(rev_portchan_map, ifname).size(), - (const int *)monitored_v6_msgs, monitored_v6_msg_sz) ? - DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; + return check_aggregate_health(ifname, tx_sock_v6, + readonly_access(rev_vlan_map, ifname).size() + + readonly_access(rev_portchan_map, ifname).size(), + (const int *)monitored_v6_msgs, monitored_v6_msg_sz); } /** @@ -494,7 +499,9 @@ void dhcp_device_print_status_debug(const std::string &ifname, dhcp_counters_typ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_check_t check_type) { - if (check_type != DHCP_DEVICE_CHECK_POSITIVE && check_type != DHCP_DEVICE_CHECK_POSITIVE_V6 && + bool hierarchy_check = check_type >= DHCP_DEVICE_CHECK_AGG_EQUAL_RX; + if (!hierarchy_check && + check_type != DHCP_DEVICE_CHECK_POSITIVE && check_type != DHCP_DEVICE_CHECK_POSITIVE_V6 && sock_mgr_counters_unchanged(ifname, (const int *)monitored_msgs, monitored_msg_sz, (const int *)monitored_v6_msgs, monitored_v6_msg_sz)) { return DHCP_MON_STATUS_INDETERMINATE;