[bpf-next] bpf, docs: Fix typos in instruction-set.rst

Commit Message

Dave Thaler Feb. 21, 2024, 5:35 p.m. UTC

* "BPF ADD" should be "BPF_ADD".
* "src" should be "src_reg" in several places.  The latter is the field name
  in the instruction.  The former refers to the value of the register, or the
  immediate.
* Add '' around field names in one sentence, for consistency with the rest
  of the document.

Signed-off-by: Dave Thaler <[e-mail]>
---
 .../bpf/standardization/instruction-set.rst   | 72 +++++++++----------
 1 file changed, 36 insertions(+), 36 deletions(-)

Comments

David Vernet Feb. 21, 2024, 6:04 p.m. UTC | #1

On Wed, Feb 21, 2024 at 09:35:35AM -0800, Dave Thaler wrote:
> * "BPF ADD" should be "BPF_ADD".
> * "src" should be "src_reg" in several places.  The latter is the field name
>   in the instruction.  The former refers to the value of the register, or the
>   immediate.
> * Add '' around field names in one sentence, for consistency with the rest
>   of the document.
> 
> Signed-off-by: Dave Thaler <[e-mail]>

Thanks for the cleanup.

Acked-by: David Vernet <[e-mail]>

> ---
>  .../bpf/standardization/instruction-set.rst   | 72 +++++++++----------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> index 868d9f617..56b5e7dad 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -178,7 +178,7 @@ Unused fields shall be cleared to zero.
>  As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
>  instruction uses two 32-bit immediate values that are constructed as follows.
>  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,
> +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.

nit: Can we make sure these columns are all wrapped to 80 characters?
This can be done in a follow-up for the whole document later.

Alexei Starovoitov Feb. 22, 2024, 5:09 p.m. UTC | #2

On Wed, Feb 21, 2024 at 10:05 AM David Vernet <[e-mail]> wrote:
>
> On Wed, Feb 21, 2024 at 09:35:35AM -0800, Dave Thaler wrote:
> > * "BPF ADD" should be "BPF_ADD".
> > * "src" should be "src_reg" in several places.  The latter is the field name
> >   in the instruction.  The former refers to the value of the register, or the
> >   immediate.
> > * Add '' around field names in one sentence, for consistency with the rest
> >   of the document.
> >
> > Signed-off-by: Dave Thaler <[e-mail]>
>
> Thanks for the cleanup.
>
> Acked-by: David Vernet <[e-mail]>
>
> > ---
> >  .../bpf/standardization/instruction-set.rst   | 72 +++++++++----------
> >  1 file changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> > index 868d9f617..56b5e7dad 100644
> > --- a/Documentation/bpf/standardization/instruction-set.rst
> > +++ b/Documentation/bpf/standardization/instruction-set.rst
> > @@ -178,7 +178,7 @@ Unused fields shall be cleared to zero.
> >  As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
> >  instruction uses two 32-bit immediate values that are constructed as follows.
> >  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,
> > +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.
>
> nit: Can we make sure these columns are all wrapped to 80 characters?
> This can be done in a follow-up for the whole document later.

Fixed up while applying,
but let's not reformat the whole doc.
Many tables are 100+ chars and it's fine.

[e-mail] Feb. 22, 2024, 5:10 p.m. UTC | #3

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Wed, 21 Feb 2024 09:35:35 -0800 you wrote:
> * "BPF ADD" should be "BPF_ADD".
> * "src" should be "src_reg" in several places.  The latter is the field name
>   in the instruction.  The former refers to the value of the register, or the
>   immediate.
> * Add '' around field names in one sentence, for consistency with the rest
>   of the document.
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf, docs: Fix typos in instruction-set.rst
    https://git.kernel.org/bpf/bpf-next/c/c1bb68f6b2f6

You are awesome, thank you!

Patch

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index 868d9f617..56b5e7dad 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -178,7 +178,7 @@  Unused fields shall be cleared to zero.
 As discussed below in `64-bit immediate instructions`_, a 64-bit immediate
 instruction uses two 32-bit immediate values that are constructed as follows.
 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,
+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.
 
 This is depicted in the following figure::
@@ -392,27 +392,27 @@  otherwise identical operations, and indicates the base64 conformance
 group unless otherwise specified.
 The 'code' field encodes the operation as below:
 
-========  =====  ===  ===============================  =============================================
-code      value  src  description                      notes
-========  =====  ===  ===============================  =============================================
-BPF_JA    0x0    0x0  PC += offset                     BPF_JMP | BPF_K only
-BPF_JA    0x0    0x0  PC += imm                        BPF_JMP32 | BPF_K 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  BPF_JMP | BPF_K only, see `Helper functions`_
-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_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
-BPF_JSLT  0xc    any  PC += offset if dst < src        signed
-BPF_JSLE  0xd    any  PC += offset if dst <= src       signed
-========  =====  ===  ===============================  =============================================
+========  =====  =======  ===============================  =============================================
+code      value  src_reg  description                      notes
+========  =====  =======  ===============================  =============================================
+BPF_JA    0x0    0x0      PC += offset                     BPF_JMP | BPF_K only
+BPF_JA    0x0    0x0      PC += imm                        BPF_JMP32 | BPF_K 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  BPF_JMP | BPF_K only, see `Helper functions`_
+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_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
+BPF_JSLT  0xc    any      PC += offset if dst < src        signed
+BPF_JSLE  0xd    any      PC += offset if dst <= src       signed
+========  =====  =======  ===============================  =============================================
 
 The BPF program needs to store the return value into register R0 before doing a
 ``BPF_EXIT``.
@@ -568,7 +568,7 @@  BPF_XOR   0xa0   atomic xor
 
   *(u32 *)(dst + offset) += src
 
-``BPF_ATOMIC | BPF_DW | BPF_STX`` with 'imm' = BPF ADD means::
+``BPF_ATOMIC | BPF_DW | BPF_STX`` with 'imm' = BPF_ADD means::
 
   *(u64 *)(dst + offset) += src
 
@@ -601,24 +601,24 @@  and loaded back to ``R0``.
 -----------------------------
 
 Instructions with the ``BPF_IMM`` 'mode' modifier use the wide instruction
-encoding defined in `Instruction encoding`_, and use the 'src' field of the
+encoding defined in `Instruction encoding`_, and use the 'src_reg' field of the
 basic instruction to hold an opcode subtype.
 
 The following table defines a set of ``BPF_IMM | BPF_DW | BPF_LD`` instructions
-with opcode subtypes in the 'src' field, using new terms such as "map"
+with opcode subtypes in the 'src_reg' field, using new terms such as "map"
 defined further below:
 
-=========================  ======  ===  =========================================  ===========  ==============
-opcode construction        opcode  src  pseudocode                                 imm type     dst type
-=========================  ======  ===  =========================================  ===========  ==============
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x0  dst = (next_imm << 32) | imm               integer      integer
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x1  dst = map_by_fd(imm)                       map fd       map
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x2  dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x3  dst = var_addr(imm)                        variable id  data pointer
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x4  dst = code_addr(imm)                       integer      code pointer
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x5  dst = map_by_idx(imm)                      map index    map
-BPF_IMM | BPF_DW | BPF_LD  0x18    0x6  dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
-=========================  ======  ===  =========================================  ===========  ==============
+=========================  ======  =======  =========================================  ===========  ==============
+opcode construction        opcode  src_reg  pseudocode                                 imm type     dst type
+=========================  ======  =======  =========================================  ===========  ==============
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x0      dst = (next_imm << 32) | imm               integer      integer
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x1      dst = map_by_fd(imm)                       map fd       map
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x2      dst = map_val(map_by_fd(imm)) + next_imm   map fd       data pointer
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x3      dst = var_addr(imm)                        variable id  data pointer
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x4      dst = code_addr(imm)                       integer      code pointer
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x5      dst = map_by_idx(imm)                      map index    map
+BPF_IMM | BPF_DW | BPF_LD  0x18    0x6      dst = map_val(map_by_idx(imm)) + next_imm  map index    data pointer
+=========================  ======  =======  =========================================  ===========  ==============
 
 where