On Fri, Jun 3, 2022 at 8:42 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> Add a selftest that calls a global function with a context object parameter
> from an freplace function to check that the program context type is
> correctly converted to the freplace target when fetching the context type
> from the kernel BTF.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 13 ++++++++++
> .../bpf/progs/freplace_global_func.c | 24 +++++++++++++++++++
> 2 files changed, 37 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/freplace_global_func.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> index d9aad15e0d24..6e86a1d92e97 100644
> --- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
> @@ -395,6 +395,17 @@ static void test_func_map_prog_compatibility(void)
> "./test_attach_probe.o");
> }
>
> +static void test_func_replace_global_func(void)
> +{
> + const char *prog_name[] = {
> + "freplace/test_pkt_access",
> + };
empty line between variables and statements
> + test_fexit_bpf2bpf_common("./freplace_global_func.o",
> + "./test_pkt_access.o",
> + ARRAY_SIZE(prog_name),
> + prog_name, false, NULL);
> +}
> +
> /* NOTE: affect other tests, must run in serial mode */
> void serial_test_fexit_bpf2bpf(void)
> {
> @@ -416,4 +427,6 @@ void serial_test_fexit_bpf2bpf(void)
> test_func_replace_multi();
> if (test__start_subtest("fmod_ret_freplace"))
> test_fmod_ret_freplace();
> + if (test__start_subtest("func_replace_global_func"))
> + test_func_replace_global_func();
> }
> diff --git a/tools/testing/selftests/bpf/progs/freplace_global_func.c b/tools/testing/selftests/bpf/progs/freplace_global_func.c
> new file mode 100644
> index 000000000000..d9f8276229cc
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/freplace_global_func.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019 Facebook */
> +#include <linux/stddef.h>
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_endian.h>
> +#include <bpf/bpf_tracing.h>
> +
> +__attribute__ ((noinline))
__noinline
> +int test_ctx_global_func(struct __sk_buff *skb)
> +{
> + volatile int retval = 1;
> + return retval;
just curious, why volatile instead of direct `return 1;`? Also, empty
line between variable and return.
> +}
> +
> +__u64 test_pkt_access_global_func = 0;
nit: it's a variable, please put it at the top before functions, or at
the very least add empty line between it and SEC()
> +SEC("freplace/test_pkt_access")
> +int new_test_pkt_access(struct __sk_buff *skb)
> +{
> + test_pkt_access_global_func = test_ctx_global_func(skb);
> + return -1;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.36.1
>
