On Wed, 2024-11-20 at 16:53 -0800, Kumar Kartikeya Dwivedi wrote:
> Include tests that check for rejection in erroneous cases, like
> unbalanced IRQ-disabled counts, within and across subprogs, invalid IRQ
> flag state or input to kfuncs, behavior upon overwriting IRQ saved state
> on stack, interaction with sleepable kfuncs/helpers, global functions,
> and out of order restore. Include some success scenarios as well to
> demonstrate usage.
>
> #123/1 irq/irq_restore_missing_1:OK
> #123/2 irq/irq_restore_missing_2:OK
> #123/3 irq/irq_restore_missing_3:OK
> #123/4 irq/irq_restore_missing_3_minus_2:OK
> #123/5 irq/irq_restore_missing_1_subprog:OK
> #123/6 irq/irq_restore_missing_2_subprog:OK
> #123/7 irq/irq_restore_missing_3_subprog:OK
> #123/8 irq/irq_restore_missing_3_minus_2_subprog:OK
> #123/9 irq/irq_balance:OK
> #123/10 irq/irq_balance_n:OK
> #123/11 irq/irq_balance_subprog:OK
> #123/12 irq/irq_balance_n_subprog:OK
> #123/13 irq/irq_global_subprog:OK
> #123/14 irq/irq_restore_ooo:OK
> #123/15 irq/irq_restore_ooo_3:OK
> #123/16 irq/irq_restore_3_subprog:OK
> #123/17 irq/irq_restore_4_subprog:OK
> #123/18 irq/irq_restore_ooo_3_subprog:OK
> #123/19 irq/irq_restore_invalid:OK
> #123/20 irq/irq_save_invalid:OK
> #123/21 irq/irq_restore_iter:OK
> #123/22 irq/irq_save_iter:OK
> #123/23 irq/irq_flag_overwrite:OK
> #123/24 irq/irq_flag_overwrite_partial:OK
> #123/25 irq/irq_sleepable_helper:OK
> #123/26 irq/irq_sleepable_kfunc:OK
> #123 irq:OK
> Summary: 1/26 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
The following error condition is not tested:
"arg#%d doesn't point to an irq flag on stack".
Also, I think a few tests are excessive.
Otherwise looks good.
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> diff --git a/tools/testing/selftests/bpf/prog_tests/irq.c b/tools/testing/selftests/bpf/prog_tests/irq.c
> new file mode 100644
> index 000000000000..496f4826ac37
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/irq.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
> +#include <test_progs.h>
> +#include <irq.skel.h>
> +
> +void test_irq(void)
> +{
> + RUN_TESTS(irq);
> +}
Nit: tools/testing/selftests/bpf/prog_tests/verifier.c
could be used instead of a separate file.
> diff --git a/tools/testing/selftests/bpf/progs/irq.c b/tools/testing/selftests/bpf/progs/irq.c
> new file mode 100644
> index 000000000000..5301b66fc752
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/irq.c
> @@ -0,0 +1,393 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +SEC("?tc")
> +__failure __msg("BPF_EXIT instruction cannot be used inside bpf_local_irq_save-ed region")
Nit: I know this is not a fault of this series, but the error message
is sort of confusing. BPF_EXIT is allowed for irq saved region,
just it has to be an exit from a sub-program, not a whole program.
> +int irq_restore_missing_1(struct __sk_buff *ctx)
> +{
> + unsigned long flags;
> +
> + bpf_local_irq_save(&flags);
> + return 0;
> +}
[...]
Nit: don't think this test adds much compared to irq_restore_missing_2.
> +{
> + unsigned long flags1;
> + unsigned long flags2;
> + unsigned long flags3;
> +
> + bpf_local_irq_save(&flags1);
> + bpf_local_irq_save(&flags2);
> + bpf_local_irq_save(&flags3);
> + return 0;
> +}
[...]
> +SEC("?tc")
> +__success
> +int irq_balance_n_subprog(struct __sk_buff *ctx)
Nit: don't think this test adds much given irq_balance_n()
and irq_balance_subprog().
> +{
> + local_irq_balance_n();
> + return 0;
> +}
[...]
