[dhcpmon] Report persistent DHCPv4 relay loss per VLAN

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<int, std::unordered_map<std::string,
+    std::unordered_map<int, relay_flow_state_t>>> 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<int, uint32_t> 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<int, uint32_t> 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<int, uint32_t> 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;
     }
 

src/dhcp_device.h

@@ -18,6 +18,7 @@
 #include <netinet/in.h>
 #include <net/ethernet.h>
 #include <string>
+#include <unordered_map>
 
 /** 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<int, uint32_t> 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);
  *

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;

src/health_check.cpp

@@ -4,6 +4,8 @@
  */
 
 #include <syslog.h>
+#include <algorithm>
+#include <limits>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ -27,10 +29,7 @@ extern std::string agg_dev_prefix;
 extern std::unordered_map<std::string, std::unordered_set<std::string>> rev_vlan_map;
 extern std::unordered_map<std::string, std::unordered_set<std::string>> 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", &params);
 }
 
-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<int64_t>(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<int64_t>(report_windows) * window_interval_sec;
+        int duration = static_cast<int>(std::min(
+            duration_value, static_cast<int64_t>(std::numeric_limits<int>::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) {