Re: [PATCH bpf-next] libbpf: fix bail out from 'ringbuf_process_ring()' on error

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Pedro Tammela <pctammela@gmail.com>
Cc: Pedro Tammela <pctammela@mojatatu.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>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH bpf-next] libbpf: fix bail out from 'ringbuf_process_ring()' on error
Date: Thu, 25 Mar 2021 21:31:39 -0700	[thread overview]
Message-ID: <CAEf4Bzby2eo3-s86rgEjOESrQdemBjYsfLCv=WPh0UHTOZQ7Tw@mail.gmail.com> (raw)
In-Reply-To: <20210325150115.138750-1-pctammela@mojatatu.com>

On Thu, Mar 25, 2021 at 8:02 AM Pedro Tammela <pctammela@gmail.com> wrote:
>
> The current code bails out with negative and positive returns.
> If the callback returns a positive return code, 'ring_buffer__consume()'
> and 'ring_buffer__poll()' will return a spurious number of records
> consumed, but mostly important will continue the processing loop.
>
> This patch makes positive returns from the callback a no-op.
>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>  tools/lib/bpf/ringbuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks. Applied to bpf tree and added:

Fixes: bf99c936f947 ("libbpf: Add BPF ring buffer support")

> diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
> index 8caaafe7e312..e7a8d847161f 100644
> --- a/tools/lib/bpf/ringbuf.c
> +++ b/tools/lib/bpf/ringbuf.c
> @@ -227,7 +227,7 @@ static int ringbuf_process_ring(struct ring* r)
>                         if ((len & BPF_RINGBUF_DISCARD_BIT) == 0) {
>                                 sample = (void *)len_ptr + BPF_RINGBUF_HDR_SZ;
>                                 err = r->sample_cb(r->ctx, sample, len);
> -                               if (err) {
> +                               if (err < 0) {
>                                         /* update consumer pos and bail out */
>                                         smp_store_release(r->consumer_pos,
>                                                           cons_pos);
> --
> 2.25.1
>