Skip to content

Commit

Permalink
cleanup(libscap): add some comments and minor cleanups to the CPU logic
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrea Terzolo <[email protected]>
  • Loading branch information
@Andreagit97 @poiana
Andreagit97 authored and poiana committed Oct 3, 2023
1 parent bcb74e7 commit 2e76d0d
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 19 deletions.
10 changes: 10 additions & 0 deletions userspace/libpman/src/ringbuffer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,16 @@ int pman_finalize_ringbuf_array_after_loading()
continue;
}

if(ringbuf_id >= g_state.n_required_buffers)
{
/* If we arrive here it means that we have too many CPUs for our allocated ring buffers
* so probably we faced a CPU hotplug.
*/
snprintf(error_message, MAX_ERROR_MESSAGE_LEN, "the actual system configuration requires more than '%d' ring buffers", g_state.n_required_buffers);
pman_print_error((const char *)error_message);
goto clean_percpu_ring_buffers;
}

if(bpf_map_update_elem(ringubuf_array_fd, &i, &ringbufs_fds[ringbuf_id], BPF_ANY))
{
snprintf(error_message, MAX_ERROR_MESSAGE_LEN, "failed to add the ringbuf map for CPU '%d' to ringbuf '%d'", i, ringbuf_id);
Expand Down
22 changes: 12 additions & 10 deletions userspace/libscap/engine/bpf/scap_bpf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1521,15 +1521,17 @@ int32_t scap_bpf_load(
//
struct scap_device_set *devset = &handle->m_dev_set;
uint32_t online_idx = 0;
// devset->m_ndevs = online CPUs in the system.
// handle->m_ncpus = available CPUs in the system.
for(uint32_t cpu_idx = 0; online_idx < devset->m_ndevs && cpu_idx < handle->m_ncpus; ++cpu_idx)
{
struct perf_event_attr attr = {
.sample_type = PERF_SAMPLE_RAW,
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_BPF_OUTPUT,
};
int pmu_fd;
int ret;
int pmu_fd = 0;
int ret = 0;

/* We suppose that CPU 0 is always online, so we only check for cpu_idx > 0 */
if(cpu_idx > 0)
Expand All @@ -1553,7 +1555,7 @@ int32_t scap_bpf_load(
online = 1;
}
// If we can't access the cpu, count it as offline.
// Some VMs or hypertreading systems export an high number of configured CPUs,
// Some VMs or hyperthreading systems export an high number of configured CPUs,
// even if they are not existing. See https://github.com/falcosecurity/falco/issues/2843 for example.
// Skip them.
}
Expand Down Expand Up @@ -1606,21 +1608,21 @@ int32_t scap_bpf_load(
online_idx++;
}

// Check that we parsed all onlince CPUs
// Check that we parsed all online CPUs
if(online_idx != devset->m_ndevs)
{
return scap_errprintf(handle->m_lasterr, 0, "processors online: %d, expected: %d", online_idx, devset->m_ndevs);
return scap_errprintf(handle->m_lasterr, 0, "mismatch, processors online after the 'for' loop: %d, '_SC_NPROCESSORS_ONLN' before the 'for' loop: %d", online_idx, devset->m_ndevs);
}

// Check that no CPUs were hotplugged during the for loop
uint32_t final_ndevs = sysconf(_SC_NPROCESSORS_ONLN);
if(final_ndevs == -1)
{
return scap_errprintf(handle->m_lasterr, errno, "_SC_NPROCESSORS_ONLN");
return scap_errprintf(handle->m_lasterr, errno, "cannot obtain the number of online CPUs from '_SC_NPROCESSORS_ONLN' to check against the previous value");
}
if (final_ndevs != online_idx)
if (online_idx != final_ndevs)
{
return scap_errprintf(handle->m_lasterr, 0, "wrong number of online processors: %d, expected: %d", final_ndevs, online_idx);
return scap_errprintf(handle->m_lasterr, 0, "mismatch, processors online after the 'for' loop: %d, '_SC_NPROCESSORS_ONLN' after the 'for' loop: %d", online_idx, final_ndevs);
}


Expand Down Expand Up @@ -1949,15 +1951,15 @@ static int32_t init(scap_t* handle, scap_open_args *oargs)
ssize_t num_cpus = sysconf(_SC_NPROCESSORS_CONF);
if(num_cpus == -1)
{
return scap_errprintf(engine.m_handle->m_lasterr, errno, "_SC_NPROCESSORS_CONF");
return scap_errprintf(engine.m_handle->m_lasterr, errno, "cannot obtain the number of available CPUs from '_SC_NPROCESSORS_CONF'");
}

engine.m_handle->m_ncpus = num_cpus;

ssize_t num_devs = sysconf(_SC_NPROCESSORS_ONLN);
if(num_devs == -1)
{
return scap_errprintf(engine.m_handle->m_lasterr, errno, "_SC_NPROCESSORS_ONLN");
return scap_errprintf(engine.m_handle->m_lasterr, errno, "cannot obtain the number of online CPUs from '_SC_NPROCESSORS_ONLN'");
}

rc = devset_init(&engine.m_handle->m_dev_set, num_devs, engine.m_handle->m_lasterr);
Expand Down
20 changes: 11 additions & 9 deletions userspace/libscap/engine/kmod/scap_kmod.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ int32_t scap_kmod_init(scap_t *handle, scap_open_args *oargs)
ncpus = sysconf(_SC_NPROCESSORS_CONF);
if(ncpus == -1)
{
return scap_errprintf(handle->m_lasterr, errno, "_SC_NPROCESSORS_CONF");
return scap_errprintf(handle->m_lasterr, errno, "cannot obtain the number of available CPUs from '_SC_NPROCESSORS_CONF'");
}

//
Expand All @@ -345,7 +345,7 @@ int32_t scap_kmod_init(scap_t *handle, scap_open_args *oargs)
ndevs = sysconf(_SC_NPROCESSORS_ONLN);
if(ndevs == -1)
{
return scap_errprintf(handle->m_lasterr, errno, "_SC_NPROCESSORS_ONLN");
return scap_errprintf(handle->m_lasterr, errno, "cannot obtain the number of online CPUs from '_SC_NPROCESSORS_ONLN'");
}

rc = devset_init(&engine.m_handle->m_dev_set, ndevs, handle->m_lasterr);
Expand All @@ -361,14 +361,16 @@ int32_t scap_kmod_init(scap_t *handle, scap_open_args *oargs)

struct scap_device_set *devset = &engine.m_handle->m_dev_set;
uint32_t online_idx = 0;
for(uint32_t all_scanned_devs = 0; online_idx < devset->m_ndevs && all_scanned_devs < ncpus; ++all_scanned_devs)
// devset->m_ndevs = online CPUs in the system.
// ncpus = available CPUs in the system.
for(uint32_t cpu_idx = 0; online_idx < devset->m_ndevs && cpu_idx < ncpus; ++cpu_idx)
{
struct scap_device *dev = &devset->m_devs[online_idx];

//
// Open the device
//
snprintf(filename, sizeof(filename), "%s/dev/" DRIVER_DEVICE_NAME "%d", scap_get_host_root(), all_scanned_devs);
snprintf(filename, sizeof(filename), "%s/dev/" DRIVER_DEVICE_NAME "%d", scap_get_host_root(), cpu_idx);

if((dev->m_fd = open(filename, O_RDWR | O_SYNC)) < 0)
{
Expand Down Expand Up @@ -493,21 +495,21 @@ int32_t scap_kmod_init(scap_t *handle, scap_open_args *oargs)
++online_idx;
}

// Check that we parsed all onlince CPUs
// Check that we parsed all online CPUs
if(online_idx != devset->m_ndevs)
{
return scap_errprintf(handle->m_lasterr, 0, "processors online: %d, expected: %d", online_idx, devset->m_ndevs);
return scap_errprintf(handle->m_lasterr, 0, "mismatch, processors online after the 'for' loop: %d, '_SC_NPROCESSORS_ONLN' before the 'for' loop: %d", online_idx, devset->m_ndevs);
}

// Check that no CPUs were hotplugged during the for loop
uint32_t final_ndevs = sysconf(_SC_NPROCESSORS_ONLN);
if(final_ndevs == -1)
{
return scap_errprintf(handle->m_lasterr, errno, "_SC_NPROCESSORS_ONLN");
return scap_errprintf(handle->m_lasterr, errno, "cannot obtain the number of online CPUs from '_SC_NPROCESSORS_ONLN' to check against the previous value");
}
if (final_ndevs != online_idx)
if (online_idx != final_ndevs)
{
return scap_errprintf(handle->m_lasterr, 0, "wrong number of online processors: %d, expected: %d", final_ndevs, online_idx);
return scap_errprintf(handle->m_lasterr, 0, "mismatch, processors online after the 'for' loop: %d, '_SC_NPROCESSORS_ONLN' after the 'for' loop: %d", online_idx, final_ndevs);
}

/* Here we are covering the case in which some syscalls don't have an associated ppm_sc
Expand Down

0 comments on commit 2e76d0d

Please sign in to comment.