Re: [PATCH bpf-next] selftests/bpf: increase verifier log limit in veristat

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Oct 21, 2024 at 03:16:16PM +0100, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> 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 <yatsenko@meta.com>
> ---
>  tools/testing/selftests/bpf/veristat.c | 40 +++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
> index c8efd44590d9..1d0708839f4b 100644
> --- a/tools/testing/selftests/bpf/veristat.c
> +++ b/tools/testing/selftests/bpf/veristat.c
> @@ -16,10 +16,12 @@
>  #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>
>  #include <math.h>
> +#include <linux/filter.h>
>  
>  #ifndef ARRAY_SIZE
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
> @@ -1109,6 +1111,42 @@ static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const ch
>  	return;
>  }
>  
> +static int max_verifier_log_size(void)
> +{
> +	const int big_log_size = UINT_MAX >> 2;
> +	const int small_log_size = UINT_MAX >> 8;
> +	struct bpf_insn insns[] = {
> +		BPF_MOV64_IMM(BPF_REG_0, 0),
> +		BPF_EXIT_INSN(),
> +	};
> +	int ret, insn_cnt = ARRAY_SIZE(insns);
> +	char *log_buf;
> +	static int log_size;
> +
> +	if (log_size != 0)
> +		return log_size;
> +
> +	log_size = small_log_size;
> +	log_buf = malloc(big_log_size);

IIUC this would try to use 1GB by default? seems to agresive.. could we perhaps
do that gradually and double the size on each failed load attempt?

jirka


> +
> +	if (!log_buf)
> +		return log_size;
> +
> +	LIBBPF_OPTS(bpf_prog_load_opts, opts,
> +		    .log_buf = log_buf,
> +		    .log_size = big_log_size,
> +		    .log_level = 2
> +	);
> +	ret = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, NULL, "GPL", insns, insn_cnt, &opts);
> +	free(log_buf);
> +
> +	if (ret > 0) {
> +		log_size = big_log_size;
> +		close(ret);
> +	}
> +	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 +1170,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
> 
>