| Message ID | 20210328161055.257504-1-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf-next] bpf: add 'BPF_RB_MAY_WAKEUP' flag | expand |
> On Mar 28, 2021, at 9:10 AM, Pedro Tammela <pctammela@gmail.com> wrote: > > The current way to provide a no-op flag to 'bpf_ringbuf_submit()', > 'bpf_ringbuf_discard()' and 'bpf_ringbuf_output()' is to provide a '0' > value. > > A '0' value might notify the consumer if it already caught up in processing, > so let's provide a more descriptive notation for this value. > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Song Liu <songliubraving@fb.com>
On Sun, Mar 28, 2021 at 9:11 AM Pedro Tammela <pctammela@gmail.com> wrote: > > The current way to provide a no-op flag to 'bpf_ringbuf_submit()', > 'bpf_ringbuf_discard()' and 'bpf_ringbuf_output()' is to provide a '0' > value. > > A '0' value might notify the consumer if it already caught up in processing, > so let's provide a more descriptive notation for this value. > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > --- flags == 0 means "no extra modifiers of behavior". That's default adaptive notification. If you want to adjust default behavior, only then you specify non-zero flags. I don't think anyone will bother typing BPF_RB_MAY_WAKEUP for this, nor I think it's really needed. The documentation update is nice (if no flags are specified notification will be sent if needed), but the new "pseudo-flag" seems like an overkill to me. > include/uapi/linux/bpf.h | 8 ++++++++ > tools/include/uapi/linux/bpf.h | 8 ++++++++ > tools/testing/selftests/bpf/progs/ima.c | 2 +- > tools/testing/selftests/bpf/progs/ringbuf_bench.c | 2 +- > tools/testing/selftests/bpf/progs/test_ringbuf.c | 2 +- > tools/testing/selftests/bpf/progs/test_ringbuf_multi.c | 2 +- > 6 files changed, 20 insertions(+), 4 deletions(-) > [...]
Em qua., 31 de mar. de 2021 às 03:54, Andrii Nakryiko <andrii.nakryiko@gmail.com> escreveu: > > On Sun, Mar 28, 2021 at 9:11 AM Pedro Tammela <pctammela@gmail.com> wrote: > > > > The current way to provide a no-op flag to 'bpf_ringbuf_submit()', > > 'bpf_ringbuf_discard()' and 'bpf_ringbuf_output()' is to provide a '0' > > value. > > > > A '0' value might notify the consumer if it already caught up in processing, > > so let's provide a more descriptive notation for this value. > > > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > > --- > > flags == 0 means "no extra modifiers of behavior". That's default > adaptive notification. If you want to adjust default behavior, only > then you specify non-zero flags. I don't think anyone will bother > typing BPF_RB_MAY_WAKEUP for this, nor I think it's really needed. The > documentation update is nice (if no flags are specified notification > will be sent if needed), but the new "pseudo-flag" seems like an > overkill to me. My intention here is to make '0' more descriptive. But if you think just the documentation update is enough, then I will remove the flag. > > > include/uapi/linux/bpf.h | 8 ++++++++ > > tools/include/uapi/linux/bpf.h | 8 ++++++++ > > tools/testing/selftests/bpf/progs/ima.c | 2 +- > > tools/testing/selftests/bpf/progs/ringbuf_bench.c | 2 +- > > tools/testing/selftests/bpf/progs/test_ringbuf.c | 2 +- > > tools/testing/selftests/bpf/progs/test_ringbuf_multi.c | 2 +- > > 6 files changed, 20 insertions(+), 4 deletions(-) > > > > [...]
On Sat, Apr 3, 2021 at 6:34 AM Pedro Tammela <pctammela@gmail.com> wrote: > > Em qua., 31 de mar. de 2021 às 03:54, Andrii Nakryiko > <andrii.nakryiko@gmail.com> escreveu: > > > > On Sun, Mar 28, 2021 at 9:11 AM Pedro Tammela <pctammela@gmail.com> wrote: > > > > > > The current way to provide a no-op flag to 'bpf_ringbuf_submit()', > > > 'bpf_ringbuf_discard()' and 'bpf_ringbuf_output()' is to provide a '0' > > > value. > > > > > > A '0' value might notify the consumer if it already caught up in processing, > > > so let's provide a more descriptive notation for this value. > > > > > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > > > --- > > > > flags == 0 means "no extra modifiers of behavior". That's default > > adaptive notification. If you want to adjust default behavior, only > > then you specify non-zero flags. I don't think anyone will bother > > typing BPF_RB_MAY_WAKEUP for this, nor I think it's really needed. The > > documentation update is nice (if no flags are specified notification > > will be sent if needed), but the new "pseudo-flag" seems like an > > overkill to me. > > My intention here is to make '0' more descriptive. > But if you think just the documentation update is enough, then I will > remove the flag. flags == 0 means "default behavior", I don't think you have to remember which verbose flag you need to specify for that, so I think just expanding documentation is sufficient and better. Thanks! > > > > > > include/uapi/linux/bpf.h | 8 ++++++++ > > > tools/include/uapi/linux/bpf.h | 8 ++++++++ > > > tools/testing/selftests/bpf/progs/ima.c | 2 +- > > > tools/testing/selftests/bpf/progs/ringbuf_bench.c | 2 +- > > > tools/testing/selftests/bpf/progs/test_ringbuf.c | 2 +- > > > tools/testing/selftests/bpf/progs/test_ringbuf_multi.c | 2 +- > > > 6 files changed, 20 insertions(+), 4 deletions(-) > > > > > > > [...]
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 598716742593..100cb2e4c104 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4058,6 +4058,8 @@ union bpf_attr { * Copy *size* bytes from *data* into a ring buffer *ringbuf*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4066,6 +4068,7 @@ union bpf_attr { * void *bpf_ringbuf_reserve(void *ringbuf, u64 size, u64 flags) * Description * Reserve *size* bytes of payload in a ring buffer *ringbuf*. + * *flags* must be 0. * Return * Valid pointer with *size* bytes of memory available; NULL, * otherwise. @@ -4075,6 +4078,8 @@ union bpf_attr { * Submit reserved ring buffer sample, pointed to by *data*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4085,6 +4090,8 @@ union bpf_attr { * Discard reserved ring buffer sample, pointed to by *data*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4965,6 +4972,7 @@ enum { * BPF_FUNC_bpf_ringbuf_output flags. */ enum { + BPF_RB_MAY_WAKEUP = 0, BPF_RB_NO_WAKEUP = (1ULL << 0), BPF_RB_FORCE_WAKEUP = (1ULL << 1), }; diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index ab9f2233607c..3d6d324184c0 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -4058,6 +4058,8 @@ union bpf_attr { * Copy *size* bytes from *data* into a ring buffer *ringbuf*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4066,6 +4068,7 @@ union bpf_attr { * void *bpf_ringbuf_reserve(void *ringbuf, u64 size, u64 flags) * Description * Reserve *size* bytes of payload in a ring buffer *ringbuf*. + * *flags* must be 0. * Return * Valid pointer with *size* bytes of memory available; NULL, * otherwise. @@ -4075,6 +4078,8 @@ union bpf_attr { * Submit reserved ring buffer sample, pointed to by *data*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4085,6 +4090,8 @@ union bpf_attr { * Discard reserved ring buffer sample, pointed to by *data*. * If **BPF_RB_NO_WAKEUP** is specified in *flags*, no notification * of new data availability is sent. + * If **BPF_RB_MAY_WAKEUP** is specified in *flags*, notification + * of new data availability is sent if needed. * If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification * of new data availability is sent unconditionally. * Return @@ -4959,6 +4966,7 @@ enum { * BPF_FUNC_bpf_ringbuf_output flags. */ enum { + BPF_RB_MAY_WAKEUP = 0, BPF_RB_NO_WAKEUP = (1ULL << 0), BPF_RB_FORCE_WAKEUP = (1ULL << 1), }; diff --git a/tools/testing/selftests/bpf/progs/ima.c b/tools/testing/selftests/bpf/progs/ima.c index 96060ff4ffc6..0f4daced6aad 100644 --- a/tools/testing/selftests/bpf/progs/ima.c +++ b/tools/testing/selftests/bpf/progs/ima.c @@ -38,7 +38,7 @@ void BPF_PROG(ima, struct linux_binprm *bprm) return; *sample = ima_hash; - bpf_ringbuf_submit(sample, 0); + bpf_ringbuf_submit(sample, BPF_RB_MAY_WAKEUP); } return; diff --git a/tools/testing/selftests/bpf/progs/ringbuf_bench.c b/tools/testing/selftests/bpf/progs/ringbuf_bench.c index 123607d314d6..808e2e0e3d64 100644 --- a/tools/testing/selftests/bpf/progs/ringbuf_bench.c +++ b/tools/testing/selftests/bpf/progs/ringbuf_bench.c @@ -24,7 +24,7 @@ static __always_inline long get_flags() long sz; if (!wakeup_data_size) - return 0; + return BPF_RB_MAY_WAKEUP; sz = bpf_ringbuf_query(&ringbuf, BPF_RB_AVAIL_DATA); return sz >= wakeup_data_size ? BPF_RB_FORCE_WAKEUP : BPF_RB_NO_WAKEUP; diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf.c b/tools/testing/selftests/bpf/progs/test_ringbuf.c index 8ba9959b036b..03a5cbd21356 100644 --- a/tools/testing/selftests/bpf/progs/test_ringbuf.c +++ b/tools/testing/selftests/bpf/progs/test_ringbuf.c @@ -21,7 +21,7 @@ struct { /* inputs */ int pid = 0; long value = 0; -long flags = 0; +long flags = BPF_RB_MAY_WAKEUP; /* outputs */ long total = 0; diff --git a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c index edf3b6953533..f33c3fdfb1d6 100644 --- a/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c +++ b/tools/testing/selftests/bpf/progs/test_ringbuf_multi.c @@ -71,7 +71,7 @@ int test_ringbuf(void *ctx) sample->seq = total; total += 1; - bpf_ringbuf_submit(sample, 0); + bpf_ringbuf_submit(sample, BPF_RB_MAY_WAKEUP); return 0; }
The current way to provide a no-op flag to 'bpf_ringbuf_submit()', 'bpf_ringbuf_discard()' and 'bpf_ringbuf_output()' is to provide a '0' value. A '0' value might notify the consumer if it already caught up in processing, so let's provide a more descriptive notation for this value. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- include/uapi/linux/bpf.h | 8 ++++++++ tools/include/uapi/linux/bpf.h | 8 ++++++++ tools/testing/selftests/bpf/progs/ima.c | 2 +- tools/testing/selftests/bpf/progs/ringbuf_bench.c | 2 +- tools/testing/selftests/bpf/progs/test_ringbuf.c | 2 +- tools/testing/selftests/bpf/progs/test_ringbuf_multi.c | 2 +- 6 files changed, 20 insertions(+), 4 deletions(-)