On Mon, Feb 5, 2024 at 1:56 PM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote:
>
> Add selftests covering the following cases:
> - A static or global subprog called from within a RCU read section works
> - A static subprog taking an RCU read lock which is released in caller works
> - A static subprog releasing the caller's RCU read lock works
>
> Global subprogs that leave the lock in an imbalanced state will not
> work, as they are verified separately, so ensure those cases fail as
> well.
>
> Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
Acked-and-Tested-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/rcu_read_lock.c | 6 +
> .../selftests/bpf/progs/rcu_read_lock.c | 120 ++++++++++++++++++
> 2 files changed, 126 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c b/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
> index 3f1f58d3a729..a1f7e7378a64 100644
> --- a/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
> +++ b/tools/testing/selftests/bpf/prog_tests/rcu_read_lock.c
> @@ -29,6 +29,10 @@ static void test_success(void)
> bpf_program__set_autoload(skel->progs.non_sleepable_1, true);
> bpf_program__set_autoload(skel->progs.non_sleepable_2, true);
> bpf_program__set_autoload(skel->progs.task_trusted_non_rcuptr, true);
> + bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog, true);
> + bpf_program__set_autoload(skel->progs.rcu_read_lock_global_subprog, true);
> + bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_lock, true);
> + bpf_program__set_autoload(skel->progs.rcu_read_lock_subprog_unlock, true);
> err = rcu_read_lock__load(skel);
> if (!ASSERT_OK(err, "skel_load"))
> goto out;
> @@ -75,6 +79,8 @@ static const char * const inproper_region_tests[] = {
> "inproper_sleepable_helper",
> "inproper_sleepable_kfunc",
> "nested_rcu_region",
> + "rcu_read_lock_global_subprog_lock",
> + "rcu_read_lock_global_subprog_unlock",
> };
>
> static void test_inproper_region(void)
> diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> index 14fb01437fb8..ab3a532b7dd6 100644
> --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> @@ -319,3 +319,123 @@ int cross_rcu_region(void *ctx)
> bpf_rcu_read_unlock();
> return 0;
> }
> +
> +__noinline
> +static int static_subprog(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + if (bpf_get_prandom_u32())
> + return ret + 42;
> + return ret + bpf_get_prandom_u32();
> +}
> +
> +__noinline
> +int global_subprog(u64 a)
> +{
> + volatile int ret = a;
> +
> + return ret + static_subprog(NULL);
> +}
> +
> +__noinline
> +static int static_subprog_lock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_lock();
> + if (bpf_get_prandom_u32())
> + return ret + 42;
> + return ret + bpf_get_prandom_u32();
> +}
> +
> +__noinline
> +int global_subprog_lock(u64 a)
> +{
> + volatile int ret = a;
> +
> + return ret + static_subprog_lock(NULL);
> +}
> +
> +__noinline
> +static int static_subprog_unlock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_unlock();
> + if (bpf_get_prandom_u32())
> + return ret + 42;
> + return ret + bpf_get_prandom_u32();
> +}
> +
> +__noinline
> +int global_subprog_unlock(u64 a)
> +{
> + volatile int ret = a;
> +
> + return ret + static_subprog_unlock(NULL);
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_subprog(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_lock();
> + if (bpf_get_prandom_u32())
> + ret += static_subprog(ctx);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_global_subprog(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_lock();
> + if (bpf_get_prandom_u32())
> + ret += global_subprog(ret);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_subprog_lock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + ret += static_subprog_lock(ctx);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_global_subprog_lock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + ret += global_subprog_lock(ret);
> + bpf_rcu_read_unlock();
> + return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_subprog_unlock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_lock();
> + ret += static_subprog_unlock(ctx);
> + return 0;
> +}
> +
> +SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
> +int rcu_read_lock_global_subprog_unlock(void *ctx)
> +{
> + volatile int ret = 0;
> +
> + bpf_rcu_read_lock();
> + ret += global_subprog_unlock(ret);
> + return 0;
> +}
> --
> 2.40.1
>
--
Regards
Yafang
