From 39566b95b3a414e0e5d18e435cdf0b1b5ae8969f Mon Sep 17 00:00:00 2001 From: yaqiangz Date: Tue, 5 Mar 2024 13:56:15 +0000 Subject: [PATCH 1/4] Revert "[counter] Clear counter table when dhcpmon init (#14)" This reverts commit bace2e0360b7004ac5e0f4f5dc0efa2615d08614. --- src/dhcp_device.cpp | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 9796a14db..8c105f129 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -194,25 +194,6 @@ void update_vlan_mapping(std::shared_ptr db_conn) { } } - -/** - * @code clear_counter(std::shared_ptr state_db); - * - * @brief Clear all counter - * - * @param state_db state_db connector pointer - * - */ -void clear_counter(std::shared_ptr state_db) { - std::string match_pattern = DB_COUNTER_TABLE + std::string("*"); - auto keys = state_db->keys(match_pattern); - - for (auto &itr : keys) { - state_db->del(itr); - } -} - - /** update ethernet interface to port-channel map * PORTCHANNEL_MEMBER|PortChannel101|Ethernet112 */ @@ -284,6 +265,7 @@ void initialize_db_counters(std::string &ifname) { auto table_name = DB_COUNTER_TABLE + ifname; auto init_value = gen_counter_json_str(db_counter); + mStateDbPtr->del(table_name); mStateDbPtr->hset(table_name, "RX", init_value); mStateDbPtr->hset(table_name, "TX", init_value); } @@ -942,7 +924,6 @@ int dhcp_device_start_capture(size_t snaplen, struct event_base *base, in_addr_t init_recv_buffers(snaplen); - clear_counter(mStateDbPtr); update_vlan_mapping(mConfigDbPtr); update_portchannel_mapping(mConfigDbPtr); update_mgmt_mapping(); From 144bec682673f7768ad5bf6568569877538fc887 Mon Sep 17 00:00:00 2001 From: yaqiangz Date: Tue, 5 Mar 2024 13:57:00 +0000 Subject: [PATCH 2/4] Revert "refine counting logic" This reverts commit 085a0870e0a9b4829ac5e4c0a53e7fab175015a5. --- src/dhcp_device.cpp | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index 8c105f129..edba78583 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -357,10 +357,7 @@ static void handle_dhcp_option_53(std::string &sock_if, std::string context_if(context->intf); // count for incomming physical interfaces - if (context_if.compare(sock_if) != 0) { - increase_db_counter(sock_if, dhcp_option[2], dir); - return; - } + increase_db_counter(sock_if, dhcp_option[2], dir); switch (dhcp_option[2]) { @@ -377,7 +374,9 @@ static void handle_dhcp_option_53(std::string &sock_if, context->counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; aggregate_dev.counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; // count for device context interfaces (-d -u -m) - increase_db_counter(context_if, dhcp_option[2], dir); + if (context_if.compare(sock_if) != 0) { + increase_db_counter(context_if, dhcp_option[2], dir); + } } break; // DHCP messages send by server @@ -388,14 +387,16 @@ static void handle_dhcp_option_53(std::string &sock_if, (!context->is_uplink && dir == DHCP_TX)) { context->counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; aggregate_dev.counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; - // count for device context interfaces (-d -u -m) - increase_db_counter(context_if, dhcp_option[2], dir); + if (context_if.compare(sock_if) != 0) { + increase_db_counter(context_if, dhcp_option[2], dir); + } } break; default: syslog(LOG_WARNING, "handle_dhcp_option_53(%s): Unknown DHCP option 53 type %d", context->intf, dhcp_option[2]); - // count for device context interfaces (-d -u -m) - increase_db_counter(context_if, dhcp_option[2], dir); + if (context_if.compare(sock_if) != 0) { + increase_db_counter(context_if, dhcp_option[2], dir); + } break; } } @@ -506,14 +507,9 @@ static void read_tx_callback(int fd, short event, void *arg) continue; } std::string intf(interfaceName); - context = find_device_context(devices, intf); + context = interface_to_dev_context(devices, intf); if (context) { client_packet_handler(intf, context, tx_recv_buffer, buffer_sz, DHCP_TX); - } else { - context = interface_to_dev_context(devices, intf); - if (context) { - client_packet_handler(intf, context, tx_recv_buffer, buffer_sz, DHCP_TX); - } } } } @@ -545,15 +541,10 @@ static void read_rx_callback(int fd, short event, void *arg) continue; } std::string intf(interfaceName); - context = find_device_context(devices, intf); + context = interface_to_dev_context(devices, intf); if (context) { client_packet_handler(intf, context, rx_recv_buffer, buffer_sz, DHCP_RX); - } else { - context = interface_to_dev_context(devices, intf); - if (context) { - client_packet_handler(intf, context, rx_recv_buffer, buffer_sz, DHCP_RX); - } - } + } } } From 417fa12560801e0f8d13a611fe41212bbbe6ba45 Mon Sep 17 00:00:00 2001 From: yaqiangz Date: Tue, 5 Mar 2024 13:57:37 +0000 Subject: [PATCH 3/4] Revert "dhcp per-interface counter init commit" This reverts commit 5cb44684939eefc08e6af9577fa205dd19ccd49b. --- .azure-pipelines/build.yml | 1 - objects.mk | 2 +- src/dhcp_device.cpp | 166 +++---------------------------------- src/dhcp_device.h | 24 ------ 4 files changed, 14 insertions(+), 179 deletions(-) diff --git a/.azure-pipelines/build.yml b/.azure-pipelines/build.yml index 6ff8e94c1..7ca40a07d 100644 --- a/.azure-pipelines/build.yml +++ b/.azure-pipelines/build.yml @@ -41,7 +41,6 @@ jobs: libnl-route-3-dev \ libnl-genl-3-dev \ libnl-nf-3-dev \ - libjsoncpp-dev \ libevent-dev displayName: "Install dependencies" - checkout: self diff --git a/objects.mk b/objects.mk index 966d4815e..dc0d09e50 100644 --- a/objects.mk +++ b/objects.mk @@ -1,4 +1,4 @@ USER_OBJS := -LIBS := -levent -lexplain -lswsscommon -ljsoncpp -pthread -lboost_thread -lboost_system -lhiredis +LIBS := -levent -lexplain -lswsscommon -pthread -lboost_thread -lboost_system -lhiredis diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index edba78583..dc07333ce 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -19,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -45,8 +43,7 @@ #define DHCP_OPTIONS_HEADER_SIZE 240 /** Offset of DHCP GIADDR */ #define DHCP_GIADDR_OFFSET 24 -/* STATE_DB DHCP counter table name */ -#define DB_COUNTER_TABLE "DHCP_COUNTER_TABLE|" +#define CLIENT_IF_PREFIX "Ethernet" #define OP_LDHA (BPF_LD | BPF_H | BPF_ABS) /** bpf ldh Abs */ #define OP_LDHI (BPF_LD | BPF_H | BPF_IND) /** bpf ldh Ind */ @@ -60,8 +57,8 @@ std::shared_ptr mConfigDbPtr = std::make_shared ("CONFIG_DB", 0); std::shared_ptr mStateDbPtr = std::make_shared ("STATE_DB", 0); std::shared_ptr mStateDbMuxTablePtr = std::make_shared ( - mStateDbPtr.get(), "HW_MUX_CABLE_TABLE" -); + mStateDbPtr.get(), "HW_MUX_CABLE_TABLE" + ); /* interface to vlan mapping */ std::unordered_map vlan_map; @@ -72,15 +69,6 @@ std::unordered_map portchan_map; /* interface to mgmt port mapping */ std::unordered_map mgmt_map; -/* db counter name array, message type rage [1, 8] */ -std::string db_counter_name[DHCP_MESSAGE_TYPE_COUNT] = { - "Unknown", "Discover", "Offer", "Request", "Decline", "Ack", "Nak", "Release", "Inform" -}; -/* db counter init value in uint64_t */ -uint64_t db_counter[DHCP_MESSAGE_TYPE_COUNT] = {}; - -const std::string init_counter_str; - /** Berkeley Packet Filter program for "udp and (port 67 or port 68)". * This program is obtained using the following command tcpdump: * `tcpdump -dd "outbound and udp and (port 67 or port 68)"` @@ -176,21 +164,13 @@ static dhcp_message_type_t monitored_msgs[] = { void update_vlan_mapping(std::shared_ptr db_conn) { auto match_pattern = std::string("VLAN_MEMBER|*"); auto keys = db_conn->keys(match_pattern); - std::unordered_map vlans; for (auto &itr : keys) { auto first = itr.find_first_of('|'); auto second = itr.find_last_of('|'); auto vlan = itr.substr(first + 1, second - first - 1); auto interface = itr.substr(second + 1); vlan_map[interface] = vlan; - vlans[vlan] = true; syslog(LOG_INFO, "add <%s, %s> into interface vlan map\n", interface.c_str(), vlan.c_str()); - std::string ifname = interface; - initialize_db_counters(ifname); - } - for (auto &itr : vlans) { - std::string ifname = itr.first; - initialize_db_counters(ifname); } } @@ -200,21 +180,13 @@ void update_vlan_mapping(std::shared_ptr db_conn) { void update_portchannel_mapping(std::shared_ptr db_conn) { auto match_pattern = std::string("PORTCHANNEL_MEMBER|*"); auto keys = db_conn->keys(match_pattern); - std::unordered_map portchannels; for (auto &itr : keys) { auto first = itr.find_first_of('|'); auto second = itr.find_last_of('|'); auto portchannel = itr.substr(first + 1, second - first - 1); auto interface = itr.substr(second + 1); portchan_map[interface] = portchannel; - portchannels[portchannel] = true; syslog(LOG_INFO, "add <%s, %s> into interface port-channel map\n", interface.c_str(), portchannel.c_str()); - std::string ifname = interface; - initialize_db_counters(ifname); - } - for (auto &itr : portchannels) { - std::string ifname = itr.first; - initialize_db_counters(ifname); } } @@ -225,99 +197,6 @@ void update_mgmt_mapping() { if (mgmt) { auto name = std::string(mgmt->intf); mgmt_map[name] = name; - initialize_db_counters(name); - } -} - -/** - * @code void gen_counter_json_str() - * - * @brief generate counter json string based on the value in counters array - * - * @return counter json string - */ -std::string gen_counter_json_str(uint64_t *counters) { - std::string init_value; - - init_value.append("{"); - for (int i = 0; i < DHCP_MESSAGE_TYPE_COUNT; i++) { - auto value = std::to_string(counters[i]); - auto json_str = "'" + db_counter_name[i] + "'"+ ":" + "'" + value + "'"; - init_value.append(json_str); - if (i + 1 < DHCP_MESSAGE_TYPE_COUNT) { - init_value.append(","); - } - } - init_value.append("}"); - return init_value; -} - -/** - * @code void increase_db_counter(std::string &ifname) - * - * @brief increase the counter in state_db with interface name - * - * @param ifname interface name - * - * @return none - */ -void initialize_db_counters(std::string &ifname) -{ - auto table_name = DB_COUNTER_TABLE + ifname; - auto init_value = gen_counter_json_str(db_counter); - mStateDbPtr->del(table_name); - mStateDbPtr->hset(table_name, "RX", init_value); - mStateDbPtr->hset(table_name, "TX", init_value); -} - -/** - * @code void increase_db_counter(std::string &ifname, uint8_t msg_type, dhcp_packet_direction_t dir) - * - * @brief increase the counter in state_db with count of each DHCP message types - * - * @param ifname interface name - * @param type dhcp message type to be increased in counter - * @param dir dhcp packet direction - * - * @return none - */ -void increase_db_counter(std::string &ifname, uint8_t type, dhcp_packet_direction_t dir) { - if (type >= DHCP_MESSAGE_TYPE_COUNT) { - syslog(LOG_WARNING, "Unexpected message type %d(0x%x)\n", type, type); - type = 0; // treate it as unknown counter - } - std::string table_name = DB_COUNTER_TABLE + ifname; - std::string msg_type = db_counter_name[type]; - auto counters_json = mStateDbPtr->hget(table_name, (dir == DHCP_RX) ? "RX" : "TX"); - if (counters_json == nullptr) { - db_counter[type] = 1; - auto json_string = gen_counter_json_str(db_counter); - mStateDbPtr->hset(table_name, (dir == DHCP_RX) ? "RX" : "TX", json_string); - db_counter[type] = 0; - } else { - Json::Value root; - Json::CharReaderBuilder builder; - Json::StreamWriterBuilder wbuilder; - JSONCPP_STRING err; - - std::replace(counters_json.get()->begin(), counters_json.get()->end(), '\'', '\"'); - auto json_begin = counters_json.get()->c_str(); - auto json_end = json_begin + counters_json.get()->length(); - const std::unique_ptr reader(builder.newCharReader()); - if (reader->parse(json_begin, json_end, &root, &err)) { - if (root.isMember(msg_type)) { - std::string cnt_string = root[msg_type].asString(); - auto cnt = std::stoull(cnt_string) + 1; - root[msg_type] = Json::Value(std::to_string(cnt)); - } else { - root[msg_type] = Json::Value(std::to_string(1)); - } - wbuilder["indentation"] = ""; // whitespace-less output - const std::string document = Json::writeString(wbuilder, root); - mStateDbPtr->hset(table_name, (dir == DHCP_RX) ? "RX" : "TX", document); - } else { - syslog(LOG_WARNING, "failed to parse counter json: %s, %s", json_begin, err.c_str()); - } } } @@ -337,7 +216,6 @@ static uint8_t monitored_msg_sz = sizeof(monitored_msgs) / sizeof(*monitored_msg * * @brief handle the logic related to DHCP option 53 * - * @param src_if Source pyhsical interface name * @param context Device (interface) context * @param dhcp_option pointer to DHCP option buffer space * @param dir packet direction @@ -346,19 +224,13 @@ static uint8_t monitored_msg_sz = sizeof(monitored_msgs) / sizeof(*monitored_msg * * @return none */ -static void handle_dhcp_option_53(std::string &sock_if, - dhcp_device_context_t *context, +static void handle_dhcp_option_53(dhcp_device_context_t *context, const u_char *dhcp_option, dhcp_packet_direction_t dir, struct ip *iphdr, uint8_t *dhcphdr) { in_addr_t giaddr; - std::string context_if(context->intf); - - // count for incomming physical interfaces - increase_db_counter(sock_if, dhcp_option[2], dir); - switch (dhcp_option[2]) { // DHCP messages send by client @@ -373,10 +245,6 @@ static void handle_dhcp_option_53(std::string &sock_if, (!context->is_uplink && dir == DHCP_RX && iphdr->ip_dst.s_addr == INADDR_BROADCAST)) { context->counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; aggregate_dev.counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; - // count for device context interfaces (-d -u -m) - if (context_if.compare(sock_if) != 0) { - increase_db_counter(context_if, dhcp_option[2], dir); - } } break; // DHCP messages send by server @@ -387,35 +255,25 @@ static void handle_dhcp_option_53(std::string &sock_if, (!context->is_uplink && dir == DHCP_TX)) { context->counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; aggregate_dev.counters[DHCP_COUNTERS_CURRENT][dir][dhcp_option[2]]++; - if (context_if.compare(sock_if) != 0) { - increase_db_counter(context_if, dhcp_option[2], dir); - } } break; default: syslog(LOG_WARNING, "handle_dhcp_option_53(%s): Unknown DHCP option 53 type %d", context->intf, dhcp_option[2]); - if (context_if.compare(sock_if) != 0) { - increase_db_counter(context_if, dhcp_option[2], dir); - } break; } } /** - * @code client_packet_handler(std::string &sock_if, dhcp_device_context_t *context, uint8_t *buffer, - * ssize_t buffer_sz, dhcp_packet_direction_t dir); + * @code client_packet_handler(dhcp_device_context_t *context, ssize_t buffer_sz); * * @brief packet handler to process received rx and tx packets * - * @param sock_if socket interface * @param context pointer to device (interface) context - * @param buffer DHCP packet * @param buffer_sz buffer that stores received packet data - * @param dir DHCP packet direction * * @return none */ -static void client_packet_handler(std::string &sock_if, dhcp_device_context_t *context, uint8_t *buffer, +static void client_packet_handler(dhcp_device_context_t *context, uint8_t *buffer, ssize_t buffer_sz, dhcp_packet_direction_t dir) { struct ip *iphdr = (struct ip*) (buffer + IP_START_OFFSET); @@ -435,7 +293,7 @@ static void client_packet_handler(std::string &sock_if, dhcp_device_context_t *c while ((offset < (dhcp_option_sz + 1)) && dhcp_option[offset] != 255) { if (dhcp_option[offset] == OPTION_DHCP_MESSAGE_TYPE) { if (offset < (dhcp_option_sz + 2)) { - handle_dhcp_option_53(sock_if, context, &dhcp_option[offset], dir, iphdr, dhcphdr); + handle_dhcp_option_53(context, &dhcp_option[offset], dir, iphdr, dhcphdr); } break; // break while loop since we are only interested in Option 53 } @@ -446,6 +304,9 @@ static void client_packet_handler(std::string &sock_if, dhcp_device_context_t *c offset += dhcp_option[offset + 1] + 2; } } + } else { + syslog(LOG_WARNING, "read_callback(%s %s): read length (%ld) is too small to capture DHCP options", + context->intf, dir == DHCP_TX ? "TX" : "RX", buffer_sz); } } @@ -473,7 +334,6 @@ static dhcp_device_context_t *interface_to_dev_context(std::unordered_mapsecond); } - return find_device_context(devices, ifname); } } return NULL; @@ -507,9 +367,9 @@ static void read_tx_callback(int fd, short event, void *arg) continue; } std::string intf(interfaceName); - context = interface_to_dev_context(devices, intf); + context = find_device_context(devices, intf); if (context) { - client_packet_handler(intf, context, tx_recv_buffer, buffer_sz, DHCP_TX); + client_packet_handler(context, tx_recv_buffer, buffer_sz, DHCP_TX); } } } @@ -543,7 +403,7 @@ static void read_rx_callback(int fd, short event, void *arg) std::string intf(interfaceName); context = interface_to_dev_context(devices, intf); if (context) { - client_packet_handler(intf, context, rx_recv_buffer, buffer_sz, DHCP_RX); + client_packet_handler(context, rx_recv_buffer, buffer_sz, DHCP_RX); } } } diff --git a/src/dhcp_device.h b/src/dhcp_device.h index a75f94395..cd8eab1ee 100644 --- a/src/dhcp_device.h +++ b/src/dhcp_device.h @@ -194,28 +194,4 @@ void dhcp_device_update_snapshot(dhcp_device_context_t *context); */ void dhcp_device_print_status(dhcp_device_context_t *context, dhcp_counters_type_t type); -/** - * @code void increase_db_counter(std::string &ifname) - * - * @brief increase the counter in state_db with interface name - * - * @param ifname interface name - * - * @return none - */ -void initialize_db_counters(std::string &ifname); - -/** - * @code void increase_db_counter(std::string &ifname, uint8_t msg_type, dhcp_packet_direction_t dir) - * - * @brief increase the counter in state_db with count of each DHCP message types - * - * @param ifname interface name - * @param type dhcp message type to be increased in counter - * @param dir dhcp packet direction - * - * @return none - */ -void increase_db_counter(std::string &ifname, uint8_t type, dhcp_packet_direction_t dir); - #endif /* DHCP_DEVICE_H_ */ From a4ca459bdcb6f928bb242b83d57ff83f1910a45e Mon Sep 17 00:00:00 2001 From: yaqiangz Date: Tue, 5 Mar 2024 13:58:05 +0000 Subject: [PATCH 4/4] Update buffer --- src/dhcp_device.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/dhcp_device.cpp b/src/dhcp_device.cpp index dc07333ce..69a40ce7f 100644 --- a/src/dhcp_device.cpp +++ b/src/dhcp_device.cpp @@ -22,6 +22,8 @@ #include #include #include +#include +#include #include "subscriberstatetable.h" #include "select.h" @@ -753,6 +755,22 @@ int dhcp_device_init(dhcp_device_context_t **context, const char *intf, uint8_t return rv; } +/** + * @code getMaxBufferSize(); + * + * @get max buffer size from /proc/sys/net/core/rmem_max. + */ +size_t getMaxBufferSize() { + std::ifstream file("/proc/sys/net/core/rmem_max"); + int maxBufferSize = 0; + if (file) { + file >> maxBufferSize; + } else { + syslog(LOG_ALERT, "Could not open /proc/sys/net/core/rmem_max"); + } + return maxBufferSize; +} + /** * @code dhcp_device_start_capture(snaplen, base, giaddr_ip); * @@ -801,6 +819,16 @@ int dhcp_device_start_capture(size_t snaplen, struct event_base *base, in_addr_t exit(1); } + size_t maxBufferSize = getMaxBufferSize(); + if (maxBufferSize == 0) { + syslog(LOG_ALERT, "dhcp_device_start_capture: failed to get max buffer size, using default"); + } + else { + if (setsockopt(tx_sock, SOL_SOCKET, SO_RCVBUF, &maxBufferSize, sizeof(maxBufferSize)) == -1) { + syslog(LOG_ALERT, "setsockopt: failed to set rcvbuf size '%s'\n", strerror(errno)); + } + } + rx_ev = event_new(base, rx_sock, EV_READ | EV_PERSIST, read_rx_callback, &intfs); tx_ev = event_new(base, tx_sock, EV_READ | EV_PERSIST, read_tx_callback, &intfs);