netdev - Re: [PATCH 3/4] bpf, docs: Generate nicer tables for instruction encodings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211231004324.wvfqqgntnpswhzby@ast-mbp>
Date:   Thu, 30 Dec 2021 16:43:24 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Christoph Hellwig <hch@....de>
Cc:     Jonathan Corbet <corbet@....net>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>, linux-doc@...r.kernel.org,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH 3/4] bpf, docs: Generate nicer tables for instruction
 encodings

On Thu, Dec 23, 2021 at 11:19:05AM +0100, Christoph Hellwig wrote:
>  
> +For class BPF_ALU or BPF_ALU64:
> +
> +  ========  =====  =========================
> +  code      value  description
> +  ========  =====  =========================
>    BPF_ADD   0x00
>    BPF_SUB   0x10
>    BPF_MUL   0x20
> @@ -68,26 +76,31 @@ If BPF_CLASS(code) == BPF_ALU or BPF_ALU64 BPF_OP(code) is one of::
>    BPF_NEG   0x80
>    BPF_MOD   0x90
>    BPF_XOR   0xa0
> -  BPF_MOV   0xb0  /* mov reg to reg */
> -  BPF_ARSH  0xc0  /* sign extending shift right */
> -  BPF_END   0xd0  /* endianness conversion */
> +  BPF_MOV   0xb0   mov reg to reg
> +  BPF_ARSH  0xc0   sign extending shift right
> +  BPF_END   0xd0   endianness conversion
> +  ========  =====  =========================
>  
> -If BPF_CLASS(code) == BPF_JMP or BPF_JMP32 BPF_OP(code) is one of::
> +For class BPF_JMP or BPF_JMP32:
>  
> -  BPF_JA    0x00  /* BPF_JMP only */
> +  ========  =====  =========================
> +  code      value  description
> +  ========  =====  =========================
> +  BPF_JA    0x00   BPF_JMP only
>    BPF_JEQ   0x10
>    BPF_JGT   0x20
>    BPF_JGE   0x30
>    BPF_JSET  0x40

Not your fault, but the new table looks odd with
only some opcodes documented.
Same issue with BPF_ALU table.
In the past the documented opcodes were for eBPF only and
not documented in both, so it wasn't that bad.
At least there was a reason for discrepancy.
Now it just odd.
May be add a comment to all rows?

> -  BPF_JNE   0x50  /* jump != */
> -  BPF_JSGT  0x60  /* signed '>' */
> -  BPF_JSGE  0x70  /* signed '>=' */
> -  BPF_CALL  0x80  /* function call */
> -  BPF_EXIT  0x90  /*  function return */
> -  BPF_JLT   0xa0  /* unsigned '<' */
> -  BPF_JLE   0xb0  /* unsigned '<=' */
> -  BPF_JSLT  0xc0  /* signed '<' */
> -  BPF_JSLE  0xd0  /* signed '<=' */
> +  BPF_JNE   0x50   jump '!='
> +  BPF_JSGT  0x60   signed '>'
> +  BPF_JSGE  0x70   signed '>='
> +  BPF_CALL  0x80   function call
> +  BPF_EXIT  0x90   function return
> +  BPF_JLT   0xa0   unsigned '<'
> +  BPF_JLE   0xb0   unsigned '<='
> +  BPF_JSLT  0xc0   signed '<'
> +  BPF_JSLE  0xd0   signed '<='
> +  ========  =====  =========================
>  
>  So BPF_ADD | BPF_X | BPF_ALU means::
>  
> @@ -108,37 +121,58 @@ the return value into register R0 before doing a BPF_EXIT. Class 6 is used as
>  BPF_JMP32 to mean exactly the same operations as BPF_JMP, but with 32-bit wide
>  operands for the comparisons instead.
>  
> -For load and store instructions the 8-bit 'code' field is divided as::
>  
> -  +--------+--------+-------------------+
> -  | 3 bits | 2 bits |   3 bits          |
> -  |  mode  |  size  | instruction class |
> -  +--------+--------+-------------------+
> -  (MSB)                             (LSB)
> +Load and store instructions
> +===========================
> +
> +For load and store instructions (BPF_LD, BPF_LDX, BPF_ST and BPF_STX), the
> +8-bit 'opcode' field is divided as:
> +
> +  ============  ======  =================
> +  3 bits (MSB)  2 bits  3 bits (LSB)
> +  ============  ======  =================
> +  mode          size    instruction class
> +  ============  ======  =================
> +
> +The size modifier is one of:
>  
> -Size modifier is one of ...
> +  =============  =====  =====================
> +  size modifier  value  description
> +  =============  =====  =====================
> +  BPF_W          0x00   word        (4 bytes)
> +  BPF_H          0x08   half word   (2 bytes)
> +  BPF_B          0x10   byte
> +  BPF_DW         0x18   double word (8 bytes)
> +  =============  =====  =====================
>  
> -::
> +The mode modifier is one of:
>  
> -  BPF_W   0x00    /* word */
> -  BPF_H   0x08    /* half word */
> -  BPF_B   0x10    /* byte */
> -  BPF_DW  0x18    /* double word */
> +  =============  =====  =====================
> +  mode modifier  value  description
> +  =============  =====  =====================
> +  BPF_IMM        0x00   used for 64-bit mov
> +  BPF_ABS        0x20
> +  BPF_IND        0x40
> +  BPF_MEM        0x60

May be say here that ABS and IND are legacy for compat with classic only?
and MEM is the most common modifier for load/store?

> +  BPF_ATOMIC     0xc0   atomic operations 

I removed trailing space in above line.

And applied all patches to bpf-next. Thanks!