On 12/14/23 7:33 PM, Hou Tao wrote:
Hi,
On 12/15/2023 8:12 AM, Yonghong Song wrote:
In the previous patch, the maximum data size for bpf_global_percpu_ma
is 512 bytes. This breaks selftest test_bpf_ma. Let us adjust it
accordingly. Also added a selftest to capture the verification failure
when the allocation size is greater than 512.
Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx>
---
.../selftests/bpf/progs/percpu_alloc_fail.c | 18 ++++++++++++++++++
.../testing/selftests/bpf/progs/test_bpf_ma.c | 9 ---------
2 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
index 1a891d30f1fe..f2b8eb2ff76f 100644
--- a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
+++ b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
@@ -17,6 +17,10 @@ struct val_with_rb_root_t {
struct bpf_spin_lock lock;
};
+struct val_600b_t {
+ char b[600];
+};
+
struct elem {
long sum;
struct val_t __percpu_kptr *pc;
@@ -161,4 +165,18 @@ int BPF_PROG(test_array_map_7)
return 0;
}
+SEC("?fentry.s/bpf_fentry_test1")
+__failure __msg("bpf_percpu_obj_new type size (600) is greater than 512")
+int BPF_PROG(test_array_map_8)
+{
+ struct val_600b_t __percpu_kptr *p;
+
+ p = bpf_percpu_obj_new(struct val_600b_t);
+ if (!p)
+ return 0;
+
+ bpf_percpu_obj_drop(p);
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_ma.c b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
index b685a4aba6bd..68cba55eb828 100644
--- a/tools/testing/selftests/bpf/progs/test_bpf_ma.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_ma.c
@@ -188,9 +188,6 @@ DEFINE_ARRAY_WITH_PERCPU_KPTR(128);
DEFINE_ARRAY_WITH_PERCPU_KPTR(192);
DEFINE_ARRAY_WITH_PERCPU_KPTR(256);
DEFINE_ARRAY_WITH_PERCPU_KPTR(512);
-DEFINE_ARRAY_WITH_PERCPU_KPTR(1024);
-DEFINE_ARRAY_WITH_PERCPU_KPTR(2048);
-DEFINE_ARRAY_WITH_PERCPU_KPTR(4096);
Considering the update in patch "bpf: Avoid unnecessary extra percpu
memory allocation", the definition of DEFINE_ARRAY_WITH_PERCPU_KPTR()
needs update as well, because for 512-sized per-cpu kptr, the tests only
allocate for (512 - sizeof(void *)) bytes. And we could do
DEFINE_ARRAY_WITH_PERCPU_KPTR(8) test after the update. I could do that
after the patch-set is landed if you don't have time to do that.
A bit of off-topic, but it is still relevant. I have a question about
how to forcibly generate BTF info for struct definition in the test ?
Currently, I have to include bin_data_xx in the definition of
map_value, but I don't want to increase the size of map_value. I had
tried to use BTF_TYPE_EMIT() in prog just like in linux kernel, but it
didn't work.
Since you mentioned the btf generation issue, I did some investigation.
To workaround btf generation issue, we can use the method in
prog_tests/local_kptr_stash.c:
====
/* This is necessary so that LLVM generates BTF for node_data struct
* If it's not included, a fwd reference for node_data will be generated but
* no struct. Example BTF of "node" field in map_value when not included:
*
* [10] PTR '(anon)' type_id=35
* [34] FWD 'node_data' fwd_kind=struct
* [35] TYPE_TAG 'kptr_ref' type_id=34
*
* (with no node_data struct defined)
* Had to do the same w/ bpf_kfunc_call_test_release below
*/
struct node_data *just_here_because_btf_bug;
struct refcounted_node *just_here_because_btf_bug2;
====
I have hacked the test_bpf_ma.c files and something like below
should work to generate btf types:
struct bin_data_##_size { \
char data[_size - sizeof(void *)]; \
}; \
+ /* See Commit 5d8d6634ccc, force btf generation for type bin_data_##_size */ \
+ struct bin_data_##_size *__bin_data_##_size; \
struct map_value_##_size { \
struct bin_data_##_size __kptr * data; \
- /* To emit BTF info for bin_data_xx */ \
- struct bin_data_##_size not_used; \
}; \
struct { \
__uint(type, BPF_MAP_TYPE_ARRAY); \
@@ -40,8 +43,12 @@ int pid = 0;
} array_##_size SEC(".maps")
#define DEFINE_ARRAY_WITH_PERCPU_KPTR(_size) \
+ struct percpu_bin_data_##_size { \
+ char data[_size]; \
+ }; \
+ struct percpu_bin_data_##_size *__percpu_bin_data_##_size; \
struct map_value_percpu_##_size { \
- struct bin_data_##_size __percpu_kptr * data; \
+ struct percpu_bin_data_##_size __percpu_kptr * data; \
}; \
struct { \
__uint(type, BPF_MAP_TYPE_ARRAY); \
I have a prototype to ensure the type (for percpu kptr) removing these
'- sizeof(void *)' and enabling DEFINE_ARRAY_WITH_PERCPU_KPTR().
Once we resolved the check_obj_size() issue, I can then post v3.
SEC("?fentry/" SYS_PREFIX "sys_nanosleep")
int test_batch_alloc_free(void *ctx)
@@ -259,9 +256,6 @@ int test_batch_percpu_alloc_free(void *ctx)
CALL_BATCH_PERCPU_ALLOC_FREE(192, 128, 6);
CALL_BATCH_PERCPU_ALLOC_FREE(256, 128, 7);
CALL_BATCH_PERCPU_ALLOC_FREE(512, 64, 8);
- CALL_BATCH_PERCPU_ALLOC_FREE(1024, 32, 9);
- CALL_BATCH_PERCPU_ALLOC_FREE(2048, 16, 10);
- CALL_BATCH_PERCPU_ALLOC_FREE(4096, 8, 11);
return 0;
}
@@ -283,9 +277,6 @@ int test_percpu_free_through_map_free(void *ctx)
CALL_BATCH_PERCPU_ALLOC(192, 128, 6);
CALL_BATCH_PERCPU_ALLOC(256, 128, 7);
CALL_BATCH_PERCPU_ALLOC(512, 64, 8);
- CALL_BATCH_PERCPU_ALLOC(1024, 32, 9);
- CALL_BATCH_PERCPU_ALLOC(2048, 16, 10);
- CALL_BATCH_PERCPU_ALLOC(4096, 8, 11);
return 0;
}
