From 00353b1ff3083b2314f305ab3d9bff8db549ba1c Mon Sep 17 00:00:00 2001 From: Nicolas Vanheuverzwijn Date: Wed, 29 Jul 2020 09:05:55 -0400 Subject: [PATCH 1/5] driver: add cmake SYSFS_NAME which is the module name as seen in /sys/module/. Linux kernel replaces dash (`-`) by underscore (`_`) when installing a module with a dash in it's name. sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn --- CMakeLists.txt | 1 + driver/driver_config.h.in | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 04e9bf0ab8..6a904df807 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -104,6 +104,7 @@ if(NOT WIN32) if(NOT DEFINED PROBE_NAME) set(PROBE_NAME "sysdig-probe") endif() + string(REGEX REPLACE "[,-]" "_" SYSFS_NAME ${PROBE_NAME}) if(NOT DEFINED PROBE_DEVICE_NAME) set(PROBE_DEVICE_NAME "sysdig") diff --git a/driver/driver_config.h.in b/driver/driver_config.h.in index 560a0cb23d..df63ccc254 100644 --- a/driver/driver_config.h.in +++ b/driver/driver_config.h.in @@ -17,3 +17,5 @@ or GPL2.txt for full copies of the license. #ifndef KBUILD_MODNAME #define KBUILD_MODNAME PROBE_NAME #endif + +#define SYSFS_NAME "${SYSFS_NAME}" From 6a18d6b52ebc61cee799a71484f67621876b1258 Mon Sep 17 00:00:00 2001 From: Nicolas Vanheuverzwijn Date: Wed, 29 Jul 2020 09:19:24 -0400 Subject: [PATCH 2/5] driver: rename 'RING_BUF_SIZE' to 'ring_buf_size' and removed 'const' sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn --- driver/main.c | 42 ++++++++++++++++++++-------------------- driver/ppm_ringbuffer.h | 2 +- userspace/libscap/scap.c | 10 +++++----- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/driver/main.c b/driver/main.c index fab205eff3..e7377a6e74 100644 --- a/driver/main.c +++ b/driver/main.c @@ -1170,15 +1170,15 @@ static int ppm_mmap(struct file *filp, struct vm_area_struct *vma) /* * Enforce ring buffer size */ - if (RING_BUF_SIZE < 2 * PAGE_SIZE) { + if (ring_buf_size < 2 * PAGE_SIZE) { pr_err("Ring buffer size too small (%ld bytes, must be at least %ld bytes\n", - (long)RING_BUF_SIZE, + (long)ring_buf_size, (long)PAGE_SIZE); ret = -EIO; goto cleanup_mmap; } - if (RING_BUF_SIZE / PAGE_SIZE * PAGE_SIZE != RING_BUF_SIZE) { + if (ring_buf_size / PAGE_SIZE * PAGE_SIZE != ring_buf_size) { pr_err("Ring buffer size is not a multiple of the page size\n"); ret = -EIO; goto cleanup_mmap; @@ -1215,7 +1215,7 @@ static int ppm_mmap(struct file *filp, struct vm_area_struct *vma) ret = 0; goto cleanup_mmap; - } else if (length == RING_BUF_SIZE * 2) { + } else if (length == ring_buf_size * 2) { long mlength; /* @@ -1658,16 +1658,16 @@ static int record_event_consumer(struct ppm_consumer_t *consumer, if (ttail > head) freespace = ttail - head - 1; else - freespace = RING_BUF_SIZE + ttail - head - 1; + freespace = ring_buf_size + ttail - head - 1; - usedspace = RING_BUF_SIZE - freespace - 1; - delta_from_end = RING_BUF_SIZE + (2 * PAGE_SIZE) - head - 1; + usedspace = ring_buf_size - freespace - 1; + delta_from_end = ring_buf_size + (2 * PAGE_SIZE) - head - 1; - ASSERT(freespace <= RING_BUF_SIZE); - ASSERT(usedspace <= RING_BUF_SIZE); - ASSERT(ttail <= RING_BUF_SIZE); - ASSERT(head <= RING_BUF_SIZE); - ASSERT(delta_from_end < RING_BUF_SIZE + (2 * PAGE_SIZE)); + ASSERT(freespace <= ring_buf_size); + ASSERT(usedspace <= ring_buf_size); + ASSERT(ttail <= ring_buf_size); + ASSERT(head <= ring_buf_size); + ASSERT(delta_from_end < ring_buf_size + (2 * PAGE_SIZE)); ASSERT(delta_from_end > (2 * PAGE_SIZE) - 1); #ifdef _HAS_SOCKETCALL /* @@ -1828,20 +1828,20 @@ static int record_event_consumer(struct ppm_consumer_t *consumer, next = head + event_size; - if (unlikely(next >= RING_BUF_SIZE)) { + if (unlikely(next >= ring_buf_size)) { /* * If something has been written in the cushion space at the end of * the buffer, copy it to the beginning and wrap the head around. * Note, we don't check that the copy fits because we assume that * filler_callback failed if the space was not enough. */ - if (next > RING_BUF_SIZE) { + if (next > ring_buf_size) { memcpy(ring->buffer, - ring->buffer + RING_BUF_SIZE, - next - RING_BUF_SIZE); + ring->buffer + ring_buf_size, + next - ring_buf_size); } - next -= RING_BUF_SIZE; + next -= ring_buf_size; } /* @@ -1874,7 +1874,7 @@ static int record_event_consumer(struct ppm_consumer_t *consumer, vpr_info("consumer:%p CPU:%d, use:%d%%, ev:%llu, dr_buf:%llu, dr_pf:%llu, pr:%llu, cs:%llu\n", consumer->consumer_id, smp_processor_id(), - (usedspace * 100) / RING_BUF_SIZE, + (usedspace * 100) / ring_buf_size, ring_info->n_evts, ring_info->n_drops_buffer, ring_info->n_drops_pf, @@ -2186,13 +2186,13 @@ static int init_ring_buffer(struct ppm_ring_buffer_context *ring) * Note how we allocate 2 additional pages: they are used as additional overflow space for * the event data generation functions, so that they always operate on a contiguous buffer. */ - ring->buffer = vmalloc(RING_BUF_SIZE + 2 * PAGE_SIZE); + ring->buffer = vmalloc(ring_buf_size + 2 * PAGE_SIZE); if (ring->buffer == NULL) { pr_err("Error allocating ring memory\n"); goto init_ring_err; } - for (j = 0; j < RING_BUF_SIZE + 2 * PAGE_SIZE; j++) + for (j = 0; j < ring_buf_size + 2 * PAGE_SIZE; j++) ring->buffer[j] = 0; /* @@ -2210,7 +2210,7 @@ static int init_ring_buffer(struct ppm_ring_buffer_context *ring) reset_ring_buffer(ring); atomic_set(&ring->preempt_count, 0); - pr_info("CPU buffer initialized, size=%d\n", RING_BUF_SIZE); + pr_info("CPU buffer initialized, size=%d\n", ring_buf_size); return 1; diff --git a/driver/ppm_ringbuffer.h b/driver/ppm_ringbuffer.h index b6761b2718..c8ce42fd02 100644 --- a/driver/ppm_ringbuffer.h +++ b/driver/ppm_ringbuffer.h @@ -14,8 +14,8 @@ or GPL2.txt for full copies of the license. #include #endif -static const __u32 RING_BUF_SIZE = 8 * 1024 * 1024; static const __u32 MIN_USERSPACE_READ_SIZE = 128 * 1024; +static __u32 ring_buf_size = 8 * 1024 * 1024; /* * This gets mapped to user level, so we want to keep it as clean as possible diff --git a/userspace/libscap/scap.c b/userspace/libscap/scap.c index 6f51588eda..1c2857d51d 100644 --- a/userspace/libscap/scap.c +++ b/userspace/libscap/scap.c @@ -323,7 +323,7 @@ scap_t* scap_open_live_int(char *error, int32_t *rc, // // Allocate the device descriptors. // - len = RING_BUF_SIZE * 2; + len = ring_buf_size * 2; for(j = 0, all_scanned_devs = 0; j < handle->m_ndevs && all_scanned_devs < handle->m_ncpus; ++all_scanned_devs) { @@ -997,7 +997,7 @@ void scap_close(scap_t* handle) if(handle->m_devs[j].m_buffer != MAP_FAILED) { munmap(handle->m_devs[j].m_bufinfo, sizeof(struct ppm_ring_buffer_info)); - munmap(handle->m_devs[j].m_buffer, RING_BUF_SIZE * 2); + munmap(handle->m_devs[j].m_buffer, ring_buf_size * 2); close(handle->m_devs[j].m_fd); } } @@ -1122,7 +1122,7 @@ void get_buf_pointers(struct ppm_ring_buffer_info* bufinfo, uint32_t* phead, uin if(*ptail > *phead) { - *pread_size = RING_BUF_SIZE - *ptail + *phead; + *pread_size = ring_buf_size - *ptail + *phead; } else { @@ -1154,13 +1154,13 @@ static void scap_advance_tail(scap_t* handle, uint32_t cpuid) // __sync_synchronize(); - if(ttail < RING_BUF_SIZE) + if(ttail < ring_buf_size) { handle->m_devs[cpuid].m_bufinfo->tail = ttail; } else { - handle->m_devs[cpuid].m_bufinfo->tail = ttail - RING_BUF_SIZE; + handle->m_devs[cpuid].m_bufinfo->tail = ttail - ring_buf_size; } handle->m_devs[cpuid].m_lastreadsize = 0; From aa6a36b927bf23e8206066bdb82c230cc87baab5 Mon Sep 17 00:00:00 2001 From: Nicolas Vanheuverzwijn Date: Wed, 29 Jul 2020 09:41:37 -0400 Subject: [PATCH 3/5] driver: fix error message whe ring_bug_size is too small instead of "Ring buffer size too small (4096 bytes, must be at least 4096 bytes", message is now "Ring buffer size too small (4096 bytes, must be at least 8192 bytes)" sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn --- driver/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/driver/main.c b/driver/main.c index e7377a6e74..b6714887da 100644 --- a/driver/main.c +++ b/driver/main.c @@ -1171,9 +1171,9 @@ static int ppm_mmap(struct file *filp, struct vm_area_struct *vma) * Enforce ring buffer size */ if (ring_buf_size < 2 * PAGE_SIZE) { - pr_err("Ring buffer size too small (%ld bytes, must be at least %ld bytes\n", + pr_err("Ring buffer size too small (%ld bytes, must be at least %ld bytes)\n", (long)ring_buf_size, - (long)PAGE_SIZE); + (long)PAGE_SIZE * 2); ret = -EIO; goto cleanup_mmap; } From 71e5f67eb07dbb8a89e37a8feaec9eb4d93e9d3b Mon Sep 17 00:00:00 2001 From: Nicolas Vanheuverzwijn Date: Wed, 29 Jul 2020 09:44:23 -0400 Subject: [PATCH 4/5] driver: ring_buf_size is now a module parameter sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn --- driver/main.c | 28 ++++++++++++++++++++++++++++ userspace/libscap/scap.c | 9 +++++++++ 2 files changed, 37 insertions(+) diff --git a/driver/main.c b/driver/main.c index b6714887da..870fbd5c02 100644 --- a/driver/main.c +++ b/driver/main.c @@ -2628,10 +2628,38 @@ void sysdig_exit(void) #endif } +static int set_ring_buf_size(const char *val, const struct kernel_param *kp) +{ + int n = 0, ret; + + ret = kstrtoint(val, 10, &n); + if (ret != 0) + return -EINVAL; + else if (n < 2 * PAGE_SIZE) { + pr_err("Ring buffer size too small (%ld bytes, must be at least %ld bytes)\n", + (long)n, + (long)PAGE_SIZE * 2); + return -EINVAL; + } + else if (n / PAGE_SIZE * PAGE_SIZE != n) { + pr_err("Ring buffer size is not a multiple of the page size\n"); + return -EINVAL; + } + + return param_set_int(val, kp); +} + +static const struct kernel_param_ops ring_buf_size_param_ops = { + .set = set_ring_buf_size, + .get = param_get_int, +}; + module_init(sysdig_init); module_exit(sysdig_exit); module_param(max_consumers, uint, 0444); MODULE_PARM_DESC(max_consumers, "Maximum number of consumers that can simultaneously open the devices"); +module_param_cb(ring_buf_size, &ring_buf_size_param_ops, &ring_buf_size, 0660); +MODULE_PARM_DESC(ring_buf_size, "Size of the ring buffer containing syscall"); #if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20) module_param(verbose, bool, 0444); #endif diff --git a/userspace/libscap/scap.c b/userspace/libscap/scap.c index 1c2857d51d..477384ce56 100644 --- a/userspace/libscap/scap.c +++ b/userspace/libscap/scap.c @@ -323,6 +323,15 @@ scap_t* scap_open_live_int(char *error, int32_t *rc, // // Allocate the device descriptors. // + + FILE * fp = fopen("/sys/module/" SYSFS_NAME "/parameters/ring_buf_size", "r"); + if (fp == NULL){ + snprintf(error, SCAP_LASTERR_SIZE, "Could not read module parameter ring_buf_size at '/sys/module/" SYSFS_NAME "/parameters/ring_buf_size'"); + *rc = SCAP_FAILURE; + return NULL; + } + fscanf(fp, "%d", &ring_buf_size); + len = ring_buf_size * 2; for(j = 0, all_scanned_devs = 0; j < handle->m_ndevs && all_scanned_devs < handle->m_ncpus; ++all_scanned_devs) From 73569dd70a056afb7db477e41e7bdc4be5837d88 Mon Sep 17 00:00:00 2001 From: Nicolas Vanheuverzwijn Date: Wed, 5 Aug 2020 11:59:50 -0400 Subject: [PATCH 5/5] libscap: use SYSFS_NAME instead of PROBE_DEVICE_NAME to read max_consumers parameter sysdig-CLA-1.0-signed-off-by: Nicolas Vanheuverzwijn --- userspace/libscap/scap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/userspace/libscap/scap.c b/userspace/libscap/scap.c index 477384ce56..c064173c73 100644 --- a/userspace/libscap/scap.c +++ b/userspace/libscap/scap.c @@ -114,7 +114,7 @@ scap_t* scap_open_udig_int(char *error, int32_t *rc, static uint32_t get_max_consumers() { uint32_t max; - FILE *pfile = fopen("/sys/module/" PROBE_DEVICE_NAME "_probe/parameters/max_consumers", "r"); + FILE *pfile = fopen("/sys/module/" SYSFS_NAME "/parameters/max_consumers", "r"); if(pfile != NULL) { int w = fscanf(pfile, "%"PRIu32, &max); @@ -353,7 +353,7 @@ scap_t* scap_open_live_int(char *error, int32_t *rc, else if(errno == EBUSY) { uint32_t curr_max_consumers = get_max_consumers(); - snprintf(error, SCAP_LASTERR_SIZE, "Too many sysdig instances attached to device %s. Current value for /sys/module/" PROBE_DEVICE_NAME "_probe/parameters/max_consumers is '%"PRIu32"'.", filename, curr_max_consumers); + snprintf(error, SCAP_LASTERR_SIZE, "Too many sysdig instances attached to device %s. Current value for /sys/module/" SYSFS_NAME "/parameters/max_consumers is '%"PRIu32"'.", filename, curr_max_consumers); } else {