From 92295d6ee5b13a22ae155a0789f43b2c76cd174a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 16:51:59 +0000 Subject: [PATCH 01/23] [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 f79d80142be8532f75b9cdd1f6319f50d02f021e Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sat, 25 Jul 2026 23:04:25 +0000 Subject: [PATCH 02/23] [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 d86c02c72e2a2df6df0414d526293f3f055f6792 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 11:03:19 +1000 Subject: [PATCH 03/23] [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 03ab260f1e2b7cfe7e3e63a5e05064d3fa26b6c3 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 17:26:46 +1000 Subject: [PATCH 04/23] [dhcpmon]: Bound hierarchy context traversal Treat configured context interfaces as roots and fail closed after the expected physical-to-PortChannel-to-VLAN depth, avoiding cyclic recursion in packet processing. 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 | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 1631b3b13..ec84c1f32 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -318,25 +318,40 @@ void dhcp_devman_free() intfs.clear(); } -const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &ifname) +static constexpr unsigned int MAX_CONTEXT_DEPTH = 3; + +static const dhcp_device_context_t *get_device_context( + const std::string &ifname, unsigned int depth) { + if (depth > MAX_CONTEXT_DEPTH) { + syslog_debug(LOG_WARNING, "Exceeded interface membership depth at %s", ifname.c_str()); + return NULL; + } const auto iter = intfs.find(ifname); 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); + return get_device_context(port_channel->second, depth + 1); } const auto vlan = vlan_map.find(ifname); if (vlan != vlan_map.end() && ifname != vlan->second) { - return dhcp_devman_get_device_context(vlan->second); + return get_device_context(vlan->second, depth + 1); } return NULL; } +const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &ifname) +{ + return get_device_context(ifname, 0); +} + std::string dhcp_devman_get_parent_ifname(const std::string &ifname) { + if (intfs.find(ifname) != intfs.end()) { + return ""; + } const auto port_channel = portchan_map.find(ifname); if (port_channel != portchan_map.end() && ifname != port_channel->second) { return port_channel->second; From c47f69f170014818843da3b513cc71b36b853098 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 13:26:35 +1000 Subject: [PATCH 05/23] [dhcpmon]: Preserve direct VLAN member precedence Resolve direct VLAN membership before falling back through PortChannel membership for both context and immediate-parent lookup. 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 | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index ec84c1f32..4fecd0fc0 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -331,14 +331,14 @@ static const dhcp_device_context_t *get_device_context( 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 get_device_context(port_channel->second, depth + 1); - } const auto vlan = vlan_map.find(ifname); if (vlan != vlan_map.end() && ifname != vlan->second) { return get_device_context(vlan->second, depth + 1); } + const auto port_channel = portchan_map.find(ifname); + if (port_channel != portchan_map.end() && ifname != port_channel->second) { + return get_device_context(port_channel->second, depth + 1); + } return NULL; } @@ -352,14 +352,14 @@ std::string dhcp_devman_get_parent_ifname(const std::string &ifname) if (intfs.find(ifname) != intfs.end()) { return ""; } - const auto port_channel = portchan_map.find(ifname); - 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() && ifname != vlan->second) { return vlan->second; } + const auto port_channel = portchan_map.find(ifname); + if (port_channel != portchan_map.end() && ifname != port_channel->second) { + return port_channel->second; + } return ""; } From 8f45cc63d33748954df1df572cc842aba91ec1b9 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 13:30:50 +1000 Subject: [PATCH 06/23] [dhcpmon]: Reuse hierarchy parent lookup Use the VLAN-first parent resolver for context traversal, aggregate selection, and tracked-interface detection. 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 | 62 +++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 41 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 4fecd0fc0..80b135974 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -180,22 +180,9 @@ int dhcp_devman_setup_dual_tor_mode(const char *name) bool dhcp_devman_is_tracked_interface(const std::string &ifname) { - auto itr = intfs.find(ifname); - if (itr != intfs.end()) { - return true; - } - auto vlan_itr = vlan_map.find(ifname); - if (vlan_itr != vlan_map.end()) { - return true; - } - auto portchan_itr = portchan_map.find(ifname); - if (portchan_itr != portchan_map.end()) { - return true; - } - if (ifname == mgmt_ifname) { - return true; - } - return false; + return intfs.find(ifname) != intfs.end() || + !dhcp_devman_get_parent_ifname(ifname).empty() || + ifname == mgmt_ifname; } /** @@ -320,6 +307,22 @@ void dhcp_devman_free() static constexpr unsigned int MAX_CONTEXT_DEPTH = 3; +std::string dhcp_devman_get_parent_ifname(const std::string &ifname) +{ + if (intfs.find(ifname) != intfs.end()) { + return ""; + } + const auto vlan = vlan_map.find(ifname); + if (vlan != vlan_map.end() && ifname != vlan->second) { + return vlan->second; + } + const auto port_channel = portchan_map.find(ifname); + if (port_channel != portchan_map.end() && ifname != port_channel->second) { + return port_channel->second; + } + return ""; +} + static const dhcp_device_context_t *get_device_context( const std::string &ifname, unsigned int depth) { @@ -331,15 +334,8 @@ static const dhcp_device_context_t *get_device_context( if (iter != intfs.end()) { return iter->second; } - const auto vlan = vlan_map.find(ifname); - if (vlan != vlan_map.end() && ifname != vlan->second) { - return get_device_context(vlan->second, depth + 1); - } - const auto port_channel = portchan_map.find(ifname); - if (port_channel != portchan_map.end() && ifname != port_channel->second) { - return get_device_context(port_channel->second, depth + 1); - } - return NULL; + const std::string parent_ifname = dhcp_devman_get_parent_ifname(ifname); + return parent_ifname.empty() ? NULL : get_device_context(parent_ifname, depth + 1); } const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &ifname) @@ -347,22 +343,6 @@ const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &i return get_device_context(ifname, 0); } -std::string dhcp_devman_get_parent_ifname(const std::string &ifname) -{ - if (intfs.find(ifname) != intfs.end()) { - return ""; - } - const auto vlan = vlan_map.find(ifname); - if (vlan != vlan_map.end() && ifname != vlan->second) { - return vlan->second; - } - const auto port_channel = portchan_map.find(ifname); - if (port_channel != portchan_map.end() && ifname != port_channel->second) { - return port_channel->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); From dbcd37ab119e7dca307dfb901ebca0759ee65556 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 13:57:33 +1000 Subject: [PATCH 07/23] [dhcpmon]: Centralize hierarchy aggregate names Generate immediate-child aggregate counter names through the device manager for initialization and packet accounting. 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 | 8 +++++++- src/dhcp_devman.h | 9 +++++++++ src/dhcp_mon.cpp | 8 ++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 80b135974..70e5347d9 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -346,7 +346,13 @@ const dhcp_device_context_t *dhcp_devman_get_device_context(const std::string &i 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; + return parent_ifname.empty() ? agg_dev_all : + dhcp_devman_get_child_agg_counter_ifname(parent_ifname); +} + +std::string dhcp_devman_get_child_agg_counter_ifname(const std::string &parent_ifname) +{ + return 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 473e4dbc7..1d87620eb 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -149,6 +149,15 @@ 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); +/** + * @brief Return the aggregate counter containing a parent's immediate children. + * + * @param parent_ifname parent interface name + * + * @return aggregate counter name + */ +std::string dhcp_devman_get_child_agg_counter_ifname(const std::string &parent_ifname); + /** * @code dhcp_devman_get_agg_counter_ifname(ifname); * diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 77ec1406f..682d7955b 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -466,7 +466,9 @@ static void initialize_all_intf_counters() initialize_all_counters(ifname); } initialize_all_counters(vlan); - sock_mgr_init_cache_counters(agg_dev_prefix + vlan, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); + sock_mgr_init_cache_counters( + dhcp_devman_get_child_agg_counter_ifname(vlan), + DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } for (const auto &[portchan, intfs] : rev_portchan_map) { @@ -474,7 +476,9 @@ static void initialize_all_intf_counters() initialize_all_counters(ifname); } initialize_all_counters(portchan); - sock_mgr_init_cache_counters(agg_dev_prefix + portchan, DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); + sock_mgr_init_cache_counters( + dhcp_devman_get_child_agg_counter_ifname(portchan), + DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } // Now all vlan and portchannel related interfaces have entries in counters, now do the rest (uplink) From 86dbebd6660da4a2da7034a15e12988da87df277 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 14:00:45 +1000 Subject: [PATCH 08/23] [dhcpmon]: Keep aggregate formatting in utilities Keep hierarchy traversal in the device manager while placing the pure aggregate-name formatter with the existing aggregate utilities. 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 | 7 +------ src/dhcp_devman.h | 21 ++++++--------------- src/dhcp_mon.cpp | 4 ++-- src/util.h | 6 ++++++ 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 70e5347d9..6fd0ace67 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -347,12 +347,7 @@ 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 : - dhcp_devman_get_child_agg_counter_ifname(parent_ifname); -} - -std::string dhcp_devman_get_child_agg_counter_ifname(const std::string &parent_ifname) -{ - return agg_dev_prefix + parent_ifname; + get_child_agg_counter_ifname(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 1d87620eb..251a4230a 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -127,17 +127,6 @@ int dhcp_devman_init(); */ void dhcp_devman_free(); -/** - * @code dhcp_devman_get_device_context(ifname); - * - * @brief find device context, if its physical interface, will query vlan_map and portchannel_map first - * - * @param ifname interface name - * - * @return pointer to device (interface) context if found, NULL otherwise - */ -const dhcp_device_context_t* dhcp_devman_get_device_context(const std::string &ifname); - /** * @code dhcp_devman_get_parent_ifname(ifname); * @@ -150,13 +139,15 @@ 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); /** - * @brief Return the aggregate counter containing a parent's immediate children. + * @code dhcp_devman_get_device_context(ifname); * - * @param parent_ifname parent interface name + * @brief find device context by following the interface hierarchy. * - * @return aggregate counter name + * @param ifname interface name + * + * @return pointer to device (interface) context if found, NULL otherwise */ -std::string dhcp_devman_get_child_agg_counter_ifname(const std::string &parent_ifname); +const dhcp_device_context_t* dhcp_devman_get_device_context(const std::string &ifname); /** * @code dhcp_devman_get_agg_counter_ifname(ifname); diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 682d7955b..5e075671c 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -467,7 +467,7 @@ static void initialize_all_intf_counters() } initialize_all_counters(vlan); sock_mgr_init_cache_counters( - dhcp_devman_get_child_agg_counter_ifname(vlan), + get_child_agg_counter_ifname(vlan), DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } @@ -477,7 +477,7 @@ static void initialize_all_intf_counters() } initialize_all_counters(portchan); sock_mgr_init_cache_counters( - dhcp_devman_get_child_agg_counter_ifname(portchan), + get_child_agg_counter_ifname(portchan), DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } diff --git a/src/util.h b/src/util.h index d5a59763d..9e171c6f8 100644 --- a/src/util.h +++ b/src/util.h @@ -216,6 +216,12 @@ 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; } +/** Return the aggregate counter containing a parent's immediate children. */ +inline std::string get_child_agg_counter_ifname(const std::string &parent_ifname) +{ + return agg_dev_prefix + parent_ifname; +} + /** * @code contains_value(v, value); * @brief Check if a vector contains a specific value From dd7f3b376b603e1d7ece71f3674504bd9ddbf2e4 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 14:06:26 +1000 Subject: [PATCH 09/23] [dhcpmon]: Clarify hierarchy lookup contracts Document unmapped parent lookup and root aggregate fallback without changing behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_devman.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index 251a4230a..ee22e02e7 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -134,7 +134,7 @@ void dhcp_devman_free(); * * @param ifname interface name * - * @return parent interface name, or an empty string for a root context interface + * @return parent interface name, or an empty string for a root or unmapped interface */ std::string dhcp_devman_get_parent_ifname(const std::string &ifname); @@ -152,11 +152,11 @@ const dhcp_device_context_t* dhcp_devman_get_device_context(const std::string &i /** * @code dhcp_devman_get_agg_counter_ifname(ifname); * - * @brief find the aggregate counter for the immediate parent of a tracked interface. + * @brief find the aggregate counter updated by an interface observation. * * @param ifname interface name * - * @return aggregate counter name + * @return immediate-parent aggregate, or the root aggregate when no parent exists */ std::string dhcp_devman_get_agg_counter_ifname(const std::string &ifname); From 2ca4b0df8ad09740137726763351798bb1fed060 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Mon, 27 Jul 2026 14:14:33 +1000 Subject: [PATCH 10/23] [dhcpmon]: Preserve aggregate helper naming Keep the established get_agg_counter_ifname utility name while changing only its hierarchy semantics. 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 | 2 +- src/dhcp_mon.cpp | 4 ++-- src/util.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 6fd0ace67..8f6014ad1 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -347,7 +347,7 @@ 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 : - get_child_agg_counter_ifname(parent_ifname); + get_agg_counter_ifname(parent_ifname); } void dhcp_devman_print_all_status(dhcp_counters_type_t type) diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 5e075671c..3cedf09b0 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -467,7 +467,7 @@ static void initialize_all_intf_counters() } initialize_all_counters(vlan); sock_mgr_init_cache_counters( - get_child_agg_counter_ifname(vlan), + get_agg_counter_ifname(vlan), DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } @@ -477,7 +477,7 @@ static void initialize_all_intf_counters() } initialize_all_counters(portchan); sock_mgr_init_cache_counters( - get_child_agg_counter_ifname(portchan), + get_agg_counter_ifname(portchan), DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT); } diff --git a/src/util.h b/src/util.h index 9e171c6f8..a11766504 100644 --- a/src/util.h +++ b/src/util.h @@ -217,7 +217,7 @@ inline bool is_agg_counter(const std::string &ifname) } /** Return the aggregate counter containing a parent's immediate children. */ -inline std::string get_child_agg_counter_ifname(const std::string &parent_ifname) +inline std::string get_agg_counter_ifname(const std::string &parent_ifname) { return agg_dev_prefix + parent_ifname; } From b037509e17b8ac6bdb266c9c05ea89e78c48e36b Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 03:06:11 +1000 Subject: [PATCH 11/23] [dhcpmon]: Document aggregate helper consistently Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/util.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util.h b/src/util.h index a11766504..293688822 100644 --- a/src/util.h +++ b/src/util.h @@ -216,7 +216,12 @@ 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; } -/** Return the aggregate counter containing a parent's immediate children. */ +/** + * @code get_agg_counter_ifname(parent_ifname); + * @brief Get aggregate counter name for a parent's immediate children + * @param parent_ifname Parent interface name + * @return Aggregate counter name + */ inline std::string get_agg_counter_ifname(const std::string &parent_ifname) { return agg_dev_prefix + parent_ifname; From 1d8b2f8e39d0412ed194c982562a49e8a532845f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 12:08:01 +1000 Subject: [PATCH 12/23] [dhcpmon]: Describe interface parent mappings directly Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_devman.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index ee22e02e7..8d0009370 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -141,7 +141,7 @@ std::string dhcp_devman_get_parent_ifname(const std::string &ifname); /** * @code dhcp_devman_get_device_context(ifname); * - * @brief find device context by following the interface hierarchy. + * @brief find device context by following VLAN and PortChannel parent mappings. * * @param ifname interface name * From 1659bef93a4eeb2fb95a1d2bfa87aff95fba3c8d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 15:58:11 +1000 Subject: [PATCH 13/23] [dhcpmon]: Clarify parent and context lookup contracts Document root-interface parent semantics and the private context traversal helper. 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 | 10 ++++++++++ src/dhcp_devman.h | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/dhcp_devman.cpp b/src/dhcp_devman.cpp index 8f6014ad1..7d8fc09f4 100644 --- a/src/dhcp_devman.cpp +++ b/src/dhcp_devman.cpp @@ -323,6 +323,16 @@ std::string dhcp_devman_get_parent_ifname(const std::string &ifname) return ""; } +/** + * @code get_device_context(ifname, depth); + * + * @brief Follow parent mappings until reaching a tracked input interface + * + * @param ifname Interface name to resolve + * @param depth Current parent traversal depth + * + * @return Tracked interface context, or NULL when no context is found + */ static const dhcp_device_context_t *get_device_context( const std::string &ifname, unsigned int depth) { diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index 8d0009370..f0cdde3be 100644 --- a/src/dhcp_devman.h +++ b/src/dhcp_devman.h @@ -134,18 +134,18 @@ void dhcp_devman_free(); * * @param ifname interface name * - * @return parent interface name, or an empty string for a root or unmapped interface + * @return Immediate parent interface name, or an empty string when the interface is a tracked root or is unmapped */ std::string dhcp_devman_get_parent_ifname(const std::string &ifname); /** * @code dhcp_devman_get_device_context(ifname); * - * @brief find device context by following VLAN and PortChannel parent mappings. + * @brief find the tracked input interface that owns an interface. * * @param ifname interface name * - * @return pointer to device (interface) context if found, NULL otherwise + * @return The interface's tracked context; a tracked input interface returns its own context */ const dhcp_device_context_t* dhcp_devman_get_device_context(const std::string &ifname); From 3ab09058651f42162edda88926b97a933509ea23 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 15:58:39 +1000 Subject: [PATCH 14/23] [dhcpmon]: Track parent and aggregate counters per interface Populate one state per discovered VLAN or PortChannel check, derive aggregate ratios from topology, and format aggregate disparity errors with the interface and duration. 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 | 135 ++++++++++++--------------- src/dhcp_device.h | 12 +-- src/dhcp_mon.cpp | 1 + src/health_check.cpp | 213 +++++++++++-------------------------------- src/health_check.h | 23 ++++- 5 files changed, 135 insertions(+), 249 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 8b1ed4f6c..e7e22ea2a 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -20,11 +20,7 @@ extern bool debug_on; -extern std::string agg_dev_all; -extern std::string agg_dev_prefix; - extern std::unordered_map> rev_vlan_map; -extern std::unordered_map> rev_portchan_map; const std::string db_counter_name[DHCP_MESSAGE_TYPE_COUNT] = { "Unknown", "Discover", "Offer", "Request", "Decline", "Ack", "Nak", "Release", "Inform", "Bootp", "Malformed", "Ignored" @@ -204,7 +200,7 @@ static dhcp_mon_status_t dhcp_device_check_negative_health_v6(const std::string * @return true if deltas are equal with given ratio, false otherwise */ static bool check_counters_delta_expected(const std::string &ifname, const std::string &other_ifname, int sock, - uint8_t ratio, const int *monitored_msgs, size_t monitored_msg_cnt) + size_t ratio, const int *monitored_msgs, size_t monitored_msg_cnt) { const sock_info_t &sock_info = sock_mgr_get_sock_info(sock); const counter_t &counters = sock_info.all_counters.at(ifname); @@ -223,63 +219,49 @@ static bool check_counters_delta_expected(const std::string &ifname, const std:: return true; } -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; -} - -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; -} - -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; -} - -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; -} - -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; -} - -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; -} - -static dhcp_mon_status_t dhcp_device_check_agg_multiple_rx_v6(const std::string &ifname) +/** + * @code get_aggregate_ratio(ifname, sock); + * + * @brief Get the expected aggregate ratio for an interface and packet direction + * + * @param ifname Parent interface name + * @param sock Socket identifying the protocol family and direction + * + * @return One for RX and PortChannel TX; direct VLAN member count for VLAN TX + */ +static size_t get_aggregate_ratio(const std::string &ifname, int sock) { - 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; + // RX crosses one edge. VLAN TX is observed on every direct member; PortChannel TX is observed on one member. + if (sock_mgr_get_sock_info(sock).is_rx) { + return 1; + } + const auto vlan = rev_vlan_map.find(ifname); + return vlan == rev_vlan_map.end() ? 1 : vlan->second.size(); } -static dhcp_mon_status_t dhcp_device_check_agg_multiple_tx_v6(const std::string &ifname) +/** + * @code check_aggregate_health(ifname, sock, monitored_msgs, monitored_msg_cnt); + * + * @brief Compare a parent interface counter with the aggregate of its direct member-interface counters + * + * @param ifname Parent interface name + * @param sock Socket identifying the protocol family and direction + * @param monitored_msgs Message types to compare + * @param monitored_msg_cnt Number of message types to compare + * + * @return HEALTHY when idle or matching, UNHEALTHY when the expected ratio does not match + */ +static dhcp_mon_status_t check_aggregate_health(const std::string &ifname, int sock, const int *monitored_msgs, + size_t monitored_msg_cnt) { - 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) ? + const std::string agg_ifname = get_agg_counter_ifname(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_HEALTHY; + } + return check_counters_delta_expected( + ifname, agg_ifname, sock, get_aggregate_ratio(ifname, sock), + monitored_msgs, monitored_msg_cnt) ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; } @@ -382,7 +364,14 @@ 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 aggregate_check = + check_type == DHCP_DEVICE_CHECK_AGG_RX || + check_type == DHCP_DEVICE_CHECK_AGG_TX || + check_type == DHCP_DEVICE_CHECK_AGG_RX_V6 || + check_type == DHCP_DEVICE_CHECK_AGG_TX_V6; + if (!aggregate_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; } @@ -395,27 +384,17 @@ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_ return dhcp_device_check_positive_health_v6(ifname); case DHCP_DEVICE_CHECK_NEGATIVE_V6: return dhcp_device_check_negative_health_v6(ifname); - case DHCP_DEVICE_CHECK_AGG_EQUAL_RX: - return dhcp_device_check_agg_equal_rx(ifname); - case DHCP_DEVICE_CHECK_AGG_EQUAL_TX: - return dhcp_device_check_agg_equal_tx(ifname); - case DHCP_DEVICE_CHECK_AGG_EQUAL_RX_V6: - return dhcp_device_check_agg_equal_rx_v6(ifname); - case DHCP_DEVICE_CHECK_AGG_EQUAL_TX_V6: - return dhcp_device_check_agg_equal_tx_v6(ifname); - case DHCP_DEVICE_CHECK_AGG_MULTIPLE_RX: - return dhcp_device_check_agg_multiple_rx(ifname); - case DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX: - return dhcp_device_check_agg_multiple_tx(ifname); - case DHCP_DEVICE_CHECK_AGG_MULTIPLE_RX_V6: - return dhcp_device_check_agg_multiple_rx_v6(ifname); - case DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX_V6: - return dhcp_device_check_agg_multiple_tx_v6(ifname); + case DHCP_DEVICE_CHECK_AGG_RX: + return check_aggregate_health(ifname, rx_sock, (const int *)monitored_msgs, monitored_msg_sz); + case DHCP_DEVICE_CHECK_AGG_TX: + return check_aggregate_health(ifname, tx_sock, (const int *)monitored_msgs, monitored_msg_sz); + case DHCP_DEVICE_CHECK_AGG_RX_V6: + return check_aggregate_health(ifname, rx_sock_v6, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); + case DHCP_DEVICE_CHECK_AGG_TX_V6: + return check_aggregate_health(ifname, tx_sock_v6, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); default: - break; + return DHCP_MON_STATUS_UNHEALTHY; } - - return DHCP_MON_STATUS_UNHEALTHY; } int initialize_intf_mac_and_ip_addr(dhcp_device_context_t *context) diff --git a/src/dhcp_device.h b/src/dhcp_device.h index 5d4b00721..7ca677850 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -136,14 +136,10 @@ typedef enum DHCP_DEVICE_CHECK_POSITIVE, /** Validate that received DORA packets are relayed */ DHCP_DEVICE_CHECK_NEGATIVE_V6, /** Presence of relayed DHCPv6 packets activity is flagged as unhealthy state */ DHCP_DEVICE_CHECK_POSITIVE_V6, /** Validate that received SARR packets are relayed */ - DHCP_DEVICE_CHECK_AGG_EQUAL_RX, /** Validate that aggregate device rx counters equal sum of member interfaces rx counters */ - DHCP_DEVICE_CHECK_AGG_EQUAL_TX, /** Validate that aggregate device tx counters equal sum of member interfaces tx counters */ - DHCP_DEVICE_CHECK_AGG_EQUAL_RX_V6, /** Validate that aggregate device rx counters equal sum of member interfaces rx counters for IPv6 */ - DHCP_DEVICE_CHECK_AGG_EQUAL_TX_V6, /** Validate that aggregate device tx counters equal sum of member interfaces tx counters for IPv6 */ - DHCP_DEVICE_CHECK_AGG_MULTIPLE_RX, /** Validate that aggregate device rx counters are multiple of member interfaces rx counters */ - DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX, /** Validate that aggregate device tx counters are multiple of member interfaces tx counters */ - DHCP_DEVICE_CHECK_AGG_MULTIPLE_RX_V6, /** Validate that aggregate device rx counters are multiple of member interfaces rx counters for IPv6 */ - DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX_V6 /** Validate that aggregate device tx counters are multiple of member interfaces tx counters for IPv6 */ + DHCP_DEVICE_CHECK_AGG_RX, /** Compare IPv4 RX on a parent interface with its member aggregate */ + DHCP_DEVICE_CHECK_AGG_TX, /** Compare IPv4 TX on a parent interface with its member aggregate */ + DHCP_DEVICE_CHECK_AGG_RX_V6, /** Compare IPv6 RX on a parent interface with its member aggregate */ + DHCP_DEVICE_CHECK_AGG_TX_V6 /** Compare IPv6 TX on a parent interface with its member aggregate */ } dhcp_device_check_t; /** Monitored DHCP message type */ diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index 3cedf09b0..93001dc8f 100644 --- a/src/dhcp_mon.cpp +++ b/src/dhcp_mon.cpp @@ -524,6 +524,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(); + initialize_dhcp_relay_health(); 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..8ea39d3ff 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include "health_check.h" @@ -22,202 +23,98 @@ int dhcp_unhealthy_max_count = 10; extern std::string mgmt_ifname; extern std::string agg_dev_all; -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 dhcp_mon_status_t check_mgmt_health() -{ - if (mgmt_ifname.size() > 0) { - return dhcp_device_get_status(mgmt_ifname, DHCP_DEVICE_CHECK_NEGATIVE); - } - return DHCP_MON_STATUS_HEALTHY; -} +static const char relay_disparity_error[] = + "dhcpmon detected DHCPv4/v6 packets received but none transmitted for intf: %s. Duration: %d (sec)"; +static const char mgmt_error[] = + "dhcpmon detected DHCP packets traveling through mgmt interface (please check BGP routes.)" + " Intf: %s. Duration: %d (sec)"; +static const char agg_rx_disparity_error[] = + "dhcpmon detected an IPv4 RX disparity between interface %s and the aggregate of its member interface counters." + " Duration: %d (sec)"; +static const char agg_tx_disparity_error[] = + "dhcpmon detected an IPv4 TX disparity between interface %s and the aggregate of its member interface counters." + " Duration: %d (sec)"; +static const char agg_rx_v6_disparity_error[] = + "dhcpmon detected an IPv6 RX disparity between interface %s and the aggregate of its member interface counters." + " Duration: %d (sec)"; +static const char agg_tx_v6_disparity_error[] = + "dhcpmon detected an IPv6 TX disparity between interface %s and the aggregate of its member interface counters." + " Duration: %d (sec)"; +/** + * @code alert_dhcp_relay_disparity(duration); + * + * @brief Publish the existing DHCP relay disparity event + * + * @param duration Unhealthy duration in seconds + * + * @return None + */ static void alert_dhcp_relay_disparity(int duration) { event_params_t params = {{ "vlan", agg_dev_all}, { "duration", std::to_string(duration)}}; event_publish(g_events_handle, "dhcp-relay-disparity", ¶ms); } -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", - duration, agg_dev_all.c_str()); -} +static std::vector state_data; -static void log_mgmt_error(int duration) +void initialize_dhcp_relay_health() { - syslog(LOG_ALERT, "dhcpmon detected DHCP packets traveling through mgmt interface (please check BGP routes.)" - " Duration: %d (sec) for intf: %s", - duration, mgmt_ifname.c_str()); -} + state_data.clear(); -static dhcp_mon_status_t check_agg_health_v6() -{ - return dhcp_device_get_status(agg_dev_all, DHCP_DEVICE_CHECK_POSITIVE_V6); -} - -static dhcp_mon_status_t check_mgmt_health_v6() -{ + state_data.push_back({agg_dev_all, DHCP_DEVICE_CHECK_POSITIVE, alert_dhcp_relay_disparity, + relay_disparity_error, 0}); if (mgmt_ifname.size() > 0) { - return dhcp_device_get_status(mgmt_ifname, DHCP_DEVICE_CHECK_NEGATIVE_V6); - } - return DHCP_MON_STATUS_HEALTHY; -} - -static dhcp_mon_status_t check_per_interface_rx_health() -{ - for (const auto &[vlan, _] : rev_vlan_map) { - if (dhcp_device_get_status(vlan, DHCP_DEVICE_CHECK_AGG_EQUAL_RX) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } - } - for (const auto &[portchan, _] : rev_portchan_map) { - if (dhcp_device_get_status(portchan, DHCP_DEVICE_CHECK_AGG_EQUAL_RX) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } - } - return DHCP_MON_STATUS_HEALTHY; -} - -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); -} - -static dhcp_mon_status_t check_per_interface_tx_health() -{ - for (const auto &[vlan, _] : rev_vlan_map) { - if (dhcp_device_get_status(vlan, DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } + state_data.push_back({mgmt_ifname, DHCP_DEVICE_CHECK_NEGATIVE, NULL, mgmt_error, 0}); } - for (const auto &[portchan, _] : rev_portchan_map) { - if (dhcp_device_get_status(portchan, DHCP_DEVICE_CHECK_AGG_EQUAL_TX) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } + state_data.push_back({agg_dev_all, DHCP_DEVICE_CHECK_POSITIVE_V6, alert_dhcp_relay_disparity, + relay_disparity_error, 0}); + if (mgmt_ifname.size() > 0) { + state_data.push_back({mgmt_ifname, DHCP_DEVICE_CHECK_NEGATIVE_V6, NULL, mgmt_error, 0}); } - return DHCP_MON_STATUS_HEALTHY; -} - -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); -} -static dhcp_mon_status_t check_per_interface_rx_health_v6() -{ for (const auto &[vlan, _] : rev_vlan_map) { - if (dhcp_device_get_status(vlan, DHCP_DEVICE_CHECK_AGG_EQUAL_RX_V6) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } + state_data.push_back({vlan, DHCP_DEVICE_CHECK_AGG_RX, NULL, agg_rx_disparity_error, 0}); + state_data.push_back({vlan, DHCP_DEVICE_CHECK_AGG_TX, NULL, agg_tx_disparity_error, 0}); + state_data.push_back({vlan, DHCP_DEVICE_CHECK_AGG_RX_V6, NULL, agg_rx_v6_disparity_error, 0}); + state_data.push_back({vlan, DHCP_DEVICE_CHECK_AGG_TX_V6, NULL, agg_tx_v6_disparity_error, 0}); } - for (const auto &[portchan, _] : rev_portchan_map) { - if (dhcp_device_get_status(portchan, DHCP_DEVICE_CHECK_AGG_EQUAL_RX_V6) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } - } - return DHCP_MON_STATUS_HEALTHY; -} -static dhcp_mon_status_t check_per_interface_tx_health_v6() -{ - for (const auto &[vlan, _] : rev_vlan_map) { - if (dhcp_device_get_status(vlan, DHCP_DEVICE_CHECK_AGG_MULTIPLE_TX_V6) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } - } for (const auto &[portchan, _] : rev_portchan_map) { - if (dhcp_device_get_status(portchan, DHCP_DEVICE_CHECK_AGG_EQUAL_TX_V6) == DHCP_MON_STATUS_UNHEALTHY) { - return DHCP_MON_STATUS_UNHEALTHY; - } + state_data.push_back({portchan, DHCP_DEVICE_CHECK_AGG_RX, NULL, agg_rx_disparity_error, 0}); + state_data.push_back({portchan, DHCP_DEVICE_CHECK_AGG_TX, NULL, agg_tx_disparity_error, 0}); + state_data.push_back({portchan, DHCP_DEVICE_CHECK_AGG_RX_V6, NULL, agg_rx_v6_disparity_error, 0}); + state_data.push_back({portchan, DHCP_DEVICE_CHECK_AGG_TX_V6, NULL, agg_tx_v6_disparity_error, 0}); } - return DHCP_MON_STATUS_HEALTHY; } -/** 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] = { - .check_health = check_agg_health_v6, - .alert = alert_dhcp_relay_disparity, - .log = log_agg_error, - .count = 0, - }, - [3] = { - .check_health = check_mgmt_health_v6, - .log = log_mgmt_error, - .count = 0, - }, - [4] = { - .check_health = check_per_interface_rx_health, - .log = log_agg_per_interface_rx_error, - .count = 0, - }, - [5] = { - .check_health = check_per_interface_tx_health, - .log = log_agg_per_interface_tx_error, - .count = 0, - }, - [6] = { - .check_health = check_per_interface_rx_health_v6, - .log = log_agg_per_interface_rx_error, - .count = 0, - }, - [7] = { - .check_health = check_per_interface_tx_health_v6, - .log = log_agg_per_interface_tx_error, - .count = 0, - }, -}; - -static size_t state_data_sz = sizeof(state_data) / sizeof(*state_data); - void check_dhcp_relay_health() { syslog_debug(LOG_INFO, "Checking DHCP relay health"); - for (uint8_t i = 0; i < state_data_sz; i++) { - dhcp_mon_status_t dhcp_mon_status = state_data[i].check_health(); + for (auto &state : state_data) { + dhcp_mon_status_t dhcp_mon_status = dhcp_device_get_status(state.ifname, state.check_type); switch (dhcp_mon_status) { case DHCP_MON_STATUS_UNHEALTHY: - if (++state_data[i].count > dhcp_unhealthy_max_count) { - int duration = state_data[i].count * window_interval_sec; + if (++state.count > dhcp_unhealthy_max_count) { + int duration = state.count * window_interval_sec; - if (state_data[i].alert) { - state_data[i].alert(duration); - } - if (state_data[i].log) { - state_data[i].log(duration); + if (state.alert) { + state.alert(duration); } + syslog(LOG_ALERT, state.error_format, state.ifname.c_str(), duration); } break; case DHCP_MON_STATUS_HEALTHY: - state_data[i].count = 0; + state.count = 0; break; case DHCP_MON_STATUS_INDETERMINATE: - if (state_data[i].count) { - state_data[i].count++; + if (state.count) { + state.count++; } break; default: diff --git a/src/health_check.h b/src/health_check.h index 552dd528c..16243438a 100644 --- a/src/health_check.h +++ b/src/health_check.h @@ -9,14 +9,16 @@ #include "dhcp_device.h" #include +#include /** DHCP device/interface state */ typedef struct { - dhcp_mon_status_t (*check_health)(); /** check function */ - 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 */ + std::string ifname; /** interface to check */ + dhcp_device_check_t check_type; /** check to apply */ + void (*alert)(int duration); /** alert function */ + const char *error_format; /** threshold error format */ + int count; /** consecutive unhealthy checks */ } dhcp_mon_state_t; extern event_handle_t g_events_handle; @@ -26,7 +28,18 @@ extern int window_interval_sec; extern int dhcp_unhealthy_max_count; /** - * @code check_dhcp_relay_health(state_data); + * @code initialize_dhcp_relay_health(); + * + * @brief Populate health states from discovered VLAN and PortChannel members + * + * @param none + * + * @return none + */ +void initialize_dhcp_relay_health(); + +/** + * @code check_dhcp_relay_health(); * * @brief check DHCP relay overall health * From 80f684f111a34c9fe94a4a1dafb0ce4f21ddcde5 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 16:46:51 +1000 Subject: [PATCH 15/23] [dhcpmon]: Disable invalid DHCPv6 disparity check Return indeterminate for same-message-type DHCPv6 relay health because client and relay message types differ across the relay boundary. 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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index e7e22ea2a..4385e5d96 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -120,10 +120,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; } /** From eb6df2195c375a529bcf85c45e3ce1021c2ec41f Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Sun, 26 Jul 2026 17:29:48 +1000 Subject: [PATCH 16/23] [dhcpmon]: Correct DHCPv6 health documentation Document that the disabled same-type check always returns indeterminate and takes no interface parameter. 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, 3 insertions(+), 6 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 4385e5d96..c550733e1 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -113,12 +113,9 @@ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &if } /** - * @code dhcp_device_check_positive_health_v6(ifname); - * @brief Check that DHCPv6 relayed messages are being transmitted out of this interface/dev - * using its counters. The interface is positively healthy if there are DHCPv6 message - * travelling through it. - * @param ifname interface name - * @return DHCP_MON_STATUS_HEALTHY, DHCP_MON_STATUS_UNHEALTHY, or DHCP_MON_STATUS_INDETERMINATE + * @code dhcp_device_check_positive_health_v6(); + * @brief Same-message-type RX/TX comparison is not valid across the DHCPv6 relay boundary. + * @return DHCP_MON_STATUS_INDETERMINATE */ static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &) { From 78dbef50bbcf563bf048be00c4826c20be0bb350 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 16:06:15 +1000 Subject: [PATCH 17/23] [dhcpmon]: Keep DHCPv6 disparity rationale in docstring 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 | 1 - 1 file changed, 1 deletion(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index c550733e1..0e95645da 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -119,7 +119,6 @@ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &if */ static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &) { - // Client and relay DHCPv6 message types differ across the relay boundary. return DHCP_MON_STATUS_INDETERMINATE; } From ceea23e55e4f3d12bd5d11343418a1a617f8ed1d Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 21:34:59 +1000 Subject: [PATCH 18/23] [dhcpmon]: Validate DHCPv6 relay transformations Require selected DHCPv6 client or nested-relay input to produce Relay-Forward output, and Relay-Reply input to produce a valid downstream reply type. Compare activity rather than magnitude so multiple configured servers remain valid. 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 | 57 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 0e95645da..456384fb0 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -51,6 +51,26 @@ const dhcpv6_message_type_t monitored_v6_msgs[] = { uint8_t monitored_v6_msg_sz = sizeof(monitored_v6_msgs) / sizeof(*monitored_v6_msgs); +static const dhcpv6_message_type_t relay_forward_rx_msgs[] = { + DHCPV6_MESSAGE_TYPE_SOLICIT, + DHCPV6_MESSAGE_TYPE_REQUEST, + DHCPV6_MESSAGE_TYPE_RELAY_FORW +}; + +static const dhcpv6_message_type_t relay_forward_tx_msgs[] = { + DHCPV6_MESSAGE_TYPE_RELAY_FORW +}; + +static const dhcpv6_message_type_t relay_reply_rx_msgs[] = { + DHCPV6_MESSAGE_TYPE_RELAY_REPL +}; + +static const dhcpv6_message_type_t relay_reply_tx_msgs[] = { + DHCPV6_MESSAGE_TYPE_ADVERTISE, + DHCPV6_MESSAGE_TYPE_REPLY, + DHCPV6_MESSAGE_TYPE_RELAY_REPL +}; + const char *intf_type_name[DHCP_DEVICE_INTF_TYPE_COUNT] = { [DHCP_DEVICE_INTF_TYPE_UPLINK] = "uplink (north)", [DHCP_DEVICE_INTF_TYPE_DOWNLINK] = "downlink (south)", @@ -112,16 +132,6 @@ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &if DHCP_MON_STATUS_UNHEALTHY : DHCP_MON_STATUS_HEALTHY; } -/** - * @code dhcp_device_check_positive_health_v6(); - * @brief Same-message-type RX/TX comparison is not valid across the DHCPv6 relay boundary. - * @return DHCP_MON_STATUS_INDETERMINATE - */ -static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &) -{ - return DHCP_MON_STATUS_INDETERMINATE; -} - /** * @code check_counter_increased(ifname, sock, monitored_msgs, monitored_msg_cnt); * @@ -149,6 +159,33 @@ static bool check_counter_increased(const std::string &ifname, int sock, const i return false; } +/** + * @code dhcp_device_check_positive_health_v6(ifname); + * @brief Check that DHCPv6 client or relay input produces the corresponding transformed relay output. + * @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) +{ + const bool forward_rx = check_counter_increased( + ifname, rx_sock_v6, (const int *)relay_forward_rx_msgs, + sizeof(relay_forward_rx_msgs) / sizeof(*relay_forward_rx_msgs)); + const bool forward_tx = check_counter_increased( + ifname, tx_sock_v6, (const int *)relay_forward_tx_msgs, + sizeof(relay_forward_tx_msgs) / sizeof(*relay_forward_tx_msgs)); + const bool reply_rx = check_counter_increased( + ifname, rx_sock_v6, (const int *)relay_reply_rx_msgs, + sizeof(relay_reply_rx_msgs) / sizeof(*relay_reply_rx_msgs)); + const bool reply_tx = check_counter_increased( + ifname, tx_sock_v6, (const int *)relay_reply_tx_msgs, + sizeof(relay_reply_tx_msgs) / sizeof(*relay_reply_tx_msgs)); + + if ((forward_rx && !forward_tx) || (reply_rx && !reply_tx)) { + return DHCP_MON_STATUS_UNHEALTHY; + } + return forward_rx || reply_rx ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; +} + /** * @code dhcp_device_check_negative_health(ifname); * From 90d5b3eb1fe6e14405150bbfdb70bf68bc2e3c00 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 29 Jul 2026 00:51:40 +1000 Subject: [PATCH 19/23] [dhcpmon]: Align monitored DHCPv6 health names Name the transformation subsets as monitored_v6 groups, restore explicit SARR terminology, and place the IPv4 and IPv6 positive-health functions together. 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 | 54 ++++++++++++++++++++++----------------------- src/dhcp_device.h | 2 +- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 456384fb0..a9ed6d165 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -51,21 +51,21 @@ const dhcpv6_message_type_t monitored_v6_msgs[] = { uint8_t monitored_v6_msg_sz = sizeof(monitored_v6_msgs) / sizeof(*monitored_v6_msgs); -static const dhcpv6_message_type_t relay_forward_rx_msgs[] = { +static const dhcpv6_message_type_t monitored_v6_forward_rx_msgs[] = { DHCPV6_MESSAGE_TYPE_SOLICIT, DHCPV6_MESSAGE_TYPE_REQUEST, DHCPV6_MESSAGE_TYPE_RELAY_FORW }; -static const dhcpv6_message_type_t relay_forward_tx_msgs[] = { +static const dhcpv6_message_type_t monitored_v6_forward_tx_msgs[] = { DHCPV6_MESSAGE_TYPE_RELAY_FORW }; -static const dhcpv6_message_type_t relay_reply_rx_msgs[] = { +static const dhcpv6_message_type_t monitored_v6_reply_rx_msgs[] = { DHCPV6_MESSAGE_TYPE_RELAY_REPL }; -static const dhcpv6_message_type_t relay_reply_tx_msgs[] = { +static const dhcpv6_message_type_t monitored_v6_reply_tx_msgs[] = { DHCPV6_MESSAGE_TYPE_ADVERTISE, DHCPV6_MESSAGE_TYPE_REPLY, DHCPV6_MESSAGE_TYPE_RELAY_REPL @@ -118,20 +118,6 @@ static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock return false; } -/** - * @code dhcp_device_check_positive_health(ifname); - * @brief Check that DHCP relayed messages are being transmitted out of this interface/dev - * using its counters. The interface is positively healthy if there are DHCP message - * travelling through it. - * @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(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; -} - /** * @code check_counter_increased(ifname, sock, monitored_msgs, monitored_msg_cnt); * @@ -159,26 +145,40 @@ static bool check_counter_increased(const std::string &ifname, int sock, const i return false; } +/** + * @code dhcp_device_check_positive_health(ifname); + * @brief Check that DHCP relayed messages are being transmitted out of this interface/dev + * using its counters. The interface is positively healthy if there are DHCP message + * travelling through it. + * @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(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; +} + /** * @code dhcp_device_check_positive_health_v6(ifname); - * @brief Check that DHCPv6 client or relay input produces the corresponding transformed relay output. + * @brief Check that SARR or nested relay input produces the corresponding DHCPv6 relay output. * @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) { const bool forward_rx = check_counter_increased( - ifname, rx_sock_v6, (const int *)relay_forward_rx_msgs, - sizeof(relay_forward_rx_msgs) / sizeof(*relay_forward_rx_msgs)); + ifname, rx_sock_v6, (const int *)monitored_v6_forward_rx_msgs, + sizeof(monitored_v6_forward_rx_msgs) / sizeof(*monitored_v6_forward_rx_msgs)); const bool forward_tx = check_counter_increased( - ifname, tx_sock_v6, (const int *)relay_forward_tx_msgs, - sizeof(relay_forward_tx_msgs) / sizeof(*relay_forward_tx_msgs)); + ifname, tx_sock_v6, (const int *)monitored_v6_forward_tx_msgs, + sizeof(monitored_v6_forward_tx_msgs) / sizeof(*monitored_v6_forward_tx_msgs)); const bool reply_rx = check_counter_increased( - ifname, rx_sock_v6, (const int *)relay_reply_rx_msgs, - sizeof(relay_reply_rx_msgs) / sizeof(*relay_reply_rx_msgs)); + ifname, rx_sock_v6, (const int *)monitored_v6_reply_rx_msgs, + sizeof(monitored_v6_reply_rx_msgs) / sizeof(*monitored_v6_reply_rx_msgs)); const bool reply_tx = check_counter_increased( - ifname, tx_sock_v6, (const int *)relay_reply_tx_msgs, - sizeof(relay_reply_tx_msgs) / sizeof(*relay_reply_tx_msgs)); + ifname, tx_sock_v6, (const int *)monitored_v6_reply_tx_msgs, + sizeof(monitored_v6_reply_tx_msgs) / sizeof(*monitored_v6_reply_tx_msgs)); if ((forward_rx && !forward_tx) || (reply_rx && !reply_tx)) { return DHCP_MON_STATUS_UNHEALTHY; diff --git a/src/dhcp_device.h b/src/dhcp_device.h index 7ca677850..e8f4411f6 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -135,7 +135,7 @@ typedef enum DHCP_DEVICE_CHECK_NEGATIVE, /** Presence of relayed DHCP packets activity is flagged as unhealthy state */ DHCP_DEVICE_CHECK_POSITIVE, /** Validate that received DORA packets are relayed */ DHCP_DEVICE_CHECK_NEGATIVE_V6, /** Presence of relayed DHCPv6 packets activity is flagged as unhealthy state */ - DHCP_DEVICE_CHECK_POSITIVE_V6, /** Validate that received SARR packets are relayed */ + DHCP_DEVICE_CHECK_POSITIVE_V6, /** Validate SARR and DHCPv6 relay-wrapper transformations */ DHCP_DEVICE_CHECK_AGG_RX, /** Compare IPv4 RX on a parent interface with its member aggregate */ DHCP_DEVICE_CHECK_AGG_TX, /** Compare IPv4 TX on a parent interface with its member aggregate */ DHCP_DEVICE_CHECK_AGG_RX_V6, /** Compare IPv6 RX on a parent interface with its member aggregate */ From d9ace6d027d0a87dac54c569e8df991bcf7efa18 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 29 Jul 2026 01:29:48 +1000 Subject: [PATCH 20/23] [dhcpmon]: Document positive health message sets State the exact DORA same-type relationship and DHCPv6 forward/reply transformation groups checked by the positive-health functions. 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 | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index a9ed6d165..656e6b1af 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -147,9 +147,7 @@ static bool check_counter_increased(const std::string &ifname, int sock, const i /** * @code dhcp_device_check_positive_health(ifname); - * @brief Check that DHCP relayed messages are being transmitted out of this interface/dev - * using its counters. The interface is positively healthy if there are DHCP message - * travelling through it. + * @brief Check that RX Discover, Offer, Request, and Ack activity has matching same-type TX activity. * @param ifname interface name * @return DHCP_MON_STATUS_HEALTHY, DHCP_MON_STATUS_UNHEALTHY, or DHCP_MON_STATUS_INDETERMINATE */ @@ -161,7 +159,8 @@ static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &if /** * @code dhcp_device_check_positive_health_v6(ifname); - * @brief Check that SARR or nested relay input produces the corresponding DHCPv6 relay output. + * @brief Check that RX Solicit, Request, or Relay-Forward activity has TX Relay-Forward activity, + * and RX Relay-Reply activity has TX Advertise, Reply, or Relay-Reply activity. * @param ifname interface name * @return DHCP_MON_STATUS_HEALTHY, DHCP_MON_STATUS_UNHEALTHY, or DHCP_MON_STATUS_INDETERMINATE */ From 6c40cf3fb3d1ec0cfdbbee58ee2708d2b6b45561 Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Wed, 29 Jul 2026 03:00:56 +1000 Subject: [PATCH 21/23] [dhcpmon]: Keep single DHCPv6 relay types direct Use monitored arrays for multi-type DHCPv6 groups and direct Relay-Forward TX and Relay-Reply RX counter lookups symmetrically. Move the shared counter-delta helper into this base PR for later 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.cpp | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 656e6b1af..386af194b 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -57,14 +57,6 @@ static const dhcpv6_message_type_t monitored_v6_forward_rx_msgs[] = { DHCPV6_MESSAGE_TYPE_RELAY_FORW }; -static const dhcpv6_message_type_t monitored_v6_forward_tx_msgs[] = { - DHCPV6_MESSAGE_TYPE_RELAY_FORW -}; - -static const dhcpv6_message_type_t monitored_v6_reply_rx_msgs[] = { - DHCPV6_MESSAGE_TYPE_RELAY_REPL -}; - static const dhcpv6_message_type_t monitored_v6_reply_tx_msgs[] = { DHCPV6_MESSAGE_TYPE_ADVERTISE, DHCPV6_MESSAGE_TYPE_REPLY, @@ -118,6 +110,21 @@ static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock return false; } +/** + * @code get_counter_delta(ifname, sock, msg_type); + * @brief Get the increase in one message-type counter since the last snapshot. + * @param ifname interface name + * @param sock socket containing the counter + * @param msg_type message type + * @return counter increase since the last snapshot + */ +static uint64_t get_counter_delta(const std::string &ifname, int sock, int msg_type) +{ + const sock_info_t &sock_info = sock_mgr_get_sock_info(sock); + return sock_info.all_counters.at(ifname).at(msg_type) - + sock_info.all_counters_snapshot.at(ifname).at(msg_type); +} + /** * @code check_counter_increased(ifname, sock, monitored_msgs, monitored_msg_cnt); * @@ -132,13 +139,9 @@ static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock */ static bool check_counter_increased(const std::string &ifname, int sock, const int *monitored_msgs, size_t monitored_msg_cnt) { - const sock_info_t &sock_info = sock_mgr_get_sock_info(sock); - const counter_t &counters = sock_info.all_counters.at(ifname); - const counter_t &counters_snapshot = sock_info.all_counters_snapshot.at(ifname); - // true if any counter has increased for (size_t i = 0; i < monitored_msg_cnt; i++) { - if (counters.at(monitored_msgs[i]) > counters_snapshot.at(monitored_msgs[i])) { + if (get_counter_delta(ifname, sock, monitored_msgs[i]) > 0) { return true; } } @@ -169,12 +172,8 @@ static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string const bool forward_rx = check_counter_increased( ifname, rx_sock_v6, (const int *)monitored_v6_forward_rx_msgs, sizeof(monitored_v6_forward_rx_msgs) / sizeof(*monitored_v6_forward_rx_msgs)); - const bool forward_tx = check_counter_increased( - ifname, tx_sock_v6, (const int *)monitored_v6_forward_tx_msgs, - sizeof(monitored_v6_forward_tx_msgs) / sizeof(*monitored_v6_forward_tx_msgs)); - const bool reply_rx = check_counter_increased( - ifname, rx_sock_v6, (const int *)monitored_v6_reply_rx_msgs, - sizeof(monitored_v6_reply_rx_msgs) / sizeof(*monitored_v6_reply_rx_msgs)); + const bool forward_tx = get_counter_delta(ifname, tx_sock_v6, DHCPV6_MESSAGE_TYPE_RELAY_FORW) > 0; + const bool reply_rx = get_counter_delta(ifname, rx_sock_v6, DHCPV6_MESSAGE_TYPE_RELAY_REPL) > 0; const bool reply_tx = check_counter_increased( ifname, tx_sock_v6, (const int *)monitored_v6_reply_tx_msgs, sizeof(monitored_v6_reply_tx_msgs) / sizeof(*monitored_v6_reply_tx_msgs)); From 72bf5fe3feb7d0678bbbcf11edfd644ec5c3ad0a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 20:06:30 +1000 Subject: [PATCH 22/23] [dhcpmon]: Validate downstream reply fan-out Allow first-hop DHCPOFFER, DHCPACK, and DHCPNAK profile validation without a destination-IP assumption. Require each IPv4 VLAN TX packet to appear on either one direct member for unicast or every direct member for broadcast; keep all other parent/member comparisons exact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 39f979be-d826-4d5c-949a-f20abb58bb83 Signed-off-by: Xichen96 --- src/dhcp_check_profile_relay.cpp | 4 +-- src/dhcp_device.cpp | 55 ++++++++++++++------------------ 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/src/dhcp_check_profile_relay.cpp b/src/dhcp_check_profile_relay.cpp index a5635066f..41088555e 100644 --- a/src/dhcp_check_profile_relay.cpp +++ b/src/dhcp_check_profile_relay.cpp @@ -62,14 +62,14 @@ dhcp_check_profile_t dhcp_check_profile_first_relay_rx = { }; // DHCP messages sent to client -// Relay sends reply packets to client with broadcast ip, and giaddr remains the first relay identifier. +// Relay replies may use broadcast or unicast IP based on the client's broadcast flag, not a relay decision. +// SONiC relay supports broadcast only while ISC supports both; giaddr remains the first relay identifier. // In single-ToR, giaddr_ip and vlan_ip are the same downstream VLAN SVI address. In dualtor, giaddr_ip is // Loopback0 for the server-facing relay identity, but client-facing replies are still sent from the downstream // VLAN SVI. Therefore src ip should be vlan_ip, while giaddr should remain giaddr_ip. static dhcp_msg_check_profile_t tx_first_relay_reply = { {DHCP_CHECK_INTF_TYPE, (const void *)(new std::vector{DHCP_DEVICE_INTF_TYPE_DOWNLINK, DHCP_DEVICE_INTF_TYPE_MGMT})}, {DHCP_CHECK_SRC_IP, (const void *)(new std::vector{&vlan_ip})}, - {DHCP_CHECK_DST_IP, (const void *)(new std::vector{&broadcast_ip})}, {DHCP_CHECK_GIADDR, (const void *)(new std::vector{&giaddr_ip})}, }; diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 386af194b..5e8d4ae8a 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -219,19 +219,18 @@ static dhcp_mon_status_t dhcp_device_check_negative_health_v6(const std::string } /** - * @code check_counters_delta_expected(ifname, other_ifname, sock, ratio, monitored_msgs, monitored_msg_cnt); - * @brief Check if the delta of counters between current and snapshot for given message types - * match expectation between two interfaces with a given ratio. + * @code check_counters_delta_expected(ifname, other_ifname, sock, member_count, monitored_msgs, monitored_msg_cnt); + * @brief Check if counter deltas match between a parent and its member aggregate. * @param ifname interface name * @param other_ifname other interface name * @param sock socket - * @param ratio expected ratio between two interfaces, ratio = other_ifname / ifname + * @param member_count expected member observations per broadcast packet; one means exact equality * @param monitored_msgs array of monitored message types * @param monitored_msg_cnt number of monitored message types - * @return true if deltas are equal with given ratio, false otherwise + * @return true if deltas match the expected relationship, false otherwise */ static bool check_counters_delta_expected(const std::string &ifname, const std::string &other_ifname, int sock, - size_t ratio, const int *monitored_msgs, size_t monitored_msg_cnt) + size_t member_count, const int *monitored_msgs, size_t monitored_msg_cnt) { const sock_info_t &sock_info = sock_mgr_get_sock_info(sock); const counter_t &counters = sock_info.all_counters.at(ifname); @@ -239,37 +238,28 @@ static bool check_counters_delta_expected(const std::string &ifname, const std:: const counter_t &other_counters = sock_info.all_counters.at(other_ifname); const counter_t &other_counters_snapshot = sock_info.all_counters_snapshot.at(other_ifname); - // for every delta increase in ifname, there is delta * ratio increase in other ifname for (size_t i = 0; i < monitored_msg_cnt; i++) { 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) { + if (member_count <= 1) { + if (delta != other_delta) { + return false; + } + continue; + } + + if (other_delta < delta) { + return false; + } + uint64_t additional_member_observations = other_delta - delta; + if (additional_member_observations % (member_count - 1) != 0 || + additional_member_observations / (member_count - 1) > delta) { return false; } } return true; } -/** - * @code get_aggregate_ratio(ifname, sock); - * - * @brief Get the expected aggregate ratio for an interface and packet direction - * - * @param ifname Parent interface name - * @param sock Socket identifying the protocol family and direction - * - * @return One for RX and PortChannel TX; direct VLAN member count for VLAN TX - */ -static size_t get_aggregate_ratio(const std::string &ifname, int sock) -{ - // RX crosses one edge. VLAN TX is observed on every direct member; PortChannel TX is observed on one member. - if (sock_mgr_get_sock_info(sock).is_rx) { - return 1; - } - const auto vlan = rev_vlan_map.find(ifname); - return vlan == rev_vlan_map.end() ? 1 : vlan->second.size(); -} - /** * @code check_aggregate_health(ifname, sock, monitored_msgs, monitored_msg_cnt); * @@ -280,19 +270,22 @@ static size_t get_aggregate_ratio(const std::string &ifname, int sock) * @param monitored_msgs Message types to compare * @param monitored_msg_cnt Number of message types to compare * - * @return HEALTHY when idle or matching, UNHEALTHY when the expected ratio does not match + * @return HEALTHY when idle or matching, UNHEALTHY when the expected relationship does not match */ static dhcp_mon_status_t check_aggregate_health(const std::string &ifname, int sock, const int *monitored_msgs, size_t monitored_msg_cnt) { const std::string agg_ifname = get_agg_counter_ifname(ifname); + const auto vlan = rev_vlan_map.find(ifname); + // Each IPv4 VLAN TX packet is observed on one member for unicast or all direct members for broadcast. + const size_t member_count = sock == tx_sock && vlan != rev_vlan_map.end() && !vlan->second.empty() ? + vlan->second.size() : 1; 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_HEALTHY; } return check_counters_delta_expected( - ifname, agg_ifname, sock, get_aggregate_ratio(ifname, sock), - monitored_msgs, monitored_msg_cnt) ? + ifname, agg_ifname, sock, member_count, monitored_msgs, monitored_msg_cnt) ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; } From 12257b72ff0a506aae848801db69810645f3f92a Mon Sep 17 00:00:00 2001 From: Xichen96 Date: Tue, 28 Jul 2026 21:41:07 +1000 Subject: [PATCH 23/23] [dhcpmon]: Validate configured server fan-out Read configured DHCP server counts from native and legacy CONFIG_DB schemas. Add separate IPv4 and IPv6 group-3 health states that compare relay forward input with configured server fan-out without changing group-1 relay-flow or group-2 management 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.cpp | 62 +++++++++++++++++++++++++++++++++++++++++++- src/dhcp_device.h | 4 ++- src/health_check.cpp | 8 ++++++ src/util.cpp | 39 ++++++++++++++++++++++++++++ src/util.h | 8 ++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 5e8d4ae8a..a621fcf93 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -40,6 +40,11 @@ const dhcp_message_type_t monitored_msgs[] = { uint8_t monitored_msg_sz = sizeof(monitored_msgs) / sizeof(*monitored_msgs); +static const dhcp_message_type_t monitored_v4_forward_msgs[] = { + DHCP_MESSAGE_TYPE_DISCOVER, + DHCP_MESSAGE_TYPE_REQUEST +}; + const dhcpv6_message_type_t monitored_v6_msgs[] = { DHCPV6_MESSAGE_TYPE_SOLICIT, DHCPV6_MESSAGE_TYPE_ADVERTISE, @@ -184,6 +189,55 @@ static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string return forward_rx || reply_rx ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; } +/** + * @code dhcp_device_check_server_fanout(ifname); + * @brief Check that each DHCPv4 forward packet is transmitted once per configured server. + * @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_server_fanout(const std::string &ifname) +{ + const size_t server_count = get_configured_dhcp_server_count(false); + if (server_count == 0) { + return DHCP_MON_STATUS_INDETERMINATE; + } + + bool has_activity = false; + for (const auto msg_type : monitored_v4_forward_msgs) { + const uint64_t rx_delta = get_counter_delta(ifname, rx_sock, msg_type); + const uint64_t tx_delta = get_counter_delta(ifname, tx_sock, msg_type); + has_activity = has_activity || rx_delta > 0 || tx_delta > 0; + if (tx_delta != rx_delta * server_count) { + return DHCP_MON_STATUS_UNHEALTHY; + } + } + return has_activity ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; +} + +/** + * @code dhcp_device_check_server_fanout_v6(ifname); + * @brief Check that each DHCPv6 forward input is transmitted once per configured server. + * @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_server_fanout_v6(const std::string &ifname) +{ + const size_t server_count = get_configured_dhcp_server_count(true); + if (server_count == 0) { + return DHCP_MON_STATUS_INDETERMINATE; + } + + uint64_t rx_delta = 0; + for (const auto msg_type : monitored_v6_forward_rx_msgs) { + rx_delta += get_counter_delta(ifname, rx_sock_v6, msg_type); + } + const uint64_t tx_delta = get_counter_delta(ifname, tx_sock_v6, DHCPV6_MESSAGE_TYPE_RELAY_FORW); + if (rx_delta == 0 && tx_delta == 0) { + return DHCP_MON_STATUS_INDETERMINATE; + } + return tx_delta == rx_delta * server_count ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; +} + /** * @code dhcp_device_check_negative_health(ifname); * @@ -392,7 +446,9 @@ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_ check_type == DHCP_DEVICE_CHECK_AGG_RX || check_type == DHCP_DEVICE_CHECK_AGG_TX || check_type == DHCP_DEVICE_CHECK_AGG_RX_V6 || - check_type == DHCP_DEVICE_CHECK_AGG_TX_V6; + check_type == DHCP_DEVICE_CHECK_AGG_TX_V6 || + check_type == DHCP_DEVICE_CHECK_SERVER_FANOUT || + check_type == DHCP_DEVICE_CHECK_SERVER_FANOUT_V6; if (!aggregate_check && sock_mgr_counters_unchanged(ifname, (const int *)monitored_msgs, monitored_msg_sz, (const int *)monitored_v6_msgs, monitored_v6_msg_sz)) { @@ -416,6 +472,10 @@ dhcp_mon_status_t dhcp_device_get_status(const std::string &ifname, dhcp_device_ return check_aggregate_health(ifname, rx_sock_v6, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); case DHCP_DEVICE_CHECK_AGG_TX_V6: return check_aggregate_health(ifname, tx_sock_v6, (const int *)monitored_v6_msgs, monitored_v6_msg_sz); + case DHCP_DEVICE_CHECK_SERVER_FANOUT: + return dhcp_device_check_server_fanout(ifname); + case DHCP_DEVICE_CHECK_SERVER_FANOUT_V6: + return dhcp_device_check_server_fanout_v6(ifname); default: return DHCP_MON_STATUS_UNHEALTHY; } diff --git a/src/dhcp_device.h b/src/dhcp_device.h index e8f4411f6..bd6b4a226 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -139,7 +139,9 @@ typedef enum DHCP_DEVICE_CHECK_AGG_RX, /** Compare IPv4 RX on a parent interface with its member aggregate */ DHCP_DEVICE_CHECK_AGG_TX, /** Compare IPv4 TX on a parent interface with its member aggregate */ DHCP_DEVICE_CHECK_AGG_RX_V6, /** Compare IPv6 RX on a parent interface with its member aggregate */ - DHCP_DEVICE_CHECK_AGG_TX_V6 /** Compare IPv6 TX on a parent interface with its member aggregate */ + DHCP_DEVICE_CHECK_AGG_TX_V6, /** Compare IPv6 TX on a parent interface with its member aggregate */ + DHCP_DEVICE_CHECK_SERVER_FANOUT, /** Compare IPv4 forward traffic with configured server fan-out */ + DHCP_DEVICE_CHECK_SERVER_FANOUT_V6 /** Compare IPv6 forward traffic with configured server fan-out */ } dhcp_device_check_t; /** Monitored DHCP message type */ diff --git a/src/health_check.cpp b/src/health_check.cpp index 8ea39d3ff..effa22ade 100644 --- a/src/health_check.cpp +++ b/src/health_check.cpp @@ -44,6 +44,12 @@ static const char agg_rx_v6_disparity_error[] = static const char agg_tx_v6_disparity_error[] = "dhcpmon detected an IPv6 TX disparity between interface %s and the aggregate of its member interface counters." " Duration: %d (sec)"; +static const char server_fanout_error[] = + "dhcpmon detected an IPv4 disparity between relay input and configured server fan-out for interface %s." + " Duration: %d (sec)"; +static const char server_fanout_v6_error[] = + "dhcpmon detected an IPv6 disparity between relay input and configured server fan-out for interface %s." + " Duration: %d (sec)"; /** * @code alert_dhcp_relay_disparity(duration); @@ -76,6 +82,8 @@ void initialize_dhcp_relay_health() if (mgmt_ifname.size() > 0) { state_data.push_back({mgmt_ifname, DHCP_DEVICE_CHECK_NEGATIVE_V6, NULL, mgmt_error, 0}); } + state_data.push_back({agg_dev_all, DHCP_DEVICE_CHECK_SERVER_FANOUT, NULL, server_fanout_error, 0}); + state_data.push_back({agg_dev_all, DHCP_DEVICE_CHECK_SERVER_FANOUT_V6, NULL, server_fanout_v6_error, 0}); for (const auto &[vlan, _] : rev_vlan_map) { state_data.push_back({vlan, DHCP_DEVICE_CHECK_AGG_RX, NULL, agg_rx_disparity_error, 0}); diff --git a/src/util.cpp b/src/util.cpp index aab7d83bd..4c288a71a 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -31,6 +31,33 @@ struct udp6_pseudo_header { uint8_t next_header; // Protocol number (17 for UDP) }; +/** + * @code get_config_list_count(table, field); + * @brief Count entries in a comma-separated CONFIG_DB list field for the downstream interface. + * @param table CONFIG_DB table name + * @param field CONFIG_DB field name + * @return number of configured entries, or zero when the field is absent + */ +static size_t get_config_list_count(const std::string &table, const std::string &field) +{ + const std::string key = table + "|" + downstream_ifname; + auto value = mConfigDbPtr->hget(key, field + "@"); + if (value == NULL) { + value = mConfigDbPtr->hget(key, field); + } + if (value == NULL || value->empty()) { + return 0; + } + + size_t count = 1; + for (const char ch : *value) { + if (ch == ',') { + count++; + } + } + return count; +} + /** * @code _addr_is_primary(ifname, addr, addr_len); * @brief Check if the given address is primary on the interface by querying ConfigDB. @@ -91,6 +118,18 @@ bool intf_is_standby(const std::string &ifname) return false; } +size_t get_configured_dhcp_server_count(bool is_v6) +{ + size_t count = is_v6 ? + get_config_list_count("DHCP_RELAY", "dhcpv6_servers") : + get_config_list_count("DHCPV4_RELAY", "dhcpv4_servers"); + if (count > 0) { + return count; + } + + return get_config_list_count("VLAN", is_v6 ? "dhcpv6_servers" : "dhcp_servers"); +} + std::string construct_counter_db_table_key(const std::string &ifname, bool is_v6) { std::string key = is_v6 ? COUNTERS_DB_COUNTER_TABLE_V6_PREFIX : COUNTERS_DB_COUNTER_TABLE_PREFIX; if (downstream_ifname.compare(ifname) != 0) { diff --git a/src/util.h b/src/util.h index 293688822..dee266a5a 100644 --- a/src/util.h +++ b/src/util.h @@ -62,6 +62,14 @@ bool addr6_is_primary(const std::string &ifname, const in6_addr *addr); */ bool intf_is_standby(const std::string &ifname); +/** + * @code get_configured_dhcp_server_count(is_v6); + * @brief Get the number of DHCP servers configured for the downstream interface. + * @param is_v6 whether to read the DHCPv6 server list + * @return configured server count, or zero when no supported server list is present + */ +size_t get_configured_dhcp_server_count(bool is_v6); + /** * @code construct_counter_db_table_key(ifname, is_v6); * @brief Function to construct key in counters_db, only add downstream prefix for non-downstream interface