On Mon, Oct 19, 2020 at 5:58 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote: > > On 10/19/20 12:20 PM, Juraj Vijtiuk wrote: > > On Wed, Oct 14, 2020 at 12:05 AM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > >> On Fri, Oct 9, 2020 at 12:58 PM Juraj Vijtiuk <juraj.vijtiuk@xxxxxxxxxx> wrote: > >>> > >>> It would be great to hear if anyone has any thoughts on running a set > >>> of BPF programs JITed while other programs are run by the interpreter. > >>> > >>> Something like that would be useful on 32-bit architectures, as the > >>> JIT compiler there doesn't support some instructions, primarily > >>> instructions that work with 64-bit data. As far as I can tell, it is > >>> unlikely that support will be coming soon as it is a general issue for > >>> all 32-bit architectures. Atomic operations like BPF_XADD look > >>> especially problematic regarding support on 32 bit platforms. From > >>> what I managed to see such a conclusion appeared in a few patches > >>> where support for 32-bit JITs was added, for example [0]. > >>> That results in some programs being runnable with BPF JIT enabled, and > >>> some failing during load time, but running successfully without JIT on > >>> 32-bit platforms. > >>> > >>> The only way to run some programs with JIT and some without, that > >>> seems possible right now, is to manually change > >>> /proc/sys/net/core/bpf_jit_enable every time a program is loaded. > >>> Although I've managed to do that and it seems to be working, it seems > >>> pretty hacky and looks like it could cause race conditions if multiple > >>> programs were loaded, especially by independent loaders. > >> > >> I agree, the global file is not flexible enough and can cause problems > >> in production environment. > >> > >> I don't see any reason why we shouldn't allow to decide interpreted vs > >> jitted mode per program during BPF_PROG_LOAD. > >> > >> See kernel/bpf/core.c, bpf_prog's jit_requested field determines > >> whether a program is going to be jitted or not. It should be trivial > >> to allow overriding that during BPF_PROG_LOAD command. > >> > >> We can probably also generalize this to allow to "force-jit" or > >> "force-interpret" by users, which would fail if kernel didn't support > >> requested mode. > > > > Thanks for the suggestion, that makes sense. I've started working on a > > patch today. > > I'll post again when I get something working and test it. > > Hmm, I'm probably missing some context, but why is it not enough to just set the > bpf_jit_enable to 1, and if 32 bit JITs don't support specific instructions like > BPF_XADD then they should transparently fall back to interpreter if you have > the latter compiled in. That is what it /should/ do today and user loading the > prog shouldn't have to care about it. Juraj, you are suggesting that this is not > happening in your case? Or is the issue tail calls? That wasn't happening last time people reported this on ARM32. BPF_XADD was causing load failure, no fail back to interpreter mode. > > Wrt force-interpret vs force-jit BPF_PROG_LOAD flag, I'm more concerned that this > decision will then be pushed to the user who should not have to care about these > internals. And how would generic loaders try to react if force-jit fails? They would > then fallback to force-interpret same way as kernel does? The way I imagined this was if the user wants to force the mode and the kernel doesn't support it (or the program can't be loaded in that mode), then it's a fail-stop, no fall back. And it's strictly an opt-in flag, if nothing is specified then it's current behavior with fallback (which apparently doesn't always work). > > Wrt BPF_XADD, maybe 32 bit platforms should just implement a function call to the > atomic64_add() internally, it will be slow but otoh the rest can then be JITed, so > most likely this still ends up being faster than using interpreter for everything > anyway. > > Thanks, > Daniel
