Commit 8082b856 authored by Andrii Nakryiko's avatar Andrii Nakryiko

Merge branch 'bpf: keep track of verifier insn_processed'

Dave Marchevsky says:

====================

This is a followup to discussion around RFC patchset "bpf: keep track of
prog verification stats" [0]. The RFC elaborates on my usecase, but to
summarize: keeping track of verifier stats for programs as they - and
the kernels they run on - change over time can help developers of
individual programs and BPF kernel folks.

The RFC added a verif_stats to the uapi which contained most of the info
which verifier prints currently. Feedback here was to avoid polluting
uapi with stats that might be meaningless after major changes to the
verifier, but that insn_processed or conceptually similar number would
exist in the long term and was safe to expose.

So let's expose just insn_processed via bpf_prog_info and fdinfo for now
and explore good ways of getting more complicated stats in the future.

[0] https://lore.kernel.org/bpf/20210920151112.3770991-1-davemarchevsky@fb.com/

v2->v3:
  * Remove unnecessary check in patch 2's test [Andrii]
  * Go back to adding new u32 in bpf_prog_info (vs using spare bits) [Andrii]
	* Rebase + add acks [Andrii, John]

v1->v2:
  * Rename uapi field from insn_processed to verified_insns [Daniel]
  * use 31 bits of existing bitfield space in bpf_prog_info [Daniel]
  * change underlying type from 64-> 32 bits [Daniel]
====================
Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents 632f96d2 e1b9023f
...@@ -887,6 +887,7 @@ struct bpf_prog_aux { ...@@ -887,6 +887,7 @@ struct bpf_prog_aux {
struct bpf_prog *prog; struct bpf_prog *prog;
struct user_struct *user; struct user_struct *user;
u64 load_time; /* ns since boottime */ u64 load_time; /* ns since boottime */
u32 verified_insns;
struct bpf_map *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]; struct bpf_map *cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE];
char name[BPF_OBJ_NAME_LEN]; char name[BPF_OBJ_NAME_LEN];
#ifdef CONFIG_SECURITY #ifdef CONFIG_SECURITY
......
...@@ -5620,6 +5620,7 @@ struct bpf_prog_info { ...@@ -5620,6 +5620,7 @@ struct bpf_prog_info {
__u64 run_time_ns; __u64 run_time_ns;
__u64 run_cnt; __u64 run_cnt;
__u64 recursion_misses; __u64 recursion_misses;
__u32 verified_insns;
} __attribute__((aligned(8))); } __attribute__((aligned(8)));
struct bpf_map_info { struct bpf_map_info {
......
...@@ -1848,7 +1848,8 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp) ...@@ -1848,7 +1848,8 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
"prog_id:\t%u\n" "prog_id:\t%u\n"
"run_time_ns:\t%llu\n" "run_time_ns:\t%llu\n"
"run_cnt:\t%llu\n" "run_cnt:\t%llu\n"
"recursion_misses:\t%llu\n", "recursion_misses:\t%llu\n"
"verified_insns:\t%u\n",
prog->type, prog->type,
prog->jited, prog->jited,
prog_tag, prog_tag,
...@@ -1856,7 +1857,8 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp) ...@@ -1856,7 +1857,8 @@ static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
prog->aux->id, prog->aux->id,
stats.nsecs, stats.nsecs,
stats.cnt, stats.cnt,
stats.misses); stats.misses,
prog->aux->verified_insns);
} }
#endif #endif
...@@ -3625,6 +3627,8 @@ static int bpf_prog_get_info_by_fd(struct file *file, ...@@ -3625,6 +3627,8 @@ static int bpf_prog_get_info_by_fd(struct file *file,
info.run_cnt = stats.cnt; info.run_cnt = stats.cnt;
info.recursion_misses = stats.misses; info.recursion_misses = stats.misses;
info.verified_insns = prog->aux->verified_insns;
if (!bpf_capable()) { if (!bpf_capable()) {
info.jited_prog_len = 0; info.jited_prog_len = 0;
info.xlated_prog_len = 0; info.xlated_prog_len = 0;
......
...@@ -14033,6 +14033,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr) ...@@ -14033,6 +14033,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr)
env->verification_time = ktime_get_ns() - start_time; env->verification_time = ktime_get_ns() - start_time;
print_verification_stats(env); print_verification_stats(env);
env->prog->aux->verified_insns = env->insn_processed;
if (log->level && bpf_verifier_log_full(log)) if (log->level && bpf_verifier_log_full(log))
ret = -ENOSPC; ret = -ENOSPC;
......
...@@ -5620,6 +5620,7 @@ struct bpf_prog_info { ...@@ -5620,6 +5620,7 @@ struct bpf_prog_info {
__u64 run_time_ns; __u64 run_time_ns;
__u64 run_cnt; __u64 run_cnt;
__u64 recursion_misses; __u64 recursion_misses;
__u32 verified_insns;
} __attribute__((aligned(8))); } __attribute__((aligned(8)));
struct bpf_map_info { struct bpf_map_info {
......
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2021 Facebook */
#include <test_progs.h>
#include "trace_vprintk.lskel.h"
void test_verif_stats(void)
{
__u32 len = sizeof(struct bpf_prog_info);
struct bpf_prog_info info = {};
struct trace_vprintk *skel;
int err;
skel = trace_vprintk__open_and_load();
if (!ASSERT_OK_PTR(skel, "trace_vprintk__open_and_load"))
goto cleanup;
err = bpf_obj_get_info_by_fd(skel->progs.sys_enter.prog_fd, &info, &len);
if (!ASSERT_OK(err, "bpf_obj_get_info_by_fd"))
goto cleanup;
if (!ASSERT_GT(info.verified_insns, 0, "verified_insns"))
goto cleanup;
cleanup:
trace_vprintk__destroy(skel);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment