cleanup(libscap): refine scap_stats_v2

Co-authored-by: Grzegorz Nosek <grzegorz.nosek@sysdig.com>
Signed-off-by: Melissa Kilby <melissa.kilby.oss@gmail.com>

userspace/libpman/include/libpman.h

@@ -298,9 +298,10 @@ extern "C"
 	 * @param scap_stats_v2_struct opaque pointer to `struct scap_stats_v2` held in modern_bpf_engine handle
 	 * @param flags holding statistics category flags.
 	 * @param nstats Pointer reflecting number of statistics in returned buffer.
-	 * @param rc Pointer to return code.
+	 *
+	 * @return `0` on success, `errno` in case of error.
 	 */
-	void* pman_get_scap_stats_v2(void* scap_stats_v2_struct, uint32_t flags, uint32_t* nstats, int32_t* rc);
+	int pman_get_scap_stats_v2(void* scap_stats_v2_struct, uint32_t flags, uint32_t* nstats);
 
 	/**
 	 * @brief Receive an array with `nCPUs` elements. For every CPU

userspace/libpman/src/capture.c

@@ -137,6 +137,7 @@ int pman_get_scap_stats(void *scap_stats_struct)
 	struct scap_stats *stats = (struct scap_stats *)scap_stats_struct;
 	char error_message[MAX_ERROR_MESSAGE_LEN];
 	struct counter_map cnt_map;
+	int ret;
 
 	if(!stats)
 	{
@@ -180,14 +181,15 @@ int pman_get_scap_stats(void *scap_stats_struct)
 	return errno;
 }
 
-void* pman_get_scap_stats_v2(void *scap_stats_v2_struct, uint32_t flags, uint32_t* nstats, int32_t* rc)
+int pman_get_scap_stats_v2(void* scap_stats_v2_struct, uint32_t flags, uint32_t* nstats)
 {
 	struct scap_stats_v2 *stats = (struct scap_stats_v2 *)scap_stats_v2_struct;
 	*nstats = 0;
+	int ret;
 	if (!stats)
 	{
 		pman_print_error("pointer to scap_stats_v2 is empty");
-		*rc = errno;
+		return SCAP_FAILURE;
 	}
 
 	if ((flags & PPM_SCAP_STATS_KERNEL_COUNTERS))
@@ -197,13 +199,14 @@ void* pman_get_scap_stats_v2(void *scap_stats_v2_struct, uint32_t flags, uint32_
 		int counter_maps_fd = bpf_map__fd(g_state.skel->maps.counter_maps);
 		if(counter_maps_fd <= 0)
 		{
-			*rc = errno;
+			pman_print_error("unable to get counter maps");
+			return SCAP_FAILURE;
 		}
 
 		/* We always take statistics from all the CPUs, even if some of them are not online.
 		* If the CPU is not online the counter map will be empty.
 		*/
-		for(uint32_t stat =  0;  stat < MODERN_BPF_MAX_KERNEL_COUNTERS_STATS; stat++)
+		for(uint32_t stat = 0; stat < MODERN_BPF_MAX_KERNEL_COUNTERS_STATS; stat++)
 		{
 			stats[stat].type = STATS_VALUE_TYPE_U64;
 			stats[stat].flags = PPM_SCAP_STATS_KERNEL_COUNTERS;
@@ -213,12 +216,12 @@ void* pman_get_scap_stats_v2(void *scap_stats_v2_struct, uint32_t flags, uint32_
 
 		for(uint32_t index = 0; index < g_state.n_possible_cpus; index++)
 		{
-			if(bpf_map_lookup_elem(counter_maps_fd, &index, &cnt_map) < 0)
+			if((ret = bpf_map_lookup_elem(counter_maps_fd, &index, &cnt_map)) != 0)
 			{
 				snprintf(error_message, MAX_ERROR_MESSAGE_LEN, "unbale to get the counter map for CPU %d", index);
 				pman_print_error((const char *)error_message);
 				close(counter_maps_fd);
-				*rc = errno;
+				return -ret;
 			}
 			stats[MODERN_BPF_N_EVTS].value.u64 += cnt_map.n_evts;
 			stats[MODERN_BPF_N_DROPS_BUFFER_TOTAL].value.u64 += cnt_map.n_drops_buffer;
@@ -229,7 +232,7 @@ void* pman_get_scap_stats_v2(void *scap_stats_v2_struct, uint32_t flags, uint32_
 	}
 	// todo @incertum add libbpf stats for modern_bpf
 	*nstats = MODERN_BPF_MAX_KERNEL_COUNTERS_STATS;
-	*rc = SCAP_SUCCESS;
+	return SCAP_SUCCESS;
 }
 
 int pman_get_n_tracepoint_hit(long *n_events_per_cpu)

userspace/libscap/engine/bpf/bpf.h

@@ -21,7 +21,7 @@ limitations under the License.
 #include "../../ringbuffer/devset.h"
 #include "scap_open.h"
 #include "attached_prog.h"
-#include "scap.h"
+#include "scap_stats_v2.h"
 #include "scap_bpf_stats.h"
 
 //
@@ -58,4 +58,5 @@ struct bpf_engine
 	uint64_t m_schema_version;
 	bool capturing;
 	scap_stats_v2* m_stats;
+	uint32_t m_nstats;
 };

userspace/libscap/engine/bpf/scap_bpf.c

@@ -49,6 +49,33 @@ limitations under the License.
 #include "noop.h"
 #include "strerror.h"
 
+static const char * const bpf_kernel_counters_stats_names[] = {
+	[BPF_N_EVTS] = "n_evts",
+	[BPF_N_DROPS_BUFFER_TOTAL] = "n_drops_buffer_total",
+	[BPF_N_DROPS_BUFFER_CLONE_FORK_ENTER] = "n_drops_buffer_clone_fork_enter",
+	[BPF_N_DROPS_BUFFER_CLONE_FORK_EXIT] = "n_drops_buffer_clone_fork_exit",
+	[BPF_N_DROPS_BUFFER_EXECVE_ENTER] = "n_drops_buffer_execve_enter",
+	[BPF_N_DROPS_BUFFER_EXECVE_EXIT] = "n_drops_buffer_execve_exit",
+	[BPF_N_DROPS_BUFFER_CONNECT_ENTER] = "n_drops_buffer_connect_enter",
+	[BPF_N_DROPS_BUFFER_CONNECT_EXIT] = "n_drops_buffer_connect_exit",
+	[BPF_N_DROPS_BUFFER_OPEN_ENTER] = "n_drops_buffer_open_enter",
+	[BPF_N_DROPS_BUFFER_OPEN_EXIT] = "n_drops_buffer_open_exit",
+	[BPF_N_DROPS_BUFFER_DIR_FILE_ENTER] = "n_drops_buffer_dir_file_enter",
+	[BPF_N_DROPS_BUFFER_DIR_FILE_EXIT] = "n_drops_buffer_dir_file_exit",
+	[BPF_N_DROPS_BUFFER_OTHER_INTEREST_ENTER] = "n_drops_buffer_other_interest_enter",
+	[BPF_N_DROPS_BUFFER_OTHER_INTEREST_EXIT] = "n_drops_buffer_other_interest_exit",
+	[BPF_N_DROPS_SCRATCH_MAP] = "n_drops_scratch_map",
+	[BPF_N_DROPS_PAGE_FAULTS] = "n_drops_page_faults",
+	[BPF_N_DROPS_BUG] = "n_drops_bug",
+	[BPF_N_DROPS] = "n_drops",
+};
+
+static const char * const bpf_libbpf_stats_names[] = {
+	[RUN_CNT] = ".run_cnt", ///< `bpf_prog_info` run_cnt.
+	[RUN_TIME_NS] = ".run_time_ns", ///<`bpf_prog_info` run_time_ns.
+	[AVG_TIME_NS] = ".avg_time_ns", ///< Average time spent in bpg program, calculation: run_time_ns / run_cnt.
+};
+
 static inline scap_evt* scap_bpf_next_event(scap_device* dev)
 {
 	return scap_bpf_evt_from_perf_sample(dev->m_sn_next_event);
@@ -821,15 +848,17 @@ static int load_all_progs(struct bpf_engine *handle)
 
 static int allocate_scap_stats_v2(struct bpf_engine *handle)
 {
-	int nprogs = 0;
+	int nprogs_attached = 0;
 	for(int j=0; j < BPF_PROG_ATTACHED_MAX; j++)
 	{
 		if (handle->m_attached_progs[j].fd != -1)
 		{
-			nprogs++;
+			nprogs_attached++;
 		}
 	}
-	handle->m_stats = (scap_stats_v2 *)malloc((BPF_MAX_KERNEL_COUNTERS_STATS + (nprogs * BPF_MAX_LIBBPF_STATS)) * sizeof(scap_stats_v2));
+	handle->m_nstats = (BPF_MAX_KERNEL_COUNTERS_STATS + (nprogs_attached * BPF_MAX_LIBBPF_STATS));
+	handle->m_stats = (scap_stats_v2 *)malloc(handle->m_nstats * sizeof(scap_stats_v2));
+
 	if(!handle->m_stats)
 	{
 		return SCAP_FAILURE;
@@ -1633,24 +1662,25 @@ int32_t scap_bpf_get_stats(struct scap_engine_handle engine, OUT scap_stats* sta
 	return SCAP_SUCCESS;
 }
 
-struct scap_stats_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine, uint32_t flags, OUT uint32_t* nstats, OUT int32_t* rc)
+const struct scap_stats_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine, uint32_t flags, OUT uint32_t* nstats, OUT int32_t* rc)
 {
 	struct bpf_engine *handle = engine.m_handle;
 	int ret;
 	int fd;
-	int i = 0; // offset in stats
+	int offset = 0; // offset in stats buffer
 	*nstats = 0;
+	uint32_t nstats_allocated = handle->m_nstats;
 	scap_stats_v2* stats = handle->m_stats;
 	if (!stats)
 	{
 		*rc = SCAP_FAILURE;
 		return NULL;
 	}
 
-	if ((flags & PPM_SCAP_STATS_KERNEL_COUNTERS))
+	if ((flags & PPM_SCAP_STATS_KERNEL_COUNTERS) && (BPF_MAX_KERNEL_COUNTERS_STATS < nstats_allocated))
 	{
 		/* KERNEL SIDE STATS COUNTERS */
-		for(uint32_t stat =  0;  stat < BPF_MAX_KERNEL_COUNTERS_STATS; stat++)
+		for(int stat = 0;  stat < BPF_MAX_KERNEL_COUNTERS_STATS; stat++)
 		{
 			stats[stat].type = STATS_VALUE_TYPE_U64;
 			stats[stat].flags = PPM_SCAP_STATS_KERNEL_COUNTERS;
@@ -1688,7 +1718,7 @@ struct scap_stats_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine, ui
 				v.n_drops_pf + \
 				v.n_drops_bug;
 		}
-		i = BPF_MAX_KERNEL_COUNTERS_STATS;
+		offset = BPF_MAX_KERNEL_COUNTERS_STATS;
 	}
 
 	/* LIBBPF STATS */
@@ -1717,37 +1747,41 @@ struct scap_stats_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine, ui
 				continue;
 			}
 
-			for(int stat =  0;  stat < BPF_MAX_LIBBPF_STATS; stat++)
+			for(int stat = 0; stat < BPF_MAX_LIBBPF_STATS; stat++)
 			{
-				stats[i].type = STATS_VALUE_TYPE_U64;
-				stats[i].flags = PPM_SCAP_STATS_LIBBPF_STATS;
-				strlcpy(stats[i].name, info.name, STATS_NAME_MAX);
-				size_t dest_len = strlen(stats[i].name);
+				if (offset > nstats_allocated - 1)
+				{
+					break;
+				}
+				stats[offset].type = STATS_VALUE_TYPE_U64;
+				stats[offset].flags = PPM_SCAP_STATS_LIBBPF_STATS;
+				strlcpy(stats[offset].name, info.name, STATS_NAME_MAX);
+				size_t dest_len = strlen(stats[offset].name);
 				switch(stat)
 				{
 				case RUN_CNT:
-					strncat(stats[i].name, bpf_libbpf_stats_names[RUN_CNT], sizeof(stats[i].name) - dest_len);
-					stats[i].value.u64 = info.run_cnt;
+					strncat(stats[offset].name, bpf_libbpf_stats_names[RUN_CNT], sizeof(stats[offset].name) - dest_len);
+					stats[offset].value.u64 = info.run_cnt;
 					break;
 				case RUN_TIME_NS:
-					strncat(stats[i].name, bpf_libbpf_stats_names[RUN_TIME_NS], sizeof(stats[i].name) - dest_len);
-					stats[i].value.u64 = info.run_time_ns;
+					strncat(stats[offset].name, bpf_libbpf_stats_names[RUN_TIME_NS], sizeof(stats[offset].name) - dest_len);
+					stats[offset].value.u64 = info.run_time_ns;
 					break;
 				case AVG_TIME_NS:
 					if (info.run_cnt > 0)
 					{
-						strncat(stats[i].name, bpf_libbpf_stats_names[AVG_TIME_NS], sizeof(stats[i].name) - dest_len);
-						stats[i].value.u64 = info.run_time_ns / info.run_cnt;
+						strncat(stats[offset].name, bpf_libbpf_stats_names[AVG_TIME_NS], sizeof(stats[offset].name) - dest_len);
+						stats[offset].value.u64 = info.run_time_ns / info.run_cnt;
 					}
 					break;
 				default:
 					break;
 				}
-				i++;
+				offset++;
 			}
 		}
 	}
-	*nstats = i; // return true number of stats that were available as libbpf metrics are a function of attached progs
+	*nstats = offset; // return true number of stats that were available as libbpf metrics are a function of attached progs
 	*rc = SCAP_SUCCESS;
 	return stats;
 }

userspace/libscap/engine/bpf/scap_bpf_stats.h

@@ -39,36 +39,9 @@ typedef enum bpf_kernel_counters_stats {
 	BPF_MAX_KERNEL_COUNTERS_STATS
 }bpf_kernel_counters_stats;
 
-static const char * const bpf_kernel_counters_stats_names[] = {
-	[BPF_N_EVTS] = "n_evts",
-	[BPF_N_DROPS_BUFFER_TOTAL] = "n_drops_buffer_total",
-	[BPF_N_DROPS_BUFFER_CLONE_FORK_ENTER] = "n_drops_buffer_clone_fork_enter",
-	[BPF_N_DROPS_BUFFER_CLONE_FORK_EXIT] = "n_drops_buffer_clone_fork_exit",
-	[BPF_N_DROPS_BUFFER_EXECVE_ENTER] = "n_drops_buffer_execve_enter",
-	[BPF_N_DROPS_BUFFER_EXECVE_EXIT] = "n_drops_buffer_execve_exit",
-	[BPF_N_DROPS_BUFFER_CONNECT_ENTER] = "n_drops_buffer_connect_enter",
-	[BPF_N_DROPS_BUFFER_CONNECT_EXIT] = "n_drops_buffer_connect_exit",
-	[BPF_N_DROPS_BUFFER_OPEN_ENTER] = "n_drops_buffer_open_enter",
-	[BPF_N_DROPS_BUFFER_OPEN_EXIT] = "n_drops_buffer_open_exit",
-	[BPF_N_DROPS_BUFFER_DIR_FILE_ENTER] = "n_drops_buffer_dir_file_enter",
-	[BPF_N_DROPS_BUFFER_DIR_FILE_EXIT] = "n_drops_buffer_dir_file_exit",
-	[BPF_N_DROPS_BUFFER_OTHER_INTEREST_ENTER] = "n_drops_buffer_other_interest_enter",
-	[BPF_N_DROPS_BUFFER_OTHER_INTEREST_EXIT] = "n_drops_buffer_other_interest_exit",
-	[BPF_N_DROPS_SCRATCH_MAP] = "n_drops_scratch_map",
-	[BPF_N_DROPS_PAGE_FAULTS] = "n_drops_page_faults",
-	[BPF_N_DROPS_BUG] = "n_drops_bug",
-	[BPF_N_DROPS] = "n_drops",
-};
-
 enum bpf_libbpf_stats {
 	RUN_CNT = 0,
 	RUN_TIME_NS,
 	AVG_TIME_NS,
 	BPF_MAX_LIBBPF_STATS,
 };
-
-static const char * const bpf_libbpf_stats_names[] = {
-	[RUN_CNT] = ".run_cnt", ///< `bpf_prog_info` run_cnt.
-	[RUN_TIME_NS] = ".run_time_ns", ///<`bpf_prog_info` run_time_ns.
-	[AVG_TIME_NS] = ".avg_time_ns", ///< Average time spent in bpg program, calculation: run_time_ns / run_cnt.
-};

userspace/libscap/engine/gvisor/gvisor.cpp

@@ -89,7 +89,7 @@ static int32_t gvisor_get_stats(struct scap_engine_handle engine, scap_stats* st
 	return engine.m_handle->get_stats(stats);
 }
 
-static struct scap_stats_v2* gvisor_get_stats_v2(struct scap_engine_handle engine, uint32_t flags, uint32_t* nstats, int32_t* rc)
+static const struct scap_stats_v2* gvisor_get_stats_v2(struct scap_engine_handle engine, uint32_t flags, uint32_t* nstats, int32_t* rc)
 {
 	return engine.m_handle->get_stats_v2(flags, nstats, rc);
 }

userspace/libscap/engine/gvisor/gvisor.h

@@ -27,6 +27,7 @@ limitations under the License.
 #include <stdint.h>
 #include <utility>
 #include "scap.h"
+#include "scap_stats_v2.h"
 #include "scap_gvisor_stats.h"
 
 namespace scap_gvisor {
@@ -146,7 +147,7 @@ class engine {
     uint32_t get_fdinfos(const scap_threadinfo *tinfo, uint64_t *n, const scap_fdinfo **fdinfos);
     uint32_t get_vxid(uint64_t pid);
     int32_t get_stats(scap_stats *stats);
-    struct scap_stats_v2* get_stats_v2(uint32_t flags, uint32_t* nstats, int32_t* rc);
+    const struct scap_stats_v2* get_stats_v2(uint32_t flags, uint32_t* nstats, int32_t* rc);
 private:
     int32_t process_message_from_fd(int fd);
     void free_sandbox_buffers();

userspace/libscap/engine/gvisor/scap_gvisor.cpp

@@ -44,6 +44,13 @@ constexpr size_t initial_event_buffer_size = 32;
 constexpr int listen_backlog_size = 128;
 const std::string default_root_path = "/var/run/docker/runtime-runc/moby";
 
+static const char * const gvisor_counters_stats_names[] = {
+	[scap_gvisor::stats::GVISOR_N_EVTS] = "n_evts",
+	[scap_gvisor::stats::GVISOR_N_DROPS_BUG] = "n_drops_bug",
+	[scap_gvisor::stats::GVISOR_N_DROPS_BUFFER_TOTAL] ="n_drops_buffer_total",
+	[scap_gvisor::stats::GVISOR_N_DROPS] = "n_drops",
+};
+
 sandbox_entry::sandbox_entry()
 {
 	m_buf.buf = nullptr;
@@ -450,7 +457,7 @@ int32_t engine::get_stats(scap_stats *stats)
 	return SCAP_SUCCESS;
 }
 
-struct scap_stats_v2* engine::get_stats_v2(uint32_t flags, uint32_t* nstats, int32_t* rc)
+const struct scap_stats_v2* engine::get_stats_v2(uint32_t flags, uint32_t* nstats, int32_t* rc)
 {
 	*nstats = scap_gvisor::stats::MAX_GVISOR_COUNTERS_STATS;
 	scap_stats_v2* stats = engine::m_stats;
@@ -462,11 +469,11 @@ struct scap_stats_v2* engine::get_stats_v2(uint32_t flags, uint32_t* nstats, int
 	}
 
 	/* GVISOR STATS COUNTERS */
-	for(uint32_t stat =  0;  stat < scap_gvisor::stats::MAX_GVISOR_COUNTERS_STATS; stat++)
+	for(uint32_t stat = 0; stat < scap_gvisor::stats::MAX_GVISOR_COUNTERS_STATS; stat++)
 	{
 		stats[stat].type = STATS_VALUE_TYPE_U64;
 		stats[stat].value.u64 = 0;
-		strlcpy(stats[stat].name, scap_gvisor::stats::gvisor_counters_stats_names[stat], STATS_NAME_MAX);
+		strlcpy(stats[stat].name, gvisor_counters_stats_names[stat], STATS_NAME_MAX);
 	}
 	stats[scap_gvisor::stats::GVISOR_N_EVTS].value.u64 = m_gvisor_stats.n_evts;
 	stats[scap_gvisor::stats::GVISOR_N_DROPS_BUG].value.u64 = m_gvisor_stats.n_drops_parsing;

userspace/libscap/engine/gvisor/scap_gvisor_stats.h

@@ -27,11 +27,5 @@ namespace stats {
         MAX_GVISOR_COUNTERS_STATS
     };
 
-    static const char * const gvisor_counters_stats_names[] = {
-        [GVISOR_N_EVTS] = "n_evts",
-        [GVISOR_N_DROPS_BUG] = "n_drops_bug",
-        [GVISOR_N_DROPS_BUFFER_TOTAL] ="n_drops_buffer_total",
-        [GVISOR_N_DROPS] = "n_drops",
-    };
 } // namespace stats
 } // namespace scap_gvisor

userspace/libscap/engine/kmod/kmod.h

@@ -19,8 +19,9 @@ limitations under the License.
 #include <stdint.h>
 #include "ringbuffer/devset.h"
 #include "scap_open.h"
+#include "scap_stats_v2.h"
 #include "scap_kmod_stats.h"
-#include "scap.h"
+
 
 struct kmod_engine
 {