[bpf-next,v2] selftests/bpf: increase verifier log limit in veristat

Commit Message

Mykyta Yatsenko Oct. 23, 2024, 3:53 p.m. UTC

From: Mykyta Yatsenko <[e-mail]>

The current default buffer size of 16MB allocated by veristat is no
longer sufficient to hold the verifier logs of some production BPF
programs. To address this issue, we need to increase the verifier log
limit.
Commit 7a9f5c65abcc ("bpf: increase verifier log limit") has already
increased the supported buffer size by the kernel, but veristat users
need to explicitly pass a log size argument to use the bigger log.

This patch adds a function to detect the maximum verifier log size
supported by the kernel and uses that by default in veristat.
This ensures that veristat can handle larger verifier logs without
requiring users to manually specify the log size.

Signed-off-by: Mykyta Yatsenko <[e-mail]>
---
 tools/testing/selftests/bpf/veristat.c | 42 +++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

Comments

Andrii Nakryiko Oct. 23, 2024, 5:48 p.m. UTC | #1

On Wed, Oct 23, 2024 at 8:53 AM Mykyta Yatsenko
<[e-mail]> wrote:
>
> From: Mykyta Yatsenko <[e-mail]>
>
> The current default buffer size of 16MB allocated by veristat is no
> longer sufficient to hold the verifier logs of some production BPF
> programs. To address this issue, we need to increase the verifier log
> limit.
> Commit 7a9f5c65abcc ("bpf: increase verifier log limit") has already
> increased the supported buffer size by the kernel, but veristat users
> need to explicitly pass a log size argument to use the bigger log.
>
> This patch adds a function to detect the maximum verifier log size
> supported by the kernel and uses that by default in veristat.
> This ensures that veristat can handle larger verifier logs without
> requiring users to manually specify the log size.
>
> Signed-off-by: Mykyta Yatsenko <[e-mail]>
> ---
>  tools/testing/selftests/bpf/veristat.c | 42 +++++++++++++++++++++++++-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index c8efd44590d9..a8498b1a2898 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -16,6 +16,7 @@
>  #include <sys/stat.h>
>  #include <bpf/libbpf.h>
>  #include <bpf/btf.h>
> +#include <bpf/bpf.h>
>  #include <libelf.h>
>  #include <gelf.h>
>  #include <float.h>
> @@ -1109,6 +1110,45 @@ static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
>         return;
>  }
>
> +static int max_verifier_log_size(void)
> +{
> +       const int SMALL_LOG_SIZE = UINT_MAX >> 8;
> +       const int BIG_LOG_SIZE = UINT_MAX >> 2;
> +       struct bpf_insn insns[] = {
> +               {
> +               .code  = BPF_ALU | BPF_MOV | BPF_X,
> +               .dst_reg = BPF_REG_0,
> +               .src_reg = 0,
> +               .off   = 0,
> +               .imm   = 0 },
> +               {
> +               .code  = BPF_JMP | BPF_EXIT,
> +               .dst_reg = 0,
> +               .src_reg = 0,
> +               .off   = 0,
> +               .imm   = 0 },
> +       };

I reformated this:

diff --git a/tools/testing/selftests/bpf/veristat.c
b/tools/testing/selftests/bpf/veristat.c
index a8498b1a2898..e12ef953fba8 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1115,18 +1115,8 @@ static int max_verifier_log_size(void)
        const int SMALL_LOG_SIZE = UINT_MAX >> 8;
        const int BIG_LOG_SIZE = UINT_MAX >> 2;
        struct bpf_insn insns[] = {
-               {
-               .code  = BPF_ALU | BPF_MOV | BPF_X,
-               .dst_reg = BPF_REG_0,
-               .src_reg = 0,
-               .off   = 0,
-               .imm   = 0 },
-               {
-               .code  = BPF_JMP | BPF_EXIT,
-               .dst_reg = 0,
-               .src_reg = 0,
-               .off   = 0,
-               .imm   = 0 },
+               { .code = BPF_ALU | BPF_MOV | BPF_X, .dst_reg = BPF_REG_0, },
+               { .code  = BPF_JMP | BPF_EXIT, },
        };
        LIBBPF_OPTS(bpf_prog_load_opts, opts,
                    .log_size = BIG_LOG_SIZE,

Applied to bpf-next, thanks!

> +       LIBBPF_OPTS(bpf_prog_load_opts, opts,
> +                   .log_size = BIG_LOG_SIZE,
> +                   .log_buf = (void *)-1,
> +                   .log_level = 4
> +       );
> +       int ret, insn_cnt = ARRAY_SIZE(insns);
> +       static int log_size;
> +
> +       if (log_size != 0)
> +               return log_size;
> +
> +       ret = bpf_prog_load(BPF_PROG_TYPE_TRACEPOINT, NULL, "GPL", insns, insn_cnt, &opts);
> +
> +       if (ret == -EFAULT)
> +               log_size = BIG_LOG_SIZE;
> +       else /* ret == -EINVAL, big log size is not supported by the verifier */
> +               log_size = SMALL_LOG_SIZE;
> +
> +       return log_size;
> +}
> +
>  static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
>  {
>         const char *base_filename = basename(strdupa(filename));
> @@ -1132,7 +1172,7 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
>         memset(stats, 0, sizeof(*stats));
>
>         if (env.verbose || env.top_src_lines > 0) {
> -               buf_sz = env.log_size ? env.log_size : 16 * 1024 * 1024;
> +               buf_sz = env.log_size ? env.log_size : max_verifier_log_size();
>                 buf = malloc(buf_sz);
>                 if (!buf)
>                         return -ENOMEM;
> --
> 2.47.0
>

[e-mail] Oct. 23, 2024, 5:50 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[e-mail]>:

On Wed, 23 Oct 2024 16:53:14 +0100 you wrote:
> From: Mykyta Yatsenko <[e-mail]>
> 
> The current default buffer size of 16MB allocated by veristat is no
> longer sufficient to hold the verifier logs of some production BPF
> programs. To address this issue, we need to increase the verifier log
> limit.
> Commit 7a9f5c65abcc ("bpf: increase verifier log limit") has already
> increased the supported buffer size by the kernel, but veristat users
> need to explicitly pass a log size argument to use the bigger log.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] selftests/bpf: increase verifier log limit in veristat
    https://git.kernel.org/bpf/bpf-next/c/1f7c33630724

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index c8efd44590d9..a8498b1a2898 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -16,6 +16,7 @@ 
 #include <sys/stat.h>
 #include <bpf/libbpf.h>
 #include <bpf/btf.h>
+#include <bpf/bpf.h>
 #include <libelf.h>
 #include <gelf.h>
 #include <float.h>
@@ -1109,6 +1110,45 @@  static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
 	return;
 }
 
+static int max_verifier_log_size(void)
+{
+	const int SMALL_LOG_SIZE = UINT_MAX >> 8;
+	const int BIG_LOG_SIZE = UINT_MAX >> 2;
+	struct bpf_insn insns[] = {
+		{
+		.code  = BPF_ALU | BPF_MOV | BPF_X,
+		.dst_reg = BPF_REG_0,
+		.src_reg = 0,
+		.off   = 0,
+		.imm   = 0 },
+		{
+		.code  = BPF_JMP | BPF_EXIT,
+		.dst_reg = 0,
+		.src_reg = 0,
+		.off   = 0,
+		.imm   = 0 },
+	};
+	LIBBPF_OPTS(bpf_prog_load_opts, opts,
+		    .log_size = BIG_LOG_SIZE,
+		    .log_buf = (void *)-1,
+		    .log_level = 4
+	);
+	int ret, insn_cnt = ARRAY_SIZE(insns);
+	static int log_size;
+
+	if (log_size != 0)
+		return log_size;
+
+	ret = bpf_prog_load(BPF_PROG_TYPE_TRACEPOINT, NULL, "GPL", insns, insn_cnt, &opts);
+
+	if (ret == -EFAULT)
+		log_size = BIG_LOG_SIZE;
+	else /* ret == -EINVAL, big log size is not supported by the verifier */
+		log_size = SMALL_LOG_SIZE;
+
+	return log_size;
+}
+
 static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
 {
 	const char *base_filename = basename(strdupa(filename));
@@ -1132,7 +1172,7 @@  static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
 	memset(stats, 0, sizeof(*stats));
 
 	if (env.verbose || env.top_src_lines > 0) {
-		buf_sz = env.log_size ? env.log_size : 16 * 1024 * 1024;
+		buf_sz = env.log_size ? env.log_size : max_verifier_log_size();
 		buf = malloc(buf_sz);
 		if (!buf)
 			return -ENOMEM;