From 77a0551f92c3a7b921fc276a5c39de4badd2cecd Mon Sep 17 00:00:00 2001 From: thiagoftsm Date: Tue, 11 Apr 2023 12:01:07 +0000 Subject: eBPF bug fixes (#14869) --- collectors/ebpf.plugin/ebpf.c | 3 +- collectors/ebpf.plugin/ebpf_hardirq.c | 179 +++++++++++++++++++++++++++------- collectors/ebpf.plugin/ebpf_hardirq.h | 12 +-- collectors/ebpf.plugin/ebpf_oomkill.c | 2 +- 4 files changed, 154 insertions(+), 42 deletions(-) (limited to 'collectors') diff --git a/collectors/ebpf.plugin/ebpf.c b/collectors/ebpf.plugin/ebpf.c index 44e072b24b..3468bb1485 100644 --- a/collectors/ebpf.plugin/ebpf.c +++ b/collectors/ebpf.plugin/ebpf.c @@ -1820,8 +1820,9 @@ void set_global_variables() ebpf_configured_log_dir = LOG_DIR; ebpf_nprocs = (int)sysconf(_SC_NPROCESSORS_ONLN); - if (ebpf_nprocs > NETDATA_MAX_PROCESSOR) { + if (ebpf_nprocs < 0) { ebpf_nprocs = NETDATA_MAX_PROCESSOR; + error("Cannot identify number of process, using default value %d", ebpf_nprocs); } isrh = get_redhat_release(); diff --git a/collectors/ebpf.plugin/ebpf_hardirq.c b/collectors/ebpf.plugin/ebpf_hardirq.c index 1b1ea8ab4c..ff98704dc1 100644 --- a/collectors/ebpf.plugin/ebpf_hardirq.c +++ b/collectors/ebpf.plugin/ebpf_hardirq.c @@ -129,11 +129,54 @@ static hardirq_static_val_t hardirq_static_vals[] = { // thread will write to netdata agent. static avl_tree_lock hardirq_pub; -// tmp store for dynamic hard IRQ values we get from a per-CPU eBPF map. -static hardirq_ebpf_val_t *hardirq_ebpf_vals = NULL; +/***************************************************************** + * + * ARAL SECTION + * + *****************************************************************/ + +// ARAL vectors used to speed up processing +ARAL *ebpf_aral_hardirq = NULL; -// tmp store for static hard IRQ values we get from a per-CPU eBPF map. -static hardirq_ebpf_static_val_t *hardirq_ebpf_static_vals = NULL; +/** + * eBPF hardirq Aral init + * + * Initiallize array allocator that will be used when integration with apps is enabled. + */ +static inline void ebpf_hardirq_aral_init() +{ + ebpf_aral_hardirq = ebpf_allocate_pid_aral(NETDATA_EBPF_HARDIRQ_ARAL_NAME, sizeof(hardirq_val_t)); +} + +/** + * eBPF hardirq get + * + * Get a hardirq_val_t entry to be used with a specific IRQ. + * + * @return it returns the address on success. + */ +hardirq_val_t *ebpf_hardirq_get(void) +{ + hardirq_val_t *target = aral_mallocz(ebpf_aral_hardirq); + memset(target, 0, sizeof(hardirq_val_t)); + return target; +} + +/** + * eBPF hardirq release + * + * @param stat Release a target after usage. + */ +void ebpf_hardirq_release(hardirq_val_t *stat) +{ + aral_freez(ebpf_aral_hardirq, stat); +} + +/***************************************************************** + * + * EXIT FUNCTIONS + * + *****************************************************************/ /** * Hardirq Free @@ -151,9 +194,6 @@ static void ebpf_hardirq_free(ebpf_module_t *em) for (int i = 0; hardirq_tracepoints[i].class != NULL; i++) { ebpf_disable_tracepoint(&hardirq_tracepoints[i]); } - freez(hardirq_ebpf_vals); - freez(hardirq_ebpf_static_vals); - pthread_mutex_lock(&ebpf_exit_cleanup); em->thread->enabled = NETDATA_THREAD_EBPF_STOPPED; pthread_mutex_unlock(&ebpf_exit_cleanup); @@ -200,8 +240,82 @@ static int hardirq_val_cmp(void *a, void *b) } } -static void hardirq_read_latency_map(int mapfd) +/** + * Parse interrupts + * + * Parse /proc/interrupts to get names used in metrics + * + * @param irq_name vector to store data. + * @param irq irq value + * + * @return It returns 0 on success and -1 otherwise + */ +static int hardirq_parse_interrupts(char *irq_name, int irq) { + static procfile *ff = NULL; + static int cpus = -1; + if(unlikely(!ff)) { + char filename[FILENAME_MAX + 1]; + snprintfz(filename, FILENAME_MAX, "%s%s", netdata_configured_host_prefix, "/proc/interrupts"); + ff = procfile_open(filename, " \t:", PROCFILE_FLAG_DEFAULT); + } + if(unlikely(!ff)) + return -1; + + ff = procfile_readall(ff); + if(unlikely(!ff)) + return -1; // we return 0, so that we will retry to open it next time + + size_t words = procfile_linewords(ff, 0); + if(unlikely(cpus == -1)) { + uint32_t w; + cpus = 0; + for(w = 0; w < words ; w++) { + if(likely(strncmp(procfile_lineword(ff, 0, w), "CPU", 3) == 0)) + cpus++; + } + } + + size_t lines = procfile_lines(ff), l; + if(unlikely(!lines)) { + collector_error("Cannot read /proc/interrupts, zero lines reported."); + return -1; + } + + for(l = 1; l < lines ;l++) { + words = procfile_linewords(ff, l); + if(unlikely(!words)) continue; + const char *id = procfile_lineword(ff, l, 0); + if (!isdigit(id[0])) + continue; + + int cmp = str2i(id); + if (cmp != irq) + continue; + + if(unlikely((uint32_t)(cpus + 2) < words)) { + const char *name = procfile_lineword(ff, l, words - 1); + // On some motherboards IRQ can have the same name, so we append IRQ id to differentiate. + snprintfz(irq_name, NETDATA_HARDIRQ_NAME_LEN - 1, "%d_%s", irq, name); + } + } + + return 0; +} + +/** + * Read Latency MAP + * + * Read data from kernel ring to user ring. + * + * @param mapfd hash map id. + * + * @return it returns 0 on success and -1 otherwise + */ +static int hardirq_read_latency_map(int mapfd) +{ + hardirq_ebpf_static_val_t hardirq_ebpf_vals[ebpf_nprocs + 1]; + hardirq_ebpf_key_t key = {}; hardirq_ebpf_key_t next_key = {}; hardirq_val_t search_v = {}; @@ -234,7 +348,7 @@ static void hardirq_read_latency_map(int mapfd) if (unlikely(v == NULL)) { // latency/name can only be added reliably at a later time. // when they're added, only then will we AVL insert. - v = callocz(1, sizeof(hardirq_val_t)); + v = ebpf_hardirq_get(); v->irq = key.irq; v->dim_exists = false; @@ -246,22 +360,10 @@ static void hardirq_read_latency_map(int mapfd) // 2. the name is unfortunately *not* available on all CPU maps - only // a single map contains the name, so we must find it. we only need // to copy it though if the IRQ is new for us. - bool name_saved = false; uint64_t total_latency = 0; int i; - int end = (running_on_kernel < NETDATA_KERNEL_V4_15) ? 1 : ebpf_nprocs; - for (i = 0; i < end; i++) { + for (i = 0; i < ebpf_nprocs; i++) { total_latency += hardirq_ebpf_vals[i].latency/1000; - - // copy name for new IRQs. - if (v_is_new && !name_saved && hardirq_ebpf_vals[i].name[0] != '\0') { - strncpyz( - v->name, - hardirq_ebpf_vals[i].name, - NETDATA_HARDIRQ_NAME_LEN - ); - name_saved = true; - } } // can now safely publish latency for existing IRQs. @@ -269,6 +371,11 @@ static void hardirq_read_latency_map(int mapfd) // can now safely publish new IRQ. if (v_is_new) { + if (hardirq_parse_interrupts(v->name, v->irq)) { + ebpf_hardirq_release(v); + return -1; + } + avl_t *check = avl_insert_lock(&hardirq_pub, (avl_t *)v); if (check != (avl_t *)v) { error("Internal error, cannot insert the AVL tree."); @@ -277,10 +384,14 @@ static void hardirq_read_latency_map(int mapfd) key = next_key; } + + return 0; } static void hardirq_read_latency_static_map(int mapfd) { + hardirq_ebpf_static_val_t hardirq_ebpf_static_vals[ebpf_nprocs + 1]; + uint32_t i; for (i = 0; i < HARDIRQ_EBPF_STATIC_END; i++) { uint32_t map_i = hardirq_static_vals[i].idx; @@ -302,11 +413,17 @@ static void hardirq_read_latency_static_map(int mapfd) /** * Read eBPF maps for hard IRQ. + * + * @return When it is not possible to parse /proc, it returns -1, on success it returns 0; */ -static void hardirq_reader() +static int hardirq_reader() { - hardirq_read_latency_map(hardirq_maps[HARDIRQ_MAP_LATENCY].map_fd); + if (hardirq_read_latency_map(hardirq_maps[HARDIRQ_MAP_LATENCY].map_fd)) + return -1; + hardirq_read_latency_static_map(hardirq_maps[HARDIRQ_MAP_LATENCY_STATIC].map_fd); + + return 0; } static void hardirq_create_charts(int update_every) @@ -375,16 +492,8 @@ static inline void hardirq_write_static_dims() */ static void hardirq_collector(ebpf_module_t *em) { - hardirq_ebpf_vals = callocz( - (running_on_kernel < NETDATA_KERNEL_V4_15) ? 1 : ebpf_nprocs, - sizeof(hardirq_ebpf_val_t) - ); - hardirq_ebpf_static_vals = callocz( - (running_on_kernel < NETDATA_KERNEL_V4_15) ? 1 : ebpf_nprocs, - sizeof(hardirq_ebpf_static_val_t) - ); - avl_init_lock(&hardirq_pub, hardirq_val_cmp); + ebpf_hardirq_aral_init(); // create chart and static dims. pthread_mutex_lock(&lock); @@ -407,7 +516,9 @@ static void hardirq_collector(ebpf_module_t *em) continue; counter = 0; - hardirq_reader(); + if (hardirq_reader()) + break; + pthread_mutex_lock(&lock); // write dims now for all hitherto discovered IRQs. diff --git a/collectors/ebpf.plugin/ebpf_hardirq.h b/collectors/ebpf.plugin/ebpf_hardirq.h index fe38b1bb12..52dea1e560 100644 --- a/collectors/ebpf.plugin/ebpf_hardirq.h +++ b/collectors/ebpf.plugin/ebpf_hardirq.h @@ -3,6 +3,9 @@ #ifndef NETDATA_EBPF_HARDIRQ_H #define NETDATA_EBPF_HARDIRQ_H 1 +#include +#include "libnetdata/avl/avl.h" + /***************************************************************** * copied from kernel-collectors repo, with modifications needed * for inclusion here. @@ -15,12 +18,6 @@ typedef struct hardirq_ebpf_key { int irq; } hardirq_ebpf_key_t; -typedef struct hardirq_ebpf_val { - uint64_t latency; - uint64_t ts; - char name[NETDATA_HARDIRQ_NAME_LEN]; -} hardirq_ebpf_val_t; - enum hardirq_ebpf_static { HARDIRQ_EBPF_STATIC_APIC_THERMAL, HARDIRQ_EBPF_STATIC_APIC_THRESHOLD, @@ -46,6 +43,9 @@ typedef struct hardirq_ebpf_static_val { * below this is eBPF plugin-specific code. *****************************************************************/ +// ARAL Name +#define NETDATA_EBPF_HARDIRQ_ARAL_NAME "ebpf_harddirq" + #define NETDATA_EBPF_MODULE_NAME_HARDIRQ "hardirq" #define NETDATA_HARDIRQ_CONFIG_FILE "hardirq.conf" diff --git a/collectors/ebpf.plugin/ebpf_oomkill.c b/collectors/ebpf.plugin/ebpf_oomkill.c index d5f3481d20..1687a8d5d8 100644 --- a/collectors/ebpf.plugin/ebpf_oomkill.c +++ b/collectors/ebpf.plugin/ebpf_oomkill.c @@ -299,7 +299,7 @@ static void oomkill_collector(ebpf_module_t *em) int counter = update_every - 1; while (!ebpf_exit_plugin) { (void)heartbeat_next(&hb, USEC_PER_SEC); - if (!ebpf_exit_plugin || ++counter != update_every) + if (ebpf_exit_plugin || ++counter != update_every) continue; counter = 0; -- cgit v1.2.3