[dhcpmon] Track physical members beneath VLAN PortChannels

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;
 }
 
 /**
@@ -248,7 +235,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 +286,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();
 
@@ -315,21 +305,59 @@ 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;
+
+std::string dhcp_devman_get_parent_ifname(const std::string &ifname)
 {
-    const auto iter = intfs.find(ifname);
-    if (iter != intfs.end()) {
-        return iter->second;
+    if (intfs.find(ifname) != intfs.end()) {
+        return "";
     }
     const auto vlan = vlan_map.find(ifname);
     if (vlan != vlan_map.end() && ifname != vlan->second) {
-        return dhcp_devman_get_device_context(vlan->second);
+        return vlan->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 port_channel->second;
     }
-    return NULL;
+    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)
+{
+    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 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)
+{
+    return get_device_context(ifname, 0);
+}
+
+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_agg_counter_ifname(parent_ifname);
 }
 
 void dhcp_devman_print_all_status(dhcp_counters_type_t type)

src/dhcp_devman.h

@@ -127,17 +127,39 @@ int dhcp_devman_init();
  */
 void dhcp_devman_free();
 
+/**
+ * @code dhcp_devman_get_parent_ifname(ifname);
+ *
+ * @brief find the immediate parent interface of a tracked interface.
+ *
+ * @param ifname           interface name
+ *
+ * @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, if its physical interface, will query vlan_map and portchannel_map first
+ * @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);
 
+/**
+ * @code dhcp_devman_get_agg_counter_ifname(ifname);
+ *
+ * @brief find the aggregate counter updated by an interface observation.
+ *
+ * @param ifname           interface 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);
+
 /**
  * @code dhcp_devman_print_all_status(type);
  *

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;
             }
@@ -125,21 +125,21 @@ static bool db_counters_initialized(const std::string &ifname)
 
     table_name = construct_counter_db_table_key(ifname, false);
     auto rx_v4 = mCountersDbPtr->hget(table_name, "RX");
-    if (rx_v4 == nullptr || rx_v4->empty()) {
+    if (rx_v4 == NULL || rx_v4->empty()) {
         return false;
     }
     auto tx_v4 = mCountersDbPtr->hget(table_name, "TX");
-    if (tx_v4 == nullptr || tx_v4->empty()) {
+    if (tx_v4 == NULL || tx_v4->empty()) {
         return false;
     }
 
     table_name = construct_counter_db_table_key(ifname, true);
     auto rx_v6 = mCountersDbPtr->hget(table_name, "RX");
-    if (rx_v6 == nullptr || rx_v6->empty()) {
+    if (rx_v6 == NULL || rx_v6->empty()) {
         return false;
     }
     auto tx_v6 = mCountersDbPtr->hget(table_name, "TX");
-    if (tx_v6 == nullptr || tx_v6->empty()) {
+    if (tx_v6 == NULL || tx_v6->empty()) {
         return false;
     }
 
@@ -466,15 +466,19 @@ 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(
+            get_agg_counter_ifname(vlan),
+            DHCP_MESSAGE_TYPE_COUNT, DHCPV6_MESSAGE_TYPE_COUNT);
     }
 
     for (const auto &[portchan, intfs] : rev_portchan_map) {
         for (const auto &ifname : intfs) {
             initialize_all_counters(ifname);
         }
         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(
+            get_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)

src/packet_handler.cpp

@@ -16,18 +16,21 @@
 #include "dhcp_check_profile.h"   /** to get dhcp/v6 check profile */
 #include "util.h"
 
+// Bound one callback to keep health/DB events responsive during packet bursts.
+static constexpr int MAX_PACKETS_PER_CALLBACK = 64;
+
 /**
- * @code _increase_cache_counter(ifname, sock, type);
+ * @code increase_single_cache_counter(ifname, sock, type);
  * @brief helper function to increase cache counter. Simple increase of counter, no complications. In the event of
  *        extremely unexpected nonexistent ifname, just fail
  * @param ifname       interface name
  * @param sock         socket number
  * @param type         message type
  * @return             none
  */
-static inline void _increase_cache_counter(const std::string &ifname, int sock, uint8_t type)
+static inline void increase_single_cache_counter(const std::string &ifname, int sock, uint8_t type)
 {
-    syslog_debug(LOG_INFO, "_increase_cache_counter: increasing cache counter for ifname %s, sock %d, type %d",
+    syslog_debug(LOG_INFO, "increase_single_cache_counter: increasing cache counter for ifname %s, sock %d, type %d",
                  ifname.c_str(), sock, type);
     sock_mgr_get_sock_info(sock).all_counters.at(ifname)[type]++;
 }
@@ -45,16 +48,14 @@ static inline void _increase_cache_counter(const std::string &ifname, int sock,
  */
 static void increase_cache_counter(const std::string &ifname, const dhcp_device_context_t *context, int sock, uint8_t type, bool dup_to_context=false)
 {
-    _increase_cache_counter(ifname, sock, type);
+    increase_single_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;
     }
 
-    // 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_single_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) {
@@ -861,11 +862,14 @@ void callback_common(int fd, short event, void *arg)
 {
     ssize_t buffer_sz;
     struct sockaddr_ll sll;
-    socklen_t slen = sizeof(sll);
     sock_info_t &sock_info = sock_mgr_get_sock_info(fd);
 
-    while ((buffer_sz = recvfrom(fd, sock_info.buffer, sock_info.snaplen, MSG_DONTWAIT, (struct sockaddr *)&sll, &slen)) > 0) 
-    {
+    for (int packet_count = 0; packet_count < MAX_PACKETS_PER_CALLBACK; packet_count++) {
+        socklen_t slen = sizeof(sll);
+        if ((buffer_sz = recvfrom(fd, sock_info.buffer, sock_info.snaplen, MSG_DONTWAIT,
+                                  (struct sockaddr *)&sll, &slen)) <= 0) {
+            break;
+        }
         char ifname_buf[IF_NAMESIZE];
         if (if_indextoname(sll.sll_ifindex, ifname_buf) == NULL) {
             syslog_debug(LOG_WARNING, "if_indextoname: invalid input interface index %d %s", sll.sll_ifindex, strerror(errno));

src/sock_mgr.cpp

@@ -153,11 +153,11 @@ static void print_bpf_prog(const struct bpf_program *bp) {
 }
 
 /**
- * @code _compile_bpf_prog(handle, bp, filter, fprog)
+ * @code compile_bpf_prog(handle, bp, filter, fprog)
  *
  * @brief helper function to compile filter to bpf byte code and store in fprog
  */
-static int _compile_bpf_prog(pcap_t *handle, struct bpf_program *bp, const char *filter, struct sock_fprog *fprog){
+static int compile_bpf_prog(pcap_t *handle, struct bpf_program *bp, const char *filter, struct sock_fprog *fprog){
     syslog(LOG_INFO, "Compiling filter %s to bpf prog", filter);
 
     if (pcap_compile(handle, bp, filter, 1, PCAP_NETMASK_UNKNOWN) < 0) {
@@ -213,7 +213,7 @@ static int sock_mgr_compile_all_bpf_prog()
 
     struct bpf_program bp;
     for (auto &[sock, info] : sock_map) {
-        if (_compile_bpf_prog(handle, &bp, info.filter, &info.bpf_prog) < 0) {
+        if (compile_bpf_prog(handle, &bp, info.filter, &info.bpf_prog) < 0) {
             syslog(LOG_ALERT, "Failed to compile %s into bpf prog", info.filter);
             sock_mgr_free_all_bpf_prog();
             pcap_close(handle);

src/util.cpp

@@ -32,14 +32,14 @@ struct udp6_pseudo_header {
 };
 
 /**
- * @code _addr_is_primary(ifname, addr, addr_len);
+ * @code addr_is_primary_impl(ifname, addr, addr_len);
  * @brief Check if the given address is primary on the interface by querying ConfigDB.
  * @param ifname    interface name
  * @param addr      pointer to the address (in_addr for IPv4 or in6_addr for IPv6)
  * @param addr_len  length of the address (4 for IPv4, 16 for IPv6)
  * @return          true if the address is primary, false if secondary or not found
  */
-static bool _addr_is_primary(const std::string &ifname, const uint8_t *addr, size_t addr_len)
+static bool addr_is_primary_impl(const std::string &ifname, const uint8_t *addr, size_t addr_len)
 {
     auto match_pattern = std::string("*INTERFACE|" + ifname + "|*");
     auto keys = mConfigDbPtr->keys(match_pattern);
@@ -73,12 +73,12 @@ static bool _addr_is_primary(const std::string &ifname, const uint8_t *addr, siz
 
 bool addr_is_primary(const std::string &ifname, const in_addr *addr)
 {
-    return _addr_is_primary(ifname, (const uint8_t *)addr, sizeof(struct in_addr));
+    return addr_is_primary_impl(ifname, (const uint8_t *)addr, sizeof(struct in_addr));
 }
 
 bool addr6_is_primary(const std::string &ifname, const in6_addr *addr)
 {
-    return _addr_is_primary(ifname, (const uint8_t *)addr, sizeof(struct in6_addr));
+    return addr_is_primary_impl(ifname, (const uint8_t *)addr, sizeof(struct in6_addr));
 }
 
 bool intf_is_standby(const std::string &ifname)

src/util.h

@@ -217,15 +217,14 @@ inline bool is_agg_counter(const std::string &ifname)
 }
 
 /**
- * @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
+ * @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 &ifname, const std::string &context_ifname)
+inline std::string get_agg_counter_ifname(const std::string &parent_ifname)
 {
-    return ifname != context_ifname ? agg_dev_prefix + context_ifname : agg_dev_all;
+    return agg_dev_prefix + parent_ifname;
 }
 
 /**