Re: [PATCH RFC bpf-next v1 9/9] selftests/bpf: Add tests for BPF exceptions

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Wed, Apr 05, 2023 at 02:42:39AM +0200, Kumar Kartikeya Dwivedi wrote:
> +static __noinline int throwing_subprog(struct __sk_buff *ctx)
> +{
> +	if (ctx)
> +		bpf_throw();
> +	return 0;
> +}
> +
> +__noinline int global_subprog(struct __sk_buff *ctx)
> +{
> +	return subprog(ctx) + 1;
> +}
> +
> +__noinline int throwing_global_subprog(struct __sk_buff *ctx)
> +{
> +	if (ctx)
> +		bpf_throw();
> +	return 0;
> +}
> +
> +static __noinline int exception_cb(void)
> +{
> +	return 16;
> +}
> +
> +SEC("tc")
> +int exception_throw_subprog(struct __sk_buff *ctx)
> +{
> +	volatile int i;
> +
> +	exception_cb();
> +	bpf_set_exception_callback(exception_cb);
> +	i = subprog(ctx);
> +	i += global_subprog(ctx) - 1;
> +	if (!i)
> +		return throwing_global_subprog(ctx);
> +	else
> +		return throwing_subprog(ctx);
> +	bpf_throw();
> +	return 0;
> +}
> +
> +__noinline int throwing_gfunc(volatile int i)
> +{
> +	bpf_assert_eq(i, 0);
> +	return 1;
> +}
> +
> +__noinline static int throwing_func(volatile int i)
> +{
> +	bpf_assert_lt(i, 1);
> +	return 1;
> +}

exception_cb() has no way of knowning which assert statement threw the exception.
How about extending a macro:
bpf_assert_eq(i, 0, MY_INT_ERR);
or
bpf_assert_eq(i, 0) {bpf_throw(MY_INT_ERR);}

bpf_throw can store it in prog->aux->exception pass the address to cb.

Also I think we shouldn't complicate the verifier with auto release of resources.
If the user really wants to assert when spin_lock is held it should be user's
job to specify what resources should be released.
Can we make it look like:

bpf_spin_lock(&lock);
bpf_assert_eq(i, 0) {
  bpf_spin_unlock(&lock);
  bpf_throw(MY_INT_ERR);
}