Skip to content

Commit 7aa424e

Browse files
zf1575192187anakryiko
authored andcommitted
selftests/bpf: Fix some bugs in map_lookup_percpu_elem testcase
comments from Andrii Nakryiko, details in here: https://lore.kernel.org/lkml/[email protected]/T/ use /* */ instead of // use libbpf_num_possible_cpus() instead of sysconf(_SC_NPROCESSORS_ONLN) use 8 bytes for value size fix memory leak use ASSERT_EQ instead of ASSERT_OK add bpf_loop to fetch values on each possible CPU Fixes: ed7c137 ("selftests/bpf: add test case for bpf_map_lookup_percpu_elem") Signed-off-by: Feng Zhou <[email protected]> Signed-off-by: Andrii Nakryiko <[email protected]> Acked-by: Yonghong Song <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 834650b commit 7aa424e

File tree

2 files changed

+73
-39
lines changed

2 files changed

+73
-39
lines changed
Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,38 @@
11
// SPDX-License-Identifier: GPL-2.0
2-
// Copyright (c) 2022 Bytedance
2+
/* Copyright (c) 2022 Bytedance */
33

44
#include <test_progs.h>
5-
65
#include "test_map_lookup_percpu_elem.skel.h"
76

8-
#define TEST_VALUE 1
9-
107
void test_map_lookup_percpu_elem(void)
118
{
129
struct test_map_lookup_percpu_elem *skel;
13-
int key = 0, ret;
14-
int nr_cpus = sysconf(_SC_NPROCESSORS_ONLN);
15-
int *buf;
10+
__u64 key = 0, sum;
11+
int ret, i, nr_cpus = libbpf_num_possible_cpus();
12+
__u64 *buf;
1613

17-
buf = (int *)malloc(nr_cpus*sizeof(int));
14+
buf = malloc(nr_cpus*sizeof(__u64));
1815
if (!ASSERT_OK_PTR(buf, "malloc"))
1916
return;
20-
memset(buf, 0, nr_cpus*sizeof(int));
21-
buf[0] = TEST_VALUE;
2217

23-
skel = test_map_lookup_percpu_elem__open_and_load();
24-
if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open_and_load"))
25-
return;
18+
for (i = 0; i < nr_cpus; i++)
19+
buf[i] = i;
20+
sum = (nr_cpus - 1) * nr_cpus / 2;
21+
22+
skel = test_map_lookup_percpu_elem__open();
23+
if (!ASSERT_OK_PTR(skel, "test_map_lookup_percpu_elem__open"))
24+
goto exit;
25+
26+
skel->rodata->my_pid = getpid();
27+
skel->rodata->nr_cpus = nr_cpus;
28+
29+
ret = test_map_lookup_percpu_elem__load(skel);
30+
if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__load"))
31+
goto cleanup;
32+
2633
ret = test_map_lookup_percpu_elem__attach(skel);
27-
ASSERT_OK(ret, "test_map_lookup_percpu_elem__attach");
34+
if (!ASSERT_OK(ret, "test_map_lookup_percpu_elem__attach"))
35+
goto cleanup;
2836

2937
ret = bpf_map_update_elem(bpf_map__fd(skel->maps.percpu_array_map), &key, buf, 0);
3038
ASSERT_OK(ret, "percpu_array_map update");
@@ -37,10 +45,14 @@ void test_map_lookup_percpu_elem(void)
3745

3846
syscall(__NR_getuid);
3947

40-
ret = skel->bss->percpu_array_elem_val == TEST_VALUE &&
41-
skel->bss->percpu_hash_elem_val == TEST_VALUE &&
42-
skel->bss->percpu_lru_hash_elem_val == TEST_VALUE;
43-
ASSERT_OK(!ret, "bpf_map_lookup_percpu_elem success");
48+
test_map_lookup_percpu_elem__detach(skel);
49+
50+
ASSERT_EQ(skel->bss->percpu_array_elem_sum, sum, "percpu_array lookup percpu elem");
51+
ASSERT_EQ(skel->bss->percpu_hash_elem_sum, sum, "percpu_hash lookup percpu elem");
52+
ASSERT_EQ(skel->bss->percpu_lru_hash_elem_sum, sum, "percpu_lru_hash lookup percpu elem");
4453

54+
cleanup:
4555
test_map_lookup_percpu_elem__destroy(skel);
56+
exit:
57+
free(buf);
4658
}

tools/testing/selftests/bpf/progs/test_map_lookup_percpu_elem.c

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,74 @@
11
// SPDX-License-Identifier: GPL-2.0
2-
// Copyright (c) 2022 Bytedance
2+
/* Copyright (c) 2022 Bytedance */
33

44
#include "vmlinux.h"
55
#include <bpf/bpf_helpers.h>
66

7-
int percpu_array_elem_val = 0;
8-
int percpu_hash_elem_val = 0;
9-
int percpu_lru_hash_elem_val = 0;
7+
__u64 percpu_array_elem_sum = 0;
8+
__u64 percpu_hash_elem_sum = 0;
9+
__u64 percpu_lru_hash_elem_sum = 0;
10+
const volatile int nr_cpus;
11+
const volatile int my_pid;
1012

1113
struct {
1214
__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
1315
__uint(max_entries, 1);
1416
__type(key, __u32);
15-
__type(value, __u32);
17+
__type(value, __u64);
1618
} percpu_array_map SEC(".maps");
1719

1820
struct {
1921
__uint(type, BPF_MAP_TYPE_PERCPU_HASH);
2022
__uint(max_entries, 1);
21-
__type(key, __u32);
22-
__type(value, __u32);
23+
__type(key, __u64);
24+
__type(value, __u64);
2325
} percpu_hash_map SEC(".maps");
2426

2527
struct {
2628
__uint(type, BPF_MAP_TYPE_LRU_PERCPU_HASH);
2729
__uint(max_entries, 1);
28-
__type(key, __u32);
29-
__type(value, __u32);
30+
__type(key, __u64);
31+
__type(value, __u64);
3032
} percpu_lru_hash_map SEC(".maps");
3133

34+
struct read_percpu_elem_ctx {
35+
void *map;
36+
__u64 sum;
37+
};
38+
39+
static int read_percpu_elem_callback(__u32 index, struct read_percpu_elem_ctx *ctx)
40+
{
41+
__u64 key = 0;
42+
__u64 *value;
43+
44+
value = bpf_map_lookup_percpu_elem(ctx->map, &key, index);
45+
if (value)
46+
ctx->sum += *value;
47+
return 0;
48+
}
49+
3250
SEC("tp/syscalls/sys_enter_getuid")
3351
int sysenter_getuid(const void *ctx)
3452
{
35-
__u32 key = 0;
36-
__u32 cpu = 0;
37-
__u32 *value;
53+
struct read_percpu_elem_ctx map_ctx;
3854

39-
value = bpf_map_lookup_percpu_elem(&percpu_array_map, &key, cpu);
40-
if (value)
41-
percpu_array_elem_val = *value;
55+
if (my_pid != (bpf_get_current_pid_tgid() >> 32))
56+
return 0;
4257

43-
value = bpf_map_lookup_percpu_elem(&percpu_hash_map, &key, cpu);
44-
if (value)
45-
percpu_hash_elem_val = *value;
58+
map_ctx.map = &percpu_array_map;
59+
map_ctx.sum = 0;
60+
bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
61+
percpu_array_elem_sum = map_ctx.sum;
4662

47-
value = bpf_map_lookup_percpu_elem(&percpu_lru_hash_map, &key, cpu);
48-
if (value)
49-
percpu_lru_hash_elem_val = *value;
63+
map_ctx.map = &percpu_hash_map;
64+
map_ctx.sum = 0;
65+
bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
66+
percpu_hash_elem_sum = map_ctx.sum;
67+
68+
map_ctx.map = &percpu_lru_hash_map;
69+
map_ctx.sum = 0;
70+
bpf_loop(nr_cpus, read_percpu_elem_callback, &map_ctx, 0);
71+
percpu_lru_hash_elem_sum = map_ctx.sum;
5072

5173
return 0;
5274
}

0 commit comments

Comments
 (0)