[PATCH] update the consistency issue in documentation

[PATCH] update the consistency issue in documentation

[Aoyang Fang (SSE, 222010547)]

From fa9f3f47ddeb3e9a615c17aea57d2ecd53a7d201 Mon Sep 17 00:00:00 2001
From: lincyawer <53161583+Lincyaw@users.noreply.github.com>
Date: Thu, 4 Jan 2024 10:51:36 +0800
Subject: [PATCH] The original documentation of BPF_JMP instructions is somehow
misleading. The code part of instruction, e.g., BPF_JEQ's value is noted as
0x1, however, in `include/uapi/linux/bpf.h`, the value of BPF_JEQ is 0x10. At
the same time, the description convention is inconsistent with the BPF_ALU,
whose code are also 4bit, but the value of BPF_ADD is 0x00

Signed-off-by: lincyawer <53161583+Lincyaw@users.noreply.github.com>
---
.../bpf/standardization/instruction-set.rst | 34 +++++++++----------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index 245b6defc..dee3b1fa8 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -355,23 +355,23 @@ The 'code' field encodes the operation as below:
======== ===== === =========================================== =========================================
code value src description notes
======== ===== === =========================================== =========================================
-BPF_JA 0x0 0x0 PC += offset BPF_JMP class
-BPF_JA 0x0 0x0 PC += imm BPF_JMP32 class
-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`_
-BPF_CALL 0x8 0x1 call PC += imm 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
+BPF_JA 0x00 0x0 PC += offset BPF_JMP class
+BPF_JA 0x00 0x0 PC += imm BPF_JMP32 class
+BPF_JEQ 0x10 any PC += offset if dst == src
+BPF_JGT 0x20 any PC += offset if dst > src unsigned
+BPF_JGE 0x30 any PC += offset if dst >= src unsigned
+BPF_JSET 0x40 any PC += offset if dst & src
+BPF_JNE 0x50 any PC += offset if dst != src
+BPF_JSGT 0x60 any PC += offset if dst > src signed
+BPF_JSGE 0x70 any PC += offset if dst >= src signed
+BPF_CALL 0x80 0x0 call helper function by address see `Helper functions`_
+BPF_CALL 0x80 0x1 call PC += imm see `Program-local functions`_
+BPF_CALL 0x80 0x2 call helper function by BTF ID see `Helper functions`_
+BPF_EXIT 0x90 0x0 return BPF_JMP only
+BPF_JLT 0xa0 any PC += offset if dst < src unsigned
+BPF_JLE 0xb0 any PC += offset if dst <= src unsigned
+BPF_JSLT 0xc0 any PC += offset if dst < src signed
+BPF_JSLE 0xd0 any PC += offset if dst <= src signed
======== ===== === =========================================== =========================================
The BPF program needs to store the return value into register R0 before doing a
-- 
2.42.0

[Bpf] [PATCH] update the consistency issue in documentation

[Aoyang Fang (SSE, 222010547)]

>From fa9f3f47ddeb3e9a615c17aea57d2ecd53a7d201 Mon Sep 17 00:00:00 2001
From: lincyawer <53161583+Lincyaw@users.noreply.github.com>
Date: Thu, 4 Jan 2024 10:51:36 +0800
Subject: [PATCH] The original documentation of BPF_JMP instructions is somehow
misleading. The code part of instruction, e.g., BPF_JEQ's value is noted as
0x1, however, in `include/uapi/linux/bpf.h`, the value of BPF_JEQ is 0x10. At
the same time, the description convention is inconsistent with the BPF_ALU,
whose code are also 4bit, but the value of BPF_ADD is 0x00

Signed-off-by: lincyawer <53161583+Lincyaw@users.noreply.github.com>
---
.../bpf/standardization/instruction-set.rst | 34 +++++++++----------
1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
index 245b6defc..dee3b1fa8 100644
--- a/Documentation/bpf/standardization/instruction-set.rst
+++ b/Documentation/bpf/standardization/instruction-set.rst
@@ -355,23 +355,23 @@ The 'code' field encodes the operation as below:
======== ===== === =========================================== =========================================
code value src description notes
======== ===== === =========================================== =========================================
-BPF_JA 0x0 0x0 PC += offset BPF_JMP class
-BPF_JA 0x0 0x0 PC += imm BPF_JMP32 class
-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`_
-BPF_CALL 0x8 0x1 call PC += imm 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
+BPF_JA 0x00 0x0 PC += offset BPF_JMP class
+BPF_JA 0x00 0x0 PC += imm BPF_JMP32 class
+BPF_JEQ 0x10 any PC += offset if dst == src
+BPF_JGT 0x20 any PC += offset if dst > src unsigned
+BPF_JGE 0x30 any PC += offset if dst >= src unsigned
+BPF_JSET 0x40 any PC += offset if dst & src
+BPF_JNE 0x50 any PC += offset if dst != src
+BPF_JSGT 0x60 any PC += offset if dst > src signed
+BPF_JSGE 0x70 any PC += offset if dst >= src signed
+BPF_CALL 0x80 0x0 call helper function by address see `Helper functions`_
+BPF_CALL 0x80 0x1 call PC += imm see `Program-local functions`_
+BPF_CALL 0x80 0x2 call helper function by BTF ID see `Helper functions`_
+BPF_EXIT 0x90 0x0 return BPF_JMP only
+BPF_JLT 0xa0 any PC += offset if dst < src unsigned
+BPF_JLE 0xb0 any PC += offset if dst <= src unsigned
+BPF_JSLT 0xc0 any PC += offset if dst < src signed
+BPF_JSLE 0xd0 any PC += offset if dst <= src signed
======== ===== === =========================================== =========================================
The BPF program needs to store the return value into register R0 before doing a
-- 
2.42.0
-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [PATCH] update the consistency issue in documentation

[dthaler1968]

> -----Original Message-----
> From: Aoyang Fang (SSE, 222010547) <aoyangfang@link.cuhk.edu.cn>
> Sent: Wednesday, January 3, 2024 7:13 PM
> To: bpf@vger.kernel.org; bpf@ietf.org
> Cc: void@manifault.com
> Subject: [PATCH] update the consistency issue in documentation
> 
> From fa9f3f47ddeb3e9a615c17aea57d2ecd53a7d201 Mon Sep 17 00:00:00
> 2001
> From: lincyawer <53161583+Lincyaw@users.noreply.github.com>
> Date: Thu, 4 Jan 2024 10:51:36 +0800
> Subject: [PATCH] The original documentation of BPF_JMP instructions is
> somehow misleading. The code part of instruction, e.g., BPF_JEQ's value is
> noted as 0x1, however, in `include/uapi/linux/bpf.h`, the value of BPF_JEQ
is
> 0x10. At the same time, the description convention is inconsistent with
the
> BPF_ALU, whose code are also 4bit, but the value of BPF_ADD is 0x00

https://www.ietf.org/archive/id/draft-ietf-bpf-isa-00.html#section-3 says:
> ==============  ======  =================
> 4 bits (MSB)    1 bit   3 bits (LSB)
> ==============  ======  =================
> code            source  instruction class
> ==============  ======  =================

Hence the code field is 4 bits, and 0x10 cannot fit in a 4 bit field.
The documentation is already internally consistent, and this proposed
change would make the documentation incorrect.

Dave

Re: [Bpf] [PATCH] update the consistency issue in documentation

[dthaler1968=40googlemail.com]

> -----Original Message-----
> From: Aoyang Fang (SSE, 222010547) <aoyangfang@link.cuhk.edu.cn>
> Sent: Wednesday, January 3, 2024 7:13 PM
> To: bpf@vger.kernel.org; bpf@ietf.org
> Cc: void@manifault.com
> Subject: [PATCH] update the consistency issue in documentation
> 
> From fa9f3f47ddeb3e9a615c17aea57d2ecd53a7d201 Mon Sep 17 00:00:00
> 2001
> From: lincyawer <53161583+Lincyaw@users.noreply.github.com>
> Date: Thu, 4 Jan 2024 10:51:36 +0800
> Subject: [PATCH] The original documentation of BPF_JMP instructions is
> somehow misleading. The code part of instruction, e.g., BPF_JEQ's value is
> noted as 0x1, however, in `include/uapi/linux/bpf.h`, the value of BPF_JEQ
is
> 0x10. At the same time, the description convention is inconsistent with
the
> BPF_ALU, whose code are also 4bit, but the value of BPF_ADD is 0x00

https://www.ietf.org/archive/id/draft-ietf-bpf-isa-00.html#section-3 says:
> ==============  ======  =================
> 4 bits (MSB)    1 bit   3 bits (LSB)
> ==============  ======  =================
> code            source  instruction class
> ==============  ======  =================

Hence the code field is 4 bits, and 0x10 cannot fit in a 4 bit field.
The documentation is already internally consistent, and this proposed
change would make the documentation incorrect.

Dave

-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

RE: [PATCH] update the consistency issue in documentation

[dthaler1968]

Aoyang Fang (SSE, 222010547) <aoyangfang@link.cuhk.edu.cn> wrote: 
> If so, the value of arithmetic instructions’ code should be 4 bit, rather than
> BPF_ADD: 0x00, BPF_SUB: 0x10, BPF_MUL: 0x20. Otherwise the convention of
> arithmetic instruction is not consistent with the convention of jump
> instructions.

Good point, you are right that section 3.1 (Arithmetic instructions) and 3.3 (Jump instructions)
are not consistent with each other.  Since 'code' is defined in section 3 as a 4-bit field,
I agree that it would be more consistent to change section 3.1 rather than defining
8-bit values for a 4-bit field.

Dave
 

Re: [Bpf] [PATCH] update the consistency issue in documentation

[dthaler1968=40googlemail.com]

Aoyang Fang (SSE, 222010547) <aoyangfang@link.cuhk.edu.cn> wrote: 
> If so, the value of arithmetic instructions’ code should be 4 bit, rather than
> BPF_ADD: 0x00, BPF_SUB: 0x10, BPF_MUL: 0x20. Otherwise the convention of
> arithmetic instruction is not consistent with the convention of jump
> instructions.

Good point, you are right that section 3.1 (Arithmetic instructions) and 3.3 (Jump instructions)
are not consistent with each other.  Since 'code' is defined in section 3 as a 4-bit field,
I agree that it would be more consistent to change section 3.1 rather than defining
8-bit values for a 4-bit field.

Dave
 


-- 
Bpf mailing list
Bpf@ietf.org
https://www.ietf.org/mailman/listinfo/bpf

Re: [PATCH] update the consistency issue in documentation

[David Vernet]

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

On Thu, Jan 04, 2024 at 03:09:32AM +0000, Aoyang Fang (SSE, 222010547) wrote:
> From fa9f3f47ddeb3e9a615c17aea57d2ecd53a7d201 Mon Sep 17 00:00:00 2001
> From: lincyawer <53161583+Lincyaw@users.noreply.github.com>
> Date: Thu, 4 Jan 2024 10:51:36 +0800
> Subject: [PATCH] The original documentation of BPF_JMP instructions is somehow
> misleading. The code part of instruction, e.g., BPF_JEQ's value is noted as
> 0x1, however, in `include/uapi/linux/bpf.h`, the value of BPF_JEQ is 0x10. At
> the same time, the description convention is inconsistent with the BPF_ALU,
> whose code are also 4bit, but the value of BPF_ADD is 0x00
> 
> Signed-off-by: lincyawer <53161583+Lincyaw@users.noreply.github.com>

Hi Aoyang,

Could you please resend this patch to the lists in plain text? The Linux
kernel mailing lists will drop html-encoded emails. Please see [0] for
more information. You can just use git send-email with a patch file as
follows:

$ git format-patch HEAD~ --subject-prefix='PATCH bpf-next' HEAD~
$ git send-email <patch> --to bpf@vger.kernel.org --cc bpf@ietf.org

[0]: https://docs.kernel.org/process/submitting-patches.html#no-mime-no-links-no-compression-no-attachments-just-plain-text

Please make sure that the Signed-off-by tag includes your name and real
email address as well.

Thanks,
David

> ---
> .../bpf/standardization/instruction-set.rst | 34 +++++++++----------
> 1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/Documentation/bpf/standardization/instruction-set.rst b/Documentation/bpf/standardization/instruction-set.rst
> index 245b6defc..dee3b1fa8 100644
> --- a/Documentation/bpf/standardization/instruction-set.rst
> +++ b/Documentation/bpf/standardization/instruction-set.rst
> @@ -355,23 +355,23 @@ The 'code' field encodes the operation as below:
> ======== ===== === =========================================== =========================================
> code value src description notes
> ======== ===== === =========================================== =========================================
> -BPF_JA 0x0 0x0 PC += offset BPF_JMP class
> -BPF_JA 0x0 0x0 PC += imm BPF_JMP32 class
> -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`_
> -BPF_CALL 0x8 0x1 call PC += imm 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
> +BPF_JA 0x00 0x0 PC += offset BPF_JMP class
> +BPF_JA 0x00 0x0 PC += imm BPF_JMP32 class
> +BPF_JEQ 0x10 any PC += offset if dst == src
> +BPF_JGT 0x20 any PC += offset if dst > src unsigned
> +BPF_JGE 0x30 any PC += offset if dst >= src unsigned
> +BPF_JSET 0x40 any PC += offset if dst & src
> +BPF_JNE 0x50 any PC += offset if dst != src
> +BPF_JSGT 0x60 any PC += offset if dst > src signed
> +BPF_JSGE 0x70 any PC += offset if dst >= src signed
> +BPF_CALL 0x80 0x0 call helper function by address see `Helper functions`_
> +BPF_CALL 0x80 0x1 call PC += imm see `Program-local functions`_
> +BPF_CALL 0x80 0x2 call helper function by BTF ID see `Helper functions`_
> +BPF_EXIT 0x90 0x0 return BPF_JMP only
> +BPF_JLT 0xa0 any PC += offset if dst < src unsigned
> +BPF_JLE 0xb0 any PC += offset if dst <= src unsigned
> +BPF_JSLT 0xc0 any PC += offset if dst < src signed
> +BPF_JSLE 0xd0 any PC += offset if dst <= src signed
> ======== ===== === =========================================== =========================================
> The BPF program needs to store the return value into register R0 before doing a
> --
> 2.42.0
> 
> 

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