On Fri, Mar 10, 2023 at 11:21:44PM +0000, Dave Thaler wrote: > From: Dave Thaler <dthaler@xxxxxxxxxxxxx> > > Add extended call instructions. Since BPF can be used in userland > or SmartNICs, this uses the more generic "runtime functions" > rather than the kernel specific "kfuncs" as a term. > > Signed-off-by: Dave Thaler <dthaler@xxxxxxxxxxxxx> Hi Dave, Thanks for sending out the patch. It's a nice improvement to the doc to disambiguate these instruction call types. Left a few comments below. > --- > Documentation/bpf/instruction-set.rst | 50 +++++++++++++++++---------- > 1 file changed, 32 insertions(+), 18 deletions(-) > > diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst > index 5e43e14abe8..bc2319a7707 100644 > --- a/Documentation/bpf/instruction-set.rst > +++ b/Documentation/bpf/instruction-set.rst > @@ -242,24 +242,26 @@ Jump instructions > otherwise identical operations. > The 'code' field encodes the operation as below: > > -======== ===== ========================= ============ > -code value description notes > -======== ===== ========================= ============ > -BPF_JA 0x00 PC += off BPF_JMP only > -BPF_JEQ 0x10 PC += off if dst == src > -BPF_JGT 0x20 PC += off if dst > src unsigned > -BPF_JGE 0x30 PC += off if dst >= src unsigned > -BPF_JSET 0x40 PC += off if dst & src > -BPF_JNE 0x50 PC += off if dst != src > -BPF_JSGT 0x60 PC += off if dst > src signed > -BPF_JSGE 0x70 PC += off if dst >= src signed > -BPF_CALL 0x80 function call see `Helper functions`_ > -BPF_EXIT 0x90 function / program return BPF_JMP only > -BPF_JLT 0xa0 PC += off if dst < src unsigned > -BPF_JLE 0xb0 PC += off if dst <= src unsigned > -BPF_JSLT 0xc0 PC += off if dst < src signed > -BPF_JSLE 0xd0 PC += off if dst <= src signed > -======== ===== ========================= ============ > +======== ===== === ========================== ======================== > +code value src description notes > +======== ===== === ========================== ======================== > +BPF_JA 0x0 0x0 PC += offset BPF_JMP only > +BPF_JEQ 0x1 any PC += offset if dst == src > +BPF_JGT 0x2 any PC += offset if dst > src unsigned > +BPF_JGE 0x3 any PC += offset if dst >= src unsigned > +BPF_JSET 0x4 any PC += offset if dst & src > +BPF_JNE 0x5 any PC += offset if dst != src > +BPF_JSGT 0x6 any PC += offset if dst > src signed > +BPF_JSGE 0x7 any PC += offset if dst >= src signed > +BPF_CALL 0x8 0x0 call helper function imm see `Helper functions`_ > +BPF_CALL 0x8 0x1 call PC += offset see `eBPF functions`_ > +BPF_CALL 0x8 0x2 call runtime function imm see `Runtime functions`_ The names "Helper functions", "eBPF functions", and "Runtime functions" feel, for lack of a better term, insufficiently distinct. I realize that it's very tricky to get the naming right here given that some of these terms (helpers + runtime functions) are conceptually the same thing, but as a reader with no background I expect I'd be confused by what the distinctions are as they all kind of sound like the same thing. What do you think of this as an alternative: 1. Standard helper functions 2. BPF-local functions 3. Platform-specific helper functions The idea with the latter is of course that the platform can choose to implement whatever helper functions (kfuncs for Linux) apply exclusively to that platform. I think we'd need some formal encoding for this in the standard, so it seems appropriate to apply it here. What do you think? > +BPF_EXIT 0x9 0x0 return BPF_JMP only > +BPF_JLT 0xa any PC += offset if dst < src unsigned > +BPF_JLE 0xb any PC += offset if dst <= src unsigned > +BPF_JSLT 0xc any PC += offset if dst < src signed > +BPF_JSLE 0xd any PC += offset if dst <= src signed > +======== ===== === ========================== ======================== > > The eBPF program needs to store the return value into register R0 before doing a > BPF_EXIT. > @@ -272,6 +274,18 @@ set of function calls exposed by the runtime. Each helper > function is identified by an integer used in a ``BPF_CALL`` instruction. > The available helper functions may differ for each program type. > > +Runtime functions > +~~~~~~~~~~~~~~~~~ > +Runtime functions are like helper functions except that they are not specific > +to eBPF programs. They use a different numbering space from helper functions, Per my suggestion above, should we rephrase this as "platform-specific" helper functions? E.g. something like: Platform-specific helper functions are helper functions that may be unique to a particular platform. An encoding for a platform-specific function on one platform may or may not correspond to the same function on another platform. Platforms are not required to implement any platform-specific function. As alluded to above, the fact that they're not specific to BPF seems like an implementation detail from the perspective of the encoding / standard. Thoughts? > +but otherwise the same considerations apply. > + > +eBPF functions > +~~~~~~~~~~~~~~ > +eBPF functions are functions exposed by the same eBPF program as the caller, > +and are referenced by offset from the call instruction, similar to ``BPF_JA``. > +A ``BPF_EXIT`` within the eBPF function will return to the caller. Suggestion: Can we simply say BPF instead of eBPF? At this point I'm not sure what the 'e' distinction really buys us, though I'm sure I'm missing context from (many) prior discussions. I also don't want to bikeshed too much on this point for your patch, so if it becomes a "thing" then feel free to ignore. Thanks, David
