Re: [Bpf] [PATCH] bpf, docs: Document BPF insn encoding in term of stored bytes

[David Vernet <void@manifault.com>]

On Fri, Feb 24, 2023 at 09:04:07PM +0100, Jose E. Marchesi wrote:
> 
> This patch modifies instruction-set.rst so it documents the encoding
> of BPF instructions in terms of how the bytes are stored (be it in an
> ELF file or as bytes in a memory buffer to be loaded into the kernel
> or some other BPF consumer) as opposed to how the instruction looks
> like once loaded.
> 
> This is hopefully easier to understand by implementors looking to
> generate and/or consume bytes conforming BPF instructions.
> 
> The patch also clarifies that the unused bytes in a pseudo-instruction
> shall be cleared with zeros.
> 
> Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>

Hi Jose,

Thanks for writing this up.

> ---
>  Documentation/bpf/instruction-set.rst | 43 +++++++++++++--------------
>  1 file changed, 21 insertions(+), 22 deletions(-)
> 
> diff --git a/Documentation/bpf/instruction-set.rst b/Documentation/bpf/instruction-set.rst
> index 01802ed9b29b..9b28c0e15bb6 100644
> --- a/Documentation/bpf/instruction-set.rst
> +++ b/Documentation/bpf/instruction-set.rst
> @@ -38,15 +38,13 @@ eBPF has two instruction encodings:
>  * the wide instruction encoding, which appends a second 64-bit immediate (i.e.,
>    constant) value after the basic instruction for a total of 128 bits.
>  
> -The basic instruction encoding looks as follows for a little-endian processor,
> -where MSB and LSB mean the most significant bits and least significant bits,
> -respectively:
> +The fields conforming an encoded basic instruction are stored in the
> +following order:
>  
> -=============  =======  =======  =======  ============
> -32 bits (MSB)  16 bits  4 bits   4 bits   8 bits (LSB)
> -=============  =======  =======  =======  ============
> -imm            offset   src_reg  dst_reg  opcode
> -=============  =======  =======  =======  ============
> +  opcode:8 src:4 dst:4 offset:16 imm:32 // In little-endian BPF.
> +  opcode:8 dst:4 src:4 offset:16 imm:32 // In big-endian BPF.

Unfortunately this won't render correctly. It'll look something like
this:

The fields conforming an encoded basic instruction are stored in the following order:

opcode:8 src:4 dst:4 offset:16 imm:32 // In little-endian BPF. opcode:8 dst:4 src:4 offset:16 imm:32 // In big-endian BPF.

You'll have to add some extra newlines. You can test out your changes
with:

make SPHINXDIRS=bpf htmldocs

And then the output is put in Documentation/output/bpf

In general, this is sort of the problem we have with rst. We want to
strike a balance between readable in a text editor, and readable when
rendered in a web browser. I think we can strike such a balance here,
but it'll probably require a bit of rst-fu. As described below, I think
we can fix this with a literal code block by just adding a : to order:

> +The fields conforming an encoded basic instruction are stored in the
> +following order::


> +
> +Where,
>  
>  **imm**
>    signed integer immediate value
> @@ -64,16 +62,17 @@ imm            offset   src_reg  dst_reg  opcode
>  **opcode**
>    operation to perform
>  
> -and as follows for a big-endian processor:
> +Note that the contents of multi-byte fields ('imm' and 'offset') are
> +stored using big-endian byte ordering in big-endian BPF and
> +little-endian byte ordering in little-endian BPF.
>  
> -=============  =======  =======  =======  ============
> -32 bits (MSB)  16 bits  4 bits   4 bits   8 bits (LSB)
> -=============  =======  =======  =======  ============
> -imm            offset   dst_reg  src_reg  opcode
> -=============  =======  =======  =======  ============
> +For example:
>  
> -Multi-byte fields ('imm' and 'offset') are similarly stored in
> -the byte order of the processor.
> +  opcode         offset imm          assembly
> +         src dst
> +  07     0   1   00 00  44 33 22 11  r1 += 0x11223344 // little
> +         dst src
> +  07     1   0   00 00  11 22 33 44  r1 += 0x11223344 // big

This also won't render. rst will think it's a "definition list" (see
[0]), so it's interpreting the line with '// big' as a term that will be
defined on the next line.

[0]: https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#lists-and-quote-like-blocks

If you do "For example::" it should render correctly. This applies
elsewhere to the patch. Let's just make all of these literal code
blocks.

Thanks,
David

>  
>  Note that most instructions do not use all of the fields.
>  Unused fields shall be cleared to zero.
> @@ -84,18 +83,18 @@ The 64 bits following the basic instruction contain a pseudo instruction
>  using the same format but with opcode, dst_reg, src_reg, and offset all set to zero,
>  and imm containing the high 32 bits of the immediate value.
>  
> -=================  ==================
> -64 bits (MSB)      64 bits (LSB)
> -=================  ==================
> -basic instruction  pseudo instruction
> -=================  ==================
> +This is depicted in the following figure:
> +
> +  basic_instruction                 pseudo_instruction
> +  code:8 regs:16 offset:16 imm:32 | unused:32 imm:32
>  
>  Thus the 64-bit immediate value is constructed as follows:
>  
>    imm64 = (next_imm << 32) | imm
>  
>  where 'next_imm' refers to the imm value of the pseudo instruction
> -following the basic instruction.
> +following the basic instruction.  The unused bytes in the pseudo
> +instruction shall be cleared to zero.
>  
>  Instruction classes
>  -------------------
> -- 
> 2.30.2
> 
> -- 
> Bpf mailing list
> Bpf@ietf.org
> https://www.ietf.org/mailman/listinfo/bpf