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 8b1ed4f6c..704e380d6 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" @@ -55,6 +51,18 @@ 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 monitored_v6_forward_rx_msgs[] = { + DHCPV6_MESSAGE_TYPE_SOLICIT, + DHCPV6_MESSAGE_TYPE_REQUEST, + DHCPV6_MESSAGE_TYPE_RELAY_FORW +}; + +static const dhcpv6_message_type_t monitored_v6_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)", @@ -69,33 +77,41 @@ static const char *counter_desc[DHCP_COUNTERS_COUNT] = { [DHCP_COUNTERS_SNAPSHOT_V6] = "Snapshot_V6", }; +/** Logical packets allowed to cross either health snapshot boundary */ +static constexpr uint64_t INFLIGHT_PACKET_TOLERANCE = 1; + /** - * @code check_counter_not_transmitted(ifname, rx_sock, tx_sock, monitored_msgs, monitored_msg_cnt); - * @brief Check if there are received DHCP messages that are not transmitted out - * of this interface/device using its counters. - * @param ifname interface name - * @param rx_sock rx socket - * @param tx_sock tx socket - * @param monitored_msgs array of monitored message types - * @param monitored_msg_cnt number of monitored message types - * @return true if there are received messages not transmitted out, false otherwise + * @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 */ -// these helpers use const int * to accept both dhcp_message_type_t and dhcpv6_message_type_t arrays -// without duplicating the function for each enum type; safe on GCC/Linux where unscoped enums use int -static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock, int tx_sock, const int *monitored_msgs, size_t monitored_msg_cnt) +static uint64_t get_counter_delta(const std::string &ifname, int sock, int msg_type) { - const sock_info_t &rx_sock_info = sock_mgr_get_sock_info(rx_sock); - const counter_t &rx_counters = rx_sock_info.all_counters.at(ifname); - const counter_t &rx_counters_snapshot = rx_sock_info.all_counters_snapshot.at(ifname); - - const sock_info_t &tx_sock_info = sock_mgr_get_sock_info(tx_sock); - const counter_t &tx_counters = tx_sock_info.all_counters.at(ifname); - const counter_t &tx_counters_snapshot = tx_sock_info.all_counters_snapshot.at(ifname); + 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); +} - // when there is packet in, no packet out +/** + * @code check_counter_not_transmitted(ifname, rx_sock, tx_sock, monitored_msgs, monitored_msg_cnt); + * @brief Check whether RX exceeds the tolerance while no same-type TX activity is observed. + * @param ifname interface name + * @param rx_sock RX socket + * @param tx_sock TX socket + * @param monitored_msgs message types to compare + * @param monitored_msg_cnt number of message types + * @return true when any message type exceeds the tolerance + */ +static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock, int tx_sock, + const int *monitored_msgs, size_t monitored_msg_cnt) +{ for (size_t i = 0; i < monitored_msg_cnt; i++) { - if (rx_counters.at(monitored_msgs[i]) > rx_counters_snapshot.at(monitored_msgs[i]) && - tx_counters.at(monitored_msgs[i]) <= tx_counters_snapshot.at(monitored_msgs[i])) { + const uint64_t rx_delta = get_counter_delta(ifname, rx_sock, monitored_msgs[i]); + const uint64_t tx_delta = get_counter_delta(ifname, tx_sock, monitored_msgs[i]); + if (rx_delta > INFLIGHT_PACKET_TOLERANCE && tx_delta == 0) { return true; } } @@ -103,31 +119,19 @@ static bool check_counter_not_transmitted(const std::string &ifname, int rx_sock } /** - * @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 + * @code counter_delta_within_tolerance(input_delta, output_delta, ratio); + * @brief Check an expected ratio while allowing configured logical packets across either snapshot boundary. + * @param input_delta input packet count in the current window + * @param output_delta output packet count in the current window + * @param ratio expected output packets per input packet + * @return true when output is within [(input - tolerance) * ratio, (input + tolerance) * ratio] */ -static dhcp_mon_status_t dhcp_device_check_positive_health(const std::string &ifname) +static bool counter_delta_within_tolerance(uint64_t input_delta, uint64_t output_delta, size_t ratio) { - 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 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 - */ -static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string &ifname) -{ - 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; + const uint64_t lower_input = input_delta > INFLIGHT_PACKET_TOLERANCE ? + input_delta - INFLIGHT_PACKET_TOLERANCE : 0; + return output_delta >= lower_input * ratio && + output_delta <= (input_delta + INFLIGHT_PACKET_TOLERANCE) * ratio; } /** @@ -144,19 +148,58 @@ static dhcp_mon_status_t dhcp_device_check_positive_health_v6(const std::string */ 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; } } return false; } +/** + * @code dhcp_device_check_positive_health(ifname); + * @brief Check DORA RX/TX relationships with configured logical-packet snapshot tolerance. + * @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) +{ + if (check_counter_not_transmitted( + ifname, rx_sock, tx_sock, (const int *)monitored_msgs, monitored_msg_sz)) { + return DHCP_MON_STATUS_UNHEALTHY; + } + return DHCP_MON_STATUS_HEALTHY; +} + +/** + * @code dhcp_device_check_positive_health_v6(ifname); + * @brief Check DHCPv6 forward/reply transformations with configured logical-packet snapshot tolerance. + * @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) +{ + uint64_t forward_rx_delta = 0; + for (const auto msg_type : monitored_v6_forward_rx_msgs) { + forward_rx_delta += get_counter_delta(ifname, rx_sock_v6, msg_type); + } + const uint64_t forward_tx_delta = get_counter_delta(ifname, tx_sock_v6, DHCPV6_MESSAGE_TYPE_RELAY_FORW); + const uint64_t reply_rx_delta = get_counter_delta(ifname, rx_sock_v6, DHCPV6_MESSAGE_TYPE_RELAY_REPL); + uint64_t reply_tx_delta = 0; + for (const auto msg_type : monitored_v6_reply_tx_msgs) { + reply_tx_delta += get_counter_delta(ifname, tx_sock_v6, msg_type); + } + + const bool forward_valid = forward_rx_delta <= INFLIGHT_PACKET_TOLERANCE || forward_tx_delta > 0; + const bool reply_valid = reply_rx_delta <= INFLIGHT_PACKET_TOLERANCE || reply_tx_delta > 0; + if (!forward_valid || !reply_valid) { + return DHCP_MON_STATUS_UNHEALTHY; + } + return forward_rx_delta > 0 || reply_rx_delta > 0 ? + DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_INDETERMINATE; +} + /** * @code dhcp_device_check_negative_health(ifname); * @@ -192,19 +235,47 @@ 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 member_delta_within_tolerance(parent_delta, aggregate_delta, member_count); + * @brief Check exact unicast-or-broadcast fan-out with configured snapshot tolerance. + * @param parent_delta parent interface packet count + * @param aggregate_delta aggregate member-interface packet count + * @param member_count direct member count + * @return true when aggregate fan-out is valid + */ +static bool member_delta_within_tolerance(uint64_t parent_delta, uint64_t aggregate_delta, size_t member_count) +{ + if (member_count <= 1) { + return counter_delta_within_tolerance(parent_delta, aggregate_delta, 1); + } + + if (INFLIGHT_PACKET_TOLERANCE > 0) { + const uint64_t lower_parent = parent_delta > INFLIGHT_PACKET_TOLERANCE ? + parent_delta - INFLIGHT_PACKET_TOLERANCE : 0; + return aggregate_delta >= lower_parent && + aggregate_delta <= (parent_delta + INFLIGHT_PACKET_TOLERANCE) * member_count; + } + + if (aggregate_delta < parent_delta) { + return false; + } + const uint64_t additional_member_observations = aggregate_delta - parent_delta; + return additional_member_observations % (member_count - 1) == 0 && + additional_member_observations / (member_count - 1) <= parent_delta; +} + +/** + * @code check_counters_delta_expected(ifname, other_ifname, sock, member_count, monitored_msgs, monitored_msg_cnt); + * @brief Check parent/member counter deltas with configured snapshot tolerance. * @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, - uint8_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); @@ -212,74 +283,42 @@ 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_delta_within_tolerance(delta, other_delta, member_count)) { return false; } } 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) -{ - 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; -} - -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 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) { - 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); + 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, member_count, monitored_msgs, monitored_msg_cnt) ? DHCP_MON_STATUS_HEALTHY : DHCP_MON_STATUS_UNHEALTHY; } @@ -382,7 +421,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 +441,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..e8f4411f6 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -135,15 +135,11 @@ 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_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_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 */ + 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_devman.cpp b/src/dhcp_devman.cpp index 9fa951e3c..7d8fc09f4 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; } /** @@ -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) diff --git a/src/dhcp_devman.h b/src/dhcp_devman.h index 73c1cd6f2..f0cdde3be 100644 --- a/src/dhcp_devman.h +++ b/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); * diff --git a/src/dhcp_mon.cpp b/src/dhcp_mon.cpp index ef4f6623d..8798c0182 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; } @@ -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,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( + get_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( + 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) @@ -520,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 * diff --git a/src/packet_handler.cpp b/src/packet_handler.cpp index 7ec5d07a3..b9532833e 100644 --- a/src/packet_handler.cpp +++ b/src/packet_handler.cpp @@ -16,8 +16,11 @@ #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 @@ -25,9 +28,9 @@ * @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)); diff --git a/src/sock_mgr.cpp b/src/sock_mgr.cpp index 8d3e48d81..f01c07a74 100644 --- a/src/sock_mgr.cpp +++ b/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); diff --git a/src/util.cpp b/src/util.cpp index aab7d83bd..8abe17787 100644 --- a/src/util.cpp +++ b/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) diff --git a/src/util.h b/src/util.h index f4fbd24c3..293688822 100644 --- a/src/util.h +++ b/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; } /**