diff --git a/src/stats/stats_printer.cpp b/src/stats/stats_printer.cpp index bcbcb18f6..67329b8a0 100644 --- a/src/stats/stats_printer.cpp +++ b/src/stats/stats_printer.cpp @@ -11,8 +11,7 @@ #include "vma/util/utils.h" #include "vma/util/vma_stats.h" #include "vma/lwip/tcp.h" -#include "vma/vma_extra.h" -#include "vma/util/sys_vars.h" +#include "vma/util/vtypes.h" typedef enum { e_K = 1024, diff --git a/src/stats/stats_publisher.cpp b/src/stats/stats_publisher.cpp index 35eaaf6ce..6098f49f7 100644 --- a/src/stats/stats_publisher.cpp +++ b/src/stats/stats_publisher.cpp @@ -102,7 +102,7 @@ void stats_data_reader::handle_timer_expired(void *ctx) void stats_data_reader::register_to_timer() { - m_timer_handler = g_p_event_handler_manager->register_timer_event(STATS_PUBLISHER_TIMER_PERIOD, g_p_stats_data_reader, PERIODIC_TIMER, 0); + m_timer_handler = g_p_event_handler_manager->register_timer_event(STATS_PUBLISHER_TIMER_PERIOD, this, PERIODIC_TIMER, 0); } void stats_data_reader::add_data_reader(void* local_addr, void* shm_addr, int size) @@ -150,6 +150,8 @@ void vma_shmem_stats_open(vlog_levels_t** p_p_vma_log_level, uint8_t** p_p_vma_l } BULLSEYE_EXCLUDE_BLOCK_END + vlog_printf(VLOG_DEBUG,"%s:%d: Allocated g_p_stats_data_reader pointer as '%p'\n", __func__, __LINE__, g_p_stats_data_reader); + shmem_size = SHMEM_STATS_SIZE(safe_mce_sys().stats_fd_num_max); buf = malloc(shmem_size); if (buf == NULL) @@ -277,8 +279,12 @@ void vma_shmem_stats_close() g_sh_mem = NULL; g_p_vlogger_level = NULL; g_p_vlogger_details = NULL; - delete g_p_stats_data_reader; - g_p_stats_data_reader = NULL; + if (g_p_stats_data_reader != NULL) { + stats_data_reader* p = g_p_stats_data_reader; + g_p_stats_data_reader = NULL; + p->set_destroying_state(true); + delete p; + } } void vma_stats_instance_create_socket_block(socket_stats_t* local_stats_addr) diff --git a/src/stats/stats_reader.cpp b/src/stats/stats_reader.cpp index 8a166fa71..8e8938daf 100644 --- a/src/stats/stats_reader.cpp +++ b/src/stats/stats_reader.cpp @@ -15,9 +15,9 @@ #include #include #include +#include #include #include /* getopt()*/ -#include #include #include #include diff --git a/src/vlogger/vlogger.cpp b/src/vlogger/vlogger.cpp index fb0c4617a..709e0cbc8 100644 --- a/src/vlogger/vlogger.cpp +++ b/src/vlogger/vlogger.cpp @@ -17,8 +17,6 @@ #include #include "utils/bullseye.h" -#include "vma/util/utils.h" -#include "vma/util/sys_vars.h" #define VLOG_DEFAULT_MODULE_NAME "VMA" #define VMA_LOG_CB_ENV_VAR "VMA_LOG_CB_FUNC_PTR" @@ -31,7 +29,7 @@ vlog_levels_t* g_p_vlogger_level = NULL; uint8_t g_vlogger_details = 0; uint8_t* g_p_vlogger_details = NULL; uint32_t g_vlogger_usec_on_startup = 0; -bool g_vlogger_log_in_colors = MCE_DEFAULT_LOG_COLORS; +bool g_vlogger_log_in_colors = false; vma_log_cb_t g_vlogger_cb = NULL; namespace log_level diff --git a/src/vma/dev/rfs.cpp b/src/vma/dev/rfs.cpp index 48588c916..562655c81 100644 --- a/src/vma/dev/rfs.cpp +++ b/src/vma/dev/rfs.cpp @@ -120,7 +120,7 @@ rfs::~rfs() delete[] m_sinks_list; while (m_attach_flow_data_vector.size() > 0) { - delete m_attach_flow_data_vector.back(); + free(m_attach_flow_data_vector.back()); m_attach_flow_data_vector.pop_back(); } } diff --git a/src/vma/dev/rfs.h b/src/vma/dev/rfs.h index adacb898a..8c1ea240e 100644 --- a/src/vma/dev/rfs.h +++ b/src/vma/dev/rfs.h @@ -8,6 +8,8 @@ #ifndef RFS_H #define RFS_H +#include +#include #include #include "vma/ib/base/verbs_extra.h" @@ -213,6 +215,13 @@ class rfs virtual bool rx_dispatch_packet(mem_buf_desc_t* p_rx_wc_buf_desc, void* pv_fd_ready_array) = 0; protected: + template + T * new_malloc(Args ... args) { + static_assert(std::is_trivially_destructible::value == true); + void * p = aligned_alloc(alignof(T), sizeof(T)); + return new(p) T(args...); + } + flow_tuple m_flow_tuple; ring_slave* m_p_ring; rfs_rule_filter* m_p_rule_filter; diff --git a/src/vma/dev/rfs_mc.cpp b/src/vma/dev/rfs_mc.cpp index 3b73f614c..869e78b50 100644 --- a/src/vma/dev/rfs_mc.cpp +++ b/src/vma/dev/rfs_mc.cpp @@ -56,7 +56,7 @@ bool rfs_mc::prepare_flow_spec() #ifdef DEFINED_IBV_FLOW_SPEC_IB attach_flow_data_ib_v1_t* attach_flow_data_ib_v1 = NULL; - attach_flow_data_ib_v1 = new attach_flow_data_ib_v1_t(p_ring->m_p_qp_mgr); + attach_flow_data_ib_v1 = new_malloc(p_ring->m_p_qp_mgr); uint8_t dst_gid[16]; create_mgid_from_ipv4_mc_ip(dst_gid, p_ring->m_p_qp_mgr->get_partiton(), m_flow_tuple.get_dst_ip()); @@ -70,7 +70,7 @@ bool rfs_mc::prepare_flow_spec() #endif } - attach_flow_data_ib_v2 = new attach_flow_data_ib_v2_t(p_ring->m_p_qp_mgr); + attach_flow_data_ib_v2 = new_malloc(p_ring->m_p_qp_mgr); ibv_flow_spec_ipv4_set(&(attach_flow_data_ib_v2->ibv_flow_attr.ipv4), m_flow_tuple.get_dst_ip(), @@ -88,7 +88,7 @@ bool rfs_mc::prepare_flow_spec() { attach_flow_data_eth_ipv4_tcp_udp_t* attach_flow_data_eth = NULL; - attach_flow_data_eth = new attach_flow_data_eth_ipv4_tcp_udp_t(p_ring->m_p_qp_mgr); + attach_flow_data_eth = new_malloc(p_ring->m_p_qp_mgr); uint8_t dst_mac[6]; create_multicast_mac_from_ip(dst_mac, m_flow_tuple.get_dst_ip()); diff --git a/src/vma/dev/rfs_uc.cpp b/src/vma/dev/rfs_uc.cpp index 708115251..b5f412ae1 100644 --- a/src/vma/dev/rfs_uc.cpp +++ b/src/vma/dev/rfs_uc.cpp @@ -59,7 +59,7 @@ bool rfs_uc::prepare_flow_spec() if (0 == p_ring->m_p_qp_mgr->get_underly_qpn()) { attach_flow_data_ib_ipv4_tcp_udp_v1_t* attach_flow_data_ib_v1 = NULL; - attach_flow_data_ib_v1 = new attach_flow_data_ib_ipv4_tcp_udp_v1_t(p_ring->m_p_qp_mgr); + attach_flow_data_ib_v1 = new_malloc(p_ring->m_p_qp_mgr); ibv_flow_spec_ib_set_by_dst_qpn(&(attach_flow_data_ib_v1->ibv_flow_attr.ib), htonl(((IPoIB_addr*)p_ring->m_p_l2_addr)->get_qpn())); p_ipv4 = &(attach_flow_data_ib_v1->ibv_flow_attr.ipv4); @@ -68,7 +68,7 @@ bool rfs_uc::prepare_flow_spec() break; } #endif - attach_flow_data_ib_v2 = new attach_flow_data_ib_ipv4_tcp_udp_v2_t(p_ring->m_p_qp_mgr); + attach_flow_data_ib_v2 = new_malloc(p_ring->m_p_qp_mgr); p_ipv4 = &(attach_flow_data_ib_v2->ibv_flow_attr.ipv4); p_tcp_udp = &(attach_flow_data_ib_v2->ibv_flow_attr.tcp_udp); @@ -77,7 +77,7 @@ bool rfs_uc::prepare_flow_spec() } case VMA_TRANSPORT_ETH: { - attach_flow_data_eth = new attach_flow_data_eth_ipv4_tcp_udp_t(p_ring->m_p_qp_mgr); + attach_flow_data_eth = new_malloc(p_ring->m_p_qp_mgr); ibv_flow_spec_eth_set(&(attach_flow_data_eth->ibv_flow_attr.eth), p_ring->m_p_l2_addr->get_address(), diff --git a/src/vma/dev/time_converter_ptp.h b/src/vma/dev/time_converter_ptp.h index 07e45eefc..34956c222 100644 --- a/src/vma/dev/time_converter_ptp.h +++ b/src/vma/dev/time_converter_ptp.h @@ -9,6 +9,7 @@ #define TIME_CONVERTER_PTP_H #include +#include "vma/ib/base/verbs_extra.h" #include "vma/event/timer_handler.h" #include #include "time_converter.h" diff --git a/src/vma/event/delta_timer.cpp b/src/vma/event/delta_timer.cpp index 1511845ea..2e86ac186 100644 --- a/src/vma/event/delta_timer.cpp +++ b/src/vma/event/delta_timer.cpp @@ -200,19 +200,11 @@ void timer::process_registered_timers() timer_node_t* iter = m_list_head; timer_node_t* next_iter; while (iter && (iter->delta_time_msec == milliseconds(0))) { - tmr_logfuncall("timer expired on %p", iter->handler); - - /* Special check is need to protect - * from using destroyed object pointed by handler - * See unregister_timer_event() - * Object can be destoyed from another thread (lock protection) - * and from current thread (lock and lock count condition) - */ - if (iter->handler && - !iter->lock_timer.trylock() && - (1 == iter->lock_timer.is_locked_by_me())) { - iter->handler->handle_timer_expired(iter->user_data); - iter->lock_timer.unlock(); + timer_handler * handler = iter->handler.load(); + tmr_logfuncall("timer expired on %p", handler); + + if (handler) { + handler->safe_handle_timer_expired(iter->user_data); } next_iter = iter->next; @@ -225,13 +217,13 @@ void timer::process_registered_timers() break; case ONE_SHOT_TIMER: - remove_timer(iter, iter->handler); + remove_timer(iter, handler); break; BULLSEYE_EXCLUDE_BLOCK_START case INVALID_TIMER: default: - tmr_logwarn("invalid timer expired on %p", iter->handler); + tmr_logwarn("invalid timer expired on %p", handler); break; } BULLSEYE_EXCLUDE_BLOCK_END diff --git a/src/vma/event/delta_timer.h b/src/vma/event/delta_timer.h index 1532b44f6..2e8933f8f 100644 --- a/src/vma/event/delta_timer.h +++ b/src/vma/event/delta_timer.h @@ -8,6 +8,7 @@ #ifndef DELTA_TIMER_H #define DELTA_TIMER_H +#include #include #include "utils/lock_wrapper.h" @@ -32,18 +33,13 @@ struct timer_node_t { std::chrono::milliseconds delta_time_msec; /* the orig timer requested (saved in order to re-register periodic timers) */ std::chrono::milliseconds orig_time_msec; - /* control thread-safe access to handler. Recursive because unregister_timer_event() - * can be called from handle_timer_expired() - * that is under trylock() inside process_registered_timers - */ - lock_spin_recursive lock_timer; /* link to the context registered */ - timer_handler* handler; - void* user_data; - timers_group* group; - timer_req_type_t req_type; - struct timer_node_t* next; - struct timer_node_t* prev; + std::atomic handler; + void* user_data; + timers_group* group; + timer_req_type_t req_type; + struct timer_node_t* next; + struct timer_node_t* prev; }; // used by the list class timer diff --git a/src/vma/event/event_handler_manager.cpp b/src/vma/event/event_handler_manager.cpp index 20f6adc75..e2b074698 100644 --- a/src/vma/event/event_handler_manager.cpp +++ b/src/vma/event/event_handler_manager.cpp @@ -74,9 +74,6 @@ void* event_handler_manager::register_timer_event(int timeout_msec, timer_handle } BULLSEYE_EXCLUDE_BLOCK_END - timer_node_t* timer_node = (timer_node_t*)node; - timer_node->lock_timer=lock_spin_recursive("timer"); - reg_action_t reg_action; memset(®_action, 0, sizeof(reg_action)); reg_action.type = REGISTER_TIMER; @@ -116,30 +113,19 @@ void event_handler_manager::unregister_timer_event(timer_handler* handler, void* reg_action.type = UNREGISTER_TIMER; reg_action.info.timer.handler = handler; reg_action.info.timer.node = node; - - /* Special protection is needed to avoid scenario when deregistration is done - * during timer_handler object destruction, timer node itself is not removed - * and time for this timer node is expired. In this case there is no guarantee - * to operate with timer_handler object. - * See timer::process_registered_timers() - * Do just lock() to protect timer_handler inside process_registered_timers() - */ - if (node) { - timer_node_t* timer_node = (timer_node_t*)node; - timer_node->lock_timer.lock(); - } - post_new_reg_action(reg_action); } void event_handler_manager::unregister_timers_event_and_delete(timer_handler* handler) { evh_logdbg("timer handler '%p'", handler); - reg_action_t reg_action; - memset(®_action, 0, sizeof(reg_action)); - reg_action.type = UNREGISTER_TIMERS_AND_DELETE; - reg_action.info.timer.handler = handler; - post_new_reg_action(reg_action); + if( handler != nullptr && !handler->set_destroying_state()) { + reg_action_t reg_action; + memset(®_action, 0, sizeof(reg_action)); + reg_action.type = UNREGISTER_TIMERS_AND_DELETE; + reg_action.info.timer.handler = handler; + post_new_reg_action(reg_action); + } } void event_handler_manager::register_ibverbs_event(int fd, event_handler_ibverbs *handler, diff --git a/src/vma/event/timer_handler.h b/src/vma/event/timer_handler.h index c0cd65331..d8acf2088 100644 --- a/src/vma/event/timer_handler.h +++ b/src/vma/event/timer_handler.h @@ -8,6 +8,11 @@ #ifndef TIMER_HANDLER_H #define TIMER_HANDLER_H +#include + +#include "vlogger/vlogger.h" +#include "utils/lock_wrapper.h" + /** * simple timer notification. * Any class that inherit timer_handler should also inherit cleanable_obj, and use clean_obj instead of delete. @@ -15,9 +20,58 @@ */ class timer_handler { -public: - virtual ~timer_handler() {}; +private: + lock_spin m_handle_mutex{"timer_handler"}; + std::atomic m_destroy_in_progress{false}; +protected: virtual void handle_timer_expired(void* user_data) = 0; + +public: + timer_handler() = default; + + virtual ~timer_handler() { + if( !m_destroy_in_progress.load()) { + m_destroy_in_progress = true; + vlog_printf(VLOG_DEBUG, "Destroying timer_handler without destroy in progress.\n + } + { + m_handle_mutex.lock(); + m_handle_mutex.unlock(); + } + }; + + void safe_handle_timer_expired(void* user_data) { + if(!m_destroy_in_progress.load()) { + if (m_handle_mutex.trylock() == 0) { + handle_timer_expired(user_data); + m_handle_mutex.unlock(); + } + } + } + +/** + * Sets the destroying state of the object to indicate that destruction is in progress. + * + * If `wait_for_handler` is set to true, this method will ensure that the mutex + * used for handling operations (m_handle_mutex) is acquired and released, effectively + * waiting for any pending handler operation to complete. + * + * @param wait_for_handler If true, waits for the handler mutex to ensure + * handler finish before proceeding. + * Defaults to false. + * + * @return The previous state of the destruction flag. + * Returns true if a destruction process was already in progress before + * this method was called, otherwise false. + */ + bool set_destroying_state(bool wait_for_handler = false) { + bool result = m_destroy_in_progress.exchange(true); + if( wait_for_handler ) { + m_handle_mutex.lock(); + m_handle_mutex.unlock(); + } + return result; + } }; #endif diff --git a/src/vma/sock/fd_collection.cpp b/src/vma/sock/fd_collection.cpp index c210843e8..c4df6b9e2 100644 --- a/src/vma/sock/fd_collection.cpp +++ b/src/vma/sock/fd_collection.cpp @@ -635,7 +635,7 @@ void fd_collection::handle_timer_expired(void* user_data) if (si_tcp) { //In case of TCP socket progress the TCP connection fdcoll_logfunc("Call to handler timer of TCP socket:%d", (*itr)->get_fd()); - si_tcp->handle_timer_expired(NULL); + si_tcp->safe_handle_timer_expired(NULL); } itr++; } diff --git a/src/vma/sock/sockinfo_tcp.cpp b/src/vma/sock/sockinfo_tcp.cpp index 95f944752..9fefecbe1 100644 --- a/src/vma/sock/sockinfo_tcp.cpp +++ b/src/vma/sock/sockinfo_tcp.cpp @@ -4798,8 +4798,9 @@ void tcp_timers_collection::handle_timer_expired(void* user_data) NOT_IN_USE(user_data); timer_node_t* iter = m_p_intervals[m_n_location]; while (iter) { - __log_funcall("timer expired on %p", iter->handler); - iter->handler->handle_timer_expired(iter->user_data); + timer_handler * handler = iter->handler.load(); + __log_funcall("timer expired on %p", handler); + handler->safe_handle_timer_expired(iter->user_data); iter = iter->next; } m_n_location = (m_n_location + 1) % m_n_intervals_size; @@ -4859,7 +4860,7 @@ void tcp_timers_collection::remove_timer(timer_node_t* node) } } - __log_dbg("TCP timer handler [%p] was removed", node->handler); + __log_dbg("TCP timer handler [%p] was removed", node->handler.load()); free(node); } diff --git a/src/vma/util/sys_vars.cpp b/src/vma/util/sys_vars.cpp index f59327ca3..a1fdb8f80 100644 --- a/src/vma/util/sys_vars.cpp +++ b/src/vma/util/sys_vars.cpp @@ -276,15 +276,21 @@ int mce_sys_var::hex_to_cpuset(char *start, cpu_set_t *cpu_set) int mce_sys_var::env_to_cpuset(char *orig_start, cpu_set_t *cpu_set) { int ret; - char* start = strdup(orig_start); // save the caller string from strtok destruction. + memset(&(cpu_set->__bits), 0, sizeof(cpu_set->__bits)); + int len = strlen(orig_start); + if (len == 2 && orig_start[0] == '-' && orig_start[1] == '1') { + return 0; + } + + char *start = strdup(orig_start); // save the caller string from strtok destruction. /* * We expect a hex number or comma delimited cpulist. Check for * starting characters of "0x" or "0X" and if present then parse * the string as a hexidecimal value, otherwise treat it as a * cpulist. */ - if ((strlen(start) > 2) && + if ((len > 2) && (start[0] == '0') && ((start[1] == 'x') || (start[1] == 'X'))) { ret = hex_to_cpuset(start + 2, cpu_set); diff --git a/src/vma/util/utils.cpp b/src/vma/util/utils.cpp index 4c31db0a1..985b2af0a 100644 --- a/src/vma/util/utils.cpp +++ b/src/vma/util/utils.cpp @@ -414,13 +414,24 @@ int priv_read_file(const char *path, char *buf, size_t size, vlog_levels_t log_l int read_file_to_int(const char *path, int default_value) { - int value = -1; - std::ifstream file_stream(path); - if (!file_stream || !(file_stream >> value)) { - __log_warn("ERROR while getting int from from file %s, we'll use default %d", path, default_value); - return default_value; + int fd, sz; + char c[21] = {}; + + fd = open(path, O_RDONLY); + if (fd >= 0) { + sz = read(fd, c, 20); + if (sz > 0) { + int value; + c[sz] = '\0'; + int n = sscanf(reinterpret_cast(&c), "%d", &value); + if (n == 1) { + return value; + } + } } - return value; + + __log_warn("ERROR while getting int from from file %s, we'll use default %d", path, default_value); + return default_value; } int get_ifinfo_from_ip(const struct sockaddr& addr, char* ifname, uint32_t& ifflags)