Re: Running JITed and interpreted programs simultaneously

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Mon, Oct 19, 2020 at 5:58 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 10/19/20 12:20 PM, Juraj Vijtiuk wrote:
> > On Wed, Oct 14, 2020 at 12:05 AM Andrii Nakryiko
> > <andrii.nakryiko@gmail.com> wrote:
> >> On Fri, Oct 9, 2020 at 12:58 PM Juraj Vijtiuk <juraj.vijtiuk@sartura.hr> 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