[PATCH bpf-next v2 0/3] bpf: Maximum combined stack depth

[PATCH bpf-next v2 0/3] bpf: Maximum combined stack depth

[Paul Chaignon]

This patchset dumps the maximum combined stack depth in verifier logs
and parses it in veristat.

Changes in v2:
  - Remove unnecessary max_stack_depth assignment (Eduard).
  - Fix and test incorrect handling of private stacks.
  - Add veristat metric (Eduard).

Paul Chaignon (3):
  bpf: Report maximum combined stack depth
  selftests/bpf: Test reported max stack depth
  veristat: Report max stack depth

 include/linux/bpf_verifier.h                      |  2 ++
 kernel/bpf/verifier.c                             |  6 +++++-
 .../selftests/bpf/progs/verifier_bpf_fastcall.c   |  3 +--
 .../selftests/bpf/progs/verifier_private_stack.c  | 15 +++++++++++++++
 tools/testing/selftests/bpf/veristat.c            | 11 ++++++++---
 5 files changed, 31 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH bpf-next v2 1/3] bpf: Report maximum combined stack depth

[Paul Chaignon]

We've hit the 512 bytes limit on stack depth a few times in Cilium
recently. As a result, we started reporting in CI our current maximum
stack depth across all configurations for each BPF program.

Unfortunately, that is not trivial to compute in userspace. The
verifier reports the stack depths of individual subprogs at the end of
the logs. However the maximum combined stack depth also depends on the
callgraph of those subprogs (the max combined stack depth is the height
of the callgraph weighted by per-subprog stack depths). We can compute
a callgraph in userspace from the loaded instructions, but it often
doesn't match the verifier's own callgraph because of dead code
elimination. Our current approach relies on dumping the BPF_LOG_LEVEL2
logs, but this feels overkill considering the verifier already has the
information we need.

The patch lets the verifier dump the maximum combined stack depth in
the logs, on the same line as the per-subprog stack depths:

    stack depth 16+256 max 272

The per-subprog stack depths and the new max stack depth are not
directly comparable. The former is sometimes updated during fixups,
while the latter is not. As a result, even with a single subprog, we
may end up with two slightly different values. The aim of the new max
value is to be closest to what is actually enforced by the verifier.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 include/linux/bpf_verifier.h | 2 ++
 kernel/bpf/verifier.c        | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index c15a4c26a43b..b051671c7239 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -952,6 +952,8 @@ struct bpf_verifier_env {
 	u32 prev_insn_processed, insn_processed;
 	/* number of jmps, calls, exits analyzed so far */
 	u32 prev_jmps_processed, jmps_processed;
+	/* maximum combined stack depth */
+	u32 max_stack_depth;
 	/* total verification time */
 	u64 verification_time;
 	/* maximum number of verifier states kept in 'branching' instructions */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e654ef01ae0..ae92223d1106 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5038,6 +5038,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
 	}
 
 	if (subprog[idx].priv_stack_mode == PRIV_STACK_ADAPTIVE) {
+		if (subprog_depth > env->max_stack_depth)
+			env->max_stack_depth = subprog_depth;
 		if (subprog_depth > MAX_BPF_STACK) {
 			verbose(env, "stack size of subprog %d is %d. Too large\n",
 				idx, subprog_depth);
@@ -5045,6 +5047,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
 		}
 	} else {
 		depth += subprog_depth;
+		if (depth > env->max_stack_depth)
+			env->max_stack_depth = depth;
 		if (depth > MAX_BPF_STACK) {
 			total = 0;
 			for (tmp = idx; tmp >= 0; tmp = dinfo[tmp].caller)
@@ -18289,7 +18293,7 @@ static void print_verification_stats(struct bpf_verifier_env *env)
 		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
 		for (i = 1; i < subprog_cnt; i++)
 			verbose(env, "+%d", env->subprog_info[i].stack_depth);
-		verbose(env, "\n");
+		verbose(env, " max %d\n", env->max_stack_depth);
 		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
 		for (i = 1; i < subprog_cnt; i++)
 			if (bpf_subprog_is_global(env, i))
-- 
2.43.0

[PATCH bpf-next v2 2/3] selftests/bpf: Test reported max stack depth

[Paul Chaignon]

This patch tests the maximum stack depth reporting in verifier logs,
with a couple special cases covered: fastcall, private stacks (main
subprog & callee), and rounding up to 16 bytes. For that last one, we
need to skip the test when JIT compilation is disabled as the rounding
is then to 32 bytes.

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 .../selftests/bpf/progs/verifier_bpf_fastcall.c   |  3 +--
 .../selftests/bpf/progs/verifier_private_stack.c  | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
index 0d9e167555b5..8d7ff38e4c06 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
@@ -799,8 +799,7 @@ __naked int bpf_loop_interaction2(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4)
-__msg("stack depth 512+0")
+__log_level(4) __msg("stack depth 512+0 max 512")
 /* just to print xlated version when debugging */
 __xlated("r0 = &(void __percpu *)(r0)")
 __success
diff --git a/tools/testing/selftests/bpf/progs/verifier_private_stack.c b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
index 646e8ef82051..2c00dd8579d1 100644
--- a/tools/testing/selftests/bpf/progs/verifier_private_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
@@ -86,6 +86,7 @@ __naked static void cumulative_stack_depth_subprog(void)
 SEC("kprobe")
 __description("Private stack, subtree > MAX_BPF_STACK")
 __success
+__log_level(4) __msg("stack depth 512+32 max 512")
 __arch_x86_64
 /* private stack fp for the main prog */
 __jited("	movabsq	$0x{{.*}}, %r9")
@@ -324,6 +325,8 @@ int private_stack_async_callback_1(void)
 SEC("fentry/bpf_fentry_test9")
 __description("Private stack, async callback, potential nesting")
 __success __retval(0)
+__load_if_JITed()
+__log_level(4) __msg("stack depth 8+0+256+0 max 272")
 __arch_x86_64
 __jited("	subq	$0x100, %rsp")
 __arch_arm64
@@ -344,6 +347,18 @@ int private_stack_async_callback_2(void)
 	return 0;
 }
 
+SEC("fentry/bpf_fentry_test9")
+__description("private stack, max stack depth is private stack")
+__success
+__log_level(4) __msg("stack depth 8+256+0 max 256")
+int private_stack_max_depth(void)
+{
+	int x = 0;
+
+	subprog1(&x);
+	return 0;
+}
+
 #else
 
 SEC("kprobe")
-- 
2.43.0

[PATCH bpf-next v2 3/3] veristat: Report max stack depth

[Paul Chaignon]

This patch adds a new "Max stack depth" field to the set of gathered
statistics. This field reports the maximum combined stack depth compared
to the 512 bytes limit. It is null for rejected programs.

Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 tools/testing/selftests/bpf/veristat.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 5c82950e6978..9a2275d19ee1 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -48,6 +48,7 @@ enum stat_id {
 	SIZE,
 	JITED_SIZE,
 	STACK,
+	MAX_STACK,
 	PROG_TYPE,
 	ATTACH_TYPE,
 	MEMORY_PEAK,
@@ -795,7 +796,7 @@ static const struct stat_specs default_csv_output_spec = {
 		TOTAL_INSNS, TOTAL_STATES, PEAK_STATES,
 		MAX_STATES_PER_INSN, MARK_READ_MAX_LEN,
 		SIZE, JITED_SIZE, PROG_TYPE, ATTACH_TYPE,
-		STACK, MEMORY_PEAK,
+		STACK, MAX_STACK, MEMORY_PEAK,
 	},
 };
 
@@ -834,6 +835,7 @@ static struct stat_def {
 	[SIZE] = { "Program size", {"prog_size"}, },
 	[JITED_SIZE] = { "Jited size", {"prog_size_jited"}, },
 	[STACK] = {"Stack depth", {"stack_depth", "stack"}, },
+	[MAX_STACK] = {"Max stack depth", {"max_stack_depth"}, },
 	[PROG_TYPE] = { "Program type", {"prog_type"}, },
 	[ATTACH_TYPE] = { "Attach type", {"attach_type", }, },
 	[MEMORY_PEAK] = { "Peak memory (MiB)", {"mem_peak", }, },
@@ -1023,7 +1025,7 @@ static int parse_verif_log(char * const buf, size_t buf_sz, struct verif_stats *
 				&s->stats[MARK_READ_MAX_LEN]))
 			continue;
 
-		if (1 == sscanf(cur, "stack depth %511s", stack))
+		if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
 			continue;
 	}
 	while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
@@ -2278,6 +2280,7 @@ static int cmp_stat(const struct verif_stats *s1, const struct verif_stats *s2,
 	case SIZE:
 	case JITED_SIZE:
 	case STACK:
+	case MAX_STACK:
 	case VERDICT:
 	case DURATION:
 	case TOTAL_INSNS:
@@ -2512,6 +2515,7 @@ static void prepare_value(const struct verif_stats *s, enum stat_id id,
 	case MAX_STATES_PER_INSN:
 	case MARK_READ_MAX_LEN:
 	case STACK:
+	case MAX_STACK:
 	case SIZE:
 	case JITED_SIZE:
 	case MEMORY_PEAK:
@@ -2602,7 +2606,8 @@ static int parse_stat_value(const char *str, enum stat_id id, struct verif_stats
 	case SIZE:
 	case JITED_SIZE:
 	case MEMORY_PEAK:
-	case STACK: {
+	case STACK:
+	case MAX_STACK: {
 		long val;
 		int err, n;
 
-- 
2.43.0

Re: [PATCH bpf-next v2 3/3] veristat: Report max stack depth

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index 5c82950e6978..9a2275d19ee1 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -48,6 +48,7 @@ enum stat_id {
>  	SIZE,
>  	JITED_SIZE,
>  	STACK,
> +	MAX_STACK,
>  	PROG_TYPE,
>  	ATTACH_TYPE,
>  	MEMORY_PEAK,

[ ... ]

> @@ -795,7 +796,7 @@ static const struct stat_specs default_csv_output_spec = {
>  		TOTAL_INSNS, TOTAL_STATES, PEAK_STATES,
>  		MAX_STATES_PER_INSN, MARK_READ_MAX_LEN,
>  		SIZE, JITED_SIZE, PROG_TYPE, ATTACH_TYPE,
> -		STACK, MEMORY_PEAK,
> +		STACK, MAX_STACK, MEMORY_PEAK,
>  	},
>  };

Does this need .spec_cnt updated to 16? The ids array now contains 16
elements (FILE_NAME, PROG_NAME, VERDICT, DURATION, TOTAL_INSNS,
TOTAL_STATES, PEAK_STATES, MAX_STATES_PER_INSN, MARK_READ_MAX_LEN, SIZE,
JITED_SIZE, PROG_TYPE, ATTACH_TYPE, STACK, MAX_STACK, MEMORY_PEAK), but
.spec_cnt is still 15. All consumers of this struct (output_stats,
output_comp_stats, output_header_underlines and the parse_stats_csv
column-count check) iterate from 0 to spec_cnt-1, so MEMORY_PEAK won't be
included in the default CSV output.

>
> @@ -834,6 +835,7 @@ static struct stat_def {
>  	[SIZE] = { "Program size", {"prog_size"}, },
>  	[JITED_SIZE] = { "Jited size", {"prog_size_jited"}, },
>  	[STACK] = {"Stack depth", {"stack_depth", "stack"}, },
> +	[MAX_STACK] = {"Max stack depth", {"max_stack_depth"}, },
>  	[PROG_TYPE] = { "Program type", {"prog_type"}, },
>  	[ATTACH_TYPE] = { "Attach type", {"attach_type", }, },
>  	[MEMORY_PEAK] = { "Peak memory (MiB)", {"mem_peak", }, },

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25813246860

Re: [PATCH bpf-next v2 1/3] bpf: Report maximum combined stack depth

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] On early verifier failure, 'max_stack_depth' logs as 0, which is inconsistent with the individual subprog stack depths shown.
--

commit 2f8f05d6e2f1d0777f2f9a6fc7f4f3d3c65baf3f
Author: Paul Chaignon <paul.chaignon@gmail.com>

bpf: Report maximum combined stack depth

This commit adds reporting for the maximum combined stack depth of BPF
programs to the verifier logs. It prints the max combined depth alongside
the per-subprog stack depths to provide a value closer to what is actually
enforced by the verifier.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 11054ad89c14d..39340a6780ebc 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -18289,7 +18293,7 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>  		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
>  		for (i = 1; i < subprog_cnt; i++)
>  			verbose(env, "+%d", env->subprog_info[i].stack_depth);
> -		verbose(env, "\n");
> +		verbose(env, " max %d\n", env->max_stack_depth);

If the verifier fails early during do_check_main() or do_check_subprogs(),
the process skips check_max_stack_depth() and jumps directly to the
cleanup and logging phase.

Since env->max_stack_depth is only calculated in check_max_stack_depth(),
could this result in confusing log output on early failures where the
subprog depths are printed with a max depth of 0?

For example, it might output something like:
stack depth 16+256 max 0

Would it be better to conditionally print this value only if the max stack
depth check actually ran, or otherwise indicate that the check was
bypassed?

>  		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
>  		for (i = 1; i < subprog_cnt; i++)
>  			if (bpf_subprog_is_global(env, i))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778689025.git.paul.chaignon@gmail.com?part=1