Re: [PATCH v3 bpf-next 14/19] bpf: relax log_buf NULL conditions when log_level>0 is requested

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 6, 2023 at 11:43 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Thu, Apr 6, 2023 at 9:15 AM Lorenz Bauer <lmb@xxxxxxxxxxxxx> wrote:
> >
> > On Wed, Apr 5, 2023 at 6:44 PM Andrii Nakryiko
> > <andrii.nakryiko@xxxxxxxxx> wrote:>
> > > We could and I thought about this, but verifier logging is quite an
> > > expensive operation due to all the extra formatting and reporting, so
> > > it's not advised to have log_level=1 permanently enabled for
> > > production BPF program loading.
> >
> > Any idea how expensive this is?
> >
>
> It will depend on the specific program, of course. But just to
> estimate, here's pyperf100 selftests with log_level=1 and log_level=4
> (just stats, basically free).
>
> [vmuser@archvm bpf]$ time sudo ./veristat pyperf100.bpf.o -v >/dev/null
>
> real    0m1.761s
> user    0m0.008s
> sys     0m1.743s
> [vmuser@archvm bpf]$ time sudo ./veristat pyperf100.bpf.o >/dev/null
>
> real    0m0.869s
> user    0m0.009s
> sys     0m0.846s
>
> 2x difference. So I'd definitely not recommend running with
> log_level=1 by default.
>
> > > Note that even if log_buf==NULL when log_level>0 we'd be
> > > doing printf()-ing everything, which is the expensive part. So do you
> > > really want to add all this extra overhead *constantly* to all
> > > production BPF programs?
> >
> > No, I'm just going off of what UAPI I would like to use. Keeping
> > semantics as they are is fine if it's too slow. We could always use a
> > small-ish buffer for the first retry and hope things fit.
>
> It's easy for me to implement it either way, Alexei and Daniel, any
> thoughts on this?

I like this part of the idea:

> Is it possible to change it so that log_buf == NULL && log_size == 0
> && log_level > 0 only fills in log_size_actual and doesn't return
> ENOSPC? Otherwise we can't do oneshot loading.
>  if PROG_LOAD(buf=NULL, size=0, level=1) >= 0:
>    return fd
>  else
>    retry PROG_LOAD(buf!=NULL, size=log_size_actual, level=1)

libbpf shouldn't be doing it by default, since load time matters,
but it could be useful in other scenarios.
Like central bpf loading daemon can use (buf=NULL, size=0, level=(1 | 4))
every 100th prog_load as part of stats collection.
veristat can do it by default.

log_true_size should more or less align with verified_insns uapi number,
but still interesting to see and track over time.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux