ISA: verify which opcodes had legacy instructions

[dthaler]

Remove any from the opcode table that were actually never defined

[dthaler]

https://elixir.bootlin.com/linux/v6.8-rc1/source/samples/bpf/bpf_insn.h#L111 has

/ Direct packet access, R0 = (uint ) (skb->data + imm32) /

define BPF_LD_ABS(SIZE, IMM) \

((struct [bpf_insn](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/bpf_insn)) {                  \
    .[code](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/code)  = [BPF_LD](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/BPF_LD) | [BPF_SIZE](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/BPF_SIZE)([SIZE](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/SIZE)) | [BPF_ABS](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/BPF_ABS),  \
    .[dst_reg](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/dst_reg) = 0,                  \
    .[src_reg](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/src_reg) = 0,                  \
    .[off](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/off)   = 0,                    \
    .[imm](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/imm)   = [IMM](https://elixir.bootlin.com/linux/v6.8-rc1/C/ident/IMM) })

and no use of BPF_IND.

[dthaler]

BPF_MSH (0xa0) is defined and used by missing from the doc. Likely it should also be in the legacy set.

https://man.freebsd.org/cgi/man.cgi?query=bpf&sektion=4&manpath=FreeBSD+4.5-RELEASE and https://opensource.apple.com/source/xnu/xnu-1504.9.17/bsd/man/man4/bpf.4.auto.html and https://docs.oracle.com/cd/E36784_01/html/E36884/bpf-7d.html show BPF_LDX | BPF_B | BPF_MSH

https://github.com/seccomp/libseccomp disasm uses BPF_MSH according to a google search.

[dthaler]

Posted question to list: https://mailarchive.ietf.org/arch/msg/bpf/zTzeo7zt4NxmLFMj2HNd25k8RYc/

[dthaler]

Jose E. Marchesi [jose.marchesi@oracle.com](mailto:jose.marchesi@oracle.com) wrote:

These we support:

/ Absolute load instructions, designed to be used in socket filters. / {BPF_INSN_LDABSB, "ldabsb%W%i32", "r0 = ( u8 ) skb [ %i32 ]", BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_B|BPF_MODE_ABS}, {BPF_INSN_LDABSH, "ldabsh%W%i32", "r0 = ( u16 ) skb [ %i32 ]", BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_H|BPF_MODE_ABS}, {BPF_INSN_LDABSW, "ldabsw%W%i32", "r0 = ( u32 ) skb [ %i32 ]", BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_W|BPF_MODE_ABS}, {BPF_INSN_LDABSDW, "ldabsdw%W%i32", "r0 = ( u64 ) skb [ %i32 ]", BPF_V1, BPF_CODE, BPF_CLASS_LD|BPF_SIZE_DW|BPF_MODE_ABS},

/ Generic load instructions (to register.) / {BPF_INSN_LDXB, "ldxb%W%dr , [ %sr %o16 ]", "%dr = ( u8 ) ( %sr %o16 )", BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_B|BPF_MODE_MEM}, {BPF_INSN_LDXH, "ldxh%W%dr , [ %sr %o16 ]", "%dr = ( u16 ) ( %sr %o16 )", BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_H|BPF_MODE_MEM}, {BPF_INSN_LDXW, "ldxw%W%dr , [ %sr %o16 ]", "%dr = ( u32 ) ( %sr %o16 )", BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_W|BPF_MODE_MEM}, {BPF_INSN_LDXDW, "ldxdw%W%dr , [ %sr %o16 ]","%dr = ( u64 ) ( %sr %o16 )", BPF_V1, BPF_CODE, BPF_CLASS_LDX|BPF_SIZE_DW|BPF_MODE_MEM},

Yonghong Song [yonghong.song@linux.dev](mailto:yonghong.song@linux.dev) wrote:

I don't know how to do proper wording in the standard. But DW and LDX variants of BPF_IND/BPF_ABS are not supported by verifier for now and they are considered illegal insns.

Although the Linux verifier doesn't support them, the fact that gcc does support them tells me that it's probably safest to list the DW and LDX variants as deprecated as well, which is what the draft already did in the appendix so that's good (nothing to change there, I think).

[dthaler]

For MSH, Yonghong wrote:

From kernel source code (net/core/filter.c), the only supported format is BPF_LDX | BPF_MSH | BPF_B

The insn (BPF_LDX | BPF_MSH | BPF_B) is only used when cBPF (classic BPF) is converted to BPF insn set. If the current BPF program has this insn, verifier will reject it and bpf kernel interpreter does not support this insn either. So technically, (BPF_LDX | BPF_MSH | BPF_B) is not supported by BPF program.

[dthaler]

Alexei wrote:

DW never existed in classic bpf, so abs/ind never had DW flavor. If some assembler/compiler decided to "support" them it's on them. The standard must not list such things as deprecated. They never existed. So nothing is deprecated. Same with MSH. BPF_LDX | BPF_MSH | BPF_B is the only insn ever existed. It's a legacy insn. Just like abs/ind.

Dave asked:

Should it be listed in the legacy conformance group then?

Currently it's not mentioned in instruction-set.rst at all, so the opcode is available to use by any new instruction. If we do list it in instruction-set.rst then, like abs/ind, it will be avoided by anyone proposing new instructions.

Alexei responded:

Yeah. The standard needs to mention it as legacy insn. It's such a weird ultra specialized insn introduced for one specific purpose parsing IP header. tcpdump only. Effectively.

[dthaler]

Here's my understanding of this thread so far:

• (IND/ABS) | (W/H/B) | LD : these are accepted by the Linux verifier and are supported by clang and gcc. They should be in the legacy conformance group of deprecated instructions.

• (IND/ABS) | DW | (LD/LDX) : these are not accepted by the Linux verifier and were never used. Clang doesn't generate them but gcc did which is now removed based on this discussion. They should NOT be in the legacy conformance group of deprecated instructions because they were never defined in the first place, and instruction-set.rst should be updated to clarify this.

• (IND/ABS) | (W/H/B) | LDX : these are not accepted by the Linux verifier and were never used. Clang doesn't generate them but gcc does. They should NOT be in the legacy conformance group of deprecated instructions because they were never defined in the first place, and instruction-set.rst should be updated to clarify this.

• (IND/ABS) | (W/H/B/DW) | (ST/STX): these are not accepted by the Linux verifier and were never used. I don't know whether clang or gcc generates them. They should NOT be in the legacy conformance group of deprecated instructions because they were never defined in the first place, and instruction-set.rst should be updated to clarify this.

• MSH | B | LDX: this existed in classic BPF but does not exist in (e)BPF since it is not accepted by the Linux verifier. I don't know whether clang ever generated them, but gcc never did. The "Legacy BPF Packet access instructions" section of instruction-set.rst says

  BPF previously introduced special instructions for access to packet data that were carried over from classic BPF. However, these instructions are deprecated and should no longer be used. It should be mentioned in instruction-set.rst, pointing to the above section, like IND/ABS do.

• MSH | (W/H/DW) | (LD/ST/STX): These are not accepted by the Linux verifier and were never used. They should NOT be in the legacy conformance group of deprecated instructions because they were never defined in the first place.

[dthaler]

Proposed change posted to https://mailarchive.ietf.org/arch/msg/bpf/0CrGSI3NZb4q_0VpdKvVH0mtBDc/

[dthaler]

Proposed change has been merged into bpf-next

[dthaler]

Fixed in draft -01