Re: [PATCH bpf-next v1 3/3] libbpf: Wire up verifier warning display logic

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Wire up the flushing of all accumulated messages in a program's stderr
> stream after verification is complete. An example is shown below of a
> warning printed now about usage of deprecated kfuncs.
>
> $ ./test_progs -t kfunc_implicit_args/test_kfunc_implicit_arg_legacy_impl -v
> ...
> libbpf: prog 'test_kfunc_implicit_arg_legacy_impl': VERIFIER WARNINGS:
> WARNING: kfunc_implicit_args.c:40 calls deprecated kfunc bpf_kfunc_implicit_arg_legacy_impl(), which will be removed.
> WARNING: Switch to kfunc bpf_kfunc_implicit_arg_legacy() instead.
> WARNING: For older kernels, choose the kfunc using bpf_ksym_exists(bpf_kfunc_implicit_arg_legacy).
> ...
>
> Note that test_progs overrides the default logging function, hence -v is
> necessary. By default these messages would be printed for the user.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  tools/lib/bpf/libbpf.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9ea41f40dc82..f308bacd083f 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7826,6 +7826,27 @@ static int libbpf_prepare_prog_load(struct bpf_program *prog,
>  
>  static void fixup_verifier_log(struct bpf_program *prog, char *buf, size_t buf_sz);
>  
> +static void bpf_object_load_prog_emit_stderr(struct bpf_program *prog, int prog_fd)

nit: Not sure if this function name is a good choice: it does not take
bpf object and does not load the program, I'd rather name it
bpf_program_emit_verifier_stderr().

The implementation looks correct.
Acked-by: Mykyta Yatsenko <yatsenko@meta.com>

> +{
> +	char chunk[256];
> +	bool emitted = false;
> +	int ret;
> +
> +	while ((ret = bpf_prog_stream_read(prog_fd, BPF_STREAM_STDERR, chunk,
> +					   sizeof(chunk), NULL)) > 0) {
> +		if (!emitted) {
> +			pr_warn("prog '%s': VERIFIER WARNINGS:\n", prog->name);
> +			emitted = true;
> +		}
> +		libbpf_print(LIBBPF_WARN, "%.*s", ret, chunk);
> +	}
> +
> +	if (ret < 0) {
> +		pr_debug("prog '%s': failed to read BPF stderr stream: %s\n",
> +			 prog->name, errstr(ret));
> +	}
> +}
> +
>  static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog,
>  				struct bpf_insn *insns, int insns_cnt,
>  				const char *license, __u32 kern_version, int *prog_fd)
> @@ -7950,6 +7971,8 @@ static int bpf_object_load_prog(struct bpf_object *obj, struct bpf_program *prog
>  				 prog->name, log_buf);
>  		}
>  
> +		bpf_object_load_prog_emit_stderr(prog, ret);
> +
>  		if (obj->has_rodata && kernel_supports(obj, FEAT_PROG_BIND_MAP)) {
>  			struct bpf_map *map;
>  			int i;
> -- 
> 2.52.0