Re: [Bpf] [PATCH bpf-next v4] bpf, docs: Add extended call instructions

[David Vernet <void@manifault.com>]

On Sat, Mar 25, 2023 at 10:23:14AM +0000, Dave Thaler wrote:
> From: Dave Thaler <dthaler@microsoft.com>
> 
> Add extended call instructions.  Uses the term "program-local" for
> call by offset.  And there are instructions for calling helper functions
> by "address" (the old way of using integer values), and for calling
> helper functions by BTF ID (for kfuncs).
> 
> ---
> V1 -> V2: addressed comments from David Vernet
> 
> V2 -> V3: make descriptions in table consistent with updated names
> 
> V3 -> V4: addressed comments from Alexei
> 
> Signed-off-by: Dave Thaler <dthaler@microsoft.com>

Looks great, thanks for driving this to consensus. Just left one nit
below, but here's my stamp regardless:

Acked-by: David Vernet <void@manifault.com>

> ---
>  Documentation/bpf/instruction-set.rst | 59 +++++++++++++++++----------
>  1 file changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst
> index 5e43e14abe8..ed8f35becb2 100644
> --- a/Documentation/bpf/instruction-set.rst
> +++ b/Documentation/bpf/instruction-set.rst
> @@ -242,35 +242,50 @@ 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 by address		   see `Helper functions`_

nit: Seems like the alignment got messed up here?

> +BPF_CALL  0x8    0x1  call PC += offset                            see `Program-local functions`_
> +BPF_CALL  0x8    0x2  call helper function by BTF ID               see `Helper functions`_
> +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.
> +``BPF_EXIT``.
>  
>  Helper functions
>  ~~~~~~~~~~~~~~~~
>  
>  Helper functions are a concept whereby BPF programs can call into a
> -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.
> +set of function calls exposed by the underlying platform.
> +
> +Historically, each helper function was identified by an address
> +encoded in the imm field.  The available helper functions may differ
> +for each program type, but address values are unique across all program types.
> +
> +Platforms that support the BPF Type Format (BTF) support identifying
> +a helper function by a BTF ID encoded in the imm field, where the BTF ID
> +identifies the helper name and type.
> +
> +Program-local functions
> +~~~~~~~~~~~~~~~~~~~~~~~
> +Program-local functions are functions exposed by the same BPF program as the
> +caller, and are referenced by offset from the call instruction, similar to
> +``BPF_JA``.  A ``BPF_EXIT`` within the program-local function will return to
> +the caller.
>  
>  Load and store instructions
>  ===========================
> -- 
> 2.33.4
> 
> -- 
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf