On Thu, Apr 6, 2023 at 12:46 PM Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> wrote: > > 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. makes sense, then no -ENOSPC for log_buf==NULL, will do in next version > > log_true_size should more or less align with verified_insns uapi number, > but still interesting to see and track over time.
