From fd8f833e4cef3a41497d614c347fdba3caaf5ba2 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 16:51:59 +0000 Subject: [PATCH 1/5] [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 9f467e5a70c6da5b55c6c6a3ff8a58e872d265f9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:04:25 +0000 Subject: [PATCH 2/5] [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 f4ee536f6..c6ea0dda7 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -47,7 +47,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 7eb1d33b6cabeb7c96fcc6a22fab299d0e7bf40e Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 3/5] [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 75308faed..1631b3b13 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -338,11 +338,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 98f18370419466760912f30fdf4a304ba877d0d1 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 15:17:17 +1000 Subject: [PATCH 4/5] [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 8b1ed4f6c..4c4c09972 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -69,6 +69,8 @@ static const char *counter_desc[DHCP_COUNTERS_COUNT] = { [DHCP_COUNTERS_SNAPSHOT_V6] = "Snapshot_V6", }; +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 @@ -217,12 +219,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 5d4b00721..86dd78014 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -242,6 +242,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_print_status(ifname, type); * diff --git a/src/health_check.cpp b/src/health_check.cpp index e9949d18c..6f8b957c6 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -90,7 +90,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() @@ -112,7 +112,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 da0519077f91df3d030fee9176a69548a2cb0ca0 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 16:07:37 +1000 Subject: [PATCH 5/5] [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 | 66 +++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 4c4c09972..63920531e 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -236,6 +236,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; @@ -243,62 +256,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); } /** @@ -400,7 +405,10 @@ 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)) { + bool hierarchy_check = check_type >= DHCP_DEVICE_CHECK_AGG_EQUAL_RX; + if (!hierarchy_check && + 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; }