Misc improvements on bpf_func_info
Subsystems (8)
cover Misc improvements on bpf_func_info
The patchset has a few improvements on bpf_func_info:
1. Improvements on the behaviors of info.func_info, info.func_info_cnt and info.func_info_rec_size.
2. Name change: s/insn_offset/insn_off/
Please see individual patch for details.
Martin KaFai Lau (4):
bpf: Improve the info.func_info and info.func_info_rec_size behavior
bpf: Change insn_offset to insn_off in bpf_func_info
bpf: tools: Sync uapi bpf.h for the name changes in bpf_func_info
bpf: Expect !info.func_info and insn_off name changes in
test_btf/libbpf/bpftool
include/uapi/linux/bpf.h | 2 +- kernel/bpf/core.c | 2 +- kernel/bpf/syscall.c | 46 +++++++++++--------------- kernel/bpf/verifier.c | 18 +++++----- tools/bpf/bpftool/prog.c | 7 ++++ tools/bpf/bpftool/xlated_dumper.c | 4 +-- tools/include/uapi/linux/bpf.h | 2 +- tools/lib/bpf/btf.c | 12 +++---- tools/testing/selftests/bpf/test_btf.c | 8 ++++- 9 files changed, 54 insertions(+), 47 deletions(-)
Signature · 1 line
2.17.1
PATCH 1/4 bpf: Improve the info.func_info and info.func_info_rec_size behavior
1) When bpf_dump_raw_ok() == false and the kernel can provide >=1 func_info to the userspace, the current behavior is setting the info.func_info_cnt to 0 instead of setting info.func_info to 0.
It is different from the behavior in jited_func_lens/nr_jited_func_lens, jited_ksyms/nr_jited_ksyms...etc.
This patch fixes it. (i.e. set func_info to 0 instead of func_info_cnt to 0 when bpf_dump_raw_ok() == false).
2) When the userspace passed in info.func_info_cnt == 0, the kernel will set the expected func_info size back to the info.func_info_rec_size. It is a way for the userspace to learn the kernel expected func_info_rec_size introduced in commit 838e96904ff3 ("bpf: Introduce bpf_func_info").
An exception is the kernel expected size is not set when func_info is not available for a bpf_prog. This makes the returned info.func_info_rec_size has different values depending on the returned value of info.func_info_cnt.
This patch sets the kernel expected size to info.func_info_rec_size independent of the info.func_info_cnt.
3) The current logic only rejects invalid func_info_rec_size if func_info_cnt is non zero. This patch also rejects invalid nonzero info.func_info_rec_size and not equal to the kernel expected size.
4) Set info.btf_id as long as prog->aux->btf != NULL. That will setup the later copy_to_user() codes look the same as others which then easier to understand and maintain.
prog->aux->btf is not NULL only if prog->aux->func_info_cnt > 0.
Breaking up info.btf_id from prog->aux->func_info_cnt is needed for the later line info patch anyway.
A similar change is made to bpf_get_prog_name().
--- kernel/bpf/core.c | 2 +- kernel/bpf/syscall.c | 46 +++++++++++++++++++------------------------- 2 files changed, 21 insertions(+), 27 deletions(-)
kernel/bpf/core.c +1 -1
@@ -410,7 +410,7 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym) sym =bin2hex (sym, prog->tag,sizeof (prog->tag));/* prog->aux->name will be ignored if full btf name is available */ -if (prog->aux->btf) { +if (prog->aux->func_info_cnt) { type =btf_type_by_id (prog->aux->btf, prog->aux->func_info[prog->aux->func_idx].type_id); func_name =btf_name_by_offset (prog->aux->btf, type->name_off);
kernel/bpf/syscall.c +20 -26
61 lines not on this page · raw message
Signature · 1 line
2.17.1
PATCH 2/4 bpf: Change insn_offset to insn_off in bpf_func_info
In reply to Martin KaFai LauThe later patch will introduce "struct bpf_line_info" which has member "line_off" and "file_off" referring back to the string section in btf. The line_"off" and file_"off" are more consistent to the naming convention in btf.h that means "offset" (e.g. name_off in "struct btf_type").
The to-be-added "struct bpf_line_info" also has another member, "insn_off" which is the same as the "insn_offset" in "struct bpf_func_info". Hence, this patch renames "insn_offset" to "insn_off" for "struct bpf_func_info".
--- include/uapi/linux/bpf.h | 2 +- kernel/bpf/verifier.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-)
include/uapi/linux/bpf.h +1 -1
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys { };struct bpf_func_info { -__u32 insn_offset; +__u32 insn_off;__u32 type_id; };
kernel/bpf/verifier.c +9 -9
@@ -4707,24 +4707,24 @@ static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env, goto free_btf; } -/* check insn_offset */ +/* check insn_off */ if (i ==0 ) { -if (krecord[i].insn_offset) { +if (krecord[i].insn_off) {verbose (env, -"nonzero insn_offset %u for the first func info record" , - krecord[i].insn_offset); +"nonzero insn_off %u for the first func info record" , + krecord[i].insn_off); ret = -EINVAL ; goto free_btf; } - }else if (krecord[i].insn_offset <= prev_offset) { + }else if (krecord[i].insn_off <= prev_offset) {verbose (env,"same or smaller insn offset (%u) than previous func info record (%u)" , - krecord[i].insn_offset, prev_offset); + krecord[i].insn_off, prev_offset); ret = -EINVAL ; goto free_btf; } -if (env->subprog_info[i].start != krecord[i].insn_offset) { +if (env->subprog_info[i].start != krecord[i].insn_off) {verbose (env,"func_info BTF section doesn't match subprog layout in BPF program\n" ); ret = -EINVAL ; goto free_btf;
@@ -4739,7 +4739,7 @@ static int check_btf_func(struct bpf_prog *prog, struct bpf_verifier_env *env, goto free_btf; } - prev_offset = krecord[i].insn_offset; + prev_offset = krecord[i].insn_off; urecord += urec_size; }
@@ -4762,7 +4762,7 @@ static void adjust_btf_func(struct bpf_verifier_env *env)return ;for (i =0 ; i < env->subprog_cnt; i++) - env->prog->aux->func_info[i].insn_offset = env->subprog_info[i].start; + env->prog->aux->func_info[i].insn_off = env->subprog_info[i].start; }/* check %cur's range satisfies %old's */
Signature · 1 line
2.17.1
PATCH 3/4 bpf: tools: Sync uapi bpf.h for the name changes in bpf_func_info
In reply to Martin KaFai LauThis patch sync the name changes in bpf_func_info to the tools/.
--- tools/include/uapi/linux/bpf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/include/uapi/linux/bpf.h +1 -1
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys { };struct bpf_func_info { -__u32 insn_offset; +__u32 insn_off;__u32 type_id; };
Signature · 1 line
2.17.1
PATCH 4/4 bpf: Expect !info.func_info and insn_off name changes in test_btf/libbpf/bpftool
In reply to Martin KaFai LauSimilar to info.jited_*, info.func_info could be 0 if bpf_dump_raw_ok() == false.
This patch makes changes to test_btf and bpftool to expect info.func_info could be 0.
This patch also makes the needed changes for s/insn_offset/insn_off/.
--- tools/bpf/bpftool/prog.c | 7 +++++++ tools/bpf/bpftool/xlated_dumper.c | 4 ++-- tools/lib/bpf/btf.c | 12 ++++++------ tools/testing/selftests/bpf/test_btf.c | 8 +++++++- 4 files changed, 22 insertions(+), 9 deletions(-)
tools/bpf/bpftool/prog.c +7 -0
@@ -589,6 +589,13 @@ static int do_dump(int argc, char **argv) goto err_free; } +if (func_info && !info.func_info) { +/* kernel.kptr_restrict is set. No func_info available. */ +free (func_info); + func_info =NULL ; + finfo_cnt =0 ; + } + if ((member_len == &info.jited_prog_len && info.jited_prog_insns == 0) || (member_len == &info.xlated_prog_len &&
tools/bpf/bpftool/xlated_dumper.c +2 -2
@@ -261,7 +261,7 @@ void dump_xlated_json(struct dump_data *dd, void *buf, unsigned int len,jsonw_start_object (json_wtr);if (btf && record) { -if (record->insn_offset == i) { +if (record->insn_off == i) {btf_dumper_type_only (btf, record->type_id, func_sig,sizeof (func_sig));
@@ -330,7 +330,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, unsigned int len, }if (btf && record) { -if (record->insn_offset == i) { +if (record->insn_off == i) {btf_dumper_type_only (btf, record->type_id, func_sig,sizeof (func_sig));
tools/lib/bpf/btf.c +6 -6
@@ -45,7 +45,7 @@ struct btf_ext {/* The minimum bpf_func_info checked by the loader */ struct bpf_func_info_min { -__u32 insn_offset; +__u32 insn_off;__u32 type_id; };
@@ -670,7 +670,7 @@ int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,memcpy (data, sinfo->data, records_len); -/* adjust the insn_offset, the data in .BTF.ext is +/* adjust the insn_off, the data in .BTF.ext is * the actual byte offset, and the kernel expects * the offset in term of bpf_insn. *
@@ -681,7 +681,7 @@ int btf_ext__reloc_init(struct btf *btf, struct btf_ext *btf_ext,struct bpf_func_info_min *record; record = data + i * record_size; - record->insn_offset /=sizeof (struct bpf_insn ); + record->insn_off /=sizeof (struct bpf_insn ); } *func_info = data;
@@ -722,15 +722,15 @@ int btf_ext__reloc(struct btf *btf, struct btf_ext *btf_ext,return -ENOMEM ;memcpy (data + existing_flen, sinfo->data, records_len); -/* adjust insn_offset only, the rest data will be passed +/* adjust insn_off only, the rest data will be passed * to the kernel. */ for (i =0 ; i < sinfo->num_func_info; i++) {struct bpf_func_info_min *record; record = data + existing_flen + i * record_size; - record->insn_offset = - record->insn_offset /sizeof (struct bpf_insn ) + + record->insn_off = + record->insn_off /sizeof (struct bpf_insn ) + insns_cnt; } *func_info = data;
tools/testing/selftests/bpf/test_btf.c +7 -1
@@ -3156,7 +3156,7 @@ static struct btf_func_type_test { }, { - .descr = "func_type (Incorrect bpf_func_info.insn_offset)", + .descr = "func_type (Incorrect bpf_func_info.insn_off)", .raw_types = {BTF_TYPE_INT_ENC (NAME_TBD ,BTF_INT_SIGNED ,0 ,32 ,4 ),/* [1] */ BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 32, 4), /* [2] */
@@ -3303,6 +3303,12 @@ static int do_test_func_type(int test_num) goto done; } +if (CHECK (!info.func_info, +"info.func_info == 0. kernel.kptr_restrict is set?" )) { + err =-1 ; + goto done; + } + finfo = func_info;for (i =0 ; i <2 ; i++) { if (CHECK(finfo->type_id != test->func_info[i][1],
Signature · 1 line
2.17.1
Alexei Starovoitov
AppliedQuoted 6 lines · Martin KaFai Lau
The patchset has a few improvements on
bpf_func_info:
1. Improvements on the behaviors of info.func_info, info.func_info_cnt and info.func_info_rec_size.
2. Name change: s/insn_offset/insn_off/Please see individual patch for details.
Applied, Thanks