2023-07-13 05:28:31

by GONG, Ruiqi

[permalink] [raw]
Subject: [PATCH] bpf: Add support of skipping the current object for bpf_iter progs

bpf_seq_read() can accept three different types of seq_ops->show()'s
return value:

err > 0: skip the obj and reuse seq_num
err < 0: abort the whole iter process
err == 0 (implicitly): continue

but bpf_iter_run_prog() is limited to the last two cases. Extend the
legal return value of bpf_iter progs so that they can skip certain
objects and then proceed to the followings.

Signed-off-by: GONG, Ruiqi <[email protected]>
---
kernel/bpf/bpf_iter.c | 9 +++++----
kernel/bpf/verifier.c | 1 +
2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 96856f130cbf..1c1d67ec466c 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -716,13 +716,14 @@ int bpf_iter_run_prog(struct bpf_prog *prog, void *ctx)
rcu_read_unlock();
}

- /* bpf program can only return 0 or 1:
- * 0 : okay
- * 1 : retry the same object
+ /* bpf program can return:
+ * 0 : has shown the object, go next
+ * 1 : has skipped the object, go next
+ * -1 : encountered error and should terminate
* The bpf_iter_run_prog() return value
* will be seq_ops->show() return value.
*/
- return ret == 0 ? 0 : -EAGAIN;
+ return ret == 0 ? 0 : (ret == 1 ? 1 : -EAGAIN);
}

BPF_CALL_4(bpf_for_each_map_elem, struct bpf_map *, map, void *, callback_fn,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 930b5555cfd3..cebd3a0b3172 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14333,6 +14333,7 @@ static int check_return_code(struct bpf_verifier_env *env)
case BPF_MODIFY_RETURN:
return 0;
case BPF_TRACE_ITER:
+ range = tnum_range(-1, 1);
break;
default:
return -ENOTSUPP;
--
2.25.1



2023-07-14 00:31:30

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH] bpf: Add support of skipping the current object for bpf_iter progs

On Thu, Jul 13, 2023 at 01:13:23PM +0800, GONG, Ruiqi wrote:
> bpf_seq_read() can accept three different types of seq_ops->show()'s
> return value:
>
> err > 0: skip the obj and reuse seq_num
> err < 0: abort the whole iter process
> err == 0 (implicitly): continue
>
> but bpf_iter_run_prog() is limited to the last two cases. Extend the
> legal return value of bpf_iter progs so that they can skip certain
> objects and then proceed to the followings.
>
> Signed-off-by: GONG, Ruiqi <[email protected]>
> ---
> kernel/bpf/bpf_iter.c | 9 +++++----
> kernel/bpf/verifier.c | 1 +
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index 96856f130cbf..1c1d67ec466c 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -716,13 +716,14 @@ int bpf_iter_run_prog(struct bpf_prog *prog, void *ctx)
> rcu_read_unlock();
> }
>
> - /* bpf program can only return 0 or 1:
> - * 0 : okay
> - * 1 : retry the same object
> + /* bpf program can return:
> + * 0 : has shown the object, go next
> + * 1 : has skipped the object, go next
> + * -1 : encountered error and should terminate
> * The bpf_iter_run_prog() return value
> * will be seq_ops->show() return value.
> */
> - return ret == 0 ? 0 : -EAGAIN;
> + return ret == 0 ? 0 : (ret == 1 ? 1 : -EAGAIN);

This breaks existing progs as you can see in CI
and you surely would have noticed if you run the selftests.

We're going to start auto rejecting patches without selftests and
those that break CI.
It's your job to test your patches.