On Wed, Apr 5, 2023 at 10:29 AM Lorenz Bauer <lmb@xxxxxxxxxxxxx> wrote: > > On Tue, Apr 4, 2023 at 5:37 AM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > Drop the log_size>0 and log_buf!=NULL condition when log_level>0. This > > allows users to request log_size_actual of a full log without providing > > actual (even if small) log buffer. Verifier log handling code was mostly ready to handle NULL log->ubuf, so only few small changes were necessary to prevent NULL log->ubuf from causing problems. > > > > Note, that user is basically guaranteed to receive -ENOSPC when > > providing log_level>0 and log_buf==NULL. We also still enforce that > > either (log_buf==NULL && log_size==0) or (log_buf!=NULL && log_size>0). > > 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) > > If the first PROG_LOAD returned ENOSPC we'd have to re-run it without > the log enabled to see whether ENOSPC is masking a real verification > error. With the current semantics we can work around this with three > syscalls, but that seems wasteful? > > if PROG_LOAD(buf=NULL, size=0, level=0) >= 0: > return fd > > PROG_LOAD(buf=NULL, size=0, level=1) == ENOSPC > PROG_LOAD(buf!=NULL, size=log_size_actual, level=1) 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. So I didn't want to encourage this pattern. 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?
