Re: [Bpf] [PATCH bpf-next v4] bpf, docs: Add callx instructions in new conformance group

[David Vernet <void@manifault.com>]

[-- Attachment #1: Type: text/plain, Size: 5709 bytes --]

On Wed, Feb 21, 2024 at 11:17:25AM -0800, Dave Thaler wrote:
> * Add a "callx" conformance group
> * Add callx row to table
> * Update helper function to section to be agnostic between BPF_K vs
>   BPF_X
> * Rename "legacy" conformance group to "packet"
> 
> Based on mailing list discussion at
> https://mailarchive.ietf.org/arch/msg/bpf/l5tNEgL-Wo7qSEuaGssOl5VChKk/
> 
> Only src=0 is currently listed for callx. Neither clang nor gcc
> use src=1 or src=2, and both use exactly the same semantics for
> src=0 which was agreed between them (Yonghong and Jose). Since src=0
> semantics are agreed upon by both and is already implemented, src=0
> is documented as implemented.

If the semantics for src=0 were already decided for both clang and gcc,
then this seems fine to me. Agreed as well with leaving src > 0 for
later, as Alexei said on the v3 thread. We can decide how to best deal
with indirect calls at a later time.

Alexei -- is this acceptable?

> v1->v2: Incorporated feedback from Will Hawkins
> 
> v2->v3: Use "dst" not "imm" field
> 
> v3->v4: Only use src=0
> 
> Signed-off-by: Dave Thaler <dthaler1968@gmail.com>
> ---
>  .../bpf/standardization/instruction-set.rst   | 29 ++++++++++++-------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> index bdfe0cd0e..a68445899 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -127,7 +127,7 @@ This document defines the following conformance groups:
>  * divmul32: includes 32-bit division, multiplication, and modulo instructions.
>  * divmul64: includes divmul32, plus 64-bit division, multiplication,
>    and modulo instructions.
> -* legacy: deprecated packet access instructions.
> +* packet: deprecated packet access instructions.
>  
>  Instruction encoding
>  ====================
> @@ -404,9 +404,10 @@ 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  BPF_JMP | BPF_K only, see `Helper functions`_
> +BPF_CALL  0x8    0x0  call_by_address(imm)             BPF_JMP | BPF_K only
> +BPF_CALL  0x8    0x0  call_by_address(dst)             BPF_JMP | BPF_X only
>  BPF_CALL  0x8    0x1  call PC += imm                   BPF_JMP | BPF_K only, see `Program-local functions`_
> -BPF_CALL  0x8    0x2  call helper function by BTF ID   BPF_JMP | BPF_K only, see `Helper functions`_
> +BPF_CALL  0x8    0x2  call_by_btfid(imm)               BPF_JMP | BPF_K only
>  BPF_EXIT  0x9    0x0  return                           BPF_JMP | BPF_K only
>  BPF_JLT   0xa    any  PC += offset if dst < src        unsigned
>  BPF_JLE   0xb    any  PC += offset if dst <= src       unsigned
> @@ -414,6 +415,11 @@ BPF_JSLT  0xc    any  PC += offset if dst < src        signed
>  BPF_JSLE  0xd    any  PC += offset if dst <= src       signed
>  ========  =====  ===  ===============================  =============================================
>  
> +where
> +
> +* call_by_address(value) means to call a helper function by the address specified by 'value' (see `Helper functions`_ for details)
> +* call_by_btfid(value) means to call a helper function by the BTF ID specified by 'value' (see `Helper functions`_ for details)
> +
>  The BPF program needs to store the return value into register R0 before doing a
>  ``BPF_EXIT``.
>  
> @@ -438,8 +444,9 @@ specified by the 'imm' field. A > 16-bit conditional jump may be
>  converted to a < 16-bit conditional jump plus a 32-bit unconditional
>  jump.
>  
> -All ``BPF_CALL`` and ``BPF_JA`` instructions belong to the
> -base32 conformance group.
> +The ``BPF_CALL | BPF_X`` instruction belongs to the callx
> +conformance group.  All other ``BPF_CALL`` instructions and all
> +``BPF_JA`` instructions belong to the base32 conformance group.
>  
>  Helper functions
>  ~~~~~~~~~~~~~~~~
> @@ -447,13 +454,13 @@ Helper functions
>  Helper functions are a concept whereby BPF programs can call into a
>  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.
> +Historically, each helper function was identified by an address.
> +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.
> +a helper function by a BTF ID, where the BTF ID identifies the helper
> +name and type.
>  
>  Program-local functions
>  ~~~~~~~~~~~~~~~~~~~~~~~
> @@ -660,4 +667,4 @@ carried over from classic BPF. These instructions used an instruction
>  class of BPF_LD, a size modifier of BPF_W, BPF_H, or BPF_B, and a
>  mode modifier of BPF_ABS or BPF_IND.  However, these instructions are
>  deprecated and should no longer be used.  All legacy packet access
> -instructions belong to the "legacy" conformance group.
> +instructions belong to the "packet" conformance group.
> -- 
> 2.40.1
> 
> -- 
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]