From 7c63b1b245fd0e14aa9eb9f19965a52aa29f45da Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 14:36:18 +1000 Subject: [PATCH 1/2] [dhcpmon]: Match DHCPv4 counters across health windows Track unmatched receive activity by message type so idle windows and adjacent transmit activity do not create false relay-loss state. 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 | 145 +++++++++++++++++++++++++++++++++++++++----- src/dhcp_device.h | 9 +++ src/dhcp_mon.cpp | 1 + 3 files changed, 140 insertions(+), 15 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 8b1ed4f6c..f60dd7c19 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -69,39 +69,136 @@ static const char *counter_desc[DHCP_COUNTERS_COUNT] = { [DHCP_COUNTERS_SNAPSHOT_V6] = "Snapshot_V6", }; +typedef struct +{ + uint64_t last_rx = 0; + uint64_t last_tx = 0; + uint32_t pending_windows = 0; + uint8_t tx_credit = 0; + bool initialized = false; +} relay_flow_state_t; + +static std::unordered_map>> relay_flow_states; + +static void initialize_relay_flow_states(const std::string &ifname) +{ + 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_sz; 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) +{ + relay_flow_states[rx_sock].erase(ifname); + initialize_relay_flow_states(ifname); +} + /** - * @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 - * of this interface/device using its counters. + * @brief Update and return unmatched RX age per DHCP message type. * @param ifname interface name * @param rx_sock rx socket * @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 message type to unmatched-window count */ -// 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, + bool *reset_detected = NULL) { 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); + + std::unordered_map result; + for (size_t i = 0; i < monitored_msg_cnt; i++) { + 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) { + if (reset_detected != NULL && state.initialized) { + *reset_detected = true; + } + 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 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); +} + +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) +{ + 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_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); + const counter_t &tx_snapshot = tx_sock_info.all_counters_snapshot.at(ifname); - // when there is packet in, no packet out 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])) { + int msg_type = monitored_msgs[i]; + if (rx_counters.at(msg_type) > rx_snapshot.at(msg_type) && + tx_counters.at(msg_type) <= tx_snapshot.at(msg_type)) { return true; } } return false; } +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 @@ -112,8 +209,24 @@ 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; + bool reset_detected = false; + 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); + auto windows_by_type = get_untransmitted_windows( + ifname, rx_sock, tx_sock, (const int *)monitored_msgs, monitored_msg_sz, + &reset_detected); + if (reset_detected) { + // Counter replacement ends any pre-reset unhealthy episode. + return DHCP_MON_STATUS_HEALTHY; + } + for (const auto &[msg_type, windows] : windows_by_type) { + if (windows > 0) { + return DHCP_MON_STATUS_UNHEALTHY; + } + } + return has_activity ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; } /** @@ -382,7 +495,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 && + 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..b0f4440cf 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,14 @@ 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); +/** + * @brief Update and return unmatched DHCPv4 relay RX age in health windows per message type. + */ +std::unordered_map dhcp_device_get_untransmitted_windows(const std::string &ifname); + +/** Reset DHCPv4 relay-flow watermarks */ +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 ef4f6623d..ee4fc09b6 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -520,6 +520,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(); + dhcp_device_reset_health_state(agg_dev_all); syslog(LOG_INFO, "Initialized all counters for tracked interfaces"); window_interval_sec = window_sec; From 9fdb23ca1dc80f55563d0edc221108457527b190 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 12:20:21 +1000 Subject: [PATCH 2/2] [dhcpmon]: Report persistent DHCPv4 relay loss per VLAN Publish one VLAN-level disparity episode from the independent per-message unmatched receive windows. 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 | 66 +++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 19 deletions(-) diff --git a/src/health_check.cpp b/src/health_check.cpp index e9949d18c..665ee40bc 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -4,6 +4,8 @@ */ #include +#include +#include #include #include #include @@ -27,10 +29,7 @@ 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 bool reported_disparity_v4 = false; static dhcp_mon_status_t check_mgmt_health() { @@ -46,12 +45,45 @@ static void alert_dhcp_relay_disparity(int duration) event_publish(g_events_handle, "dhcp-relay-disparity", ¶ms); } -static void log_agg_error(int duration) +static void log_v4_agg_error(int duration) +{ + syslog(LOG_ALERT, "dhcpmon detected DHCPv4 receive activity without a corresponding transmit. Duration: %d (sec) for intf: %s", + duration, agg_dev_all.c_str()); +} + +static void log_v6_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 DHCPv6 packets received but none transmitted. Duration: %d (sec) for intf: %s", duration, agg_dev_all.c_str()); } +static void check_relay_disparity() +{ + auto windows_by_type = dhcp_device_get_untransmitted_windows(agg_dev_all); + uint32_t report_windows = 0; + bool has_pending = false; + + for (const auto &entry : windows_by_type) { + uint32_t windows = entry.second; + has_pending |= windows > 0; + if (static_cast(windows) > dhcp_unhealthy_max_count) { + report_windows = std::max(report_windows, windows); + } + } + + if (!has_pending) { + reported_disparity_v4 = false; + } + if (report_windows > 0 && !reported_disparity_v4) { + int64_t duration_value = static_cast(report_windows) * window_interval_sec; + int duration = static_cast(std::min( + duration_value, static_cast(std::numeric_limits::max()))); + alert_dhcp_relay_disparity(duration); + log_v4_agg_error(duration); + reported_disparity_v4 = true; + } +} + static void log_mgmt_error(int duration) { syslog(LOG_ALERT, "dhcpmon detected DHCP packets traveling through mgmt interface (please check BGP routes.)" @@ -148,43 +180,37 @@ 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, }, - [2] = { + [1] = { .check_health = check_agg_health_v6, .alert = alert_dhcp_relay_disparity, - .log = log_agg_error, + .log = log_v6_agg_error, .count = 0, }, - [3] = { + [2] = { .check_health = check_mgmt_health_v6, .log = log_mgmt_error, .count = 0, }, - [4] = { + [3] = { .check_health = check_per_interface_rx_health, .log = log_agg_per_interface_rx_error, .count = 0, }, - [5] = { + [4] = { .check_health = check_per_interface_tx_health, .log = log_agg_per_interface_tx_error, .count = 0, }, - [6] = { + [5] = { .check_health = check_per_interface_rx_health_v6, .log = log_agg_per_interface_rx_error, .count = 0, }, - [7] = { + [6] = { .check_health = check_per_interface_tx_health_v6, .log = log_agg_per_interface_tx_error, .count = 0, @@ -197,6 +223,8 @@ void check_dhcp_relay_health() { 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) {