On Mon, May 12, 2025 at 6:53 PM Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> wrote: > > On Mon, 12 May 2025 at 17:42, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Mon, May 12, 2025 at 2:02 PM Kumar Kartikeya Dwivedi > > <memxor@xxxxxxxxx> wrote: > > > > > > Instead of hardcoding the list of kfuncs that need prog->aux passed to > > > them with a combination of fixup_kfunc_call adjustment + __ign suffix, > > > combine both in __aux suffix, which ignores the argument passed in, and > > > fixes it up to the prog->aux. This allows kfuncs to have the prog->aux > > > passed into them without having to touch the verifier. > > > > I have this wet dream that one day we'll make sure that all BPF > > programs have bpf_run_ctx set up for them, across all program types, > > and we'll just use that for these "BPF environment"-like things like > > getting currently-running bpf_prog/bpf_prog_aux pointer. > > > > For this particular patch, it's mostly refactoring. But I agree in > general having bpf_run_ctx everywhere would be useful. > Currently being able to rely on it in some cases and not in others is > a bit annoying. Yep, agreed. > > > Do you think it makes sense? One of the concerns for wiring > > bpf_run_ctx for, say, XDP programs was perceived potential tiny perf > > regression (but it's just current->bpf_ctx swap, so shouldn't be a big > > deal at all, IMO) and just generally no immediate use case. > > I think the performance concern may be real. I can do some > benchmarking with xdp-bench, but it's probably not the raw swaps that > cost too much but potential cache miss on touching current. I am not > sure how problematic that would be in terms of pps regression for > small programs, but even if amortized it may end up regressing when > done unconditionally. Yep, cache line access is the expensive thing, not the swap itself, of course. My hope is that current is frequently accessed enough and we might get lucky with that cache line already being present. Or if XDP calls are frequent enough, we'll just amortize the cost across multiple packets, as you mentioned. But yes, maybe a bit of benchmarking would give us a bit better idea, thanks! > > XDP is sort of an edge case though. I am not sure the concern on > ns-scale effects holds for any other program type. Sure, XDP is an extreme case, but if bpf_run_ctx is everywhere *but* XDP, that still makes it a non-universal mechanism, and we'll have to always remember that some run_ctx-related functionality doesn't work on XDP. Maybe that's fine, I don't know, but it certainly would be better to not have these exceptions. > > > So maybe > > this bpf_prog_aux access is a good enough reason now, WDYT? > > > > > > > > Cc: Tejun Heo <tj@xxxxxxxxxx> > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> > > > --- > > > include/linux/bpf_verifier.h | 1 + > > > kernel/bpf/helpers.c | 4 ++-- > > > kernel/bpf/verifier.c | 33 +++++++++++++++++++++++++++------ > > > 3 files changed, 30 insertions(+), 8 deletions(-) > > > > > > > [...]
