* [PATCH bpf-next 0/4] Misc improvements on bpf_func_info
@ 2018-12-06 1:35 Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 1/4] bpf: Improve the info.func_info and info.func_info_rec_size behavior Martin KaFai Lau
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2018-12-06 1:35 UTC (permalink / raw
To: netdev; +Cc: kernel-team, Yonghong Song
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(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/4] bpf: Improve the info.func_info and info.func_info_rec_size behavior
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
@ 2018-12-06 1:35 ` Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 2/4] bpf: Change insn_offset to insn_off in bpf_func_info Martin KaFai Lau
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2018-12-06 1:35 UTC (permalink / raw
To: netdev; +Cc: kernel-team, Yonghong Song
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().
Fixes: 838e96904ff3 ("bpf: Introduce bpf_func_info")
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/core.c | 2 +-
kernel/bpf/syscall.c | 46 +++++++++++++++++++-------------------------
2 files changed, 21 insertions(+), 27 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index f93ed667546f..2a73fda1db5f 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -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);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 4445d0d084d8..aa05aa38f4a8 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2083,6 +2083,12 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
return -EFAULT;
}
+ if ((info.func_info_cnt || info.func_info_rec_size) &&
+ info.func_info_rec_size != sizeof(struct bpf_func_info))
+ return -EINVAL;
+
+ info.func_info_rec_size = sizeof(struct bpf_func_info);
+
if (!capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
@@ -2226,35 +2232,23 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
}
- if (prog->aux->btf) {
- u32 krec_size = sizeof(struct bpf_func_info);
- u32 ucnt, urec_size;
-
+ if (prog->aux->btf)
info.btf_id = btf_id(prog->aux->btf);
- ucnt = info.func_info_cnt;
- info.func_info_cnt = prog->aux->func_info_cnt;
- urec_size = info.func_info_rec_size;
- info.func_info_rec_size = krec_size;
- if (ucnt) {
- /* expect passed-in urec_size is what the kernel expects */
- if (urec_size != info.func_info_rec_size)
- return -EINVAL;
-
- if (bpf_dump_raw_ok()) {
- char __user *user_finfo;
-
- user_finfo = u64_to_user_ptr(info.func_info);
- ucnt = min_t(u32, info.func_info_cnt, ucnt);
- if (copy_to_user(user_finfo, prog->aux->func_info,
- krec_size * ucnt))
- return -EFAULT;
- } else {
- info.func_info_cnt = 0;
- }
+ ulen = info.func_info_cnt;
+ info.func_info_cnt = prog->aux->func_info_cnt;
+ if (info.func_info_cnt && ulen) {
+ if (bpf_dump_raw_ok()) {
+ char __user *user_finfo;
+
+ user_finfo = u64_to_user_ptr(info.func_info);
+ ulen = min_t(u32, info.func_info_cnt, ulen);
+ if (copy_to_user(user_finfo, prog->aux->func_info,
+ info.func_info_rec_size * ulen))
+ return -EFAULT;
+ } else {
+ info.func_info = 0;
}
- } else {
- info.func_info_cnt = 0;
}
done:
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/4] bpf: Change insn_offset to insn_off in bpf_func_info
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 1/4] bpf: Improve the info.func_info and info.func_info_rec_size behavior Martin KaFai Lau
@ 2018-12-06 1:35 ` Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 3/4] bpf: tools: Sync uapi bpf.h for the name changes " Martin KaFai Lau
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2018-12-06 1:35 UTC (permalink / raw
To: netdev; +Cc: kernel-team, Yonghong Song
The 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".
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
include/uapi/linux/bpf.h | 2 +-
kernel/bpf/verifier.c | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c8e1eeee2c5f..a84fd232d934 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys {
};
struct bpf_func_info {
- __u32 insn_offset;
+ __u32 insn_off;
__u32 type_id;
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 71988337ac14..7658c61c1a88 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -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 */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 3/4] bpf: tools: Sync uapi bpf.h for the name changes in bpf_func_info
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 1/4] bpf: Improve the info.func_info and info.func_info_rec_size behavior Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 2/4] bpf: Change insn_offset to insn_off in bpf_func_info Martin KaFai Lau
@ 2018-12-06 1:35 ` Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 4/4] bpf: Expect !info.func_info and insn_off name changes in test_btf/libbpf/bpftool Martin KaFai Lau
2018-12-06 2:53 ` [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Alexei Starovoitov
4 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2018-12-06 1:35 UTC (permalink / raw
To: netdev; +Cc: kernel-team, Yonghong Song
This patch sync the name changes in bpf_func_info to
the tools/.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/include/uapi/linux/bpf.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 64262890feb2..16263e8827fc 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2991,7 +2991,7 @@ struct bpf_flow_keys {
};
struct bpf_func_info {
- __u32 insn_offset;
+ __u32 insn_off;
__u32 type_id;
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 4/4] bpf: Expect !info.func_info and insn_off name changes in test_btf/libbpf/bpftool
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
` (2 preceding siblings ...)
2018-12-06 1:35 ` [PATCH bpf-next 3/4] bpf: tools: Sync uapi bpf.h for the name changes " Martin KaFai Lau
@ 2018-12-06 1:35 ` Martin KaFai Lau
2018-12-06 2:53 ` [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Alexei Starovoitov
4 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2018-12-06 1:35 UTC (permalink / raw
To: netdev; +Cc: kernel-team, Yonghong Song
Similar 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/.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
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(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 56db61c5a91f..3148bc0e225b 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -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 &&
diff --git a/tools/bpf/bpftool/xlated_dumper.c b/tools/bpf/bpftool/xlated_dumper.c
index e06ac0286a75..131ecd175533 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -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));
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index c2d641f3e16e..85d6446cf832 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -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;
diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index bae7308b7ec5..ff0952ea757a 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -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],
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 0/4] Misc improvements on bpf_func_info
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
` (3 preceding siblings ...)
2018-12-06 1:35 ` [PATCH bpf-next 4/4] bpf: Expect !info.func_info and insn_off name changes in test_btf/libbpf/bpftool Martin KaFai Lau
@ 2018-12-06 2:53 ` Alexei Starovoitov
4 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2018-12-06 2:53 UTC (permalink / raw
To: Martin KaFai Lau; +Cc: netdev, kernel-team, Yonghong Song
On Wed, Dec 05, 2018 at 05:35:43PM -0800, Martin KaFai Lau wrote:
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-06 2:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-06 1:35 [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 1/4] bpf: Improve the info.func_info and info.func_info_rec_size behavior Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 2/4] bpf: Change insn_offset to insn_off in bpf_func_info Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 3/4] bpf: tools: Sync uapi bpf.h for the name changes " Martin KaFai Lau
2018-12-06 1:35 ` [PATCH bpf-next 4/4] bpf: Expect !info.func_info and insn_off name changes in test_btf/libbpf/bpftool Martin KaFai Lau
2018-12-06 2:53 ` [PATCH bpf-next 0/4] Misc improvements on bpf_func_info Alexei Starovoitov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.