Skip to content

Commit

Permalink
fix(bpf): Fix issue introduced with bpf refactor
Browse files Browse the repository at this point in the history
This fixes a few issues identified with the bpf code
format and refactor.

1. Zero initialize all variables
2. Use the bpf_perf_event_read_value helper exlusively

In addition, add some logging around eBPF array resizing.

Fixes: #1402 #1411

Signed-off-by: Dave Tucker <[email protected]>
  • Loading branch information
dave-tucker committed May 8, 2024
1 parent f7885b4 commit d7b6775
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 15 deletions.
32 changes: 17 additions & 15 deletions bpfassets/libbpf/src/kepler.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ int counter_sched_switch = 0;

static inline u64 get_on_cpu_time(u32 cur_pid, u32 prev_pid, u64 cur_ts)
{
u64 cpu_time, *prev_ts;
pid_time_t prev_pid_key, new_pid_key;
u64 cpu_time = 0;
u64 *prev_ts;
pid_time_t prev_pid_key, new_pid_key = {};

prev_pid_key.pid = prev_pid;
prev_ts = bpf_map_lookup_elem(&pid_time, &prev_pid_key);
Expand All @@ -115,7 +116,7 @@ static inline u64 get_on_cpu_time(u32 cur_pid, u32 prev_pid, u64 cur_ts)

static inline u64 calc_delta(u64 *prev_val, u64 *val)
{
u64 delta;
u64 delta = 0;

if (prev_val) {
if (*val > *prev_val)
Expand All @@ -127,14 +128,14 @@ static inline u64 calc_delta(u64 *prev_val, u64 *val)
static inline u64 get_on_cpu_cycles(u32 *cpu_id)
{
u64 delta, val, *prev_val;
struct bpf_perf_event_value c;
int error;
struct bpf_perf_event_value c = {};

error = bpf_perf_event_read_value(
&cpu_cycles_event_reader, *cpu_id, &c, sizeof(c));
if (error)
return 0;

// TODO: Fix Verifier errors upon changing this to bpf_perf_event_read_value
error = bpf_perf_event_read(&cpu_cycles_event_reader, *cpu_id);
if (error < 0) {
return delta;
}
val = c.counter;
prev_val = bpf_map_lookup_elem(&cpu_cycles, cpu_id);
delta = calc_delta(prev_val, &val);
Expand All @@ -147,12 +148,12 @@ static inline u64 get_on_cpu_instr(u32 *cpu_id)
{
u64 delta, val, *prev_val;
int error;
struct bpf_perf_event_value c;
struct bpf_perf_event_value c = {};

error = bpf_perf_event_read_value(
&cpu_instructions_event_reader, *cpu_id, &c, sizeof(c));
if (error) {
return delta;
return 0;
}
val = c.counter;
prev_val = bpf_map_lookup_elem(&cpu_instructions, cpu_id);
Expand All @@ -166,12 +167,12 @@ static inline u64 get_on_cpu_cache_miss(u32 *cpu_id)
{
u64 delta, val, *prev_val;
int error;
struct bpf_perf_event_value c;
struct bpf_perf_event_value c = {};

error = bpf_perf_event_read_value(
&cache_miss_event_reader, *cpu_id, &c, sizeof(c));
if (error) {
return delta;
return 0;
}
val = c.counter;
prev_val = bpf_map_lookup_elem(&cache_miss, cpu_id);
Expand Down Expand Up @@ -203,7 +204,8 @@ int kepler_sched_switch_trace(struct sched_switch_info *ctx)
u64 pid_tgid, cgroup_id, cur_ts;
pid_t cur_pid;

struct process_metrics_t *cur_pid_metrics, *prev_pid_metrics, buf;
struct process_metrics_t *cur_pid_metrics, *prev_pid_metrics;
struct process_metrics_t buf;

if (SAMPLE_RATE > 0) {
if (counter_sched_switch > 0) {
Expand Down Expand Up @@ -236,7 +238,7 @@ int kepler_sched_switch_trace(struct sched_switch_info *ctx)
prev_pid_metrics->cache_miss += buf.cache_miss;
}

// creat new process metrics
// create new process metrics
cur_pid_metrics = bpf_map_lookup_elem(&processes, &cur_pid);
if (!cur_pid_metrics) {
process_metrics_t new_process = {};
Expand Down
1 change: 1 addition & 0 deletions pkg/bpfassets/attacher/libbpf_attacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func attachLibbpfModule() (*bpf.Module, error) {
return nil, fmt.Errorf("failed to load module: %v", err)
}
// resize array entries
klog.Infof("%d CPU cores detected. Resizing eBPF Perf Event Arrays", cpuCores)
for _, arrayName := range bpfArrays {
err = resizeArrayEntries(arrayName, cpuCores)
if err != nil {
Expand Down

0 comments on commit d7b6775

Please sign in to comment.