Re: [PATCH bpf-next v2 5/5] selftests/bpf: Add a selftest with available_filter_functions_addrs

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Mar 21, 2024 at 01:01:24PM -0700, Yonghong Song wrote:
> The current kprobe_multi_bench_attach/kernel test
> reads sym names from /sys/kernel/tracing/available_filter_functions.
> Some names do not agree with the corresponding entries in /proc/kallsyms
> since the corresponding /proc/kallsyms syms have suffix '.llvm.<hash>'.
> Actually, if we pass symbol names in /proc/kallsyms,
> kprobe_multi_attach will be okay.
> 
> This patch added a new subtest where addresses are retrieved from
> /sys/kernel/tracing/available_filter_functions_addrs, and use the
> address to consule /proc/kallsyms to get the function name.

hm, I don't understand the reason for this test.. AFAICS test_kprobe_multi_bench_attach
is doing that already, just reading available_filter_functions file

both available_filter_functions_addrs and available_filter_functions have the
same functions, there's just extra addresses in available_filter_functions_addrs

> +	*symsp = syms;
> +	*cntp = cnt;
> +
> +error:
> +	fclose(f);
> +	hashmap__free(map);
> +	if (err) {
> +		for (i = 0; i < cnt; i++)
> +			free(syms[i]);
> +		free(syms);
> +	}
> +	return err;
> +}
> +
>  static void test_kprobe_multi_bench_attach(bool kernel)
>  {
>  	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
> @@ -521,6 +617,47 @@ static void test_attach_override(void)
>  	kprobe_multi_override__destroy(skel);
>  }

there's lot of duplicated code in both
  get_syms_from_addr/get_syms
  test_attach_kernel_addrs_to_sym/test_kprobe_multi_bench_attach

would be great to put it together

>  
> +static void test_attach_kernel_addrs_to_sym(void)
> +{
> +	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
> +	struct kprobe_multi_empty *skel;
> +	struct bpf_link *link;
> +	char **syms = NULL;
> +	size_t cnt = 0;
> +	int i, err;
> +
> +	err = get_syms_from_addr(&syms, &cnt);
> +	if (err == -ENOENT) {
> +		test__skip();
> +		return;
> +	}
> +	if (!ASSERT_OK(err, "get_syms_from_addr"))
> +		return;
> +
> +	skel = kprobe_multi_empty__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "kprobe_multi_empty__open_and_load"))
> +		goto cleanup;
> +
> +	opts.syms = (const char **) syms;
> +	opts.cnt = cnt;
> +
> +	link = bpf_program__attach_kprobe_multi_opts(skel->progs.test_kprobe_empty,
> +						     NULL, &opts);
> +
> +	if (!ASSERT_OK_PTR(link, "bpf_program__attach_kprobe_multi_opts"))
> +		goto cleanup;
> +
> +	bpf_link__destroy(link);
> +
> +cleanup:
> +	kprobe_multi_empty__destroy(skel);
> +	if (syms) {
> +		for (i = 0; i < cnt; i++)
> +			free(syms[i]);
> +		free(syms);
> +	}
> +}
> +
>  void serial_test_kprobe_multi_bench_attach(void)
>  {
>  	if (test__start_subtest("kernel"))
> @@ -550,4 +687,6 @@ void test_kprobe_multi_test(void)
>  		test_attach_api_fails();
>  	if (test__start_subtest("attach_override"))
>  		test_attach_override();
> +	if (test__start_subtest("kernel_addrs_to_sym"))
> +		test_attach_kernel_addrs_to_sym();

we moved the bench subtests to serial_test_kprobe_multi_bench_attach,
not to clash with others in parallel mode

jirka