2022-10-12 23:55:41

by David Vernet

[permalink] [raw]
Subject: [PATCH 0/2] Allow bpf_user_ringbuf_drain() callbacks to return 1

The bpf_user_ringbuf_drain() helper function allows a BPF program to
specify a callback that is invoked when draining entries from a
BPF_MAP_TYPE_USER_RINGBUF ring buffer map. The API is meant to allow the
callback to return 0 if it wants to continue draining samples, and 1 if
it's done draining. Unfortunately, bpf_user_ringbuf_drain() landed shortly
after commit 1bfe26fb0827 ("bpf: Add verifier support for custom
callback return range"), which changed the default behavior of callbacks
to only support returning 0, and the corresponding necessary change to
bpf_user_ringbuf_drain() callbacks was missed.

This patch set fixes this oversight, and updates the user_ringbuf
selftests to return 1 in a callback to catch future instances of
regression.

This patch set should be merged to the bpf tree.

David Vernet (2):
bpf: Allow bpf_user_ringbuf_drain() callbacks to return 1
selftests/bpf: Make bpf_user_ringbuf_drain() selftest callback return
1

kernel/bpf/verifier.c | 1 +
tools/testing/selftests/bpf/progs/user_ringbuf_success.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)

--
2.38.0


2022-10-13 00:09:43

by David Vernet

[permalink] [raw]
Subject: [PATCH 1/2] bpf: Allow bpf_user_ringbuf_drain() callbacks to return 1

The bpf_user_ringbuf_drain() helper function allows a BPF program to
specify a callback that is invoked when draining entries from a
BPF_MAP_TYPE_USER_RINGBUF ring buffer map. The API is meant to allow the
callback to return 0 if it wants to continue draining samples, and 1 if
it's done draining. Unfortunately, bpf_user_ringbuf_drain() landed shortly
after commit 1bfe26fb0827 ("bpf: Add verifier support for custom
callback return range"), which changed the default behavior of callbacks
to only support returning 0.

This patch corrects that oversight by allowing bpf_user_ringbuf_drain()
callbacks to return 0 or 1. A follow-on patch will update the
user_ringbuf selftests to also return 1 from a bpf_user_ringbuf_drain()
callback to prevent this from regressing in the future.

Fixes: 205715673844 ("bpf: Add bpf_user_ringbuf_drain() helper")
Signed-off-by: David Vernet <[email protected]>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6f6d2d511c06..9ab7188d8f68 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6946,6 +6946,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
__mark_reg_not_init(env, &callee->regs[BPF_REG_5]);

callee->in_callback_fn = true;
+ callee->callback_ret_range = tnum_range(0, 1);
return 0;
}

--
2.38.0

2022-10-13 15:41:44

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH 0/2] Allow bpf_user_ringbuf_drain() callbacks to return 1

On Wed, Oct 12, 2022 at 4:20 PM David Vernet <[email protected]> wrote:
>
> The bpf_user_ringbuf_drain() helper function allows a BPF program to
> specify a callback that is invoked when draining entries from a
> BPF_MAP_TYPE_USER_RINGBUF ring buffer map. The API is meant to allow the
> callback to return 0 if it wants to continue draining samples, and 1 if
> it's done draining. Unfortunately, bpf_user_ringbuf_drain() landed shortly
> after commit 1bfe26fb0827 ("bpf: Add verifier support for custom
> callback return range"), which changed the default behavior of callbacks
> to only support returning 0, and the corresponding necessary change to
> bpf_user_ringbuf_drain() callbacks was missed.
>
> This patch set fixes this oversight, and updates the user_ringbuf
> selftests to return 1 in a callback to catch future instances of
> regression.
>
> This patch set should be merged to the bpf tree.

Please tag patch as [PATCH bpf x/N] next time. This will make it clear
that it's targeted against the bpf tree and will let our CI know that
it should be applied and tested in bpf (it chooses bpf-next by
default).

>
> David Vernet (2):
> bpf: Allow bpf_user_ringbuf_drain() callbacks to return 1
> selftests/bpf: Make bpf_user_ringbuf_drain() selftest callback return
> 1
>
> kernel/bpf/verifier.c | 1 +
> tools/testing/selftests/bpf/progs/user_ringbuf_success.c | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> --
> 2.38.0
>

2022-10-13 16:21:57

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH 0/2] Allow bpf_user_ringbuf_drain() callbacks to return 1

Hello:

This series was applied to bpf/bpf.git (master)
by Andrii Nakryiko <[email protected]>:

On Wed, 12 Oct 2022 18:20:13 -0500 you wrote:
> The bpf_user_ringbuf_drain() helper function allows a BPF program to
> specify a callback that is invoked when draining entries from a
> BPF_MAP_TYPE_USER_RINGBUF ring buffer map. The API is meant to allow the
> callback to return 0 if it wants to continue draining samples, and 1 if
> it's done draining. Unfortunately, bpf_user_ringbuf_drain() landed shortly
> after commit 1bfe26fb0827 ("bpf: Add verifier support for custom
> callback return range"), which changed the default behavior of callbacks
> to only support returning 0, and the corresponding necessary change to
> bpf_user_ringbuf_drain() callbacks was missed.
>
> [...]

Here is the summary with links:
- [1/2] bpf: Allow bpf_user_ringbuf_drain() callbacks to return 1
https://git.kernel.org/bpf/bpf/c/c92a7a522438
- [2/2] selftests/bpf: Make bpf_user_ringbuf_drain() selftest callback return 1
https://git.kernel.org/bpf/bpf/c/6e44b9f375a3

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html