Re: [PATCH bpf-next] libbpf: clarify flags in ringbuf helpers

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Joe Stringer <joe@cilium.io>
Cc: Pedro Tammela <pctammela@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Quentin Monnet <quentin@isovalent.com>,
	"open list:BPF (Safe dynamic programs and tools)" 
	<netdev@vger.kernel.org>,
	"open list:BPF (Safe dynamic programs and tools)" 
	<bpf@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>,
	Pedro Tammela <pctammela@mojatatu.com>
Subject: Re: [PATCH bpf-next] libbpf: clarify flags in ringbuf helpers
Date: Wed, 7 Apr 2021 12:58:44 -0700	[thread overview]
Message-ID: <CAEf4BzaBJH-=iO-P6ZTj3zmycz0VESzBzpZkbVOVTvPaZ9OEaA@mail.gmail.com> (raw)
In-Reply-To: <CAOftzPgmZSB7oWDLLoO-NEDq3s8LdLxSXdhoaB2feScuTP-JSA@mail.gmail.com>

On Wed, Apr 7, 2021 at 11:43 AM Joe Stringer <joe@cilium.io> wrote:
>
> Hi Pedro,
>
> On Tue, Apr 6, 2021 at 11:58 AM Pedro Tammela <pctammela@gmail.com> wrote:
> >
> > In 'bpf_ringbuf_reserve()' we require the flag to '0' at the moment.
> >
> > For 'bpf_ringbuf_{discard,submit,output}' a flag of '0' might send a
> > notification to the process if needed.
> >
> > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> > ---
> >  include/uapi/linux/bpf.h       | 7 +++++++
> >  tools/include/uapi/linux/bpf.h | 7 +++++++
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 49371eba98ba..8c5c7a893b87 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -4061,12 +4061,15 @@ union bpf_attr {
> >   *             of new data availability is sent.
> >   *             If **BPF_RB_FORCE_WAKEUP** is specified in *flags*, notification
> >   *             of new data availability is sent unconditionally.
> > + *             If **0** is specified in *flags*, notification
> > + *             of new data availability is sent if needed.
>
> Maybe a trivial question, but what does "if needed" mean? Does that
> mean "when the buffer is full"?

I used to call it ns "adaptive notification", so maybe let's use that
term instead of "if needed"? It means that in kernel BPF ringbuf code
will check if the user-space consumer has caught up and consumed all
the available data. In that case user-space might be waiting
(sleeping) in epoll_wait() already and not processing samples
actively. That means that we have to send notification, otherwise
user-space might never wake up. But if the kernel sees that user-space
is still processing previous record (consumer position < producer
position), then we can bypass sending another notification, because
user-space consumer protocol dictates that it needs to consume all the
record until consumer position == producer position. So no
notification is necessary for the newly submitted sample, as
user-space will eventually see it without notification.

Of course there is careful writes and memory ordering involved to make
sure that we never miss notification.

Does someone want to try to condense it into a succinct description? ;)