Re: [PATCH bpf-next v13 13/14] selftests/bpf: test case for register_bpf_struct_ops().

[Martin KaFai Lau <martin.lau@linux.dev>]

On 12/8/23 4:27 PM, thinker.li@gmail.com wrote:
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> new file mode 100644
> index 000000000000..55a4c6ed92aa
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +#include <test_progs.h>
> +#include <time.h>
> +
> +#include "struct_ops_module.skel.h"
> +#include "testmod_unload.skel.h"
> +
> +static void test_regular_load(void)
> +{
> +	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
> +	struct struct_ops_module *skel;
> +	struct testmod_unload *skel_unload;
> +	struct bpf_link *link_map_free = NULL;
> +	struct bpf_link *link;
> +	int err, i;
> +
> +	skel = struct_ops_module__open_opts(&opts);
> +	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
> +		return;
> +
> +	err = struct_ops_module__load(skel);
> +	if (!ASSERT_OK(err, "struct_ops_module_load"))
> +		goto cleanup;
> +
> +	link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
> +	ASSERT_OK_PTR(link, "attach_test_mod_1");
> +
> +	/* test_2() will be called from bpf_dummy_reg() in bpf_testmod.c */
> +	ASSERT_EQ(skel->bss->test_2_result, 7, "test_2_result");
> +
> +	bpf_link__destroy(link);
> +
> +cleanup:
> +	skel_unload = testmod_unload__open_and_load();
> +
> +	if (ASSERT_OK_PTR(skel_unload, "testmod_unload_open"))
> +		link_map_free = bpf_program__attach(skel_unload->progs.trace_map_free);
> +	struct_ops_module__destroy(skel);
> +
> +	if (!ASSERT_OK_PTR(link_map_free, "create_link_map_free"))
> +		return;
> +
> +	/* Wait for the struct_ops map to be freed. Struct_ops maps hold a
> +	 * refcount to the module btf. And, this function unloads and then
> +	 * loads bpf_testmod. Without waiting the map to be freed, the next
> +	 * test may fail to unload the bpf_testmod module since the map is
> +	 * still holding a refcnt to the module.
> +	 */
> +	for (i = 0; i < 10; i++) {
> +		if (skel_unload->bss->bpf_testmod_put)
> +			break;
> +		usleep(100000);
> +	}
> +	ASSERT_EQ(skel_unload->bss->bpf_testmod_put, 1, "map_free");
> +
> +	bpf_link__destroy(link_map_free);
> +	testmod_unload__destroy(skel_unload);
> +}
> +
> +static void test_load_without_module(void)
> +{
> +	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
> +	struct struct_ops_module *skel;
> +	int err;
> +
> +	err = unload_bpf_testmod(false);
> +	if (!ASSERT_OK(err, "unload_bpf_testmod"))
> +		return;
> +
> +	skel = struct_ops_module__open_opts(&opts);
> +	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
> +		goto cleanup;
> +	err = struct_ops_module__load(skel);

Both the module btf and the .ko itself are gone from the kernel now?
This is basically testing libbpf cannot find 'struct bpf_testmod_ops' from the 
running kernel?

How about create another struct_ops_module_notfound.c bpf program:
SEC(".struct_ops.link")
struct bpf_testmod_ops_notfound testmod_1 = {
	.test_1 = (void *)test_1,
	.test_2 = (void *)test_2,
};

and avoid the usleep() wait and the unload_bpf_testmod()?

> +	ASSERT_ERR(err, "struct_ops_module_load");
> +
> +	struct_ops_module__destroy(skel);
> +
> +cleanup:
> +	/* Without this, the next test may fail */
> +	load_bpf_testmod(false);
> +}
> +
> +void serial_test_struct_ops_module(void)
> +{
> +	if (test__start_subtest("regular_load"))
> +		test_regular_load();
> +
> +	if (test__start_subtest("load_without_module"))
> +		test_load_without_module();
> +}
> +