On Sat, Mar 11, 2023 at 03:53:47PM -0600, David Vernet wrote: > On Sat, Mar 11, 2023 at 09:00:19PM +0000, Dave Thaler wrote: > > David Vernet <void@xxxxxxxxxxxxx> wrote: > > [...] > > > > +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 > > > > I like where you're going with this. However, the fact is that some of the existing > > Helper functions are actually very platform-specific and won't apply to other > > platforms. So retroactively labeling all of them "standard" is somewhat problematic > > in my view. > > That makes sense. For what it's worth, I was envisioning us specifying > both the helper number (and likely the semantics of those helpers) in > the standard, and just skipping over any which are Linux-specific. > That's of course assuming we do decide to include functions in the > standard, which to my understanding is not yet finalized. > > Regardless, I'll certainly defer to your expertise on when it's > appropriate to use the word "standard", and I could see why it would be > problematic to do so here. > > > > > I do like the idea of using "<some adjective> helper functions" for both 1 and 3 > > though. Since we might not choose to standardize all the existing type 1 functions, > > maybe "Platform-agnostic helper functions" is synonymous and pairs nicely > > With "Platform-specific helper functions" as a term. And then we could just have > > a note in the linux-notes.rst saying Linux has some platform-specific helper functions that for historical reasons are used with the platform-agnostic helper function > > Instruction. > > That's a reasonable option as well. The only thing that gives me pause > is that, as you know, the plan of record for now in Linux is to have all > new BPF -> main kernel functions added as kfuncs. That includes features > which are "platform agnostic", such as BPF iterators. I know you've > previously raised the idea of having the traditional helpers be used as > standard / platform-agnostic helpers in BPF office hours, so this isn't > out of the blue. It seems that the time has come to discuss it more > concretely. One thing to clarify -- I'm _not_ saying we should revisit the kfunc vs. BPF helper discussion. Rather, just that we should decide exactly what the older BPF helper instruction encoding means in terms of a generic BPF instruction set. > > > > > 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? > > > > Agree with that. > > > > > > +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. > > > > That looks good to me, will incorporate. > > > > > > > > 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. > > > > Will remove for consistency with the other patches I submitted that already > > omitted the "e". I think Alexei had the same comment a while back and > > I missed updating this proposed section at the time. Thanks. > > > > Dave > > > > > > > > Thanks, > > > David
