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.